Remove GrColorSpaceXform from GrGradientEffect

Use a local xform to convert color stops for analytic gradients.
For texture-based gradients, wrap the FP with an xform effect.
To simplify this code, add a new AdjustFP helper to do the color
xform, and also add the MulOutputByInputAlpha stage.

Bug: skia:
Change-Id: Icde19b5ec1c66aae76f894e9978c90a5f00c852e
Reviewed-on: https://skia-review.googlesource.com/62500
Reviewed-by: Florin Malita <fmalita@chromium.org>
Reviewed-by: Brian Salomon <bsalomon@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>

7 files changed, 74 insertions, 89 deletions

diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 35e8342f7a..2e784bb987 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -1147,11 +1147,11 @@ SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
 
 #if SK_SUPPORT_GPU
 
+#include "GrColorSpaceXform.h"
 #include "GrContext.h"
 #include "GrShaderCaps.h"
 #include "GrTextureStripAtlas.h"
 #include "gl/GrGLContext.h"
-#include "glsl/GrGLSLColorSpaceXformHelper.h"
 #include "glsl/GrGLSLFragmentShaderBuilder.h"
 #include "glsl/GrGLSLProgramDataManager.h"
 #include "glsl/GrGLSLUniformHandler.h"
@@ -1254,9 +1254,6 @@ void GrGradientEffect::GLSLProcessor::onSetData(const GrGLSLProgramDataManager&
                 pdman.set1f(fFSYUni, yCoord);
                 fCachedYCoord = yCoord;
             }
-            if (SkToBool(e.fColorSpaceXform)) {
-                fColorSpaceHelper.setData(pdman, e.fColorSpaceXform.get());
-            }
             break;
         }
     }
@@ -1295,8 +1292,6 @@ uint32_t GrGradientEffect::GLSLProcessor::GenBaseGradientKey(const GrProcessor&
             break;
     }
 
-    key |= GrColorSpaceXform::XformKey(e.fColorSpaceXform.get()) << kReservedBits;
-
     return key;
 }
 
@@ -1409,9 +1404,10 @@ void GrGradientEffect::GLSLProcessor::emitAnalyticalColor(GrGLSLFPFragmentBuilde
     if (GrGradientEffect::kAfterInterp_PremulType == ge.getPremulType()) {
         fragBuilder->codeAppend("colorTemp.rgb *= colorTemp.a;");
     }
-    if (ge.fColorSpaceXform) {
-        fragBuilder->codeAppend("colorTemp.rgb = clamp(colorTemp.rgb, 0, colorTemp.a);");
-    }
+
+    // If the input colors were floats, or there was a color space xform, we may end up out of
+    // range. The simplest solution is to always clamp our (premul) value here.
+    fragBuilder->codeAppend("colorTemp.rgb = clamp(colorTemp.rgb, 0, colorTemp.a);");
 
     fragBuilder->codeAppendf("%s = %s * colorTemp;", outputColor, inputColor);
 }
@@ -1430,14 +1426,12 @@ void GrGradientEffect::GLSLProcessor::emitColor(GrGLSLFPFragmentBuilder* fragBui
         return;
     }
 
-    fColorSpaceHelper.emitCode(uniformHandler, ge.fColorSpaceXform.get());
-
     const char* fsyuni = uniformHandler->getUniformCStr(fFSYUni);
 
     fragBuilder->codeAppendf("half2 coord = half2(%s, %s);", gradientTValue, fsyuni);
     fragBuilder->codeAppendf("%s = ", outputColor);
     fragBuilder->appendTextureLookupAndModulate(inputColor, texSamplers[0], "coord",
-                                                kFloat2_GrSLType, &fColorSpaceHelper);
+                                                kFloat2_GrSLType);
     fragBuilder->codeAppend(";");
 }
 
@@ -1457,7 +1451,6 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
     fIsOpaque = shader.isOpaque();
 
     fColorType = this->determineColorType(shader);
