diff options
author | scroggo <scroggo@chromium.org> | 2016-05-20 13:56:13 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2016-05-20 13:56:13 -0700 |
commit | ba5848953e00685f9a57343aef94372c5803130a (patch) | |
tree | d649d222b437821786d485abd515a81963c1b265 | |
parent | 17b4a109219d063dc10a988b1ed46aaefe278477 (diff) |
Finish supporting decoding opaque to non-opaque
When decoding to 565 or Gray, allow the client to incorrectly ask for premul.
When checking whether it's possible to decode to 565, return whether the
source is opaque.
In DM, allow decoding to 565 or Gray, even if the client also asked for premul.
This fixes a bug introduced in crrev.com/1999593003 when we stopped ever
requesting Opaque, resulting in us not testing 565 or Gray.
BUG=skia:4616
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=1996993003
Review-Url: https://codereview.chromium.org/1996993003
-rw-r--r-- | dm/DMSrcSink.cpp | 18 | ||||
-rw-r--r-- | src/codec/SkCodecPriv.h | 7 | ||||
-rw-r--r-- | src/codec/SkWbmpCodec.cpp | 9 | ||||
-rw-r--r-- | tests/CodecTest.cpp | 46 |
4 files changed, 56 insertions, 24 deletions
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp index 028dff886b..33848975e9 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -314,7 +314,7 @@ static void premultiply_if_necessary(SkBitmap& bitmap) { } static bool get_decode_info(SkImageInfo* decodeInfo, SkColorType canvasColorType, - CodecSrc::DstColorType dstColorType) { + CodecSrc::DstColorType dstColorType, SkAlphaType dstAlphaType) { switch (dstColorType) { case CodecSrc::kIndex8_Always_DstColorType: if (kRGB_565_SkColorType == canvasColorType) { @@ -348,6 +348,7 @@ static bool get_decode_info(SkImageInfo* decodeInfo, SkColorType canvasColorType break; } + *decodeInfo = decodeInfo->makeAlphaType(dstAlphaType); return true; } @@ -373,8 +374,9 @@ Error CodecSrc::draw(SkCanvas* canvas) const { return SkStringPrintf("Couldn't create codec for %s.", fPath.c_str()); } - SkImageInfo decodeInfo = codec->getInfo().makeAlphaType(fDstAlphaType); - if (!get_decode_info(&decodeInfo, canvas->imageInfo().colorType(), fDstColorType)) { + SkImageInfo decodeInfo = codec->getInfo(); + if (!get_decode_info(&decodeInfo, canvas->imageInfo().colorType(), fDstColorType, + fDstAlphaType)) { return Error::Nonfatal("Testing non-565 to 565 is uninteresting."); } @@ -656,8 +658,9 @@ Error AndroidCodecSrc::draw(SkCanvas* canvas) const { return SkStringPrintf("Couldn't create android codec for %s.", fPath.c_str()); } - SkImageInfo decodeInfo = codec->getInfo().makeAlphaType(fDstAlphaType); - if (!get_decode_info(&decodeInfo, canvas->imageInfo().colorType(), fDstColorType)) { + SkImageInfo decodeInfo = codec->getInfo(); + if (!get_decode_info(&decodeInfo, canvas->imageInfo().colorType(), fDstColorType, + fDstAlphaType)) { return Error::Nonfatal("Testing non-565 to 565 is uninteresting."); } @@ -793,11 +796,6 @@ Error ImageGenSrc::draw(SkCanvas* canvas) const { // Test various color and alpha types on CPU SkImageInfo decodeInfo = gen->getInfo().makeAlphaType(fDstAlphaType); - if (kGray_8_SkColorType == decodeInfo.colorType() && - kOpaque_SkAlphaType != decodeInfo.alphaType()) { - return Error::Nonfatal("Avoid requesting non-opaque kGray8 decodes."); - } - int bpp = SkColorTypeBytesPerPixel(decodeInfo.colorType()); size_t rowBytes = decodeInfo.width() * bpp; SkAutoMalloc pixels(decodeInfo.height() * rowBytes); diff --git a/src/codec/SkCodecPriv.h b/src/codec/SkCodecPriv.h index 1694784785..6771ef72b8 100644 --- a/src/codec/SkCodecPriv.h +++ b/src/codec/SkCodecPriv.h @@ -132,12 +132,7 @@ inline bool conversion_possible(const SkImageInfo& dst, const SkImageInfo& src) case kBGRA_8888_SkColorType: return true; case kRGB_565_SkColorType: - return kOpaque_SkAlphaType == dst.alphaType(); - case kGray_8_SkColorType: - if (kOpaque_SkAlphaType != dst.alphaType()) { - return false; - } - // Fall through + return kOpaque_SkAlphaType == src.alphaType(); default: return dst.colorType() == src.colorType(); } diff --git a/src/codec/SkWbmpCodec.cpp b/src/codec/SkWbmpCodec.cpp index 527565f6ab..326fc2206a 100644 --- a/src/codec/SkWbmpCodec.cpp +++ b/src/codec/SkWbmpCodec.cpp @@ -30,15 +30,14 @@ static inline void setup_color_table(SkColorType colorType, } } -static inline bool valid_color_type(SkColorType colorType, SkAlphaType alphaType) { +static inline bool valid_color_type(SkColorType colorType) { switch (colorType) { case kRGBA_8888_SkColorType: case kBGRA_8888_SkColorType: case kIndex_8_SkColorType: - return true; case kGray_8_SkColorType: case kRGB_565_SkColorType: - return kOpaque_SkAlphaType == alphaType; + return true; default: return false; } @@ -128,7 +127,7 @@ SkCodec::Result SkWbmpCodec::onGetPixels(const SkImageInfo& info, return kUnimplemented; } - if (!valid_color_type(info.colorType(), info.alphaType()) || + if (!valid_color_type(info.colorType()) || !valid_alpha(info.alphaType(), this->getInfo().alphaType())) { return kInvalidConversion; } @@ -196,7 +195,7 @@ SkCodec::Result SkWbmpCodec::onStartScanlineDecode(const SkImageInfo& dstInfo, return kUnimplemented; } - if (!valid_color_type(dstInfo.colorType(), dstInfo.alphaType()) || + if (!valid_color_type(dstInfo.colorType()) || !valid_alpha(dstInfo.alphaType(), this->getInfo().alphaType())) { return kInvalidConversion; } diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index 5ac19d9f70..12d6ac8d3d 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -103,9 +103,49 @@ static void test_codec(skiatest::Reporter* r, Codec* codec, SkBitmap& bm, const { // Test decoding to 565 SkImageInfo info565 = info.makeColorType(kRGB_565_SkColorType); - SkCodec::Result expected565 = info.alphaType() == kOpaque_SkAlphaType ? - expectedResult : SkCodec::kInvalidConversion; - test_info(r, codec, info565, expected565, nullptr); + if (info.alphaType() == kOpaque_SkAlphaType) { + // Decoding to 565 should succeed. + SkBitmap bm565; + bm565.allocPixels(info565); + SkAutoLockPixels alp(bm565); + + // This will allow comparison even if the image is incomplete. + bm565.eraseColor(SK_ColorBLACK); + + REPORTER_ASSERT(r, expectedResult == codec->getPixels(info565, + bm565.getPixels(), bm565.rowBytes())); + + SkMD5::Digest digest565; + md5(bm565, &digest565); + + // A dumb client's request for non-opaque should also succeed. + for (auto alpha : { kPremul_SkAlphaType, kUnpremul_SkAlphaType }) { + info565 = info565.makeAlphaType(alpha); + test_info(r, codec, info565, expectedResult, &digest565); + } + } else { + test_info(r, codec, info565, SkCodec::kInvalidConversion, nullptr); + } + } + + if (codec->getInfo().colorType() == kGray_8_SkColorType) { + SkImageInfo grayInfo = codec->getInfo(); + SkBitmap grayBm; + grayBm.allocPixels(grayInfo); + SkAutoLockPixels alp(grayBm); + + grayBm.eraseColor(SK_ColorBLACK); + + REPORTER_ASSERT(r, expectedResult == codec->getPixels(grayInfo, + grayBm.getPixels(), grayBm.rowBytes())); + + SkMD5::Digest grayDigest; + md5(grayBm, &grayDigest); + + for (auto alpha : { kPremul_SkAlphaType, kUnpremul_SkAlphaType }) { + grayInfo = grayInfo.makeAlphaType(alpha); + test_info(r, codec, grayInfo, expectedResult, &grayDigest); + } } // Verify that re-decoding gives the same result. It is interesting to check this after |