aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar scroggo <scroggo@chromium.org>2016-05-20 13:56:13 -0700
committerGravatar Commit bot <commit-bot@chromium.org>2016-05-20 13:56:13 -0700
commitba5848953e00685f9a57343aef94372c5803130a (patch)
treed649d222b437821786d485abd515a81963c1b265
parent17b4a109219d063dc10a988b1ed46aaefe278477 (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.cpp18
-rw-r--r--src/codec/SkCodecPriv.h7
-rw-r--r--src/codec/SkWbmpCodec.cpp9
-rw-r--r--tests/CodecTest.cpp46
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