Revert "Reland "Switch SkCodec to use skcms" plus fixes"
This reverts commit 49894f450fb155f4f236067109abd9017c85096e.
Reason for revert: Breaking a CTS test on the Android roll:
java.lang.AssertionError: expected:<67043583> but was:<50266367>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:834)
at org.junit.Assert.assertEquals(Assert.java:645)
at org.junit.Assert.assertEquals(Assert.java:631)
at android.graphics.cts.BitmapColorSpaceTest.verifyGetPixel(BitmapColorSpaceTest.java:301)
at android.graphics.cts.BitmapColorSpaceTest.inColorSpaceP3ToSRGB(BitmapColorSpaceTest.java:612)
Expected: 3FF00FF Actual: 2FF00FF
Original change's description:
> Reland "Switch SkCodec to use skcms" plus fixes
>
> This reverts commit 33d5394d08f645a9f612ba96115acb0e8408a97c,
> relanding 81886e8f9461c7049751c6a2c194a1df632dd362 as well as
> "Fix CMYK handling in JPEG codec" (commit
> f8ae5ce20cf6df2dab4149fb2838d9d8dc6bd1af)
>
> Add a test based on the CTS test that failed in the original commit.
> purple-displayprofile.png is the image used in the CTS test, with the
> Android license.
>
> This also adds a fix for SkAndroidCodec, ensuring that we continue to
> use a wide gamut SkColorSpace for images that do not have a numerical
> transfer function and have a wide gamut. This includes a test, with
> wide-gamut.png, which was created with Photoshop and the profile
> "sRGB_Calibrated_Homogeneous.icc" from the skcms tree.
>
> Bug: skia:6839
> Bug: skia:8052
> Bug: skia:8278
>
> TBR=djsollen@google.com
> As with the original, no API change
>
> Change-Id: I4e5bba6a3151f9dc6491e8eda73d4de0535bd692
> Reviewed-on: https://skia-review.googlesource.com/149043
> Commit-Queue: Leon Scroggins <scroggo@google.com>
> Reviewed-by: Brian Osman <brianosman@google.com>
> Reviewed-by: Leon Scroggins <scroggo@google.com>
TBR=djsollen@google.com,mtklein@google.com,scroggo@google.com,brianosman@google.com
Change-Id: Ie71e1fecc26de8225d2fe603765c1e1e0d738634
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: skia:6839, skia:8052, skia:8278
Reviewed-on: https://skia-review.googlesource.com/149262
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/src/codec/SkHeifCodec.cpp b/src/codec/SkHeifCodec.cpp
index f5785d6..31057a0 100644
--- a/src/codec/SkHeifCodec.cpp
+++ b/src/codec/SkHeifCodec.cpp
@@ -133,37 +133,40 @@
return nullptr;
}
- std::unique_ptr<SkEncodedInfo::ICCProfile> profile = nullptr;
- if ((frameInfo.mIccSize > 0) && (frameInfo.mIccData != nullptr)) {
- // FIXME: Would it be possible to use MakeWithoutCopy?
- auto icc = SkData::MakeWithCopy(frameInfo.mIccData.get(), frameInfo.mIccSize);
- profile = SkEncodedInfo::ICCProfile::Make(std::move(icc));
- }
- if (!profile || profile->profile()->data_color_space != skcms_Signature_RGB) {
- profile = SkEncodedInfo::ICCProfile::MakeSRGB();
- }
+ SkEncodedInfo info = SkEncodedInfo::Make(
+ SkEncodedInfo::kYUV_Color, SkEncodedInfo::kOpaque_Alpha, 8);
- SkEncodedInfo info = SkEncodedInfo::Make(frameInfo.mWidth, frameInfo.mHeight,
- SkEncodedInfo::kYUV_Color, SkEncodedInfo::kOpaque_Alpha, 8, std::move(profile));
SkEncodedOrigin orientation = get_orientation(frameInfo);
+ sk_sp<SkColorSpace> colorSpace = nullptr;
+ if ((frameInfo.mIccSize > 0) && (frameInfo.mIccData != nullptr)) {
+ colorSpace = SkColorSpace::MakeICC(frameInfo.mIccData.get(),
+ frameInfo.mIccSize);
+ }
+ if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
+ colorSpace = SkColorSpace::MakeSRGB();
+ }
+
*result = kSuccess;
- return std::unique_ptr<SkCodec>(new SkHeifCodec(std::move(info), heifDecoder.release(),
- orientation));
+ return std::unique_ptr<SkCodec>(new SkHeifCodec(frameInfo.mWidth, frameInfo.mHeight,
+ info, heifDecoder.release(), std::move(colorSpace), orientation));
}
-SkHeifCodec::SkHeifCodec(SkEncodedInfo&& info, HeifDecoder* heifDecoder, SkEncodedOrigin origin)
- : INHERITED(std::move(info), skcms_PixelFormat_RGBA_8888, nullptr, origin)
+SkHeifCodec::SkHeifCodec(int width, int height, const SkEncodedInfo& info,
+ HeifDecoder* heifDecoder, sk_sp<SkColorSpace> colorSpace, SkEncodedOrigin origin)
+ : INHERITED(width, height, info, SkColorSpaceXform::kRGBA_8888_ColorFormat,
+ nullptr, std::move(colorSpace), origin)
, fHeifDecoder(heifDecoder)
, fSwizzleSrcRow(nullptr)
, fColorXformSrcRow(nullptr)
{}
-
-bool SkHeifCodec::conversionSupported(const SkImageInfo& dstInfo, SkColorType /*srcColorType*/,
- bool srcIsOpaque, bool needsColorXform) {
- SkASSERT(srcIsOpaque);
-
+/*
+ * Checks if the conversion between the input image and the requested output
+ * image has been implemented
+ * Sets the output color format
+ */
+bool SkHeifCodec::setOutputColorFormat(const SkImageInfo& dstInfo) {
if (kUnknown_SkAlphaType == dstInfo.alphaType()) {
return false;
}
@@ -181,14 +184,14 @@
return fHeifDecoder->setOutputColor(kHeifColorFormat_BGRA_8888);
case kRGB_565_SkColorType:
- if (needsColorXform) {
+ if (this->colorXform()) {
return fHeifDecoder->setOutputColor(kHeifColorFormat_RGBA_8888);
} else {
return fHeifDecoder->setOutputColor(kHeifColorFormat_RGB565);
}
case kRGBA_F16_SkColorType:
- SkASSERT(needsColorXform);
+ SkASSERT(this->colorXform());
return fHeifDecoder->setOutputColor(kHeifColorFormat_RGBA_8888);
default:
@@ -237,7 +240,7 @@
}
if (this->colorXform()) {
- this->applyColorXform(dst, swizzleDst, dstWidth);
+ this->applyColorXform(dst, swizzleDst, dstWidth, kOpaque_SkAlphaType);
dst = SkTAddOffset<void>(dst, rowBytes);
}
@@ -262,6 +265,11 @@
return kUnimplemented;
}
+ // Check if we can decode to the requested destination and set the output color space
+ if (!this->setOutputColorFormat(dstInfo)) {
+ return kInvalidConversion;
+ }
+
if (!fHeifDecoder->decode(&fFrameInfo)) {
return kInvalidInput;
}
@@ -305,13 +313,15 @@
void SkHeifCodec::initializeSwizzler(
const SkImageInfo& dstInfo, const Options& options) {
+ SkEncodedInfo swizzlerInfo = this->getEncodedInfo();
+
SkImageInfo swizzlerDstInfo = dstInfo;
if (this->colorXform()) {
// The color xform will be expecting RGBA 8888 input.
swizzlerDstInfo = swizzlerDstInfo.makeColorType(kRGBA_8888_SkColorType);
}
- fSwizzler.reset(SkSwizzler::CreateSwizzler(this->getEncodedInfo(), nullptr,
+ fSwizzler.reset(SkSwizzler::CreateSwizzler(swizzlerInfo, nullptr,
swizzlerDstInfo, options, nullptr, true));
SkASSERT(fSwizzler);
}
@@ -329,6 +339,11 @@
SkCodec::Result SkHeifCodec::onStartScanlineDecode(
const SkImageInfo& dstInfo, const Options& options) {
+ // Check if we can decode to the requested destination and set the output color space
+ if (!this->setOutputColorFormat(dstInfo)) {
+ return kInvalidConversion;
+ }
+
// TODO: For now, just decode the whole thing even when there is a subset.
// If the heif image has tiles, we could potentially do this much faster,
// but the tile configuration needs to be retrieved from the metadata.