Image decoder fixes (mostly) around A8.
Opaquness:
When decoding Gray to A8 in libpng, set reallyHasAlpha to true
and add a comment why we do not check for the real answer.
Add comments in jpeg decoder explaining why A8 is not opaque.
Fix a bug where an A8 subset is considered to be opaque.
Other fixes:
In SkJPEGImageDecoder, only allocate as much memory as
needed for each source row, based on the input config.
Also pull out common code into a static function.
When performing the check for requiring unpremultiplied colors,
allow A8 to succeed, since that setting should have no effect on
A8.
Add the check for requiring unpremultiplied colors to subset
decoding.
Fix a bug where attempting to sample gray to A8 does not sample.
R=reed@google.com
Review URL: https://codereview.chromium.org/26210007
git-svn-id: http://skia.googlecode.com/svn/trunk/src@11897 2bbb7eff-a529-9590-31e7-b0007b416f81
diff --git a/images/SkImageDecoder_libjpeg.cpp b/images/SkImageDecoder_libjpeg.cpp
index 67b1782..b08835b 100644
--- a/images/SkImageDecoder_libjpeg.cpp
+++ b/images/SkImageDecoder_libjpeg.cpp
@@ -495,6 +495,39 @@
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
@@ -543,6 +576,10 @@
#endif
if (1 == sampleSize && SkImageDecoder::kDecodeBounds_Mode == mode) {
+ // Assume an A8 bitmap is not 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.
return bm->setConfig(config, cinfo.image_width, cinfo.image_height, 0,
SkBitmap::kA8_Config == config ?
kPremul_SkAlphaType : kOpaque_SkAlphaType);
@@ -565,6 +602,10 @@
if (SkImageDecoder::kDecodeBounds_Mode == mode && valid_output_dimensions(cinfo)) {
SkScaledBitmapSampler smpl(cinfo.output_width, cinfo.output_height,
recompute_sampleSize(sampleSize, cinfo));
+ // Assume an A8 bitmap is not 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.
return bm->setConfig(config, smpl.scaledWidth(), smpl.scaledHeight(),
0, SkBitmap::kA8_Config == config ?
kPremul_SkAlphaType : kOpaque_SkAlphaType);
@@ -580,6 +621,10 @@
}
SkScaledBitmapSampler sampler(cinfo.output_width, cinfo.output_height, sampleSize);
+ // Assume an A8 bitmap is not 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->setConfig(config, sampler.scaledWidth(), sampler.scaledHeight(), 0,
SkBitmap::kA8_Config != config ? kOpaque_SkAlphaType : kPremul_SkAlphaType);
if (SkImageDecoder::kDecodeBounds_Mode == mode) {
@@ -625,21 +670,9 @@
// 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");
}
@@ -647,8 +680,7 @@
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]
@@ -801,7 +833,13 @@
SkScaledBitmapSampler sampler(width, height, skiaSampleSize);
SkBitmap bitmap;
+ bitmap.setConfig(config, sampler.scaledWidth(), sampler.scaledHeight());
+ // Assume an A8 bitmap is not 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.setConfig(config, sampler.scaledWidth(), sampler.scaledHeight(), 0,
+ config == SkBitmap::kA8_Config ? kPremul_SkAlphaType :
kOpaque_SkAlphaType);
// Check ahead of time if the swap(dest, src) is possible or not.
@@ -869,21 +907,9 @@
// 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");
}
@@ -891,8 +917,7 @@
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]
diff --git a/images/SkImageDecoder_libpng.cpp b/images/SkImageDecoder_libpng.cpp
index e54387a..4047fea 100644
--- a/images/SkImageDecoder_libpng.cpp
+++ b/images/SkImageDecoder_libpng.cpp
@@ -372,9 +372,14 @@
if ((SkBitmap::kA8_Config == config || SkBitmap::kIndex8_Config == config)
&& 1 == sampleSize) {
- // A8 is only allowed if the original was GRAY.
- SkASSERT(config != SkBitmap::kA8_Config
- || PNG_COLOR_TYPE_GRAY == colorType);
+ if (SkBitmap::kA8_Config == config) {
+ // For an A8 bitmap, we assume there is an alpha for speed. It is
+ // possible the bitmap is opaque, but that is an unlikely use case
+ // since it would not be very interesting.
+ reallyHasAlpha = true;
+ // A8 is only allowed if the original was GRAY.
+ SkASSERT(PNG_COLOR_TYPE_GRAY == colorType);
+ }
for (int i = 0; i < number_passes; i++) {
for (png_uint_32 y = 0; y < origHeight; y++) {
uint8_t* bmRow = decodedBitmap->getAddr8(0, y);
@@ -456,17 +461,22 @@
if (0 != theTranspColor) {
reallyHasAlpha |= substituteTranspColor(decodedBitmap, theTranspColor);
}
- if (reallyHasAlpha && this->getRequireUnpremultipliedColors() &&
- SkBitmap::kARGB_8888_Config != decodedBitmap->config()) {
- // If the caller wants an unpremultiplied bitmap, and we let them get
- // away with a config other than 8888, and it has alpha after all,
- // return false, since the result will have premultiplied colors.
- return false;
+ if (reallyHasAlpha && this->getRequireUnpremultipliedColors()) {
+ switch (decodedBitmap->config()) {
+ case SkBitmap::kIndex8_Config:
+ // Fall through.
+ case SkBitmap::kARGB_4444_Config:
+ // We have chosen not to support unpremul for these configs.
+ return false;
+ default: {
+ // Fall through to finish the decode. This config either
+ // supports unpremul or it is irrelevant because it has no
+ // alpha (or only alpha).
+ // These brackets prevent a warning.
+ }
+ }
}
- if (SkBitmap::kA8_Config == decodedBitmap->config()) {
- reallyHasAlpha = true;
- }
-
+
SkAlphaType alphaType = kOpaque_SkAlphaType;
if (reallyHasAlpha) {
if (this->getRequireUnpremultipliedColors()) {
@@ -852,9 +862,14 @@
if ((SkBitmap::kA8_Config == config || SkBitmap::kIndex8_Config == config)
&& 1 == sampleSize) {
- // A8 is only allowed if the original was GRAY.
- SkASSERT(config != SkBitmap::kA8_Config
- || PNG_COLOR_TYPE_GRAY == colorType);
+ if (SkBitmap::kA8_Config == config) {
+ // For an A8 bitmap, we assume there is an alpha for speed. It is
+ // possible the bitmap is opaque, but that is an unlikely use case
+ // since it would not be very interesting.
+ reallyHasAlpha = true;
+ // A8 is only allowed if the original was GRAY.
+ SkASSERT(PNG_COLOR_TYPE_GRAY == colorType);
+ }
for (int i = 0; i < number_passes; i++) {
png_configure_decoder(png_ptr, &actualTop, i);
@@ -945,8 +960,20 @@
if (0 != theTranspColor) {
reallyHasAlpha |= substituteTranspColor(&decodedBitmap, theTranspColor);
}
- if (SkBitmap::kA8_Config == decodedBitmap.config()) {
- reallyHasAlpha = true;
+ if (reallyHasAlpha && this->getRequireUnpremultipliedColors()) {
+ switch (decodedBitmap.config()) {
+ case SkBitmap::kIndex8_Config:
+ // Fall through.
+ case SkBitmap::kARGB_4444_Config:
+ // We have chosen not to support unpremul for these configs.
+ return false;
+ default: {
+ // Fall through to finish the decode. This config either
+ // supports unpremul or it is irrelevant because it has no
+ // alpha (or only alpha).
+ // These brackets prevent a warning.
+ }
+ }
}
SkAlphaType alphaType = kOpaque_SkAlphaType;
if (reallyHasAlpha) {
diff --git a/images/SkScaledBitmapSampler.cpp b/images/SkScaledBitmapSampler.cpp
index 825f9d5..03ee2ee 100644
--- a/images/SkScaledBitmapSampler.cpp
+++ b/images/SkScaledBitmapSampler.cpp
@@ -549,7 +549,11 @@
const uint8_t* SK_RESTRICT src,
int width, int deltaSrc, int,
const SkPMColor[]) {
- memcpy(dstRow, src, width);
+ // Sampling Gray to A8 uses the same function as Index to Index8,
+ // except we assume that there is alpha for speed, since an A8
+ // bitmap with no alpha is not interesting.
+ (void) Sample_Index_DI(dstRow, src, width, deltaSrc, /* y unused */ 0,
+ /* ctable unused */ NULL);
return true;
}
