Adding premul and 565 swizzles for bmp:

src/codec/SkCodec_libbmp.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkCodec_libbmp.h"
 #include "SkCodecPriv.h"
 #include "SkColorPriv.h"
 #include "SkStream.h"
 
 /*
  *
  * Checks if the conversion between the input image and the requested output
  * image has been implemented
  *
  */
 static bool conversion_possible(const SkImageInfo& dst,
                                 const SkImageInfo& src) {
-    // All of the swizzles convert to kN32
-    // TODO: Update this when more swizzles are supported
-    if (kN32_SkColorType != dst.colorType()) {
-        return false;
+    // Check for supported color and alpha types
+    switch (dst.colorType()) {
+        case kN32_SkColorType:
+            return src.alphaType() == dst.alphaType() ||
+                    premul_and_unpremul(dst.alphaType(), src.alphaType());

[scroggo, 2015/03/17 20:02:58]

Oh, clever. You don't need to check this in both orders, since we know the src
could only have been unpremul or opaque. I naively checked in both directions
for PNG....

Given that, it might be clearer to remove premul_and_unpremul and just inline
the code (both here and in PNG).

[msarett, 2015/03/18 13:02:52]

Agreed.

+        case kRGB_565_SkColorType:
+            return src.alphaType() == dst.alphaType() &&
+                    kOpaque_SkAlphaType == dst.alphaType();
+        default:
+            return false;
     }
-    // Support the swizzle if the requested alpha type is the same as our guess
-    // for the input alpha type
-    if (src.alphaType() == dst.alphaType()) {
-        return true;
-    }
-    // TODO: Support more swizzles, especially premul
-    return false;
 }
 
 /*
  *
  * Defines the version and type of the second bitmap header
  *
  */
 enum BitmapHeaderType {
     kInfoV1_BitmapHeaderType,
     kInfoV2_BitmapHeaderType,
@@ skipping 197 matching lines @@
     }
     static const int kBmpMaxDim = 1 << 16;
     if (width < 0 || width >= kBmpMaxDim || height >= kBmpMaxDim) {
         // TODO: Decide if we want to support really large bmps.
         SkDebugf("Error: invalid bitmap dimensions.\n");
         return NULL;
     }
 
     // Create mask struct
     SkMasks::InputMasks inputMasks;
     memset(&inputMasks, 0, 4*sizeof(uint32_t));

[scroggo, 2015/03/17 20:02:58]

nit: Maybe this should be sizeof(SkMasks::InputMasks)? Probably fine as is, but
if we changed the members of InputMasks, this might not zero them out properly.

[msarett, 2015/03/18 13:02:52]

Done.

 
     // Determine the input compression format and set bit masks if necessary
     uint32_t maskBytes = 0;
     BitmapInputFormat inputFormat = kUnknown_BitmapInputFormat;
     switch (compression) {
         case kNone_BitmapCompressionMethod:
             inputFormat = kStandard_BitmapInputFormat;
             break;
         case k8BitRLE_BitmapCompressionMethod:
             if (bitsPerPixel != 8) {
@@ skipping 89 matching lines @@
         // Header types are matched based on size.  If the header is
         // V4+, we are guaranteed to be able to read at least this size.
         SkASSERT(infoBytesRemaining > 52);
         inputMasks.alpha = get_int(iBuffer.get(), 48);
         if (inputMasks.alpha != 0) {
             alphaType = kUnpremul_SkAlphaType;
         }
     }
     iBuffer.free();
 
+    // Check for a valid number of total bytes when in RLE mode
+    const int RLEBytes = totalBytes - offset;

[scroggo, 2015/03/17 20:02:58]

size_t

[msarett, 2015/03/18 13:02:52]

Done.

+    if (RLEBytes <= 0 && kRLE_BitmapInputFormat == inputFormat) {
+        SkDebugf("Error: RLE requires valid input size.\n");
+        return NULL;
+    }
+
     // Check for valid bits per pixel input
     switch (bitsPerPixel) {
         // In addition to more standard pixel compression formats, bmp supports
         // the use of bit masks to determine pixel components.  The standard
         // format for representing 16-bit colors is 555 (XRRRRRGGGGGBBBBB),
         // which does not map well to any Skia color formats.  For this reason,
         // we will always enable mask mode with 16 bits per pixel.
         case 16:
             if (kBitMask_BitmapInputFormat != inputFormat) {
                 inputMasks.red = 0x7C00;
@@ skipping 15 matching lines @@
     }
 
     // Check that input bit masks are valid and create the masks object
     SkAutoTDelete<SkMasks>
             masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
     if (NULL == masks) {
         SkDebugf("Error: invalid input masks.\n");
         return NULL;
     }
 
-    // Process the color table
-    uint32_t colorBytes = 0;
-    SkPMColor* colorTable = NULL;
-    if (bitsPerPixel < 16) {
-        // Verify the number of colors for the color table
-        const uint32_t maxColors = 1 << bitsPerPixel;
-        // Zero is a default for maxColors
-        // Also set numColors to maxColors when input is too large
-        if (numColors <= 0 || numColors > maxColors) {
-            numColors = maxColors;
-        }
-        colorTable = SkNEW_ARRAY(SkPMColor, maxColors);
-
-        // Construct the color table
-        colorBytes = numColors * bytesPerColor;
-        SkAutoTDeleteArray<uint8_t> cBuffer(SkNEW_ARRAY(uint8_t, colorBytes));
-        if (stream->read(cBuffer.get(), colorBytes) != colorBytes) {
-            SkDebugf("Error: unable to read color table.\n");
-            return NULL;
-        }
-
-        // Fill in the color table (colors are stored unpremultiplied)
-        uint32_t i = 0;
-        for (; i < numColors; i++) {
-            uint8_t blue = get_byte(cBuffer.get(), i*bytesPerColor);
-            uint8_t green = get_byte(cBuffer.get(), i*bytesPerColor + 1);
-            uint8_t red = get_byte(cBuffer.get(), i*bytesPerColor + 2);
-            uint8_t alpha = 0xFF;
-            if (kOpaque_SkAlphaType != alphaType) {
-                alpha = (inputMasks.alpha >> 24) &
-                        get_byte(cBuffer.get(), i*bytesPerColor + 3);
-            }
-            // Store the unpremultiplied color
-            colorTable[i] = SkPackARGB32NoCheck(alpha, red, green, blue);
-        }
-
-        // To avoid segmentation faults on bad pixel data, fill the end of the
-        // color table with black.  This is the same the behavior as the
-        // chromium decoder.
-        for (; i < maxColors; i++) {
-            colorTable[i] = SkPackARGB32NoCheck(0xFF, 0, 0, 0);
-        }
-    }
-
-    // Ensure that the stream now points to the start of the pixel array
-    uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes + colorBytes;
-
-    // Check that we have not read past the pixel array offset
-    if(bytesRead > offset) {
-        // This may occur on OS 2.1 and other old versions where the color
-        // table defaults to max size, and the bmp tries to use a smaller color
-        // table.  This is invalid, and our decision is to indicate an error,
-        // rather than try to guess the intended size of the color table and
-        // rewind the stream to display the image.
-        SkDebugf("Error: pixel data offset less than header size.\n");
-        return NULL;
-    }
-
-    // Skip to the start of the pixel array
-    if (stream->skip(offset - bytesRead) != offset - bytesRead) {
-        SkDebugf("Error: unable to skip to image data.\n");
-        return NULL;
-    }
-
-    // Remaining bytes is only used for RLE
-    const int remainingBytes = totalBytes - offset;
-    if (remainingBytes <= 0 && kRLE_BitmapInputFormat == inputFormat) {
-        SkDebugf("Error: RLE requires valid input size.\n");
-        return NULL;
-    }
+    // Calculate the number of bytes read so far
+    const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
 
     // Return the codec
     // We will use ImageInfo to store width, height, and alpha type.  We will
-    // choose kN32_SkColorType as the input color type because that is the
-    // expected choice for a destination color type.  In reality, the input
-    // color type has many possible formats.
+    // set color type to kN32_SkColorType because that should be the default
+    // output.
     const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
             kN32_SkColorType, alphaType);
     return SkNEW_ARGS(SkBmpCodec, (imageInfo, stream, bitsPerPixel,
-                                   inputFormat, masks.detach(), colorTable,
-                                   rowOrder, remainingBytes));
+                                   inputFormat, masks.detach(), numColors,
+                                   bytesPerColor, offset, bytesRead,
+                                   rowOrder, RLEBytes));
 }
 
 /*
  *
  * Creates an instance of the decoder
  * Called only by NewFromStream
  *
  */
 SkBmpCodec::SkBmpCodec(const SkImageInfo& info, SkStream* stream,
                        uint16_t bitsPerPixel, BitmapInputFormat inputFormat,
-                       SkMasks* masks, SkPMColor* colorTable,
-                       RowOrder rowOrder,
-                       const uint32_t remainingBytes)
+                       SkMasks* masks, uint32_t numColors,
+                       uint32_t bytesPerColor, uint32_t offset,
+                       uint32_t bytesRead, RowOrder rowOrder, uint32_t RLEBytes)
     : INHERITED(info, stream)
     , fBitsPerPixel(bitsPerPixel)
     , fInputFormat(inputFormat)
     , fMasks(masks)
-    , fColorTable(colorTable)
+    , fColorTable(NULL)
+    , fNumColors(numColors)
+    , fBytesPerColor(bytesPerColor)
+    , fBytesRead(bytesRead)
+    , fOffset(offset)
     , fRowOrder(rowOrder)
-    , fRemainingBytes(remainingBytes)
+    , fRLEBytes(RLEBytes)
 {}
 
 /*
  *
  * Initiates the bitmap decode
  *
  */
 SkCodec::Result SkBmpCodec::onGetPixels(const SkImageInfo& dstInfo,
                                         void* dst, size_t dstRowBytes,
                                         const Options&,
                                         SkPMColor*, int*) {
+    // Check for proper input and output formats
     if (!this->rewindIfNeeded()) {
         return kCouldNotRewind;
     }
     if (dstInfo.dimensions() != this->getOriginalInfo().dimensions()) {
         SkDebugf("Error: scaling not supported.\n");
         return kInvalidScale;
     }
     if (!conversion_possible(dstInfo, this->getOriginalInfo())) {
         SkDebugf("Error: cannot convert input type to output type.\n");
         return kInvalidConversion;
     }
 
+    // Create the color table if necessary and prepare the stream for decode
+    if (!createColorTable(dstInfo.alphaType())) {
+        SkDebugf("Error: could not create color table.\n");
+        return kInvalidInput;
+    }
+
+    // Perform the decode
     switch (fInputFormat) {
         case kBitMask_BitmapInputFormat:
             return decodeMask(dstInfo, dst, dstRowBytes);
         case kRLE_BitmapInputFormat:
             return decodeRLE(dstInfo, dst, dstRowBytes);
         case kStandard_BitmapInputFormat:
             return decode(dstInfo, dst, dstRowBytes);
         default:
             SkASSERT(false);
             return kInvalidInput;
     }
 }
 
 /*
  *
+ * Process the color table for the bmp input
+ *
+ */
+ bool SkBmpCodec::createColorTable(SkAlphaType alphaType) {
+    SkAutoTDeleteArray<SkPMColor> colorTable(NULL);
+    uint32_t colorBytes = 0;
+    uint32_t maxColors = 0;
+    if (fBitsPerPixel < 16) {
+        // Allocate memory for a color table
+        maxColors = 1 << fBitsPerPixel;
+        colorTable.reset(SkNEW_ARRAY(SkPMColor, maxColors));
+
+        // Read the color table from the stream
+        colorBytes = fNumColors * fBytesPerColor;
+        SkAutoTDeleteArray<uint8_t> cBuffer(SkNEW_ARRAY(uint8_t, colorBytes));
+        if (stream()->read(cBuffer.get(), colorBytes) != colorBytes) {
+            SkDebugf("Error: unable to read color table.\n");
+            return NULL;
+        }
+
+        // Fill in the color table
+        uint32_t i = 0;
+        for (; i < fNumColors; i++) {
+            uint8_t blue = get_byte(cBuffer.get(), i*fBytesPerColor);
+            uint8_t green = get_byte(cBuffer.get(), i*fBytesPerColor + 1);
+            uint8_t red = get_byte(cBuffer.get(), i*fBytesPerColor + 2);
+            uint8_t alpha;
+            switch (alphaType) {
+                case kOpaque_SkAlphaType:
+                    colorTable.get()[i] = SkPackARGB32NoCheck(0xFF, red, green,
+                            blue);
+                    break;
+                case kUnpremul_SkAlphaType:
+                    alpha = (fMasks->getAlphaMask() >> 24) &
+                            get_byte(cBuffer.get(), i*fBytesPerColor + 3);
+                    colorTable.get()[i] = SkPackARGB32NoCheck(alpha, red, green,
+                            blue);
+                    break;
+                case kPremul_SkAlphaType:
+                    alpha = (fMasks->getAlphaMask() >> 24) &
+                            get_byte(cBuffer.get(), i*fBytesPerColor + 3);
+                    colorTable.get()[i] = SkPreMultiplyARGB(alpha, red, green,
+                            blue);
+                    break;
+                default:
+                    // This should not be reached because conversion possible
+                    // should fail if the alpha type is not one of the above
+                    // values.
+                    SkASSERT(false);
+                    break;
+            }
+        }
+
+        // To avoid segmentation faults on bad pixel data, fill the end of the
+        // color table with black.  This is the same the behavior as the
+        // chromium decoder.
+        for (; i < maxColors; i++) {
+            colorTable.get()[i] = SkPackARGB32NoCheck(0xFF, 0, 0, 0);
+        }
+    }
+
+    // Calculate the number of bytes read so far (including the color table)
+    fBytesRead += colorBytes;
+
+    // Check that we have not read past the pixel array offset
+    if(fBytesRead > fOffset) {
+        // This may occur on OS 2.1 and other old versions where the color
+        // table defaults to max size, and the bmp tries to use a smaller color
+        // table.  This is invalid, and our decision is to indicate an error,
+        // rather than try to guess the intended size of the color table.
+        SkDebugf("Error: pixel data offset less than header size.\n");
+        return NULL;
+    }
+
+    // After reading the color table, skip to the start of the pixel array
+    if (stream()->skip(fOffset - fBytesRead) != fOffset - fBytesRead) {
+        SkDebugf("Error: unable to skip to image data.\n");
+        return false;
+    }
+
+    // Set the color table and return true on success
+    fColorTable.reset(
+            SkNEW_ARGS(SkColorTable, (colorTable.detach(), maxColors)));

[scroggo, 2015/03/17 20:02:58]

Unfortunately, this constructor doesn't do what you think it does. (Even worse,
it's not documented in any way...). Instead of taking ownership, it makes a copy
of the passed in colors.

Take a look at what SkPngCodec does - it stack allocates 256 colors and then
copies them into a new SkColorTable.

[msarett, 2015/03/18 13:02:52]

Yeah I had no idea.  This should be better.

+    return true;
+}
+
+/*
+ *
  * Performs the bitmap decoding for bit masks input format
  *
  */
 SkCodec::Result SkBmpCodec::decodeMask(const SkImageInfo& dstInfo,
                                        void* dst, size_t dstRowBytes) {
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
     const size_t rowBytes = SkAlign4(compute_row_bytes(width, fBitsPerPixel));
 
-    // Allocate space for a row buffer and a source for the swizzler
-    SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, rowBytes));
+    // Allocate a buffer large enough to hold the full input stream
+    SkAutoTDeleteArray<uint8_t>
+        srcBuffer(SkNEW_ARRAY(uint8_t, height*rowBytes));
+    uint8_t* srcRow = srcBuffer.get();
 
     // Get the destination start row and delta
