From 595599f46261225dfc67ab4d91d326e099558239 Mon Sep 17 00:00:00 2001 From: Matt Sarett Date: Thu, 15 Dec 2016 13:05:53 -0500 Subject: [PATCH] Rearrange ICC profile parsing None of the small details have changed, just some high level reorganization: (1) Check for XYZ spaces before A2B. (2) If we fail to parse the XYZ space, fallback by trying to parse the A2B space. This should cause no image diffs on Gold. There is an image from the ICC website that is *supposed* to test that we parse the A2B tag before the XYZ tag. Our behavior on this image will actually not change - the XYZ tag is invalid (non-D50 matrix), so we fall back to A2B anyway. I think this behavior is ok. BUG:674584 Change-Id: I271fd990937268e03e98f5037a0837a574e775ef Reviewed-on: https://skia-review.googlesource.com/6143 Reviewed-by: Robert Aftias Commit-Queue: Matt Sarett --- src/core/SkColorSpace_A2B.h | 2 +- src/core/SkColorSpace_Base.h | 4 +- src/core/SkColorSpace_ICC.cpp | 437 ++++++++++++++++++---------------- src/core/SkColorSpace_XYZ.h | 18 +- 4 files changed, 238 insertions(+), 223 deletions(-) diff --git a/src/core/SkColorSpace_A2B.h b/src/core/SkColorSpace_A2B.h index 77f1fa2e43..ed90713a2c 100644 --- a/src/core/SkColorSpace_A2B.h +++ b/src/core/SkColorSpace_A2B.h @@ -166,10 +166,10 @@ public: InputColorFormat inputColorFormat() const { return fInputColorFormat; } -private: SkColorSpace_A2B(InputColorFormat inputColorFormat, std::vector elements, PCS pcs, sk_sp profileData); +private: InputColorFormat fInputColorFormat; std::vector fElements; PCS fPCS; diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h index e1c004e5a1..75a90a6f1b 100644 --- a/src/core/SkColorSpace_Base.h +++ b/src/core/SkColorSpace_Base.h @@ -183,6 +183,8 @@ public: static sk_sp MakeICC(const void* input, size_t len, InputColorFormat inputColorFormat); + static sk_sp MakeRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50); + protected: SkColorSpace_Base(sk_sp profileData); @@ -197,8 +199,6 @@ private: */ sk_sp writeToICC() const; - static sk_sp MakeRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50); - SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44& toXYZ); sk_sp fProfileData; diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp index 2ae1090483..6d7462cfc5 100644 --- a/src/core/SkColorSpace_ICC.cpp +++ b/src/core/SkColorSpace_ICC.cpp @@ -1322,6 +1322,197 @@ static inline bool is_close_to_d50(const SkMatrix44& matrix) { (SkTAbs(Z - kD50_WhitePoint[2]) <= 0.04f); } +static sk_sp make_xyz(const ICCProfileHeader& header, ICCTag* tags, int tagCount, + const uint8_t* base, sk_sp profileData) { + if (kLAB_PCSSpace == header.fPCS) { + return nullptr; + } + + // Recognize the rXYZ, gXYZ, and bXYZ tags. + const ICCTag* r = ICCTag::Find(tags, tagCount, kTAG_rXYZ); + const ICCTag* g = ICCTag::Find(tags, tagCount, kTAG_gXYZ); + const ICCTag* b = ICCTag::Find(tags, tagCount, kTAG_bXYZ); + if (!r || !g || !b) { + return nullptr; + } + + float toXYZ[9]; + if (!load_xyz(&toXYZ[0], r->addr(base), r->fLength) || + !load_xyz(&toXYZ[3], g->addr(base), g->fLength) || + !load_xyz(&toXYZ[6], b->addr(base), b->fLength)) + { + return_null("Need valid rgb tags for XYZ space"); + } + SkMatrix44 mat(SkMatrix44::kUninitialized_Constructor); + mat.set3x3(toXYZ[0], toXYZ[1], toXYZ[2], + toXYZ[3], toXYZ[4], toXYZ[5], + toXYZ[6], toXYZ[7], toXYZ[8]); + if (!is_close_to_d50(mat)) { + return_null("XYZ matrix is not D50"); + } + + // If some, but not all, of the gamma tags are missing, assume that all + // gammas are meant to be the same. + r = ICCTag::Find(tags, tagCount, kTAG_rTRC); + g = ICCTag::Find(tags, tagCount, kTAG_gTRC); + b = ICCTag::Find(tags, tagCount, kTAG_bTRC); + if ((!r || !g || !b)) { + if (!r) { + r = g ? g : b; + } + if (!g) { + g = r ? r : b; + } + if (!b) { + b = r ? r : g; + } + } + + SkGammaNamed gammaNamed = kNonStandard_SkGammaNamed; + sk_sp gammas = nullptr; + size_t tagBytes; + if (r && g && b) { + if (tag_equals(r, g, base) && tag_equals(g, b, base)) { + SkGammas::Data data; + SkColorSpaceTransferFn params; + SkGammas::Type type = + parse_gamma(&data, ¶ms, &tagBytes, r->addr(base), r->fLength); + handle_invalid_gamma(&type, &data); + + if (SkGammas::Type::kNamed_Type == type) { + gammaNamed = data.fNamed; + } else { + size_t allocSize = sizeof(SkGammas); + if (!safe_add(allocSize, gamma_alloc_size(type, data), &allocSize)) { + return_null("SkGammas struct is too large to allocate"); + } + void* memory = sk_malloc_throw(allocSize); + gammas = sk_sp(new (memory) SkGammas(3)); + load_gammas(memory, 0, type, &data, params, r->addr(base)); + + for (int i = 0; i < 3; ++i) { + gammas->fType[i] = type; + gammas->fData[i] = data; + } + } + } else { + SkGammas::Data rData; + SkColorSpaceTransferFn rParams; + SkGammas::Type rType = + parse_gamma(&rData, &rParams, &tagBytes, r->addr(base), r->fLength); + handle_invalid_gamma(&rType, &rData); + + SkGammas::Data gData; + SkColorSpaceTransferFn gParams; + SkGammas::Type gType = + parse_gamma(&gData, &gParams, &tagBytes, g->addr(base), g->fLength); + handle_invalid_gamma(&gType, &gData); + + SkGammas::Data bData; + SkColorSpaceTransferFn bParams; + SkGammas::Type bType = + parse_gamma(&bData, &bParams, &tagBytes, b->addr(base), b->fLength); + handle_invalid_gamma(&bType, &bData); + + size_t allocSize = sizeof(SkGammas); + if (!safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize) || + !safe_add(allocSize, gamma_alloc_size(gType, gData), &allocSize) || + !safe_add(allocSize, gamma_alloc_size(bType, bData), &allocSize)) { + return_null("SkGammas struct is too large to allocate"); + } + void* memory = sk_malloc_throw(allocSize); + gammas = sk_sp(new (memory) SkGammas(3)); + + uint32_t offset = 0; + gammas->fType[0] = rType; + offset += load_gammas(memory, offset, rType, &rData, rParams, + r->addr(base)); + + gammas->fType[1] = gType; + offset += load_gammas(memory, offset, gType, &gData, gParams, + g->addr(base)); + + gammas->fType[2] = bType; + load_gammas(memory, offset, bType, &bData, bParams, b->addr(base)); + + gammas->fData[0] = rData; + gammas->fData[1] = gData; + gammas->fData[2] = bData; + } + } else { + // Guess sRGB if the profile is missing transfer functions. + gammaNamed = kSRGB_SkGammaNamed; + } + + if (kNonStandard_SkGammaNamed == gammaNamed) { + // It's possible that we'll initially detect non-matching gammas, only for + // them to evaluate to the same named gamma curve. + gammaNamed = is_named(gammas); + } + + if (kNonStandard_SkGammaNamed == gammaNamed) { + return sk_sp(new SkColorSpace_XYZ(gammaNamed, + std::move(gammas), + mat, std::move(profileData))); + } + + return SkColorSpace_Base::MakeRGB(gammaNamed, mat); +} + +static sk_sp make_gray(const ICCProfileHeader& header, ICCTag* tags, int tagCount, + const uint8_t* base, sk_sp profileData) { + if (kLAB_PCSSpace == header.fPCS) { + return nullptr; + } + + const ICCTag* grayTRC = ICCTag::Find(tags, tagCount, kTAG_kTRC); + if (!grayTRC) { + return_null("grayTRC tag required for monochrome profiles."); + } + SkGammas::Data data; + SkColorSpaceTransferFn params; + size_t tagBytes; + SkGammas::Type type = + parse_gamma(&data, ¶ms, &tagBytes, grayTRC->addr(base), grayTRC->fLength); + handle_invalid_gamma(&type, &data); + + std::vector elements; + if (SkGammas::Type::kNamed_Type == type) { + elements.push_back(SkColorSpace_A2B::Element(data.fNamed, 1)); + } else { + size_t allocSize = sizeof(SkGammas); + if (!safe_add(allocSize, gamma_alloc_size(type, data), &allocSize)) { + return_null("SkGammas struct is too large to allocate"); + } + void* memory = sk_malloc_throw(allocSize); + sk_sp gammas = sk_sp(new (memory) SkGammas(1)); + load_gammas(memory, 0, type, &data, params, grayTRC->addr(base)); + gammas->fType[0] = type; + gammas->fData[0] = data; + elements.push_back(SkColorSpace_A2B::Element(std::move(gammas))); + } + return sk_sp(new SkColorSpace_A2B(SkColorSpace_Base::InputColorFormat::kGray, + std::move(elements), + SkColorSpace_A2B::PCS::kXYZ, + std::move(profileData))); +} + +static sk_sp make_a2b(SkColorSpace_Base::InputColorFormat inputColorFormat, const ICCProfileHeader& header, ICCTag* tags, int tagCount, const uint8_t* base, sk_sp profileData) { + const ICCTag* a2b0 = ICCTag::Find(tags, tagCount, kTAG_A2B0); + if (a2b0) { + const SkColorSpace_A2B::PCS pcs = kXYZ_PCSSpace == header.fPCS + ? SkColorSpace_A2B::PCS::kXYZ + : SkColorSpace_A2B::PCS::kLAB; + std::vector elements; + if (load_a2b0(&elements, a2b0->addr(base), a2b0->fLength, pcs, inputColorFormat)) { + return sk_sp(new SkColorSpace_A2B(inputColorFormat, std::move(elements), + pcs, std::move(profileData))); + } + } + + return nullptr; +} + sk_sp SkColorSpace::MakeICC(const void* input, size_t len) { return SkColorSpace_Base::MakeICC(input, len, SkColorSpace_Base::InputColorFormat::kRGB); } @@ -1346,26 +1537,6 @@ sk_sp SkColorSpace_Base::MakeICC(const void* input, size_t len, return nullptr; } - switch (inputColorFormat) { - case InputColorFormat::kRGB: - if (header.fInputColorSpace != kRGB_ColorSpace) { - return_null("Provided input color format (RGB) does not match profile.\n"); - } - break; - case InputColorFormat::kCMYK: - if (header.fInputColorSpace != kCMYK_ColorSpace) { - return_null("Provided input color format (CMYK) does not match profile.\n"); - } - break; - case InputColorFormat::kGray: - if (header.fInputColorSpace != kGray_ColorSpace) { - return_null("Provided input color format (Gray) does not match profile.\n"); - } - break; - default: - return_null("Provided input color format not supported"); - } - // Adjust ptr and len before reading the tags. if (len < header.fSize) { SkColorSpacePrintf("ICC profile might be truncated.\n"); @@ -1395,200 +1566,44 @@ sk_sp SkColorSpace_Base::MakeICC(const void* input, size_t len, } } - // Recognize color profile specified by A2B0 tag. - // this must be done before XYZ profile checking, as a profile can have both - // in which case we should use the A2B case to be accurate - // (XYZ is there as a fallback / quick preview) - const ICCTag* a2b0 = ICCTag::Find(tags.get(), tagCount, kTAG_A2B0); - if (a2b0) { - const SkColorSpace_A2B::PCS pcs = kXYZ_PCSSpace == header.fPCS - ? SkColorSpace_A2B::PCS::kXYZ - : SkColorSpace_A2B::PCS::kLAB; - std::vector elements; - if (load_a2b0(&elements, a2b0->addr(base), a2b0->fLength, pcs, inputColorFormat)) { - return sk_sp(new SkColorSpace_A2B(inputColorFormat, std::move(elements), - pcs, std::move(profileData))); + switch (header.fInputColorSpace) { + case kRGB_ColorSpace: { + if (InputColorFormat::kRGB != inputColorFormat) { + return_null("Provided input color format (RGB) does not match profile."); + } + + sk_sp colorSpace = + make_xyz(header, tags.get(), tagCount, base, profileData); + if (colorSpace) { + return colorSpace; + } + + break; } - SkColorSpacePrintf("Ignoring malformed A2B0 tag.\n"); + case kGray_ColorSpace: { + if (InputColorFormat::kGray != inputColorFormat) { + return_null("Provided input color format (Gray) does not match profile."); + } + + sk_sp colorSpace = + make_gray(header, tags.get(), tagCount, base, profileData); + if (colorSpace) { + return colorSpace; + } + + break; + } + case kCMYK_ColorSpace: + if (InputColorFormat::kCMYK != inputColorFormat) { + return_null("Provided input color format (CMYK) does not match profile."); + } + + break; + default: + return_null("ICC profile contains unsupported colorspace"); } - // Lab PCS means the profile is required to be an n-component LUT-based - // profile, so monochrome/3-component matrix-based profiles can only have an XYZ PCS - if (kLAB_PCSSpace == header.fPCS) { - return_null("Invalid PCS. PCSLAB only supported on A2B0 profiles."); - } - SkASSERT(kXYZ_PCSSpace == header.fPCS); - - if (kRGB_ColorSpace == header.fInputColorSpace) { - // Recognize the rXYZ, gXYZ, and bXYZ tags. - const ICCTag* r = ICCTag::Find(tags.get(), tagCount, kTAG_rXYZ); - const ICCTag* g = ICCTag::Find(tags.get(), tagCount, kTAG_gXYZ); - const ICCTag* b = ICCTag::Find(tags.get(), tagCount, kTAG_bXYZ); - if (r && g && b) { - float toXYZ[9]; - if (!load_xyz(&toXYZ[0], r->addr(base), r->fLength) || - !load_xyz(&toXYZ[3], g->addr(base), g->fLength) || - !load_xyz(&toXYZ[6], b->addr(base), b->fLength)) - { - return_null("Need valid rgb tags for XYZ space"); - } - SkMatrix44 mat(SkMatrix44::kUninitialized_Constructor); - mat.set3x3(toXYZ[0], toXYZ[1], toXYZ[2], - toXYZ[3], toXYZ[4], toXYZ[5], - toXYZ[6], toXYZ[7], toXYZ[8]); - if (!is_close_to_d50(mat)) { - // QCMS treats these profiles as "bogus". I'm not sure if that's - // correct, but we certainly do not handle non-D50 matrices - // correctly. So I'll disable this for now. - SkColorSpacePrintf("Matrix is not close to D50"); - return nullptr; - } - - r = ICCTag::Find(tags.get(), tagCount, kTAG_rTRC); - g = ICCTag::Find(tags.get(), tagCount, kTAG_gTRC); - b = ICCTag::Find(tags.get(), tagCount, kTAG_bTRC); - - // If some, but not all, of the gamma tags are missing, assume that all - // gammas are meant to be the same. This behavior is an arbitrary guess, - // but it simplifies the code below. - if ((!r || !g || !b) && (r || g || b)) { - if (!r) { - r = g ? g : b; - } - - if (!g) { - g = r ? r : b; - } - - if (!b) { - b = r ? r : g; - } - } - - SkGammaNamed gammaNamed = kNonStandard_SkGammaNamed; - sk_sp gammas = nullptr; - size_t tagBytes; - if (r && g && b) { - if (tag_equals(r, g, base) && tag_equals(g, b, base)) { - SkGammas::Data data; - SkColorSpaceTransferFn params; - SkGammas::Type type = - parse_gamma(&data, ¶ms, &tagBytes, r->addr(base), r->fLength); - handle_invalid_gamma(&type, &data); - - if (SkGammas::Type::kNamed_Type == type) { - gammaNamed = data.fNamed; - } else { - size_t allocSize = sizeof(SkGammas); - if (!safe_add(allocSize, gamma_alloc_size(type, data), &allocSize)) { - return_null("SkGammas struct is too large to allocate"); - } - void* memory = sk_malloc_throw(allocSize); - gammas = sk_sp(new (memory) SkGammas(3)); - load_gammas(memory, 0, type, &data, params, r->addr(base)); - - for (int i = 0; i < 3; ++i) { - gammas->fType[i] = type; - gammas->fData[i] = data; - } - } - } else { - SkGammas::Data rData; - SkColorSpaceTransferFn rParams; - SkGammas::Type rType = - parse_gamma(&rData, &rParams, &tagBytes, r->addr(base), r->fLength); - handle_invalid_gamma(&rType, &rData); - - SkGammas::Data gData; - SkColorSpaceTransferFn gParams; - SkGammas::Type gType = - parse_gamma(&gData, &gParams, &tagBytes, g->addr(base), g->fLength); - handle_invalid_gamma(&gType, &gData); - - SkGammas::Data bData; - SkColorSpaceTransferFn bParams; - SkGammas::Type bType = - parse_gamma(&bData, &bParams, &tagBytes, b->addr(base), b->fLength); - handle_invalid_gamma(&bType, &bData); - - size_t allocSize = sizeof(SkGammas); - if (!safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize) || - !safe_add(allocSize, gamma_alloc_size(gType, gData), &allocSize) || - !safe_add(allocSize, gamma_alloc_size(bType, bData), &allocSize)) { - return_null("SkGammas struct is too large to allocate"); - } - void* memory = sk_malloc_throw(allocSize); - gammas = sk_sp(new (memory) SkGammas(3)); - - uint32_t offset = 0; - gammas->fType[0] = rType; - offset += load_gammas(memory, offset, rType, &rData, rParams, - r->addr(base)); - - gammas->fType[1] = gType; - offset += load_gammas(memory, offset, gType, &gData, gParams, - g->addr(base)); - - gammas->fType[2] = bType; - load_gammas(memory, offset, bType, &bData, bParams, b->addr(base)); - - gammas->fData[0] = rData; - gammas->fData[1] = gData; - gammas->fData[2] = bData; - } - } else { - // Guess sRGB if the profile is missing transfer functions. - gammaNamed = kSRGB_SkGammaNamed; - } - - if (kNonStandard_SkGammaNamed == gammaNamed) { - // It's possible that we'll initially detect non-matching gammas, only for - // them to evaluate to the same named gamma curve. - gammaNamed = is_named(gammas); - } - - if (kNonStandard_SkGammaNamed == gammaNamed) { - return sk_sp(new SkColorSpace_XYZ(gammaNamed, - std::move(gammas), - mat, std::move(profileData))); - } - - return SkColorSpace_Base::MakeRGB(gammaNamed, mat); - } - } else if (kGray_ColorSpace == header.fInputColorSpace) { - const ICCTag* grayTRC = ICCTag::Find(tags.get(), tagCount, kTAG_kTRC); - if (!grayTRC) { - return_null("grayTRC tag required for monochrome profiles."); - } - SkGammas::Data data; - SkColorSpaceTransferFn params; - size_t tagBytes; - SkGammas::Type type = - parse_gamma(&data, ¶ms, &tagBytes, grayTRC->addr(base), grayTRC->fLength); - handle_invalid_gamma(&type, &data); - - std::vector elements; - if (SkGammas::Type::kNamed_Type == type) { - elements.push_back(SkColorSpace_A2B::Element(data.fNamed, 1)); - } else { - size_t allocSize = sizeof(SkGammas); - if (!safe_add(allocSize, gamma_alloc_size(type, data), &allocSize)) { - return_null("SkGammas struct is too large to allocate"); - } - void* memory = sk_malloc_throw(allocSize); - sk_sp gammas = sk_sp(new (memory) SkGammas(1)); - load_gammas(memory, 0, type, &data, params, grayTRC->addr(base)); - gammas->fType[0] = type; - gammas->fData[0] = data; - elements.push_back(SkColorSpace_A2B::Element(std::move(gammas))); - } - return sk_sp(new SkColorSpace_A2B(inputColorFormat, - std::move(elements), - SkColorSpace_A2B::PCS::kXYZ, - std::move(profileData))); - } - - return_null("ICC profile contains unsupported colorspace"); + return make_a2b(inputColorFormat, header, tags.get(), tagCount, base, profileData); } /////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/SkColorSpace_XYZ.h b/src/core/SkColorSpace_XYZ.h index a179c1a501..4a32188d53 100644 --- a/src/core/SkColorSpace_XYZ.h +++ b/src/core/SkColorSpace_XYZ.h @@ -20,34 +20,34 @@ public: const SkMatrix44* fromXYZD50() const override; bool onGammaCloseToSRGB() const override; - + bool onGammaIsLinear() const override; - + Type type() const override { return Type::kXYZ; } - + sk_sp makeLinearGamma() override; sk_sp makeSRGBGamma() override; - + SkGammaNamed gammaNamed() const { return fGammaNamed; } - + const SkGammas* gammas() const { return fGammas.get(); } - + void toDstGammaTables(const uint8_t* tables[3], sk_sp* storage, int numTables) const; -private: SkColorSpace_XYZ(SkGammaNamed gammaNamed, const SkMatrix44& toXYZ); SkColorSpace_XYZ(SkGammaNamed gammaNamed, sk_sp gammas, const SkMatrix44& toXYZ, sk_sp profileData); +private: const SkGammaNamed fGammaNamed; sk_sp fGammas; const SkMatrix44 fToXYZD50; uint32_t fToXYZD50Hash; - + mutable SkMatrix44 fFromXYZD50; mutable SkOnce fFromXYZOnce; - + mutable sk_sp fDstStorage; mutable const uint8_t* fToDstGammaTables[3]; mutable SkOnce fToDstGammaOnce;