Add documention on SkBlitter for runs, and small cleanups.

src/core/SkBlitter_ARGB32.cpp

Index: src/core/SkBlitter_ARGB32.cpp
diff --git a/src/core/SkBlitter_ARGB32.cpp b/src/core/SkBlitter_ARGB32.cpp
index af62f2eb1d8c8d1d0807b04e36eabf6892ebc2d6..e4f8178b67a1bc46a416c0638e8b6a5bc8f3121e 100644
--- a/src/core/SkBlitter_ARGB32.cpp
+++ b/src/core/SkBlitter_ARGB32.cpp
@@ -170,14 +170,16 @@ void SkARGB32_Blitter::blitMask(const SkMask& mask, const SkIRect& clip) {
         return;
     }
 
-    if (SkBlitMask::BlitColor(fDevice, mask, clip, fColor)) {
-        return;
-    }
-
-    if (mask.fFormat == SkMask::kBW_Format) {
-        SkARGB32_BlendBW(fDevice, mask, clip, fPMColor, SkAlpha255To256(255 - fSrcA));
-    } else if (SkMask::kARGB32_Format == mask.fFormat) {
-        SkARGB32_Blit32(fDevice, mask, clip, fPMColor);
+    switch (mask.fFormat) {
+        case SkMask::kBW_Format:
+            SkARGB32_BlendBW(fDevice, mask, clip, fPMColor, SkAlpha255To256(255 - fSrcA));
+            break;
+        case SkMask::kARGB32_Format:
+            SkARGB32_Blit32(fDevice, mask, clip, fPMColor);
+            break;
+        default:
+            // If this asserts then the mask case was not handled.
+            SkAssertResult(SkBlitMask::BlitColor(fDevice, mask, clip, fColor));
     }
 }
 
@@ -185,14 +187,16 @@ void SkARGB32_Opaque_Blitter::blitMask(const SkMask& mask,
                                        const SkIRect& clip) {
     SkASSERT(mask.fBounds.contains(clip));
 
-    if (SkBlitMask::BlitColor(fDevice, mask, clip, fColor)) {
-        return;
-    }
-
-    if (mask.fFormat == SkMask::kBW_Format) {
-        SkARGB32_BlitBW(fDevice, mask, clip, fPMColor);
-    } else if (SkMask::kARGB32_Format == mask.fFormat) {
-        SkARGB32_Blit32(fDevice, mask, clip, fPMColor);
+    switch (mask.fFormat) {
+        case SkMask::kBW_Format:
+            SkARGB32_BlitBW(fDevice, mask, clip, fPMColor);
+            break;
+        case SkMask::kARGB32_Format:
+            SkARGB32_Blit32(fDevice, mask, clip, fPMColor);
+            break;
+        default:
+            // If this asserts then the mask case was not handled.
+            SkAssertResult(SkBlitMask::BlitColor(fDevice, mask, clip, fColor));

[reed1, 2016/06/13 11:03:14]

My read is the BlitColor was written as a chance for optimizations, so the
caller invoked it first, and only then handled the other cases (as needed). This
change subtly changes that, not giving it a chance at BW or ARGB32. If this is
the new correct, perhaps we should rename or somehow indicate that in BlitColor
(even moving the assert inside it instead of w/ the caller). What do you think?

[herb_g, 2016/06/22 17:54:53]

Fixed.
I had assumed that the BlitColor call and the if statement were disjoint,
because the if statement did not handle all cases. 

     }
 }