From a12f795f4ab4b97d31acfd8bfbbbb6e06a70e999 Mon Sep 17 00:00:00 2001 From: Brian Osman Date: Tue, 19 Dec 2017 14:44:12 -0500 Subject: 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 Reviewed-by: Mike Klein Reviewed-by: Mike Klein Commit-Queue: Brian Osman --- resources/icc_profiles/AdobeRGB1998.icc | Bin 0 -> 560 bytes src/core/SkColorSpace.cpp | 2 ++ src/core/SkColorSpacePriv.h | 11 +++++++++++ tests/ICCTest.cpp | 12 +++++++++++- 4 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 resources/icc_profiles/AdobeRGB1998.icc diff --git a/resources/icc_profiles/AdobeRGB1998.icc b/resources/icc_profiles/AdobeRGB1998.icc new file mode 100644 index 0000000000..a79f576b59 Binary files /dev/null and b/resources/icc_profiles/AdobeRGB1998.icc differ diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp index e9efffd96d..235518d3b9 100644 --- a/src/core/SkColorSpace.cpp +++ b/src/core/SkColorSpace.cpp @@ -119,6 +119,7 @@ sk_sp SkColorSpace_Base::MakeRGB(SkGammaNamed gammaNamed, const Sk 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 @@ sk_sp SkColorSpace_Base::MakeRGB(SkGammaNamed gammaNamed, const Sk return sk_sp(new SkColorSpace_XYZ(k2Dot2Curve_SkGammaNamed, adobe44)); } break; +#endif case kLinear_SkGammaNamed: if (xyz_almost_equal(toXYZD50, gSRGB_toXYZD50)) { return SkColorSpace::MakeSRGBLinear(); diff --git a/src/core/SkColorSpacePriv.h b/src/core/SkColorSpacePriv.h index 64fe31fce3..b0a8c1dcbc 100644 --- a/src/core/SkColorSpacePriv.h +++ b/src/core/SkColorSpacePriv.h @@ -10,6 +10,7 @@ #include #include "SkColorSpace_Base.h" +#include "SkFixed.h" #define SkColorSpacePrintf(...) @@ -20,9 +21,19 @@ static constexpr float gSRGB_toXYZD50[] { }; 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[] { diff --git a/tests/ICCTest.cpp b/tests/ICCTest.cpp index cfe4dc04a0..378a430dc0 100644 --- a/tests/ICCTest.cpp +++ b/tests/ICCTest.cpp @@ -98,6 +98,16 @@ DEF_TEST(ICC_IsNumericalTransferFn, r) { 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 data = GetResourceAsData("icc_profiles/AdobeRGB1998.icc"); + sk_sp fromIcc = SkColorSpace::MakeICC(data->data(), data->size()); + sk_sp 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 profile = SkICC::WriteToICC(fn, toXYZD50); @@ -124,7 +134,7 @@ DEF_TEST(ICC_WriteICC, r) { 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; -- cgit v1.2.3