From 9a0e3467da2a61ec81f676c147c21c77de91fccf Mon Sep 17 00:00:00 2001 From: msarett Date: Fri, 11 Dec 2015 07:38:50 -0800 Subject: [PATCH] Make BitmapRegionDecoder succeed on invalid requests If the client requests a color type or alpha type that is not supported, we should decode to the default/appropriate color and alpha types to match existing behavior in Android. BUG=skia: Review URL: https://codereview.chromium.org/1513023002 --- include/codec/SkAndroidCodec.h | 18 +++++++++++++ src/android/SkBitmapRegionCanvas.cpp | 3 +-- src/android/SkBitmapRegionCanvas.h | 12 +++++++-- src/android/SkBitmapRegionCodec.cpp | 15 +++++++---- src/codec/SkAndroidCodec.cpp | 39 ++++++++++++++++++++++++++++ 5 files changed, 78 insertions(+), 9 deletions(-) diff --git a/include/codec/SkAndroidCodec.h b/include/codec/SkAndroidCodec.h index 9d02de06f6..4062e64dc5 100644 --- a/include/codec/SkAndroidCodec.h +++ b/include/codec/SkAndroidCodec.h @@ -52,6 +52,24 @@ public: */ SkEncodedFormat getEncodedFormat() const { return fCodec->getEncodedFormat(); } + /** + * @param requestedColorType Color type requested by the client + * + * If it is possible to decode to requestedColorType, this returns + * requestedColorType. Otherwise, this returns whichever color type + * is suggested by the codec as the best match for the encoded data. + */ + SkColorType computeOutputColorType(SkColorType requestedColorType); + + /** + * @param requestedUnpremul Indicates if the client requested + * unpremultiplied output + * + * Returns the appropriate alpha type to decode to. If the image + * has alpha, the value of requestedUnpremul will be honored. + */ + SkAlphaType computeOutputAlphaType(bool requestedUnpremul); + /** * Returns the dimensions of the scaled output image, for an input * sampleSize. diff --git a/src/android/SkBitmapRegionCanvas.cpp b/src/android/SkBitmapRegionCanvas.cpp index bac5dc1ffc..c7c42bd9a2 100644 --- a/src/android/SkBitmapRegionCanvas.cpp +++ b/src/android/SkBitmapRegionCanvas.cpp @@ -18,6 +18,7 @@ SkBitmapRegionCanvas::SkBitmapRegionCanvas(SkCodec* decoder) bool SkBitmapRegionCanvas::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* allocator, const SkIRect& desiredSubset, int sampleSize, SkColorType dstColorType, bool requireUnpremul) { + // Reject color types not supported by this method if (kIndex_8_SkColorType == dstColorType || kGray_8_SkColorType == dstColorType) { SkCodecPrintf("Error: Color type not supported.\n"); @@ -34,8 +35,6 @@ bool SkBitmapRegionCanvas::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* alloca dstAlphaType = kPremul_SkAlphaType; } - // FIXME: Can we add checks and support kIndex8 or unpremultiplied alpha in special cases? - // Fix the input sampleSize if necessary. if (sampleSize < 1) { sampleSize = 1; diff --git a/src/android/SkBitmapRegionCanvas.h b/src/android/SkBitmapRegionCanvas.h index 2edbf1ff3b..c01f96be3c 100644 --- a/src/android/SkBitmapRegionCanvas.h +++ b/src/android/SkBitmapRegionCanvas.h @@ -14,8 +14,16 @@ * an SkCanvas. It uses the scanline decoder to subset the height. It then * will subset the width and scale by drawing to an SkCanvas. */ -// FIXME (msarett): This implementation does not support WEBP, because WEBP -// does not have a scanline decoder. +// FIXME: This class works well as a performance/quality comparison for +// SkBitmapRegionCodec, but it lacks several capabilities that are +// required by BitmapRegionDecoder in Android. +// (1) WEBP decodes - because SkWebpCodec does not have a scanline +// decoder. +// (2) Decodes to kGray8 and kIndex8. +// (3) Decodes to kUnpremul. +// (4) Correcting an invalid dstColorType. For example, if the +// client requests kRGB_565 for a non-opaque image, rather than +// fail, we need to go ahead and decode to kN32. class SkBitmapRegionCanvas : public SkBitmapRegionDecoder { public: diff --git a/src/android/SkBitmapRegionCodec.cpp b/src/android/SkBitmapRegionCodec.cpp index 415b60c5ab..be3d5bcce7 100644 --- a/src/android/SkBitmapRegionCodec.cpp +++ b/src/android/SkBitmapRegionCodec.cpp @@ -17,7 +17,7 @@ SkBitmapRegionCodec::SkBitmapRegionCodec(SkAndroidCodec* codec) {} bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* allocator, - const SkIRect& desiredSubset, int sampleSize, SkColorType dstColorType, + const SkIRect& desiredSubset, int sampleSize, SkColorType prefColorType, bool requireUnpremul) { // Fix the input sampleSize if necessary. @@ -50,10 +50,8 @@ bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* allocat SkISize scaledSize = fCodec->getSampledSubsetDimensions(sampleSize, subset); // Create the image info for the decode - SkAlphaType dstAlphaType = fCodec->getInfo().alphaType(); - if (kOpaque_SkAlphaType != dstAlphaType) { - dstAlphaType = requireUnpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType; - } + SkColorType dstColorType = fCodec->computeOutputColorType(prefColorType); + SkAlphaType dstAlphaType = fCodec->computeOutputAlphaType(requireUnpremul); SkImageInfo decodeInfo = SkImageInfo::Make(scaledSize.width(), scaledSize.height(), dstColorType, dstAlphaType); @@ -94,6 +92,13 @@ bool SkBitmapRegionCodec::decodeRegion(SkBitmap* bitmap, SkBRDAllocator* allocat scaledOutHeight += scaledOutY + scaledExtraY; } SkImageInfo outInfo = decodeInfo.makeWH(scaledOutWidth, scaledOutHeight); + if (kGray_8_SkColorType == dstColorType) { + // The legacy implementations of BitmapFactory and BitmapRegionDecoder + // used kAlpha8 for grayscale images (before kGray8 existed). While + // the codec recognizes kGray8, we need to decode into a kAlpha8 + // bitmap in order to avoid a behavior change. + outInfo = SkImageInfo::MakeA8(scaledOutWidth, scaledOutHeight); + } bitmap->setInfo(outInfo); if (!bitmap->tryAllocPixels(allocator, colorTable.get())) { SkCodecPrintf("Error: Could not allocate pixels.\n"); diff --git a/src/codec/SkAndroidCodec.cpp b/src/codec/SkAndroidCodec.cpp index 5962102a4c..491daf5e68 100644 --- a/src/codec/SkAndroidCodec.cpp +++ b/src/codec/SkAndroidCodec.cpp @@ -50,6 +50,45 @@ SkAndroidCodec* SkAndroidCodec::NewFromData(SkData* data, SkPngChunkReader* chun return NewFromStream(new SkMemoryStream(data), chunkReader); } +SkColorType SkAndroidCodec::computeOutputColorType(SkColorType requestedColorType) { + SkColorType suggestedColorType = this->getInfo().colorType(); + switch (requestedColorType) { + case kARGB_4444_SkColorType: + case kN32_SkColorType: + return kN32_SkColorType; + case kIndex_8_SkColorType: + if (kIndex_8_SkColorType == suggestedColorType) { + return kIndex_8_SkColorType; + } + break; + case kAlpha_8_SkColorType: + // Fall through to kGray_8. Before kGray_8_SkColorType existed, + // we allowed clients to request kAlpha_8 when they wanted a + // grayscale decode. + case kGray_8_SkColorType: + if (kGray_8_SkColorType == suggestedColorType) { + return kGray_8_SkColorType; + } + break; + case kRGB_565_SkColorType: + if (kOpaque_SkAlphaType == this->getInfo().alphaType()) { + return kRGB_565_SkColorType; + } + break; + default: + break; + } + + return suggestedColorType; +} + +SkAlphaType SkAndroidCodec::computeOutputAlphaType(bool requestedUnpremul) { + if (kOpaque_SkAlphaType == this->getInfo().alphaType()) { + return kOpaque_SkAlphaType; + } + return requestedUnpremul ? kUnpremul_SkAlphaType : kPremul_SkAlphaType; +} + SkISize SkAndroidCodec::getSampledDimensions(int sampleSize) const { if (!is_valid_sample_size(sampleSize)) { return SkISize::Make(0, 0);