Allow decoding without color conversion - part 2
Bug: b/135133301
Follow-on to 196f319b.
- Add SkCodec::getICCProfile to match the SkAndroidCodec version.
- Update comments on getPixels() regarding how the SkColorSpace on the
SkImageInfo is treated.
- Add two new images that have ICC profiles that do not map to an
SkColorSpace. Add a test to verify that they have the un-transformed
color we expect.
- Stop uploading ColorCodecSrc images decoded to a null SkColorSpace to
Gold. Though they may be correct, they do not match other images they're
compared against. The new test above verifies that we do not do color
conversion with a null SkColorSpace.
Change-Id: I08635e4262f16500fab32ef97511d305c2c06483
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/269236
Reviewed-by: Derek Sollenberger <djsollen@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt
index a61441c..0b923df 100644
--- a/RELEASE_NOTES.txt
+++ b/RELEASE_NOTES.txt
@@ -23,8 +23,8 @@
Use CTFontManagerCreateFontDescriptorFromData instead of
CGFontCreateWithDataProvider to create CTFonts to avoid memory use issues.
- * Added SkAndroidCodec::getICCProfile for reporting the native ICC profile of
- an encoded image, even if it doesn't map to an SkColorSpace.
+ * Added SkCodec:: and SkAndroidCodec::getICCProfile for reporting the native
+ ICC profile of an encoded image, even if it doesn't map to an SkColorSpace.
* * *
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index 41ae1e4..30a27fe 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -980,7 +980,16 @@
SkImageInfo info = codec->getInfo();
if (fDecodeToDst) {
- info = canvas->imageInfo().makeDimensions(info.dimensions());
+ SkImageInfo canvasInfo = canvas->imageInfo();
+ if (!canvasInfo.colorSpace()) {
+ // This will skip color conversion, and the resulting images will
+ // look different from images they are compared against in Gold, but
+ // that doesn't mean they are wrong. We have a test verifying that
+ // passing a null SkColorSpace skips conversion, so skip this
+ // misleading test.
+ return Error::Nonfatal("Skipping decoding without color transform.");
+ }
+ info = canvasInfo.makeDimensions(info.dimensions());
}
SkBitmap bitmap;
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index 2f9f93c..9578096 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -190,6 +190,9 @@
/**
* Return a reasonable SkImageInfo to decode into.
+ *
+ * If the image has an ICC profile that does not map to an SkColorSpace,
+ * the returned SkImageInfo will use SRGB.
*/
SkImageInfo getInfo() const { return fEncodedInfo.makeImageInfo(); }
@@ -199,6 +202,13 @@
}
/**
+ * Return the ICC profile of the encoded data.
+ */
+ const skcms_ICCProfile* getICCProfile() const {
+ return this->getEncodedInfo().profile();
+ }
+
+ /**
* Returns the image orientation stored in the EXIF data.
* If there is no EXIF data, or if we cannot read the EXIF data, returns kTopLeft.
*/
@@ -344,9 +354,13 @@
*
* If the info contains a non-null SkColorSpace, the codec
* will perform the appropriate color space transformation.
- * If the caller passes in the same color space that was
- * reported by the codec, the color space transformation is
- * a no-op.
+ *
+ * If the caller passes in the SkColorSpace that maps to the
+ * ICC profile reported by getICCProfile(), the color space
+ * transformation is a no-op.
+ *
+ * If the caller passes a null SkColorSpace, no color space
+ * transformation will be done.
*
* If a scanline decode is in progress, scanline mode will end, requiring the client to call
* startScanlineDecode() in order to return to decoding scanlines.
diff --git a/resources/images/cmyk_yellow_224_224_32.jpg b/resources/images/cmyk_yellow_224_224_32.jpg
new file mode 100644
index 0000000..abee66c
--- /dev/null
+++ b/resources/images/cmyk_yellow_224_224_32.jpg
Binary files differ
diff --git a/resources/images/wide_gamut_yellow_224_224_64.jpeg b/resources/images/wide_gamut_yellow_224_224_64.jpeg
new file mode 100644
index 0000000..e28ab06
--- /dev/null
+++ b/resources/images/wide_gamut_yellow_224_224_64.jpeg
Binary files differ
diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp
index 7ec94dd..9897742 100644
--- a/tests/CodecTest.cpp
+++ b/tests/CodecTest.cpp
@@ -1787,3 +1787,47 @@
.makeColorSpace(nullptr);
test_info(r, codec.get(), info, SkCodec::kSuccess, nullptr);
}
+
+// These test images have ICC profiles that do not map to an SkColorSpace.
+// Verify that decoding them with a null destination space does not perform
+// color space transformations.
+DEF_TEST(Codec_noConversion, r) {
+ const struct Rec {
+ const char* name;
+ SkColor color;
+ } recs[] = {
+ { "images/cmyk_yellow_224_224_32.jpg", 0xFFD8FC04 },
+ { "images/wide_gamut_yellow_224_224_64.jpeg",0xFFE0E040 },
+ };
+
+ for (const auto& rec : recs) {
+ auto data = GetResourceAsData(rec.name);
+ if (!data) {
+ continue;
+ }
+
+ auto codec = SkCodec::MakeFromData(std::move(data));
+ if (!codec) {
+ ERRORF(r, "Failed to create a codec from %s", rec.name);
+ continue;
+ }
+
+ const auto* profile = codec->getICCProfile();
+ if (!profile) {
+ ERRORF(r, "Expected %s to have a profile", rec.name);
+ continue;
+ }
+
+ auto cs = SkColorSpace::Make(*profile);
+ REPORTER_ASSERT(r, !cs.get());
+
+ SkImageInfo info = codec->getInfo().makeColorSpace(nullptr);
+ SkBitmap bm;
+ bm.allocPixels(info);
+ if (codec->getPixels(info, bm.getPixels(), bm.rowBytes()) != SkCodec::kSuccess) {
+ ERRORF(r, "Failed to decode %s", rec.name);
+ continue;
+ }
+ REPORTER_ASSERT(r, bm.getColor(0, 0) == rec.color);
+ }
+}
