diff options
author | Matt Sarett <msarett@google.com> | 2016-10-31 13:41:57 -0400 |
---|---|---|
committer | Skia Commit-Bot <skia-commit-bot@chromium.org> | 2016-10-31 18:06:24 +0000 |
commit | 0186661e85737ac2f4805f876c8d2d4157126f68 (patch) | |
tree | 55f34efb6e15069b659ebbd088ab4f3334f57b7f | |
parent | 6506ebf7d9748cc160ebe26219ebbcc300301291 (diff) |
Reject non-D50 matrices from ICC profiles
BUG:660838
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=4200
Change-Id: Ib57eb3705d6fe638e3a9cb56788937fc7e282847
Reviewed-on: https://skia-review.googlesource.com/4200
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
-rw-r--r-- | resources/icc_profiles/SM2333SW.icc | bin | 0 -> 2404 bytes | |||
-rw-r--r-- | src/core/SkColorSpace_ICC.cpp | 25 | ||||
-rw-r--r-- | tests/ColorSpaceTest.cpp | 8 |
3 files changed, 33 insertions, 0 deletions
diff --git a/resources/icc_profiles/SM2333SW.icc b/resources/icc_profiles/SM2333SW.icc Binary files differnew file mode 100644 index 0000000000..cf6e91fb68 --- /dev/null +++ b/resources/icc_profiles/SM2333SW.icc diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp index cba2ee1783..37c19bf442 100644 --- a/src/core/SkColorSpace_ICC.cpp +++ b/src/core/SkColorSpace_ICC.cpp @@ -940,6 +940,24 @@ static bool tag_equals(const ICCTag* a, const ICCTag* b, const uint8_t* base) { return !memcmp(a->addr(base), b->addr(base), a->fLength); } +static inline bool is_close_to_d50(const SkMatrix44& matrix) { + // rX + gX + bX + float X = matrix.getFloat(0, 0) + matrix.getFloat(0, 1) + matrix.getFloat(0, 2); + + // rY + gY + bY + float Y = matrix.getFloat(1, 0) + matrix.getFloat(1, 1) + matrix.getFloat(1, 2); + + // rZ + gZ + bZ + float Z = matrix.getFloat(2, 0) + matrix.getFloat(2, 1) + matrix.getFloat(2, 2); + + static const float kD50_WhitePoint[3] = { 0.96420f, 1.00000f, 0.82491f }; + + // This is a bit more lenient than QCMS and Adobe. Is there a reason to be stricter here? + return (SkTAbs(X - kD50_WhitePoint[0]) <= 0.04f) && + (SkTAbs(Y - kD50_WhitePoint[1]) <= 0.04f) && + (SkTAbs(Z - kD50_WhitePoint[2]) <= 0.04f); +} + sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) { if (!input || len < kICCHeaderSize) { return_null("Data is null or not large enough to contain an ICC profile"); @@ -1008,6 +1026,13 @@ sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) { 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); diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp index 7d86ae6d63..ca08563581 100644 --- a/tests/ColorSpaceTest.cpp +++ b/tests/ColorSpaceTest.cpp @@ -394,3 +394,11 @@ DEF_TEST(ColorSpace_Primaries, r) { 0.1446290f, 0.0974520f, 0.7708399f); check_primaries(r, ntsc, ntscToXYZ); } + +DEF_TEST(ColorSpace_InvalidICC, r) { + // This color space has a matrix that is not D50. + sk_sp<SkData> data = SkData::MakeFromFileName( + GetResourcePath("icc_profiles/SM2333SW.icc").c_str()); + sk_sp<SkColorSpace> cs = SkColorSpace::MakeICC(data->data(), data->size()); + REPORTER_ASSERT(r, !cs); +} |