From 81f71b6630a9b7398bf983689436cccdd8dd3ff7 Mon Sep 17 00:00:00 2001 From: robertphillips Date: Tue, 11 Nov 2014 04:54:49 -0800 Subject: [PATCH] Change where layer hoisting data is gathered This CL: 1) removes the EXPERIMENTAL_optimize on SkCanvas & SkDevice 2) moves the saveLayer gathering step to endRecording 3) Replaces GPUOptimize with SkRecordComputeLayers 4) Update bench_pictures & render_pictures to provide the new flag #2 also necessitated moving the BBH computation (and record optimization) out of SkPicture's ctor (and into endRecording) Review URL: https://codereview.chromium.org/718443002 --- gm/multipicturedraw.cpp | 29 ++++++++++++++++----- include/core/SkCanvas.h | 8 ------ include/core/SkDevice.h | 6 ----- include/core/SkPictureRecorder.h | 7 ++++++ src/core/SkCanvas.cpp | 7 ------ src/core/SkDevice.cpp | 4 --- src/core/SkMultiPictureDraw.cpp | 4 --- src/core/SkPicture.cpp | 12 +-------- src/core/SkPictureRecorder.cpp | 43 +++++++++++++++++++++++++++++++- src/gpu/GrPictureUtils.cpp | 35 +++++++++----------------- src/gpu/GrPictureUtils.h | 2 -- src/gpu/SkGpuDevice.cpp | 26 ++++++------------- src/gpu/SkGpuDevice.h | 2 -- tests/PictureTest.cpp | 17 ++++++------- tools/PictureRenderer.cpp | 12 +++++++-- 15 files changed, 109 insertions(+), 105 deletions(-) diff --git a/gm/multipicturedraw.cpp b/gm/multipicturedraw.cpp index 8ce3deb763..250c28a8d9 100644 --- a/gm/multipicturedraw.cpp +++ b/gm/multipicturedraw.cpp @@ -55,9 +55,12 @@ static const SkPicture* make_hex_plane_picture(SkColor fillColor) { stroke.setStrokeWidth(3); SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; SkCanvas* canvas = recorder.beginRecording(SkIntToScalar(kPicWidth), - SkIntToScalar(kPicHeight)); + SkIntToScalar(kPicHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); SkScalar xPos, yPos = 0; @@ -102,9 +105,11 @@ static const SkPicture* make_single_layer_hex_plane_picture() { stroke.setStrokeWidth(3); SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; static const SkScalar kBig = 10000.0f; - SkCanvas* canvas = recorder.beginRecording(kBig, kBig); + SkCanvas* canvas = recorder.beginRecording(kBig, kBig, &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); canvas->saveLayer(NULL, NULL); @@ -156,9 +161,12 @@ static const SkPicture* make_tri_picture() { stroke.setStrokeWidth(3); SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; SkCanvas* canvas = recorder.beginRecording(SkIntToScalar(kPicWidth), - SkIntToScalar(kPicHeight)); + SkIntToScalar(kPicHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); SkRect r = tri.getBounds(); r.outset(2.0f, 2.0f); // outset for stroke canvas->clipRect(r); @@ -173,9 +181,12 @@ static const SkPicture* make_tri_picture() { static const SkPicture* make_sub_picture(const SkPicture* tri) { SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; SkCanvas* canvas = recorder.beginRecording(SkIntToScalar(kPicWidth), - SkIntToScalar(kPicHeight)); + SkIntToScalar(kPicHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); canvas->scale(1.0f/2.0f, 1.0f/2.0f); @@ -205,9 +216,12 @@ static const SkPicture* make_sierpinski_picture() { SkAutoTUnref pic(make_tri_picture()); SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; SkCanvas* canvas = recorder.beginRecording(SkIntToScalar(kPicWidth), - SkIntToScalar(kPicHeight)); + SkIntToScalar(kPicHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); static const int kNumLevels = 4; for (int i = 0; i < kNumLevels; ++i) { @@ -343,9 +357,12 @@ static void create_content(SkMultiPictureDraw* mpd, PFContentMtd pfGen, { SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; SkCanvas* pictureCanvas = recorder.beginRecording(SkIntToScalar(kPicWidth), - SkIntToScalar(kPicHeight)); + SkIntToScalar(kPicHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); (*pfGen)(pictureCanvas, pictures); diff --git a/include/core/SkCanvas.h b/include/core/SkCanvas.h index 4df523edef..2445c6e60d 100644 --- a/include/core/SkCanvas.h +++ b/include/core/SkCanvas.h @@ -996,14 +996,6 @@ public: */ void drawTextBlob(const SkTextBlob* blob, SkScalar x, SkScalar y, const SkPaint& paint); - /** PRIVATE / EXPERIMENTAL -- do not call - Perform back-end analysis/optimization of a picture. This may attach - optimization data to the picture which can be used by a later - drawPicture call. - @param picture The recorded drawing commands to analyze/optimize - */ - void EXPERIMENTAL_optimize(const SkPicture* picture); - /** Draw the picture into this canvas. This method effective brackets the playback of the picture's draw calls with save/restore, so the state of this canvas will be unchanged after this call. diff --git a/include/core/SkDevice.h b/include/core/SkDevice.h index 5984681622..4a61321ff7 100644 --- a/include/core/SkDevice.h +++ b/include/core/SkDevice.h @@ -325,12 +325,6 @@ protected: return *fLeakyProperties; } - /** - * PRIVATE / EXPERIMENTAL -- do not call - * Construct an acceleration object and attach it to 'picture' - */ - virtual void EXPERIMENTAL_optimize(const SkPicture* picture); - /** * PRIVATE / EXPERIMENTAL -- do not call * This entry point gives the backend an opportunity to take over the rendering diff --git a/include/core/SkPictureRecorder.h b/include/core/SkPictureRecorder.h index c48f35de6d..de216b4e5b 100644 --- a/include/core/SkPictureRecorder.h +++ b/include/core/SkPictureRecorder.h @@ -37,6 +37,12 @@ public: } #endif + enum RecordFlags { + // This flag indicates that, if some BHH is being computed, saveLayer + // information should also be extracted at the same time. + kComputeSaveLayerInfo_RecordFlag = 0x01 + }; + /** Returns the canvas that records the drawing commands. @param width the width of the cull rect used when recording this picture. @param height the height of the cull rect used when recording this picture. @@ -72,6 +78,7 @@ private: friend class SkPictureRecorderReplayTester; // for unit testing void partialReplay(SkCanvas* canvas) const; + uint32_t fFlags; SkScalar fCullWidth; SkScalar fCullHeight; SkAutoTUnref fBBH; diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index 70b705e1e0..2d9b55929b 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -2430,13 +2430,6 @@ void SkCanvas::drawTextOnPathHV(const void* text, size_t byteLength, } /////////////////////////////////////////////////////////////////////////////// -void SkCanvas::EXPERIMENTAL_optimize(const SkPicture* picture) { - SkBaseDevice* device = this->getDevice(); - if (device) { - device->EXPERIMENTAL_optimize(picture); - } -} - void SkCanvas::drawPicture(const SkPicture* picture) { TRACE_EVENT0("skia", "SkCanvas::drawPicture()"); if (picture) { diff --git a/src/core/SkDevice.cpp b/src/core/SkDevice.cpp index b912e6aef0..3d5000f569 100644 --- a/src/core/SkDevice.cpp +++ b/src/core/SkDevice.cpp @@ -182,10 +182,6 @@ void* SkBaseDevice::onAccessPixels(SkImageInfo* info, size_t* rowBytes) { return NULL; } -void SkBaseDevice::EXPERIMENTAL_optimize(const SkPicture* picture) { - // The base class doesn't perform any analysis but derived classes may -} - bool SkBaseDevice::EXPERIMENTAL_drawPicture(SkCanvas*, const SkPicture*, const SkMatrix*, const SkPaint*) { // The base class doesn't perform any accelerated picture rendering diff --git a/src/core/SkMultiPictureDraw.cpp b/src/core/SkMultiPictureDraw.cpp index 0e21b5c54a..0285598413 100644 --- a/src/core/SkMultiPictureDraw.cpp +++ b/src/core/SkMultiPictureDraw.cpp @@ -112,10 +112,6 @@ void SkMultiPictureDraw::draw() { SkASSERT(data.fCanvas->getGrContext() == context); if (!data.fPaint && data.fMatrix.isIdentity()) { - // TODO: this path always tries to optimize pictures. Should we - // switch to this API approach (vs. SkCanvas::EXPERIMENTAL_optimize)? - data.fCanvas->EXPERIMENTAL_optimize(data.fPicture); - SkRect clipBounds; if (!data.fCanvas->getClipBounds(&clipBounds)) { continue; diff --git a/src/core/SkPicture.cpp b/src/core/SkPicture.cpp index 01b315a758..5231cb9570 100644 --- a/src/core/SkPicture.cpp +++ b/src/core/SkPicture.cpp @@ -625,23 +625,13 @@ uint32_t SkPicture::uniqueID() const { return fUniqueID; } - -static SkRecord* optimized(SkRecord* r) { - SkRecordOptimize(r); - return r; -} - // fRecord OK SkPicture::SkPicture(SkScalar width, SkScalar height, SkRecord* record, SkBBoxHierarchy* bbh) : fCullWidth(width) , fCullHeight(height) - , fRecord(optimized(record)) + , fRecord(record) , fBBH(SkSafeRef(bbh)) , fAnalysis(*fRecord) { - // TODO: delay as much of this work until just before first playback? - if (fBBH.get()) { - SkRecordFillBounds(this->cullRect(), *fRecord, fBBH.get()); - } this->needsNewGenID(); } diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp index 9ae5c21484..2985b2dfff 100644 --- a/src/core/SkPictureRecorder.cpp +++ b/src/core/SkPictureRecorder.cpp @@ -5,10 +5,15 @@ * found in the LICENSE file. */ +#if SK_SUPPORT_GPU +#include "GrPictureUtils.h" +#endif + #include "SkPictureRecorder.h" #include "SkRecord.h" #include "SkRecordDraw.h" #include "SkRecorder.h" +#include "SkRecordOpts.h" #include "SkTypes.h" SkPictureRecorder::SkPictureRecorder() {} @@ -18,6 +23,7 @@ SkPictureRecorder::~SkPictureRecorder() {} SkCanvas* SkPictureRecorder::beginRecording(SkScalar width, SkScalar height, SkBBHFactory* bbhFactory /* = NULL */, uint32_t recordFlags /* = 0 */) { + fFlags = recordFlags; fCullWidth = width; fCullHeight = height; @@ -36,7 +42,42 @@ SkCanvas* SkPictureRecorder::getRecordingCanvas() { } SkPicture* SkPictureRecorder::endRecording() { - return SkNEW_ARGS(SkPicture, (fCullWidth, fCullHeight, fRecord.detach(), fBBH.get())); + // TODO: delay as much of this work until just before first playback? + SkRecordOptimize(fRecord); + +#if SK_SUPPORT_GPU + SkAutoTUnref saveLayerData; + + if (fBBH && (fFlags & kComputeSaveLayerInfo_RecordFlag)) { + SkPicture::AccelData::Key key = GrAccelData::ComputeAccelDataKey(); + + saveLayerData.reset(SkNEW_ARGS(GrAccelData, (key))); + } +#endif + + if (fBBH.get()) { + SkRect cullRect = SkRect::MakeWH(fCullWidth, fCullHeight); + +#if SK_SUPPORT_GPU + if (saveLayerData) { + SkRecordComputeLayers(cullRect, *fRecord, fBBH.get(), saveLayerData); + } else { +#endif + SkRecordFillBounds(cullRect, *fRecord, fBBH.get()); +#if SK_SUPPORT_GPU + } +#endif + } + + SkPicture* pict = SkNEW_ARGS(SkPicture, (fCullWidth, fCullHeight, fRecord.detach(), fBBH.get())); + +#if SK_SUPPORT_GPU + if (saveLayerData) { + pict->EXPERIMENTAL_addAccelData(saveLayerData); + } +#endif + + return pict; } void SkPictureRecorder::partialReplay(SkCanvas* canvas) const { diff --git a/src/gpu/GrPictureUtils.cpp b/src/gpu/GrPictureUtils.cpp index e1a70ab6eb..c0b53f9665 100644 --- a/src/gpu/GrPictureUtils.cpp +++ b/src/gpu/GrPictureUtils.cpp @@ -7,6 +7,7 @@ #include "GrPictureUtils.h" +#include "SkBBoxHierarchy.h" #include "SkPaintPriv.h" #include "SkPatchUtils.h" #include "SkRecord.h" @@ -61,6 +62,10 @@ public: this->popSaveLayerInfo(); } //--------- LAYER HOISTING + + // Finally feed all stored bounds into the BBH. They'll be returned in this order. + SkASSERT(bbh); + bbh->insert(&fBounds, record.count()); } template void operator()(const T& op) { @@ -425,7 +430,13 @@ private: const GrAccelData* childData = static_cast(dp.picture->EXPERIMENTAL_getAccelData(key)); if (!childData) { - childData = GPUOptimize(dp.picture); + // If the child layer hasn't been generated with saveLayer data we + // assume the worst (i.e., that it does contain layers which nest + // inside existing layers). Layers within sub-pictures that don't + // have saveLayer data cannot be hoisted. + // TODO: could the analysis data be use to fine tune this? + this->updateStackForSaveLayer(); + return; } for (int i = 0; i < childData->numSaveLayers(); ++i) { @@ -587,31 +598,9 @@ private: } // namespace SkRecords - void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, SkBBoxHierarchy* bbh, GrAccelData* data) { - SkRecords::CollectLayers collector(cullRect, record, bbh, data); } -const GrAccelData* GPUOptimize(const SkPicture* pict) { - if (NULL == pict || pict->cullRect().isEmpty()) { - return NULL; - } - SkPicture::AccelData::Key key = GrAccelData::ComputeAccelDataKey(); - - const GrAccelData* existing = - static_cast(pict->EXPERIMENTAL_getAccelData(key)); - if (existing) { - return existing; - } - - SkAutoTUnref data(SkNEW_ARGS(GrAccelData, (key))); - - pict->EXPERIMENTAL_addAccelData(data); - - SkRecordComputeLayers(pict->cullRect(), *pict->fRecord, NULL, data); - - return data; -} diff --git a/src/gpu/GrPictureUtils.h b/src/gpu/GrPictureUtils.h index 6aa277edbf..edd45dbf49 100644 --- a/src/gpu/GrPictureUtils.h +++ b/src/gpu/GrPictureUtils.h @@ -79,6 +79,4 @@ private: void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, SkBBoxHierarchy* bbh, GrAccelData* data); -const GrAccelData* GPUOptimize(const SkPicture* pict); - #endif // GrPictureUtils_DEFINED diff --git a/src/gpu/SkGpuDevice.cpp b/src/gpu/SkGpuDevice.cpp index b072142a0b..06f6eb7778 100644 --- a/src/gpu/SkGpuDevice.cpp +++ b/src/gpu/SkGpuDevice.cpp @@ -1789,25 +1789,6 @@ SkSurface* SkGpuDevice::newSurface(const SkImageInfo& info, const SkSurfaceProps return SkSurface::NewRenderTarget(fContext, info, fRenderTarget->numSamples(), &props); } -void SkGpuDevice::EXPERIMENTAL_optimize(const SkPicture* picture) { - fContext->getLayerCache()->processDeletedPictures(); - - if (picture->fData.get() && !picture->fData->suitableForLayerOptimization()) { - return; - } - - SkPicture::AccelData::Key key = GrAccelData::ComputeAccelDataKey(); - - const SkPicture::AccelData* existing = picture->EXPERIMENTAL_getAccelData(key); - if (existing) { - return; - } - - GPUOptimize(picture); - - fContext->getLayerCache()->trackPicture(picture); -} - bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* mainCanvas, const SkPicture* mainPicture, const SkMatrix* matrix, const SkPaint* paint) { // todo: should handle these natively @@ -1815,6 +1796,13 @@ bool SkGpuDevice::EXPERIMENTAL_drawPicture(SkCanvas* mainCanvas, const SkPicture return false; } + SkPicture::AccelData::Key key = GrAccelData::ComputeAccelDataKey(); + + const SkPicture::AccelData* data = mainPicture->EXPERIMENTAL_getAccelData(key); + if (!data) { + return false; + } + SkRect clipBounds; if (!mainCanvas->getClipBounds(&clipBounds)) { return true; diff --git a/src/gpu/SkGpuDevice.h b/src/gpu/SkGpuDevice.h index 6bba97434a..f7ff8c82a4 100644 --- a/src/gpu/SkGpuDevice.h +++ b/src/gpu/SkGpuDevice.h @@ -120,8 +120,6 @@ protected: virtual bool onReadPixels(const SkImageInfo&, void*, size_t, int, int) SK_OVERRIDE; virtual bool onWritePixels(const SkImageInfo&, const void*, size_t, int, int) SK_OVERRIDE; - /** PRIVATE / EXPERIMENTAL -- do not call */ - virtual void EXPERIMENTAL_optimize(const SkPicture* picture) SK_OVERRIDE; /** PRIVATE / EXPERIMENTAL -- do not call */ virtual bool EXPERIMENTAL_drawPicture(SkCanvas* canvas, const SkPicture* picture, const SkMatrix*, const SkPaint*) SK_OVERRIDE; diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index 1dc37e32ed..42bd25806c 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -900,11 +900,14 @@ static void test_gpu_picture_optimization(skiatest::Reporter* reporter, complexPaint.setImageFilter(filter); SkAutoTUnref pict, child; + SkRTreeFactory bbhFactory; { SkPictureRecorder recorder; - SkCanvas* c = recorder.beginRecording(SkIntToScalar(kWidth), SkIntToScalar(kHeight)); + SkCanvas* c = recorder.beginRecording(SkIntToScalar(kWidth), SkIntToScalar(kHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); c->saveLayer(NULL, &complexPaint); c->restore(); @@ -937,7 +940,9 @@ static void test_gpu_picture_optimization(skiatest::Reporter* reporter, SkPictureRecorder recorder; SkCanvas* c = recorder.beginRecording(SkIntToScalar(kWidth), - SkIntToScalar(kHeight)); + SkIntToScalar(kHeight), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); // 1) c->saveLayer(NULL, &complexPaint); // layer #0 c->restore(); @@ -980,14 +985,6 @@ static void test_gpu_picture_optimization(skiatest::Reporter* reporter, // Now test out the SaveLayer extraction { - SkImageInfo info = SkImageInfo::MakeN32Premul(kWidth, kHeight); - - SkAutoTUnref surface(SkSurface::NewScratchRenderTarget(context, info)); - - SkCanvas* canvas = surface->getCanvas(); - - canvas->EXPERIMENTAL_optimize(pict); - SkPicture::AccelData::Key key = GrAccelData::ComputeAccelDataKey(); const SkPicture::AccelData* data = pict->EXPERIMENTAL_getAccelData(key); diff --git a/tools/PictureRenderer.cpp b/tools/PictureRenderer.cpp index 1c31e6865a..f2fd6b73fa 100644 --- a/tools/PictureRenderer.cpp +++ b/tools/PictureRenderer.cpp @@ -223,10 +223,14 @@ void PictureRenderer::buildBBoxHierarchy() { if (kNone_BBoxHierarchyType != fBBoxHierarchyType && fPicture) { SkAutoTDelete factory(this->getFactory()); SkPictureRecorder recorder; + uint32_t flags = this->recordFlags(); + if (fUseMultiPictureDraw) { + flags |= SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag; + } SkCanvas* canvas = recorder.beginRecording(fPicture->cullRect().width(), fPicture->cullRect().height(), factory.get(), - this->recordFlags()); + flags); fPicture->playback(canvas); fPicture.reset(recorder.endRecording()); } @@ -707,8 +711,12 @@ bool TiledPictureRenderer::render(SkBitmap** out) { surfaces[i]->getCanvas()->setMatrix(fCanvas->getTotalMatrix()); SkPictureRecorder recorder; + SkRTreeFactory bbhFactory; + SkCanvas* c = recorder.beginRecording(SkIntToScalar(fTileRects[i].width()), - SkIntToScalar(fTileRects[i].height())); + SkIntToScalar(fTileRects[i].height()), + &bbhFactory, + SkPictureRecorder::kComputeSaveLayerInfo_RecordFlag); c->save(); SkMatrix mat; mat.setTranslate(-SkIntToScalar(fTileRects[i].fLeft),