Fix bugs in SkBmpCodec
authormsarett <msarett@google.com>
Wed, 3 Feb 2016 21:12:38 +0000 (13:12 -0800)
committerCommit bot <commit-bot@chromium.org>
Wed, 3 Feb 2016 21:12:38 +0000 (13:12 -0800)
The decode should not depend on the requested alpha type.

These were exposed by:
https://codereview.chromium.org/1641273003

This should cause the number of untriaged images in Gold
to go to zero.

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

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

src/codec/SkBmpStandardCodec.cpp
src/codec/SkMaskSwizzler.cpp

index e73d55e..203e7da 100644 (file)
@@ -67,7 +67,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
 /*
  * Process the color table for the bmp input
  */
- bool SkBmpStandardCodec::createColorTable(SkAlphaType alphaType, int* numColors) {
+ bool SkBmpStandardCodec::createColorTable(SkAlphaType dstAlphaType, int* numColors) {
     // Allocate memory for color table
     uint32_t colorBytes = 0;
     SkPMColor colorTable[256];
@@ -94,21 +94,11 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
 
         // Choose the proper packing function
         SkPMColor (*packARGB) (uint32_t, uint32_t, uint32_t, uint32_t);
-        switch (alphaType) {
-            case kOpaque_SkAlphaType:
-            case kUnpremul_SkAlphaType:
-                packARGB = &SkPackARGB32NoCheck;
-                break;
-            case kPremul_SkAlphaType:
-                packARGB = &SkPreMultiplyARGB;
-                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);
-                packARGB = nullptr;
-                break;
+        SkAlphaType encodedAlphaType = this->getInfo().alphaType();
+        if (kOpaque_SkAlphaType == encodedAlphaType || kUnpremul_SkAlphaType == dstAlphaType) {
+            packARGB = &SkPackARGB32NoCheck;
+        } else {
+            packARGB = &SkPremultiplyARGBInline;
         }
 
         // Fill in the color table
@@ -118,7 +108,7 @@ SkCodec::Result SkBmpStandardCodec::onGetPixels(const SkImageInfo& dstInfo,
             uint8_t green = get_byte(cBuffer.get(), i*fBytesPerColor + 1);
             uint8_t red = get_byte(cBuffer.get(), i*fBytesPerColor + 2);
             uint8_t alpha;
-            if (kOpaque_SkAlphaType == alphaType) {
+            if (kOpaque_SkAlphaType == encodedAlphaType) {
                 alpha = 0xFF;
             } else {
                 alpha = get_byte(cBuffer.get(), i*fBytesPerColor + 3);
index 1b77a85..7630a7b 100644 (file)
@@ -235,18 +235,19 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(const SkImageInfo& dstInfo,
         case 16:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
-                    switch (dstInfo.alphaType()) {
-                        case kUnpremul_SkAlphaType:
-                            proc = &swizzle_mask16_to_n32_unpremul;
-                            break;
-                        case kPremul_SkAlphaType:
-                            proc = &swizzle_mask16_to_n32_premul;
-                            break;
-                        case kOpaque_SkAlphaType:
-                            proc = &swizzle_mask16_to_n32_opaque;
-                            break;
-                        default:
-                            break;
+                    if (kOpaque_SkAlphaType == srcInfo.alphaType()) {
+                        proc = &swizzle_mask16_to_n32_opaque;
+                    } else {
+                        switch (dstInfo.alphaType()) {
+                            case kUnpremul_SkAlphaType:
+                                proc = &swizzle_mask16_to_n32_unpremul;
+                                break;
+                            case kPremul_SkAlphaType:
+                                proc = &swizzle_mask16_to_n32_premul;
+                                break;
+                            default:
+                                break;
+                        }
                     }
                     break;
                 case kRGB_565_SkColorType:
@@ -259,18 +260,19 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(const SkImageInfo& dstInfo,
         case 24:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
-                    switch (dstInfo.alphaType()) {
-                        case kUnpremul_SkAlphaType:
-                            proc = &swizzle_mask24_to_n32_unpremul;
-                            break;
-                        case kPremul_SkAlphaType:
-                            proc = &swizzle_mask24_to_n32_premul;
-                            break;
-                        case kOpaque_SkAlphaType:
-                            proc = &swizzle_mask24_to_n32_opaque;
-                            break;
-                        default:
-                            break;
+                    if (kOpaque_SkAlphaType == srcInfo.alphaType()) {
+                        proc = &swizzle_mask24_to_n32_opaque;
+                    } else {
+                        switch (dstInfo.alphaType()) {
+                            case kUnpremul_SkAlphaType:
+                                proc = &swizzle_mask24_to_n32_unpremul;
+                                break;
+                            case kPremul_SkAlphaType:
+                                proc = &swizzle_mask24_to_n32_premul;
+                                break;
+                            default:
+                                break;
+                        }
                     }
                     break;
                 case kRGB_565_SkColorType:
@@ -283,18 +285,19 @@ SkMaskSwizzler* SkMaskSwizzler::CreateMaskSwizzler(const SkImageInfo& dstInfo,
         case 32:
             switch (dstInfo.colorType()) {
                 case kN32_SkColorType:
-                    switch (dstInfo.alphaType()) {
-                        case kUnpremul_SkAlphaType:
-                            proc = &swizzle_mask32_to_n32_unpremul;
-                            break;
-                        case kPremul_SkAlphaType:
-                            proc = &swizzle_mask32_to_n32_premul;
-                            break;
-                        case kOpaque_SkAlphaType:
-                            proc = &swizzle_mask32_to_n32_opaque;
-                            break;
-                        default:
-                            break;
+                    if (kOpaque_SkAlphaType == srcInfo.alphaType()) {
+                        proc = &swizzle_mask32_to_n32_opaque;
+                    } else {
+                        switch (dstInfo.alphaType()) {
+                            case kUnpremul_SkAlphaType:
+                                proc = &swizzle_mask32_to_n32_unpremul;
+                                break;
+                            case kPremul_SkAlphaType:
+                                proc = &swizzle_mask32_to_n32_premul;
+                                break;
+                            default:
+                                break;
+                        }
                     }
                     break;
                 case kRGB_565_SkColorType: