SkRasterPipelineBlitter simplifications

The main idea here is to simplify, mainly to reduce the number of SkRasterPipeline objects the blitter holds.

  - Use SkBlendMode instead of SkXfermode, just store SkBlendMode.
  - Fuse the shader and color filter together into one SkRasterPipeline
    during blitter creation.
  - I noticed all calls to append_load_d() and append_store() now have the same ctx argument, so I folded that through.

I'll be following up with more of this sort of refactoring... I think I can fold everything into a single SkRasterPipeline during blitter creation now, but I'll want to make sure I've got my ducks in a row for how that works with pipeline strength reduction (like skipping dst loads when drawing in Src mode).

BUG=skia:

GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4021

Change-Id: I88252ab8ad16f102c71bad871f8b6aec2fc1f226
Reviewed-on: https://skia-review.googlesource.com/4021
Reviewed-by: Herb Derby <herb@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

6 files changed, 86 insertions, 102 deletions

diff --git a/include/core/SkXfermode.h b/include/core/SkXfermode.h
index 82036dfc39..aa243664ef 100644
--- a/include/core/SkXfermode.h
+++ b/include/core/SkXfermode.h
@@ -187,8 +187,6 @@ public:
 
     virtual SkXfermodeProc4f getProc4f() const;
 
-    bool appendStages(SkRasterPipeline*) const;
-
     /**
      *  If the specified mode can be represented by a pair of Coeff, then return
      *  true and set (if not NULL) the corresponding coeffs. If the mode is
@@ -309,7 +307,6 @@ protected:
 
     virtual D32Proc onGetD32Proc(uint32_t flags) const;
     virtual F16Proc onGetF16Proc(uint32_t flags) const;
-    virtual bool onAppendStages(SkRasterPipeline*) const;
 
 private:
     enum {
diff --git a/src/core/SkBlendModePriv.h b/src/core/SkBlendModePriv.h
index b5d9e751e6..fc2d6b34df 100644
--- a/src/core/SkBlendModePriv.h
+++ b/src/core/SkBlendModePriv.h
@@ -9,11 +9,15 @@
 #define SkBlendModePriv_DEFINED
 
 #include "SkBlendMode.h"
+#include "SkRasterPipeline.h"
 
 bool SkBlendMode_SupportsCoverageAsAlpha(SkBlendMode);
 
 #if SK_SUPPORT_GPU
+#include "GrXferProcessor.h"
 sk_sp<GrXPFactory> SkBlendMode_AsXPFactory(SkBlendMode);
 #endif
 
+bool SkBlendMode_AppendStages(SkBlendMode, SkRasterPipeline* = nullptr);
+
 #endif
diff --git a/src/core/SkModeColorFilter.cpp b/src/core/SkModeColorFilter.cpp
index 977e87f7e5..98cb3b2ae6 100644
--- a/src/core/SkModeColorFilter.cpp
+++ b/src/core/SkModeColorFilter.cpp
@@ -6,6 +6,7 @@
  */
 
 #include "SkBlitRow.h"
+#include "SkBlendModePriv.h"
 #include "SkColorFilter.h"
 #include "SkColorPriv.h"
 #include "SkModeColorFilter.h"
