Avoid 2PtConical gradient flipping

Currently, when startRadius > endRadius conical gradients are inverting their data
(start/end points and radii, colors and positions) to guarantee r0 < r1 at raster time.

But the radii ordering is only mildly interesting to the impl: it controls which of the
two quadratic solutions we select for the inside case, but doesn't fundamentally change
the algorithm.

Furthermore, for the "outside" case, inverting the order is already inconvenient and
needs to be taken into account (both CPU/GPU impls are already tracking this bit of
info).

Instead of transforming the gradient definition, we can detect the inverted case and
adjust the quadratic solution selector.

In practice this means:

  * |edge|    case - no change, the equation is linear
  * |inside|  case - select the smaller root instead of largest
  * |outside| case - [gpu-only] invert the clamp/limiting function

Change-Id: Ie3106464e39a4dd3848dc43671d973f7e1958e63
Reviewed-on: https://skia-review.googlesource.com/61660
Reviewed-by: Brian Salomon <bsalomon@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Florin Malita <fmalita@chromium.org>

5 files changed, 127 insertions, 234 deletions

diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 8eefc81cb6..2925ce14c2 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -288,26 +288,6 @@ void SkGradientShaderBase::flatten(SkWriteBuffer& buffer) const {
     desc.flatten(buffer);
 }
 
-void SkGradientShaderBase::FlipGradientColors(SkColor* colorDst, Rec* recDst,
-                                              SkColor* colorSrc, Rec* recSrc,
-                                              int count) {
-    SkAutoSTArray<8, SkColor> colorsTemp(count);
-    for (int i = 0; i < count; ++i) {
-        int offset = count - i - 1;
-        colorsTemp[i] = colorSrc[offset];
-    }
-    if (count > 2) {
-        SkAutoSTArray<8, Rec> recsTemp(count);
-        for (int i = 0; i < count; ++i) {
-            int offset = count - i - 1;
-            recsTemp[i].fPos = SK_Fixed1 - recSrc[offset].fPos;
-            recsTemp[i].fScale = recSrc[offset].fScale;
-        }
-        memcpy(recDst, recsTemp.get(), count * sizeof(Rec));
-    }
-    memcpy(colorDst, colorsTemp.get(), count * sizeof(SkColor));
-}
-
 static void add_stop_color(SkJumper_GradientCtx* ctx, size_t stop, SkPM4f Fs, SkPM4f Bs) {
     (ctx->fs[0])[stop] = Fs.r();
     (ctx->fs[1])[stop] = Fs.g();
@@ -903,25 +883,11 @@ void SkGradientShaderBase::getGradientTableBitmap(SkBitmap* bitmap,
     }
 }
 
