Fix srcBounds computation in SkMatrixConvolutionImageFilter

Note that this does change the behavior of the cropRect for the repeated case. The cropRect now only acts as a hard clip on the output.

BUG= skia:7766

Change-Id: I1d66678bc797cd4835701cd20c36e68b22ac880a
Reviewed-on: https://skia-review.googlesource.com/127338
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Robert Phillips <robertphillips@google.com>

1 files changed, 86 insertions, 58 deletions

diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index 556c3f040d..8e46715fbb 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -150,7 +150,6 @@ public:
     FilterList(sk_sp<SkImageFilter> input, const SkImageFilter::CropRect* cropRect = nullptr) {
         SkPoint3 location = SkPoint3::Make(0, 0, SK_Scalar1);
         const SkScalar five = SkIntToScalar(5);
-
         {
             sk_sp<SkColorFilter> cf(SkColorFilter::MakeModeFilter(SK_ColorRED,
                                                                   SkBlendMode::kSrcIn));
@@ -158,7 +157,6 @@ public:
             this->addFilter("color filter",
                 SkColorFilterImageFilter::Make(std::move(cf), input, cropRect));
         }
-
         {
             sk_sp<SkImage> gradientImage(SkImage::MakeFromBitmap(make_gradient_circle(64, 64)));
             sk_sp<SkImageFilter> gradientSource(SkImageSource::Make(std::move(gradientImage)));
@@ -169,7 +167,6 @@ public:
                                               20.0f,
                                               std::move(gradientSource), input, cropRect));
         }
-
         this->addFilter("blur", SkBlurImageFilter::Make(SK_Scalar1,
                                                         SK_Scalar1,
                                                         input,
@@ -193,13 +190,14 @@ public:
             const SkISize kernelSize = SkISize::Make(3, 3);
             const SkScalar gain = SK_Scalar1, bias = 0;
 
+            // This filter needs a saveLayer bc it is in repeat mode
             this->addFilter("matrix convolution",
-                  SkMatrixConvolutionImageFilter::Make(
-                      kernelSize, kernel, gain, bias, SkIPoint::Make(1, 1),
-                      SkMatrixConvolutionImageFilter::kRepeat_TileMode, false,
-                      input, cropRect));
+                            SkMatrixConvolutionImageFilter::Make(
+                                          kernelSize, kernel, gain, bias, SkIPoint::Make(1, 1),
+                                          SkMatrixConvolutionImageFilter::kRepeat_TileMode, false,
+                                          input, cropRect),
+                            true);
         }
