Skip to content

Commit 9ff1d84

Browse files
rphilliSkia Commit-Bot
rphilli
authored and
Skia Commit-Bot
committed
Revert "Update DDL test harness to use backendTextures to back tiles"
This reverts commit 7ae9d2f. Reason for revert: Triggering Vulkan Debug layer errors Original change's description: > Update DDL test harness to use backendTextures to back tiles > > 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> TBR=egdaniel@google.com,bsalomon@google.com,robertphillips@google.com Change-Id: Ife023ede0774ec2cce4c0d6e7708c036347ebf54 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://skia-review.googlesource.com/c/skia/+/283648 Reviewed-by: Robert Phillips <robertphillips@google.com> Commit-Queue: Robert Phillips <robertphillips@google.com>
1 parent 7ae9d2f commit 9ff1d84

File tree

5 files changed

+25
-138
lines changed

5 files changed

+25
-138
lines changed

‎dm/DMSrcSink.cpp‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1662,8 +1662,6 @@ Result GPUDDLSink::ddlDraw(const Src& src,
16621662
constexpr int kNumDivisions = 3;
16631663
DDLTileHelper tiles(dstSurface, dstCharacterization, viewport, kNumDivisions);
16641664

1665-
tiles.createBackendTextures(gpuTaskGroup, gpuThreadCtx);
1666-
16671665
// Reinflate the compressed picture individually for each thread.
16681666
tiles.createSKPPerTile(compressedPictureData.get(), promiseImageHelper);
16691667

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

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

2105-
tiles.createBackendTextures(nullptr, context);
2106-
21072101
// Second, reinflate the compressed picture individually for each thread
21082102
// This recreates the promise SkImages on each replay iteration. We are currently
21092103
// relying on this to test using a SkPromiseImageTexture to fulfill different
@@ -2127,10 +2121,8 @@ Result ViaDDL::draw(const Src& src, SkBitmap* bitmap, SkWStream* stream, SkStrin
21272121
// Finally, compose the drawn tiles into the result
21282122
// Note: the separation between the tiles and the final composition better
21292123
// matches Chrome but costs us a copy
2130-
tiles.composeAllTiles(context);
2124+
tiles.composeAllTiles();
21312125
context->flush();
2132-
2133-
tiles.deleteBackendTextures(nullptr, context);
21342126
}
21352127
return Result::Ok();
21362128
};

‎include/core/SkDeferredDisplayList.h‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,7 @@ class SkDeferredDisplayList {
7272
public:
7373
// Upon being replayed - this field will be filled in (by the DrawingManager) with the
7474
// proxy backing the destination SkSurface. Note that, since there is no good place to
75-
// clear it, it can become a dangling pointer. Additionally, since the renderTargetProxy
76-
// doesn't get a ref here, the SkSurface that owns it must remain alive until the DDL
77-
// is flushed.
78-
// TODO: the drawing manager could ref the renderTargetProxy for the DDL and then add
79-
// a renderingTask to unref it after the DDL's ops have been executed.
75+
// clear it, it can become a dangling pointer.
8076
GrRenderTargetProxy* fReplayDest = nullptr;
8177
#endif
8278
};

‎tools/DDLTileHelper.cpp‎

Lines changed: 19 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ void DDLTileHelper::TileData::init(int id,
2828

2929
fCharacterization = dstSurfaceCharacterization.createResized(clip.width(), clip.height());
3030
SkASSERT(fCharacterization.isValid());
31-
SkASSERT(!fBackendTexture.isValid());
3231
}
3332

3433
DDLTileHelper::TileData::~TileData() {}
@@ -92,100 +91,51 @@ void DDLTileHelper::TileData::precompile(GrContext* context) {
9291
}
9392
}
9493

