Image decoder fixes (mostly) around A8.

src/images/SkImageDecoder_libjpeg.cpp

Index: src/images/SkImageDecoder_libjpeg.cpp
diff --git a/src/images/SkImageDecoder_libjpeg.cpp b/src/images/SkImageDecoder_libjpeg.cpp
index 3e34a277fd0494a46dfafc7fa8a2364f889c2915..b738a88ed9afefba30006d9901e77c5607619262 100644
--- a/src/images/SkImageDecoder_libjpeg.cpp
+++ b/src/images/SkImageDecoder_libjpeg.cpp
@@ -480,6 +480,39 @@ static void fill_below_level(int y, SkBitmap* bitmap) {
     canvas.drawColor(SK_ColorWHITE);
 }
 
+/**
+ *  Get the config and bytes per pixel of the source data. Return
+ *  whether the data is supported.
+ */
+static bool get_src_config(const jpeg_decompress_struct& cinfo,
+                           SkScaledBitmapSampler::SrcConfig* sc,
+                           int* srcBytesPerPixel) {
+    SkASSERT(sc != NULL && srcBytesPerPixel != NULL);
+    if (JCS_CMYK == cinfo.out_color_space) {
+        // In this case we will manually convert the CMYK values to RGB
+        *sc = SkScaledBitmapSampler::kRGBX;
+        // The CMYK work-around relies on 4 components per pixel here
+        *srcBytesPerPixel = 4;
+    } else if (3 == cinfo.out_color_components && JCS_RGB == cinfo.out_color_space) {
+        *sc = SkScaledBitmapSampler::kRGB;
+        *srcBytesPerPixel = 3;
+#ifdef ANDROID_RGB
+    } else if (JCS_RGBA_8888 == cinfo.out_color_space) {
+        *sc = SkScaledBitmapSampler::kRGBX;
+        *srcBytesPerPixel = 4;
+    } else if (JCS_RGB_565 == cinfo.out_color_space) {
+        *sc = SkScaledBitmapSampler::kRGB_565;
+        *srcBytesPerPixel = 2;
+#endif
+    } else if (1 == cinfo.out_color_components &&
+               JCS_GRAYSCALE == cinfo.out_color_space) {
+        *sc = SkScaledBitmapSampler::kGray;
+        *srcBytesPerPixel = 1;
+    } else {
+        return false;
+    }
+    return true;
+}
 
 bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
 #ifdef TIME_DECODE
@@ -529,6 +562,10 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
 
     if (1 == sampleSize && SkImageDecoder::kDecodeBounds_Mode == mode) {
         bm->setConfig(config, cinfo.image_width, cinfo.image_height);
+        // Assume an A8 bitmap is opaque to avoid the check of each

[reed1, 2013/10/08 16:24:17]

"is not" ?

same question for all copies of this comment block

[scroggo, 2013/10/10 21:46:59]

Done.

+        // individual pixel. It is very unlikely to be opaque, since
+        // an opaque A8 bitmap would not be very interesting.
+        // Otherwise, a jpeg image is opaque.
         bm->setIsOpaque(config != SkBitmap::kA8_Config);
         return true;
     }
@@ -551,6 +588,10 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
             SkScaledBitmapSampler smpl(cinfo.output_width, cinfo.output_height,
                                        recompute_sampleSize(sampleSize, cinfo));
             bm->setConfig(config, smpl.scaledWidth(), smpl.scaledHeight());
+            // Assume an A8 bitmap is opaque to avoid the check of each
+            // individual pixel. It is very unlikely to be opaque, since
+            // an opaque A8 bitmap would not be very interesting.
+            // Otherwise, a jpeg image is opaque.
             bm->setIsOpaque(config != SkBitmap::kA8_Config);
             return true;
         } else {
@@ -566,6 +607,10 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
 
     SkScaledBitmapSampler sampler(cinfo.output_width, cinfo.output_height, sampleSize);
     bm->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight());
