Scanline decoding for bmp

src/codec/SkSwizzler.cpp

Index: src/codec/SkSwizzler.cpp
diff --git a/src/codec/SkSwizzler.cpp b/src/codec/SkSwizzler.cpp
index 8315cc7145a406f698f0f9ca40f91df9b5af2eff..7344e281f28d35701a041a7d549643df49a3cc37 100644
--- a/src/codec/SkSwizzler.cpp
+++ b/src/codec/SkSwizzler.cpp
@@ -22,7 +22,7 @@ SkSwizzler::ResultAlpha SkSwizzler::GetResult(uint8_t zeroAlpha,
 
 // samples the row. Does not do anything else but sampling
 static SkSwizzler::ResultAlpha sample565(void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src,
-        int width, int deltaSrc, int offset, const SkPMColor ctable[]){
+        int width, int bpp, int deltaSrc, int offset, const SkPMColor ctable[]){
 
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
@@ -44,7 +44,7 @@ static SkSwizzler::ResultAlpha sample565(void* SK_RESTRICT dstRow, const uint8_t
 // same as swizzle_bit_to_index and swizzle_bit_to_n32 except for value assigned to dst[x]
 static SkSwizzler::ResultAlpha swizzle_bit_to_grayscale(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
+        int bpp, int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
 
     uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
 
@@ -71,7 +71,7 @@ static SkSwizzler::ResultAlpha swizzle_bit_to_grayscale(
 // same as swizzle_bit_to_grayscale and swizzle_bit_to_n32 except for value assigned to dst[x]
 static SkSwizzler::ResultAlpha swizzle_bit_to_index(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
+        int bpp, int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
     uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
 
     // increment src by byte offset and bitIndex by bit offset
@@ -94,7 +94,7 @@ static SkSwizzler::ResultAlpha swizzle_bit_to_index(
 // same as swizzle_bit_to_grayscale and swizzle_bit_to_index except for value assigned to dst[x]
 static SkSwizzler::ResultAlpha swizzle_bit_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
+        int bpp, int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
     SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow;
 
     // increment src by byte offset and bitIndex by bit offset
@@ -119,7 +119,7 @@ static SkSwizzler::ResultAlpha swizzle_bit_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_bit_to_565(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
+        int bpp, int deltaSrc, int offset, const SkPMColor* /*ctable*/) {
     uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
 
     // increment src by byte offset and bitIndex by bit offset
@@ -146,72 +146,73 @@ static SkSwizzler::ResultAlpha swizzle_bit_to_565(
 
 static SkSwizzler::ResultAlpha swizzle_small_index_to_index(

[msarett, 2015/08/19 14:22:40]

These functions use a similar strategy to the swizzle_bit functions.

[scroggo, 2015/08/26 22:40:09]

I'm not sure I understand the comment. Should this comment go in the code? Or
are you proposing merging them somehow?

[msarett, 2015/08/27 15:00:27]

I was just trying to provide some context for how I redesigned these in case it
helps with the review.

         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int bitsPerPixel, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
-    src += offset;
-    uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
+    uint8_t* dst = (uint8_t*) dstRow;
     INIT_RESULT_ALPHA;
-    const uint32_t pixelsPerByte = 8 / bitsPerPixel;
-    const size_t rowBytes = compute_row_bytes_ppb(dstWidth, pixelsPerByte);
-    const uint8_t mask = (1 << bitsPerPixel) - 1;
-    int x = 0;
-    for (uint32_t byte = 0; byte < rowBytes; byte++) {
-        uint8_t pixelData = src[byte];
-        for (uint32_t p = 0; p < pixelsPerByte && x < dstWidth; p++) {
-            uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
-            UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
-            dst[x] = index;
-            pixelData <<= bitsPerPixel;
-            x++;
-        }
+    src += offset / 8;
+    int bitIndex = offset % 8;
+    uint8_t currByte = *src;
+    const uint8_t mask = (1 << bpp) - 1;

[msarett, 2015/08/19 14:22:40]

Unfortunately, these functions need to know the bitsPerPixel.  We could
eliminate a parameter by passing sampleX instead of deltaSrc and offset, but
that would bring more math into these swizzling functions.  I think it's fine as
is, but am happy with it either way.

+    uint8_t index = (currByte >> (8 - bpp - bitIndex)) & mask;
+    dst[0] = index;
+    UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
+
+    for (int x = 1; x < dstWidth; x++) {
+        int bitOffset = bitIndex + deltaSrc;
+        bitIndex = bitOffset % 8;
+        currByte = *(src += bitOffset / 8);
+        index = (currByte >> (8 - bpp - bitIndex)) & mask;
+        dst[x] = index;
+        UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
     }
     return COMPUTE_RESULT_ALPHA;
 }
 
 static SkSwizzler::ResultAlpha swizzle_small_index_to_565(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int bitsPerPixel, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
-    src += offset;
-    uint16_t* SK_RESTRICT dst = (uint16_t*) dstRow;
-    const uint32_t pixelsPerByte = 8 / bitsPerPixel;
-    const size_t rowBytes = compute_row_bytes_ppb(dstWidth, pixelsPerByte);
-    const uint8_t mask = (1 << bitsPerPixel) - 1;
-    int x = 0;
-    for (uint32_t byte = 0; byte < rowBytes; byte++) {
-        uint8_t pixelData = src[byte];
-        for (uint32_t p = 0; p < pixelsPerByte && x < dstWidth; p++) {
-            uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
-            uint16_t c = SkPixel32ToPixel16(ctable[index]);
-            dst[x] = c;
-            pixelData <<= bitsPerPixel;
-            x++;
-        }
+    uint16_t* dst = (uint16_t*) dstRow;
+    INIT_RESULT_ALPHA;
+    src += offset / 8;
+    int bitIndex = offset % 8;
+    uint8_t currByte = *src;
+    const uint8_t mask = (1 << bpp) - 1;
+    uint8_t index = (currByte >> (8 - bpp - bitIndex)) & mask;
+    dst[0] = SkPixel32ToPixel16(ctable[index]);
+
+    for (int x = 1; x < dstWidth; x++) {
+        int bitOffset = bitIndex + deltaSrc;
+        bitIndex = bitOffset % 8;
+        currByte = *(src += bitOffset / 8);
+        index = (currByte >> (8 - bpp - bitIndex)) & mask;
+        dst[x] = SkPixel32ToPixel16(ctable[index]);
     }
-    return SkSwizzler::kOpaque_ResultAlpha;
+    return SkAlphaType::kOpaque_SkAlphaType;
 }
 
 static SkSwizzler::ResultAlpha swizzle_small_index_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int bitsPerPixel, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
-    src += offset;
-    SkPMColor* SK_RESTRICT dst = (SkPMColor*) dstRow;
+    SkPMColor* dst = (SkPMColor*) dstRow;
     INIT_RESULT_ALPHA;
-    const uint32_t pixelsPerByte = 8 / bitsPerPixel;
-    const size_t rowBytes = compute_row_bytes_ppb(dstWidth, pixelsPerByte);
-    const uint8_t mask = (1 << bitsPerPixel) - 1;
-    int x = 0;
-    for (uint32_t byte = 0; byte < rowBytes; byte++) {
-        uint8_t pixelData = src[byte];
-        for (uint32_t p = 0; p < pixelsPerByte && x < dstWidth; p++) {
-            uint8_t index = (pixelData >> (8 - bitsPerPixel)) & mask;
-            SkPMColor c = ctable[index];
-            UPDATE_RESULT_ALPHA(c >> SK_A32_SHIFT);
-            dst[x] = c;
-            pixelData <<= bitsPerPixel;
-            x++;
-        }
+    src += offset / 8;
+    int bitIndex = offset % 8;
+    uint8_t currByte = *src;
+    const uint8_t mask = (1 << bpp) - 1;
+    uint8_t index = (currByte >> (8 - bpp - bitIndex)) & mask;
+    dst[0] = ctable[index];
+    UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
+
+    for (int x = 1; x < dstWidth; x++) {
+        int bitOffset = bitIndex + deltaSrc;
+        bitIndex = bitOffset % 8;
+        currByte = *(src += bitOffset / 8);
+        index = (currByte >> (8 - bpp - bitIndex)) & mask;
+        dst[x] = ctable[index];
+        UPDATE_RESULT_ALPHA(ctable[index] >> SK_A32_SHIFT);
     }
     return COMPUTE_RESULT_ALPHA;
 }
@@ -220,7 +221,7 @@ static SkSwizzler::ResultAlpha swizzle_small_index_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_index_to_index(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
@@ -248,7 +249,7 @@ static SkSwizzler::ResultAlpha swizzle_index_to_index(
 
 static SkSwizzler::ResultAlpha swizzle_index_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -264,7 +265,7 @@ static SkSwizzler::ResultAlpha swizzle_index_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_index_to_n32_skipZ(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -282,14 +283,14 @@ static SkSwizzler::ResultAlpha swizzle_index_to_n32_skipZ(
 
 static SkSwizzler::ResultAlpha swizzle_index_to_565(
       void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-      int bytesPerPixel, int offset, const SkPMColor ctable[]) {
+      int bytesPerPixel, int deltaSrc, int offset, const SkPMColor ctable[]) {
     // FIXME: Support dithering? Requires knowing y, which I think is a bigger
     // change.
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPixel32ToPixel16(ctable[*src]);
-        src += bytesPerPixel;
+        src += deltaSrc;
     }
     return SkSwizzler::kOpaque_ResultAlpha;
 }
@@ -301,7 +302,7 @@ static SkSwizzler::ResultAlpha swizzle_index_to_565(
 
 static SkSwizzler::ResultAlpha swizzle_gray_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -314,7 +315,7 @@ static SkSwizzler::ResultAlpha swizzle_gray_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_gray_to_gray(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     uint8_t* SK_RESTRICT dst = (uint8_t*) dstRow;
@@ -331,13 +332,13 @@ static SkSwizzler::ResultAlpha swizzle_gray_to_gray(
 
 static SkSwizzler::ResultAlpha swizzle_gray_to_565(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int bytesPerPixel, int offset, const SkPMColor ctable[]) {
+        int bytesPerPixel, int deltaSrc, int offset, const SkPMColor ctable[]) {
     // FIXME: Support dithering?
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPack888ToRGB16(src[0], src[0], src[0]);
-        src += bytesPerPixel;
+        src += deltaSrc;
     }
     return SkSwizzler::kOpaque_ResultAlpha;
 }
@@ -346,7 +347,7 @@ static SkSwizzler::ResultAlpha swizzle_gray_to_565(
 
 static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -359,7 +360,7 @@ static SkSwizzler::ResultAlpha swizzle_bgrx_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_bgrx_to_565(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
     // FIXME: Support dithering?
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
@@ -374,7 +375,7 @@ static SkSwizzler::ResultAlpha swizzle_bgrx_to_565(
 
 static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_unpremul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -390,7 +391,7 @@ static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_unpremul(
 
 static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_premul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -407,7 +408,7 @@ static SkSwizzler::ResultAlpha swizzle_bgra_to_n32_premul(
 // kRGBX
 static SkSwizzler::ResultAlpha swizzle_rgbx_to_n32(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -420,13 +421,13 @@ static SkSwizzler::ResultAlpha swizzle_rgbx_to_n32(
 
 static SkSwizzler::ResultAlpha swizzle_rgbx_to_565(
        void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-       int bytesPerPixel, int offset, const SkPMColor ctable[]) {
+       int bytesPerPixel, int deltaSrc, int offset, const SkPMColor ctable[]) {
     // FIXME: Support dithering?
     src += offset;
     uint16_t* SK_RESTRICT dst = (uint16_t*)dstRow;
     for (int x = 0; x < dstWidth; x++) {
         dst[x] = SkPack888ToRGB16(src[0], src[1], src[2]);
-        src += bytesPerPixel;
+        src += deltaSrc;
     }
     return SkSwizzler::kOpaque_ResultAlpha;
 }
@@ -435,7 +436,7 @@ static SkSwizzler::ResultAlpha swizzle_rgbx_to_565(
 // kRGBA
 static SkSwizzler::ResultAlpha swizzle_rgba_to_n32_premul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -451,7 +452,7 @@ static SkSwizzler::ResultAlpha swizzle_rgba_to_n32_premul(
 
 static SkSwizzler::ResultAlpha swizzle_rgba_to_n32_unpremul(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     uint32_t* SK_RESTRICT dst = reinterpret_cast<uint32_t*>(dstRow);
@@ -467,7 +468,7 @@ static SkSwizzler::ResultAlpha swizzle_rgba_to_n32_unpremul(
 
 static SkSwizzler::ResultAlpha swizzle_rgba_to_n32_premul_skipZ(
         void* SK_RESTRICT dstRow, const uint8_t* SK_RESTRICT src, int dstWidth,
-        int deltaSrc, int offset, const SkPMColor ctable[]) {
+        int bpp, int deltaSrc, int offset, const SkPMColor ctable[]) {
 
     src += offset;
     SkPMColor* SK_RESTRICT dst = (SkPMColor*)dstRow;
@@ -700,7 +701,7 @@ SkSwizzler::SkSwizzler(RowProc proc, const SkPMColor* ctable,
     , fDeltaSrc(deltaSrc)
     , fDstInfo(info)
     , fSampleX(sampleX)
-    , fX0(sampleX == 1 ? 0 : sampleX >> 1)
+    , fX0(SkScaledCodec::GetStartCoord(sampleX))
 {
     // check that fX0 is less than original width
     SkASSERT(fX0 >= 0 && fX0 < fDstInfo.width() * fSampleX);
@@ -708,11 +709,13 @@ SkSwizzler::SkSwizzler(RowProc proc, const SkPMColor* ctable,
 
 SkSwizzler::ResultAlpha SkSwizzler::swizzle(void* dst, const uint8_t* SK_RESTRICT src) {
     SkASSERT(NULL != dst && NULL != src);
-    return fRowProc(dst, src, fDstInfo.width(), fSampleX * fDeltaSrc, fX0 * fDeltaSrc, fColorTable);
+    return fRowProc(dst, src, fDstInfo.width(), fDeltaSrc, fSampleX * fDeltaSrc,
+            fX0 * fDeltaSrc, fColorTable);
 }
 
 void SkSwizzler::Fill(void* dstStartRow, const SkImageInfo& dstInfo, size_t dstRowBytes,

[msarett, 2015/08/19 14:22:40]

Sorry I didn't mean to put all of this refactoring of the Fill() function in
this CL.  There are incomplete bmps on the bots, so SkScaledCodec needs to
support filling.  I originally thought SkScaledCodec would call Fill(), but I've
actually implemented it so that is not necessary.

I think a follow up CL will need to test on incomplete images on all types.  I
suspect that we don't handle incomplete pngs gracefully.

-        uint32_t numRows, uint32_t colorOrIndex, const SkPMColor* colorTable) {
+        uint32_t numRows, uint32_t colorOrIndex, const SkPMColor* colorTable,
+        SkCodec::ZeroInitialized zeroInit) {
     SkASSERT(dstStartRow != NULL);
     SkASSERT(numRows <= (uint32_t) dstInfo.height());
 
@@ -725,13 +728,17 @@ void SkSwizzler::Fill(void* dstStartRow, const SkImageInfo& dstInfo, size_t dstR
             // Assume input is an index if we have a color table
             uint32_t color;
             if (NULL != colorTable) {
-                SkASSERT(colorOrIndex == (uint8_t) colorOrIndex);
-                color = colorTable[colorOrIndex];
+                color = colorTable[(uint8_t) colorOrIndex];
             // Otherwise, assume the input is a color
             } else {
                 color = colorOrIndex;
             }
 
+            // If memory is zero initialized, we may not need to fill
+            if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == color) {

[msarett, 2015/08/19 14:22:40]

This fixes a bug.

We used to check if colorOrIndex was 0 instead of if color is 0.  When the
output color type is kN32, we really need to know if the actual color is 0.

When I follow up with a CL testing incomplete images, I'll add a test.

+                return;
+            }
+
             // We must fill row by row in the case of unaligned row bytes
             if (SkIsAlign4((size_t) dstStartRow) && SkIsAlign4(dstRowBytes)) {
                 sk_memset32((uint32_t*) dstStartRow, color,
@@ -748,28 +755,34 @@ void SkSwizzler::Fill(void* dstStartRow, const SkImageInfo& dstInfo, size_t dstR
                 }
             }
             break;
-        // On an index destination color type, always assume the input is an index
+        case kRGB_565_SkColorType:
+            // If the destination is k565, the caller passes in a 16-bit color.
+            // We will not assert that the high bits of colorOrIndex must be zeroed.
+            // This allows us to take advantage of the fact that the low 16 bits of an
+            // SKPMColor may be a valid a 565 color.  For example, the low 16
+            // bits of SK_ColorBLACK are identical to the 565 representation
+            // for black.
+            // If we ever want to fill with colorOrIndex != 0, we will probably need
+            // to implement this with sk_memset16().
+            SkASSERT((uint16_t) colorOrIndex == (uint8_t) colorOrIndex);
+            // Fall through
         case kIndex_8_SkColorType:
-            SkASSERT(colorOrIndex == (uint8_t) colorOrIndex);
-            memset(dstStartRow, colorOrIndex, bytesToFill);
-            break;
+            // On an index destination color type, always assume the input is an index.
+            // Fall through
         case kGray_8_SkColorType:
             // If the destination is kGray, the caller passes in an 8-bit color.
             // We will not assert that the high bits of colorOrIndex must be zeroed.
             // This allows us to take advantage of the fact that the low 8 bits of an
             // SKPMColor may be a valid a grayscale color.  For example, the low 8
             // bits of SK_ColorBLACK are identical to the grayscale representation
-            // for black. 
-            memset(dstStartRow, (uint8_t) colorOrIndex, bytesToFill);
-            break;
-        case kRGB_565_SkColorType:
-            // If the destination is k565, the caller passes in a 16-bit color.
-            // We will not assert that the high bits of colorOrIndex must be zeroed.
-            // This allows us to take advantage of the fact that the low 16 bits of an
-            // SKPMColor may be a valid a 565 color.  For example, the low 16
-            // bits of SK_ColorBLACK are identical to the 565 representation
             // for black.
-            memset(dstStartRow, (uint16_t) colorOrIndex, bytesToFill);
+
+            // If memory is zero initialized, we may not need to fill
+            if (SkCodec::kYes_ZeroInitialized == zeroInit && 0 == (uint8_t) colorOrIndex) {

[msarett, 2015/08/19 14:22:40]

This fixes a bug.

Only the low 8-bits of colorOrIndex need to be 0 for this to qualify as a zero
fill.

+                return;
+            }
+
+            memset(dstStartRow, (uint8_t) colorOrIndex, bytesToFill);
             break;
         default:
             SkCodecPrintf("Error: Unsupported dst color type for fill().  Doing nothing.\n");