From 9b3aa54bc9605257c701cf465813f5fb1d7ba39e Mon Sep 17 00:00:00 2001 From: reed Date: Wed, 11 Mar 2015 08:47:12 -0700 Subject: [PATCH] optimization/fix: dirty the clip-bounds when we mod the clip in savelayer Before the fix, we could use a stale cache of the clipbounds in quickReject. Often this could return false negatives, meaning we would try to draw more than we should (it would eventually be really clipped). Occasionally this could also report false positives (if the layer were outside of the normal canvas bounds, e.g. a layer with an offset imagefilter). BUG=skia: NOTREECHECKS=True Review URL: https://codereview.chromium.org/983243003 --- src/core/SkCanvas.cpp | 4 +++- tests/QuickRejectTest.cpp | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp index e4c03f3e5e..55b3b7ba7a 100644 --- a/src/core/SkCanvas.cpp +++ b/src/core/SkCanvas.cpp @@ -831,7 +831,7 @@ static bool bounds_affects_clip(SkCanvas::SaveFlags flags) { } bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags, - SkIRect* intersection, const SkImageFilter* imageFilter) { + SkIRect* intersection, const SkImageFilter* imageFilter) { SkIRect clipBounds; SkRegion::Op op = SkRegion::kIntersect_Op; if (!this->getClipDeviceBounds(&clipBounds)) { @@ -854,6 +854,7 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags, // early exit if the layer's bounds are clipped out if (!ir.intersect(clipBounds)) { if (bounds_affects_clip(flags)) { + fCachedLocalClipBoundsDirty = true; fMCRec->fRasterClip.setEmpty(); } return false; @@ -863,6 +864,7 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveFlags flags, } if (bounds_affects_clip(flags)) { + fCachedLocalClipBoundsDirty = true; fClipStack->clipDevRect(ir, op); // early exit if the clip is now empty if (!fMCRec->fRasterClip.op(ir, op)) { diff --git a/tests/QuickRejectTest.cpp b/tests/QuickRejectTest.cpp index cefde12354..447d81615b 100644 --- a/tests/QuickRejectTest.cpp +++ b/tests/QuickRejectTest.cpp @@ -87,6 +87,23 @@ static void test_drawBitmap(skiatest::Reporter* reporter) { REPORTER_ASSERT(reporter, 0xFFFFFFFF == *dst.getAddr32(5, 5)); } +static void test_layers(skiatest::Reporter* reporter) { + SkCanvas canvas(100, 100); + + SkRect r = SkRect::MakeWH(10, 10); + REPORTER_ASSERT(reporter, false == canvas.quickReject(r)); + + r.offset(300, 300); + REPORTER_ASSERT(reporter, true == canvas.quickReject(r)); + + // Test that saveLayer updates quickReject + SkRect bounds = SkRect::MakeLTRB(50, 50, 70, 70); + canvas.saveLayer(&bounds, NULL); + REPORTER_ASSERT(reporter, true == canvas.quickReject(SkRect::MakeWH(10, 10))); + REPORTER_ASSERT(reporter, false == canvas.quickReject(SkRect::MakeWH(60, 60))); +} + DEF_TEST(QuickReject, reporter) { test_drawBitmap(reporter); + test_layers(reporter); }