commit a12f795f4ab4b97d31acfd8bfbbbb6e06a70e999
author Brian Osman <brianosman@google.com> Tue Dec 19 14:44:12 2017 -0500
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Wed Dec 20 14:09:34 2017 +0000
tree f3c132ed909a214ac67e3352f1f2d965d1a10dc9
parent b5086961f335d6757a88ce7507c445485aaba2e6

Fix Adobe RGB color space in Skia

Our runtime definition of the XYZ matrix was fairly inaccurate. It also
didn't round-trip through ICC fixed point correctly. Now, constructing the
color space at runtime produces exactly the same matrix as constructing
the space from the ICC profile. And the values can then be serialized back
to ICC exactly. This eliminates the need for the snapping logic, too.

Bug: skia:
Change-Id: I69f4a9bfec3eeef153935e21ab3a0630794b1607
Reviewed-on: https://skia-review.googlesource.com/84840
Reviewed-by: Brian Osman <brianosman@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>
Commit-Queue: Brian Osman <brianosman@google.com>

resources/icc_profiles/AdobeRGB1998.icc

diff --git a/resources/icc_profiles/AdobeRGB1998.icc b/resources/icc_profiles/AdobeRGB1998.icc
new file mode 100644
index 0000000..a79f576
--- /dev/null
+++ b/resources/icc_profiles/AdobeRGB1998.icc
Binary files differ

src/core/SkColorSpace.cpp

diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index e9efffd..235518d 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -119,6 +119,7 @@
                 return SkColorSpace::MakeSRGB();
             }
             break;
+#ifdef SK_SUPPORT_LEGACY_ADOBE_XYZ
         case k2Dot2Curve_SkGammaNamed:
             if (xyz_almost_equal(toXYZD50, gAdobeRGB_toXYZD50)) {
                 SkMatrix44 adobe44(SkMatrix44::kUninitialized_Constructor);
@@ -126,6 +127,7 @@
                 return sk_sp<SkColorSpace>(new SkColorSpace_XYZ(k2Dot2Curve_SkGammaNamed, adobe44));
             }
             break;
+#endif
         case kLinear_SkGammaNamed:
             if (xyz_almost_equal(toXYZD50, gSRGB_toXYZD50)) {
                 return SkColorSpace::MakeSRGBLinear();

src/core/SkColorSpacePriv.h

diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h
index 64fe31f..b0a8c1d 100644
--- a/src/core/SkColorSpacePriv.h
+++ b/src/core/SkColorSpacePriv.h
@@ -10,6 +10,7 @@
 #include <math.h>
 
 #include "SkColorSpace_Base.h"
+#include "SkFixed.h"
 
 #define SkColorSpacePrintf(...)
 
@@ -20,9 +21,19 @@
 };
 
 static constexpr float gAdobeRGB_toXYZD50[] {
+#ifdef SK_SUPPORT_LEGACY_ADOBE_XYZ
     0.6097559f, 0.2052401f, 0.1492240f, // Rx, Gx, Bx
     0.3111242f, 0.6256560f, 0.0632197f, // Ry, Gy, Gz
     0.0194811f, 0.0608902f, 0.7448387f, // Rz, Gz, Bz
+#else
+    // ICC fixed-point (16.16) repesentation of:
+    // 0.60974, 0.20528, 0.14919,
+    // 0.31111, 0.62567, 0.06322,
+    // 0.01947, 0.06087, 0.74457,
+    SkFixedToFloat(0x9c18), SkFixedToFloat(0x348d), SkFixedToFloat(0x2631), // Rx, Gx, Bx
+    SkFixedToFloat(0x4fa5), SkFixedToFloat(0xa02c), SkFixedToFloat(0x102f), // Ry, Gy, Gz
+    SkFixedToFloat(0x04fc), SkFixedToFloat(0x0f95), SkFixedToFloat(0xbe9c), // Rz, Gz, Bz
+#endif
 };
 
 static constexpr float gDCIP3_toXYZD50[] {

tests/ICCTest.cpp

diff --git a/tests/ICCTest.cpp b/tests/ICCTest.cpp
index cfe4dc0..378a430 100644
--- a/tests/ICCTest.cpp
+++ b/tests/ICCTest.cpp
@@ -98,6 +98,16 @@
     test_is_numerical_transfer_fn(r, upperRight.get(), false, referenceFn);
 }
 
+DEF_TEST(ICC_Adobe, r) {
+    // Test that the color spaces produced by our procedural Adobe factory, and the official
+    // Adobe ICC profile match exactly.
+    sk_sp<SkData> data = GetResourceAsData("icc_profiles/AdobeRGB1998.icc");
+    sk_sp<SkColorSpace> fromIcc = SkColorSpace::MakeICC(data->data(), data->size());
+    sk_sp<SkColorSpace> procedural = SkColorSpace::MakeRGB(g2Dot2_TransferFn,
+                                                           SkColorSpace::kAdobeRGB_Gamut);
+    REPORTER_ASSERT(r, SkColorSpace::Equals(fromIcc.get(), procedural.get()));
+}
+
 static inline void test_write_icc(skiatest::Reporter* r, const SkColorSpaceTransferFn& fn,
                                   const SkMatrix44& toXYZD50, bool writeToFile) {
     sk_sp<SkData> profile = SkICC::WriteToICC(fn, toXYZD50);
@@ -124,7 +134,7 @@
     adobeMatrix.set3x3RowMajorf(gAdobeRGB_toXYZD50);
     // TODO: Restore this test once we fix our Adobe matrix to be based on the decoded ICC
     // fixed point values, and once we use a rounding conversion to fixed-point.
-//    test_write_icc(r, adobeFn, adobeMatrix, false);
+    test_write_icc(r, adobeFn, adobeMatrix, false);
 
     SkColorSpaceTransferFn srgbFn;
     srgbFn.fA = 1.0f / 1.055f;