+    SkPMColor* dstStart;
     SkPMColor* dstRow;
     int delta;
     if (kTopDown_RowOrder == fRowOrder) {
-        dstRow = (SkPMColor*) dst;
+        dstStart = (SkPMColor*) dst;
+        dstRow = dstStart;
         delta = (int) dstRowBytes;
     } else {
-        dstRow = (SkPMColor*) SkTAddOffset<void>(dst, (height-1) * dstRowBytes);
+        dstStart = (SkPMColor*) SkTAddOffset<void>(
+                dst, (height - 1) * dstRowBytes);
+        dstRow = dstStart;
         delta = -((int) dstRowBytes);
     }
 
     // Create the swizzler
-    SkMaskSwizzler* swizzler = SkMaskSwizzler::CreateMaskSwizzler(
-            dstInfo, fMasks, fBitsPerPixel);
+    SkAutoTDelete<SkMaskSwizzler> maskSwizzler(
+            SkMaskSwizzler::CreateMaskSwizzler(dstInfo, fMasks, fBitsPerPixel));
 
     // Iterate over rows of the image
     bool transparent = true;
     for (int y = 0; y < height; y++) {
         // Read a row of the input
-        if (stream()->read(srcBuffer.get(), rowBytes) != rowBytes) {
+        if (stream()->read(srcRow, rowBytes) != rowBytes) {
             SkDebugf("Warning: incomplete input stream.\n");
             return kIncompleteInput;
         }
 
         // Decode the row in destination format
-        SkSwizzler::ResultAlpha r = swizzler->next(dstRow, srcBuffer.get());
+        SkSwizzler::ResultAlpha r = maskSwizzler->next(dstRow, srcRow);
         transparent &= SkSwizzler::IsTransparent(r);
 
         // Move to the next row
         dstRow = SkTAddOffset<SkPMColor>(dstRow, delta);
+        srcRow = SkTAddOffset<uint8_t>(srcRow, rowBytes);
     }
 
     // Some fully transparent bmp images are intended to be opaque.  Here, we
     // correct for this possibility.
-    dstRow = (SkPMColor*) dst;
     if (transparent) {
+        const SkImageInfo& opaqueInfo =
+                dstInfo.makeAlphaType(kOpaque_SkAlphaType);
+        SkAutoTDelete<SkMaskSwizzler> opaqueSwizzler(
+                SkMaskSwizzler::CreateMaskSwizzler(opaqueInfo, fMasks,
+                                                   fBitsPerPixel));
+        srcRow = srcBuffer.get();
+        dstRow = dstStart;
         for (int y = 0; y < height; y++) {
-            for (int x = 0; x < width; x++) {
-                dstRow[x] |= 0xFF000000;
-            }
-            dstRow = SkTAddOffset<SkPMColor>(dstRow, dstRowBytes);
+            // Decode the row in new format
+            opaqueSwizzler->next(dstRow, srcRow);
+
+            // Move to the next row
+            dstRow = SkTAddOffset<SkPMColor>(dstRow, delta);
+            srcRow = SkTAddOffset<uint8_t>(srcRow, rowBytes);
         }
     }
 
     // Finished decoding the entire image
     return kSuccess;
 }
 
 /*
  *
  * Set an RLE pixel using the color table
  *
  */