-    fColorSpaceXform = std::move(args.fColorSpaceXform);
     fWrapMode = args.fWrapMode;
 
     if (kTexture_ColorType == fColorType) {
@@ -1471,10 +1464,12 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
         }
 
         // Convert input colors to GrColor4f, possibly premul, and apply color space xform
+        auto colorSpaceXform = GrColorSpaceXform::Make(shader.fColorSpace.get(),
+                                                       args.fDstColorSpace);
         SkASSERT(shader.fOrigColors && shader.fOrigColors4f);
         fColors4f.setCount(shader.fColorCount);
         for (int i = 0; i < shader.fColorCount; ++i) {
-            if (args.fGammaCorrect) {
+            if (args.fDstColorSpace) {
                 fColors4f[i] = GrColor4f::FromSkColor4f(shader.fOrigColors4f[i]);
             } else {
                 GrColor grColor = SkColorToUnpremulGrColor(shader.fOrigColors[i]);
@@ -1485,9 +1480,9 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
                 fColors4f[i] = fColors4f[i].premul();
             }
 
-            if (fColorSpaceXform) {
+            if (colorSpaceXform) {
                 // We defer clamping to after interpolation (see emitAnalyticalColor)
-                fColors4f[i] = fColorSpaceXform->unclampedXform(fColors4f[i]);
+                fColors4f[i] = colorSpaceXform->unclampedXform(fColors4f[i]);
             }
         }
 
@@ -1512,7 +1507,7 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
         case kTexture_ColorType:
             SkGradientShaderBase::GradientBitmapType bitmapType =
                 SkGradientShaderBase::GradientBitmapType::kLegacy;
-            if (args.fGammaCorrect) {
+            if (args.fDstColorSpace) {
                 // Try to use F16 if we can
                 if (args.fContext->caps()->isConfigTexturable(kRGBA_half_GrPixelConfig)) {
                     bitmapType = SkGradientShaderBase::GradientBitmapType::kHalfFloat;
@@ -1580,7 +1575,6 @@ GrGradientEffect::GrGradientEffect(ClassID classID, const CreateArgs& args, bool
 GrGradientEffect::GrGradientEffect(const GrGradientEffect& that)
         : INHERITED(that.classID(), OptFlags(that.fIsOpaque))
         , fColors4f(that.fColors4f)
-        , fColorSpaceXform(that.fColorSpaceXform)
         , fPositions(that.fPositions)
         , fWrapMode(that.fWrapMode)
         , fCoordTransform(that.fCoordTransform)
@@ -1634,7 +1628,7 @@ bool GrGradientEffect::onIsEqual(const GrFragmentProcessor& processor) const {
             }
         }
     }
-    return GrColorSpaceXform::Equals(this->fColorSpaceXform.get(), ge.fColorSpaceXform.get());
+    return true;
 }
 
 #if GR_TEST_UTILS
diff --git a/src/shaders/gradients/SkGradientShaderPriv.h b/src/shaders/gradients/SkGradientShaderPriv.h
index 1310589781..775cb39b85 100644
--- a/src/shaders/gradients/SkGradientShaderPriv.h
+++ b/src/shaders/gradients/SkGradientShaderPriv.h
@@ -280,7 +280,6 @@ static inline int next_dither_toggle(int toggle) {
 #include "GrColorSpaceXform.h"
 #include "GrCoordTransform.h"
 #include "GrFragmentProcessor.h"
-#include "glsl/GrGLSLColorSpaceXformHelper.h"
 #include "glsl/GrGLSLFragmentProcessor.h"
 #include "glsl/GrGLSLProgramDataManager.h"
 
@@ -319,13 +318,11 @@ public:
                    const SkGradientShaderBase* shader,
                    const SkMatrix* matrix,
                    SkShader::TileMode tileMode,
-                   sk_sp<GrColorSpaceXform> colorSpaceXform,
-                   bool gammaCorrect)
+                   const SkColorSpace* dstColorSpace)
                 : fContext(context)
                 , fShader(shader)
                 , fMatrix(matrix)
-                , fColorSpaceXform(std::move(colorSpaceXform))
-                , fGammaCorrect(gammaCorrect) {
+                , fDstColorSpace(dstColorSpace) {
             switch (tileMode) {
                 case SkShader::kClamp_TileMode:
                     fWrapMode = GrSamplerState::WrapMode::kClamp;
@@ -343,21 +340,18 @@ public:
                    const SkGradientShaderBase* shader,
                    const SkMatrix* matrix,
                    GrSamplerState::WrapMode wrapMode,
-                   sk_sp<GrColorSpaceXform> colorSpaceXform,
-                   bool gammaCorrect)
+                   const SkColorSpace* dstColorSpace)
                 : fContext(context)
                 , fShader(shader)
                 , fMatrix(matrix)
                 , fWrapMode(wrapMode)
-                , fColorSpaceXform(std::move(colorSpaceXform))
-                , fGammaCorrect(gammaCorrect) {}
+                , fDstColorSpace(dstColorSpace) {}
 
         GrContext*                  fContext;
         const SkGradientShaderBase* fShader;
         const SkMatrix*             fMatrix;
         GrSamplerState::WrapMode    fWrapMode;
-        sk_sp<GrColorSpaceXform>    fColorSpaceXform;
-        bool                        fGammaCorrect;
+        const SkColorSpace*         fDstColorSpace;
     };
 
     class GLSLProcessor;
@@ -404,7 +398,28 @@ protected:
     GrGradientEffect(ClassID classID, const CreateArgs&, bool isOpaque);
     explicit GrGradientEffect(const GrGradientEffect&);  // facilitates clone() implementations
 
-    #if GR_TEST_UTILS
+    // Helper function used by derived class factories to handle color space transformation and
+    // modulation by input alpha.
+    static std::unique_ptr<GrFragmentProcessor> AdjustFP(
+            std::unique_ptr<GrGradientEffect> gradientFP, const CreateArgs& args) {
+        if (!gradientFP->isValid()) {
+            return nullptr;
+        }
+        std::unique_ptr<GrFragmentProcessor> fp;
+        // With analytic gradients, we pre-convert the stops to the destination color space, so no
+        // xform is needed. With texture-based gradients, we leave the data in the source color
+        // space (to avoid clamping if we can't use F16)... Add an extra FP to do the xform.
+        if (kTexture_ColorType == gradientFP->getColorType()) {
+            fp = GrColorSpaceXformEffect::Make(std::move(gradientFP),
+                                               args.fShader->fColorSpace.get(),
+                                               args.fDstColorSpace);
+        } else {
+            fp = std::move(gradientFP);
+        }
+        return GrFragmentProcessor::MulOutputByInputAlpha(std::move(fp));
+    }
+
+#if GR_TEST_UTILS
     /** Helper struct that stores (and populates) parameters to construct a random gradient.
         If fUseColors4f is true, then the SkColor4f factory should be called, with fColors4f and
         fColorSpace. Otherwise, the SkColor factory should be called, with fColors. fColorCount
@@ -441,9 +456,6 @@ private:
 
     SkTDArray<GrColor4f> fColors4f;
 
-    // Only present if a color space transformation is needed
-    sk_sp<GrColorSpaceXform> fColorSpaceXform;
-
     SkTDArray<SkScalar> fPositions;
     GrSamplerState::WrapMode fWrapMode;
 
@@ -532,7 +544,6 @@ private:
     GrGLSLProgramDataManager::UniformHandle fColorsUni;
     GrGLSLProgramDataManager::UniformHandle fExtraStopT;
     GrGLSLProgramDataManager::UniformHandle fFSYUni;
-    GrGLSLColorSpaceXformHelper             fColorSpaceHelper;
 
     typedef GrGLSLFragmentProcessor INHERITED;
 };
diff --git a/src/shaders/gradients/SkLinearGradient.cpp b/src/shaders/gradients/SkLinearGradient.cpp
index 8b439cef56..c0cade38dc 100644
--- a/src/shaders/gradients/SkLinearGradient.cpp
+++ b/src/shaders/gradients/SkLinearGradient.cpp
@@ -100,8 +100,9 @@ public:
     class GLSLLinearProcessor;
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args) {
-        auto processor = std::unique_ptr<GrLinearGradient>(new GrLinearGradient(args));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<GrLinearGradient>(
+                new GrLinearGradient(args)),
+                args);
     }
 
     const char* name() const override { return "Linear Gradient"; }
@@ -212,15 +213,8 @@ std::unique_ptr<GrFragmentProcessor> SkLinearGradient::asFragmentProcessor(
     }
     matrix.postConcat(fPtsToUnit);
 
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(fColorSpace.get(),
-                                                                       args.fDstColorSpace);
-    auto inner = GrLinearGradient::Make(GrGradientEffect::CreateArgs(
-            args.fContext, this, &matrix, fTileMode, std::move(colorSpaceXform),
-            SkToBool(args.fDstColorSpace)));
-    if (!inner) {
-        return nullptr;
-    }
-    return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
+    return GrLinearGradient::Make(GrGradientEffect::CreateArgs(
+            args.fContext, this, &matrix, fTileMode, args.fDstColorSpace));
 }
 
 
diff --git a/src/shaders/gradients/SkRadialGradient.cpp b/src/shaders/gradients/SkRadialGradient.cpp
index 5c31688b93..e704554fcb 100644
--- a/src/shaders/gradients/SkRadialGradient.cpp
+++ b/src/shaders/gradients/SkRadialGradient.cpp
@@ -69,8 +69,9 @@ public:
     class GLSLRadialProcessor;
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args) {
-        auto processor = std::unique_ptr<GrRadialGradient>(new GrRadialGradient(args));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<GrRadialGradient>(
+                new GrRadialGradient(args)),
+                args);
     }
 
     const char* name() const override { return "Radial Gradient"; }
@@ -184,15 +185,9 @@ std::unique_ptr<GrFragmentProcessor> SkRadialGradient::asFragmentProcessor(
         matrix.postConcat(inv);
     }
     matrix.postConcat(fPtsToUnit);
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(fColorSpace.get(),
-                                                                       args.fDstColorSpace);
-    auto inner = GrRadialGradient::Make(GrGradientEffect::CreateArgs(
-            args.fContext, this, &matrix, fTileMode, std::move(colorSpaceXform),
-            SkToBool(args.fDstColorSpace)));
-    if (!inner) {
-        return nullptr;
-    }
-    return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
+
+    return GrRadialGradient::Make(GrGradientEffect::CreateArgs(
+            args.fContext, this, &matrix, fTileMode, args.fDstColorSpace));
 }
 
 #endif
diff --git a/src/shaders/gradients/SkSweepGradient.cpp b/src/shaders/gradients/SkSweepGradient.cpp
index 792024f1c7..551a6fbd02 100644
--- a/src/shaders/gradients/SkSweepGradient.cpp
+++ b/src/shaders/gradients/SkSweepGradient.cpp
@@ -76,8 +76,9 @@ public:
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args, SkScalar tBias,
                                                      SkScalar tScale) {
-        auto processor = std::unique_ptr<GrSweepGradient>(new GrSweepGradient(args, tBias, tScale));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<GrSweepGradient>(
+                new GrSweepGradient(args, tBias, tScale)),
+                args);
     }
 
     const char* name() const override { return "Sweep Gradient"; }
@@ -240,16 +241,10 @@ std::unique_ptr<GrFragmentProcessor> SkSweepGradient::asFragmentProcessor(
     }
     matrix.postConcat(fPtsToUnit);
 
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(fColorSpace.get(),
-                                                                       args.fDstColorSpace);
-    auto inner = GrSweepGradient::Make(
+    return GrSweepGradient::Make(
             GrGradientEffect::CreateArgs(args.fContext, this, &matrix, fTileMode,
-                                         std::move(colorSpaceXform), SkToBool(args.fDstColorSpace)),
+                                         args.fDstColorSpace),
             fTBias, fTScale);
-    if (!inner) {
-        return nullptr;
-    }
-    return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
 }
 
 #endif
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.cpp b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
index 04448b45a1..d84108b98c 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.cpp
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
@@ -130,15 +130,8 @@ void SkTwoPointConicalGradient::flatten(SkWriteBuffer& buffer) const {
 std::unique_ptr<GrFragmentProcessor> SkTwoPointConicalGradient::asFragmentProcessor(
         const AsFPArgs& args) const {
     SkASSERT(args.fContext);
-    sk_sp<GrColorSpaceXform> colorSpaceXform = GrColorSpaceXform::Make(fColorSpace.get(),
-                                                                       args.fDstColorSpace);
-    auto inner = Gr2PtConicalGradientEffect::Make(GrGradientEffect::CreateArgs(
-            args.fContext, this, args.fLocalMatrix, fTileMode, std::move(colorSpaceXform),
-            SkToBool(args.fDstColorSpace)));
-    if (!inner) {
-        return nullptr;
-    }
-    return GrFragmentProcessor::MulOutputByInputAlpha(std::move(inner));
+    return Gr2PtConicalGradientEffect::Make(GrGradientEffect::CreateArgs(
+            args.fContext, this, args.fLocalMatrix, fTileMode, args.fDstColorSpace));
 }
 
 #endif
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp b/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
index a48c8d9d39..f370caf3e7 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
+++ b/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
@@ -64,8 +64,9 @@ public:
     class GLSLEdge2PtConicalProcessor;
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args) {
-        auto processor = std::unique_ptr<Edge2PtConicalEffect>(new Edge2PtConicalEffect(args));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<Edge2PtConicalEffect>(
+                new Edge2PtConicalEffect(args)),
+                args);
     }
 
     const char* name() const override {
@@ -383,9 +384,9 @@ public:
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args, SkScalar focalX,
                                                      bool isFlipped) {
-        auto processor = std::unique_ptr<FocalOutside2PtConicalEffect>(
-                new FocalOutside2PtConicalEffect(args, focalX, isFlipped));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<FocalOutside2PtConicalEffect>(
+                new FocalOutside2PtConicalEffect(args, focalX, isFlipped)),
+                args);
     }
 
     const char* name() const override {
@@ -590,9 +591,9 @@ public:
     class GLSLFocalInside2PtConicalProcessor;
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args, SkScalar focalX) {
-        auto processor = std::unique_ptr<FocalInside2PtConicalEffect>(
-                new FocalInside2PtConicalEffect(args, focalX));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<FocalInside2PtConicalEffect>(
+                new FocalInside2PtConicalEffect(args, focalX)),
+                args);
     }
 
     const char* name() const override {
@@ -848,9 +849,9 @@ public:
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args,
                                                      const CircleConicalInfo& info) {
-        auto processor = std::unique_ptr<CircleInside2PtConicalEffect>(
-                new CircleInside2PtConicalEffect(args, info));
-        return processor->isValid() ? std::move(processor) : nullptr;
+        return GrGradientEffect::AdjustFP(std::unique_ptr<CircleInside2PtConicalEffect>(
+                new CircleInside2PtConicalEffect(args, info)),
+                args);
     }
 
     const char* name() const override { return "Two-Point Conical Gradient Inside"; }
@@ -1069,7 +1070,9 @@ public:
 
     static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args,
                                                      const CircleConicalInfo& info) {
-        return std::unique_ptr<GrFragmentProcessor>(new CircleOutside2PtConicalEffect(args, info));
+        return GrGradientEffect::AdjustFP(std::unique_ptr<CircleOutside2PtConicalEffect>(
+                new CircleOutside2PtConicalEffect(args, info)),
+                args);
     }
 
     const char* name() const override { return "Two-Point Conical Gradient Outside"; }
@@ -1335,7 +1338,7 @@ std::unique_ptr<GrFragmentProcessor> Gr2PtConicalGradientEffect::Make(
     }
 
     GrGradientEffect::CreateArgs newArgs(args.fContext, args.fShader, &matrix, args.fWrapMode,
-                                         std::move(args.fColorSpaceXform), args.fGammaCorrect);
+                                         args.fDstColorSpace);
 
     if (shader.getStartRadius() < kErrorTol) {
         SkScalar focalX;