spriteblitter for memcpy case (for all configs)

src/core/SkBlitter_Sprite.cpp

Index: src/core/SkBlitter_Sprite.cpp
diff --git a/src/core/SkBlitter_Sprite.cpp b/src/core/SkBlitter_Sprite.cpp
index 27cbd61768ed6db20aad874e0cbb570d1c449a19..5fb9fd9ad9442952e95a4ffa332b242d988a3622 100644
--- a/src/core/SkBlitter_Sprite.cpp
+++ b/src/core/SkBlitter_Sprite.cpp
@@ -38,6 +38,64 @@ void SkSpriteBlitter::blitMask(const SkMask&, const SkIRect& clip) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+//  Only valid if...
+//      1. src == dst format
+//      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_memcpy : public SkSpriteBlitter {
+public:
+    static bool Supports(const SkPixmap& dst, const SkPixmap& src, const SkPaint& paint) {
+        if (dst.colorType() != src.colorType()) {
+            return false;
+        }
+        if (paint.getMaskFilter() || paint.getColorFilter() || paint.getImageFilter()) {
+            return false;
+        }
+        if (0xFF != paint.getAlpha()) {
+            return false;
+        }
+        SkXfermode::Mode mode;
+        if (!SkXfermode::AsMode(paint.getXfermode(), &mode)) {
+            return false;
+        }
+        if (SkXfermode::kSrc_Mode == mode) {
+            return true;
+        }
+        if (SkXfermode::kSrcOver_Mode == mode && src.isOpaque()) {
+            return true;
+        }
+        return false;
+    }
+
+    SkSpriteBlitter_memcpy(const SkPixmap& src)  : INHERITED(src) {}
+
+    void setup(const SkPixmap& dst, int left, int top, const SkPaint& paint) override {
+        SkASSERT(Supports(dst, fSource, paint));
+        this->INHERITED::setup(dst, left, top, paint);
+    }
+
+    void blitRect(int x, int y, int width, int height) override {
+        SkASSERT(fDst.colorType() == fSource.colorType());
+        SkASSERT(width > 0 && height > 0);
+
+        void* dst = fDst.writable_addr(x, y);

[mtklein, 2016/04/15 13:29:21]

It might be clearer to cast to char* early:

   auto dst = (      char*)fDst.writeable_addr(x,y);
   auto src = (const char*)fSource.addr(x-fLeft, y-fTop);

   size_t dstRB = ...;
   size_t srcRB = ...;
   size_t bytesToCopy = ...;

   while (height --> 0) {
       memcpy(dst, src, bytesToCopy);
       dst += dstRB;
       src += srcRB;
   }
   

[reed1, 2016/04/15 13:49:10]

Done.

+        const void* src = fSource.addr(x - fLeft, y - fTop);
+        size_t dstRB = fDst.rowBytes();
+        size_t srcRB = fSource.rowBytes();
+        size_t bytesToCopy = SkLeftShift(width, fSource.shiftPerPixel());

[mtklein, 2016/04/15 13:29:21]

Seems like it's not sensible for width to be negative here, so I am skeptical of
SkLeftShift.  We've asserted that width is positive, and if it were negative,
bytesToCopy would become impossibly large.  Might want to write this the old
usual way,

    width << fSource.shiftPerPixel()

which would let ubsan point out any negative values should that case ever come
up.  (The fix wouldn't be use SkLeftShift... it'd be to figure out how width got
negative.)

[reed1, 2016/04/15 13:49:10]

Done.

+
+        do {
+            memcpy(dst, src, bytesToCopy);
+            dst = (char*)dst + dstRB;
+            src = (const char*)src + srcRB;
+        } while (--height != 0);
+    }
+
+    typedef SkSpriteBlitter INHERITED;
+};
+
+
 // returning null means the caller will call SkBlitter::Choose() and
 // have wrapped the source bitmap inside a shader
 SkBlitter* SkBlitter::ChooseSprite(const SkPixmap& dst, const SkPaint& paint,
@@ -55,23 +113,27 @@ SkBlitter* SkBlitter::ChooseSprite(const SkPixmap& dst, const SkPaint& paint,
 
     SkSpriteBlitter* blitter;
 
-    switch (dst.colorType()) {
-        case kRGB_565_SkColorType:
-            blitter = SkSpriteBlitter::ChooseD16(source, paint, allocator);
-            break;
-        case kN32_SkColorType:
-            if (dst.info().isSRGB()) {
-                blitter = SkSpriteBlitter::ChooseS32(source, paint, allocator);
-            } else {
-                blitter = SkSpriteBlitter::ChooseL32(source, paint, allocator);
-            }
-            break;
-        case kRGBA_F16_SkColorType:
-            blitter = SkSpriteBlitter::ChooseF16(source, paint, allocator);
-            break;
-        default:
-            blitter = nullptr;
-            break;
+    if (SkSpriteBlitter_memcpy::Supports(dst, source, paint)) {
+        blitter = allocator->createT<SkSpriteBlitter_memcpy>(source);
+    } else {
+        switch (dst.colorType()) {
+            case kRGB_565_SkColorType:
+                blitter = SkSpriteBlitter::ChooseD16(source, paint, allocator);
+                break;
+            case kN32_SkColorType:
+                if (dst.info().isSRGB()) {
+                    blitter = SkSpriteBlitter::ChooseS32(source, paint, allocator);
+                } else {
+                    blitter = SkSpriteBlitter::ChooseL32(source, paint, allocator);
+                }
+                break;
+            case kRGBA_F16_SkColorType:
+                blitter = SkSpriteBlitter::ChooseF16(source, paint, allocator);
+                break;
+            default:
+                blitter = nullptr;
+                break;
+        }
     }
 
     if (blitter) {