From 28648fe4a69b0cee8df42b5966e4e645c3aabefb Mon Sep 17 00:00:00 2001 From: senorblanco Date: Mon, 25 Aug 2014 08:06:57 -0700 Subject: [PATCH] Fix recursive computation of filter bounds for drop shadow, morphology, blur. Because we're computing "backwards" from a clip rect of destination pixels to be filled to the required source pixels, we should use tail recursion rather than head recursion in onFilterBounds(). This actually only makes a difference for drop-shadow, where the computation is non-commutative. Blur and morphology commute, but I moved them to tail recursion anyway for clarity (so all onFilterBounds use tail recursion). BUG=skia: R=bsalomon@google.com Author: senorblanco@chromium.org Review URL: https://codereview.chromium.org/481273005 --- src/effects/SkBlurImageFilter.cpp | 6 ++-- src/effects/SkDropShadowImageFilter.cpp | 6 ++-- src/effects/SkMorphologyImageFilter.cpp | 6 ++-- tests/ImageFilterTest.cpp | 44 +++++++++++++++++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/effects/SkBlurImageFilter.cpp b/src/effects/SkBlurImageFilter.cpp index 71bc8114df..8787ccec8c 100644 --- a/src/effects/SkBlurImageFilter.cpp +++ b/src/effects/SkBlurImageFilter.cpp @@ -253,13 +253,13 @@ void SkBlurImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) const bool SkBlurImageFilter::onFilterBounds(const SkIRect& src, const SkMatrix& ctm, SkIRect* dst) const { SkIRect bounds = src; - if (getInput(0) && !getInput(0)->filterBounds(src, ctm, &bounds)) { - return false; - } SkVector sigma = SkVector::Make(fSigma.width(), fSigma.height()); ctm.mapVectors(&sigma, 1); bounds.outset(SkScalarCeilToInt(SkScalarMul(sigma.x(), SkIntToScalar(3))), SkScalarCeilToInt(SkScalarMul(sigma.y(), SkIntToScalar(3)))); + if (getInput(0) && !getInput(0)->filterBounds(bounds, ctm, &bounds)) { + return false; + } *dst = bounds; return true; } diff --git a/src/effects/SkDropShadowImageFilter.cpp b/src/effects/SkDropShadowImageFilter.cpp index f1ebae8d97..29d685becc 100644 --- a/src/effects/SkDropShadowImageFilter.cpp +++ b/src/effects/SkDropShadowImageFilter.cpp @@ -121,9 +121,6 @@ void SkDropShadowImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) bool SkDropShadowImageFilter::onFilterBounds(const SkIRect& src, const SkMatrix& ctm, SkIRect* dst) const { SkIRect bounds = src; - if (getInput(0) && !getInput(0)->filterBounds(src, ctm, &bounds)) { - return false; - } SkVector offsetVec = SkVector::Make(fDx, fDy); ctm.mapVectors(&offsetVec, 1); bounds.offset(-SkScalarCeilToInt(offsetVec.x()), @@ -133,6 +130,9 @@ bool SkDropShadowImageFilter::onFilterBounds(const SkIRect& src, const SkMatrix& bounds.outset(SkScalarCeilToInt(SkScalarMul(sigma.x(), SkIntToScalar(3))), SkScalarCeilToInt(SkScalarMul(sigma.y(), SkIntToScalar(3)))); bounds.join(src); + if (getInput(0) && !getInput(0)->filterBounds(bounds, ctm, &bounds)) { + return false; + } *dst = bounds; return true; } diff --git a/src/effects/SkMorphologyImageFilter.cpp b/src/effects/SkMorphologyImageFilter.cpp index eef2a7d623..6a6dd4d451 100644 --- a/src/effects/SkMorphologyImageFilter.cpp +++ b/src/effects/SkMorphologyImageFilter.cpp @@ -248,13 +248,13 @@ void SkMorphologyImageFilter::computeFastBounds(const SkRect& src, SkRect* dst) bool SkMorphologyImageFilter::onFilterBounds(const SkIRect& src, const SkMatrix& ctm, SkIRect* dst) const { SkIRect bounds = src; - if (getInput(0) && !getInput(0)->filterBounds(src, ctm, &bounds)) { - return false; - } SkVector radius = SkVector::Make(SkIntToScalar(this->radius().width()), SkIntToScalar(this->radius().height())); ctm.mapVectors(&radius, 1); bounds.outset(SkScalarCeilToInt(radius.x()), SkScalarCeilToInt(radius.y())); + if (getInput(0) && !getInput(0)->filterBounds(bounds, ctm, &bounds)) { + return false; + } *dst = bounds; return true; } diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp index f87f99ce98..e7e0f84385 100644 --- a/tests/ImageFilterTest.cpp +++ b/tests/ImageFilterTest.cpp @@ -469,6 +469,50 @@ DEF_TEST(ImageFilterDrawMatrixBBH, reporter) { } } +static SkImageFilter* makeBlur(SkImageFilter* input = NULL) { + return SkBlurImageFilter::Create(SK_Scalar1, SK_Scalar1, input); +} + +static SkImageFilter* makeDropShadow(SkImageFilter* input = NULL) { + return SkDropShadowImageFilter::Create( + SkIntToScalar(100), SkIntToScalar(100), + SkIntToScalar(10), SkIntToScalar(10), + SK_ColorBLUE, input); +} + +DEF_TEST(ImageFilterBlurThenShadowBounds, reporter) { + SkAutoTUnref filter1(makeBlur()); + SkAutoTUnref filter2(makeDropShadow(filter1.get())); + + SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); + SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236); + filter2->filterBounds(bounds, SkMatrix(), &bounds); + + REPORTER_ASSERT(reporter, bounds == expectedBounds); +} + +DEF_TEST(ImageFilterShadowThenBlurBounds, reporter) { + SkAutoTUnref filter1(makeDropShadow()); + SkAutoTUnref filter2(makeBlur(filter1.get())); + + SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); + SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236); + filter2->filterBounds(bounds, SkMatrix(), &bounds); + + REPORTER_ASSERT(reporter, bounds == expectedBounds); +} + +DEF_TEST(ImageFilterDilateThenBlurBounds, reporter) { + SkAutoTUnref filter1(SkDilateImageFilter::Create(2, 2)); + SkAutoTUnref filter2(makeDropShadow(filter1.get())); + + SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100); + SkIRect expectedBounds = SkIRect::MakeXYWH(-132, -132, 234, 234); + filter2->filterBounds(bounds, SkMatrix(), &bounds); + + REPORTER_ASSERT(reporter, bounds == expectedBounds); +} + static void drawBlurredRect(SkCanvas* canvas) { SkAutoTUnref filter(SkBlurImageFilter::Create(SkIntToScalar(8), 0)); SkPaint filterPaint;