From 0aeea6d344f12e35e29a79f4bbc48af88f913204 Mon Sep 17 00:00:00 2001 From: reed Date: Thu, 7 Aug 2014 11:19:11 -0700 Subject: [PATCH] Revert of add isRect() check to AAClip, to detect if a soft-clip is really just an irect (https://codereview.chromium.org/445233006/) Reason for revert: new code asserts on some pictures Original issue's description: > add isRect() check to AAClip, to detect if a soft-clip is really just an irect > > BUG=skia: > > Committed: https://skia.googlesource.com/skia/+/592cb8d552556b1e922887d506d00b64bc5d0547 R=bsalomon@google.com, humper@google.com TBR=bsalomon@google.com, humper@google.com NOTREECHECKS=true NOTRY=true BUG=skia: Author: reed@google.com Review URL: https://codereview.chromium.org/452533002 --- src/core/SkAAClip.cpp | 29 ----------------------------- src/core/SkAAClip.h | 4 ---- src/core/SkRasterClip.cpp | 5 +---- src/core/SkRasterClip.h | 12 ++---------- tests/AAClipTest.cpp | 25 ------------------------- 5 files changed, 3 insertions(+), 72 deletions(-) diff --git a/src/core/SkAAClip.cpp b/src/core/SkAAClip.cpp index 54a6dcd795..14152f8317 100644 --- a/src/core/SkAAClip.cpp +++ b/src/core/SkAAClip.cpp @@ -684,35 +684,6 @@ bool SkAAClip::setRect(const SkIRect& bounds) { #endif } -bool SkAAClip::isRect() const { - if (this->isEmpty()) { - return false; - } - - const RunHead* head = fRunHead; - if (head->fRowCount != 1) { - return false; - } - const YOffset* yoff = head->yoffsets(); - if (yoff->fY != fBounds.fBottom - 1) { - return false; - } - SkASSERT(0 == yoff->fOffset); - - const uint8_t* row = head->data(); - int width = fBounds.width(); - do { - if (row[1] != 0xFF) { - return false; - } - int n = row[0]; - SkASSERT(n <= width); - width -= n; - row += 2; - } while (width > 0); - return true; -} - bool SkAAClip::setRect(const SkRect& r, bool doAA) { if (r.isEmpty()) { return this->setEmpty(); diff --git a/src/core/SkAAClip.h b/src/core/SkAAClip.h index c36a3e98ce..f2cde62dbc 100644 --- a/src/core/SkAAClip.h +++ b/src/core/SkAAClip.h @@ -29,10 +29,6 @@ public: bool isEmpty() const { return NULL == fRunHead; } const SkIRect& getBounds() const { return fBounds; } - // Returns true iff the clip is not empty, and is just a hard-edged rect (no partial alpha). - // If true, getBounds() can be used in place of this clip. - bool isRect() const; - bool setEmpty(); bool setRect(const SkIRect&); bool setRect(const SkRect&, bool doAA = true); diff --git a/src/core/SkRasterClip.cpp b/src/core/SkRasterClip.cpp index d1615a3445..664211f64f 100644 --- a/src/core/SkRasterClip.cpp +++ b/src/core/SkRasterClip.cpp @@ -222,10 +222,7 @@ void SkRasterClip::convertToAA() { SkASSERT(fIsBW); fAA.setRegion(fBW); fIsBW = false; - - // since we are being explicitly asked to convert-to-aa, we pass false so we don't "optimize" - // ourselves back to BW. - (void)this->updateCacheAndReturnNonEmpty(false); + (void)this->updateCacheAndReturnNonEmpty(); } #ifdef SK_DEBUG diff --git a/src/core/SkRasterClip.h b/src/core/SkRasterClip.h index 29a925f2a2..0c2723314c 100644 --- a/src/core/SkRasterClip.h +++ b/src/core/SkRasterClip.h @@ -89,19 +89,11 @@ private: } bool computeIsRect() const { - return fIsBW ? fBW.isRect() : fAA.isRect(); + return fIsBW ? fBW.isRect() : false; } - bool updateCacheAndReturnNonEmpty(bool detectAARect = true) { + bool updateCacheAndReturnNonEmpty() { fIsEmpty = this->computeIsEmpty(); - - // detect that our computed AA is really just a (hard-edged) rect - if (detectAARect && !fIsEmpty && !fIsBW && fAA.isRect()) { - fBW.setRect(fAA.getBounds()); - fAA.setEmpty(); // don't need this guy anymore - fIsBW = true; - } - fIsRect = this->computeIsRect(); return !fIsEmpty; } diff --git a/tests/AAClipTest.cpp b/tests/AAClipTest.cpp index 776cd5267a..26c1ec1d0e 100644 --- a/tests/AAClipTest.cpp +++ b/tests/AAClipTest.cpp @@ -318,30 +318,6 @@ static void test_path_with_hole(skiatest::Reporter* reporter) { } } -static void test_really_a_rect(skiatest::Reporter* reporter) { - SkRRect rrect; - rrect.setRectXY(SkRect::MakeWH(100, 100), 5, 5); - - SkPath path; - path.addRRect(rrect); - - SkAAClip clip; - clip.setPath(path); - - REPORTER_ASSERT(reporter, clip.getBounds() == SkIRect::MakeWH(100, 100)); - REPORTER_ASSERT(reporter, !clip.isRect()); - - // This rect should intersect the clip, but slice-out all of the "soft" parts, - // leaving just a rect. - const SkIRect ir = SkIRect::MakeLTRB(10, -10, 50, 90); - - clip.op(ir, SkRegion::kIntersect_Op); - - REPORTER_ASSERT(reporter, clip.getBounds() == SkIRect::MakeLTRB(10, 0, 50, 90)); - // the clip recognized that that it is just a rect! - REPORTER_ASSERT(reporter, clip.isRect()); -} - #include "SkRasterClip.h" static void copyToMask(const SkRasterClip& rc, SkMask* mask) { @@ -428,5 +404,4 @@ DEF_TEST(AAClip, reporter) { test_path_with_hole(reporter); test_regressions(); test_nearly_integral(reporter); - test_really_a_rect(reporter); }