Fix various SkColorSpace bugs

(1) Fixes serialization/deserialization of wacky SkColorSpaces
(2) Fix gamma equals checking

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

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

10 files changed, 77 insertions, 48 deletions

diff --git a/bench/ColorCodecBench.cpp b/bench/ColorCodecBench.cpp
index 5079b482c7..84814226f1 100644
--- a/bench/ColorCodecBench.cpp
+++ b/bench/ColorCodecBench.cpp
@@ -140,7 +140,7 @@ void ColorCodecBench::onDelayedSetup() {
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(fEncoded.get()));
     fSrcData = codec->getICCData();
     sk_sp<SkData> dstData = SkData::MakeFromFileName(
-            GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+            GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
     SkASSERT(dstData);
 
     fDstSpace = nullptr;
diff --git a/dm/DMSrcSink.cpp b/dm/DMSrcSink.cpp
index c03a8c4c98..f4c8956b25 100644
--- a/dm/DMSrcSink.cpp
+++ b/dm/DMSrcSink.cpp
@@ -869,7 +869,7 @@ Error ColorCodecSrc::draw(SkCanvas* canvas) const {
 
     // Load the dst ICC profile.  This particular dst is fairly similar to Adobe RGB.
     sk_sp<SkData> dstData = SkData::MakeFromFileName(
-            GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+            GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
     if (!dstData) {
         return "Cannot read monitor profile.  Is the resource path set correctly?";
     }
diff --git a/resources/icc_profiles/HP_Z32x.icc b/resources/icc_profiles/HP_Z32x.icc
new file mode 100644
index 0000000000..3d532b89ed
--- /dev/null
+++ b/resources/icc_profiles/HP_Z32x.icc
diff --git a/resources/monitor_profiles/HP_ZR30w.icc b/resources/icc_profiles/HP_ZR30w.icc
index b3f860714c..b3f860714c 100644
--- a/resources/monitor_profiles/HP_ZR30w.icc
+++ b/resources/icc_profiles/HP_ZR30w.icc
diff --git a/resources/icc_profiles/upperLeft.icc b/resources/icc_profiles/upperLeft.icc
new file mode 100644
index 0000000000..8c521a5c25
--- /dev/null
+++ b/resources/icc_profiles/upperLeft.icc
diff --git a/resources/icc_profiles/upperRight.icc b/resources/icc_profiles/upperRight.icc
new file mode 100644
index 0000000000..44ffa87ec8
--- /dev/null
+++ b/resources/icc_profiles/upperRight.icc
diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index 6c34277b31..3874f0e7d0 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -201,49 +201,52 @@ struct ColorSpaceHeader {
 };
 
 size_t SkColorSpace::writeToMemory(void* memory) const {
-    // If we have a named profile, only write the enum.
-    switch (fNamed) {
-        case kSRGB_Named:
-        case kAdobeRGB_Named: {
-            if (memory) {
-                *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
+    // 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) {
+        // If we have a named profile, only write the enum.
+        switch (fNamed) {
+            case kSRGB_Named:
+            case kAdobeRGB_Named: {
+                if (memory) {
+                    *((ColorSpaceHeader*) memory) =
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed, 0);
+                }
+                return sizeof(ColorSpaceHeader);
             }
-            return sizeof(ColorSpaceHeader);
+            default:
+                break;
         }
-        default:
-            break;
-    }
 
-    // If we have a named gamma, write the enum and the matrix.
-    switch (fGammaNamed) {
-        case kSRGB_GammaNamed:
-        case k2Dot2Curve_GammaNamed:
-        case kLinear_GammaNamed: {
-            if (memory) {
-                *((ColorSpaceHeader*) memory) =
-                        ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
-                                               ColorSpaceHeader::kMatrix_Flag);
-                memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
-                fToXYZD50.as4x3ColMajorf((float*) memory);
+        // If we have a named gamma, write the enum and the matrix.
+        switch (fGammaNamed) {
+            case kSRGB_GammaNamed:
+            case k2Dot2Curve_GammaNamed:
+            case kLinear_GammaNamed: {
+                if (memory) {
+                    *((ColorSpaceHeader*) memory) =
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
+                                                   ColorSpaceHeader::kMatrix_Flag);
+                    memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
+                    fToXYZD50.as4x3ColMajorf((float*) memory);
+                }
+                return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
             }
-            return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
+            default:
+                SkASSERT(false);
+                return 0;
         }
-        default:
-            break;
     }
 
-    // If we do not have a named gamma, this must have been created from an ICC profile.
-    // Since we were unable to recognize the gamma, we will have saved the ICC data.
-    SkASSERT(as_CSB(this)->fProfileData);
-
+    // Otherwise, serialize the ICC data.
     size_t profileSize = as_CSB(this)->fProfileData->size();
     if (SkAlign4(profileSize) != (uint32_t) SkAlign4(profileSize)) {
         return 0;
     }
 
     if (memory) {
-        *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
+        *((ColorSpaceHeader*) memory) = ColorSpaceHeader::Pack(k0_Version, kUnknown_Named,
+                                                               kNonStandard_GammaNamed,
                                                                ColorSpaceHeader::kICC_Flag);
         memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
 
@@ -316,7 +319,8 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) {
 bool SkColorSpace::gammasAreMatching() const {
     const SkGammas* gammas = as_CSB(this)->gammas();
     SkASSERT(gammas);
-    return gammas->fRedData == gammas->fGreenData && gammas->fGreenData == gammas->fBlueData;
+    return gammas->fRedType == gammas->fGreenType && gammas->fGreenType == gammas->fBlueType &&
+           gammas->fRedData == gammas->fGreenData && gammas->fGreenData == gammas->fBlueData;
 }
 
 bool SkColorSpace::gammasAreNamed() const {
diff --git a/src/core/SkColorSpaceXform.cpp b/src/core/SkColorSpaceXform.cpp
index 4fbfa6cd9e..4a7f175082 100644
--- a/src/core/SkColorSpaceXform.cpp
+++ b/src/core/SkColorSpaceXform.cpp
@@ -404,7 +404,7 @@ static void build_gamma_tables(const T* outGammaTables[3], T* gammaTableStorage,
                     // share the same table pointer.  This should almost always be true.
                     // I've never seen a profile where all three gamma curves didn't match.
                     // But it is possible that they won't.
-                    if (gammas->data(0) == gammas->data(i)) {
+                    if (gammas->type(0) == gammas->type(i) && gammas->data(0) == gammas->data(i)) {
                         outGammaTables[i] = outGammaTables[0];
                         continue;
                     }
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 6cfbb8c837..f4507732b5 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -70,38 +70,57 @@ struct SkGammas : SkRefCnt {
     };
 
     bool isNamed(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kNamed_Type;
+        return Type::kNamed_Type == this->type(i);
     }
 
     bool isValue(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kValue_Type;
+        return Type::kValue_Type == this->type(i);
     }
 
     bool isTable(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kTable_Type;
+        return Type::kTable_Type == this->type(i);
     }
 
     bool isParametric(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kParam_Type;
+        return Type::kParam_Type == this->type(i);
     }
 
     const Data& data(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedData)[i];
+        switch (i) {
+            case 0:
+                return fRedData;
+            case 1:
+                return fGreenData;
+            case 2:
+                return fBlueData;
+            default:
+                SkASSERT(false);
+                return fRedData;
+        }
     }
 
     const float* table(int i) const {
         SkASSERT(isTable(i));
-        return (&fRedData)[i].fTable.table(this);
+        return this->data(i).fTable.table(this);
     }
 
     const Params& params(int i) const {
         SkASSERT(isParametric(i));
-        return (&fRedData)[i].params(this);
+        return this->data(i).params(this);
+    }
+
+    Type type(int i) const {
+        switch (i) {
+            case 0:
+                return fRedType;
+            case 1:
+                return fGreenType;
+            case 2:
+                return fBlueType;
+            default:
+                SkASSERT(false);
+                return fRedType;
+        }
     }
 
     SkGammas()
diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index f9593d91f9..62a898c2f7 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -125,7 +125,7 @@ DEF_TEST(ColorSpaceWriteICC, r) {
 
     // Test saving the original ICC data
     sk_sp<SkData> monitorData = SkData::MakeFromFileName(
-            GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+            GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
     REPORTER_ASSERT(r, monitorData);
     if (!monitorData) {
         return;
@@ -211,7 +211,13 @@ DEF_TEST(ColorSpace_Serialize, r) {
     test_serialize(r, SkColorSpace::NewNamed(SkColorSpace::kAdobeRGB_Named).get(), true);
 
     sk_sp<SkData> monitorData = SkData::MakeFromFileName(
-            GetResourcePath("monitor_profiles/HP_ZR30w.icc").c_str());
+            GetResourcePath("icc_profiles/HP_ZR30w.icc").c_str());
+    test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+    monitorData = SkData::MakeFromFileName( GetResourcePath("icc_profiles/HP_Z32x.icc").c_str());
+    test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+    monitorData = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperLeft.icc").c_str());
+    test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
+    monitorData = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str());
     test_serialize(r, SkColorSpace::NewICC(monitorData->data(), monitorData->size()).get(), false);
 }