Cache the inverse matrix on SkColorSpace. Rename xyz() to toXYZ().

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2323003002

Review-Url: https://codereview.chromium.org/2323003002

diff --git a/include/core/SkColorSpace.h b/include/core/SkColorSpace.h
index 045117552626b55373074c6fa3c94d3b02295c9e..b2b005e4aa6b8c2e6e82b4f36474a5daec68dea9 100644 (file)
--- a/include/core/SkColorSpace.h
+++ b/include/core/SkColorSpace.h
@@ -66,7 +66,7 @@ public:
     /**
      *  Returns the matrix used to transform src gamut to XYZ D50.
      */
-    const SkMatrix44& xyz() const { return fToXYZD50; }
+    const SkMatrix44& toXYZD50() const { return fToXYZD50; }
 
     /**
      *  Returns true if the color space gamma is near enough to be approximated as sRGB.

diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index e6676066bb24beac7ee8125478d1715921829fb9..9f04de0537f852ee744a69f0ad9cbb848d6a2a3d 100644 (file)
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -19,6 +19,7 @@ SkColorSpace_Base::SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44&
     , fGammaNamed(gammaNamed)
     , fGammas(nullptr)
     , fProfileData(nullptr)
+    , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
 {}
 
 SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, SkGammaNamed gammaNamed,
@@ -29,6 +30,7 @@ SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, SkGamma
     , fGammaNamed(gammaNamed)
     , fGammas(std::move(gammas))
     , fProfileData(std::move(profileData))
+    , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
 {}
 
 static constexpr float gSRGB_toXYZD50[] {
@@ -182,6 +184,20 @@ bool SkColorSpace::gammaIsLinear() const {
     return kLinear_SkGammaNamed == as_CSB(this)->fGammaNamed;
 }
 
+const SkMatrix44& SkColorSpace_Base::fromXYZD50() const {
+    fFromXYZOnce([this] {
+        if (!fToXYZD50.invert(&fFromXYZD50)) {
+            // If a client gives us a dst gamut with a transform that we can't invert, we will
+            // simply give them back a transform to sRGB gamut.
+            SkDEBUGFAIL("Non-invertible XYZ matrix, defaulting to sRGB");
+            SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
+            srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50);
+            srgbToxyzD50.invert(&fFromXYZD50);
+        }
+    });
+    return fFromXYZD50;
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 enum Version {

diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index f20886af7b11bd80e29e986b2df521ac41463f8e..136b4da6b922522c72fb6e8de955061304fa3e02 100644 (file)
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -446,14 +446,10 @@ static void build_gamma_tables(const T* outGammaTables[3], T* gammaTableStorage,
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-static inline bool compute_gamut_xform(SkMatrix44* srcToDst, const SkMatrix44& srcToXYZ,
-                                       const SkMatrix44& dstToXYZ) {
-    if (!dstToXYZ.invert(srcToDst)) {
-        return false;
-    }
-
-    srcToDst->postConcat(srcToXYZ);
-    return true;
+static inline void compute_gamut_xform(SkMatrix44* srcToDst, const SkColorSpace* src,
+                                       const SkColorSpace* dst) {
+    *srcToDst = as_CSB(dst)->fromXYZD50();
+    srcToDst->postConcat(src->toXYZD50());
 }
 
 static inline bool is_almost_identity(const SkMatrix44& srcToDst) {
@@ -482,11 +478,13 @@ std::unique_ptr<SkColorSpaceXform> SkColorSpaceXform::New(const sk_sp<SkColorSpa
     if (SkColorSpace::Equals(srcSpace.get(), dstSpace.get())) {
         srcToDst.setIdentity();
         csm = kFull_ColorSpaceMatch;
-    } else if (!compute_gamut_xform(&srcToDst, srcSpace->xyz(), dstSpace->xyz())) {
-        return nullptr;
-    } else if (is_almost_identity(srcToDst)) {
-        srcToDst.setIdentity();
-        csm = kGamut_ColorSpaceMatch;
+    } else {
+        compute_gamut_xform(&srcToDst, srcSpace.get(), dstSpace.get());
+
+        if (is_almost_identity(srcToDst)) {
+            srcToDst.setIdentity();
+            csm = kGamut_ColorSpaceMatch;
+        }
     }
 
     switch (csm) {

diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 31f96dd7f9dde6552acd430b04071c9ad7cd4eaa..9382153967461b7fa8b417e20229c5d9598ac81a 100644 (file)
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -10,6 +10,7 @@
 
 #include "SkColorSpace.h"
 #include "SkData.h"
+#include "SkOnce.h"
 #include "SkTemplates.h"
 
 enum SkGammaNamed : uint8_t {
@@ -188,6 +189,8 @@ public:
 
     const SkColorLookUpTable* colorLUT() const { return fColorLUT.get(); }
 
+    const SkMatrix44& fromXYZD50() const;
+
 private:
 
     /**
@@ -211,6 +214,9 @@ private:
     sk_sp<SkGammas>           fGammas;
     sk_sp<SkData>             fProfileData;
 
+    mutable SkMatrix44        fFromXYZD50;
+    mutable SkOnce            fFromXYZOnce;
+
     friend class SkColorSpace;
     friend class ColorSpaceXformTest;
     friend class ColorSpaceTest;

diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp
index 03aa27164492f184159a3d7fdec4074bf2e94331..8ff6fda0cbc80eebc1f49cfdc5c14be18f85746f 100644 (file)
--- a/src/gpu/GrColorSpaceXform.cpp
+++ b/src/gpu/GrColorSpaceXform.cpp
@@ -7,6 +7,7 @@
 
 #include "GrColorSpaceXform.h"
 #include "SkColorSpace.h"
+#include "SkColorSpace_Base.h"
 #include "SkMatrix44.h"
 
 static inline bool sk_float_almost_equals(float x, float y, float tol) {
@@ -51,11 +52,8 @@ sk_sp<GrColorSpaceXform> GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace
         return nullptr;
     }
 
-    SkMatrix44 srcToDst(SkMatrix44::kUninitialized_Constructor);
-    if (!dst->xyz().invert(&srcToDst)) {
-        return nullptr;
-    }
-    srcToDst.postConcat(src->xyz());
+    SkMatrix44 srcToDst = as_CSB(dst)->fromXYZD50();
+    srcToDst.postConcat(src->toXYZD50());
 
     if (matrix_is_almost_identity(srcToDst)) {
         return nullptr;

diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index 67015c78201293b952bcc5ac22aa0aed9d7b25e4..969e0929a404fd7bbd6fa5d19521174e1f85b8e9 100644 (file)
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -24,7 +24,7 @@ static void test_space(skiatest::Reporter* r, SkColorSpace* space,
     REPORTER_ASSERT(r, nullptr != space);
     REPORTER_ASSERT(r, expectedGamma == as_CSB(space)->gammaNamed());
 
-    const SkMatrix44& mat = space->xyz();
+    const SkMatrix44& mat = space->toXYZD50();
     const float src[] = {
         1, 0, 0, 1,
         0, 1, 0, 1,
@@ -142,7 +142,7 @@ DEF_TEST(ColorSpaceWriteICC, r) {
     sk_sp<SkData> newMonitorData = ColorSpaceTest::WriteToICC(monitorSpace.get());
     sk_sp<SkColorSpace> newMonitorSpace = SkColorSpace::NewICC(newMonitorData->data(),
                                                                newMonitorData->size());
-    REPORTER_ASSERT(r, monitorSpace->xyz() == newMonitorSpace->xyz());
+    REPORTER_ASSERT(r, monitorSpace->toXYZD50() == newMonitorSpace->toXYZD50());
     REPORTER_ASSERT(r, as_CSB(monitorSpace)->gammaNamed() == as_CSB(newMonitorSpace)->gammaNamed());
 }
 

diff --git a/tests/SurfaceTest.cpp b/tests/SurfaceTest.cpp
index 58555d37058d7050340704847fa204956337f7db..775b8c6c0567c527aa6f76de15fbe7bfb3f40b78 100644 (file)
--- a/tests/SurfaceTest.cpp
+++ b/tests/SurfaceTest.cpp
@@ -921,7 +921,7 @@ static void test_surface_creation_and_snapshot_with_color_space(
 
     auto srgbColorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
     auto adobeColorSpace = SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named);
-    SkMatrix44 srgbMatrix = srgbColorSpace->xyz();
+    SkMatrix44 srgbMatrix = srgbColorSpace->toXYZD50();
     const float oddGamma[] = { 2.4f, 2.4f, 2.4f };
     auto oddColorSpace = SkColorSpace_Base::NewRGB(oddGamma, srgbMatrix);
     auto linearColorSpace = srgbColorSpace->makeLinearGamma();

diff --git a/tests/TestConfigParsing.cpp b/tests/TestConfigParsing.cpp
index a68a52d8ff318f2ef414ba14580fc210a30092f0..1804423c0da5bce71c5377176648b6297a0ab4e1 100644 (file)
--- a/tests/TestConfigParsing.cpp
+++ b/tests/TestConfigParsing.cpp
@@ -125,8 +125,8 @@ DEF_TEST(ParseConfigs_DefaultConfigs, reporter) {
     REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorType() == kRGBA_F16_SkColorType);
     REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorSpace());
     REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorSpace()->gammaIsLinear());
-    REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorSpace()->xyz() ==
-                              srgbColorSpace->xyz());
+    REPORTER_ASSERT(reporter, configs[25]->asConfigGpu()->getColorSpace()->toXYZD50() ==
+                              srgbColorSpace->toXYZD50());
     REPORTER_ASSERT(reporter, configs[26]->asConfigGpu()->getColorType() == kN32_SkColorType);
     REPORTER_ASSERT(reporter, configs[26]->asConfigGpu()->getColorSpace() == srgbColorSpace.get());
     REPORTER_ASSERT(reporter, configs[33]->asConfigGpu()->getContextType() ==

diff --git a/tools/visualize_color_gamut.cpp b/tools/visualize_color_gamut.cpp
index 3da1e3c9362f2e0bc720974f825e047c3009dca6..a0b8b82c6516fb6cfbb773d28057122b99ffc79d 100644 (file)
--- a/tools/visualize_color_gamut.cpp
+++ b/tools/visualize_color_gamut.cpp
@@ -141,13 +141,13 @@ int main(int argc, char** argv) {
     // Draw the sRGB gamut if requested.
     if (FLAGS_sRGB) {
         sk_sp<SkColorSpace> sRGBSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
-        draw_gamut(&canvas, sRGBSpace->xyz(), "sRGB", 0xFFFF9394, false);
+        draw_gamut(&canvas, sRGBSpace->toXYZD50(), "sRGB", 0xFFFF9394, false);
     }
 
     // Draw the Adobe RGB gamut if requested.
     if (FLAGS_adobeRGB) {
         sk_sp<SkColorSpace> adobeRGBSpace = SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named);
-        draw_gamut(&canvas, adobeRGBSpace->xyz(), "Adobe RGB", 0xFF31a9e1, false);
+        draw_gamut(&canvas, adobeRGBSpace->toXYZD50(), "Adobe RGB", 0xFF31a9e1, false);
     }
 
     // Draw gamut for the input image.
@@ -156,7 +156,7 @@ int main(int argc, char** argv) {
         SkDebugf("Image had no embedded color space information.  Defaulting to sRGB.\n");
         colorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
     }
-    draw_gamut(&canvas, colorSpace->xyz(), input, 0xFF000000, true);
+    draw_gamut(&canvas, colorSpace->toXYZD50(), input, 0xFF000000, true);
 
     // Finally, encode the result to the output file.
     sk_sp<SkData> out(SkImageEncoder::EncodeData(gamut, SkImageEncoder::kPNG_Type, 100));