-void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes, int height,
-                             uint32_t x, uint32_t y, uint8_t index) {
+void SkBmpCodec::setRLEPixel(SkPMColor* dst, size_t dstRowBytes,
+                             const SkImageInfo& dstInfo, uint32_t x, uint32_t y,
+                             uint8_t index) {
+    // Set the row
+    int height = dstInfo.height();
+    int row;
     if (kBottomUp_RowOrder == fRowOrder) {
-        y = height - y - 1;
+        row = height - y - 1;
+    } else {
+        row = y;
     }
-    SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst, y * dstRowBytes);
-    dstRow[x] = fColorTable.get()[index];
+
+    // Set the pixel based on destination color type
+    switch (dstInfo.colorType()) {
+        case kN32_SkColorType: {
+            SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst,
+                    row * (int) dstRowBytes);
+            dstRow[x] = fColorTable.get()[0][index];

[scroggo, 2015/03/17 20:02:58]

Why do you need [0]?

Does this get around the fact that get() returns a pointer, which is then
treated as an array?

As an alternative (maybe equally ugly...), I think you can use

  fColorTable->operator[](index)

[msarett, 2015/03/18 13:02:52]

Yeah I think that's a little better.

I kept going back and forth between [0] and (*(...)) both of which I didn't
like.

[scroggo, 2015/03/18 13:33:20]

Haha, yeah, it's an unfortunate bit of ambiguity that I wish had a better fix.

+            break;
+        }
+        case kRGB_565_SkColorType: {
+            uint16_t* dstRow = SkTAddOffset<uint16_t>(dst,
+                    row * (int) dstRowBytes);
+            dstRow[x] = SkPixel32ToPixel16(fColorTable.get()[0][index]);
+            break;
+        }
+        default:
+            // This case should not be reached.  We should catch an invalid
+            // color type when we check that the conversion is possible.
+            SkASSERT(false);
+            break;
+    }
 }
 
 /*
+ *
+ * Set an RLE pixel from R, G, B values
+ *
+ */
+void SkBmpCodec::setRLE24Pixel(SkPMColor* dst, size_t dstRowBytes,
+                               const SkImageInfo& dstInfo, uint32_t x,
+                               uint32_t y, uint8_t red, uint8_t green,
+                               uint8_t blue) {
+    // Set the row
+    int height = dstInfo.height();
+    int row;
+    if (kBottomUp_RowOrder == fRowOrder) {
+        row = height - y - 1;
+    } else {
+        row = y;
+    }
+
+    // Set the pixel based on destination color type
+    switch (dstInfo.colorType()) {
+        case kN32_SkColorType: {
+            SkPMColor* dstRow = SkTAddOffset<SkPMColor>(dst,
+                    row * (int) dstRowBytes);
+            dstRow[x] = SkPackARGB32NoCheck(0xFF, red, green, blue);
+            break;
+        }
+        case kRGB_565_SkColorType: {
+            uint16_t* dstRow = SkTAddOffset<uint16_t>(dst,
+                    row * (int) dstRowBytes);
+            dstRow[x] = SkPack888ToRGB16(red, green, blue);
+            break;
+        }
+        default:
+            // This case should not be reached.  We should catch an invalid
+            // color type when we check that the conversion is possible.
+            SkASSERT(false);
+            break;
+    }
+}
+
+/*
  *
  * Performs the bitmap decoding for RLE input format
  * RLE decoding is performed all at once, rather than a one row at a time
  *
  */
 SkCodec::Result SkBmpCodec::decodeRLE(const SkImageInfo& dstInfo,
                                       void* dst, size_t dstRowBytes) {
     // Set RLE flags
     static const uint8_t RLE_ESCAPE = 0;
     static const uint8_t RLE_EOL = 0;
     static const uint8_t RLE_EOF = 1;
     static const uint8_t RLE_DELTA = 2;
 
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
 
     // Input buffer parameters
     uint32_t currByte = 0;
-    SkAutoTDeleteArray<uint8_t> buffer(SkNEW_ARRAY(uint8_t, fRemainingBytes));
-    size_t totalBytes = stream()->read(buffer.get(), fRemainingBytes);
-    if ((uint32_t) totalBytes < fRemainingBytes) {
+    SkAutoTDeleteArray<uint8_t> buffer(SkNEW_ARRAY(uint8_t, fRLEBytes));
+    size_t totalBytes = stream()->read(buffer.get(), fRLEBytes);
+    if (totalBytes < fRLEBytes) {
         SkDebugf("Warning: incomplete RLE file.\n");
     } else if (totalBytes <= 0) {
         SkDebugf("Error: could not read RLE image data.\n");
         return kInvalidInput;
     }
 
     // Destination parameters
     int x = 0;
     int y = 0;
     // If the code skips pixels, remaining pixels are transparent or black
@@ skipping 58 matching lines @@
                             fBitsPerPixel);
                     // Abort if setting numPixels moves us off the edge of the
                     // image.  Also abort if there are not enough bytes
                     // remaining in the stream to set numPixels.
                     if (x + numPixels > width ||
                             (int) totalBytes - currByte < SkAlign2(rowBytes)) {
                         SkDebugf("Warning: invalid RLE input.\n");
                         return kIncompleteInput;
                     }
                     // Set numPixels number of pixels
-                    SkPMColor* dstRow = SkTAddOffset<SkPMColor>(
-                            dstPtr, y * dstRowBytes);
                     while (numPixels > 0) {
                         switch(fBitsPerPixel) {
                             case 4: {
                                 SkASSERT(currByte < totalBytes);
                                 uint8_t val = buffer.get()[currByte++];
-                                setRLEPixel(dstPtr, dstRowBytes, height, x++, y,
-                                        val >> 4);
+                                setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++,
+                                        y, val >> 4);
                                 numPixels--;
                                 if (numPixels != 0) {
-                                    setRLEPixel(dstPtr, dstRowBytes, height,
+                                    setRLEPixel(dstPtr, dstRowBytes, dstInfo,
                                             x++, y, val & 0xF);
                                     numPixels--;
                                 }
                                 break;
                             }
                             case 8:
                                 SkASSERT(currByte < totalBytes);
-                                setRLEPixel(dstPtr, dstRowBytes, height, x++, y,
-                                        buffer.get()[currByte++]);
+                                setRLEPixel(dstPtr, dstRowBytes, dstInfo, x++,
+                                        y, buffer.get()[currByte++]);
                                 numPixels--;
                                 break;
                             case 24: {
                                 SkASSERT(currByte + 2 < totalBytes);
                                 uint8_t blue = buffer.get()[currByte++];
                                 uint8_t green = buffer.get()[currByte++];
                                 uint8_t red = buffer.get()[currByte++];
-                                SkPMColor color = SkPackARGB32NoCheck(
-                                        0xFF, red, green, blue);
-                                dstRow[x++] = color;
+                                setRLE24Pixel(dstPtr, dstRowBytes, dstInfo,
+                                            x++, y, red, green, blue);
                                 numPixels--;
                             }
                             default:
                                 SkASSERT(false);
                                 return kInvalidInput;
                         }
                     }
                     // Skip a byte if necessary to maintain alignment
                     if (!SkIsAlign2(rowBytes)) {
                         currByte++;
@@ skipping 13 matching lines @@
                 // color.
                 if ((int) totalBytes - currByte < 2) {
                     SkDebugf("Warning: incomplete RLE input\n");
                     return kIncompleteInput;
                 }
 
                 // Fill the pixels up to endX with the specified color
                 uint8_t blue = task;
                 uint8_t green = buffer.get()[currByte++];
                 uint8_t red = buffer.get()[currByte++];
-                SkPMColor color = SkPackARGB32NoCheck(0xFF, red, green, blue);
-                SkPMColor* dstRow =
-                        SkTAddOffset<SkPMColor>(dstPtr, y * dstRowBytes);
                 while (x < endX) {
-                    dstRow[x++] = color;
+                    setRLE24Pixel(dstPtr, dstRowBytes, dstInfo, x++, y, red,
+                            green, blue);
                 }
             } else {
                 // In RLE8 or RLE4, the second byte read gives the index in the
                 // color table to look up the pixel color.
                 // RLE8 has one color index that gets repeated
                 // RLE4 has two color indexes in the upper and lower 4 bits of
                 // the bytes, which are alternated
                 uint8_t indices[2] = { task, task };
                 if (4 == fBitsPerPixel) {
                     indices[0] >>= 4;
                     indices[1] &= 0xf;
                 }
 
                 // Set the indicated number of pixels
                 for (int which = 0; x < endX; x++) {
-                    setRLEPixel(dstPtr, dstRowBytes, height, x, y,
+                    setRLEPixel(dstPtr, dstRowBytes, dstInfo, x, y,
                             indices[which]);
                     which = !which;
                 }
             }
         }
     }
 }
 
 /*
  *
  * Performs the bitmap decoding for standard input format
  *
  */
 SkCodec::Result SkBmpCodec::decode(const SkImageInfo& dstInfo,
                                    void* dst, size_t dstRowBytes) {
     // Set constant values
     const int width = dstInfo.width();
     const int height = dstInfo.height();
     const size_t rowBytes = SkAlign4(compute_row_bytes(width, fBitsPerPixel));
-    const uint32_t alphaMask = fMasks->getAlphaMask();
 
     // Get swizzler configuration
     SkSwizzler::SrcConfig config;
     switch (fBitsPerPixel) {
         case 1:
             config = SkSwizzler::kIndex1;
             break;
         case 2:
             config = SkSwizzler::kIndex2;
             break;
         case 4:
             config = SkSwizzler::kIndex4;
             break;
         case 8:
             config = SkSwizzler::kIndex;
             break;
         case 24:
             config = SkSwizzler::kBGR;
             break;
         case 32:
-            if (0 == alphaMask) {
+            if (kOpaque_SkAlphaType == dstInfo.alphaType()) {
                 config = SkSwizzler::kBGRX;
             } else {
                 config = SkSwizzler::kBGRA;
             }
             break;
         default:
             SkASSERT(false);
             return kInvalidInput;
     }
 
     // Create swizzler
-    SkSwizzler* swizzler = SkSwizzler::CreateSwizzler(config, fColorTable.get(),
-            dstInfo, dst, dstRowBytes, SkImageGenerator::kNo_ZeroInitialized);
+    SkAutoTDelete<SkSwizzler> swizzler(SkSwizzler::CreateSwizzler(config,
+            fColorTable.get()->readColors(), dstInfo, dst, dstRowBytes,

[scroggo, 2015/03/17 20:02:58]

SkAutoTDelete implements operator->, so you can drop the .get() here, and treat
it as if it were a raw pointer (that's what allows the ->operator[]() trick I
mentioned above).

[msarett, 2015/03/18 13:02:52]

Done.

+            SkImageGenerator::kNo_ZeroInitialized));
 
     // Allocate space for a row buffer and a source for the swizzler
     SkAutoTDeleteArray<uint8_t> srcBuffer(SkNEW_ARRAY(uint8_t, rowBytes));
 
     // Iterate over rows of the image
     // FIXME: bool transparent = true;
     for (int y = 0; y < height; y++) {
         // Read a row of the input
         if (stream()->read(srcBuffer.get(), rowBytes) != rowBytes) {
             SkDebugf("Warning: incomplete input stream.\n");
@@ skipping 36 matching lines @@
                 }
                 dstRow = SkTAddOffset<SkPMColor>(dstRow, dstRowBytes);
             }
         }
     }
     */
 
     // Finished decoding the entire image
     return kSuccess;
 }