Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

Commit

Permalink
Update DDL test harness to use backendTextures to back tiles
Browse files Browse the repository at this point in the history
This better matches Chrome's use of DDLs.

With path, image, and text draws stripped out, here is the perf impact of this change:

           before CL   after CL
w/ DDLs      7.792      1.038
w/o DDLs     0.800      0.876

This perf improvement (in the DDL case) is from backend texture wrapping SkSurfaces being created w/o initialization. The prior method of SkSurface creation was resulting in double clearing of all the surfaces.

This perf improvement won't be seen by Chrome since they've always being using wrapped backend texture SkSurfaces.

TBR=bsalomon@google.com
Change-Id: Ice3993ca125fce37804e58c353c265cf659dbe2f
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/283456
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>
  • Loading branch information
rphilli authored and Skia Commit-Bot committed Apr 15, 2020
1 parent 5ed3c2f commit 7ae9d2f
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 25 deletions.
10 changes: 9 additions & 1 deletion dm/DMSrcSink.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1662,6 +1662,8 @@ Result GPUDDLSink::ddlDraw(const Src& src,
constexpr int kNumDivisions = 3;
DDLTileHelper tiles(dstSurface, dstCharacterization, viewport, kNumDivisions);

tiles.createBackendTextures(gpuTaskGroup, gpuThreadCtx);

// Reinflate the compressed picture individually for each thread.
tiles.createSKPPerTile(compressedPictureData.get(), promiseImageHelper);

Expand All @@ -1677,6 +1679,8 @@ Result GPUDDLSink::ddlDraw(const Src& src,
// It is simpler to also delete them at this point on the gpuThread.
promiseImageHelper.deleteAllFromGPU(gpuTaskGroup, gpuThreadCtx);

tiles.deleteBackendTextures(gpuTaskGroup, gpuThreadCtx);

// A flush has already been scheduled on the gpu thread along with the clean up of the backend
// textures so it is safe to schedule making 'mainCtx' not current on the gpuThread.
gpuTaskGroup->add([gpuTestCtx] { gpuTestCtx->makeNotCurrent(); });
Expand Down Expand Up @@ -2098,6 +2102,8 @@ Result ViaDDL::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkStrin
// First, create all the tiles (including their individual dest surfaces)
DDLTileHelper tiles(dstSurface, dstCharacterization, viewport, fNumDivisions);

tiles.createBackendTextures(nullptr, context);

// Second, reinflate the compressed picture individually for each thread
// This recreates the promise SkImages on each replay iteration. We are currently
// relying on this to test using a SkPromiseImageTexture to fulfill different
Expand All @@ -2121,8 +2127,10 @@ Result ViaDDL::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkStrin
// Finally, compose the drawn tiles into the result
// Note: the separation between the tiles and the final composition better
// matches Chrome but costs us a copy
tiles.composeAllTiles();
tiles.composeAllTiles(context);
context->flush();

tiles.deleteBackendTextures(nullptr, context);
}
return Result::Ok();
};
Expand Down
6 changes: 5 additions & 1 deletion include/core/SkDeferredDisplayList.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ class SkDeferredDisplayList {
public:
// Upon being replayed - this field will be filled in (by the DrawingManager) with the
// proxy backing the destination SkSurface. Note that, since there is no good place to
// clear it, it can become a dangling pointer.
// clear it, it can become a dangling pointer. Additionally, since the renderTargetProxy
// doesn't get a ref here, the SkSurface that owns it must remain alive until the DDL
// is flushed.
// TODO: the drawing manager could ref the renderTargetProxy for the DDL and then add
// a renderingTask to unref it after the DDL's ops have been executed.
GrRenderTargetProxy* fReplayDest = nullptr;
#endif
};
Expand Down
120 changes: 101 additions & 19 deletions tools/DDLTileHelper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ void DDLTileHelper::TileData::init(int id,

fCharacterization = dstSurfaceCharacterization.createResized(clip.width(), clip.height());
SkASSERT(fCharacterization.isValid());
SkASSERT(!fBackendTexture.isValid());
}

DDLTileHelper::TileData::~TileData() {}
Expand Down Expand Up @@ -91,51 +92,100 @@ void DDLTileHelper::TileData::precompile(GrContext* context) {
}
}

sk_sp<SkSurface> DDLTileHelper::TileData::makeWrappedTileDest(GrContext* context) {
if (!fBackendTexture.isValid()) {
return nullptr;
}

return SkSurface::MakeFromBackendTexture(context,
fBackendTexture,
fCharacterization.origin(),
fCharacterization.sampleCount(),
fCharacterization.colorType(),
fCharacterization.refColorSpace(),
&fCharacterization.surfaceProps());
}