+    // Assume an A8 bitmap is opaque to avoid the check of each
+    // individual pixel. It is very unlikely to be opaque, since
+    // an opaque A8 bitmap would not be very interesting.
+    // Otherwise, a jpeg image is opaque.
     bm->setIsOpaque(config != SkBitmap::kA8_Config);
     if (SkImageDecoder::kDecodeBounds_Mode == mode) {
         return true;
@@ -610,21 +655,9 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
 
     // check for supported formats
     SkScaledBitmapSampler::SrcConfig sc;
-    if (JCS_CMYK == cinfo.out_color_space) {
-        // In this case we will manually convert the CMYK values to RGB
-        sc = SkScaledBitmapSampler::kRGBX;
-    } else if (3 == cinfo.out_color_components && JCS_RGB == cinfo.out_color_space) {
-        sc = SkScaledBitmapSampler::kRGB;
-#ifdef ANDROID_RGB
-    } else if (JCS_RGBA_8888 == cinfo.out_color_space) {
-        sc = SkScaledBitmapSampler::kRGBX;
-    } else if (JCS_RGB_565 == cinfo.out_color_space) {
-        sc = SkScaledBitmapSampler::kRGB_565;
-#endif
-    } else if (1 == cinfo.out_color_components &&
-               JCS_GRAYSCALE == cinfo.out_color_space) {
-        sc = SkScaledBitmapSampler::kGray;
-    } else {
+    int srcBytesPerPixel;
+
+    if (!get_src_config(cinfo, &sc, &srcBytesPerPixel)) {
         return return_false(cinfo, *bm, "jpeg colorspace");
     }
 
@@ -632,8 +665,7 @@ bool SkJPEGImageDecoder::onDecode(SkStream* stream, SkBitmap* bm, Mode mode) {
         return return_false(cinfo, *bm, "sampler.begin");
     }
 
-    // The CMYK work-around relies on 4 components per pixel here
-    SkAutoMalloc srcStorage(cinfo.output_width * 4);
+    SkAutoMalloc srcStorage(cinfo.output_width * srcBytesPerPixel);
     uint8_t* srcRow = (uint8_t*)srcStorage.get();
 
     //  Possibly skip initial rows [sampler.srcY0]
@@ -787,7 +819,11 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
 
     SkBitmap bitmap;
     bitmap.setConfig(config, sampler.scaledWidth(), sampler.scaledHeight());
-    bitmap.setIsOpaque(true);
+    // Assume an A8 bitmap is opaque to avoid the check of each
+    // individual pixel. It is very unlikely to be opaque, since
+    // an opaque A8 bitmap would not be very interesting.
+    // Otherwise, a jpeg image is opaque.
+    bitmap.setIsOpaque(config != SkBitmap::kA8_Config);
 
     // Check ahead of time if the swap(dest, src) is possible or not.
     // If yes, then we will stick to AllocPixelRef since it's cheaper with the
@@ -854,21 +890,9 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
 
     // check for supported formats
     SkScaledBitmapSampler::SrcConfig sc;
-    if (JCS_CMYK == cinfo->out_color_space) {
-        // In this case we will manually convert the CMYK values to RGB
-        sc = SkScaledBitmapSampler::kRGBX;
-    } else if (3 == cinfo->out_color_components && JCS_RGB == cinfo->out_color_space) {
-        sc = SkScaledBitmapSampler::kRGB;
-#ifdef ANDROID_RGB
-    } else if (JCS_RGBA_8888 == cinfo->out_color_space) {
-        sc = SkScaledBitmapSampler::kRGBX;
-    } else if (JCS_RGB_565 == cinfo->out_color_space) {
-        sc = SkScaledBitmapSampler::kRGB_565;
-#endif
-    } else if (1 == cinfo->out_color_components &&
-               JCS_GRAYSCALE == cinfo->out_color_space) {
-        sc = SkScaledBitmapSampler::kGray;
-    } else {
+    int srcBytesPerPixel;
+
+    if (!get_src_config(*cinfo, &sc, &srcBytesPerPixel)) {
         return return_false(*cinfo, *bm, "jpeg colorspace");
     }
 
@@ -876,8 +900,7 @@ bool SkJPEGImageDecoder::onDecodeSubset(SkBitmap* bm, const SkIRect& region) {
         return return_false(*cinfo, bitmap, "sampler.begin");
     }
 
-    // The CMYK work-around relies on 4 components per pixel here
-    SkAutoMalloc  srcStorage(width * 4);
+    SkAutoMalloc  srcStorage(width * srcBytesPerPixel);
     uint8_t* srcRow = (uint8_t*)srcStorage.get();
 
     //  Possibly skip initial rows [sampler.srcY0]