Remove color space xform from alpha threshold FP

This does math on the sampled color. In order to do that math in the
destination color space, I split the behavior up - the threshold FP
now operates on the input color, and we construct a series with a
simple texture effect (possibly wrapped in a color xform), then the
threshold.

I also added a GM that verifies this behavior. All other GMs using
this effect were operating on a layer source, which is always
created in the destination color space. The new GM explicitly makes
a DAG with an image source, so the alpha threshold filter needs to
handle any color space mismatch.

Bug: skia:
Change-Id: I1ed08c99f4eed17f68176bf751677a3ae1614fe3
Reviewed-on: https://skia-review.googlesource.com/61942
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

gm/imagealphathreshold.cpp

@@ -7,7 +7,9 @@
 
 #include "gm.h"
 #include "SkAlphaThresholdFilter.h"
+#include "SkImageSource.h"
 #include "SkOffsetImageFilter.h"
+#include "SkRandom.h"
 #include "SkRegion.h"
 #include "SkSurface.h"
 
@@ -153,3 +155,43 @@ private:
 DEF_GM(return new ImageAlphaThresholdGM(true);)
 DEF_GM(return new ImageAlphaThresholdGM(false);)
 DEF_GM(return new ImageAlphaThresholdSurfaceGM();)
+
+//////////////////////////////////////////////////////////////////////////////
+
+static sk_sp<SkImage> make_img() {
+    SkBitmap bitmap;
+    bitmap.allocPixels(SkImageInfo::MakeS32(WIDTH, HEIGHT, kPremul_SkAlphaType));
+    SkCanvas canvas(bitmap);
+
+    SkPaint paint;
+    SkRect rect = SkRect::MakeWH(WIDTH, HEIGHT);
+    SkRandom rnd;
+
+    while (!rect.isEmpty()) {
+        paint.setColor(rnd.nextU() | (0xFF << 24));
+        canvas.drawRect(rect, paint);
+        rect.inset(25, 25);
+    }
+
+    return SkImage::MakeFromBitmap(bitmap);
+}
+
+DEF_SIMPLE_GM_BG(imagealphathreshold_image, canvas, WIDTH * 2, HEIGHT, SK_ColorBLACK) {
+    sk_sp<SkImage> image(make_img());
+
+    SkIRect rects[2];
+    rects[0] = SkIRect::MakeXYWH(0, 150, WIDTH, HEIGHT - 300);
+    rects[1] = SkIRect::MakeXYWH(150, 0, WIDTH - 300, HEIGHT);
+    SkRegion region;
+    region.setRects(rects, 2);
+
+    SkPaint filterPaint;
+    sk_sp<SkImageFilter> imageSource(SkImageSource::Make(image));
+    filterPaint.setImageFilter(SkAlphaThresholdFilter::Make(region, 0.2f, 0.7f,
+                                                            std::move(imageSource)));
+
+    canvas->saveLayer(nullptr, &filterPaint);
+    canvas->restore();
+    canvas->translate(WIDTH, 0);
+    canvas->drawImage(image, 0, 0);
+}

src/effects/GrAlphaThresholdFragmentProcessor.cpp

@@ -34,39 +34,25 @@ public:
         const GrAlphaThresholdFragmentProcessor& _outer =
                 args.fFp.cast<GrAlphaThresholdFragmentProcessor>();
         (void)_outer;
-        auto colorXform = _outer.colorXform();
-        (void)colorXform;
         auto innerThreshold = _outer.innerThreshold();
         (void)innerThreshold;
         auto outerThreshold = _outer.outerThreshold();
         (void)outerThreshold;
-        fColorSpaceHelper.emitCode(args.fUniformHandler, _outer.colorXform().get());
         fInnerThresholdVar = args.fUniformHandler->addUniform(
                 kFragment_GrShaderFlag, kHalf_GrSLType, kDefault_GrSLPrecision, "innerThreshold");
         fOuterThresholdVar = args.fUniformHandler->addUniform(
                 kFragment_GrShaderFlag, kHalf_GrSLType, kDefault_GrSLPrecision, "outerThreshold");
         SkString sk_TransformedCoords2D_0 = fragBuilder->ensureCoords2D(args.fTransformedCoords[0]);
-        SkString sk_TransformedCoords2D_1 = fragBuilder->ensureCoords2D(args.fTransformedCoords[1]);
         fragBuilder->codeAppendf(
-                "half4 _tmpVar1;half4 color = %stexture(%s, %s).%s%s;\nhalf4 mask_color = "
+                "half4 color = %s;\nhalf4 mask_color = texture(%s, %s).%s;\nif "
-                "texture(%s, %s).%s;\nif (float(mask_color.w) < 0.5) {\n    if (color.w > %s) {\n  "
+                "(float(mask_color.w) < 0.5) {\n    if (color.w > %s) {\n        half scale = %s / "
-                "      half scale = %s / color.w;\n        color.xyz *= scale;\n        color.w = "
+                "color.w;\n        color.xyz *= scale;\n        color.w = %s;\n    }\n} else if "
-                "%s;\n    }\n} else if (color.w < %s) {\n    half scale = float(%s) / max(0.001, "
+                "(color.w < %s) {\n    half scale = float(%s) / max(0.001, float(color.w));\n    "
-                "float(color.w));\n    color.xyz *= scale;\n    color.w = %s;\n}\n%s = color;\n",
+                "color.xyz *= scale;\n    color.w = %s;\n}\n%s = color;\n",
-                fColorSpaceHelper.isValid() ? "(_tmpVar1 = " : "",
+                args.fInputColor ? args.fInputColor : "half4(1)",
                 fragBuilder->getProgramBuilder()->samplerVariable(args.fTexSamplers[0]).c_str(),
                 sk_TransformedCoords2D_0.c_str(),
                 fragBuilder->getProgramBuilder()->samplerSwizzle(args.fTexSamplers[0]).c_str(),
-                fColorSpaceHelper.isValid()
-                        ? SkStringPrintf(", half4(clamp((%s * half4(_tmpVar1.rgb, 1.0)).rgb, 0.0, "
-                                         "_tmpVar1.a), _tmpVar1.a))",
-                                         args.fUniformHandler->getUniformCStr(
-                                                 fColorSpaceHelper.gamutXformUniform()))
-                                  .c_str()
-                        : "",
-                fragBuilder->getProgramBuilder()->samplerVariable(args.fTexSamplers[1]).c_str(),
-                sk_TransformedCoords2D_1.c_str(),
-                fragBuilder->getProgramBuilder()->samplerSwizzle(args.fTexSamplers[1]).c_str(),
                 args.fUniformHandler->getUniformCStr(fOuterThresholdVar),
                 args.fUniformHandler->getUniformCStr(fOuterThresholdVar),
                 args.fUniformHandler->getUniformCStr(fOuterThresholdVar),
@@ -81,31 +67,22 @@ private:
         const GrAlphaThresholdFragmentProcessor& _outer =
                 _proc.cast<GrAlphaThresholdFragmentProcessor>();
         {
-            if (fColorSpaceHelper.isValid()) {
-                fColorSpaceHelper.setData(pdman, _outer.colorXform().get());
-            }
             pdman.set1f(fInnerThresholdVar, _outer.innerThreshold());
             pdman.set1f(fOuterThresholdVar, _outer.outerThreshold());
         }
     }
-    UniformHandle fImageVar;
     UniformHandle fMaskVar;
     UniformHandle fInnerThresholdVar;
     UniformHandle fOuterThresholdVar;