95-
sk_sp<SkSurface> DDLTileHelper::TileData::makeWrappedTileDest(GrContext* context) {
96-
if (!fBackendTexture.isValid()) {
97-
return nullptr;
98-
}
99-
100-
return SkSurface::MakeFromBackendTexture(context,
101-
fBackendTexture,
102-
fCharacterization.origin(),
103-
fCharacterization.sampleCount(),
104-
fCharacterization.colorType(),
105-
fCharacterization.refColorSpace(),
106-
&fCharacterization.surfaceProps());
107-
}
108-
10994
void DDLTileHelper::TileData::drawSKPDirectly(GrContext* context) {
110-
SkASSERT(!fDisplayList && !fTileSurface && fReconstitutedPicture);
95+
SkASSERT(!fDisplayList && !fImage && fReconstitutedPicture);
11196

112-
fTileSurface = this->makeWrappedTileDest(context);
113-
if (fTileSurface) {
114-
SkCanvas* tileCanvas = fTileSurface->getCanvas();
97+
sk_sp<SkSurface> tileSurface = SkSurface::MakeRenderTarget(context, fCharacterization,
98+
SkBudgeted::kYes);
99+
if (tileSurface) {
100+
SkCanvas* tileCanvas = tileSurface->getCanvas();
115101

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

119105
tileCanvas->drawPicture(fReconstitutedPicture);
120106

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

126111
void DDLTileHelper::TileData::draw(GrContext* context) {
127-
SkASSERT(fDisplayList && !fTileSurface);
112+
SkASSERT(fDisplayList && !fImage);
128113

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

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

139123
// TODO: We should create a single DDL for the composition step and just add replaying it
140124
// as the last GPU task
141-
void DDLTileHelper::TileData::compose(GrContext* context) {
142-
SkASSERT(context->priv().asDirectContext());
143-
SkASSERT(fDstSurface);
144-
145-
if (!fBackendTexture.isValid()) {
146-
return;
147-
}
148-
149-
// Here we are, unfortunately, aliasing 'fBackendTexture'. It is backing both 'fTileSurface'
150-
// and 'tmp'.
151-
sk_sp<SkImage> tmp = SkImage::MakeFromTexture(context,
152-
fBackendTexture,
153-
fCharacterization.origin(),
154-
fCharacterization.colorType(),
155-
kPremul_SkAlphaType,
156-
fCharacterization.refColorSpace());
125+
void DDLTileHelper::TileData::compose() {
126+
SkASSERT(fDstSurface && fImage);
157127

158128
SkCanvas* canvas = fDstSurface->getCanvas();
159129
canvas->save();
160130
canvas->clipRect(SkRect::Make(fClip));
161-
canvas->drawImage(tmp, fClip.fLeft, fClip.fTop);
131+
canvas->drawImage(fImage, fClip.fLeft, fClip.fTop);
162132
canvas->restore();
163133
}
164134

165135
void DDLTileHelper::TileData::reset() {
166136
// TODO: when DDLs are re-renderable we don't need to do this
167137
fDisplayList = nullptr;
168-
fTileSurface = nullptr;
169-
}
170-
171-
void DDLTileHelper::TileData::CreateBackendTexture(GrContext* context, TileData* tile) {
172-
SkASSERT(context->priv().asDirectContext());
173-
SkASSERT(!tile->fBackendTexture.isValid());
174-
175-
tile->fBackendTexture = context->createBackendTexture(tile->fCharacterization);
176-
// TODO: it seems that, on the Linux bots, backend texture creation is failing
177-
// a lot (skbug.com/10142)
178-
//SkASSERT(tile->fBackendTexture.isValid());
179-
}
180-
181-
void DDLTileHelper::TileData::DeleteBackendTexture(GrContext* context, TileData* tile) {
182-
SkASSERT(context->priv().asDirectContext());
183-
// TODO: it seems that, on the Linux bots, backend texture creation is failing
184-
// a lot (skbug.com/10142)
185-
//SkASSERT(tile->fBackendTexture.isValid());
186-
187-
tile->fTileSurface = nullptr;
188-
context->deleteBackendTexture(tile->fBackendTexture);
138+
fImage = nullptr;
189139
}
190140

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

250200
// TODO: we should actually have a separate DDL that does
251201
// the final composition draw
252-
tile->compose(context);
202+
tile->compose();
253203
}
254204

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

299-
void DDLTileHelper::composeAllTiles(GrContext* context) {
249+
void DDLTileHelper::composeAllTiles() {
300250
for (int i = 0; i < this->numTiles(); ++i) {
301-
fTiles[i].compose(context);
251+
fTiles[i].compose();
302252
}
303253
}
304254

@@ -307,35 +257,3 @@ void DDLTileHelper::resetAllTiles() {
307257
fTiles[i].reset();
308258
}
309259
}
310-
311-
void DDLTileHelper::createBackendTextures(SkTaskGroup* taskGroup, GrContext* context) {
312-
SkASSERT(context->priv().asDirectContext());
313-
314-
if (taskGroup) {
315-
for (int i = 0; i < this->numTiles(); ++i) {
316-
TileData* tile = &fTiles[i];
317-
318-
taskGroup->add([context, tile]() { TileData::CreateBackendTexture(context, tile); });
319-
}
320-
} else {
321-
for (int i = 0; i < this->numTiles(); ++i) {
322-
TileData::CreateBackendTexture(context, &fTiles[i]);
323-
}
324-
}
325-
}
326-
327-
void DDLTileHelper::deleteBackendTextures(SkTaskGroup* taskGroup, GrContext* context) {
328-
SkASSERT(context->priv().asDirectContext());
329-
330-
if (taskGroup) {
331-
for (int i = 0; i < this->numTiles(); ++i) {
332-
TileData* tile = &fTiles[i];
333-
334-
taskGroup->add([context, tile]() { TileData::DeleteBackendTexture(context, tile); });
335-
}
336-
} else {
337-
for (int i = 0; i < this->numTiles(); ++i) {
338-
TileData::DeleteBackendTexture(context, &fTiles[i]);
339-
}
340-
}
341-
}

‎tools/DDLTileHelper.h‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -55,30 +55,21 @@ class DDLTileHelper {
5555

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

6060
void reset();
6161

6262
int id() const { return fID; }
6363

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

66-
static void CreateBackendTexture(GrContext*, TileData*);
67-
static void DeleteBackendTexture(GrContext*, TileData*);
68-
6966
private:
70-
sk_sp<SkSurface> makeWrappedTileDest(GrContext* context);
71-
7267
int fID = -1;
7368
sk_sp<SkSurface> fDstSurface; // the ultimate target for composition
74-
75-
GrBackendTexture fBackendTexture; // destination for this tile's content
7669
SkSurfaceCharacterization fCharacterization; // characterization for the tile's surface
7770
SkIRect fClip; // in the device space of the 'fDstSurface'
7871

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

117-
void composeAllTiles(GrContext*);
108+
void composeAllTiles();
118109

119110
void resetAllTiles();
120111

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

123-
void createBackendTextures(SkTaskGroup*, GrContext*);
124-
void deleteBackendTextures(SkTaskGroup*, GrContext*);
125-
126114
private:
127115
int fNumDivisions; // number of tiles along a side
128116
TileData* fTiles; // 'fNumDivisions' x 'fNumDivisions'

‎tools/skpbench/skpbench.cpp‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,8 +258,6 @@ static void run_ddl_benchmark(GrContext* context, sk_sp<SkSurface> surface,
258258

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

261-
tiles.createBackendTextures(nullptr, context);
262-
263261
tiles.createSKPPerTile(compressedPictureData.get(), promiseImageHelper);
264262

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

286284
if (!FLAGS_png.isEmpty()) {
287285
// The user wants to see the final result
288-
tiles.composeAllTiles(context);
286+
tiles.composeAllTiles();
289287
}
290288

291289
tiles.resetAllTiles();
@@ -295,11 +293,6 @@ static void run_ddl_benchmark(GrContext* context, sk_sp<SkSurface> surface,
295293
GrFlushInfo flushInfo;
296294
flushInfo.fFlags = kSyncCpu_GrFlushFlag;
297295
context->flush(flushInfo);
298-
299-
promiseImageHelper.deleteAllFromGPU(nullptr, context);
300-
301-
tiles.deleteBackendTextures(nullptr, context);
302-
303296
}
304297

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

0 commit comments

Comments
 (0)