Clarify adjustMatrixAndAppendStages semantics

A false return from adjustMatrixAndAppendStages() currently means we'll
attempt to create a legacy context wrapper stage.

Only two gradient impls can return false, and they don't really want the
same thing:

1) linear returns false when the stops are compressed, to get the "nice"
   legacy blend color behavior

2) two-point radial returns false when it cannot map the center points
   (degenerate matrix); in this case we prolly don't want to draw at all

This setup made sense while we were retrofitting gradients with stages,
but now that all of them support RP, it is confusing to sometimes get
a legacy context wrapper even when we are specifically asked for stages.

I propose we align the semantics with SkShader::onAppendStages(), such
that a false return means "don't draw" rather than "try to draw with a
legacy context".

This means we're giving up the compressed stop behavior for linear, but
that's such an arbitrary corner case that I don't think we care at all.

Change-Id: I01256c4acb81b16fb68e6c74cf8d91ea77b95a3b
Reviewed-on: https://skia-review.googlesource.com/22541
Commit-Queue: Herb Derby <herb@google.com>
Reviewed-by: Herb Derby <herb@google.com>

3 files changed, 4 insertions, 12 deletions

diff --git a/src/shaders/gradients/SkGradientShader.cpp b/src/shaders/gradients/SkGradientShader.cpp
index 213db4e4ff..27b986ca50 100644
--- a/src/shaders/gradients/SkGradientShader.cpp
+++ b/src/shaders/gradients/SkGradientShader.cpp
@@ -378,7 +378,7 @@ bool SkGradientShaderBase::onAppendStages(SkRasterPipeline* p,
     SkRasterPipeline_<256> tPipeline;
     SkRasterPipeline_<256> postPipeline;
     if (!this->adjustMatrixAndAppendStages(alloc, &matrix, &tPipeline, &postPipeline)) {
-        return this->INHERITED::onAppendStages(p, dstCS, alloc, ctm, paint, localM);
+        return false;
     }
 
     p->append(SkRasterPipeline::seed_shader);
diff --git a/src/shaders/gradients/SkGradientShaderPriv.h b/src/shaders/gradients/SkGradientShaderPriv.h
index 6e6e7df074..47ef70c5b0 100644
--- a/src/shaders/gradients/SkGradientShaderPriv.h
+++ b/src/shaders/gradients/SkGradientShaderPriv.h
@@ -240,9 +240,7 @@ protected:
     virtual bool adjustMatrixAndAppendStages(SkArenaAlloc* alloc,
                                              SkMatrix* matrix,
                                              SkRasterPipeline* tPipeline,
-                                             SkRasterPipeline* postPipeline) const {
-        return false;
-    }
+                                             SkRasterPipeline* postPipeline) const = 0;
 
     template <typename T, typename... Args>
     static Context* CheckedMakeContext(SkArenaAlloc* alloc, Args&&... args) {
diff --git a/src/shaders/gradients/SkLinearGradient.cpp b/src/shaders/gradients/SkLinearGradient.cpp
index 0496aea33d..08ce2db813 100644
--- a/src/shaders/gradients/SkLinearGradient.cpp
+++ b/src/shaders/gradients/SkLinearGradient.cpp
@@ -79,17 +79,11 @@ SkShaderBase::Context* SkLinearGradient::onMakeBurstPipelineContext(
                            : nullptr;
 }
 
-bool SkLinearGradient::adjustMatrixAndAppendStages(SkArenaAlloc* alloc,
+bool SkLinearGradient::adjustMatrixAndAppendStages(SkArenaAlloc*,
                                                    SkMatrix* matrix,
-                                                   SkRasterPipeline* p,
+                                                   SkRasterPipeline*,
                                                    SkRasterPipeline*) const {
     *matrix = SkMatrix::Concat(fPtsToUnit, *matrix);
-    // 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;
-    }
     return true;
 }