-    GrGLSLColorSpaceXformHelper fColorSpaceHelper;
 };
 GrGLSLFragmentProcessor* GrAlphaThresholdFragmentProcessor::onCreateGLSLInstance() const {
     return new GrGLSLAlphaThresholdFragmentProcessor();
 }
 void GrAlphaThresholdFragmentProcessor::onGetGLSLProcessorKey(const GrShaderCaps& caps,
-                                                              GrProcessorKeyBuilder* b) const {
+                                                              GrProcessorKeyBuilder* b) const {}
-    b->add32(GrColorSpaceXform::XformKey(fColorXform.get()));
-}
 bool GrAlphaThresholdFragmentProcessor::onIsEqual(const GrFragmentProcessor& other) const {
     const GrAlphaThresholdFragmentProcessor& that = other.cast<GrAlphaThresholdFragmentProcessor>();
     (void)that;
-    if (fImage != that.fImage) return false;
-    if (fColorXform != that.fColorXform) return false;
     if (fMask != that.fMask) return false;
     if (fInnerThreshold != that.fInnerThreshold) return false;
     if (fOuterThreshold != that.fOuterThreshold) return false;
@@ -114,16 +91,11 @@ bool GrAlphaThresholdFragmentProcessor::onIsEqual(const GrFragmentProcessor& oth
 GrAlphaThresholdFragmentProcessor::GrAlphaThresholdFragmentProcessor(
         const GrAlphaThresholdFragmentProcessor& src)
         : INHERITED(kGrAlphaThresholdFragmentProcessor_ClassID, src.optimizationFlags())
-        , fImage(src.fImage)
-        , fColorXform(src.fColorXform)
         , fMask(src.fMask)
         , fInnerThreshold(src.fInnerThreshold)
         , fOuterThreshold(src.fOuterThreshold)
-        , fImageCoordTransform(src.fImageCoordTransform)
         , fMaskCoordTransform(src.fMaskCoordTransform) {
-    this->addTextureSampler(&fImage);
     this->addTextureSampler(&fMask);
-    this->addCoordTransform(&fImageCoordTransform);
     this->addCoordTransform(&fMaskCoordTransform);
 }
 std::unique_ptr<GrFragmentProcessor> GrAlphaThresholdFragmentProcessor::clone() const {
@@ -133,7 +105,6 @@ GR_DEFINE_FRAGMENT_PROCESSOR_TEST(GrAlphaThresholdFragmentProcessor);
 #if GR_TEST_UTILS
 std::unique_ptr<GrFragmentProcessor> GrAlphaThresholdFragmentProcessor::TestCreate(
         GrProcessorTestData* testData) {
-    sk_sp<GrTextureProxy> bmpProxy = testData->textureProxy(GrProcessorUnitTest::kSkiaPMTextureIdx);
     sk_sp<GrTextureProxy> maskProxy = testData->textureProxy(GrProcessorUnitTest::kAlphaTextureIdx);
     // Make the inner and outer thresholds be in (0, 1) exclusive and be sorted correctly.
     float innerThresh = testData->fRandom->nextUScalar1() * .99f + 0.005f;
@@ -145,9 +116,7 @@ std::unique_ptr<GrFragmentProcessor> GrAlphaThresholdFragmentProcessor::TestCrea
     uint32_t x = testData->fRandom->nextULessThan(kMaxWidth - width);
     uint32_t y = testData->fRandom->nextULessThan(kMaxHeight - height);
     SkIRect bounds = SkIRect::MakeXYWH(x, y, width, height);
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrTest::TestColorXform(testData->fRandom);
+    return GrAlphaThresholdFragmentProcessor::Make(std::move(maskProxy), innerThresh, outerThresh,
-    return GrAlphaThresholdFragmentProcessor::Make(std::move(bmpProxy), colorSpaceXform,
-                                                   std::move(maskProxy), innerThresh, outerThresh,
                                                    bounds);
 }
 #endif

src/effects/GrAlphaThresholdFragmentProcessor.fp

@@ -1,5 +1,3 @@
-in uniform sampler2D image;
-in uniform colorSpaceXform colorXform;
 in uniform sampler2D mask;
 in uniform half innerThreshold;
 in uniform half outerThreshold;
@@ -13,21 +11,15 @@ in uniform half outerThreshold;
 }
 
 @make {
-    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> image,
+    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> mask,
-                                                     sk_sp<GrColorSpaceXform> colorXform,
-                                                     sk_sp<GrTextureProxy> mask,
                                                      float innerThreshold,
                                                      float outerThreshold,
                                                      const SkIRect& bounds) {
         return std::unique_ptr<GrFragmentProcessor>(new GrAlphaThresholdFragmentProcessor(
-                image, colorXform, mask, innerThreshold, outerThreshold, bounds));
+                mask, innerThreshold, outerThreshold, bounds));
     }
 }
 
