Make SkBlitter hierarchy explicit about what needs to be implemented.

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

Review-Url: https://codereview.chromium.org/2053823002

8 files changed, 68 insertions, 33 deletions

diff --git a/src/core/SkBlitter.cpp b/src/core/SkBlitter.cpp
index 343160c4b4..daced55ef2 100644
--- a/src/core/SkBlitter.cpp
+++ b/src/core/SkBlitter.cpp
@@ -38,14 +38,17 @@ const SkPixmap* SkBlitter::justAnOpaqueColor(uint32_t* value) {
     return nullptr;
 }
 
+/*
 void SkBlitter::blitH(int x, int y, int width) {
     SkDEBUGFAIL("unimplemented");
 }
 
+
 void SkBlitter::blitAntiH(int x, int y, const SkAlpha antialias[],
                           const int16_t runs[]) {
     SkDEBUGFAIL("unimplemented");
 }
+ */
 
 void SkBlitter::blitV(int x, int y, int height, SkAlpha alpha) {
     if (alpha == 255) {
diff --git a/src/core/SkBlitter.h b/src/core/SkBlitter.h
index 0095826248..42ef34249a 100644
--- a/src/core/SkBlitter.h
+++ b/src/core/SkBlitter.h
@@ -28,16 +28,18 @@ public:
     virtual ~SkBlitter();
 
     /// Blit a horizontal run of one or more pixels.
-    virtual void blitH(int x, int y, int width);
+    virtual void blitH(int x, int y, int width) = 0;
+
     /// Blit a horizontal run of antialiased pixels; runs[] is a *sparse*
     /// zero-terminated run-length encoding of spans of constant alpha values.
-    virtual void blitAntiH(int x, int y, const SkAlpha antialias[],
-                           const int16_t runs[]);
+    virtual void blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) = 0;
 
     /// Blit a vertical run of pixels with a constant alpha value.
     virtual void blitV(int x, int y, int height, SkAlpha alpha);
+
     /// Blit a solid rectangle one or more pixels wide.
     virtual void blitRect(int x, int y, int width, int height);
+
     /** Blit a rectangle with one alpha-blended column on the left,
         width (zero or more) opaque pixels, and one alpha-blended column
         on the right.
@@ -45,6 +47,7 @@ public:
     */
     virtual void blitAntiRect(int x, int y, int width, int height,
                               SkAlpha leftAlpha, SkAlpha rightAlpha);
+
     /// Blit a pattern of pixels defined by a rectangle-clipped mask;
     /// typically used for text.
     virtual void blitMask(const SkMask&, const SkIRect& clip);
diff --git a/src/core/SkBlitter_Sprite.cpp b/src/core/SkBlitter_Sprite.cpp
index dc912b0f65..90e38f498a 100644
--- a/src/core/SkBlitter_Sprite.cpp
+++ b/src/core/SkBlitter_Sprite.cpp
@@ -9,7 +9,8 @@
 #include "SkSmallAllocator.h"
 #include "SkSpriteBlitter.h"
 
-SkSpriteBlitter::SkSpriteBlitter(const SkPixmap& source) : fSource(source) {}
+SkSpriteBlitter::SkSpriteBlitter(const SkPixmap& source)
+    : fSource(source) {}
 
 void SkSpriteBlitter::setup(const SkPixmap& dst, int left, int top, const SkPaint& paint) {
     fDst = dst;
@@ -18,24 +19,32 @@ void SkSpriteBlitter::setup(const SkPixmap& dst, int left, int top, const SkPain
     fPaint = &paint;
 }
 
-#ifdef SK_DEBUG
 void SkSpriteBlitter::blitH(int x, int y, int width) {
     SkDEBUGFAIL("how did we get here?");
+
+    // Fallback to blitRect.
+    this->blitRect(x, y, width, 1);
 }
 
-void SkSpriteBlitter::blitAntiH(int x, int y, const SkAlpha antialias[],
-                                const int16_t runs[]) {
+void SkSpriteBlitter::blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) {
     SkDEBUGFAIL("how did we get here?");
+
+    // No fallback strategy.
 }
 
 void SkSpriteBlitter::blitV(int x, int y, int height, SkAlpha alpha) {
     SkDEBUGFAIL("how did we get here?");
+
+    // Fall back to superclass if the code gets here in release mode.
+    INHERITED::blitV(x, y, height, alpha);
 }
 
-void SkSpriteBlitter::blitMask(const SkMask&, const SkIRect& clip) {
+void SkSpriteBlitter::blitMask(const SkMask& mask, const SkIRect& clip) {
     SkDEBUGFAIL("how did we get here?");
+
+    // Fall back to superclass if the code gets here in release mode.
+    INHERITED::blitMask(mask, clip);
 }
-#endif
 
 ///////////////////////////////////////////////////////////////////////////////
 
@@ -44,7 +53,7 @@ void SkSpriteBlitter::blitMask(const SkMask&, const SkIRect& clip) {
 //      2. paint has no modifiers (i.e. alpha, colorfilter, etc.)
 //      3. xfermode needs no blending: e.g. kSrc_Mode or kSrcOver_Mode + opaque src
 //
-class SkSpriteBlitter_Src_SrcOver : public SkSpriteBlitter {
+class SkSpriteBlitter_Src_SrcOver final : public SkSpriteBlitter {
 public:
     static bool Supports(const SkPixmap& dst, const SkPixmap& src, const SkPaint& paint) {
         if (dst.colorType() != src.colorType()) {
@@ -80,7 +89,8 @@ public:
         return SkXfermode::kSrcOver_Mode == mode;
     }
 
-    SkSpriteBlitter_Src_SrcOver(const SkPixmap& src)  : INHERITED(src) {}
+    SkSpriteBlitter_Src_SrcOver(const SkPixmap& src)
+        : INHERITED(src) {}
 
     void setup(const SkPixmap& dst, int left, int top, const SkPaint& paint) override {
         SkASSERT(Supports(dst, fSource, paint));
diff --git a/src/core/SkLinearBitmapPipeline_sample.h b/src/core/SkLinearBitmapPipeline_sample.h
index 930759f271..5184a41af6 100644
--- a/src/core/SkLinearBitmapPipeline_sample.h
+++ b/src/core/SkLinearBitmapPipeline_sample.h
@@ -428,6 +428,7 @@ private:
     PixelAccessor<colorType, colorProfile> fStrategy;
 };
 
+// -- BilerpSampler --------------------------------------------------------------------------------
 // BilerpSampler - use a bilerp filter to create runs of destination pixels.
 template<SkColorType colorType, SkColorProfileType colorProfile, typename Next>
 class BilerpSampler : public SkLinearBitmapPipeline::SampleProcessorInterface {
diff --git a/src/core/SkRegion_path.cpp b/src/core/SkRegion_path.cpp
index 9289641c69..47df8f6d71 100644
--- a/src/core/SkRegion_path.cpp
+++ b/src/core/SkRegion_path.cpp
@@ -45,6 +45,9 @@ public:
     void    copyToRgn(SkRegion::RunType runs[]) const;
 
     void blitH(int x, int y, int width) override;
+    void blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) override {
+        SkDEBUGFAIL("blitAntiH not implemented");
+    }
 
 #ifdef SK_DEBUG
     void dump() const {
diff --git a/src/core/SkSpriteBlitter.h b/src/core/SkSpriteBlitter.h
index 62c50c8b35..3a62860867 100644
--- a/src/core/SkSpriteBlitter.h
+++ b/src/core/SkSpriteBlitter.h
@@ -15,18 +15,23 @@
 
 class SkPaint;
 
+// SkSpriteBlitter specializes SkBlitter in a way to move large rectangles of pixels around.
+// Because of this use, the main primitive shifts from blitH style things to the more efficient
+// blitRect.
 class SkSpriteBlitter : public SkBlitter {
 public:
     SkSpriteBlitter(const SkPixmap& source);
 
     virtual void setup(const SkPixmap& dst, int left, int top, const SkPaint&);
 
-#ifdef SK_DEBUG
+    // blitH, blitAntiH, blitV and blitMask should not be called on an SkSpriteBlitter.
     void blitH(int x, int y, int width) override;
     void blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) override;
     void blitV(int x, int y, int height, SkAlpha alpha) override;
     void blitMask(const SkMask&, const SkIRect& clip) override;
-#endif
+
+    // A SkSpriteBlitter must implement blitRect.
+    void blitRect(int x, int y, int width, int height) override = 0;
 
     static SkSpriteBlitter* ChooseD16(const SkPixmap& source, const SkPaint&, SkTBlitterAllocator*);
     static SkSpriteBlitter* ChooseL32(const SkPixmap& source, const SkPaint&, SkTBlitterAllocator*);
@@ -38,6 +43,9 @@ protected:
     const SkPixmap  fSource;
     int             fLeft, fTop;
     const SkPaint*  fPaint;
+
+private:
+    typedef SkBlitter INHERITED;
 };
 
 #endif
diff --git a/tests/BlitMaskClip.cpp b/tests/BlitMaskClip.cpp
index 8ba2cea3e0..21fe562368 100644
--- a/tests/BlitMaskClip.cpp
+++ b/tests/BlitMaskClip.cpp
@@ -24,6 +24,10 @@ public:
         REPORTER_ASSERT(fReporter, right > fBounds.fLeft && right <= fBounds.fRight);
     }
 
+    void blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) override {
+        SkDEBUGFAIL("blitAntiH not implemented");
+    }
+
 private:
     SkIRect fBounds;
     skiatest::Reporter* fReporter;
diff --git a/tests/FillPathTest.cpp b/tests/FillPathTest.cpp
index cc8d329652..1f14d4f7f3 100644
--- a/tests/FillPathTest.cpp
+++ b/tests/FillPathTest.cpp
@@ -12,34 +12,37 @@
 #include "Test.h"
 
 struct FakeBlitter : public SkBlitter {
-  FakeBlitter()
-      : m_blitCount(0)
-  {}
+    FakeBlitter()
+        : m_blitCount(0) { }
 
-  virtual void blitH(int x, int y, int width) {
-    m_blitCount++;
-  }
+    void blitH(int x, int y, int width) override {
+        m_blitCount++;
+    }
 
-  int m_blitCount;
+    void blitAntiH(int x, int y, const SkAlpha antialias[], const int16_t runs[]) override {
+      SkDEBUGFAIL("blitAntiH not implemented");
+    }
+
+    int m_blitCount;
 };
 
 // http://code.google.com/p/skia/issues/detail?id=87
 // Lines which is not clipped by boundary based clipping,
 // but skipped after tessellation, should be cleared by the blitter.
 DEF_TEST(FillPathInverse, reporter) {
-  FakeBlitter blitter;
-  SkIRect clip;
-  SkPath path;
-  int height = 100;
-  int width  = 200;
-  int expected_lines = 5;
-  clip.set(0, height - expected_lines, width, height);
-  path.moveTo(0.0f, 0.0f);
-  path.quadTo(SkIntToScalar(width/2), SkIntToScalar(height),
+    FakeBlitter blitter;
+    SkIRect clip;
+    SkPath path;
+    int height = 100;
+    int width  = 200;
+    int expected_lines = 5;
+    clip.set(0, height - expected_lines, width, height);
+    path.moveTo(0.0f, 0.0f);
+    path.quadTo(SkIntToScalar(width/2), SkIntToScalar(height),
               SkIntToScalar(width), 0.0f);
-  path.close();
-  path.setFillType(SkPath::kInverseWinding_FillType);
-  SkScan::FillPath(path, clip, &blitter);
+    path.close();
+    path.setFillType(SkPath::kInverseWinding_FillType);
+    SkScan::FillPath(path, clip, &blitter);
 
-  REPORTER_ASSERT(reporter, blitter.m_blitCount == expected_lines);
+    REPORTER_ASSERT(reporter, blitter.m_blitCount == expected_lines);
 }