@@ -89,13 +90,7 @@ bool SkModeColorFilter::onAppendStages(SkRasterPipeline* p) const {
     // and applying the opposite xfermode, e.g. dst-in instead of src-in.
     p->append(SkRasterPipeline::swap_src_dst);
     p->append(SkRasterPipeline::constant_color, &fPM4f);
-
-    // TODO: This is ugly.  I think we want static SkXfermode::AppendStages(Mode).
-    if (auto xfermode = SkXfermode::Make(fMode)) {
-        return xfermode->appendStages(p);
-    }
-    p->append(SkRasterPipeline::srcover);
-    return true;
+    return SkBlendMode_AppendStages((SkBlendMode)fMode, p);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
diff --git a/src/core/SkRasterPipelineBlitter.cpp b/src/core/SkRasterPipelineBlitter.cpp
index 51c2fc1805..daf34a2a6d 100644
--- a/src/core/SkRasterPipelineBlitter.cpp
+++ b/src/core/SkRasterPipelineBlitter.cpp
@@ -6,6 +6,7 @@
  */
 
 #include "SkBlitter.h"
+#include "SkBlendModePriv.h"
 #include "SkColor.h"
 #include "SkColorFilter.h"
 #include "SkOpts.h"
@@ -21,13 +22,11 @@ public:
 
     SkRasterPipelineBlitter(SkPixmap dst,
                             SkRasterPipeline shader,
-                            SkRasterPipeline colorFilter,
-                            SkRasterPipeline xfermode,
+                            SkBlendMode blend,
                             SkPM4f paintColor)
         : fDst(dst)
         , fShader(shader)
-        , fColorFilter(colorFilter)
-        , fXfermode(xfermode)
+        , fBlend(blend)
         , fPaintColor(paintColor)
     {}
 
@@ -40,11 +39,13 @@ public:
     // blits using something like a SkRasterPipeline::runFew() method.
 
 private:
-    void append_load_d(SkRasterPipeline*, const void*) const;
-    void append_store (SkRasterPipeline*,       void*) const;
+    void append_load_d(SkRasterPipeline*) const;
+    void append_store (SkRasterPipeline*) const;
+    void append_blend (SkRasterPipeline*) const;
 
     SkPixmap         fDst;
-    SkRasterPipeline fShader, fColorFilter, fXfermode;
+    SkRasterPipeline fShader;
+    SkBlendMode      fBlend;
     SkPM4f           fPaintColor;
 
     // These functions are compiled lazily when first used.
@@ -92,10 +93,13 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
     if (paint.getShader()) {
         return nullptr;  // TODO: need to work out how shaders and their contexts work
     }
+    SkBlendMode blend = paint.getBlendMode();
+    if (!SkBlendMode_AppendStages(blend)) {
+        return nullptr;  // TODO
+    }
 
-    SkRasterPipeline shader, colorFilter, xfermode;
-    if (!append_effect_stages(paint.getColorFilter(),                 &colorFilter) ||
-        !append_effect_stages(SkXfermode::Peek(paint.getBlendMode()), &xfermode   )) {
+    SkRasterPipeline shader, colorFilter;
+    if (paint.getColorFilter() && !paint.getColorFilter()->appendStages(&colorFilter)) {
         return nullptr;
     }
 
@@ -108,69 +112,65 @@ SkBlitter* SkRasterPipelineBlitter::Create(const SkPixmap& dst,
         swizzle_rb(SkNx_cast<float>(Sk4b::Load(&paintColor)) * (1/255.0f)).store(&color);
     }
 
-    auto blitter = alloc->createT<SkRasterPipelineBlitter>(
-            dst,
-            shader, colorFilter, xfermode,
-            color.premul());
+    auto blitter = alloc->createT<SkRasterPipelineBlitter>(dst, shader, blend, color.premul());
 
     if (!paint.getShader()) {
         blitter->fShader.append(SkRasterPipeline::constant_color, &blitter->fPaintColor);
     }
-    if (paint.isSrcOver()) {
-        blitter->fXfermode.append(SkRasterPipeline::srcover);
-    }
+    blitter->fShader.extend(colorFilter);
 
     return blitter;
 }
 
-void SkRasterPipelineBlitter::append_load_d(SkRasterPipeline* p, const void* dst) const {
+void SkRasterPipelineBlitter::append_load_d(SkRasterPipeline* p) const {
     SkASSERT(supported(fDst.info()));
 
     switch (fDst.info().colorType()) {
         case kN32_SkColorType:
             if (fDst.info().gammaCloseToSRGB()) {
-                p->append(SkRasterPipeline::load_d_srgb, dst);
+                p->append(SkRasterPipeline::load_d_srgb, &fDstPtr);
             }
             break;
         case kRGBA_F16_SkColorType:
-            p->append(SkRasterPipeline::load_d_f16, dst);
+            p->append(SkRasterPipeline::load_d_f16, &fDstPtr);
             break;
         case kRGB_565_SkColorType:
-            p->append(SkRasterPipeline::load_d_565, dst);
+            p->append(SkRasterPipeline::load_d_565, &fDstPtr);
             break;
         default: break;
     }
 }
 
-void SkRasterPipelineBlitter::append_store(SkRasterPipeline* p, void* dst) const {
+void SkRasterPipelineBlitter::append_store(SkRasterPipeline* p) const {
     SkASSERT(supported(fDst.info()));
 
     switch (fDst.info().colorType()) {
         case kN32_SkColorType:
             if (fDst.info().gammaCloseToSRGB()) {
-                p->append(SkRasterPipeline::store_srgb, dst);
+                p->append(SkRasterPipeline::store_srgb, &fDstPtr);
             }
             break;
         case kRGBA_F16_SkColorType:
-            p->append(SkRasterPipeline::store_f16, dst);
+            p->append(SkRasterPipeline::store_f16, &fDstPtr);
             break;
         case kRGB_565_SkColorType:
-            p->append(SkRasterPipeline::store_565, dst);
+            p->append(SkRasterPipeline::store_565, &fDstPtr);
             break;
         default: break;
     }
 }
 
-// TODO: Figure out how to cache some of the compiled pipelines.
+void SkRasterPipelineBlitter::append_blend(SkRasterPipeline* p) const {
+    SkAssertResult(SkBlendMode_AppendStages(fBlend, p));
+}
 
 void SkRasterPipelineBlitter::blitH(int x, int y, int w) {
     if (!fBlitH) {
         SkRasterPipeline p;
         p.extend(fShader);
-        p.extend(fColorFilter);
-        this->append_load_d(&p, &fDstPtr);
-        p.extend(fXfermode);
-        this->append_store(&p, &fDstPtr);
+        this->append_load_d(&p);
+        this->append_blend(&p);
+        this->append_store(&p);
         fBlitH = p.compile();
     }
 
@@ -182,11 +182,10 @@ void SkRasterPipelineBlitter::blitAntiH(int x, int y, const SkAlpha aa[], const
     if (!fBlitAntiH) {
         SkRasterPipeline p;
         p.extend(fShader);
-        p.extend(fColorFilter);
-        this->append_load_d(&p, &fDstPtr);
-        p.extend(fXfermode);
+        this->append_load_d(&p);
+        this->append_blend(&p);
         p.append(SkRasterPipeline::lerp_constant_float, &fConstantCoverage);
-        this->append_store(&p, &fDstPtr);
+        this->append_store(&p);
         fBlitAntiH = p.compile();
     }
 
@@ -210,22 +209,20 @@ void SkRasterPipelineBlitter::blitMask(const SkMask& mask, const SkIRect& clip)
     if (mask.fFormat == SkMask::kA8_Format && !fBlitMaskA8) {
         SkRasterPipeline p;
         p.extend(fShader);
-        p.extend(fColorFilter);
-        this->append_load_d(&p, &fDstPtr);
-        p.extend(fXfermode);
+        this->append_load_d(&p);
+        this->append_blend(&p);
         p.append(SkRasterPipeline::lerp_u8, &fMaskPtr);
-        this->append_store(&p, &fDstPtr);
+        this->append_store(&p);
         fBlitMaskA8 = p.compile();
     }
 
     if (mask.fFormat == SkMask::kLCD16_Format && !fBlitMaskLCD16) {
         SkRasterPipeline p;
         p.extend(fShader);
-        p.extend(fColorFilter);
-        this->append_load_d(&p, &fDstPtr);
-        p.extend(fXfermode);
+        this->append_load_d(&p);
+        this->append_blend(&p);
         p.append(SkRasterPipeline::lerp_565, &fMaskPtr);
-        this->append_store(&p, &fDstPtr);
+        this->append_store(&p);
         fBlitMaskLCD16 = p.compile();
     }
 
diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index 38a160870e..9b6172671d 100644
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -1421,58 +1421,10 @@ bool SkXfermode::IsOpaque(const SkXfermode* xfer, SrcColorOpacity opacityType) {
     return xfer->isOpaque(opacityType);
 }
 
-bool SkXfermode::appendStages(SkRasterPipeline* pipeline) const {
-    return this->onAppendStages(pipeline);
-}
-
-bool SkXfermode::onAppendStages(SkRasterPipeline*) const {
-    return false;
-}
-
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_START(SkXfermode)
     SK_DEFINE_FLATTENABLE_REGISTRAR_ENTRY(SkProcCoeffXfermode)
 SK_DEFINE_FLATTENABLE_REGISTRAR_GROUP_END
 
-
-bool SkProcCoeffXfermode::onAppendStages(SkRasterPipeline* p) const {
-    switch (fMode) {
-        case kSrc_Mode:    /*This stage is a no-op.*/             return true;
-        case kDst_Mode:     p->append(SkRasterPipeline::dst);     return true;
-        case kSrcATop_Mode: p->append(SkRasterPipeline::srcatop); return true;
-        case kDstATop_Mode: p->append(SkRasterPipeline::dstatop); return true;
-        case kSrcIn_Mode:   p->append(SkRasterPipeline::srcin);   return true;
-        case kDstIn_Mode:   p->append(SkRasterPipeline::dstin);   return true;
-        case kSrcOut_Mode:  p->append(SkRasterPipeline::srcout);  return true;
-        case kDstOut_Mode:  p->append(SkRasterPipeline::dstout);  return true;
-        case kSrcOver_Mode: p->append(SkRasterPipeline::srcover); return true;
-        case kDstOver_Mode: p->append(SkRasterPipeline::dstover); return true;
-
-        case kClear_Mode:    p->append(SkRasterPipeline::clear);    return true;
-        case kModulate_Mode: p->append(SkRasterPipeline::modulate); return true;
-        case kMultiply_Mode: p->append(SkRasterPipeline::multiply); return true;
-        case kPlus_Mode:     p->append(SkRasterPipeline::plus_);    return true;
-        case kScreen_Mode:   p->append(SkRasterPipeline::screen);   return true;
-        case kXor_Mode:      p->append(SkRasterPipeline::xor_);     return true;
-
-        case kColorBurn_Mode:  p->append(SkRasterPipeline::colorburn);  return true;
-        case kColorDodge_Mode: p->append(SkRasterPipeline::colordodge); return true;
-        case kDarken_Mode:     p->append(SkRasterPipeline::darken);     return true;
-        case kDifference_Mode: p->append(SkRasterPipeline::difference); return true;
-        case kExclusion_Mode:  p->append(SkRasterPipeline::exclusion);  return true;
-        case kHardLight_Mode:  p->append(SkRasterPipeline::hardlight);  return true;
-        case kLighten_Mode:    p->append(SkRasterPipeline::lighten);    return true;
-        case kOverlay_Mode:    p->append(SkRasterPipeline::overlay);    return true;
-        case kSoftLight_Mode:  p->append(SkRasterPipeline::softlight);  return true;
-
-        // TODO
-        case kColor_Mode:       return false;
-        case kHue_Mode:         return false;
-        case kLuminosity_Mode:  return false;
-        case kSaturation_Mode:  return false;
-    }
-    return false;
-}
-
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool SkBlendMode_SupportsCoverageAsAlpha(SkBlendMode mode) {
@@ -1533,3 +1485,44 @@ sk_sp<GrXPFactory> SkBlendMode_AsXPFactory(SkBlendMode mode) {
     return GrCustomXfermode::MakeXPFactory((SkXfermode::Mode)mode);
 }
 #endif
+
+bool SkBlendMode_AppendStages(SkBlendMode mode, SkRasterPipeline* p) {
+    auto stage = SkRasterPipeline::srcover;
+    switch (mode) {
+        case SkBlendMode::kClear:    stage = SkRasterPipeline::clear; break;
+        case SkBlendMode::kSrc:      return true;  // This stage is a no-op.
+        case SkBlendMode::kDst:      stage = SkRasterPipeline::dst; break;
+        case SkBlendMode::kSrcOver:  stage = SkRasterPipeline::srcover; break;
+        case SkBlendMode::kDstOver:  stage = SkRasterPipeline::dstover; break;
+        case SkBlendMode::kSrcIn:    stage = SkRasterPipeline::srcin; break;
+        case SkBlendMode::kDstIn:    stage = SkRasterPipeline::dstin; break;
+        case SkBlendMode::kSrcOut:   stage = SkRasterPipeline::srcout; break;
+        case SkBlendMode::kDstOut:   stage = SkRasterPipeline::dstout; break;
+        case SkBlendMode::kSrcATop:  stage = SkRasterPipeline::srcatop; break;
+        case SkBlendMode::kDstATop:  stage = SkRasterPipeline::dstatop; break;
+        case SkBlendMode::kXor:      stage = SkRasterPipeline::xor_; break;
+        case SkBlendMode::kPlus:     stage = SkRasterPipeline::plus_; break;
+        case SkBlendMode::kModulate: stage = SkRasterPipeline::modulate; break;
+
+        case SkBlendMode::kScreen:     stage = SkRasterPipeline::screen; break;
+        case SkBlendMode::kOverlay:    stage = SkRasterPipeline::overlay; break;
+        case SkBlendMode::kDarken:     stage = SkRasterPipeline::darken; break;
+        case SkBlendMode::kLighten:    stage = SkRasterPipeline::lighten; break;
+        case SkBlendMode::kColorDodge: stage = SkRasterPipeline::colordodge; break;
+        case SkBlendMode::kColorBurn:  stage = SkRasterPipeline::colorburn; break;
+        case SkBlendMode::kHardLight:  stage = SkRasterPipeline::hardlight; break;
+        case SkBlendMode::kSoftLight:  stage = SkRasterPipeline::softlight; break;
+        case SkBlendMode::kDifference: stage = SkRasterPipeline::difference; break;
+        case SkBlendMode::kExclusion:  stage = SkRasterPipeline::exclusion; break;
+        case SkBlendMode::kMultiply:   stage = SkRasterPipeline::multiply; break;
+
+        case SkBlendMode::kHue:
+        case SkBlendMode::kSaturation:
+        case SkBlendMode::kColor:
+        case SkBlendMode::kLuminosity: return false;  // TODO
+    }
+    if (p) {
+        p->append(stage);
+    }
+    return true;
+}
diff --git a/src/core/SkXfermode_proccoeff.h b/src/core/SkXfermode_proccoeff.h
index 1e6cc482a6..357de4324c 100644
--- a/src/core/SkXfermode_proccoeff.h
+++ b/src/core/SkXfermode_proccoeff.h
@@ -53,8 +53,6 @@ public:
     SK_TO_STRING_OVERRIDE()
     SK_DECLARE_PUBLIC_FLATTENABLE_DESERIALIZATION_PROCS(SkProcCoeffXfermode)
 
-    bool onAppendStages(SkRasterPipeline*) const override;
-
 protected:
     void flatten(SkWriteBuffer& buffer) const override;