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() ||
+                    (kPremul_SkAlphaType == dst.alphaType() &&
+                    kUnpremul_SkAlphaType == src.alphaType());
+        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));
+    memset(&inputMasks, 0, sizeof(SkMasks::InputMasks));
 
     // 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 124 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");
+    // Check for a valid number of total bytes when in RLE mode
+    if (totalBytes <= offset && kRLE_BitmapInputFormat == inputFormat) {
+        SkDebugf("Error: RLE requires valid input size.\n");
         return NULL;
     }
+    const size_t RLEBytes = totalBytes - offset;
 
-    // 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");
+    // Calculate the number of bytes read so far
+    const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
+    if (offset < bytesRead) {
+        SkDebugf("Error: pixel data offset less than header size.\n");
         return NULL;
     }
 
     // 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,
+                       RowOrder rowOrder, size_t RLEBytes)
     : INHERITED(info, stream)
     , fBitsPerPixel(bitsPerPixel)
     , fInputFormat(inputFormat)
     , fMasks(masks)
-    , fColorTable(colorTable)
+    , fColorTable(NULL)
+    , fNumColors(numColors)
+    , fBytesPerColor(bytesPerColor)
+    , 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) {
+    SkPMColor* colorTablePtr = NULL;
+    uint32_t colorBytes = 0;
+    uint32_t maxColors = 0;
+    if (fBitsPerPixel <= 8) {
+        // Allocate memory for a color table
+        maxColors = 1 << fBitsPerPixel;
+        SkPMColor colorTable[maxColors];

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

Maybe this works, but I think it would be better to put colorTable in the same
scope as colorTablePtr.

That said, once you move colorTable into the main scope, it looks like you could
just use colorTable the whole time, and never bother with colorTablePtr.

(FWIW, the PngCodec has a separate pointer because it uses pointer arithmetic
instead of indexing, whereas your code uses an index. I'm not sure whether the
original author of the PNG code uses pointer arithmetic as a matter of
preference or for some perceived benefit.)

[msarett, 2015/03/18 14:09:12]

The reason I set it up this wasn't really because of png.  I just wanted to
perform the stack allocation after maxColors was calculated.  That said, the
calculation of maxColors can also be hoisted out of the loop.

Another alternative would be to always allocate space for 256 entries.  I doubt
this would be too bad, but I am trying to avoid it given that many bmps don't
have a color table or use a color table with 1, 2, or 4 bit indices.

[msarett, 2015/03/18 14:14:24]

What I meant to say was "moved out of the if statement", not "hoisted out of the
loop".

[scroggo, 2015/03/18 14:59:32]

I think that would be fine (it's what we do for PNG). This approach is fine,
too, though.

FWIW, the problem with the approach in patch set 6 is that colorTablePtr points
to an array that goes out of scope before you use colorTablePtr, so it's not
guaranteed to still be valid. (It probably will be, though, leading to subtle
bugs that don't reproduce consistently.)

+        colorTablePtr = colorTable;
+
+        // 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[i] = SkPackARGB32NoCheck(0xFF, red, green,
+                            blue);
+                    break;
+                case kUnpremul_SkAlphaType:
+                    alpha = (fMasks->getAlphaMask() >> 24) &
+                            get_byte(cBuffer.get(), i*fBytesPerColor + 3);
+                    colorTable[i] = SkPackARGB32NoCheck(alpha, red, green,
+                            blue);
+                    break;
+                case kPremul_SkAlphaType:
+                    alpha = (fMasks->getAlphaMask() >> 24) &
+                            get_byte(cBuffer.get(), i*fBytesPerColor + 3);
+                    colorTable[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[i] = SkPackARGB32NoCheck(0xFF, 0, 0, 0);
+        }
+    }
+
+    // Check that we have not read past the pixel array offset
+    if(fOffset < colorBytes) {
+        // 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 color table size.\n");
+        return NULL;
+    }
+
+    // After reading the color table, skip to the start of the pixel array
+    if (stream()->skip(fOffset - colorBytes) != fOffset - colorBytes) {
+        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, (colorTablePtr, maxColors)));
+    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->operator[](index);
+            break;
+        }
+        case kRGB_565_SkColorType: {
+            uint16_t* dstRow = SkTAddOffset<uint16_t>(dst,
+                    row * (int) dstRowBytes);
+            dstRow[x] = SkPixel32ToPixel16(fColorTable->operator[](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->readColors(), dstInfo, dst, dstRowBytes,
+            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;
 }