Minor refactoring to image filters

This pulls the boring parts out of: https://skia-review.googlesource.com/c/skia/+/127338 (Fix srcBounds computation in SkMatrixConvolutionImageFilter)

TBR=bsalomon@google.com
Change-Id: Iade788fcc96c4c16989d13e7592030a6a1d3f170
Reviewed-on: https://skia-review.googlesource.com/127573
Commit-Queue: Robert Phillips <robertphillips@google.com>
Reviewed-by: Greg Daniel <egdaniel@google.com>

10 files changed, 69 insertions, 65 deletions

diff --git a/include/core/SkImageFilter.h b/include/core/SkImageFilter.h
index 84389b4b5a..2528009e24 100644
--- a/include/core/SkImageFilter.h
+++ b/include/core/SkImageFilter.h
@@ -156,8 +156,7 @@ public:
      * implementation recursively unions all input bounds, or returns the
      * source rect if no inputs.
      */
-    SkIRect filterBounds(const SkIRect& src, const SkMatrix& ctm,
-                         MapDirection = kReverse_MapDirection) const;
+    SkIRect filterBounds(const SkIRect& src, const SkMatrix& ctm, MapDirection) const;
 
 #if SK_SUPPORT_GPU
     static sk_sp<SkSpecialImage> DrawWithFP(GrContext* context,
@@ -332,7 +331,7 @@ protected:
      * Performs a forwards or reverse mapping of the given rect to accommodate
      * this filter's margin requirements. kForward_MapDirection is used to
      * determine the destination pixels which would be touched by filtering
-     * the given given source rect (e.g., given source bitmap bounds,
+     * the given source rect (e.g., given source bitmap bounds,
      * determine the optimal bounds of the filtered offscreen bitmap).
      * kReverse_MapDirection is used to determine which pixels of the
      * input(s) would be required to fill the given destination rect
@@ -385,8 +384,8 @@ protected:
      *  which are not capable of processing a smaller source bitmap into a
      *  larger destination.
      */
-    sk_sp<SkSpecialImage> applyCropRect(const Context&, SkSpecialImage* src, SkIPoint* srcOffset,
-                                        SkIRect* bounds) const;
+    sk_sp<SkSpecialImage> applyCropRectAndPad(const Context&, SkSpecialImage* src,
+                                              SkIPoint* srcOffset, SkIRect* bounds) const;
 
     /**
      *  Creates a modified Context for use when recursing up the image filter DAG.
diff --git a/src/core/SkCanvas.cpp b/src/core/SkCanvas.cpp
index 82237cf823..a467cac060 100644
--- a/src/core/SkCanvas.cpp
+++ b/src/core/SkCanvas.cpp
@@ -880,7 +880,8 @@ bool SkCanvas::clipRectBounds(const SkRect* bounds, SaveLayerFlags saveLayerFlag
     const SkMatrix& ctm = fMCRec->fMatrix;  // this->getTotalMatrix()
 
     if (imageFilter) {
-        clipBounds = imageFilter->filterBounds(clipBounds, ctm);
+        clipBounds = imageFilter->filterBounds(clipBounds, ctm,
+                                               SkImageFilter::kReverse_MapDirection);
         if (bounds && !imageFilter->canComputeFastBounds()) {
             bounds = nullptr;
         }
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 2a898fda14..a9447212d3 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -223,7 +223,7 @@ sk_sp<SkSpecialImage> SkImageFilter::filterImage(SkSpecialImage* src, const Cont
 }
 
 SkIRect SkImageFilter::filterBounds(const SkIRect& src, const SkMatrix& ctm,
-                                 MapDirection direction) const {
+                                    MapDirection direction) const {
     if (kReverse_MapDirection == direction) {
         SkIRect bounds = this->onFilterNodeBounds(src, ctm, direction);
         return this->onFilterBounds(bounds, ctm, direction);
@@ -327,8 +327,8 @@ bool SkImageFilter::canHandleComplexCTM() const {
 
 bool SkImageFilter::applyCropRect(const Context& ctx, const SkIRect& srcBounds,
                                   SkIRect* dstBounds) const {
-    SkIRect temp = this->onFilterNodeBounds(srcBounds, ctx.ctm(), kForward_MapDirection);
-    fCropRect.applyTo(temp, ctx.ctm(), this->affectsTransparentBlack(), dstBounds);
+    SkIRect tmpDst = this->onFilterNodeBounds(srcBounds, ctx.ctm(), kForward_MapDirection);
+    fCropRect.applyTo(tmpDst, ctx.ctm(), this->affectsTransparentBlack(), dstBounds);
     // Intersect against the clip bounds, in case the crop rect has
     // grown the bounds beyond the original clip. This can happen for
     // example in tiling, where the clip is much smaller than the filtered
@@ -407,16 +407,14 @@ static sk_sp<SkSpecialImage> pad_image(SkSpecialImage* src,
     return surf->makeImageSnapshot();
 }
 
-sk_sp<SkSpecialImage> SkImageFilter::applyCropRect(const Context& ctx,
-                                                   SkSpecialImage* src,
-                                                   SkIPoint* srcOffset,
-                                                   SkIRect* bounds) const {
+sk_sp<SkSpecialImage> SkImageFilter::applyCropRectAndPad(const Context& ctx,
+                                                         SkSpecialImage* src,
+                                                         SkIPoint* srcOffset,
+                                                         SkIRect* bounds) const {
     const SkIRect srcBounds = SkIRect::MakeXYWH(srcOffset->x(), srcOffset->y(),
                                                 src->width(), src->height());
 
-    SkIRect dstBounds = this->onFilterNodeBounds(srcBounds, ctx.ctm(), kForward_MapDirection);
-    fCropRect.applyTo(dstBounds, ctx.ctm(), this->affectsTransparentBlack(), bounds);
-    if (!bounds->intersect(ctx.clipBounds())) {
+    if (!this->applyCropRect(ctx, srcBounds, bounds)) {
         return nullptr;
     }
 
diff --git a/src/effects/SkComposeImageFilter.cpp b/src/effects/SkComposeImageFilter.cpp
index e379f37bed..7769f6d845 100644
--- a/src/effects/SkComposeImageFilter.cpp
+++ b/src/effects/SkComposeImageFilter.cpp
@@ -38,7 +38,8 @@ sk_sp<SkSpecialImage> SkComposeImageFilter::onFilterImage(SkSpecialImage* source
     // filter, so that the inner filter produces the pixels that the outer
     // filter requires as input. This matters if the outer filter moves pixels.
     SkIRect innerClipBounds;
-    innerClipBounds = this->getInput(0)->filterBounds(ctx.clipBounds(), ctx.ctm());
+    innerClipBounds = this->getInput(0)->filterBounds(ctx.clipBounds(), ctx.ctm(),
+                                                      kReverse_MapDirection);
     Context innerContext(ctx.ctm(), innerClipBounds, ctx.cache(), ctx.outputProperties());
     SkIPoint innerOffset = SkIPoint::Make(0, 0);
     sk_sp<SkSpecialImage> inner(this->filterInput(1, source, innerContext, &innerOffset));
diff --git a/src/effects/SkDisplacementMapEffect.cpp b/src/effects/SkDisplacementMapEffect.cpp
index 5dd3ece316..fe425c3cc3 100644
--- a/src/effects/SkDisplacementMapEffect.cpp
+++ b/src/effects/SkDisplacementMapEffect.cpp
@@ -259,7 +259,7 @@ sk_sp<SkSpecialImage> SkDisplacementMapEffect::onFilterImage(SkSpecialImage* sou
     }
 
     SkIRect displBounds;
-    displ = this->applyCropRect(ctx, displ.get(), &displOffset, &displBounds);
+    displ = this->applyCropRectAndPad(ctx, displ.get(), &displOffset, &displBounds);
     if (!displ) {
         return nullptr;
     }
diff --git a/src/effects/SkMatrixConvolutionImageFilter.cpp b/src/effects/SkMatrixConvolutionImageFilter.cpp
index 3c4d7b34b2..9412b604ab 100644
--- a/src/effects/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/SkMatrixConvolutionImageFilter.cpp
@@ -295,8 +295,8 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(SkSpecialIma
         return nullptr;
     }
 
-    SkIRect bounds;
-    input = this->applyCropRect(this->mapContext(ctx), input.get(), &inputOffset, &bounds);
+    SkIRect dstBounds;
+    input = this->applyCropRectAndPad(this->mapContext(ctx), input.get(), &inputOffset, &dstBounds);
     if (!input) {
         return nullptr;
     }
@@ -316,12 +316,12 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(SkSpecialIma
         sk_sp<GrTextureProxy> inputProxy(input->asTextureProxyRef(context));
         SkASSERT(inputProxy);
 
-        offset->fX = bounds.left();
-        offset->fY = bounds.top();
-        bounds.offset(-inputOffset);
+        offset->fX = dstBounds.left();
+        offset->fY = dstBounds.top();
+        dstBounds.offset(-inputOffset);
 
         auto fp = GrMatrixConvolutionEffect::Make(std::move(inputProxy),
-                                                  bounds,
+                                                  dstBounds,
                                                   fKernelSize,
                                                   fKernel,
                                                   fGain,
@@ -333,7 +333,7 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(SkSpecialIma
             return nullptr;
         }
 
-        return DrawWithFP(context, std::move(fp), bounds, ctx.outputProperties());
+        return DrawWithFP(context, std::move(fp), dstBounds, ctx.outputProperties());
     }
 #endif
 
@@ -355,7 +355,7 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(SkSpecialIma
         return nullptr;
     }
 
-    const SkImageInfo info = SkImageInfo::MakeN32(bounds.width(), bounds.height(),
+    const SkImageInfo info = SkImageInfo::MakeN32(dstBounds.width(), dstBounds.height(),
                                                   inputBM.alphaType());
 
     SkBitmap dst;
@@ -363,26 +363,27 @@ sk_sp<SkSpecialImage> SkMatrixConvolutionImageFilter::onFilterImage(SkSpecialIma
         return nullptr;
     }
 
-    offset->fX = bounds.fLeft;
-    offset->fY = bounds.fTop;
-    bounds.offset(-inputOffset);
-    SkIRect interior = SkIRect::MakeXYWH(bounds.left() + fKernelOffset.fX,
-                                         bounds.top() + fKernelOffset.fY,
-                                         bounds.width() - fKernelSize.fWidth + 1,
-                                         bounds.height() - fKernelSize.fHeight + 1);
-    SkIRect top = SkIRect::MakeLTRB(bounds.left(), bounds.top(), bounds.right(), interior.top());
-    SkIRect bottom = SkIRect::MakeLTRB(bounds.left(), interior.bottom(),
-                                       bounds.right(), bounds.bottom());
-    SkIRect left = SkIRect::MakeLTRB(bounds.left(), interior.top(),
+    offset->fX = dstBounds.fLeft;
+    offset->fY = dstBounds.fTop;
+    dstBounds.offset(-inputOffset);
+    SkIRect interior = SkIRect::MakeXYWH(dstBounds.left() + fKernelOffset.fX,
+                                         dstBounds.top() + fKernelOffset.fY,
+                                         dstBounds.width() - fKernelSize.fWidth + 1,
+                                         dstBounds.height() - fKernelSize.fHeight + 1);
+    SkIRect top = SkIRect::MakeLTRB(dstBounds.left(), dstBounds.top(),
+                                    dstBounds.right(), interior.top());
+    SkIRect bottom = SkIRect::MakeLTRB(dstBounds.left(), interior.bottom(),
+                                       dstBounds.right(), dstBounds.bottom());
+    SkIRect left = SkIRect::MakeLTRB(dstBounds.left(), interior.top(),
                                      interior.left(), interior.bottom());
     SkIRect right = SkIRect::MakeLTRB(interior.right(), interior.top(),
-                                      bounds.right(), interior.bottom());
-    this->filterBorderPixels(inputBM, &dst, top, bounds);
-    this->filterBorderPixels(inputBM, &dst, left, bounds);
-    this->filterInteriorPixels(inputBM, &dst, interior, bounds);
-    this->filterBorderPixels(inputBM, &dst, right, bounds);
-    this->filterBorderPixels(inputBM, &dst, bottom, bounds);
-    return SkSpecialImage::MakeFromRaster(SkIRect::MakeWH(bounds.width(), bounds.height()),
+                                      dstBounds.right(), interior.bottom());
+    this->filterBorderPixels(inputBM, &dst, top, dstBounds);
+    this->filterBorderPixels(inputBM, &dst, left, dstBounds);
+    this->filterInteriorPixels(inputBM, &dst, interior, dstBounds);
+    this->filterBorderPixels(inputBM, &dst, right, dstBounds);
+    this->filterBorderPixels(inputBM, &dst, bottom, dstBounds);
+    return SkSpecialImage::MakeFromRaster(SkIRect::MakeWH(dstBounds.width(), dstBounds.height()),
                                           dst);
 }
 
diff --git a/src/effects/SkMorphologyImageFilter.cpp b/src/effects/SkMorphologyImageFilter.cpp
index 6ee409ef06..d985562c62 100644
--- a/src/effects/SkMorphologyImageFilter.cpp
+++ b/src/effects/SkMorphologyImageFilter.cpp
@@ -543,7 +543,7 @@ sk_sp<SkSpecialImage> SkMorphologyImageFilter::onFilterImage(SkSpecialImage* sou
     }
 
     SkIRect bounds;
-    input = this->applyCropRect(this->mapContext(ctx), input.get(), &inputOffset, &bounds);
+    input = this->applyCropRectAndPad(this->mapContext(ctx), input.get(), &inputOffset, &bounds);
     if (!input) {
         return nullptr;
     }
diff --git a/src/gpu/effects/GrMatrixConvolutionEffect.cpp b/src/gpu/effects/GrMatrixConvolutionEffect.cpp
index f7e6675c80..cd1ef1a623 100644
--- a/src/gpu/effects/GrMatrixConvolutionEffect.cpp
+++ b/src/gpu/effects/GrMatrixConvolutionEffect.cpp
@@ -144,8 +144,8 @@ void GrGLMatrixConvolutionEffect::onSetData(const GrGLSLProgramDataManager& pdma
     fDomain.setData(pdman, conv.domain(), proxy);
 }
 
-GrMatrixConvolutionEffect::GrMatrixConvolutionEffect(sk_sp<GrTextureProxy> proxy,
-                                                     const SkIRect& bounds,
+GrMatrixConvolutionEffect::GrMatrixConvolutionEffect(sk_sp<GrTextureProxy> srcProxy,
+                                                     const SkIRect& srcBounds,
                                                      const SkISize& kernelSize,
                                                      const SkScalar* kernel,
                                                      SkScalar gain,
@@ -156,9 +156,11 @@ GrMatrixConvolutionEffect::GrMatrixConvolutionEffect(sk_sp<GrTextureProxy> proxy
         // To advertise either the modulation or opaqueness optimizations we'd have to examine the
         // parameters.
         : INHERITED(kGrMatrixConvolutionEffect_ClassID, kNone_OptimizationFlags)
-        , fCoordTransform(proxy.get())
-        , fDomain(proxy.get(), GrTextureDomain::MakeTexelDomainForMode(bounds, tileMode), tileMode)
-        , fTextureSampler(std::move(proxy))
+        , fCoordTransform(srcProxy.get())
+        , fDomain(srcProxy.get(),
+                  GrTextureDomain::MakeTexelDomainForMode(srcBounds, tileMode),
+                  tileMode)
+        , fTextureSampler(std::move(srcProxy))
         , fKernelSize(kernelSize)
         , fGain(SkScalarToFloat(gain))
         , fBias(SkScalarToFloat(bias) / 255.0f)
@@ -255,8 +257,8 @@ static void fill_in_2D_gaussian_kernel(float* kernel, int width, int height,
 
 // Static function to create a 2D convolution
 std::unique_ptr<GrFragmentProcessor> GrMatrixConvolutionEffect::MakeGaussian(
-        sk_sp<GrTextureProxy> proxy,
-        const SkIRect& bounds,
+        sk_sp<GrTextureProxy> srcProxy,
+        const SkIRect& srcBounds,
         const SkISize& kernelSize,
         SkScalar gain,
         SkScalar bias,
@@ -270,8 +272,8 @@ std::unique_ptr<GrFragmentProcessor> GrMatrixConvolutionEffect::MakeGaussian(
     fill_in_2D_gaussian_kernel(kernel, kernelSize.width(), kernelSize.height(), sigmaX, sigmaY);
 
     return std::unique_ptr<GrFragmentProcessor>(
-            new GrMatrixConvolutionEffect(std::move(proxy), bounds, kernelSize, kernel, gain, bias,
-                                          kernelOffset, tileMode, convolveAlpha));
+            new GrMatrixConvolutionEffect(std::move(srcProxy), srcBounds, kernelSize, kernel,
+                                          gain, bias, kernelOffset, tileMode, convolveAlpha));
 }
 
 GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrMatrixConvolutionEffect);
diff --git a/src/gpu/effects/GrMatrixConvolutionEffect.h b/src/gpu/effects/GrMatrixConvolutionEffect.h
index 2e859b1cdd..844f721864 100644
--- a/src/gpu/effects/GrMatrixConvolutionEffect.h
+++ b/src/gpu/effects/GrMatrixConvolutionEffect.h
@@ -16,8 +16,8 @@
 
 class GrMatrixConvolutionEffect : public GrFragmentProcessor {
 public:
-    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> proxy,
-                                                     const SkIRect& bounds,
+    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> srcProxy,
+                                                     const SkIRect& srcBounds,
                                                      const SkISize& kernelSize,
                                                      const SkScalar* kernel,
                                                      SkScalar gain,
@@ -26,12 +26,12 @@ public:
                                                      GrTextureDomain::Mode tileMode,
                                                      bool convolveAlpha) {
         return std::unique_ptr<GrFragmentProcessor>(
-                new GrMatrixConvolutionEffect(std::move(proxy), bounds, kernelSize, kernel, gain,
-                                              bias, kernelOffset, tileMode, convolveAlpha));
+                new GrMatrixConvolutionEffect(std::move(srcProxy), srcBounds, kernelSize, kernel,
+                                              gain, bias, kernelOffset, tileMode, convolveAlpha));
     }
 
-    static std::unique_ptr<GrFragmentProcessor> MakeGaussian(sk_sp<GrTextureProxy> proxy,
-                                                             const SkIRect& bounds,
+    static std::unique_ptr<GrFragmentProcessor> MakeGaussian(sk_sp<GrTextureProxy> srcProxy,
+                                                             const SkIRect& srcBounds,
                                                              const SkISize& kernelSize,
                                                              SkScalar gain,
                                                              SkScalar bias,
@@ -55,8 +55,10 @@ public:
     std::unique_ptr<GrFragmentProcessor> clone() const override;
 
 private:
-    GrMatrixConvolutionEffect(sk_sp<GrTextureProxy> proxy,
-                              const SkIRect& bounds,
+    // srcProxy is the texture that is going to be convolved
+    // srcBounds is the subset of 'srcProxy' that will be used (e.g., for clamp mode)
+    GrMatrixConvolutionEffect(sk_sp<GrTextureProxy> srcProxy,
+                              const SkIRect& srcBounds,
                               const SkISize& kernelSize,
                               const SkScalar* kernel,
                               SkScalar gain,
diff --git a/tests/ImageFilterTest.cpp b/tests/ImageFilterTest.cpp
index e3c7e35e74..556c3f040d 100644
--- a/tests/ImageFilterTest.cpp
+++ b/tests/ImageFilterTest.cpp
@@ -850,7 +850,7 @@ 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());
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }
@@ -861,7 +861,7 @@ 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());
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }
@@ -872,7 +872,7 @@ 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());
+    bounds = filter2->filterBounds(bounds, SkMatrix::I(), SkImageFilter::kReverse_MapDirection);
 
     REPORTER_ASSERT(reporter, bounds == expectedBounds);
 }