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>
diff --git a/resources/icc_profiles/SM2333SW.icc b/resources/icc_profiles/SM2333SW.icc
new file mode 100644
index 0000000..cf6e91f
--- /dev/null
+++ b/resources/icc_profiles/SM2333SW.icc
Binary files differ
diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp
index cba2ee1..37c19bf 100644
--- a/src/core/SkColorSpace_ICC.cpp
+++ b/src/core/SkColorSpace_ICC.cpp
@@ -940,6 +940,24 @@
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 @@
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 7d86ae6..ca08563 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -394,3 +394,11 @@
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);
+}