diff options
-rw-r--r-- | src/core/SkPictureRecorder.cpp | 21 | ||||
-rw-r--r-- | src/core/SkRecordDraw.cpp | 54 | ||||
-rw-r--r-- | src/core/SkRecordDraw.h | 12 | ||||
-rw-r--r-- | tests/RecordDrawTest.cpp | 72 |
4 files changed, 63 insertions, 96 deletions
diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp index 6397ca00fa..62fa0e968f 100644 --- a/src/core/SkPictureRecorder.cpp +++ b/src/core/SkPictureRecorder.cpp @@ -72,11 +72,16 @@ SkPicture* SkPictureRecorder::endRecordingAsPicture() { drawableList ? drawableList->newDrawableSnapshot() : nullptr; if (fBBH.get()) { + SkAutoTMalloc<SkRect> bounds(fRecord->count()); if (saveLayerData) { - SkRecordComputeLayers(fCullRect, *fRecord, pictList, fBBH.get(), saveLayerData); + SkRecordComputeLayers(fCullRect, *fRecord, bounds, pictList, saveLayerData); } else { - SkRecordFillBounds(fCullRect, *fRecord, fBBH.get()); + SkRecordFillBounds(fCullRect, *fRecord, bounds); } + fBBH->insert(bounds, fRecord->count()); + + // Now that we've calculated content bounds, we can update fCullRect, often trimming it. + // TODO: get updated fCullRect from bounds instead of forcing the BBH to return it? SkRect bbhBound = fBBH->getRootBound(); SkASSERT((bbhBound.isEmpty() || fCullRect.contains(bbhBound)) || (bbhBound.isEmpty() && fCullRect.isEmpty())); @@ -153,14 +158,12 @@ protected: } SkAutoTUnref<SkLayerInfo> saveLayerData; - if (fBBH && fDoSaveLayerInfo) { + // TODO: can we avoid work by not allocating / filling these bounds? + SkAutoTMalloc<SkRect> scratchBounds(fRecord->count()); saveLayerData.reset(new SkLayerInfo); - SkBBoxHierarchy* bbh = nullptr; // we've already computed fBBH (received in constructor) - // TODO: update saveLayer info computation to reuse the already computed - // bounds in 'fBBH' - SkRecordComputeLayers(fBounds, *fRecord, pictList, bbh, saveLayerData); + SkRecordComputeLayers(fBounds, *fRecord, scratchBounds, pictList, saveLayerData); } size_t subPictureBytes = 0; @@ -183,7 +186,9 @@ SkDrawable* SkPictureRecorder::endRecordingAsDrawable() { SkRecordOptimize(fRecord); if (fBBH.get()) { - SkRecordFillBounds(fCullRect, *fRecord, fBBH.get()); + SkAutoTMalloc<SkRect> bounds(fRecord->count()); + SkRecordFillBounds(fCullRect, *fRecord, bounds); + fBBH->insert(bounds, fRecord->count()); } SkDrawable* drawable = diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index 12920da361..ab0cb71407 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -150,20 +150,15 @@ template <> void Draw::draw(const DrawDrawable& r) { // in for all the control ops we stashed away. class FillBounds : SkNoncopyable { public: - FillBounds(const SkRect& cullRect, const SkRecord& record) + FillBounds(const SkRect& cullRect, const SkRecord& record, SkRect bounds[]) : fNumRecords(record.count()) , fCullRect(cullRect) - , fBounds(record.count()) - { - // Calculate bounds for all ops. This won't go quite in order, so we'll need - // to store the bounds separately then feed them in to the BBH later in order. + , fBounds(bounds) { fCTM = &SkMatrix::I(); fCurrentClipBounds = fCullRect; } - void setCurrentOp(int currentOp) { fCurrentOp = currentOp; } - - void cleanUp(SkBBoxHierarchy* bbh) { + void cleanUp() { // If we have any lingering unpaired Saves, simulate restores to make // sure all ops in those Save blocks have their bounds calculated. while (!fSaveStack.isEmpty()) { @@ -174,13 +169,11 @@ public: while (!fControlIndices.isEmpty()) { this->popControl(fCullRect); } - - // Finally feed all stored bounds into the BBH. They'll be returned in this order. - if (bbh) { - bbh->insert(fBounds.get(), fNumRecords); - } } + void setCurrentOp(int currentOp) { fCurrentOp = currentOp; } + + template <typename T> void operator()(const T& op) { this->updateCTM(op); this->updateClipBounds(op); @@ -580,7 +573,7 @@ private: const SkRect fCullRect; // Conservative identity-space bounds for each op in the SkRecord. - SkAutoTMalloc<Bounds> fBounds; + Bounds* fBounds; // We walk fCurrentOp through the SkRecord, as we go using updateCTM() // and updateClipBounds() to maintain the exact CTM (fCTM) and conservative @@ -597,27 +590,27 @@ private: // SkRecord visitor to gather saveLayer/restore information. class CollectLayers : SkNoncopyable { public: - CollectLayers(const SkRect& cullRect, const SkRecord& record, + CollectLayers(const SkRect& cullRect, const SkRecord& record, SkRect bounds[], const SkBigPicture::SnapshotArray* pictList, SkLayerInfo* accelData) : fSaveLayersInStack(0) , fAccelData(accelData) , fPictList(pictList) - , fFillBounds(cullRect, record) + , fFillBounds(cullRect, record, bounds) {} - void setCurrentOp(int currentOp) { fFillBounds.setCurrentOp(currentOp); } - - void cleanUp(SkBBoxHierarchy* bbh) { + void cleanUp() { // fFillBounds must perform its cleanUp first so that all the bounding // boxes associated with unbalanced restores are updated (prior to // fetching their bound in popSaveLayerInfo). - fFillBounds.cleanUp(bbh); - + fFillBounds.cleanUp(); while (!fSaveLayerStack.isEmpty()) { this->popSaveLayerInfo(); } } + void setCurrentOp(int currentOp) { fFillBounds.setCurrentOp(currentOp); } + + template <typename T> void operator()(const T& op) { fFillBounds(op); this->trackSaveLayers(op); @@ -792,27 +785,22 @@ private: } // namespace SkRecords -void SkRecordFillBounds(const SkRect& cullRect, const SkRecord& record, SkBBoxHierarchy* bbh) { - SkRecords::FillBounds visitor(cullRect, record); - +void SkRecordFillBounds(const SkRect& cullRect, const SkRecord& record, SkRect bounds[]) { + SkRecords::FillBounds visitor(cullRect, record, bounds); for (int curOp = 0; curOp < record.count(); curOp++) { visitor.setCurrentOp(curOp); record.visit<void>(curOp, visitor); } - - visitor.cleanUp(bbh); + visitor.cleanUp(); } -void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, - const SkBigPicture::SnapshotArray* pictList, SkBBoxHierarchy* bbh, - SkLayerInfo* data) { - SkRecords::CollectLayers visitor(cullRect, record, pictList, data); - +void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, SkRect bounds[], + const SkBigPicture::SnapshotArray* pictList, SkLayerInfo* data) { + SkRecords::CollectLayers visitor(cullRect, record, bounds, pictList, data); for (int curOp = 0; curOp < record.count(); curOp++) { visitor.setCurrentOp(curOp); record.visit<void>(curOp, visitor); } - - visitor.cleanUp(bbh); + visitor.cleanUp(); } diff --git a/src/core/SkRecordDraw.h b/src/core/SkRecordDraw.h index c2db37aebd..fdf98824ac 100644 --- a/src/core/SkRecordDraw.h +++ b/src/core/SkRecordDraw.h @@ -17,12 +17,14 @@ class SkDrawable; class SkLayerInfo; -// Fill a BBH to be used by SkRecordDraw to accelerate playback. -void SkRecordFillBounds(const SkRect& cullRect, const SkRecord&, SkBBoxHierarchy*); +// Calculate conservative identity space bounds for each op in the record. +void SkRecordFillBounds(const SkRect& cullRect, const SkRecord&, SkRect bounds[]); -void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord& record, - const SkBigPicture::SnapshotArray*, - SkBBoxHierarchy* bbh, SkLayerInfo* data); +// SkRecordFillBounds(), and gathers information about saveLayers and stores it for later +// use (e.g., layer hoisting). The gathered information is sufficient to determine +// where each saveLayer will land and which ops in the picture it represents. +void SkRecordComputeLayers(const SkRect& cullRect, const SkRecord&, SkRect bounds[], + const SkBigPicture::SnapshotArray*, SkLayerInfo* data); // Draw an SkRecord into an SkCanvas. A convenience wrapper around SkRecords::Draw. void SkRecordDraw(const SkRecord&, SkCanvas*, SkPicture const* const drawablePicts[], diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp index d8ca48d796..13be115253 100644 --- a/tests/RecordDrawTest.cpp +++ b/tests/RecordDrawTest.cpp @@ -122,26 +122,6 @@ DEF_TEST(RecordDraw_SetMatrixClobber, r) { REPORTER_ASSERT(r, setMatrix->matrix == expected); } -struct TestBBH : public SkBBoxHierarchy { - void insert(const SkRect boundsArray[], int N) override { - fEntries.setCount(N); - for (int i = 0; i < N; i++) { - Entry e = { i, boundsArray[i] }; - fEntries[i] = e; - } - } - - void search(const SkRect& query, SkTDArray<int>* results) const override {} - size_t bytesUsed() const override { return 0; } - SkRect getRootBound() const override { return SkRect::MakeEmpty(); } - - struct Entry { - int opIndex; - SkRect bounds; - }; - SkTDArray<Entry> fEntries; -}; - // Like a==b, with a little slop recognizing that float equality can be weird. static bool sloppy_rect_eq(SkRect a, SkRect b) { SkRect inset(a), outset(a); @@ -150,9 +130,7 @@ static bool sloppy_rect_eq(SkRect a, SkRect b) { return outset.contains(b) && !inset.contains(b); } -// This test is not meant to make total sense yet. It's testing the status quo -// of SkRecordFillBounds(), which itself doesn't make total sense yet. -DEF_TEST(RecordDraw_BBH, r) { +DEF_TEST(RecordDraw_BasicBounds, r) { SkRecord record; SkRecorder recorder(&record, W, H); recorder.save(); @@ -161,14 +139,11 @@ DEF_TEST(RecordDraw_BBH, r) { recorder.drawRect(SkRect::MakeWH(320, 240), SkPaint()); recorder.restore(); - TestBBH bbh; - SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, &bbh); - - REPORTER_ASSERT(r, bbh.fEntries.count() == 5); - for (int i = 0; i < bbh.fEntries.count(); i++) { - REPORTER_ASSERT(r, bbh.fEntries[i].opIndex == i); + SkAutoTMalloc<SkRect> bounds(record.count()); + SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, bounds); - REPORTER_ASSERT(r, sloppy_rect_eq(SkRect::MakeWH(400, 480), bbh.fEntries[i].bounds)); + for (int i = 0; i < record.count(); i++) { + REPORTER_ASSERT(r, sloppy_rect_eq(SkRect::MakeWH(400, 480), bounds[i])); } } @@ -187,15 +162,14 @@ DEF_TEST(RecordDraw_TextBounds, r) { const SkPoint pos[] = { {40, 50}, {60, 70} }; recorder.drawPosText(text, bytes, pos, SkPaint()); - TestBBH bbh; - SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, &bbh); - REPORTER_ASSERT(r, bbh.fEntries.count() == 2); + SkAutoTMalloc<SkRect> bounds(record.count()); + SkRecordFillBounds(SkRect::MakeWH(SkIntToScalar(W), SkIntToScalar(H)), record, bounds); // We can make these next assertions confidently because SkRecordFillBounds // builds its bounds by overestimating font metrics in a platform-independent way. // If that changes, these tests will need to be more flexible. - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(0, 0, 140, 60))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(0, 20, 180, 100))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[0], SkRect::MakeLTRB(0, 0, 140, 60))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[1], SkRect::MakeLTRB(0, 20, 180, 100))); } // Base test to ensure start/stop range is respected @@ -248,13 +222,12 @@ DEF_TEST(RecordDraw_SaveLayerAffectsClipBounds, r) { // // The second bug showed up as adjusting the picture bounds (0,0,50,50) by the drop shadow too. // The saveLayer, clipRect, and restore bounds were incorrectly (0,0,70,50). - TestBBH bbh; - SkRecordFillBounds(SkRect::MakeWH(50, 50), record, &bbh); - REPORTER_ASSERT(r, bbh.fEntries.count() == 4); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(0, 0, 50, 50))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(0, 0, 50, 50))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[2].bounds, SkRect::MakeLTRB(0, 0, 40, 40))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[3].bounds, SkRect::MakeLTRB(0, 0, 50, 50))); + SkAutoTMalloc<SkRect> bounds(record.count()); + SkRecordFillBounds(SkRect::MakeWH(50, 50), record, bounds); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[0], SkRect::MakeLTRB(0, 0, 50, 50))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[1], SkRect::MakeLTRB(0, 0, 50, 50))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[2], SkRect::MakeLTRB(0, 0, 40, 40))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[3], SkRect::MakeLTRB(0, 0, 50, 50))); } // When a saveLayer provides an explicit bound and has a complex paint (e.g., one that @@ -267,18 +240,17 @@ DEF_TEST(RecordDraw_SaveLayerBoundsAffectsClipBounds, r) { SkPaint p; p.setXfermodeMode(SkXfermode::kSrc_Mode); - SkRect bounds = SkRect::MakeLTRB(10, 10, 40, 40); - recorder.saveLayer(&bounds, &p); + SkRect layerBounds = SkRect::MakeLTRB(10, 10, 40, 40); + recorder.saveLayer(&layerBounds, &p); recorder.drawRect(SkRect::MakeLTRB(20, 20, 30, 30), SkPaint()); recorder.restore(); - TestBBH bbh; - SkRecordFillBounds(SkRect::MakeWH(50, 50), record, &bbh); - REPORTER_ASSERT(r, bbh.fEntries.count() == 3); + SkAutoTMalloc<SkRect> bounds(record.count()); + SkRecordFillBounds(SkRect::MakeWH(50, 50), record, bounds); if (!SkCanvas::Internal_Private_GetIgnoreSaveLayerBounds()) { - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[0].bounds, SkRect::MakeLTRB(10, 10, 40, 40))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[1].bounds, SkRect::MakeLTRB(20, 20, 30, 30))); - REPORTER_ASSERT(r, sloppy_rect_eq(bbh.fEntries[2].bounds, SkRect::MakeLTRB(10, 10, 40, 40))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[0], SkRect::MakeLTRB(10, 10, 40, 40))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[1], SkRect::MakeLTRB(20, 20, 30, 30))); + REPORTER_ASSERT(r, sloppy_rect_eq(bounds[2], SkRect::MakeLTRB(10, 10, 40, 40))); } } |