Make SkColorFilter::appendStages() not fail.

This makes SkColorFilter::appendStages() first try onAppendStages(),
and if it's unimplemented or fails, fall back to filterSpan4f().

This also makes onAppendStages() private to try to ensure that
appendStages() is now its only caller, ensuring everyone goes
through this fallback path.

The fallback uses the color filter transformed into the dst colorspace
using our new SkColorSpaceXformer... that seem ok Matt?

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

diff --git a/include/core/SkColorFilter.h b/include/core/SkColorFilter.h
index 5845573c42f494c43f852885dd951b21d18f8ab5..16375499e53a229c1673a75b4b863b143e07066d 100644 (file)
--- a/include/core/SkColorFilter.h
+++ b/include/core/SkColorFilter.h
@@ -74,8 +74,7 @@ public:
 
     virtual void filterSpan4f(const SkPM4f src[], int count, SkPM4f result[]) const = 0;
 
-    bool appendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*,
-                      bool shaderIsOpaque) const;
+    void appendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*, bool shaderIsOpaque) const;
 
     enum Flags {
         /** If set the filter methods will not change the alpha channel of the colors.
@@ -161,9 +160,6 @@ public:
 protected:
     SkColorFilter() {}
 
-    virtual bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*,
-                                bool shaderIsOpaque) const;
-
     sk_sp<SkColorFilter> makeColorSpace(SkColorSpaceXformer* xformer) const {
         return this->onMakeColorSpace(xformer);
     }
@@ -189,6 +185,10 @@ private:
         return false;
     }
 
+    virtual bool onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*,
+                                bool shaderIsOpaque) const;
+
+
     friend class SkColorSpaceXformer;
     friend class SkComposeColorFilter;
 

diff --git a/src/core/SkColorFilter.cpp b/src/core/SkColorFilter.cpp
index 6dcee068ea20e12d7f8f53729bcfb78b7d4373fd..07e97919f5ec6a7ade4c8cf40cc0f919552c08f6 100644 (file)
--- a/src/core/SkColorFilter.cpp
+++ b/src/core/SkColorFilter.cpp
@@ -10,12 +10,14 @@
 #include "SkColorSpaceXformer.h"
 #include "SkNx.h"
 #include "SkPM4f.h"
+#include "SkRasterPipeline.h"
 #include "SkReadBuffer.h"
 #include "SkRefCnt.h"
 #include "SkString.h"
 #include "SkTDArray.h"
 #include "SkUnPreMultiply.h"
 #include "SkWriteBuffer.h"
+#include "../jumper/SkJumper.h"
 
 #if SK_SUPPORT_GPU
 #include "GrFragmentProcessor.h"
@@ -39,11 +41,27 @@ sk_sp<GrFragmentProcessor> SkColorFilter::asFragmentProcessor(GrContext*, SkColo
 }
 #endif
 
-bool SkColorFilter::appendStages(SkRasterPipeline* pipeline,
-                                 SkColorSpace* dst,
-                                 SkArenaAlloc* scratch,
+void SkColorFilter::appendStages(SkRasterPipeline* p,
+                                 SkColorSpace* dstCS,
+                                 SkArenaAlloc* alloc,
                                  bool shaderIsOpaque) const {
-    return this->onAppendStages(pipeline, dst, scratch, shaderIsOpaque);
+    SkRasterPipeline subclass;
+    if (this->onAppendStages(&subclass, dstCS, alloc, shaderIsOpaque)) {
+        p->extend(subclass);
+        return;
+    }
+
+    struct Ctx : SkJumper_CallbackCtx {
+        sk_sp<SkColorFilter> cf;
+    };
+    auto ctx = alloc->make<Ctx>();
+    ctx->cf = SkColorSpaceXformer::Make(sk_ref_sp(dstCS))->apply(this);
+    ctx->fn = [](SkJumper_CallbackCtx* arg, int active_pixels) {
+        auto ctx = (Ctx*)arg;
+        auto buf = (SkPM4f*)ctx->rgba;
+        ctx->cf->filterSpan4f(buf, active_pixels, buf);
+    };
+    p->append(SkRasterPipeline::callback, ctx);
 }
 
 bool SkColorFilter::onAppendStages(SkRasterPipeline*, SkColorSpace*, SkArenaAlloc*, bool) const {
@@ -108,8 +126,9 @@ public:
         if (!(fInner->getFlags() & kAlphaUnchanged_Flag)) {
             innerIsOpaque = false;
         }
-        return fInner->appendStages(p, dst, scratch, shaderIsOpaque) &&
-               fOuter->appendStages(p, dst, scratch, innerIsOpaque);
+        fInner->appendStages(p, dst, scratch, shaderIsOpaque);
+        fOuter->appendStages(p, dst, scratch, innerIsOpaque);
+        return true;
     }
 
 #if SK_SUPPORT_GPU

diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp
index 9b882c2ca19b46ca6ebab082a205a86e2591a120..badd83d73cf7612b48e2112885eb7954ec7c09b7 100644 (file)
--- a/src/core/SkRasterPipelineBlitter.cpp
+++ b/src/core/SkRasterPipelineBlitter.cpp
@@ -125,9 +125,7 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
     }
 
     if (colorFilter) {
-        if (!colorFilter->appendStages(pipeline, dst.colorSpace(), alloc, is_opaque)) {
-            return nullptr;
-        }
+        colorFilter->appendStages(pipeline, dst.colorSpace(), alloc, is_opaque);
         is_opaque = is_opaque && (colorFilter->getFlags() & SkColorFilter::kAlphaUnchanged_Flag);
     }
 

diff --git a/src/effects/SkColorMatrixFilter.cpp b/src/effects/SkColorMatrixFilter.cpp
index 52bf71eb70a95f0ce4ae50628447dc746e645bd0..ebd806e3c002749133758acd36128dbcfa70d281 100644 (file)
--- a/src/effects/SkColorMatrixFilter.cpp
+++ b/src/effects/SkColorMatrixFilter.cpp
@@ -58,7 +58,8 @@ public:
     }
     bool onAppendStages(SkRasterPipeline* p, SkColorSpace* cs, SkArenaAlloc* alloc,
                         bool shaderIsOpaque) const override {
-        return fMatrixFilter->appendStages(p, cs, alloc, shaderIsOpaque);
+        fMatrixFilter->appendStages(p, cs, alloc, shaderIsOpaque);
+        return true;
     }
 
     // TODO: might want to remember we're a lighting color filter through serialization?