void DDLTileHelper::TileData::drawSKPDirectly(GrContext* context) {
SkASSERT(!fDisplayList && !fImage && fReconstitutedPicture);
SkASSERT(!fDisplayList && !fTileSurface && fReconstitutedPicture);

sk_sp<SkSurface> tileSurface = SkSurface::MakeRenderTarget(context, fCharacterization,
SkBudgeted::kYes);
if (tileSurface) {
SkCanvas* tileCanvas = tileSurface->getCanvas();
fTileSurface = this->makeWrappedTileDest(context);
if (fTileSurface) {
SkCanvas* tileCanvas = fTileSurface->getCanvas();

tileCanvas->clipRect(SkRect::MakeWH(fClip.width(), fClip.height()));
tileCanvas->translate(-fClip.fLeft, -fClip.fTop);

tileCanvas->drawPicture(fReconstitutedPicture);

fImage = tileSurface->makeImageSnapshot();
// We can't snap an image here bc, since we're using wrapped backend textures for the
// surfaces, that would incur a copy.
}
}

void DDLTileHelper::TileData::draw(GrContext* context) {
SkASSERT(fDisplayList && !fImage);
SkASSERT(fDisplayList && !fTileSurface);

sk_sp<SkSurface> tileSurface = SkSurface::MakeRenderTarget(context, fCharacterization,
SkBudgeted::kYes);
if (tileSurface) {
tileSurface->draw(fDisplayList.get());
// The tile's surface needs to be held until after the DDL is flushed
fTileSurface = this->makeWrappedTileDest(context);
if (fTileSurface) {
fTileSurface->draw(fDisplayList.get());

fImage = tileSurface->makeImageSnapshot();
// We can't snap an image here bc, since we're using wrapped backend textures for the
// surfaces, that would incur a copy.
}
}

// TODO: We should create a single DDL for the composition step and just add replaying it
// as the last GPU task
void DDLTileHelper::TileData::compose() {
SkASSERT(fDstSurface && fImage);
void DDLTileHelper::TileData::compose(GrContext* context) {
SkASSERT(context->priv().asDirectContext());
SkASSERT(fDstSurface);

if (!fBackendTexture.isValid()) {
return;
}

// Here we are, unfortunately, aliasing 'fBackendTexture'. It is backing both 'fTileSurface'
// and 'tmp'.
sk_sp<SkImage> tmp = SkImage::MakeFromTexture(context,
fBackendTexture,
fCharacterization.origin(),
fCharacterization.colorType(),
kPremul_SkAlphaType,
fCharacterization.refColorSpace());

SkCanvas* canvas = fDstSurface->getCanvas();
canvas->save();
canvas->clipRect(SkRect::Make(fClip));
canvas->drawImage(fImage, fClip.fLeft, fClip.fTop);
canvas->drawImage(tmp, fClip.fLeft, fClip.fTop);
canvas->restore();
}

void DDLTileHelper::TileData::reset() {
// TODO: when DDLs are re-renderable we don't need to do this
fDisplayList = nullptr;
fImage = nullptr;
fTileSurface = nullptr;
}

void DDLTileHelper::TileData::CreateBackendTexture(GrContext* context, TileData* tile) {
SkASSERT(context->priv().asDirectContext());
SkASSERT(!tile->fBackendTexture.isValid());

tile->fBackendTexture = context->createBackendTexture(tile->fCharacterization);
// TODO: it seems that, on the Linux bots, backend texture creation is failing
// a lot (skbug.com/10142)
//SkASSERT(tile->fBackendTexture.isValid());
}

void DDLTileHelper::TileData::DeleteBackendTexture(GrContext* context, TileData* tile) {
SkASSERT(context->priv().asDirectContext());
// TODO: it seems that, on the Linux bots, backend texture creation is failing
// a lot (skbug.com/10142)
//SkASSERT(tile->fBackendTexture.isValid());

tile->fTileSurface = nullptr;
context->deleteBackendTexture(tile->fBackendTexture);
}

///////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -199,7 +249,7 @@ static void do_gpu_stuff(GrContext* context, DDLTileHelper::TileData* tile) {

// TODO: we should actually have a separate DDL that does
// the final composition draw
tile->compose();
tile->compose(context);
}

// We expect to have more than one recording thread but just one gpu thread
Expand Down Expand Up @@ -246,9 +296,9 @@ void DDLTileHelper::drawAllTilesDirectly(GrContext* context) {
}
}

void DDLTileHelper::composeAllTiles() {
void DDLTileHelper::composeAllTiles(GrContext* context) {
for (int i = 0; i < this->numTiles(); ++i) {
fTiles[i].compose();
fTiles[i].compose(context);
}
}

Expand All @@ -257,3 +307,35 @@ void DDLTileHelper::resetAllTiles() {
fTiles[i].reset();
}
}

void DDLTileHelper::createBackendTextures(SkTaskGroup* taskGroup, GrContext* context) {
SkASSERT(context->priv().asDirectContext());

if (taskGroup) {
for (int i = 0; i < this->numTiles(); ++i) {
TileData* tile = &fTiles[i];

taskGroup->add([context, tile]() { TileData::CreateBackendTexture(context, tile); });
}
} else {
for (int i = 0; i < this->numTiles(); ++i) {
TileData::CreateBackendTexture(context, &fTiles[i]);
}
}
}

void DDLTileHelper::deleteBackendTextures(SkTaskGroup* taskGroup, GrContext* context) {
SkASSERT(context->priv().asDirectContext());

if (taskGroup) {
for (int i = 0; i < this->numTiles(); ++i) {
TileData* tile = &fTiles[i];

taskGroup->add([context, tile]() { TileData::DeleteBackendTexture(context, tile); });
}
} else {
for (int i = 0; i < this->numTiles(); ++i) {
TileData::DeleteBackendTexture(context, &fTiles[i]);
}
}
}
18 changes: 15 additions & 3 deletions tools/DDLTileHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,30 @@ class DDLTileHelper {

// Draw the result of replaying the DDL (i.e., 'fImage') into the
// final destination surface ('fDstSurface').
void compose();
void compose(GrContext*);

void reset();

int id() const { return fID; }

SkDeferredDisplayList* ddl() { return fDisplayList.get(); }

static void CreateBackendTexture(GrContext*, TileData*);
static void DeleteBackendTexture(GrContext*, TileData*);

private:
sk_sp<SkSurface> makeWrappedTileDest(GrContext* context);

int fID = -1;
sk_sp<SkSurface> fDstSurface; // the ultimate target for composition

GrBackendTexture fBackendTexture; // destination for this tile's content
SkSurfaceCharacterization fCharacterization; // characterization for the tile's surface
SkIRect fClip; // in the device space of the 'fDstSurface'

sk_sp<SkImage> fImage; // the result of replaying the DDL
// 'fTileSurface' wraps 'fBackendTexture' and must exist until after 'fDisplayList'
// has been flushed.
sk_sp<SkSurface> fTileSurface;
sk_sp<SkPicture> fReconstitutedPicture;
SkTArray<sk_sp<SkImage>> fPromiseImages; // All the promise images in the
// reconstituted picture
Expand Down Expand Up @@ -105,12 +114,15 @@ class DDLTileHelper {
// DDLs first - all on a single thread.
void drawAllTilesDirectly(GrContext*);

void composeAllTiles();
void composeAllTiles(GrContext*);

void resetAllTiles();

int numTiles() const { return fNumDivisions * fNumDivisions; }

void createBackendTextures(SkTaskGroup*, GrContext*);
void deleteBackendTextures(SkTaskGroup*, GrContext*);

private:
int fNumDivisions; // number of tiles along a side
TileData* fTiles; // 'fNumDivisions' x 'fNumDivisions'
Expand Down
9 changes: 8 additions & 1 deletion tools/skpbench/skpbench.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ static void run_ddl_benchmark(GrContext* context, sk_sp<SkSurface> surface,

DDLTileHelper tiles(surface, dstCharacterization, viewport, FLAGS_ddlTilingWidthHeight);

tiles.createBackendTextures(nullptr, context);

tiles.createSKPPerTile(compressedPictureData.get(), promiseImageHelper);

SkTaskGroup::Enabler enabled(FLAGS_ddlNumAdditionalThreads);
Expand All @@ -283,7 +285,7 @@ static void run_ddl_benchmark(GrContext* context, sk_sp<SkSurface> surface,

if (!FLAGS_png.isEmpty()) {
// The user wants to see the final result
tiles.composeAllTiles();
tiles.composeAllTiles(context);
}

tiles.resetAllTiles();
Expand All @@ -293,6 +295,11 @@ static void run_ddl_benchmark(GrContext* context, sk_sp<SkSurface> surface,
GrFlushInfo flushInfo;
flushInfo.fFlags = kSyncCpu_GrFlushFlag;
context->flush(flushInfo);

promiseImageHelper.deleteAllFromGPU(nullptr, context);

tiles.deleteBackendTextures(nullptr, context);

}

static void run_benchmark(GrContext* context, SkSurface* surface, SkpProducer* skpp,
Expand Down

0 comments on commit 7ae9d2f

Please sign in to comment.