Fix subset errors in blur, matrix, and morphology filters
Encountered these errors previously, but did not recognize their cause
(see negative marked GM_imagemakewithfilter_crop results on gold). They
came up again when working on fixing snapBackImage() to avoid a copy,
which increased the likelihood of an input image with non-zero origin.
These changes fix the matrix convolution and morphology errors in that
GM so that it now matches the non-crop cases. These had been special
because the last row in that GM didn't require calling applyCropRectAndPad,
so it actually processed an image with a non-zero origin.
The blur fix was discovered when evaluating a blur with a sufficiently
large enough sigma that it needed to be decimated over multiple iterations.
In that case, the second iteration uses a new input proxy so it shouldn't
offset the source coordinates any more.
Change-Id: I7d51025140342c93ca798ca0708c8675ab411beb
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/237125
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/src/core/SkGpuBlurUtils.cpp b/src/core/SkGpuBlurUtils.cpp
index 4ee12e3..2dbdf1c 100644
--- a/src/core/SkGpuBlurUtils.cpp
+++ b/src/core/SkGpuBlurUtils.cpp
@@ -306,6 +306,9 @@
SkIRect dstRect(srcRect);
+ // Map the src rect into proxy space, this only has to happen once since subsequent loops
+ // to decimate will have created a new proxy that has its origin at (0, 0).
+ srcRect.offset(proxyOffset.x(), proxyOffset.y());
std::unique_ptr<GrRenderTargetContext> dstRenderTargetContext;
for (int i = 1; i < scaleFactorX || i < scaleFactorY; i *= 2) {
@@ -364,8 +367,7 @@
dstRenderTargetContext->fillRectToRect(GrFixedClip::Disabled(), std::move(paint), GrAA::kNo,
SkMatrix::I(), SkRect::Make(dstRect),
- SkRect::Make(srcRect.makeOffset(proxyOffset.x(),
- proxyOffset.y())));
+ SkRect::Make(srcRect));
src = dstRenderTargetContext->asTextureProxyRef();
if (!src) {
diff --git a/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp b/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
index d6d4e6b..611f027 100644
--- a/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
+++ b/src/effects/imagefilters/SkMatrixConvolutionImageFilter.cpp
@@ -467,6 +467,13 @@
return nullptr;
}
+ // FIXME (michaelludwig) - Clean this up as part of the imagefilter refactor, some filters
+ // instead require a coord transform on the FP. At very least, be consistent, at best make
+ // it so that filter impls don't need to worry about the subset origin.
+
+ // Must also map the dstBounds since it is used as the src rect in DrawWithFP when
+ // evaluating the FP, and the dst rect just uses the size of dstBounds.
+ dstBounds.offset(input->subset().x(), input->subset().y());
return DrawWithFP(context, std::move(fp), dstBounds, ctx.colorType(), ctx.colorSpace(),
isProtected ? GrProtected::kYes : GrProtected::kNo);
}
diff --git a/src/effects/imagefilters/SkMorphologyImageFilter.cpp b/src/effects/imagefilters/SkMorphologyImageFilter.cpp
index 2f1a008..6518d3c 100644
--- a/src/effects/imagefilters/SkMorphologyImageFilter.cpp
+++ b/src/effects/imagefilters/SkMorphologyImageFilter.cpp
@@ -555,7 +555,8 @@
const SkIRect dstRect = SkIRect::MakeWH(rect.width(), rect.height());
SkIRect srcRect = rect;
-
+ // Map into proxy space
+ srcRect.offset(input->subset().x(), input->subset().y());
SkASSERT(radius.width() > 0 || radius.height() > 0);
if (radius.fWidth > 0) {