diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index a61441c084..0b923df13f 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -23,8 +23,8 @@ Milestone 82 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 41ae1e4e57..30a27fe4ac 100644 --- a/dm/DMSrcSink.cpp +++ b/dm/DMSrcSink.cpp @@ -980,7 +980,16 @@ Error ColorCodecSrc::draw(SkCanvas* canvas) const { 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 2f9f93c0e7..9578096fb9 100644 --- a/include/codec/SkCodec.h +++ b/include/codec/SkCodec.h @@ -190,6 +190,9 @@ public: /** * 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(); } @@ -198,6 +201,13 @@ public: return SkIRect::MakeWH(fEncodedInfo.width(), fEncodedInfo.height()); } + /** + * 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 @@ public: * * 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 0000000000..abee66cad2 Binary files /dev/null and b/resources/images/cmyk_yellow_224_224_32.jpg 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 0000000000..e28ab068b6 Binary files /dev/null and b/resources/images/wide_gamut_yellow_224_224_64.jpeg differ diff --git a/tests/CodecTest.cpp b/tests/CodecTest.cpp index 7ec94dd082..9897742a4b 100644 --- a/tests/CodecTest.cpp +++ b/tests/CodecTest.cpp @@ -1787,3 +1787,47 @@ DEF_TEST(Codec_F16_noColorSpace, r) { .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); + } +}