Revert of Delete SkColorSpace::kUnknown_Named, remove fNamed field (patchset #1 id...

Reason for revert:
This CL introduced two static initializers (gAdobeRGB and gSRGB) which are causing a sizes regression on Chromium builders:

https://build.chromium.org/p/chromium/builders/Linux%20x64/builds/24981

Original issue's description:
> Delete SkColorSpace::kUnknown_Named, remove fNamed field
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2302413002
>
> Committed: https://skia.googlesource.com/skia/+/54682e856cb66c653bc7e253981a421a2618398e

TBR=reed@google.com,brianosman@google.com,msarett@google.com
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=skia:5724

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

diff --git a/include/core/SkColorSpace.h b/include/core/SkColorSpace.h
index 257893f..5809577 100644 (file)
--- a/include/core/SkColorSpace.h
+++ b/include/core/SkColorSpace.h
@@ -19,21 +19,13 @@ public:
     /**
      *  Common, named profiles that we can recognize.
      */
-    enum Named : uint8_t {
-        /**
-         *  By far the most common color space.
-         *  This is the default space for images, unmarked content, and monitors.
-         */
+    enum Named {
+        kUnknown_Named,
         kSRGB_Named,
-
-        /**
-         *  Very common wide gamut color space.
-         *  Often used by images and monitors.
-         */
         kAdobeRGB_Named,
     };
 
-    enum GammaNamed : uint8_t {
+    enum GammaNamed {
         kLinear_GammaNamed,
 
         /**
@@ -124,10 +116,11 @@ public:
     static bool Equals(const SkColorSpace* src, const SkColorSpace* dst);
 
 protected:
-    SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50);
+    SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named);
 
     const GammaNamed fGammaNamed;
     const SkMatrix44 fToXYZD50;
+    const Named      fNamed;
 };
 
 #endif

diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index 682c980..abdd647 100644 (file)
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -10,13 +10,14 @@
 #include "SkColorSpacePriv.h"
 #include "SkOnce.h"
 
-SkColorSpace::SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50)
+SkColorSpace::SkColorSpace(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named)
     : fGammaNamed(gammaNamed)
     , fToXYZD50(toXYZD50)
+    , fNamed(named)
 {}
 
-SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZD50)
-    : INHERITED(gammaNamed, toXYZD50)
+SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZD50, Named named)
+    : INHERITED(gammaNamed, toXYZD50, named)
     , fGammas(nullptr)
     , fProfileData(nullptr)
 {}
@@ -24,7 +25,7 @@ SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& to
 SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, GammaNamed gammaNamed,
                                      sk_sp<SkGammas> gammas, const SkMatrix44& toXYZD50,
                                      sk_sp<SkData> profileData)
-    : INHERITED(gammaNamed, toXYZD50)
+    : INHERITED(gammaNamed, toXYZD50, kUnknown_Named)
     , fColorLUT(std::move(colorLUT))
     , fGammas(std::move(gammas))
     , fProfileData(std::move(profileData))
@@ -117,19 +118,18 @@ sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(GammaNamed gammaNamed, const SkMat
             break;
     }
 
-    return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50));
+    return sk_sp<SkColorSpace>(new SkColorSpace_Base(gammaNamed, toXYZD50, kUnknown_Named));
 }
 
 sk_sp<SkColorSpace> SkColorSpace::NewRGB(GammaNamed gammaNamed, const SkMatrix44& toXYZD50) {
     return SkColorSpace_Base::NewRGB(gammaNamed, toXYZD50);
 }
 
-static sk_sp<SkColorSpace> gAdobeRGB;
-static sk_sp<SkColorSpace> gSRGB;
-
 sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
     static SkOnce sRGBOnce;
+    static sk_sp<SkColorSpace> sRGB;
     static SkOnce adobeRGBOnce;
+    static sk_sp<SkColorSpace> adobeRGB;
 
     switch (named) {
         case kSRGB_Named: {
@@ -139,9 +139,9 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
 
                 // Force the mutable type mask to be computed.  This avoids races.
                 (void)srgbToxyzD50.getType();
-                gSRGB.reset(new SkColorSpace_Base(kSRGB_GammaNamed, srgbToxyzD50));
+                sRGB.reset(new SkColorSpace_Base(kSRGB_GammaNamed, srgbToxyzD50, kSRGB_Named));
             });
-            return gSRGB;
+            return sRGB;
         }
         case kAdobeRGB_Named: {
             adobeRGBOnce([] {
@@ -150,9 +150,10 @@ sk_sp<SkColorSpace> SkColorSpace::NewNamed(Named named) {
 
                 // Force the mutable type mask to be computed.  This avoids races.
                 (void)adobergbToxyzD50.getType();
-                gAdobeRGB.reset(new SkColorSpace_Base(k2Dot2Curve_GammaNamed, adobergbToxyzD50));
+                adobeRGB.reset(new SkColorSpace_Base(k2Dot2Curve_GammaNamed, adobergbToxyzD50,
+                                                     kAdobeRGB_Named));
             });
-            return gAdobeRGB;
+            return adobeRGB;
         }
         default:
             break;
@@ -193,8 +194,8 @@ struct ColorSpaceHeader {
      */
     static constexpr uint8_t kFloatGamma_Flag = 1 << 2;
 
-    static ColorSpaceHeader Pack(Version version, uint8_t named, uint8_t gammaNamed, uint8_t flags)
-    {
+    static ColorSpaceHeader Pack(Version version, SkColorSpace::Named named,
+                                 SkColorSpace::GammaNamed gammaNamed, uint8_t flags) {
         ColorSpaceHeader header;
 
         SkASSERT(k0_Version == version);
@@ -222,17 +223,17 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
     // we must have a profile that we can serialize easily.
     if (!as_CSB(this)->fProfileData) {
         // If we have a named profile, only write the enum.
-        if (this == gSRGB.get()) {
-            if (memory) {
-                *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, kSRGB_Named, fGammaNamed, 0);
-            }
-            return sizeof(ColorSpaceHeader);
-        } else if (this == gAdobeRGB.get()) {
-            if (memory) {
-                *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, kAdobeRGB_Named, fGammaNamed, 0);
+        switch (fNamed) {
+            case kSRGB_Named:
+            case kAdobeRGB_Named: {
+                if (memory) {
+                    *((ColorSpaceHeader*) memory) =
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
+                }
+                return sizeof(ColorSpaceHeader);
             }
+            default:
+                break;
         }
 
         // If we have a named gamma, write the enum and the matrix.
@@ -242,7 +243,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
             case kLinear_GammaNamed: {
                 if (memory) {
                     *((ColorSpaceHeader*) memory) =
-                            ColorSpaceHeader::Pack(k0_Version, 0, fGammaNamed,
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
                                                    ColorSpaceHeader::kMatrix_Flag);
                     memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
                     fToXYZD50.as4x3ColMajorf((float*) memory);
@@ -253,7 +254,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
                 // Otherwise, write the gamma values and the matrix.
                 if (memory) {
                     *((ColorSpaceHeader*) memory) =
-                            ColorSpaceHeader::Pack(k0_Version, 0, fGammaNamed,
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
                                                    ColorSpaceHeader::kFloatGamma_Flag);
                     memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
 
@@ -280,7 +281,7 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
     }
 
     if (memory) {
-        *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, 0,
+        *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, kUnknown_Named,
                                                                kNonStandard_GammaNamed,
                                                                ColorSpaceHeader::kICC_Flag);
         memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
@@ -313,8 +314,12 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) {
     ColorSpaceHeader header = *((const ColorSpaceHeader*) data);
     data = SkTAddOffset<const void>(data, sizeof(ColorSpaceHeader));
     length -= sizeof(ColorSpaceHeader);
-    if (0 == header.fFlags) {
-        return NewNamed((Named) header.fNamed);
+    switch ((Named) header.fNamed) {
+        case kSRGB_Named:
+        case kAdobeRGB_Named:
+            return NewNamed((Named) header.fNamed);
+        default:
+            break;
     }
 
     switch ((GammaNamed) header.fGammaNamed) {
@@ -377,6 +382,17 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
         return false;
     }
 
+    switch (src->fNamed) {
+        case kSRGB_Named:
+        case kAdobeRGB_Named:
+            return src->fNamed == dst->fNamed;
+        case kUnknown_Named:
+            if (kUnknown_Named != dst->fNamed) {
+                return false;
+            }
+            break;
+    }
+
     SkData* srcData = as_CSB(src)->fProfileData.get();
     SkData* dstData = as_CSB(dst)->fProfileData.get();
     if (srcData || dstData) {

diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 5c36e44..38e0ba0 100644 (file)
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -193,7 +193,7 @@ private:
 
     static sk_sp<SkColorSpace> NewRGB(GammaNamed gammaNamed, const SkMatrix44& toXYZD50);
 
-    SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZ);
+    SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& toXYZ, Named named);
 
     SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, GammaNamed gammaNamed,
                       sk_sp<SkGammas> gammas, const SkMatrix44& toXYZ, sk_sp<SkData> profileData);

diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index b59b6cc..3eb145b 100644 (file)
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -149,15 +149,17 @@ DEF_TEST(ColorSpaceWriteICC, r) {
 DEF_TEST(ColorSpace_Named, r) {
     const struct {
         SkColorSpace::Named fNamed;
+        bool fExpectedToSucceed;
         bool fIsSRGB;
     } recs[] {
-        { SkColorSpace::kSRGB_Named,     true },
-        { SkColorSpace::kAdobeRGB_Named, false },
+        { SkColorSpace::kUnknown_Named,  false, false },
+        { SkColorSpace::kSRGB_Named,     true,  true },
+        { SkColorSpace::kAdobeRGB_Named, true,  false },
     };
 
     for (auto rec : recs) {
         auto cs = SkColorSpace::NewNamed(rec.fNamed);
-        REPORTER_ASSERT(r, cs);
+        REPORTER_ASSERT(r, !cs == !rec.fExpectedToSucceed);
         if (cs) {
             if (rec.fIsSRGB) {
                 REPORTER_ASSERT(r, SkColorSpace::kSRGB_GammaNamed == cs->gammaNamed());