-
         this->addFilter("merge", SkMergeImageFilter::Make(input, input, cropRect));
 
         {
@@ -273,18 +271,21 @@ public:
     int count() const { return fFilters.count(); }
     SkImageFilter* getFilter(int index) const { return fFilters[index].fFilter.get(); }
     const char* getName(int index) const { return fFilters[index].fName; }
+    bool needsSaveLayer(int index) const { return fFilters[index].fNeedsSaveLayer; }
 private:
     struct Filter {
-        Filter() : fName(nullptr) {}
-        Filter(const char* name, sk_sp<SkImageFilter> filter)
+        Filter() : fName(nullptr), fNeedsSaveLayer(false) {}
+        Filter(const char* name, sk_sp<SkImageFilter> filter, bool needsSaveLayer)
             : fName(name)
-            , fFilter(std::move(filter)) {
+            , fFilter(std::move(filter))
+            , fNeedsSaveLayer(needsSaveLayer) {
         }
         const char*                 fName;
         sk_sp<SkImageFilter>        fFilter;
+        bool                        fNeedsSaveLayer;
     };
-    void addFilter(const char* name, sk_sp<SkImageFilter> filter) {
-        fFilters.push_back(Filter(name, std::move(filter)));
+    void addFilter(const char* name, sk_sp<SkImageFilter> filter, bool needsSaveLayer = false) {
+        fFilters.push_back(Filter(name, std::move(filter), needsSaveLayer));
     }
 
     SkTArray<Filter> fFilters;
@@ -305,7 +306,8 @@ private:
     }
     sk_sp<SkImageFilter> onMakeColorSpace(SkColorSpaceXformer*) const override { return nullptr; }
 
-    SkIRect onFilterBounds(const SkIRect&, const SkMatrix&, MapDirection) const override {
+    SkIRect onFilterBounds(const SkIRect&, const SkMatrix&,
+                           MapDirection, const SkIRect*) const override {
         return fBounds;
     }
 
@@ -743,34 +745,51 @@ DEF_TEST(ImageFilterDrawTiled, reporter) {
     tiledResult.allocN32Pixels(width, height);
     SkCanvas tiledCanvas(tiledResult);
     SkCanvas untiledCanvas(untiledResult);
-    int tileSize = 8;
+    const int tileSize = 8;
+
+    SkPaint textPaint;
+    textPaint.setTextSize(SkIntToScalar(height));
+    textPaint.setColor(SK_ColorWHITE);
+
+    const char* text = "ABC";
+    const SkScalar yPos = SkIntToScalar(height);
 
     for (int scale = 1; scale <= 2; ++scale) {
         for (int i = 0; i < filters.count(); ++i) {
-            tiledCanvas.clear(0);
-            untiledCanvas.clear(0);
-            SkPaint paint;
-            paint.setImageFilter(sk_ref_sp(filters.getFilter(i)));
-            paint.setTextSize(SkIntToScalar(height));
-            paint.setColor(SK_ColorWHITE);
-            SkString str;
-            const char* text = "ABC";
-            SkScalar ypos = SkIntToScalar(height);
+            SkPaint combinedPaint;
+            combinedPaint.setTextSize(SkIntToScalar(height));
+            combinedPaint.setColor(SK_ColorWHITE);
+            combinedPaint.setImageFilter(sk_ref_sp(filters.getFilter(i)));
+
+            untiledCanvas.clear(SK_ColorTRANSPARENT);
             untiledCanvas.save();
             untiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale));
-            untiledCanvas.drawString(text, 0, ypos, paint);
+            untiledCanvas.drawString(text, 0, yPos, combinedPaint);
             untiledCanvas.restore();
+            untiledCanvas.flush();
+
+            tiledCanvas.clear(SK_ColorTRANSPARENT);
             for (int y = 0; y < height; y += tileSize) {
                 for (int x = 0; x < width; x += tileSize) {
                     tiledCanvas.save();
-                    tiledCanvas.clipRect(SkRect::Make(SkIRect::MakeXYWH(x, y, tileSize, tileSize)));
-                    tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale));
-                    tiledCanvas.drawString(text, 0, ypos, paint);
+                    const SkRect clipRect = SkRect::MakeXYWH(x, y, tileSize, tileSize);
+                    tiledCanvas.clipRect(clipRect);
+                    if (filters.needsSaveLayer(i)) {
+                        const SkRect layerBounds = SkRect::MakeWH(width, height);
+                        tiledCanvas.saveLayer(&layerBounds, &combinedPaint);
+                            tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale));
+                            tiledCanvas.drawString(text, 0, yPos, textPaint);
+                        tiledCanvas.restore();
+                    } else {
+                        tiledCanvas.scale(SkIntToScalar(scale), SkIntToScalar(scale));
+                        tiledCanvas.drawString(text, 0, yPos, combinedPaint);
+                    }
+
                     tiledCanvas.restore();
                 }
             }
-            untiledCanvas.flush();
             tiledCanvas.flush();
+
             if (!sk_tool_utils::equal_pixels(untiledResult, tiledResult, 1)) {
                 REPORTER_ASSERT(reporter, false, filters.getName(i));
                 break;
@@ -850,7 +869,8 @@ DEF_TEST(ImageFilterBlurThenShadowBounds, reporter) {
 
     SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100);
     SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236);
-    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(),
+                                   SkImageFilter::kReverse_MapDirection, &bounds);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }
@@ -861,7 +881,8 @@ DEF_TEST(ImageFilterShadowThenBlurBounds, reporter) {
 
     SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100);
     SkIRect expectedBounds = SkIRect::MakeXYWH(-133, -133, 236, 236);
