Reland "Support sharing promise images between DDLs"
This reverts commit 38b9a4bc3e3c11e0f17545d15e714a74c10211e3.
Reason for revert: Fixed ASAN, TSAN, and other bugs via other CLs.
Original change's description:
> Revert "Support sharing promise images between DDLs"
>
> This reverts commit 07e11d48cb02ba9a3845e653c1772c25021e65a3.
>
> Reason for revert: Broke DDL3_ASAN and DDL3_TSAN
>
> Original change's description:
> > Support sharing promise images between DDLs
> >
> > - Migrate our code to SkImage::MakePromiseTexture
> > - Have DDLTileHelper share one SKP and one set of promise images across all tiles.
> > - Disallow on-the-fly allocation of mips for promise textures.
> >
> > Bug: skia:10286
> > Change-Id: Ie35976958454fc520f3c9d860e6285441260c9f7
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291938
> > Commit-Queue: Adlai Holler <adlai@google.com>
> > Reviewed-by: Robert Phillips <robertphillips@google.com>
>
> TBR=robertphillips@google.com,adlai@google.com
>
> Change-Id: I939b14875d1a20e4a92eab94680adcfe9596ad81
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: skia:10286
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375738
> Reviewed-by: Adlai Holler <adlai@google.com>
> Commit-Queue: Adlai Holler <adlai@google.com>
Bug: skia:10286
Change-Id: Ibfd7dfcd72f10a4e29a87fa8c610f2dfd018e0db
Cq-Include-Trybots: luci.skia.skia.primary:Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Debug-All-DDL3_ASAN,Test-Ubuntu18-Clang-Golo-GPU-QuadroP400-x86_64-Release-All-DDL3_TSAN
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375739
Reviewed-by: Robert Phillips <robertphillips@google.com>
Commit-Queue: Adlai Holler <adlai@google.com>
diff --git a/tools/DDLTileHelper.cpp b/tools/DDLTileHelper.cpp
index 315c8a8..ae83ca3 100644
--- a/tools/DDLTileHelper.cpp
+++ b/tools/DDLTileHelper.cpp
@@ -43,66 +43,25 @@
DDLTileHelper::TileData::TileData() {}
DDLTileHelper::TileData::~TileData() {}
-void DDLTileHelper::TileData::createTileSpecificSKP(SkData* compressedPictureData,
- const DDLPromiseImageHelper& helper) {
- if (!this->initialized()) {
- return;
- }
-
- SkASSERT(!fReconstitutedPicture);
-
- auto recordingChar = fPlaybackChar.createResized(fClip.width(), fClip.height());
- SkASSERT(recordingChar.isValid());
-
- // This is bending the DDLRecorder contract! The promise images in the SKP should be
- // created by the same recorder used to create the matching DDL.
- SkDeferredDisplayListRecorder recorder(recordingChar);
-
- fReconstitutedPicture = helper.reinflateSKP(&recorder, compressedPictureData, &fPromiseImages);
-
- auto ddl = recorder.detach();
- if (ddl && ddl->priv().numRenderTasks()) {
- // TODO: remove this once skbug.com/8424 is fixed. If the DDL resulting from the
- // reinflation of the SKPs contains opsTasks that means some image subset operation
- // created a draw.
- fReconstitutedPicture.reset();
- }
-}
-
-void DDLTileHelper::TileData::createDDL() {
- if (!this->initialized()) {
- return;
- }
-
- SkASSERT(!fDisplayList && fReconstitutedPicture);
+void DDLTileHelper::TileData::createDDL(const SkPicture* picture) {
+ SkASSERT(!fDisplayList && picture);
auto recordingChar = fPlaybackChar.createResized(fClip.width(), fClip.height());
SkASSERT(recordingChar.isValid());
SkDeferredDisplayListRecorder recorder(recordingChar);
- // DDL TODO: the DDLRecorder's GrContext isn't initialized until getCanvas is called.
+ // DDL TODO: the DDLRecorder's rContext isn't initialized until getCanvas is called.
// Maybe set it up in the ctor?
SkCanvas* recordingCanvas = recorder.getCanvas();
- // Because we cheated in createTileSpecificSKP and used the wrong DDLRecorder, the GrContext's
- // stored in fReconstitutedPicture's promise images are incorrect. Patch them with the correct
- // one now.
- for (int i = 0; i < fPromiseImages.count(); ++i) {
- if (fPromiseImages[i]->isTextureBacked()) {
- auto rContext = recordingCanvas->recordingContext();
- SkImage_GpuBase* gpuImage = (SkImage_GpuBase*) fPromiseImages[i].get();
- gpuImage->resetContext(sk_ref_sp(rContext));
- }
- }
-
// We always record the DDL in the (0,0) .. (clipWidth, clipHeight) coordinates
recordingCanvas->clipRect(SkRect::MakeWH(fClip.width(), fClip.height()));
recordingCanvas->translate(-fClip.fLeft, -fClip.fTop);
// Note: in this use case we only render a picture to the deferred canvas
// but, more generally, clients will use arbitrary draw calls.
- recordingCanvas->drawPicture(fReconstitutedPicture);
+ recordingCanvas->drawPicture(picture);
fDisplayList = recorder.detach();
}
@@ -120,7 +79,8 @@
continue;
}
- sk_sp<SkImage> promiseImage = tile->makePromiseImageForDst(&recorder);
+ sk_sp<SkImage> promiseImage = tile->makePromiseImageForDst(
+ recordingCanvas->recordingContext()->threadSafeProxy());
SkRect dstRect = SkRect::Make(tile->clipRect());
SkIRect srcRect = tile->clipRect();
@@ -150,7 +110,7 @@
}
}
-sk_sp<SkSurface> DDLTileHelper::TileData::makeWrappedTileDest(GrRecordingContext* context) {
+sk_sp<SkSurface> DDLTileHelper::TileData::makeWrappedTileDest(GrRecordingContext* rContext) {
SkASSERT(fCallbackContext && fCallbackContext->promiseImageTexture());
auto promiseImageTexture = fCallbackContext->promiseImageTexture();
@@ -161,7 +121,7 @@
// Here we are, unfortunately, aliasing the backend texture held by the SkPromiseImageTexture.
// Both the tile's destination surface and the promise image used to draw the tile will be
// backed by the same backendTexture - unbeknownst to Ganesh.
- return SkSurface::MakeFromBackendTexture(context,
+ return SkSurface::MakeFromBackendTexture(rContext,
promiseImageTexture->backendTexture(),
fPlaybackChar.origin(),
fPlaybackChar.sampleCount(),
@@ -170,10 +130,11 @@
&fPlaybackChar.surfaceProps());
}
-void DDLTileHelper::TileData::drawSKPDirectly(GrRecordingContext* context) {
- SkASSERT(!fDisplayList && !fTileSurface && fReconstitutedPicture);
+void DDLTileHelper::TileData::drawSKPDirectly(GrDirectContext* dContext,
+ const SkPicture* picture) {
+ SkASSERT(!fDisplayList && !fTileSurface && picture);
- fTileSurface = this->makeWrappedTileDest(context);
+ fTileSurface = this->makeWrappedTileDest(dContext);
if (fTileSurface) {
SkCanvas* tileCanvas = fTileSurface->getCanvas();
@@ -181,7 +142,7 @@
tileCanvas->clipRect(SkRect::MakeWH(fClip.width(), fClip.height()));
tileCanvas->translate(-fClip.fLeft, -fClip.fTop);
- tileCanvas->drawPicture(fReconstitutedPicture);
+ tileCanvas->drawPicture(picture);
// We can't snap an image here bc, since we're using wrapped backend textures for the
// surfaces, that would incur a copy.
@@ -208,22 +169,22 @@
}
sk_sp<SkImage> DDLTileHelper::TileData::makePromiseImageForDst(
- SkDeferredDisplayListRecorder* recorder) {
+ sk_sp<GrContextThreadSafeProxy> threadSafeProxy) {
SkASSERT(fCallbackContext);
// The promise image gets a ref on the promise callback context
sk_sp<SkImage> promiseImage =
- recorder->makePromiseTexture(fCallbackContext->backendFormat(),
- this->paddedRectSize().width(),
- this->paddedRectSize().height(),
- GrMipmapped::kNo,
- GrSurfaceOrigin::kBottomLeft_GrSurfaceOrigin,
- fPlaybackChar.colorType(),
- kPremul_SkAlphaType,
- fPlaybackChar.refColorSpace(),
- PromiseImageCallbackContext::PromiseImageFulfillProc,
- PromiseImageCallbackContext::PromiseImageReleaseProc,
- (void*)this->refCallbackContext().release());
+ SkImage::MakePromiseTexture(std::move(threadSafeProxy),
+ fCallbackContext->backendFormat(),
+ this->paddedRectSize(),
+ GrMipmapped::kNo,
+ GrSurfaceOrigin::kBottomLeft_GrSurfaceOrigin,
+ fPlaybackChar.colorType(),
+ kPremul_SkAlphaType,
+ fPlaybackChar.refColorSpace(),
+ PromiseImageCallbackContext::PromiseImageFulfillProc,
+ PromiseImageCallbackContext::PromiseImageReleaseProc,
+ (void*)this->refCallbackContext().release());
fCallbackContext->wasAddedToImage();
return promiseImage;
@@ -298,22 +259,26 @@
}
}
-void DDLTileHelper::createSKPPerTile(SkData* compressedPictureData,
- const DDLPromiseImageHelper& helper) {
- for (int i = 0; i < this->numTiles(); ++i) {
- fTiles[i].createTileSpecificSKP(compressedPictureData, helper);
- }
+void DDLTileHelper::createSKP(sk_sp<GrContextThreadSafeProxy> threadSafeProxy,
+ SkData* compressedPictureData,
+ const DDLPromiseImageHelper& helper) {
+ SkASSERT(!fReconstitutedPicture);
+
+ fReconstitutedPicture = helper.reinflateSKP(std::move(threadSafeProxy), compressedPictureData,
+ &fPromiseImages);
}
void DDLTileHelper::createDDLsInParallel() {
#if 1
- SkTaskGroup().batch(this->numTiles(), [&](int i) { fTiles[i].createDDL(); });
+ SkTaskGroup().batch(this->numTiles(), [&](int i) {
+ fTiles[i].createDDL(fReconstitutedPicture.get());
+ });
SkTaskGroup().add([this]{ this->createComposeDDL(); });
SkTaskGroup().wait();
#else
// Use this code path to debug w/o threads
for (int i = 0; i < this->numTiles(); ++i) {
- fTiles[i].createDDL();
+ fTiles[i].createDDL(fReconstitutedPicture.get());
}
this->createComposeDDL();
#endif
@@ -350,8 +315,8 @@
// schedule gpu-thread processing of the DDL
// Note: a finer grained approach would be add a scheduling task which would evaluate
// which DDLs were ready to be rendered based on their prerequisites
- recordingTaskGroup->add([tile, gpuTaskGroup, dContext]() {
- tile->createDDL();
+ recordingTaskGroup->add([this, tile, gpuTaskGroup, dContext]() {
+ tile->createDDL(fReconstitutedPicture.get());
gpuTaskGroup->add([dContext, tile]() {
do_gpu_stuff(dContext, tile);
@@ -365,7 +330,7 @@
// Only called from skpbench
void DDLTileHelper::interleaveDDLCreationAndDraw(GrDirectContext* direct) {
for (int i = 0; i < this->numTiles(); ++i) {
- fTiles[i].createDDL();
+ fTiles[i].createDDL(fReconstitutedPicture.get());
fTiles[i].draw(direct);
}
}
@@ -373,7 +338,7 @@
// Only called from skpbench
void DDLTileHelper::drawAllTilesDirectly(GrDirectContext* context) {
for (int i = 0; i < this->numTiles(); ++i) {
- fTiles[i].drawSKPDirectly(context);
+ fTiles[i].drawSKPDirectly(context, fReconstitutedPicture.get());
}
}