From a0865b4620cc614586a5c02f258da2436ed3ab2b Mon Sep 17 00:00:00 2001 From: joshualitt Date: Thu, 5 Mar 2015 12:18:38 -0800 Subject: [PATCH] Revert of Update SkPicture cull rects with RTree information (patchset #6 id:140001 of https://codereview.chromium.org/971803002/) Reason for revert: Might be breaking deps roll Original issue's description: > Update SkPicture cull rects with RTree information > > When computed, the RTree for an SkPicture will have a root > bounds that reflects the best bounding information available, > rather than the best estimate at the time the picture recorder > is created. Given that creators frequently don't know ahead of > time what will be drawn, the RTree bound is often tighter. > > Perf testing on Chrome indicates a small raster performance > advantage. For upcoming painting changes in Chrome the > performance advantage is much larger. > > BUG= > > Committed: https://skia.googlesource.com/skia/+/2dd3b6647dc726f36fd8774b3d0d2e83b493aeac TBR=mtklein@google.com,schenney@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Review URL: https://codereview.chromium.org/977413003 --- src/core/SkBBoxHierarchy.h | 3 --- src/core/SkPictureRecorder.cpp | 4 ---- src/core/SkRTree.cpp | 8 -------- src/core/SkRTree.h | 3 --- src/core/SkRecordDraw.cpp | 8 ++------ tests/PictureTest.cpp | 9 +++------ tests/RecordDrawTest.cpp | 1 - 7 files changed, 5 insertions(+), 31 deletions(-) diff --git a/src/core/SkBBoxHierarchy.h b/src/core/SkBBoxHierarchy.h index 1ea9e6bdb0..f925a91940 100644 --- a/src/core/SkBBoxHierarchy.h +++ b/src/core/SkBBoxHierarchy.h @@ -34,9 +34,6 @@ public: virtual size_t bytesUsed() const = 0; - // Get the root bound. - virtual SkRect getRootBound() const = 0; - SK_DECLARE_INST_COUNT(SkBBoxHierarchy) private: typedef SkRefCnt INHERITED; diff --git a/src/core/SkPictureRecorder.cpp b/src/core/SkPictureRecorder.cpp index 1972ad3341..42f8732c79 100644 --- a/src/core/SkPictureRecorder.cpp +++ b/src/core/SkPictureRecorder.cpp @@ -60,10 +60,6 @@ SkPicture* SkPictureRecorder::endRecordingAsPicture() { } else { SkRecordFillBounds(fCullRect, *fRecord, fBBH.get()); } - SkRect bbhBound = fBBH->getRootBound(); - SkASSERT((bbhBound.isEmpty() || fCullRect.contains(bbhBound)) - || (bbhBound.isEmpty() && fCullRect.isEmpty())); - fCullRect = bbhBound; } SkPicture* pict = SkNEW_ARGS(SkPicture, (fCullRect, fRecord, pictList, fBBH)); diff --git a/src/core/SkRTree.cpp b/src/core/SkRTree.cpp index 2fded6f72b..ba5843e537 100644 --- a/src/core/SkRTree.cpp +++ b/src/core/SkRTree.cpp @@ -9,14 +9,6 @@ SkRTree::SkRTree(SkScalar aspectRatio) : fCount(0), fAspectRatio(aspectRatio) {} -SkRect SkRTree::getRootBound() const { - if (fCount) { - return fRoot.fBounds; - } else { - return SkRect::MakeEmpty(); - } -} - void SkRTree::insert(const SkRect boundsArray[], int N) { SkASSERT(0 == fCount); diff --git a/src/core/SkRTree.h b/src/core/SkRTree.h index 9a118d4897..320b0bd438 100644 --- a/src/core/SkRTree.h +++ b/src/core/SkRTree.h @@ -52,9 +52,6 @@ public: // Insertion count (not overall node count, which may be greater). int getCount() const { return fCount; } - // Get the root bound. - SkRect getRootBound() const SK_OVERRIDE; - // These values were empirically determined to produce reasonable performance in most cases. static const int kMinChildren = 6, kMaxChildren = 11; diff --git a/src/core/SkRecordDraw.cpp b/src/core/SkRecordDraw.cpp index b66c721aee..9b08426670 100644 --- a/src/core/SkRecordDraw.cpp +++ b/src/core/SkRecordDraw.cpp @@ -387,12 +387,8 @@ private: Bounds bounds(const DrawPaint&) const { return fCurrentClipBounds; } Bounds bounds(const NoOp&) const { return Bounds::MakeEmpty(); } // NoOps don't draw. - Bounds bounds(const DrawSprite& op) const { // Ignores the matrix, but respects the clip. - SkRect rect = Bounds::MakeXYWH(op.left, op.top, op.bitmap.width(), op.bitmap.height()); - if (!rect.intersect(fCurrentClipBounds)) { - return Bounds::MakeEmpty(); - } - return rect; + Bounds bounds(const DrawSprite& op) const { // Ignores the matrix. + return Bounds::MakeXYWH(op.left, op.top, op.bitmap.width(), op.bitmap.height()); } Bounds bounds(const DrawRect& op) const { return this->adjustAndMap(op.rect, &op.paint); } diff --git a/tests/PictureTest.cpp b/tests/PictureTest.cpp index ac750e9a85..33e058cc2e 100644 --- a/tests/PictureTest.cpp +++ b/tests/PictureTest.cpp @@ -1248,9 +1248,8 @@ DEF_TEST(DontOptimizeSaveLayerDrawDrawRestore, reporter) { struct CountingBBH : public SkBBoxHierarchy { mutable int searchCalls; - SkRect rootBound; - CountingBBH(const SkRect& bound) : searchCalls(0), rootBound(bound) {} + CountingBBH() : searchCalls(0) {} void search(const SkRect& query, SkTDArray* results) const SK_OVERRIDE { this->searchCalls++; @@ -1258,7 +1257,6 @@ struct CountingBBH : public SkBBoxHierarchy { void insert(const SkRect[], int) SK_OVERRIDE {} virtual size_t bytesUsed() const SK_OVERRIDE { return 0; } - SkRect getRootBound() const SK_OVERRIDE { return rootBound; } }; class SpoonFedBBHFactory : public SkBBHFactory { @@ -1273,12 +1271,11 @@ private: // When the canvas clip covers the full picture, we don't need to call the BBH. DEF_TEST(Picture_SkipBBH, r) { - SkRect bound = SkRect::MakeWH(320, 240); - CountingBBH bbh(bound); + CountingBBH bbh; SpoonFedBBHFactory factory(&bbh); SkPictureRecorder recorder; - recorder.beginRecording(bound, &factory); + recorder.beginRecording(320, 240, &factory); SkAutoTUnref picture(recorder.endRecording()); SkCanvas big(640, 480), small(300, 200); diff --git a/tests/RecordDrawTest.cpp b/tests/RecordDrawTest.cpp index baee712b37..cf138b8d4e 100644 --- a/tests/RecordDrawTest.cpp +++ b/tests/RecordDrawTest.cpp @@ -133,7 +133,6 @@ struct TestBBH : public SkBBoxHierarchy { void search(const SkRect& query, SkTDArray* results) const SK_OVERRIDE {} size_t bytesUsed() const SK_OVERRIDE { return 0; } - SkRect getRootBound() const SK_OVERRIDE { return SkRect::MakeEmpty(); } struct Entry { unsigned opIndex;