-    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(),
+                                   SkImageFilter::kReverse_MapDirection, &bounds);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }
@@ -872,7 +893,8 @@ DEF_TEST(ImageFilterDilateThenBlurBounds, reporter) {
 
     SkIRect bounds = SkIRect::MakeXYWH(0, 0, 100, 100);
     SkIRect expectedBounds = SkIRect::MakeXYWH(-132, -132, 234, 234);
-    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(),
+                                   SkImageFilter::kReverse_MapDirection, &bounds);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }
@@ -891,20 +913,20 @@ DEF_TEST(ImageFilterScaledBlurRadius, reporter) {
 
         SkIRect expectedBlurBounds = SkIRect::MakeLTRB(-6, -6, 206, 206);
         SkIRect blurBounds = blur->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr);
         REPORTER_ASSERT(reporter, blurBounds == expectedBlurBounds);
         SkIRect reverseBlurBounds = blur->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds);
         REPORTER_ASSERT(reporter, reverseBlurBounds == expectedBlurBounds);
 
         SkIRect expectedShadowBounds = SkIRect::MakeLTRB(0, 0, 460, 460);
         SkIRect shadowBounds = dropShadow->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr);
         REPORTER_ASSERT(reporter, shadowBounds == expectedShadowBounds);
         SkIRect expectedReverseShadowBounds =
             SkIRect::MakeLTRB(-260, -260, 200, 200);
         SkIRect reverseShadowBounds = dropShadow->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds);
         REPORTER_ASSERT(reporter,
             reverseShadowBounds == expectedReverseShadowBounds);
     }
@@ -916,20 +938,20 @@ DEF_TEST(ImageFilterScaledBlurRadius, reporter) {
 
         SkIRect expectedBlurBounds = SkIRect::MakeLTRB(-3, -103, 103, 3);
         SkIRect blurBounds = blur->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr);
         REPORTER_ASSERT(reporter, blurBounds == expectedBlurBounds);
         SkIRect reverseBlurBounds = blur->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds);
         REPORTER_ASSERT(reporter, reverseBlurBounds == expectedBlurBounds);
 
         SkIRect expectedShadowBounds = SkIRect::MakeLTRB(0, -230, 230, 0);
         SkIRect shadowBounds = dropShadow->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kForward_MapDirection, nullptr);
         REPORTER_ASSERT(reporter, shadowBounds == expectedShadowBounds);
         SkIRect expectedReverseShadowBounds =
             SkIRect::MakeLTRB(-130, -100, 100, 130);
         SkIRect reverseShadowBounds = dropShadow->filterBounds(
-            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection);
+            bounds, scaleMatrix, SkImageFilter::kReverse_MapDirection, &bounds);
         REPORTER_ASSERT(reporter,
             reverseShadowBounds == expectedReverseShadowBounds);
     }
@@ -1919,8 +1941,8 @@ DEF_TEST(XfermodeImageFilterBounds, reporter) {
     for (int i = 0; i < kModeCount; ++i) {
         sk_sp<SkImageFilter> xfermode(SkXfermodeImageFilter::Make(static_cast<SkBlendMode>(i),
                                                                   background, foreground, nullptr));
-        auto bounds =
-                xfermode->filterBounds(src, SkMatrix::I(), SkImageFilter::kForward_MapDirection);
+        auto bounds = xfermode->filterBounds(src, SkMatrix::I(),
+                                             SkImageFilter::kForward_MapDirection, nullptr);
         REPORTER_ASSERT(reporter, bounds == expectedBounds[i]);
     }
 
@@ -1929,7 +1951,8 @@ DEF_TEST(XfermodeImageFilterBounds, reporter) {
     sk_sp<SkImageFilter> foreground2(new FixedBoundsImageFilter(SkIRect::MakeXYWH(40, 40, 50, 50)));
     sk_sp<SkImageFilter> xfermode(SkXfermodeImageFilter::Make(
             SkBlendMode::kSrcIn, std::move(background2), std::move(foreground2), nullptr));
-    auto bounds = xfermode->filterBounds(src, SkMatrix::I(), SkImageFilter::kForward_MapDirection);
+    auto bounds = xfermode->filterBounds(src, SkMatrix::I(),
+                                         SkImageFilter::kForward_MapDirection, nullptr);
     REPORTER_ASSERT(reporter, bounds.isEmpty());
 }
 
@@ -1939,12 +1962,12 @@ DEF_TEST(OffsetImageFilterBounds, reporter) {
 
     SkIRect expectedForward = SkIRect::MakeXYWH(-50, -50, 100, 100);
     SkIRect boundsForward = offset->filterBounds(src, SkMatrix::I(),
-                                                 SkImageFilter::kForward_MapDirection);
+                                                 SkImageFilter::kForward_MapDirection, nullptr);
     REPORTER_ASSERT(reporter, boundsForward == expectedForward);
 
     SkIRect expectedReverse = SkIRect::MakeXYWH(50, 50, 100, 100);
     SkIRect boundsReverse = offset->filterBounds(src, SkMatrix::I(),
-                                                 SkImageFilter::kReverse_MapDirection);
+                                                 SkImageFilter::kReverse_MapDirection, &src);
     REPORTER_ASSERT(reporter, boundsReverse == expectedReverse);
 }
 