-void SkGradientShaderBase::commonAsAGradient(GradientInfo* info, bool flipGrad) const {
+void SkGradientShaderBase::commonAsAGradient(GradientInfo* info) const {
     if (info) {
         if (info->fColorCount >= fColorCount) {
-            SkColor* colorLoc;
-            Rec*     recLoc;
-            SkAutoSTArray<8, SkColor> colorStorage;
-            SkAutoSTArray<8, Rec> recStorage;
-            if (flipGrad && (info->fColors || info->fColorOffsets)) {
-                colorStorage.reset(fColorCount);
-                recStorage.reset(fColorCount);
-                colorLoc = colorStorage.get();
-                recLoc = recStorage.get();
-                FlipGradientColors(colorLoc, recLoc, fOrigColors, fRecs, fColorCount);
-            } else {
-                colorLoc = fOrigColors;
-                recLoc = fRecs;
-            }
             if (info->fColors) {
-                memcpy(info->fColors, colorLoc, fColorCount * sizeof(SkColor));
+                memcpy(info->fColors, fOrigColors, fColorCount * sizeof(SkColor));
             }
             if (info->fColorOffsets) {
                 if (fColorCount == 2) {
@@ -929,7 +895,7 @@ void SkGradientShaderBase::commonAsAGradient(GradientInfo* info, bool flipGrad)
                     info->fColorOffsets[1] = SK_Scalar1;
                 } else if (fColorCount > 2) {
                     for (int i = 0; i < fColorCount; ++i) {
-                        info->fColorOffsets[i] = SkFixedToScalar(recLoc[i].fPos);
+                        info->fColorOffsets[i] = SkFixedToScalar(fRecs[i].fPos);
                     }
                 }
             }
@@ -1191,36 +1157,10 @@ sk_sp<SkShader> SkGradientShader::MakeTwoPointConical(const SkPoint& start,
 
     ColorStopOptimizer opt(colors, pos, colorCount, mode);
 
-    bool flipGradient = startRadius > endRadius;
-
     SkGradientShaderBase::Descriptor desc;
-
-    if (!flipGradient) {
-        desc_init(&desc, opt.fColors, std::move(colorSpace), opt.fPos, opt.fCount, mode, flags,
-                  localMatrix);
-        return SkTwoPointConicalGradient::Create(start, startRadius, end, endRadius, flipGradient,
-                                                 desc);
-    } else {
-        SkAutoSTArray<8, SkColor4f> colorsNew(opt.fCount);
-        SkAutoSTArray<8, SkScalar> posNew(opt.fCount);
-        for (int i = 0; i < opt.fCount; ++i) {
-            colorsNew[i] = opt.fColors[opt.fCount - i - 1];
-        }
-
-        if (pos) {
-            for (int i = 0; i < opt.fCount; ++i) {
-                posNew[i] = 1 - opt.fPos[opt.fCount - i - 1];
-            }
-            desc_init(&desc, colorsNew.get(), std::move(colorSpace), posNew.get(), opt.fCount, mode,
-                      flags, localMatrix);
-        } else {
-            desc_init(&desc, colorsNew.get(), std::move(colorSpace), nullptr, opt.fCount, mode,
-                      flags, localMatrix);
-        }
-
-        return SkTwoPointConicalGradient::Create(end, endRadius, start, startRadius, flipGradient,
-                                                 desc);
-    }
+    desc_init(&desc, opt.fColors, std::move(colorSpace), opt.fPos, opt.fCount, mode, flags,
+              localMatrix);
+    return SkTwoPointConicalGradient::Create(start, startRadius, end, endRadius, desc);
 }
 
 sk_sp<SkShader> SkGradientShader::MakeSweep(SkScalar cx, SkScalar cy,
diff --git a/src/shaders/gradients/SkGradientShaderPriv.h b/src/shaders/gradients/SkGradientShaderPriv.h
index 30e9440816..0abfd35e99 100644
--- a/src/shaders/gradients/SkGradientShaderPriv.h
+++ b/src/shaders/gradients/SkGradientShaderPriv.h
@@ -212,23 +212,12 @@ protected:
     void flatten(SkWriteBuffer&) const override;
     SK_TO_STRING_OVERRIDE()
 
-    void commonAsAGradient(GradientInfo*, bool flipGrad = false) const;
+    void commonAsAGradient(GradientInfo*) const;
 
     bool onAsLuminanceColor(SkColor*) const override;
 
     void initLinearBitmap(SkBitmap* bitmap) const;
 
-    /*
-     * Takes in pointers to gradient color and Rec info as colorSrc and recSrc respectively.
-     * Count is the number of colors in the gradient
-     * It will then flip all the color and rec information and return in their respective Dst
-     * pointers. It is assumed that space has already been allocated for the Dst pointers.
-     * The rec src and dst are only assumed to be valid if count > 2
-     */
-    static void FlipGradientColors(SkColor* colorDst, Rec* recDst,
-                                   SkColor* colorSrc, Rec* recSrc,
-                                   int count);
-
     bool onAppendStages(const StageRec&) const override;
 
     virtual void appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* tPipeline,
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.cpp b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
index a1634727a9..04448b45a1 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.cpp
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.cpp
@@ -12,14 +12,15 @@
 
 sk_sp<SkShader> SkTwoPointConicalGradient::Create(const SkPoint& c0, SkScalar r0,
                                                   const SkPoint& c1, SkScalar r1,
-                                                  bool flipped, const Descriptor& desc) {
+                                                  const Descriptor& desc) {
     SkMatrix gradientMatrix;
     Type     gradientType;
 
     if (SkScalarNearlyZero((c0 - c1).length())) {
         // Concentric case: we can pretend we're radial (with a tiny twist).
+        const SkScalar scale = 1.0f / SkTMax(r0, r1);
         gradientMatrix = SkMatrix::MakeTrans(-c1.x(), -c1.y());
-        gradientMatrix.postScale(1 / r1, 1 / r1);
+        gradientMatrix.postScale(scale, scale);
 
         gradientType = Type::kRadial;
     } else {
@@ -35,21 +36,19 @@ sk_sp<SkShader> SkTwoPointConicalGradient::Create(const SkPoint& c0, SkScalar r0
         gradientType = Type::kTwoPoint;
     }
 
-    return sk_sp<SkShader>(new SkTwoPointConicalGradient(c0, r0, c1, r1, flipped, desc,
+    return sk_sp<SkShader>(new SkTwoPointConicalGradient(c0, r0, c1, r1, desc,
                                                          gradientType, gradientMatrix));
 }
 
 SkTwoPointConicalGradient::SkTwoPointConicalGradient(
         const SkPoint& start, SkScalar startRadius,
         const SkPoint& end, SkScalar endRadius,
-        bool flippedGrad, const Descriptor& desc,
-        Type type, const SkMatrix& gradientMatrix)
+        const Descriptor& desc, Type type, const SkMatrix& gradientMatrix)
     : SkGradientShaderBase(desc, gradientMatrix)
     , fCenter1(start)
     , fCenter2(end)
     , fRadius1(startRadius)
     , fRadius2(endRadius)
-    , fFlippedGrad(flippedGrad)
     , fType(type)
 {
     // this is degenerate, and should be caught by our caller
@@ -64,18 +63,13 @@ bool SkTwoPointConicalGradient::isOpaque() const {
 }
 
 // Returns the original non-sorted version of the gradient
-SkShader::GradientType SkTwoPointConicalGradient::asAGradient(
-    GradientInfo* info) const {
+SkShader::GradientType SkTwoPointConicalGradient::asAGradient(GradientInfo* info) const {
     if (info) {
-        commonAsAGradient(info, fFlippedGrad);
+        commonAsAGradient(info);
         info->fPoint[0] = fCenter1;
         info->fPoint[1] = fCenter2;
         info->fRadius[0] = fRadius1;
         info->fRadius[1] = fRadius2;
-        if (fFlippedGrad) {
-            SkTSwap(info->fPoint[0], info->fPoint[1]);
-            SkTSwap(info->fRadius[0], info->fRadius[1]);
-        }
     }
     return kConical_GradientType;
 }
@@ -90,7 +84,8 @@ sk_sp<SkFlattenable> SkTwoPointConicalGradient::CreateProc(SkReadBuffer& buffer)
     SkScalar r1 = buffer.readScalar();
     SkScalar r2 = buffer.readScalar();
 
-    if (buffer.readBool()) {    // flipped
+    if (buffer.isVersionLT(SkReadBuffer::k2PtConicalNoFlip_Version) && buffer.readBool()) {
+        // legacy flipped gradient
         SkTSwap(c1, c2);
         SkTSwap(r1, r2);
 
@@ -125,7 +120,6 @@ void SkTwoPointConicalGradient::flatten(SkWriteBuffer& buffer) const {
     buffer.writePoint(fCenter2);
     buffer.writeScalar(fRadius1);
     buffer.writeScalar(fRadius2);
-    buffer.writeBool(fFlippedGrad);
 }
 
 #if SK_SUPPORT_GPU
@@ -150,29 +144,11 @@ std::unique_ptr<GrFragmentProcessor> SkTwoPointConicalGradient::asFragmentProces
 #endif
 
 sk_sp<SkShader> SkTwoPointConicalGradient::onMakeColorSpace(SkColorSpaceXformer* xformer) const {
-    SkSTArray<8, SkColor> origColorsStorage(fColorCount);
-    SkSTArray<8, SkScalar> origPosStorage(fColorCount);
-    SkSTArray<8, SkColor> xformedColorsStorage(fColorCount);
-    SkColor* origColors = origColorsStorage.begin();
-    SkScalar* origPos = fOrigPos ? origPosStorage.begin() : nullptr;
-    SkColor* xformedColors = xformedColorsStorage.begin();
-
-    // Flip if necessary
-    SkPoint center1 = fFlippedGrad ? fCenter2 : fCenter1;
-    SkPoint center2 = fFlippedGrad ? fCenter1 : fCenter2;
-    SkScalar radius1 = fFlippedGrad ? fRadius2 : fRadius1;
-    SkScalar radius2 = fFlippedGrad ? fRadius1 : fRadius2;
-    for (int i = 0; i < fColorCount; i++) {
-        origColors[i] = fFlippedGrad ? fOrigColors[fColorCount - i - 1] : fOrigColors[i];
-        if (origPos) {
-            origPos[i] = fFlippedGrad ? 1.0f - fOrigPos[fColorCount - i - 1] : fOrigPos[i];
-        }
-    }
-
-    xformer->apply(xformedColors, origColors, fColorCount);
-    return SkGradientShader::MakeTwoPointConical(center1, radius1, center2, radius2, xformedColors,
-                                                 origPos, fColorCount, fTileMode, fGradFlags,
-                                                 &this->getLocalMatrix());
+    SkSTArray<8, SkColor> xformedColors(fColorCount);
+    xformer->apply(xformedColors.begin(), fOrigColors, fColorCount);
+    return SkGradientShader::MakeTwoPointConical(fCenter1, fRadius1, fCenter2, fRadius2,
+                                                 xformedColors.begin(), fOrigPos, fColorCount,
+                                                 fTileMode, fGradFlags, &this->getLocalMatrix());
 }
 
 
@@ -205,13 +181,12 @@ void SkTwoPointConicalGradient::toString(SkString* str) const {
 void SkTwoPointConicalGradient::appendGradientStages(SkArenaAlloc* alloc, SkRasterPipeline* p,
                                                      SkRasterPipeline* postPipeline) const {
     const auto dRadius = fRadius2 - fRadius1;
-    SkASSERT(dRadius >= 0);
 
     if (fType == Type::kRadial) {
         p->append(SkRasterPipeline::xy_to_radius);
 
         // Tiny twist: radial computes a t for [0, r2], but we want a t for [r1, r2].
-        auto scale =  fRadius2 / dRadius;
+        auto scale =  SkTMax(fRadius1, fRadius2) / dRadius;
         auto bias  = -fRadius1 / dRadius;
 
         p->append_matrix(alloc, SkMatrix::Concat(SkMatrix::MakeTrans(bias, 0),
@@ -238,24 +213,21 @@ void SkTwoPointConicalGradient::appendGradientStages(SkArenaAlloc* alloc, SkRast
         p->append(SkRasterPipeline::xy_to_2pt_conical_linear, ctx);
         isWellBehaved = false;
     } else {
-        if (dCenter + fRadius1 > fRadius2) {
-            // The focal point is outside the end circle.
-
-            // We want the larger root, per spec:
-            //   "For all values of ω where r(ω) > 0, starting with the value of ω nearest
-            //    to positive infinity and ending with the value of ω nearest to negative
-            //    infinity, draw the circumference of the circle with radius r(ω) at position
-            //    (x(ω), y(ω)), with the color at ω, but only painting on the parts of the
-            //    bitmap that have not yet been painted on by earlier circles in this step for
-            //    this rendering of the gradient."
-            // (https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-createradialgradient)
-            p->append(fFlippedGrad ? SkRasterPipeline::xy_to_2pt_conical_quadratic_min
-                                   : SkRasterPipeline::xy_to_2pt_conical_quadratic_max, ctx);
-            isWellBehaved = false;
-        } else {
-            // The focal point is inside (well-behaved case).
-            p->append(SkRasterPipeline::xy_to_2pt_conical_quadratic_max, ctx);
-        }
+        isWellBehaved = SkScalarAbs(dRadius) >= dCenter;
+        bool isFlipped = isWellBehaved && dRadius < 0;
+
+        // We want the larger root, per spec:
+        //   "For all values of ω where r(ω) > 0, starting with the value of ω nearest
+        //    to positive infinity and ending with the value of ω nearest to negative
+        //    infinity, draw the circumference of the circle with radius r(ω) at position
+        //    (x(ω), y(ω)), with the color at ω, but only painting on the parts of the
+        //    bitmap that have not yet been painted on by earlier circles in this step for
+        //    this rendering of the gradient."
+        // (https://html.spec.whatwg.org/multipage/canvas.html#dom-context-2d-createradialgradient)
+        //
+        // ... except when the gradient is flipped.
+        p->append(isFlipped ? SkRasterPipeline::xy_to_2pt_conical_quadratic_min
+                            : SkRasterPipeline::xy_to_2pt_conical_quadratic_max, ctx);
     }
 
     if (!isWellBehaved) {
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient.h b/src/shaders/gradients/SkTwoPointConicalGradient.h
index 4b2827117c..96039e4771 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient.h
+++ b/src/shaders/gradients/SkTwoPointConicalGradient.h
@@ -15,7 +15,7 @@ class SkTwoPointConicalGradient final : public SkGradientShaderBase {
 public:
     static sk_sp<SkShader> Create(const SkPoint& start, SkScalar startRadius,
                                   const SkPoint& end, SkScalar endRadius,
-                                  bool flippedGrad, const Descriptor&);
+                                  const Descriptor&);
 
     SkShader::GradientType asAGradient(GradientInfo* info) const  override;
 #if SK_SUPPORT_GPU
@@ -29,7 +29,6 @@ public:
     const SkPoint& getStartCenter() const { return fCenter1; }
     const SkPoint& getEndCenter() const { return fCenter2; }
     SkScalar getEndRadius() const { return fRadius2; }
-    bool isFlippedGrad() const { return fFlippedGrad; }
 
     SK_TO_STRING_OVERRIDE()
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkTwoPointConicalGradient)
@@ -51,14 +50,12 @@ private:
 
     SkTwoPointConicalGradient(const SkPoint& c0, SkScalar r0,
                               const SkPoint& c1, SkScalar r1,
-                              bool flippedGrad, const Descriptor&,
-                              Type, const SkMatrix&);
+                              const Descriptor&, Type, const SkMatrix&);
 
     SkPoint  fCenter1;
     SkPoint  fCenter2;
     SkScalar fRadius1;
     SkScalar fRadius2;
-    bool     fFlippedGrad;
     Type     fType;
 
     friend class SkGradientShader;
diff --git a/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp b/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
index 5743d24b91..a48c8d9d39 100644
--- a/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
+++ b/src/shaders/gradients/SkTwoPointConicalGradient_gpu.cpp
@@ -16,6 +16,8 @@
 #include "glsl/GrGLSLUniformHandler.h"
 #include "SkTwoPointConicalGradient_gpu.h"
 
+#include <cmath>
+
 // For brevity
 typedef GrGLSLProgramDataManager::UniformHandle UniformHandle;
 
@@ -379,9 +381,10 @@ class FocalOutside2PtConicalEffect : public GrGradientEffect {
 public:
     class GLSLFocalOutside2PtConicalProcessor;
 
-    static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args, SkScalar focalX) {
+    static std::unique_ptr<GrFragmentProcessor> Make(const CreateArgs& args, SkScalar focalX,
+                                                     bool isFlipped) {
         auto processor = std::unique_ptr<FocalOutside2PtConicalEffect>(
-                new FocalOutside2PtConicalEffect(args, focalX));
+                new FocalOutside2PtConicalEffect(args, focalX, isFlipped));
         return processor->isValid() ? std::move(processor) : nullptr;
     }
 
@@ -408,17 +411,11 @@ private:
                 this->fIsFlipped == s.fIsFlipped);
     }
 
-    static bool IsFlipped(const CreateArgs& args) {
-        // eww.
-        return static_cast<const SkTwoPointConicalGradient*>(args.fShader)->isFlippedGrad();
-    }
-
-    FocalOutside2PtConicalEffect(const CreateArgs& args, SkScalar focalX)
+    FocalOutside2PtConicalEffect(const CreateArgs& args, SkScalar focalX, bool isFlipped)
             : INHERITED(kFocalOutside2PtConicalEffect_ClassID, args,
                         false /* opaque: draws transparent black outside of the cone. */)
             , fFocalX(focalX)
-            , fIsFlipped(IsFlipped(args)) {
-    }
+            , fIsFlipped(isFlipped) {}
 
     explicit FocalOutside2PtConicalEffect(const FocalOutside2PtConicalEffect& that)
             : INHERITED(that), fFocalX(that.fFocalX), fIsFlipped(that.fIsFlipped) {
@@ -449,12 +446,11 @@ protected:
     const char* fVSVaryingName;
     const char* fFSVaryingName;
 
-    bool fIsFlipped;
-
     // @{
     /// Values last uploaded as uniforms
 
     SkScalar fCachedFocal;
+    SkScalar fCachedFlipSign;
 
     // @}
 
@@ -513,24 +509,27 @@ FocalOutside2PtConicalEffect::GLSLFocalOutside2PtConicalProcessor
                             ::GLSLFocalOutside2PtConicalProcessor(const GrProcessor& processor)
     : fVSVaryingName(nullptr)
     , fFSVaryingName(nullptr)
-    , fCachedFocal(SK_ScalarMax) {
-    const FocalOutside2PtConicalEffect& data = processor.cast<FocalOutside2PtConicalEffect>();
-    fIsFlipped = data.isFlipped();
-}
+    , fCachedFocal(SK_ScalarMax)
+    , fCachedFlipSign(SK_ScalarMax) {}
 
 void FocalOutside2PtConicalEffect::GLSLFocalOutside2PtConicalProcessor::emitCode(EmitArgs& args) {
     const FocalOutside2PtConicalEffect& ge = args.fFp.cast<FocalOutside2PtConicalEffect>();
     GrGLSLUniformHandler* uniformHandler = args.fUniformHandler;
     this->emitUniforms(uniformHandler, ge);
-    fParamUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf2_GrSLType,
+    fParamUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf3_GrSLType,
                                            "Conical2FSParams");
     SkString tName("t");
+    // TODO: get rid of these locals?
     SkString p0; // focalX
     SkString p1; // 1 - focalX * focalX
-
     p0.appendf("%s.x", uniformHandler->getUniformVariable(fParamUni).getName().c_str());
     p1.appendf("%s.y", uniformHandler->getUniformVariable(fParamUni).getName().c_str());
 
+    // params.x = focalX
+    // params.y = 1 - focalX * focalX
+    // params.z = flipSign
+    GrShaderVar params = uniformHandler->getUniformVariable(fParamUni);
+
     // if we have a float3 from being in perspective, convert it to a float2 first
     GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
     SkString coords2DString = fragBuilder->ensureCoords2D(args.fTransformedCoords[0]);
@@ -545,18 +544,11 @@ void FocalOutside2PtConicalEffect::GLSLFocalOutside2PtConicalProcessor::emitCode
     fragBuilder->codeAppendf("\thalf xs = %s.x * %s.x;\n", coords2D, coords2D);
     fragBuilder->codeAppendf("\thalf ys = %s.y * %s.y;\n", coords2D, coords2D);
     fragBuilder->codeAppendf("\thalf d = xs + %s * ys;\n", p1.c_str());
+    fragBuilder->codeAppendf("\thalf %s = %s.x * %s  + sqrt(d);\n",
+                             tName.c_str(), coords2D, p0.c_str());
 
-    // Must check to see if we flipped the circle order (to make sure start radius < end radius)
-    // If so we must also flip sign on sqrt
-    if (!fIsFlipped) {
-        fragBuilder->codeAppendf("\thalf %s = %s.x * %s  + sqrt(d);\n", tName.c_str(),
-                                 coords2D, p0.c_str());
-    } else {
-        fragBuilder->codeAppendf("\thalf %s = %s.x * %s  - sqrt(d);\n", tName.c_str(),
-                                 coords2D, p0.c_str());
-    }
-
-    fragBuilder->codeAppendf("\tif (%s >= 0.0 && d >= 0.0) {\n", tName.c_str());
+    fragBuilder->codeAppendf("\tif (%s.z * %s >= 0.0 && d >= 0.0) {\n",
+                             params.c_str(), tName.c_str());
     fragBuilder->codeAppend("\t\t");
     this->emitColor(fragBuilder,
                     uniformHandler,
@@ -573,23 +565,22 @@ void FocalOutside2PtConicalEffect::GLSLFocalOutside2PtConicalProcessor::onSetDat
         const GrGLSLProgramDataManager& pdman, const GrFragmentProcessor& processor) {
     INHERITED::onSetData(pdman, processor);
     const FocalOutside2PtConicalEffect& data = processor.cast<FocalOutside2PtConicalEffect>();
-    SkASSERT(data.isFlipped() == fIsFlipped);
     SkScalar focal = data.focal();
+    SkScalar flipSign = data.isFlipped() ? -1 : 1;
 
-    if (fCachedFocal != focal) {
+    if (fCachedFocal != focal || fCachedFlipSign != flipSign) {
         SkScalar oneMinus2F = 1.f - focal * focal;
 
-        pdman.set2f(fParamUni, SkScalarToFloat(focal), SkScalarToFloat(oneMinus2F));
+        pdman.set3f(fParamUni, focal, oneMinus2F, flipSign);
         fCachedFocal = focal;
+        fCachedFlipSign = flipSign;
     }
 }
 
 void FocalOutside2PtConicalEffect::GLSLFocalOutside2PtConicalProcessor::GenKey(
                                             const GrProcessor& processor,
                                             const GrShaderCaps&, GrProcessorKeyBuilder* b) {
-    uint32_t* key = b->add32n(2);
-    key[0] = GenBaseGradientKey(processor);
-    key[1] = processor.cast<FocalOutside2PtConicalEffect>().isFlipped();
+    b->add32(GenBaseGradientKey(processor));
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -781,6 +772,21 @@ struct CircleConicalInfo {
     SkScalar fA;
     SkScalar fB;
     SkScalar fC;
+
+    bool operator==(const CircleConicalInfo& other) const {
+        return fCenterEnd == other.fCenterEnd
+            && fA         == other.fA
+            && fB         == other.fB
+            && fC         == other.fC;
+    }
+
+    bool operator!=(const CircleConicalInfo& other) const { return !(*this == other); }
+
+    // true when endRadius < startRadius
+    bool isFlipped() const {
+        // B = (endRadius/startRadius - 1) * C
+        return std::signbit(fB) != std::signbit(fC);
+    }
 };
 
 // Returns focal distance along x-axis in transformed coords
@@ -858,6 +864,7 @@ public:
     SkScalar A() const { return fInfo.fA; }
     SkScalar B() const { return fInfo.fB; }
     SkScalar C() const { return fInfo.fC; }
+    bool isFlipped() const { return fInfo.isFlipped(); }
 
 private:
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
@@ -867,17 +874,12 @@ private:
 
     bool onIsEqual(const GrFragmentProcessor& sBase) const override {
         const CircleInside2PtConicalEffect& s = sBase.cast<CircleInside2PtConicalEffect>();
-        return (INHERITED::onIsEqual(sBase) &&
-                this->fInfo.fCenterEnd == s.fInfo.fCenterEnd &&
-                this->fInfo.fA == s.fInfo.fA &&
-                this->fInfo.fB == s.fInfo.fB &&
-                this->fInfo.fC == s.fInfo.fC);
+        return INHERITED::onIsEqual(sBase) && fInfo == s.fInfo;
     }
 
     CircleInside2PtConicalEffect(const CreateArgs& args, const CircleConicalInfo& info)
             : INHERITED(kCircleInside2PtConicalEffect_ClassID, args,
-                        args.fShader->colorsAreOpaque()), fInfo(info) {
-    }
+                        args.fShader->colorsAreOpaque()), fInfo(info) {}
 
     explicit CircleInside2PtConicalEffect(const CircleInside2PtConicalEffect& that)
             : INHERITED(that), fInfo(that.fInfo) {}
@@ -915,6 +917,7 @@ protected:
     SkScalar fCachedA;
     SkScalar fCachedB;
     SkScalar fCachedC;
+    SkScalar fCachedFlipSign;
 
     // @}
 
@@ -977,7 +980,8 @@ CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor
     , fCachedCenterY(SK_ScalarMax)
     , fCachedA(SK_ScalarMax)
     , fCachedB(SK_ScalarMax)
-    , fCachedC(SK_ScalarMax) {}
+    , fCachedC(SK_ScalarMax)
+    , fCachedFlipSign(SK_ScalarMax) {}
 
 void CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor::emitCode(EmitArgs& args) {
     const CircleInside2PtConicalEffect& ge = args.fFp.cast<CircleInside2PtConicalEffect>();
@@ -985,7 +989,7 @@ void CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor::emitCode
     this->emitUniforms(uniformHandler, ge);
     fCenterUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf2_GrSLType,
                                             "Conical2FSCenter");
-    fParamUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf3_GrSLType,
+    fParamUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf4_GrSLType,
                                            "Conical2FSParams");
     SkString tName("t");
 
@@ -993,6 +997,7 @@ void CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor::emitCode
     // params.x = A
     // params.y = B
     // params.z = C
+    // params.w = flipSign
     GrShaderVar params = uniformHandler->getUniformVariable(fParamUni);
 
     // if we have a float3 from being in perspective, convert it to a float2 first
@@ -1011,8 +1016,8 @@ void CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor::emitCode
     fragBuilder->codeAppendf("\thalf pDotp = dot(%s,  %s);\n", coords2D, coords2D);
     fragBuilder->codeAppendf("\thalf d = dot(%s,  %s) + %s.y;\n", coords2D, center.c_str(),
                              params.c_str());
-    fragBuilder->codeAppendf("\thalf %s = d + sqrt(d * d - %s.x * pDotp + %s.z);\n",
-                             tName.c_str(), params.c_str(), params.c_str());
+    fragBuilder->codeAppendf("\thalf %s = d + %s.w * sqrt(d * d - %s.x * pDotp + %s.z);\n",
+                             tName.c_str(), params.c_str(), params.c_str(), params.c_str());
 
     this->emitColor(fragBuilder,
                     uniformHandler,
@@ -1033,18 +1038,20 @@ void CircleInside2PtConicalEffect::GLSLCircleInside2PtConicalProcessor::onSetDat
     SkScalar A = data.A();
     SkScalar B = data.B();
     SkScalar C = data.C();
+    SkScalar flipSign = data.isFlipped() ? -1 : 1;
 
-    if (fCachedCenterX != centerX || fCachedCenterY != centerY ||
-        fCachedA != A || fCachedB != B || fCachedC != C) {
-
-        pdman.set2f(fCenterUni, SkScalarToFloat(centerX), SkScalarToFloat(centerY));
-        pdman.set3f(fParamUni, SkScalarToFloat(A), SkScalarToFloat(B), SkScalarToFloat(C));
-
+    if (fCachedCenterX != centerX || fCachedCenterY != centerY) {
+        pdman.set2f(fCenterUni, centerX, centerY);
         fCachedCenterX = centerX;
         fCachedCenterY = centerY;
+    }
+
+    if (fCachedA != A || fCachedB != B || fCachedC != C || fCachedFlipSign != flipSign) {
+        pdman.set4f(fParamUni, A, B, C, flipSign);
         fCachedA = A;
         fCachedB = B;
         fCachedC = C;
+        fCachedFlipSign = flipSign;
     }
 }
 
@@ -1077,7 +1084,7 @@ public:
     SkScalar B() const { return fInfo.fB; }
     SkScalar C() const { return fInfo.fC; }
     SkScalar tLimit() const { return fTLimit; }
-    bool isFlipped() const { return fIsFlipped; }
+    bool isFlipped() const { return fInfo.isFlipped(); }
 
 private:
     GrGLSLFragmentProcessor* onCreateGLSLInstance() const override;
@@ -1086,13 +1093,7 @@ private:
 
     bool onIsEqual(const GrFragmentProcessor& sBase) const override {
         const CircleOutside2PtConicalEffect& s = sBase.cast<CircleOutside2PtConicalEffect>();
-        return (INHERITED::onIsEqual(sBase) &&
-                this->fInfo.fCenterEnd == s.fInfo.fCenterEnd &&
-                this->fInfo.fA == s.fInfo.fA &&
-                this->fInfo.fB == s.fInfo.fB &&
-                this->fInfo.fC == s.fInfo.fC &&
-                this->fTLimit == s.fTLimit &&
-                this->fIsFlipped == s.fIsFlipped);
+        return INHERITED::onIsEqual(sBase) && fInfo == s.fInfo && fTLimit == s.fTLimit;
     }
 
     CircleOutside2PtConicalEffect(const CreateArgs& args, const CircleConicalInfo& info)
@@ -1106,21 +1107,17 @@ private:
         } else {
             fTLimit = SK_ScalarMin;
         }
-
-        fIsFlipped = shader.isFlippedGrad();
     }
 
     explicit CircleOutside2PtConicalEffect(const CircleOutside2PtConicalEffect& that)
             : INHERITED(that)
             , fInfo(that.fInfo)
-            , fTLimit(that.fTLimit)
-            , fIsFlipped(that.fIsFlipped) {}
+            , fTLimit(that.fTLimit) {}
 
     GR_DECLARE_FRAGMENT_PROCESSOR_TEST
 
     const CircleConicalInfo fInfo;
     SkScalar fTLimit;
-    bool fIsFlipped;
 
     typedef GrGradientEffect INHERITED;
 };
@@ -1139,12 +1136,11 @@ protected:
 
     UniformHandle fCenterUni;
     UniformHandle fParamUni;
+    UniformHandle fFlipSignUni;
 
     const char* fVSVaryingName;
     const char* fFSVaryingName;
 
-    bool fIsFlipped;
-
     // @{
     /// Values last uploaded as uniforms
 
@@ -1154,6 +1150,7 @@ protected:
     SkScalar fCachedB;
     SkScalar fCachedC;
     SkScalar fCachedTLimit;
+    SkScalar fCachedFlipSign;
 
     // @}
 
@@ -1218,10 +1215,8 @@ CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor
     , fCachedA(SK_ScalarMax)
     , fCachedB(SK_ScalarMax)
     , fCachedC(SK_ScalarMax)
-    , fCachedTLimit(SK_ScalarMax) {
-    const CircleOutside2PtConicalEffect& data = processor.cast<CircleOutside2PtConicalEffect>();
-    fIsFlipped = data.isFlipped();
-    }
+    , fCachedTLimit(SK_ScalarMax)
+    , fCachedFlipSign(SK_ScalarMax) {}
 
 void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::emitCode(EmitArgs& args) {
     const CircleOutside2PtConicalEffect& ge = args.fFp.cast<CircleOutside2PtConicalEffect>();
@@ -1231,6 +1226,8 @@ void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::emitCo
                                             "Conical2FSCenter");
     fParamUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf4_GrSLType,
                                            "Conical2FSParams");
+    fFlipSignUni = uniformHandler->addUniform(kFragment_GrShaderFlag, kHalf_GrSLType,
+                                           "Conical2FSFlipSign");
     SkString tName("t");
 
     GrShaderVar center = uniformHandler->getUniformVariable(fCenterUni);
@@ -1238,6 +1235,7 @@ void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::emitCo
     // params.y = B
     // params.z = C
     GrShaderVar params = uniformHandler->getUniformVariable(fParamUni);
+    GrShaderVar flipsign = uniformHandler->getUniformVariable(fFlipSignUni);
 
     // if we have a float3 from being in perspective, convert it to a float2 first
     GrGLSLFPFragmentBuilder* fragBuilder = args.fFragBuilder;
@@ -1262,17 +1260,11 @@ void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::emitCo
                              params.c_str());
     fragBuilder->codeAppendf("\thalf deter = d * d - %s.x * pDotp + %s.z;\n", params.c_str(),
                              params.c_str());
+    fragBuilder->codeAppendf("\thalf %s = d + sqrt(deter);\n",
+                             tName.c_str());
 
-    // Must check to see if we flipped the circle order (to make sure start radius < end radius)
-    // If so we must also flip sign on sqrt
-    if (!fIsFlipped) {
-        fragBuilder->codeAppendf("\thalf %s = d + sqrt(deter);\n", tName.c_str());
-    } else {
-        fragBuilder->codeAppendf("\thalf %s = d - sqrt(deter);\n", tName.c_str());
-    }
-
-    fragBuilder->codeAppendf("\tif (%s >= %s.w && deter >= 0.0) {\n",
-                             tName.c_str(), params.c_str());
+    fragBuilder->codeAppendf("\tif (%s * (%s - %s.w) >= 0 && deter >= 0.0) {\n",
+                             flipsign.c_str(), tName.c_str(), params.c_str());
     fragBuilder->codeAppend("\t\t");
     this->emitColor(fragBuilder,
                     uniformHandler,
@@ -1289,36 +1281,38 @@ void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::onSetD
         const GrGLSLProgramDataManager& pdman, const GrFragmentProcessor& processor) {
     INHERITED::onSetData(pdman, processor);
     const CircleOutside2PtConicalEffect& data = processor.cast<CircleOutside2PtConicalEffect>();
-    SkASSERT(data.isFlipped() == fIsFlipped);
     SkScalar centerX = data.centerX();
     SkScalar centerY = data.centerY();
     SkScalar A = data.A();
     SkScalar B = data.B();
     SkScalar C = data.C();
     SkScalar tLimit = data.tLimit();
+    SkScalar flipSign = data.isFlipped() ? -1 : 1;
 
-    if (fCachedCenterX != centerX || fCachedCenterY != centerY ||
-        fCachedA != A || fCachedB != B || fCachedC != C || fCachedTLimit != tLimit) {
-
-        pdman.set2f(fCenterUni, SkScalarToFloat(centerX), SkScalarToFloat(centerY));
-        pdman.set4f(fParamUni, SkScalarToFloat(A), SkScalarToFloat(B), SkScalarToFloat(C),
-                   SkScalarToFloat(tLimit));
-
+    if (fCachedCenterX != centerX || fCachedCenterY != centerY) {
+        pdman.set2f(fCenterUni, centerX, centerY);
         fCachedCenterX = centerX;
         fCachedCenterY = centerY;
+    }
+
+    if (fCachedA != A || fCachedB != B || fCachedC != C || fCachedTLimit != tLimit) {
+        pdman.set4f(fParamUni, A, B, C, tLimit);
         fCachedA = A;
         fCachedB = B;
         fCachedC = C;
         fCachedTLimit = tLimit;
     }
+
+    if (fCachedFlipSign != flipSign) {
+        pdman.set1f(fFlipSignUni, flipSign);
+        fCachedFlipSign = flipSign;
+    }
 }
 
 void CircleOutside2PtConicalEffect::GLSLCircleOutside2PtConicalProcessor::GenKey(
                                             const GrProcessor& processor,
                                             const GrShaderCaps&, GrProcessorKeyBuilder* b) {
-    uint32_t* key = b->add32n(2);
-    key[0] = GenBaseGradientKey(processor);
-    key[1] = processor.cast<CircleOutside2PtConicalEffect>().isFlipped();
+    b->add32(GenBaseGradientKey(processor));
 }
 
 //////////////////////////////////////////////////////////////////////////////
@@ -1352,7 +1346,8 @@ std::unique_ptr<GrFragmentProcessor> Gr2PtConicalGradientEffect::Make(
             set_matrix_edge_conical(shader, &matrix);
             return Edge2PtConicalEffect::Make(newArgs);
         } else {
-            return FocalOutside2PtConicalEffect::Make(newArgs, focalX);
+            const bool isFlipped = shader.getStartRadius() > shader.getEndRadius();
+            return FocalOutside2PtConicalEffect::Make(newArgs, focalX, isFlipped);
         }
     }