Refactor SkBmpCodec

I started working on indicating the native encoded formats
and things got really complicated for bmp.  I think starting
with this refactor may help a little, and I also think that
this is a good change by itself.

BUG=skia:4133
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1820283002

Review URL: https://codereview.chromium.org/1820283002

src/codec/SkBmpCodec.cpp

@@ -286,6 +286,18 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
     switch (compression) {
         case kNone_BmpCompressionMethod:
             inputFormat = kStandard_BmpInputFormat;
+
+            // 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.
+            if (16 == bitsPerPixel) {
+                inputMasks.red = 0x7C00;
+                inputMasks.green = 0x03E0;
+                inputMasks.blue = 0x001F;
+                inputFormat = kBitMask_BmpInputFormat;
+            }
             break;
         case k8BitRLE_BmpCompressionMethod:
             if (bitsPerPixel != 8) {
@@ -331,6 +343,27 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
                     inputMasks.red = get_int(iBuffer.get(), 36);
                     inputMasks.green = get_int(iBuffer.get(), 40);
                     inputMasks.blue = get_int(iBuffer.get(), 44);
+
+                    if (kInfoV2_BmpHeaderType == headerType ||
+                            (kInfoV3_BmpHeaderType == headerType && !inIco)) {
+                        break;
+                    }
+
+                    // V3+ bmp files introduce an alpha mask and allow the creator of the image
+                    // to use the alpha channels.  However, many of these images leave the
+                    // alpha channel blank and expect to be rendered as opaque.  This is the
+                    // case for almost all V3 images, so we ignore the alpha mask.  For V4+
+                    // images in kMask mode, we will use the alpha mask.  Additionally, V3
+                    // bmp-in-ico expect us to use the alpha mask.
+                    //
+                    // skbug.com/4116: We should perhaps also apply the alpha mask in kStandard
+                    //                 mode.  We just haven't seen any images that expect this
+                    //                 behavior.
+                    //
+                    // Header types are matched based on size.  If the header is
+                    // V3+, we are guaranteed to be able to read at least this size.
+                    SkASSERT(infoBytesRemaining > 52);
+                    inputMasks.alpha = get_int(iBuffer.get(), 48);
                     break;
                 case kOS2VX_BmpHeaderType:
                     // TODO: Decide if we intend to support this.
@@ -366,101 +399,8 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
             SkCodecPrintf("Error: invalid format for bitmap decoding.\n");
             return false;
     }
-
-    // Most versions of bmps should be rendered as opaque.  Either they do
-    // not have an alpha channel, or they expect the alpha channel to be
-    // ignored.  V3+ bmp files introduce an alpha mask and allow the creator
-    // of the image to use the alpha channels.  However, many of these images
-    // leave the alpha channel blank and expect to be rendered as opaque.  This
-    // is the case for almost all V3 images, so we render these as opaque.  For
-    // V4+ images in kMask mode, we will use the alpha mask.
-    //
-    // skbug.com/4116: We should perhaps also apply the alpha mask in kStandard
-    //                 mode.  We just haven't seen any images that expect this
-    //                 behavior.
-    //
-    // Additionally, V3 bmp-in-ico may use the alpha mask.
-    SkAlphaType alphaType = kOpaque_SkAlphaType;
-    if ((kInfoV3_BmpHeaderType == headerType && inIco) ||
-            kInfoV4_BmpHeaderType == headerType ||
-            kInfoV5_BmpHeaderType == headerType) {
-        // Header types are matched based on size.  If the header is
-        // V3+, 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.reset();
 
-    // Additionally, 32 bit bmp-in-icos use the alpha channel.
-    // FIXME (msarett): Don't all bmp-in-icos use the alpha channel?
-    // And, RLE inputs may skip pixels, leaving them as transparent.  This
-    // is uncommon, but we cannot be certain that an RLE bmp will be opaque.
-    if ((inIco && 32 == bitsPerPixel) || (kRLE_BmpInputFormat == inputFormat)) {
-        alphaType = kUnpremul_SkAlphaType;
-    }
-
-    // Check for valid bits per pixel.
-    // At the same time, use this information to choose a suggested color type
-    // and to set default masks.
-    SkColorType colorType = kN32_SkColorType;
-    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_BmpInputFormat != inputFormat) {
-                inputMasks.red = 0x7C00;
-                inputMasks.green = 0x03E0;
-                inputMasks.blue = 0x001F;
-                inputFormat = kBitMask_BmpInputFormat;
-            }
-            break;
-        // We want to decode to kIndex_8 for input formats that are already
-        // designed in index format.
-        case 1:
-        case 2:
-        case 4:
-        case 8:
-            // However, we cannot in RLE format since we may need to leave some
-            // pixels as transparent.  Similarly, we also cannot for ICO images
-            // since we may need to apply a transparent mask.
-            if (kRLE_BmpInputFormat != inputFormat && !inIco) {
-                colorType = kIndex_8_SkColorType;
-            }
-
-            // Mask bmps must have 16, 24, or 32 bits per pixel.
-            if (kBitMask_BmpInputFormat == inputFormat) {
-                SkCodecPrintf("Error: invalid input value of bits per pixel for mask bmp.\n");
-                return false;
-            }
-        case 24:
-        case 32:
-            break;
-        default:
-            SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
-            return false;
-    }
-
-    // Check that input bit masks are valid and create the masks object
-    SkAutoTDelete<SkMasks>
-            masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
-    if (nullptr == masks) {
-        SkCodecPrintf("Error: invalid input masks.\n");
-        return false;
-    }
-
-    // Check for a valid number of total bytes when in RLE mode
-    if (totalBytes <= offset && kRLE_BmpInputFormat == inputFormat) {
-        SkCodecPrintf("Error: RLE requires valid input size.\n");
-        return false;
-    }
-    const size_t RLEBytes = totalBytes - offset;
-
     // Calculate the number of bytes read so far
     const uint32_t bytesRead = kBmpHeaderBytes + infoBytes + maskBytes;
     if (!inIco && offset < bytesRead) {
@@ -471,63 +411,133 @@ bool SkBmpCodec::ReadHeader(SkStream* stream, bool inIco, SkCodec** codecOut) {
         return false;
     }
 
-    // Skip to the start of the pixel array.
-    // We can do this here because there is no color table to read
-    // in bit mask mode.
-    if (!inIco && kBitMask_BmpInputFormat == inputFormat) {
-        if (stream->skip(offset - bytesRead) != offset - bytesRead) {
-            SkCodecPrintf("Error: unable to skip to image data.\n");
-            return false;
-        }
-    }
 
-    if (codecOut) {
-        // BMPs-in-ICOs contain an alpha mask after the image which means we
-        // cannot guarantee that an image is opaque, even if the bmp thinks
-        // it is.
-        bool isOpaque = kOpaque_SkAlphaType == alphaType;
-        if (inIco) {
-            alphaType = kUnpremul_SkAlphaType;
-        }
 
-        // Set the image info
+    switch (inputFormat) {
-        const SkImageInfo& imageInfo = SkImageInfo::Make(width, height,
+        case kStandard_BmpInputFormat: {
-                colorType, alphaType);
+            // BMPs-in-ICOs often contain an alpha mask after the image, which
+            // means we cannot guarantee that an image is opaque, even if the
+            // embedded bmp is opaque.
+            // We use |isOpaque| to indicate if the BMP itself is opaque, but
+            // still need to recommend kUnpremul when it is contained in an ICO.
+            SkColorType colorType = kN32_SkColorType;
+            SkAlphaType alphaType = inIco ? kUnpremul_SkAlphaType : kOpaque_SkAlphaType;
+            bool isOpaque = true;
+            switch (bitsPerPixel) {
+                // Palette formats
+                case 1:
+                case 2:
+                case 4:
+                case 8:
+                    // We cannot recommend a palette color type for ICOs because they
+                    // may contain a transparency mask.
+                    if (!inIco) {
+                        colorType = kIndex_8_SkColorType;
+                    }
+                    break;
+                case 24:
+                case 32:
+                    // 32-bit BMP-in-ICOs actually use the alpha channel in place of a
+                    // transparency mask.
+                    if (inIco) {
+                        isOpaque = false;
+                    }
+                    break;
+                default:
+                    SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
+                    return false;
+            }
 
-        // Return the codec
+            if (codecOut) {
-        switch (inputFormat) {
-            case kStandard_BmpInputFormat:
                 // We require streams to have a memory base for Bmp-in-Ico decodes.
                 SkASSERT(!inIco || nullptr != stream->getMemoryBase());
+
+                // Set the image info and create a codec.
+                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, colorType,
+                        alphaType);
                 *codecOut = new SkBmpStandardCodec(imageInfo, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, isOpaque, inIco);
-                return true;
+
-            case kBitMask_BmpInputFormat:
+            }
-                // Bmp-in-Ico must be standard mode
+            return true;
-                if (inIco) {
+        }
-                    SkCodecPrintf("Error: Icos may not use bit mask format.\n");
+
+        case kBitMask_BmpInputFormat: {
+            // Bmp-in-Ico must be standard mode
+            if (inIco) {
+                SkCodecPrintf("Error: Icos may not use bit mask format.\n");
+                return false;
+            }
+
+            switch (bitsPerPixel) {
+                case 16:
+                case 24:
+                case 32:
+                    break;
+                default:
+                    SkCodecPrintf("Error: invalid input value for bits per pixel.\n");
+                    return false;
+            }
+
+            // Skip to the start of the pixel array.
+            // We can do this here because there is no color table to read
+            // in bit mask mode.
+            if (stream->skip(offset - bytesRead) != offset - bytesRead) {
+                SkCodecPrintf("Error: unable to skip to image data.\n");
+                return false;
+            }
+
+            if (codecOut) {
+                // Check that input bit masks are valid and create the masks object
+                SkAutoTDelete<SkMasks> masks(SkMasks::CreateMasks(inputMasks, bitsPerPixel));
+                if (nullptr == masks) {
+                    SkCodecPrintf("Error: invalid input masks.\n");
                     return false;
                 }
 
+                // Set the image info
+                SkAlphaType alphaType = masks->getAlphaMask() ? kUnpremul_SkAlphaType :
+                        kOpaque_SkAlphaType;
+                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
+                        alphaType);
                 *codecOut = new SkBmpMaskCodec(imageInfo, stream, bitsPerPixel, masks.release(),
                         rowOrder);
-                return true;
+            }
-            case kRLE_BmpInputFormat:
+            return true;
-                // Bmp-in-Ico must be standard mode
+        }
-                // When inIco is true, this line cannot be reached, since we
+
-                // require that RLE Bmps have a valid number of totalBytes, and
+        case kRLE_BmpInputFormat: {
-                // Icos skip the header that contains totalBytes.
+            // We should not reach this point without a valid value of bitsPerPixel.
-                SkASSERT(!inIco);
+            SkASSERT(4 == bitsPerPixel || 8 == bitsPerPixel || 24 == bitsPerPixel);
+
+            // Check for a valid number of total bytes when in RLE mode
+            if (totalBytes <= offset) {
+                SkCodecPrintf("Error: RLE requires valid input size.\n");
+                return false;
+            }
+            const size_t RLEBytes = totalBytes - offset;
+
+            // Bmp-in-Ico must be standard mode
+            // When inIco is true, this line cannot be reached, since we
+            // require that RLE Bmps have a valid number of totalBytes, and
+            // Icos skip the header that contains totalBytes.
+            SkASSERT(!inIco);
+
+            if (codecOut) {
+                // RLE inputs may skip pixels, leaving them as transparent.  This
+                // is uncommon, but we cannot be certain that an RLE bmp will be
+                // opaque.
+                const SkImageInfo imageInfo = SkImageInfo::Make(width, height, kN32_SkColorType,
+                        kUnpremul_SkAlphaType);
                 *codecOut = new SkBmpRLECodec(imageInfo, stream, bitsPerPixel, numColors,
                         bytesPerColor, offset - bytesRead, rowOrder, RLEBytes);
-                return true;
+            }
-            default:
+            return true;
-                SkASSERT(false);
-                return false;
         }
+        default:
+            SkASSERT(false);
+            return false;
     }
-
-    return true;
 }
 
 /*

src/codec/SkBmpStandardCodec.cpp

@@ -228,7 +228,7 @@ int SkBmpStandardCodec::decodeRows(const SkImageInfo& dstInfo, void* dst, size_t
         fSwizzler->swizzle(dstRow, fSrcBuffer.get());
     }
 
-    if (fInIco) {
+    if (fInIco && fIsOpaque) {
         const int startScanline = this->currScanline();
         if (startScanline < 0) {
             // We are not performing a scanline decode.