Make SkCreateRasterPipelineBlitter() not fail.

Mostly this is about extending the fallback in SkShader::appendStages()
to cover more cases, and making sure subclasses call appendStages() so
they can get the fallback, not onAppendStages() directly.

We still need to watch for SkShader::makeContext() failing in the
fallback itself, so sadly SkShader::appendStages() may still fail.

Change-Id: I2314b234a24bdcecac401a385ce050d7fdf0a83e
Reviewed-on: https://skia-review.googlesource.com/16369
Reviewed-by: Matt Sarett <msarett@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

6 files changed, 29 insertions, 33 deletions

diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 2a34ffcbf5..c2a3a89ca2 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -849,9 +849,9 @@ SkBlitter* SkBlitter::Choose(const SkPixmap& device,
 
     // By policy we choose not to handle legacy 8888 with SkRasterPipelineBlitter.
     if (device.colorSpace() || device.colorType() != kN32_SkColorType) {
-        if (SkBlitter* blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc)) {
-            return blitter;
-        }
+        auto blitter = SkCreateRasterPipelineBlitter(device, *paint, matrix, alloc);
+        SkASSERT(blitter);
+        return blitter;
     }
 
     if (nullptr == shader) {
diff --git a/src/core/SkLocalMatrixShader.cpp b/src/core/SkLocalMatrixShader.cpp
index 33472f1f59..8a9a1656d9 100644
--- a/src/core/SkLocalMatrixShader.cpp
+++ b/src/core/SkLocalMatrixShader.cpp
@@ -72,9 +72,8 @@ bool SkLocalMatrixShader::onAppendStages(SkRasterPipeline* p,
     if (localM) {
         tmp.setConcat(*localM, this->getLocalMatrix());
     }
-
-    return fProxyShader->onAppendStages(p, dst, scratch, ctm, paint,
-                                        localM ? &tmp : &this->getLocalMatrix());
+    return fProxyShader->appendStages(p, dst, scratch, ctm, paint,
+                                      localM ? &tmp : &this->getLocalMatrix());
 }
 
 #ifndef SK_IGNORE_TO_STRING
diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp
index badd83d73c..363324ba4f 100644
--- a/src/core/SkRasterPipelineBlitter.cpp
+++ b/src/core/SkRasterPipelineBlitter.cpp
@@ -111,7 +111,8 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
     if (shader) {
         pipeline->append(SkRasterPipeline::seed_shader, &blitter->fCurrentY);
         if (!shader->appendStages(pipeline, dst.colorSpace(), alloc, ctm, paint)) {
-            return nullptr;
+            // When a shader fails to append stages, it means it has vetoed drawing entirely.
+            return alloc->make<SkNullBlitter>();
         }
         if (!is_opaque) {
             pipeline->append(SkRasterPipeline::scale_1_float,
diff --git a/src/core/SkShader.cpp b/src/core/SkShader.cpp
index 5f62947008..6f03bd5984 100644
--- a/src/core/SkShader.cpp
+++ b/src/core/SkShader.cpp
@@ -255,22 +255,21 @@ void SkShader::toString(SkString* str) const {
 }
 #endif
 
-bool SkShader::appendStages(SkRasterPipeline* pipeline,
-                            SkColorSpace* dst,
-                            SkArenaAlloc* scratch,
+bool SkShader::appendStages(SkRasterPipeline* p,
+                            SkColorSpace* dstCS,
+                            SkArenaAlloc* alloc,
                             const SkMatrix& ctm,
-                            const SkPaint& paint) const {
-    return this->onAppendStages(pipeline, dst, scratch, ctm, paint, nullptr);
-}
+                            const SkPaint& paint,
+                            const SkMatrix* localM) const {
+    SkRasterPipeline subclass;
+    if (this->onAppendStages(&subclass, dstCS, alloc, ctm, paint, localM)) {
+        p->extend(subclass);
+        return true;
+    }
 
-bool SkShader::onAppendStages(SkRasterPipeline* p,
-                              SkColorSpace* dstCS,
-                              SkArenaAlloc* alloc,
-                              const SkMatrix& ctm,
-                              const SkPaint& paint,
-                              const SkMatrix* localM) const {
-    // Legacy shaders handle the paint opacity internally,
-    // but RP applies it as a separate stage.
+    // SkShader::Context::shadeSpan4f() handles the paint opacity internally,
+    // but SkRasterPipelineBlitter applies it as a separate stage.
+    // We skip the internal shadeSpan4f() step by forcing the paint opaque.
     SkTCopyOnFirstWrite<SkPaint> opaquePaint(paint);
     if (paint.getAlpha() != SK_AlphaOPAQUE) {
         opaquePaint.writable()->setAlpha(SK_AlphaOPAQUE);
@@ -300,6 +299,15 @@ bool SkShader::onAppendStages(SkRasterPipeline* p,
     return false;
 }
 
+bool SkShader::onAppendStages(SkRasterPipeline* p,
+                              SkColorSpace* dstCS,
+                              SkArenaAlloc* alloc,
+                              const SkMatrix& ctm,
+                              const SkPaint& paint,
+                              const SkMatrix* localM) const {
+    return false;
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 sk_sp<SkFlattenable> SkEmptyShader::CreateProc(SkReadBuffer&) {
diff --git a/src/effects/gradients/SkTwoPointConicalGradient.cpp b/src/effects/gradients/SkTwoPointConicalGradient.cpp
index bc97700827..ced299ee23 100644
--- a/src/effects/gradients/SkTwoPointConicalGradient.cpp
+++ b/src/effects/gradients/SkTwoPointConicalGradient.cpp
@@ -419,12 +419,3 @@ void SkTwoPointConicalGradient::toString(SkString* str) const {
     str->append(")");
 }
 #endif
-
-bool SkTwoPointConicalGradient::onAppendStages(SkRasterPipeline* p,
-                                               SkColorSpace* dstCS,
-                                               SkArenaAlloc* alloc,
-                                               const SkMatrix& ctm,
-                                               const SkPaint& paint,
-                                               const SkMatrix* localM) const {
-    return SkShader::onAppendStages(p, dstCS, alloc, ctm, paint, localM);
-}
diff --git a/src/effects/gradients/SkTwoPointConicalGradient.h b/src/effects/gradients/SkTwoPointConicalGradient.h
index d5de47eb9a..b32f52c1e0 100644
--- a/src/effects/gradients/SkTwoPointConicalGradient.h
+++ b/src/effects/gradients/SkTwoPointConicalGradient.h
@@ -79,9 +79,6 @@ protected:
     Context* onMakeContext(const ContextRec&, SkArenaAlloc*) const override;
     sk_sp<SkShader> onMakeColorSpace(SkColorSpaceXformer* xformer) const override;
 
-    bool onAppendStages(SkRasterPipeline*, SkColorSpace* dstCS, SkArenaAlloc*,
-                        const SkMatrix& ctm, const SkPaint&, const SkMatrix* localM) const override;
-
 private:
     SkPoint fCenter1;
     SkPoint fCenter2;