-@coordTransform(image) {
-    SkMatrix::I()
-}
-
 @coordTransform(mask) {
     SkMatrix::MakeTrans(SkIntToScalar(-bounds.x()), SkIntToScalar(-bounds.y()))
 }
@@ -45,8 +37,8 @@ in uniform half outerThreshold;
 }
 
 void main() {
-    half4 color = texture(image, sk_TransformedCoords2D[0], colorXform);
+    half4 color = sk_InColor;
-    half4 mask_color = texture(mask, sk_TransformedCoords2D[1]);
+    half4 mask_color = texture(mask, sk_TransformedCoords2D[0]);
     if (mask_color.a < 0.5) {
         if (color.a > outerThreshold) {
             half scale = outerThreshold / color.a;
@@ -62,7 +54,6 @@ void main() {
 }
 
 @test(testData) {
-    sk_sp<GrTextureProxy> bmpProxy = testData->textureProxy(GrProcessorUnitTest::kSkiaPMTextureIdx);
     sk_sp<GrTextureProxy> maskProxy = testData->textureProxy(GrProcessorUnitTest::kAlphaTextureIdx);
     // Make the inner and outer thresholds be in (0, 1) exclusive and be sorted correctly.
     float innerThresh = testData->fRandom->nextUScalar1() * .99f + 0.005f;
@@ -74,11 +65,6 @@ void main() {
     uint32_t x = testData->fRandom->nextULessThan(kMaxWidth - width);
     uint32_t y = testData->fRandom->nextULessThan(kMaxHeight - height);
     SkIRect bounds = SkIRect::MakeXYWH(x, y, width, height);
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrTest::TestColorXform(testData->fRandom);
+    return GrAlphaThresholdFragmentProcessor::Make(std::move(maskProxy), innerThresh, outerThresh,
-    return GrAlphaThresholdFragmentProcessor::Make(
+                                                   bounds);
-                                std::move(bmpProxy),
-                                colorSpaceXform,
-                                std::move(maskProxy),
-                                innerThresh, outerThresh,
-                                bounds);
 }

src/effects/GrAlphaThresholdFragmentProcessor.h

@@ -18,55 +18,40 @@
 class GrAlphaThresholdFragmentProcessor : public GrFragmentProcessor {
 public:
     inline OptimizationFlags optFlags(float outerThreshold);
-    sk_sp<GrColorSpaceXform> colorXform() const { return fColorXform; }
     float innerThreshold() const { return fInnerThreshold; }
     float outerThreshold() const { return fOuterThreshold; }
 
-    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> image,
+    static std::unique_ptr<GrFragmentProcessor> Make(sk_sp<GrTextureProxy> mask,
-                                                     sk_sp<GrColorSpaceXform>
-                                                             colorXform,
-                                                     sk_sp<GrTextureProxy>
-                                                             mask,
                                                      float innerThreshold,
                                                      float outerThreshold,
                                                      const SkIRect& bounds) {
         return std::unique_ptr<GrFragmentProcessor>(new GrAlphaThresholdFragmentProcessor(
-                image, colorXform, mask, innerThreshold, outerThreshold, bounds));
+                mask, innerThreshold, outerThreshold, bounds));
     }
     GrAlphaThresholdFragmentProcessor(const GrAlphaThresholdFragmentProcessor& src);
     std::unique_ptr<GrFragmentProcessor> clone() const override;
     const char* name() const override { return "AlphaThresholdFragmentProcessor"; }
 
 private:
-    GrAlphaThresholdFragmentProcessor(sk_sp<GrTextureProxy> image,
+    GrAlphaThresholdFragmentProcessor(sk_sp<GrTextureProxy> mask, float innerThreshold,
-                                      sk_sp<GrColorSpaceXform> colorXform,
-                                      sk_sp<GrTextureProxy> mask, float innerThreshold,
                                       float outerThreshold, const SkIRect& bounds)
             : INHERITED(kGrAlphaThresholdFragmentProcessor_ClassID, kNone_OptimizationFlags)
-            , fImage(std::move(image))
-            , fColorXform(colorXform)
             , fMask(std::move(mask))
             , fInnerThreshold(innerThreshold)
             , fOuterThreshold(outerThreshold)
-            , fImageCoordTransform(SkMatrix::I(), fImage.proxy())
             , fMaskCoordTransform(
                       SkMatrix::MakeTrans(SkIntToScalar(-bounds.x()), SkIntToScalar(-bounds.y())),
                       fMask.proxy()) {
-        this->addTextureSampler(&fImage);
         this->addTextureSampler(&fMask);
-        this->addCoordTransform(&fImageCoordTransform);
         this->addCoordTransform(&fMaskCoordTransform);
     }
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
     void onGetGLSLProcessorKey(const GrShaderCaps&, GrProcessorKeyBuilder*) const override;
     bool onIsEqual(const GrFragmentProcessor&) const override;
     GR_DECLARE_FRAGMENT_PROCESSOR_TEST
-    TextureSampler fImage;
-    sk_sp<GrColorSpaceXform> fColorXform;
     TextureSampler fMask;
     float fInnerThreshold;
     float fOuterThreshold;
-    GrCoordTransform fImageCoordTransform;
     GrCoordTransform fMaskCoordTransform;
     typedef GrFragmentProcessor INHERITED;
 };

src/effects/SkAlphaThresholdFilter.cpp

@@ -20,6 +20,7 @@
 #include "GrFixedClip.h"
 #include "GrRenderTargetContext.h"
 #include "GrTextureProxy.h"
+#include "effects/GrSimpleTextureEffect.h"
 #endif
 
 class SK_API SkAlphaThresholdFilterImpl : public SkImageFilter {
@@ -170,19 +171,25 @@ sk_sp<SkSpecialImage> SkAlphaThresholdFilterImpl::onFilterImage(SkSpecialImage*
         }
 
         const OutputProperties& outProps = ctx.outputProperties();
-        sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(input->getColorSpace(),
+        auto textureFP = GrSimpleTextureEffect::Make(std::move(inputProxy), SkMatrix::I());
-                                                                           outProps.colorSpace());
+        textureFP = GrColorSpaceXformEffect::Make(std::move(textureFP), input->getColorSpace(),
-
+                                                  outProps.colorSpace());
-        auto fp = GrAlphaThresholdFragmentProcessor::Make(std::move(inputProxy),
+        if (!textureFP) {
-                                                          std::move(colorSpaceXform),
-                                                          std::move(maskProxy),
-                                                          fInnerThreshold,
-                                                          fOuterThreshold,
-                                                          bounds);
-        if (!fp) {
             return nullptr;
         }
 
+        auto thresholdFP = GrAlphaThresholdFragmentProcessor::Make(std::move(maskProxy),
+                                                                   fInnerThreshold,
+                                                                   fOuterThreshold,
+                                                                   bounds);
+        if (!thresholdFP) {
+            return nullptr;
+        }
+
+        std::unique_ptr<GrFragmentProcessor> fpSeries[] = { std::move(textureFP),
+                                                            std::move(thresholdFP) };
+        auto fp = GrFragmentProcessor::RunInSeries(fpSeries, 2);
+
         return DrawWithFP(context, std::move(fp), bounds, outProps);
     }
 #endif