Scanline decoding for wbmp

src/codec/SkSwizzler.cpp

Index: src/codec/SkSwizzler.cpp
diff --git a/src/codec/SkSwizzler.cpp b/src/codec/SkSwizzler.cpp
index a91cf511ee99ce312fcafada4bd040f43d5e0394..906a4999f373a3b84e0b551f407fcd1aa5715016 100644
--- a/src/codec/SkSwizzler.cpp
+++ b/src/codec/SkSwizzler.cpp
@@ -19,6 +19,84 @@ SkSwizzler::ResultAlpha SkSwizzler::GetResult(uint8_t zeroAlpha,
     return (((uint16_t) maxAlpha) << 8) | zeroAlpha;
 }
 
+// kBit
+// These routines exclusively choose between white and black
+
+#define GRAYSCALE_BLACK 0
+#define GRAYSCALE_WHITE 0xFF
+
+static SkSwizzler::ResultAlpha swizzle_bit_to_grayscale(
+        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
+        int bitsPerPixel, const SkPMColor ctable[]) {

[scroggo, 2015/07/29 17:47:12]

I believe these parameters are unused? I think we have some macros for
documenting/enforcing that, but I believe they are used sporadically.

Another approach would be to declare them like so:

int /* bitsPerPixel */, const SkPMColor* /* ctable */

[msarett, 2015/07/29 18:53:57]

Done.

+    uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
+    int bytes = width >> 3;
+    for (int i = 0; i < bytes; i++) {
+        U8CPU mask = *src++;

[scroggo, 2015/07/29 17:47:12]

I don't have an idea for a better name for this, but I feel like a "mask" is
something which does masking, whereas this is the thing being masked?

[msarett, 2015/07/29 18:53:57]

Yeah you're right.  Renaming to currByte.

+        for (int j = 0; j < 8; j++) {
+            dst[j] = ((mask >> (7 - j)) & 1) ? GRAYSCALE_WHITE : GRAYSCALE_BLACK;
+        }
+        dst += 8;

[scroggo, 2015/07/29 17:47:12]

It seems a little odd that here you index dst and then add 8, but below you do
pointer arithmetic on dst. I suppose it's because here you happen to also need j
for other reasons?

[msarett, 2015/07/29 18:53:57]

There is no reason for this other than that is how it was done before.  Which is
not a good reason.

I prefer not using ptr arithmetic and have made changes.  Sorry I didn't really
look this routine over, I just borrowed it from SkCodec_wbmp.

[scroggo, 2015/07/29 21:49:16]

I do not have a problem with pointer arithmetic in general; my only confusion
reading the code was that one block used it, and the other used indices.

+    }
+    width &= 7;
+    if (width > 0) {
+        U8CPU mask = *src;
+        do {
+            *dst++ = ((mask >> 7) & 1) ? GRAYSCALE_WHITE : GRAYSCALE_BLACK;
+            mask <<= 1;
+        } while (--width != 0);
+    }
+    return SkSwizzler::kOpaque_ResultAlpha;
+}
+
+#undef GRAYSCALE_BLACK
+#undef GRAYSCALE_WHITE
+
+static SkSwizzler::ResultAlpha swizzle_bit_to_index(
+        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
+        int bitsPerPixel, const SkPMColor ctable[]) {
+    uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
+    int bytes = width >> 3;
+    for (int i = 0; i < bytes; i++) {
+        U8CPU mask = *src++;
+        for (int j = 0; j < 8; j++) {
+            dst[j] = (mask >> (7 - j)) & 1;
+        }
+        dst += 8;
+    }
+    width &= 7;
+    if (width > 0) {
+        U8CPU mask = *src;
+        do {
+            *dst++ = (mask >> 7) & 1;
+            mask <<= 1;
+        } while (--width != 0);
+    }
+    return SkSwizzler::kOpaque_ResultAlpha;
+}
+
+static SkSwizzler::ResultAlpha swizzle_bit_to_n32(
+        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int width,
+        int bitsPerPixel, const SkPMColor ctable[]) {
+    SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow;
+    int bytes = width >> 3;
+    for (int i = 0; i < bytes; i++) {
+        U8CPU mask = *src++;
+        for (int j = 0; j < 8; j++) {
+            dst[j] = ((mask >> (7 - j)) & 1) ? SK_ColorWHITE : SK_ColorBLACK;
+        }
+        dst += 8;
+    }
+    width &= 7;
+    if (width > 0) {
+        U8CPU mask = *src;
+        do {
+            *dst++ = ((mask >> 7) & 1) ? SK_ColorWHITE : SK_ColorBLACK;
+            mask <<= 1;
+        } while (--width != 0);
+    }
+    return SkSwizzler::kOpaque_ResultAlpha;
+}
+
 // kIndex1, kIndex2, kIndex4
 
 static SkSwizzler::ResultAlpha swizzle_small_index_to_index(
@@ -284,6 +362,21 @@ SkSwizzler* SkSwizzler::CreateSwizzler(SkSwizzler::SrcConfig sc,
     }
     RowProc proc = NULL;
     switch (sc) {
+        case kBit:
+            switch (info.colorType()) {
+                case kN32_SkColorType:
+                    proc = &swizzle_bit_to_n32;
+                    break;
+                case kIndex_8_SkColorType:
+                    proc = &swizzle_bit_to_index;
+                    break;
+                case kGray_8_SkColorType:
+                    proc = &swizzle_bit_to_grayscale;
+                    break;
+                default:
+                    break;
+            }
+            break;
         case kIndex1:
         case kIndex2:
         case kIndex4: