Hash the gamut of XYZ color spaces, to speed up comparison

Also going to use this to allow caching of GrColorSpaceXforms

BUG=skia:

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

Change-Id: I56ed2dcbdddc22046263f56d68f2d6aea55547c8
Reviewed-on: https://skia-review.googlesource.com/3670
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
Reviewed-by: Brian Salomon <bsalomon@google.com>

8 files changed, 38 insertions, 52 deletions

diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index e90254c969..ba45d9b6f4 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -576,16 +576,20 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
     const SkColorSpace_XYZ* srcXYZ = static_cast<const SkColorSpace_XYZ*>(src);
     const SkColorSpace_XYZ* dstXYZ = static_cast<const SkColorSpace_XYZ*>(dst);
 
+    if (srcXYZ->gammaNamed() != dstXYZ->gammaNamed()) {
+        return false;
+    }
+
     switch (srcXYZ->gammaNamed()) {
         case kSRGB_SkGammaNamed:
         case k2Dot2Curve_SkGammaNamed:
         case kLinear_SkGammaNamed:
-            return (srcXYZ->gammaNamed() == dstXYZ->gammaNamed()) &&
-                   (*srcXYZ->toXYZD50() == *dstXYZ->toXYZD50());
-        default:
-            if (srcXYZ->gammaNamed() != dstXYZ->gammaNamed()) {
-                return false;
+            if (srcXYZ->toXYZD50Hash() == dstXYZ->toXYZD50Hash()) {
+                SkASSERT(*srcXYZ->toXYZD50() == *dstXYZ->toXYZD50() && "Hash collision");
+                return true;
             }
+            return false;
+        default:
             // It is unlikely that we will reach this case.
             sk_sp<SkData> serializedSrcData = src->serialize();
             sk_sp<SkData> serializedDstData = dst->serialize();
diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index cacb38878c..fbd5972ffc 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -337,20 +337,6 @@ void SkColorSpaceXform_Base::BuildDstGammaTables(const uint8_t* dstGammaTables[3
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-static inline bool is_almost_identity(const SkMatrix44& srcToDst) {
-    for (int i = 0; i < 4; i++) {
-        for (int j = 0; j < 4; j++) {
-            float expected = (i == j) ? 1.0f : 0.0f;
-            if (!color_space_almost_equal(srcToDst.getFloat(i,j), expected)) {
-                return false;
-            }
-        }
-    }
-    return true;
-}
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
 std::unique_ptr<SkColorSpaceXform> SkColorSpaceXform::New(SkColorSpace* srcSpace,
                                                           SkColorSpace* dstSpace) {
     if (!srcSpace || !dstSpace) {
@@ -381,11 +367,12 @@ std::unique_ptr<SkColorSpaceXform> SkColorSpaceXform::New(SkColorSpace* srcSpace
         srcToDst.setIdentity();
         csm = kFull_ColorSpaceMatch;
     } else {
-        srcToDst.setConcat(*dstSpaceXYZ->fromXYZD50(), *srcSpaceXYZ->toXYZD50());
-
-        if (is_almost_identity(srcToDst)) {
+        if (srcSpaceXYZ->toXYZD50Hash() == dstSpaceXYZ->toXYZD50Hash()) {
+            SkASSERT(*srcSpaceXYZ->toXYZD50() == *dstSpaceXYZ->toXYZD50() && "Hash collision");
             srcToDst.setIdentity();
             csm = kGamut_ColorSpaceMatch;
+        } else {
+            srcToDst.setConcat(*dstSpaceXYZ->fromXYZD50(), *srcSpaceXYZ->toXYZD50());
         }
     }
 
diff --git a/src/core/SkColorSpace_A2B.h b/src/core/SkColorSpace_A2B.h
index 0fc952bd6c..a0bd4a0319 100644
--- a/src/core/SkColorSpace_A2B.h
+++ b/src/core/SkColorSpace_A2B.h
@@ -34,6 +34,11 @@ public:
         return nullptr;
     }
 
+    uint32_t toXYZD50Hash() const override {
+        // See toXYZD50()'s comment.
+        return 0;
+    }
+
     const SkMatrix44* fromXYZD50() const override {
         // See toXYZD50()'s comment. Also, A2B0 profiles are not supported
         // as destination color spaces, so an inverse matrix is never wanted.
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index d8470950c4..31edfa28be 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -177,6 +177,13 @@ public:
     virtual const SkMatrix44* toXYZD50() const = 0;
 
     /**
+     *  Returns a hash of the gamut transofmration to XYZ D50. Allows for fast equality checking
+     *  of gamuts, at the (very small) risk of collision.
+     *  Returns 0 if color gamut cannot be described in terms of XYZ D50.
+     */
+    virtual uint32_t toXYZD50Hash() const = 0;
+
+    /**
      *  Describes color space gamut as a transformation from XYZ D50
      *  Returns nullptr if color gamut cannot be described in terms of XYZ D50.
      */
diff --git a/src/core/SkColorSpace_XYZ.cpp b/src/core/SkColorSpace_XYZ.cpp
index 1f62245d73..3570e25b62 100644
--- a/src/core/SkColorSpace_XYZ.cpp
+++ b/src/core/SkColorSpace_XYZ.cpp
@@ -6,6 +6,7 @@
  */
 
 #include "SkColorSpace_XYZ.h"
+#include "SkChecksum.h"
 #include "SkColorSpaceXform_Base.h"
 
 static constexpr float gSRGB_toXYZD50[] {
@@ -19,6 +20,7 @@ SkColorSpace_XYZ::SkColorSpace_XYZ(SkGammaNamed gammaNamed, const SkMatrix44& to
     , fGammaNamed(gammaNamed)
     , fGammas(nullptr)
     , fToXYZD50(toXYZD50)
+    , fToXYZD50Hash(SkGoodHash()(toXYZD50))
     , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
 {}
 
@@ -28,6 +30,7 @@ SkColorSpace_XYZ::SkColorSpace_XYZ(SkGammaNamed gammaNamed, sk_sp<SkGammas> gamm
     , fGammaNamed(gammaNamed)
     , fGammas(std::move(gammas))
     , fToXYZD50(toXYZD50)
+    , fToXYZD50Hash(SkGoodHash()(toXYZD50))
     , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
 {}
 
diff --git a/src/core/SkColorSpace_XYZ.h b/src/core/SkColorSpace_XYZ.h
index 72ec3f1b67..07f44cab4c 100644
--- a/src/core/SkColorSpace_XYZ.h
+++ b/src/core/SkColorSpace_XYZ.h
@@ -15,7 +15,8 @@
 class SkColorSpace_XYZ : public SkColorSpace_Base {
 public:
     const SkMatrix44* toXYZD50() const override { return &fToXYZD50; }
-    
+    uint32_t toXYZD50Hash() const override { return fToXYZD50Hash; }
+
     const SkMatrix44* fromXYZD50() const override;
 
     bool onGammaCloseToSRGB() const override;
@@ -41,6 +42,7 @@ private:
     const SkGammaNamed     fGammaNamed;
     sk_sp<SkGammas>        fGammas;
     const SkMatrix44       fToXYZD50;
+    uint32_t               fToXYZD50Hash;
     
     mutable SkMatrix44     fFromXYZD50;
     mutable SkOnce         fFromXYZOnce;
diff --git a/src/gpu/GrColorSpaceXform.cpp b/src/gpu/GrColorSpaceXform.cpp
index ef96087147..8cfd374e8a 100644
--- a/src/gpu/GrColorSpaceXform.cpp
+++ b/src/gpu/GrColorSpaceXform.cpp
@@ -10,31 +10,6 @@
 #include "SkColorSpace_Base.h"
 #include "SkMatrix44.h"
 
-static inline bool sk_float_almost_equals(float x, float y, float tol) {
-    return sk_float_abs(x - y) <= tol;
-}
-
-static inline bool matrix_is_almost_identity(const SkMatrix44& m,
-                                             SkMScalar tol = SK_MScalar1 / (1 << 12)) {
-    return
-        sk_float_almost_equals(m.getFloat(0, 0), 1.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(0, 1), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(0, 2), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(0, 3), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(1, 0), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(1, 1), 1.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(1, 2), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(1, 3), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(2, 0), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(2, 1), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(2, 2), 1.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(2, 3), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(3, 0), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(3, 1), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(3, 2), 0.0f, tol) &&
-        sk_float_almost_equals(m.getFloat(3, 3), 1.0f, tol);
-}
-
 GrColorSpaceXform::GrColorSpaceXform(const SkMatrix44& srcToDst) 
     : fSrcToDst(srcToDst) {}
 
@@ -48,7 +23,6 @@ sk_sp<GrColorSpaceXform> GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace
         // Quick equality check - no conversion needed in this case
         return nullptr;
     }
-
     
     const SkMatrix44* toXYZD50   = as_CSB(src)->toXYZD50();
     const SkMatrix44* fromXYZD50 = as_CSB(dst)->fromXYZD50();
@@ -56,13 +30,16 @@ sk_sp<GrColorSpaceXform> GrColorSpaceXform::Make(SkColorSpace* src, SkColorSpace
         // unsupported colour spaces -- cannot specify gamut as a matrix
         return nullptr;
     }
-    SkMatrix44 srcToDst(SkMatrix44::kUninitialized_Constructor);
-    srcToDst.setConcat(*fromXYZD50, *toXYZD50);
 
-    if (matrix_is_almost_identity(srcToDst)) {
+    if (as_CSB(src)->toXYZD50Hash() == as_CSB(dst)->toXYZD50Hash()) {
+        // Identical gamut - no conversion needed in this case
+        SkASSERT(*toXYZD50 == *as_CSB(dst)->toXYZD50() && "Hash collision");
         return nullptr;
     }
 
+    SkMatrix44 srcToDst(SkMatrix44::kUninitialized_Constructor);
+    srcToDst.setConcat(*fromXYZD50, *toXYZD50);
+
     return sk_make_sp<GrColorSpaceXform>(srcToDst);
 }
 
diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index 04d401012d..d0d88050f6 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -175,6 +175,7 @@ DEF_TEST(ColorSpaceWriteICC, r) {
     SkASSERT(SkColorSpace_Base::Type::kXYZ == as_CSB(newMonitorSpace)->type());
     SkColorSpace_XYZ* newMonitorSpaceXYZ = static_cast<SkColorSpace_XYZ*>(newMonitorSpace.get());
     REPORTER_ASSERT(r, *monitorSpaceXYZ->toXYZD50() == *newMonitorSpaceXYZ->toXYZD50());
+    REPORTER_ASSERT(r, monitorSpaceXYZ->toXYZD50Hash() == newMonitorSpaceXYZ->toXYZD50Hash());
     REPORTER_ASSERT(r, monitorSpaceXYZ->gammaNamed() == newMonitorSpaceXYZ->gammaNamed());
 }