Refactored SkColorSpace and added in a Lab PCS GM

src/core/SkColorSpace.cpp

 /*
  * Copyright 2016 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkColorSpace.h"
 #include "SkColorSpace_Base.h"
+#include "SkColorSpace_A2B0.h"
+#include "SkColorSpace_XYZTRC.h"
 #include "SkColorSpacePriv.h"
 #include "SkOnce.h"
 
-SkColorSpace_Base::SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50)
-    : fGammaNamed(gammaNamed)
-    , fGammas(nullptr)
-    , fProfileData(nullptr)
-    , fToXYZD50(toXYZD50)
-    , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
-{}
-
-SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, SkGammaNamed gammaNamed,
-                                     sk_sp<SkGammas> gammas, const SkMatrix44& toXYZD50,
-                                     sk_sp<SkData> profileData)
-    : fColorLUT(std::move(colorLUT))
-    , fGammaNamed(gammaNamed)
-    , fGammas(std::move(gammas))
-    , fProfileData(std::move(profileData))
-    , fToXYZD50(toXYZD50)
-    , fFromXYZD50(SkMatrix44::kUninitialized_Constructor)
+SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkData> profileData)
+    : fProfileData(std::move(profileData))
 {}
 
 static constexpr float gSRGB_toXYZD50[] {
     0.4358f, 0.3853f, 0.1430f,    // Rx, Gx, Bx
     0.2224f, 0.7170f, 0.0606f,    // Ry, Gy, Gz
     0.0139f, 0.0971f, 0.7139f,    // Rz, Gz, Bz
 };
 
 static constexpr float gAdobeRGB_toXYZD50[] {
     0.6098f, 0.2052f, 0.1492f,    // Rx, Gx, Bx
@@ skipping 43 matching lines @@
     }
 
     if (kNonStandard_SkGammaNamed == gammaNamed) {
         sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new SkGammas());
         gammas->fRedType = SkGammas::Type::kValue_Type;
         gammas->fGreenType = SkGammas::Type::kValue_Type;
         gammas->fBlueType = SkGammas::Type::kValue_Type;
         gammas->fRedData.fValue = values[0];
         gammas->fGreenData.fValue = values[1];
         gammas->fBlueData.fValue = values[2];
-        return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, kNonStandard_SkGammaNamed, gammas,
-                                                         toXYZD50, nullptr));
+        return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(kNonStandard_SkGammaNamed,
+                                                           gammas, toXYZD50, nullptr));
     }
 
     return SkColorSpace_Base::NewRGB(gammaNamed, toXYZD50);
 }
 
 sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50) {
     switch (gammaNamed) {
         case kSRGB_SkGammaNamed:
             if (xyz_almost_equal(toXYZD50, gSRGB_toXYZD50)) {
                 return SkColorSpace::NewNamed(kSRGB_Named);
             }
             break;
         case k2Dot2Curve_SkGammaNamed:
             if (xyz_almost_equal(toXYZD50, gAdobeRGB_toXYZD50)) {
                 return SkColorSpace::NewNamed(kAdobeRGB_Named);
             }
             break;
         case kLinear_SkGammaNamed:
             if (xyz_almost_equal(toXYZD50, gSRGB_toXYZD50)) {
                 return SkColorSpace::NewNamed(kSRGBLinear_Named);
             }
             break;
         case kNonStandard_SkGammaNamed:
             // This is not allowed.
             return nullptr;
         default:
             break;
     }
 
-    return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50));
+    return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(gammaNamed, toXYZD50));
 }
 
 sk_sp<SkColorSpace> SkColorSpace::NewRGB(RenderTargetGamma gamma, const SkMatrix44& toXYZD50) {
     switch (gamma) {
         case kLinear_RenderTargetGamma:
             return SkColorSpace_Base::NewRGB(kLinear_SkGammaNamed, toXYZD50);
         case kSRGB_RenderTargetGamma:
             return SkColorSpace_Base::NewRGB(kSRGB_SkGammaNamed, toXYZD50);
         default:
             return nullptr;
@@ skipping 10 matching lines @@
     static SkOnce sRGBLinearOnce;
 
     switch (named) {
         case kSRGB_Named: {
             sRGBOnce([] {
                 SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
                 srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50);
 
                 // Force the mutable type mask to be computed.  This avoids races.
                 (void)srgbToxyzD50.getType();
-                gSRGB = new SkColorSpace_Base(kSRGB_SkGammaNamed, srgbToxyzD50);
+                gSRGB = new SkColorSpace_XYZTRC(kSRGB_SkGammaNamed, srgbToxyzD50);
             });
             return sk_ref_sp<SkColorSpace>(gSRGB);
         }
         case kAdobeRGB_Named: {
             adobeRGBOnce([] {
                 SkMatrix44 adobergbToxyzD50(SkMatrix44::kUninitialized_Constructor);
                 adobergbToxyzD50.set3x3RowMajorf(gAdobeRGB_toXYZD50);
 
                 // Force the mutable type mask to be computed.  This avoids races.
                 (void)adobergbToxyzD50.getType();
-                gAdobeRGB = new SkColorSpace_Base(k2Dot2Curve_SkGammaNamed, adobergbToxyzD50);
+                gAdobeRGB = new SkColorSpace_XYZTRC(k2Dot2Curve_SkGammaNamed, adobergbToxyzD50);
             });
             return sk_ref_sp<SkColorSpace>(gAdobeRGB);
         }
         case kSRGBLinear_Named: {
             sRGBLinearOnce([] {
                 SkMatrix44 srgbToxyzD50(SkMatrix44::kUninitialized_Constructor);
                 srgbToxyzD50.set3x3RowMajorf(gSRGB_toXYZD50);
 
                 // Force the mutable type mask to be computed.  This avoids races.
                 (void)srgbToxyzD50.getType();
-                gSRGBLinear = new SkColorSpace_Base(kLinear_SkGammaNamed, srgbToxyzD50);
+                gSRGBLinear = new SkColorSpace_XYZTRC(kLinear_SkGammaNamed, srgbToxyzD50);
             });
             return sk_ref_sp<SkColorSpace>(gSRGBLinear);
         }
         default:
             break;
     }
     return nullptr;
 }
 
 sk_sp<SkColorSpace> SkColorSpace::makeLinearGamma() {
-    if (this->gammaIsLinear()) {
-        return sk_ref_sp(this);
-    }
-    return SkColorSpace_Base::NewRGB(kLinear_SkGammaNamed, as_CSB(this)->fToXYZD50);
+    return as_CSB(this)->makeLinearGammaImpl();
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 bool SkColorSpace::gammaCloseToSRGB() const {
-    return kSRGB_SkGammaNamed == as_CSB(this)->fGammaNamed ||
-           k2Dot2Curve_SkGammaNamed == as_CSB(this)->fGammaNamed;
+    return as_CSB(this)->gammaCloseToSRGBImpl();
 }
 
 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;
+    return as_CSB(this)->gammaIsLinearImpl();
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 enum Version {
     k0_Version, // Initial version, header + flags for matrix and profile
 };
 
 struct ColorSpaceHeader {
     /**
@@ skipping 39 matching lines @@
     uint8_t fVersion;    // Always zero
     uint8_t fNamed;      // Must be a SkColorSpace::Named
     uint8_t fGammaNamed; // Must be a SkGammaNamed
     uint8_t fFlags;      // Some combination of the flags listed above
 };
 
 size_t SkColorSpace::writeToMemory(void* memory) const {
     // Start by trying the serialization fast path.  If we haven't saved ICC profile data,
     // we must have a profile that we can serialize easily.
     if (!as_CSB(this)->fProfileData) {
+        // Profile data is mandatory for A2B0 color spaces.
         // If we have a named profile, only write the enum.

[msarett, 2016/10/11 13:40:30]

Please move this comment down to just above the "if" statement.

+        SkASSERT(as_CSB(this)->type() == SkColorSpace_Base::Type::kXYZTRC);
+        const SkColorSpace_XYZTRC* thisXYZ = static_cast<const SkColorSpace_XYZTRC*>(this);
+        const SkGammaNamed gammaNamed = thisXYZ->gammaNamed();
         if (this == gSRGB) {
             if (memory) {
                 *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, kSRGB_Named,
-                                               as_CSB(this)->fGammaNamed, 0);
+                        ColorSpaceHeader::Pack(k0_Version, kSRGB_Named, gammaNamed, 0);
             }
             return sizeof(ColorSpaceHeader);
         } else if (this == gAdobeRGB) {
             if (memory) {
                 *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named,
-                                               as_CSB(this)->fGammaNamed, 0);
+                        ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named, gammaNamed, 0);
             }
             return sizeof(ColorSpaceHeader);
         } else if (this == gSRGBLinear) {
             if (memory) {
                 *((ColorSpaceHeader*)memory) =
-                        ColorSpaceHeader::Pack(k0_Version, kSRGBLinear_Named,
-                                               as_CSB(this)->fGammaNamed, 0);
+                        ColorSpaceHeader::Pack(k0_Version, kSRGBLinear_Named, gammaNamed, 0);
             }
             return sizeof(ColorSpaceHeader);
         }
 
         // If we have a named gamma, write the enum and the matrix.
-        switch (as_CSB(this)->fGammaNamed) {
+        switch (gammaNamed) {
             case kSRGB_SkGammaNamed:
             case k2Dot2Curve_SkGammaNamed:
             case kLinear_SkGammaNamed: {
                 if (memory) {
                     *((ColorSpaceHeader*) memory) =
-                            ColorSpaceHeader::Pack(k0_Version, 0, as_CSB(this)->fGammaNamed,
+                            ColorSpaceHeader::Pack(k0_Version, 0, gammaNamed,
                                                    ColorSpaceHeader::kMatrix_Flag);
                     memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
-                    as_CSB(this)->fToXYZD50.as3x4RowMajorf((float*) memory);
+                    thisXYZ->toXYZD50()->as3x4RowMajorf((float*) memory);
                 }
                 return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
             }
             default:
                 // Otherwise, write the gamma values and the matrix.
                 if (memory) {
                     *((ColorSpaceHeader*) memory) =
-                            ColorSpaceHeader::Pack(k0_Version, 0, as_CSB(this)->fGammaNamed,
+                            ColorSpaceHeader::Pack(k0_Version, 0, gammaNamed,
                                                    ColorSpaceHeader::kFloatGamma_Flag);
                     memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
 
-                    const SkGammas* gammas = as_CSB(this)->gammas();
+                    const SkGammas* gammas = thisXYZ->gammas();
                     SkASSERT(gammas);
                     SkASSERT(SkGammas::Type::kValue_Type == gammas->fRedType &&
                              SkGammas::Type::kValue_Type == gammas->fGreenType &&
                              SkGammas::Type::kValue_Type == gammas->fBlueType);
                     *(((float*) memory) + 0) = gammas->fRedData.fValue;
                     *(((float*) memory) + 1) = gammas->fGreenData.fValue;
                     *(((float*) memory) + 2) = gammas->fBlueData.fValue;
                     memory = SkTAddOffset<void>(memory, 3 * sizeof(float));
 
-                    as_CSB(this)->fToXYZD50.as3x4RowMajorf((float*) memory);
+                    thisXYZ->toXYZD50()->as3x4RowMajorf((float*) memory);
                 }
                 return sizeof(ColorSpaceHeader) + 15 * sizeof(float);
         }
     }
 
     // Otherwise, serialize the ICC data.
     size_t profileSize = as_CSB(this)->fProfileData->size();
     if (SkAlign4(profileSize) != (uint32_t) SkAlign4(profileSize)) {
         return 0;
     }
@@ skipping 88 matching lines @@
 }
 
 bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
     if (src == dst) {
         return true;
     }
 
     if (!src || !dst) {
         return false;
     }
+    
+    if (as_CSB(src)->type() != as_CSB(dst)->type()) {
+        return false;
+    }
 
     SkData* srcData = as_CSB(src)->fProfileData.get();
     SkData* dstData = as_CSB(dst)->fProfileData.get();
     if (srcData || dstData) {
         if (srcData && dstData) {
             return srcData->size() == dstData->size() &&
                    0 == memcmp(srcData->data(), dstData->data(), srcData->size());
         }
 
         return false;
     }
 
     // It's important to check fProfileData before named gammas.  Some profiles may have named
-    // gammas, but also include other wacky features that cause us to save the data.
-    switch (as_CSB(src)->fGammaNamed) {
-        case kSRGB_SkGammaNamed:
-        case k2Dot2Curve_SkGammaNamed:
-        case kLinear_SkGammaNamed:
-            return (as_CSB(src)->fGammaNamed == as_CSB(dst)->fGammaNamed) &&
-                   (as_CSB(src)->fToXYZD50 == as_CSB(dst)->fToXYZD50);
-        default:
-            if (as_CSB(src)->fGammaNamed != as_CSB(dst)->fGammaNamed) {
+    // gammas, but also include other wacky features that cause us to save the data.    

[msarett, 2016/10/11 13:40:31]

I believe you don't need any of this?  Won't the fProfileData case catch this?

[raftias, 2016/10/14 17:10:15]

Yeah, I guess there wouldn't be any A2B0 profiles without profile data, unlike
the XYZTRC ones.

+    if (as_CSB(src)->type() == SkColorSpace_Base::Type::kXYZTRC) {
+        const SkColorSpace_XYZTRC& srcXYZ = *static_cast<const SkColorSpace_XYZTRC*>(src);
+        const SkColorSpace_XYZTRC& dstXYZ = *static_cast<const SkColorSpace_XYZTRC*>(dst);
+
+        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;
+                }
+        }
+        // TRC tables still need checking
+    } else {
+        const SkColorSpace_A2B0& srcA2B = *static_cast<const SkColorSpace_A2B0*>(src);
+        const SkColorSpace_A2B0& dstA2B = *static_cast<const SkColorSpace_A2B0*>(dst);
+
+        const SkGammaNamed aNamed = srcA2B.aCurveNamed();
+        const SkGammaNamed bNamed = srcA2B.bCurveNamed();
+        const SkGammaNamed mNamed = srcA2B.mCurveNamed();
+        if ((aNamed != dstA2B.aCurveNamed()) ||
+            (bNamed != dstA2B.bCurveNamed()) ||
+            (mNamed != dstA2B.mCurveNamed())) {
+            return false;
+        }
+        if (aNamed != kNonStandard_SkGammaNamed &&
+            bNamed != kNonStandard_SkGammaNamed &&
+            mNamed != kNonStandard_SkGammaNamed) {
+            // All gammas are the same type and are not tables/parametric, so see if
+            // the matrices differ
+            if ((srcA2B.matrix() != dstA2B.matrix()) ||
+                (srcA2B.matrix() != dstA2B.matrix()) ||
+                (srcA2B.matrix() != dstA2B.matrix())) {
                 return false;
             }
+        }
+        // a/b/m-curve tables and CLUT (if applicable) still need checking
+    }
 
-            // It is unlikely that we will reach this case.
-            sk_sp<SkData> srcData = src->serialize();
-            sk_sp<SkData> dstData = dst->serialize();
-            return srcData->size() == dstData->size() &&
-                   0 == memcmp(srcData->data(), dstData->data(), srcData->size());
-    }
+    // It is unlikely that we will reach this case.
+    sk_sp<SkData> serializedSrcData = src->serialize();

[msarett, 2016/10/11 13:40:30]

Thanks, I should not have shadowed my other variable names...

+    sk_sp<SkData> serializedDstData = dst->serialize();
+    return serializedSrcData->size() == serializedDstData->size() &&
+           0 == memcmp(serializedSrcData->data(), serializedDstData->data(),
+                       serializedSrcData->size());
 }