@@ -1956,7 +1979,7 @@ static void test_arithmetic_bounds(skiatest::Reporter* reporter, float k1, float
             SkArithmeticImageFilter::Make(k1, k2, k3, k4, false, background, foreground, crop));
     // The value of the input rect doesn't matter because we use inputs with fixed bounds.
     SkIRect bounds = arithmetic->filterBounds(SkIRect::MakeXYWH(11, 22, 33, 44), SkMatrix::I(),
-                                              SkImageFilter::kForward_MapDirection);
+                                              SkImageFilter::kForward_MapDirection, nullptr);
     REPORTER_ASSERT(reporter, expected == bounds);
 }
 
@@ -2027,18 +2050,20 @@ DEF_TEST(ImageSourceBounds, reporter) {
     SkIRect input(SkIRect::MakeXYWH(10, 20, 30, 40));
     REPORTER_ASSERT(reporter,
                     imageBounds == source1->filterBounds(input, SkMatrix::I(),
-                                                         SkImageFilter::kForward_MapDirection));
+                                                         SkImageFilter::kForward_MapDirection,
+                                                         nullptr));
     REPORTER_ASSERT(reporter,
                     input == source1->filterBounds(input, SkMatrix::I(),
-                                                   SkImageFilter::kReverse_MapDirection));
+                                                   SkImageFilter::kReverse_MapDirection, &input));
     SkMatrix scale(SkMatrix::MakeScale(2));
     SkIRect scaledBounds = SkIRect::MakeWH(128, 128);
     REPORTER_ASSERT(reporter,
                     scaledBounds == source1->filterBounds(input, scale,
-                                                          SkImageFilter::kForward_MapDirection));
-    REPORTER_ASSERT(
-            reporter,
-            input == source1->filterBounds(input, scale, SkImageFilter::kReverse_MapDirection));
+                                                          SkImageFilter::kForward_MapDirection,
+                                                          nullptr));
+    REPORTER_ASSERT(reporter, input == source1->filterBounds(input, scale,
+                                                             SkImageFilter::kReverse_MapDirection,
+                                                             &input));
 
     // Specified src and dst rects.
     SkRect src(SkRect::MakeXYWH(0.5, 0.5, 100.5, 100.5));
@@ -2046,16 +2071,19 @@ DEF_TEST(ImageSourceBounds, reporter) {
     sk_sp<SkImageFilter> source2(SkImageSource::Make(image, src, dst, kMedium_SkFilterQuality));
     REPORTER_ASSERT(reporter,
                     dst.roundOut() == source2->filterBounds(input, SkMatrix::I(),
-                                                            SkImageFilter::kForward_MapDirection));
+                                                            SkImageFilter::kForward_MapDirection,
+                                                            nullptr));
     REPORTER_ASSERT(reporter,
                     input == source2->filterBounds(input, SkMatrix::I(),
-                                                   SkImageFilter::kReverse_MapDirection));
+                                                   SkImageFilter::kReverse_MapDirection, &input));
     scale.mapRect(&dst);
     scale.mapRect(&src);
     REPORTER_ASSERT(reporter,
                     dst.roundOut() == source2->filterBounds(input, scale,
-                                                            SkImageFilter::kForward_MapDirection));
-    REPORTER_ASSERT(
-            reporter,
-            input == source2->filterBounds(input, scale, SkImageFilter::kReverse_MapDirection));
+                                                            SkImageFilter::kForward_MapDirection,
+                                                            nullptr));
+    REPORTER_ASSERT(reporter, input == source2->filterBounds(input, scale,
+                                                             SkImageFilter::kReverse_MapDirection,
+                                                             &input));
 }
+