[skif] Update SkShaderImageFilter to use FilterResult
Bug: skia:9282
Bug: b/263134493
Bug: oss-fuzz:59022
Change-Id: I059330de6f601c44ae375e94f0370478e0352352
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/697099
Reviewed-by: Robert Phillips <robertphillips@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Michael Ludwig <michaelludwig@google.com>
diff --git a/gm/composecolorfilter.cpp b/gm/composecolorfilter.cpp
index dbb3658..ca5c574 100644
--- a/gm/composecolorfilter.cpp
+++ b/gm/composecolorfilter.cpp
@@ -22,7 +22,9 @@
#include "include/core/SkTileMode.h"
#include "include/core/SkTypes.h"
#include "include/effects/SkGradientShader.h"
+#include "include/effects/SkImageFilters.h"
#include "include/effects/SkLumaColorFilter.h"
+#include "include/effects/SkPerlinNoiseShader.h"
#include "include/effects/SkRuntimeEffect.h"
#include "src/core/SkRuntimeEffectPriv.h"
#include "tools/Resources.h"
@@ -100,3 +102,46 @@
canvas->translate(0, 100);
}
}
+
+DEF_SIMPLE_GM(composeCFIF, canvas, 604, 200) {
+ // This GM draws a ::Shader image filter composed with a ::ColorFilter image filter in two
+ // ways (direct and via ::Compose). This ensures the use (or non-use in this case) of the source
+ // image is the same across both means of composition.
+ auto cf = MakeTintColorFilter(0xff300000, 0xffa00000, /*useSkSL=*/false);
+ auto shader = SkPerlinNoiseShader::MakeTurbulence(0.01f, 0.01f, 2, 0.f);
+
+ auto shaderIF = SkImageFilters::Shader(shader, SkImageFilters::Dither::kNo);
+ auto directCompose = SkImageFilters::ColorFilter(cf, shaderIF);
+ auto indirectCompose = SkImageFilters::Compose(
+ /*outer=*/SkImageFilters::ColorFilter(cf, nullptr),
+ /*inner=*/shaderIF);
+
+ { // Directly draw the shader composed with the color filter
+ canvas->save();
+ canvas->clipRect({0, 0, 200, 200});
+ SkPaint p;
+ p.setShader(shader);
+ p.setColorFilter(cf);
+ canvas->drawPaint(p);
+ canvas->restore();
+ }
+ canvas->translate(202, 0);
+ { // Draw with the directly composed image filter
+ canvas->save();
+ canvas->clipRect({0, 0, 200, 200});
+ SkPaint p;
+ p.setImageFilter(directCompose);
+ canvas->drawPaint(p);
+ canvas->restore();
+ }
+ canvas->translate(202, 0);
+ {
+ // Draw with the indirectly composed image filter
+ canvas->save();
+ canvas->clipRect({0, 0, 200, 200});
+ SkPaint p;
+ p.setImageFilter(indirectCompose);
+ canvas->drawPaint(p);
+ canvas->restore();
+ }
+}
diff --git a/include/effects/SkImageFilters.h b/include/effects/SkImageFilters.h
index e0ca7aa..a4bfad8 100644
--- a/include/effects/SkImageFilters.h
+++ b/include/effects/SkImageFilters.h
@@ -399,6 +399,9 @@
* Like Image() and Picture(), this is a leaf filter that can be used to introduce inputs to
* a complex filter graph, but should generally be combined with a filter that as at least
* one null input to use the implicit source image.
+ *
+ * Returns an image filter that evaluates to transparent black if 'shader' is null.
+ *
* @param shader The shader that fills the result image
*/
static sk_sp<SkImageFilter> Shader(sk_sp<SkShader> shader, const CropRect& cropRect = {}) {
diff --git a/relnotes/skimagefilters_shader.md b/relnotes/skimagefilters_shader.md
new file mode 100644
index 0000000..e227ba7
--- /dev/null
+++ b/relnotes/skimagefilters_shader.md
@@ -0,0 +1,4 @@
+`SkImageFilters::Shader` now returns a non-null image filter if the input `sk_sp<SkShader>` is
+null. The returned filter evaluates to transparent black, which is equivalent to a null or empty
+shader. Previously, returning a null image filter would mean that the dynamic source image could
+be surprisingly injected into the filter evaluation where it might not have been intended.
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 993c52e..a7c419b 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -147,14 +147,15 @@
}
SkImageFilter_Base::SkImageFilter_Base(sk_sp<SkImageFilter> const* inputs,
- int inputCount, const SkRect* cropRect)
- : fUsesSrcInput(false)
+ int inputCount, const SkRect* cropRect,
+ std::optional<bool> usesSrc)
+ : fUsesSrcInput(usesSrc.has_value() ? *usesSrc : false)
, fCropRect(cropRect)
, fUniqueID(next_image_filter_unique_id()) {
fInputs.reset(inputCount);
for (int i = 0; i < inputCount; ++i) {
- if (!inputs[i] || as_IFB(inputs[i])->fUsesSrcInput) {
+ if (!usesSrc.has_value() && (!inputs[i] || as_IFB(inputs[i])->usesSource())) {
fUsesSrcInput = true;
}
fInputs[i] = inputs[i];
diff --git a/src/core/SkImageFilterTypes.cpp b/src/core/SkImageFilterTypes.cpp
index 4828976..eb14478 100644
--- a/src/core/SkImageFilterTypes.cpp
+++ b/src/core/SkImageFilterTypes.cpp
@@ -745,4 +745,21 @@
return surface.snap();
}
+FilterResult FilterResult::MakeFromShader(const Context& ctx,
+ sk_sp<SkShader> shader,
+ bool dither) {
+ if (!shader) {
+ return {};
+ }
+
+ AutoSurface surface{ctx, ctx.desiredOutput(), /*renderInParameterSpace=*/true};
+ if (surface) {
+ SkPaint paint;
+ paint.setShader(shader);
+ paint.setDither(dither);
+ surface->drawPaint(paint);
+ }
+ return surface.snap();
+}
+
} // end namespace skif
diff --git a/src/core/SkImageFilterTypes.h b/src/core/SkImageFilterTypes.h
index 8b406d9..76f725e 100644
--- a/src/core/SkImageFilterTypes.h
+++ b/src/core/SkImageFilterTypes.h
@@ -656,11 +656,19 @@
// Renders the 'pic', clipped by 'cullRect', into an optimally sized surface (depending on
// picture bounds and 'ctx's desired output). The picture is transformed by the context's
- // layer matrix. Treats null pictures as full transparent.
+ // layer matrix. Treats null pictures as fully transparent.
static FilterResult MakeFromPicture(const Context& ctx,
sk_sp<SkPicture> pic,
ParameterSpace<SkRect> cullRect);
+ // Renders 'shader' into a surface that fills the context's desired output bounds. Treats null
+ // shaders as fully transparent.
+ // TODO: Update 'dither' to SkImageFilters::Dither, but that cannot be forward declared at the
+ // moment because SkImageFilters is a class and not a namespace.
+ static FilterResult MakeFromShader(const Context& ctx,
+ sk_sp<SkShader> shader,
+ bool dither);
+
// Bilinear is used as the default because it can be downgraded to nearest-neighbor when the
// final transform is pixel-aligned, and chaining multiple bilinear samples and transforms is
// assumed to be visually close enough to sampling once at highest quality and final transform.
diff --git a/src/core/SkImageFilter_Base.h b/src/core/SkImageFilter_Base.h
index d4412c1..56a5ba0 100644
--- a/src/core/SkImageFilter_Base.h
+++ b/src/core/SkImageFilter_Base.h
@@ -16,6 +16,8 @@
#include "src/core/SkImageFilterTypes.h"
+#include <optional>
+
class GrFragmentProcessor;
class GrRecordingContext;
@@ -103,6 +105,9 @@
// color other than transparent black.
bool affectsTransparentBlack() const;
+ // Returns true if this image filter graph references the Context's source image.
+ bool usesSource() const { return fUsesSrcInput; }
+
/**
* Most ImageFilters can natively handle scaling and translate components in the CTM. Only
* some of them can handle affine (or more complex) matrices. Some may only handle translation.
@@ -195,8 +200,9 @@
kYes = true
};
+ // If 'usesSrc' is not provided, this filter uses the OR of all input's usesSource() values.
SkImageFilter_Base(sk_sp<SkImageFilter> const* inputs, int inputCount,
- const SkRect* cropRect);
+ const SkRect* cropRect, std::optional<bool> usesSrc = {});
~SkImageFilter_Base() override;
diff --git a/src/effects/imagefilters/SkComposeImageFilter.cpp b/src/effects/imagefilters/SkComposeImageFilter.cpp
index e1500f0..405baab 100644
--- a/src/effects/imagefilters/SkComposeImageFilter.cpp
+++ b/src/effects/imagefilters/SkComposeImageFilter.cpp
@@ -16,6 +16,7 @@
#include "src/core/SkImageFilter_Base.h"
#include "src/core/SkSpecialImage.h"
+#include <optional>
#include <utility>
class SkMatrix;
@@ -26,7 +27,10 @@
class SkComposeImageFilter final : public SkImageFilter_Base {
public:
explicit SkComposeImageFilter(sk_sp<SkImageFilter> inputs[2])
- : INHERITED(inputs, 2, nullptr) {
+ : INHERITED(inputs, 2, nullptr,
+ // Compose only uses the source if the inner filter uses the source image.
+ // Any outer reference to source is rebound to the result of the inner.
+ inputs[1] ? as_IFB(inputs[1])->usesSource() : false) {
SkASSERT(inputs[0].get());
SkASSERT(inputs[1].get());
}
diff --git a/src/effects/imagefilters/SkShaderImageFilter.cpp b/src/effects/imagefilters/SkShaderImageFilter.cpp
index a32eca4..b7dd646 100644
--- a/src/effects/imagefilters/SkShaderImageFilter.cpp
+++ b/src/effects/imagefilters/SkShaderImageFilter.cpp
@@ -5,25 +5,22 @@
* found in the LICENSE file.
*/
-#include "include/core/SkCanvas.h"
#include "include/core/SkColorSpace.h"
#include "include/core/SkFlattenable.h"
#include "include/core/SkImageFilter.h"
-#include "include/core/SkMatrix.h"
#include "include/core/SkPaint.h"
-#include "include/core/SkPoint.h"
#include "include/core/SkRect.h"
#include "include/core/SkRefCnt.h"
-#include "include/core/SkScalar.h"
#include "include/core/SkShader.h"
#include "include/core/SkTypes.h"
#include "include/effects/SkImageFilters.h"
+#include "src/core/SkImageFilterTypes.h"
#include "src/core/SkImageFilter_Base.h"
#include "src/core/SkPicturePriv.h"
#include "src/core/SkReadBuffer.h"
-#include "src/core/SkSpecialImage.h"
-#include "src/core/SkSpecialSurface.h"
+#include "src/core/SkRectPriv.h"
#include "src/core/SkWriteBuffer.h"
+#include "src/effects/imagefilters/SkCropImageFilter.h"
#include <utility>
@@ -31,14 +28,17 @@
class SkShaderImageFilter final : public SkImageFilter_Base {
public:
- SkShaderImageFilter(sk_sp<SkShader> shader, SkImageFilters::Dither dither, const SkRect* rect)
- : INHERITED(nullptr, 0, rect)
+ SkShaderImageFilter(sk_sp<SkShader> shader, SkImageFilters::Dither dither)
+ : SkImageFilter_Base(nullptr, 0, nullptr)
, fShader(std::move(shader))
, fDither(dither) {}
+ SkRect computeFastBounds(const SkRect& /*bounds*/) const override {
+ return fShader ? SkRectPriv::MakeLargeS32() : SkRect::MakeEmpty();
+ }
+
protected:
void flatten(SkWriteBuffer&) const override;
- sk_sp<SkSpecialImage> onFilterImage(const Context&, SkIPoint* offset) const override;
private:
friend void ::SkRegisterShaderImageFilterFlattenable();
@@ -46,10 +46,22 @@
bool onAffectsTransparentBlack() const override { return true; }
+ MatrixCapability onGetCTMCapability() const override { return MatrixCapability::kComplex; }
+
+ skif::FilterResult onFilterImage(const skif::Context&) const override;
+
+ skif::LayerSpace<SkIRect> onGetInputLayerBounds(
+ const skif::Mapping&,
+ const skif::LayerSpace<SkIRect>& desiredOutput,
+ const skif::LayerSpace<SkIRect>& contentBounds,
+ VisitChildren) const override;
+
+ skif::LayerSpace<SkIRect> onGetOutputLayerBounds(
+ const skif::Mapping&,
+ const skif::LayerSpace<SkIRect>& contentBounds) const override;
+
sk_sp<SkShader> fShader;
SkImageFilters::Dither fDither;
-
- using INHERITED = SkImageFilter_Base;
};
} // end namespace
@@ -57,7 +69,11 @@
sk_sp<SkImageFilter> SkImageFilters::Shader(sk_sp<SkShader> shader,
Dither dither,
const CropRect& cropRect) {
- return sk_sp<SkImageFilter>(new SkShaderImageFilter(std::move(shader), dither, cropRect));
+ sk_sp<SkImageFilter> filter{new SkShaderImageFilter(std::move(shader), dither)};
+ if (cropRect) {
+ filter = SkMakeCropImageFilter(*cropRect, std::move(filter));
+ }
+ return filter;
}
void SkRegisterShaderImageFilterFlattenable() {
@@ -88,48 +104,36 @@
}
void SkShaderImageFilter::flatten(SkWriteBuffer& buffer) const {
- this->INHERITED::flatten(buffer);
+ this->SkImageFilter_Base::flatten(buffer);
buffer.writeFlattenable(fShader.get());
buffer.writeBool(fDither == SkImageFilters::Dither::kYes);
}
///////////////////////////////////////////////////////////////////////////////////////////////////
-sk_sp<SkSpecialImage> SkShaderImageFilter::onFilterImage(const Context& ctx,
- SkIPoint* offset) const {
- SkIRect bounds;
- const SkIRect srcBounds = SkIRect::MakeWH(ctx.sourceImage()->width(),
- ctx.sourceImage()->height());
- if (!this->applyCropRect(ctx, srcBounds, &bounds)) {
- return nullptr;
+skif::FilterResult SkShaderImageFilter::onFilterImage(const skif::Context& ctx) const {
+ const bool dither = fDither == SkImageFilters::Dither::kYes;
+ return skif::FilterResult::MakeFromShader(ctx, fShader, dither);
+}
+
+skif::LayerSpace<SkIRect> SkShaderImageFilter::onGetInputLayerBounds(
+ const skif::Mapping&,
+ const skif::LayerSpace<SkIRect>&,
+ const skif::LayerSpace<SkIRect>&,
+ VisitChildren) const {
+ // This is a leaf filter, it requires no input and no further recursion
+ return skif::LayerSpace<SkIRect>::Empty();
+}
+
+skif::LayerSpace<SkIRect> SkShaderImageFilter::onGetOutputLayerBounds(
+ const skif::Mapping&,
+ const skif::LayerSpace<SkIRect>&) const {
+ if (fShader) {
+ // The output of a shader is infinite, unless we were to inspect the shader for a decal
+ // tile mode around a gradient or image.
+ return skif::LayerSpace<SkIRect>(SkRectPriv::MakeILarge());
+ } else {
+ // An empty shader is fully transparent
+ return skif::LayerSpace<SkIRect>::Empty();
}
-
- sk_sp<SkSpecialSurface> surf(ctx.makeSurface(bounds.size()));
- if (!surf) {
- return nullptr;
- }
-
- SkCanvas* canvas = surf->getCanvas();
- SkASSERT(canvas);
-
- canvas->clear(0x0);
-
- SkMatrix matrix(ctx.ctm());
- matrix.postTranslate(SkIntToScalar(-bounds.left()), SkIntToScalar(-bounds.top()));
- SkRect rect = SkRect::MakeIWH(bounds.width(), bounds.height());
- SkMatrix inverse;
- if (matrix.invert(&inverse)) {
- inverse.mapRect(&rect);
- }
- canvas->setMatrix(matrix);
- if (rect.isFinite()) {
- SkPaint paint;
- paint.setShader(fShader);
- paint.setDither(fDither == SkImageFilters::Dither::kYes);
- canvas->drawRect(rect, paint);
- }
-
- offset->fX = bounds.fLeft;
- offset->fY = bounds.fTop;
- return surf->makeImageSnapshot();
}
