spin off: always clamp linear gradients

While we're refactoring how gradients work it's going to be easier
to centralized how and when we tile.

  - PS2 changed linear and radial in place to alwys clamp.
  - PS3 moved tiling to the base class, where it's even harder to
    screw up.  Sweeps don't need but don't mind tiling.
  - PS4 clamps when iff evenly spaced

PS4 has image diffs for only a few GMs that I'm not familiar with.
If its logic reads as correct to you, they may be bug fixes?

Change-Id: I5e37d6e88aaea898356d4c57db0cd5bf414c0295
Reviewed-on: https://skia-review.googlesource.com/16501
Commit-Queue: Mike Klein <mtklein@chromium.org>
Reviewed-by: Florin Malita <fmalita@chromium.org>

3 files changed, 29 insertions, 36 deletions

diff --git a/src/effects/gradients/SkGradientShader.cpp b/src/effects/gradients/SkGradientShader.cpp
index e5cf0715bf..6f9b404015 100644
--- a/src/effects/gradients/SkGradientShader.cpp
+++ b/src/effects/gradients/SkGradientShader.cpp
@@ -347,30 +347,44 @@ void SkGradientShaderBase::FlipGradientColors(SkColor* colorDst, Rec* recDst,
     memcpy(colorDst, colorsTemp.get(), count * sizeof(SkColor));
 }
 
-bool SkGradientShaderBase::onAppendStages(
-    SkRasterPipeline* pipeline, SkColorSpace* dstCS, SkArenaAlloc* alloc,
-    const SkMatrix& ctm, const SkPaint& paint,
-    const SkMatrix* localM) const
-{
+bool SkGradientShaderBase::onAppendStages(SkRasterPipeline* p,
+                                          SkColorSpace* dstCS,
+                                          SkArenaAlloc* alloc,
+                                          const SkMatrix& ctm,
+                                          const SkPaint& paint,
+                                          const SkMatrix* localM) const {
     SkMatrix matrix;
     if (!this->computeTotalInverse(ctm, localM, &matrix)) {
         return false;
     }
 
-    SkRasterPipeline p;
-    if (!this->adjustMatrixAndAppendStages(alloc, &matrix, &p)) {
+    SkRasterPipeline subclass;
+    if (!this->adjustMatrixAndAppendStages(alloc, &matrix, &subclass)) {
         return false;
     }
 
     auto* m = alloc->makeArrayDefault<float>(9);
     if (matrix.asAffine(m)) {
-        pipeline->append(SkRasterPipeline::matrix_2x3, m);
+        p->append(SkRasterPipeline::matrix_2x3, m);
     } else {
         matrix.get9(m);
-        pipeline->append(SkRasterPipeline::matrix_perspective, m);
+        p->append(SkRasterPipeline::matrix_perspective, m);
     }
 
-    pipeline->extend(p);
+    p->extend(subclass);
+
+    switch(fTileMode) {
+        case kMirror_TileMode: p->append(SkRasterPipeline::mirror_x, alloc->make<float>(1)); break;
+        case kRepeat_TileMode: p->append(SkRasterPipeline::repeat_x, alloc->make<float>(1)); break;
+        case kClamp_TileMode:
+            if (!fOrigPos) {
+                // We clamp only when the stops are evenly spaced.
+                // If not, there may be hard stops, and clamping ruins hard stops at 0 and/or 1.
+                // In that case, we must make sure we're using the general linear_gradient stage,
+                // which is the only stage that will correctly handle unclamped t.
+                p->append(SkRasterPipeline::clamp_x,  alloc->make<float>(1));
+            }
+    }
 
     const bool premulGrad = fGradFlags & SkGradientShader::kInterpolateColorsInPremul_Flag;
     auto prepareColor = [premulGrad, dstCS, this](int i) {
@@ -390,7 +404,7 @@ bool SkGradientShaderBase::onAppendStages(
         f_and_b[0] = SkPM4f::From4f(c_r.to4f() - c_l.to4f());
         f_and_b[1] = c_l;
 
-        pipeline->append(SkRasterPipeline::linear_gradient_2stops, f_and_b);
+        p->append(SkRasterPipeline::linear_gradient_2stops, f_and_b);
     } else {
 
         struct Stop { float t; SkPM4f f, b; };
@@ -479,11 +493,11 @@ bool SkGradientShaderBase::onAppendStages(
             ctx->stops = stopsArray;
         }
 
-        pipeline->append(SkRasterPipeline::linear_gradient, ctx);
+        p->append(SkRasterPipeline::linear_gradient, ctx);
     }
 
     if (!premulGrad && !this->colorsAreOpaque()) {
-        pipeline->append(SkRasterPipeline::premul);
+        p->append(SkRasterPipeline::premul);
     }
 
     return true;
diff --git a/src/effects/gradients/SkLinearGradient.cpp b/src/effects/gradients/SkLinearGradient.cpp
index 1c3dbffa99..bd46452fb2 100644
--- a/src/effects/gradients/SkLinearGradient.cpp
+++ b/src/effects/gradients/SkLinearGradient.cpp
@@ -90,17 +90,8 @@ bool SkLinearGradient::adjustMatrixAndAppendStages(SkArenaAlloc* alloc,
     // If the gradient is less than a quarter of a pixel, this falls into the
     // subpixel gradient code handled on a different path.
     SkVector dx = matrix->mapVector(1, 0);
-    if (dx.fX >= 4) { return false; }
-
-    switch(fTileMode) {
-        case kMirror_TileMode: p->append(SkRasterPipeline::mirror_x, alloc->make<float>(1)); break;
-        case kRepeat_TileMode: p->append(SkRasterPipeline::repeat_x, alloc->make<float>(1)); break;
-        case kClamp_TileMode:
-           if (fColorCount == 2 && fOrigPos == nullptr) {
-               // The general strategy does not need clamping due to implicit hard stops at 0 and 1,
-               // but the 2-point specialization must be clamped.
-               p->append(SkRasterPipeline::clamp_x, alloc->make<float>(1));
-           }
+    if (dx.fX >= 4) {
+        return false;
     }
     return true;
 }
diff --git a/src/effects/gradients/SkRadialGradient.cpp b/src/effects/gradients/SkRadialGradient.cpp
index 2133227403..339c5ff473 100644
--- a/src/effects/gradients/SkRadialGradient.cpp
+++ b/src/effects/gradients/SkRadialGradient.cpp
@@ -383,18 +383,6 @@ bool SkRadialGradient::adjustMatrixAndAppendStages(SkArenaAlloc* alloc,
     matrix->postScale(1/fRadius, 1/fRadius);
 
     p->append(SkRasterPipeline::xy_to_radius);
-
-    switch(fTileMode) {
-        case kMirror_TileMode: p->append(SkRasterPipeline::mirror_x, alloc->make<float>(1)); break;
-        case kRepeat_TileMode: p->append(SkRasterPipeline::repeat_x, alloc->make<float>(1)); break;
-        case kClamp_TileMode:
-            if (fColorCount == 2 && fOrigPos == nullptr) {
-                // The general strategy does not need clamping due to implicit hard stops at 0 and 1,
-                // but the 2-point specialization must be clamped.
-                p->append(SkRasterPipeline::clamp_x, alloc->make<float>(1));
-            }
-    }
-
     return true;
 }