Fix Equals and serialization for rare pngs

PNGs may contain a gAMA chunk that specifies gamma
values as floats.  If so, we will use these floats
to create an SkColorSpace.

This CL fixes Equals(), serialize(), and
Deserialize() to correctly handle SkColorSpaces
with strange gammas, where we are unable to fall
back on the profile data.

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

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

3 files changed, 90 insertions, 47 deletions

diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index cb5c8695ee..3a5d9c196a 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -68,7 +68,7 @@ static bool xyz_almost_equal(const SkMatrix44& toXYZD50, const float* standard)
            color_space_almost_equal(toXYZD50.getFloat(3, 3), 1.0f);
 }
 
-sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(float values[3], const SkMatrix44& toXYZD50) {
+sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(const float values[3], const SkMatrix44& toXYZD50) {
     if (0.0f > values[0] || 0.0f > values[1] || 0.0f > values[2]) {
         return nullptr;
     }
@@ -164,17 +164,25 @@ enum Version {
 struct ColorSpaceHeader {
     /**
      *  If kMatrix_Flag is set, we will write 12 floats after the header.
-     *  Should not be set at the same time as the kICC_Flag.
+     *  Should not be set at the same time as the kICC_Flag or kFloatGamma_Flag.
      */
-    static constexpr uint8_t kMatrix_Flag = 1 << 0;
+    static constexpr uint8_t kMatrix_Flag     = 1 << 0;
 
     /**
      *  If kICC_Flag is set, we will write an ICC profile after the header.
      *  The ICC profile will be written as a uint32 size, followed immediately
      *  by the data (padded to 4 bytes).
-     *  Should not be set at the same time as the kMatrix_Flag.
+     *  Should not be set at the same time as the kMatrix_Flag or kFloatGamma_Flag.
      */
-    static constexpr uint8_t kICC_Flag    = 1 << 1;
+    static constexpr uint8_t kICC_Flag        = 1 << 1;
+
+    /**
+     *  If kFloatGamma_Flag is set, we will write 15 floats after the header.
+     *  The first three are the gamma values, and the next twelve are the
+     *  matrix.
+     *  Should not be set at the same time as the kICC_Flag or kMatrix_Flag.
+     */
+    static constexpr uint8_t kFloatGamma_Flag = 1 << 2;
 
     static ColorSpaceHeader Pack(Version version, SkColorSpace::Named named,
                                  SkColorSpace::GammaNamed gammaNamed, uint8_t flags) {
@@ -189,7 +197,7 @@ struct ColorSpaceHeader {
         SkASSERT(gammaNamed <= SkColorSpace::kNonStandard_GammaNamed);
         header.fGammaNamed = (uint8_t) gammaNamed;
 
-        SkASSERT(flags <= kICC_Flag);
+        SkASSERT(flags <= kFloatGamma_Flag);
         header.fFlags = flags;
         return header;
     }
@@ -233,8 +241,26 @@ size_t SkColorSpace::writeToMemory(void* memory) const {
                 return sizeof(ColorSpaceHeader) + 12 * sizeof(float);
             }
             default:
-                SkASSERT(false);
-                return 0;
+                // Otherwise, write the gamma values and the matrix.
+                if (memory) {
+                    *((ColorSpaceHeader*) memory) =
+                            ColorSpaceHeader::Pack(k0_Version, fNamed, fGammaNamed,
+                                                   ColorSpaceHeader::kFloatGamma_Flag);
+                    memory = SkTAddOffset<void>(memory, sizeof(ColorSpaceHeader));
+
+                    const SkGammas* gammas = as_CSB(this)->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));
+
+                    fToXYZD50.as4x3ColMajorf((float*) memory);
+                }
+                return sizeof(ColorSpaceHeader) + 15 * sizeof(float);
         }
     }
 
@@ -302,18 +328,39 @@ sk_sp<SkColorSpace> SkColorSpace::Deserialize(const void* data, size_t length) {
             break;
     }
 
-    if (ColorSpaceHeader::kICC_Flag != header.fFlags || length < sizeof(uint32_t)) {
-        return nullptr;
-    }
+    switch (header.fFlags) {
+        case ColorSpaceHeader::kICC_Flag: {
+            if (length < sizeof(uint32_t)) {
+                return nullptr;
+            }
 
-    uint32_t profileSize = *((uint32_t*) data);
-    data = SkTAddOffset<const void>(data, sizeof(uint32_t));
-    length -= sizeof(uint32_t);
-    if (length < profileSize) {
-        return nullptr;
-    }
+            uint32_t profileSize = *((uint32_t*) data);
+            data = SkTAddOffset<const void>(data, sizeof(uint32_t));
+            length -= sizeof(uint32_t);
+            if (length < profileSize) {
+                return nullptr;
+            }
+
+            return NewICC(data, profileSize);
+        }
+        case ColorSpaceHeader::kFloatGamma_Flag: {
+            if (length < 15 * sizeof(float)) {
+                return nullptr;
+            }
 
-    return NewICC(data, profileSize);
+            float gammas[3];
+            gammas[0] = *(((const float*) data) + 0);
+            gammas[1] = *(((const float*) data) + 1);
+            gammas[2] = *(((const float*) data) + 2);
+            data = SkTAddOffset<const void>(data, 3 * sizeof(float));
+
+            SkMatrix44 toXYZ(SkMatrix44::kUninitialized_Constructor);
+            toXYZ.set4x3ColMajorf((const float*) data);
+            return SkColorSpace_Base::NewRGB(gammas, toXYZ);
+        }
+        default:
+            return nullptr;
+    }
 }
 
 bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
@@ -355,11 +402,15 @@ bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
         case kLinear_GammaNamed:
             return (src->fGammaNamed == dst->fGammaNamed) && (src->fToXYZD50 == dst->fToXYZD50);
         default:
-            // If |src| does not have a named gamma, fProfileData should be non-null.
-            // FIXME (msarett): We may hit this case on pngs that specify float gammas.
-            //                  Gamma can be non-standard, but we don't have a profile
-            //                  to fall back on.  What do we do?
-            return false;
+            if (src->fGammaNamed != dst->fGammaNamed) {
+                return false;
+            }
+
+            // 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());
     }
 }
 
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 3a2518db9c..38e0ba0026 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -174,7 +174,7 @@ struct SkColorLookUpTable : public SkRefCnt {
 class SkColorSpace_Base : public SkColorSpace {
 public:
 
-    static sk_sp<SkColorSpace> NewRGB(float gammas[3], const SkMatrix44& toXYZD50);
+    static sk_sp<SkColorSpace> NewRGB(const float gammas[3], const SkMatrix44& toXYZD50);
 
     const SkGammas* gammas() const { return fGammas.get(); }
 
diff --git a/tests/ColorSpaceTest.cpp b/tests/ColorSpaceTest.cpp
index 66860da590..abc02b74d8 100644
--- a/tests/ColorSpaceTest.cpp
+++ b/tests/ColorSpaceTest.cpp
@@ -173,25 +173,6 @@ DEF_TEST(ColorSpace_Named, r) {
     REPORTER_ASSERT(r, info.gammaCloseToSRGB());
 }
 
-static void matrix_equals(skiatest::Reporter* r, SkColorSpace* space1, SkColorSpace* space2) {
-    REPORTER_ASSERT(r, space1->xyz().getFloat(0, 0) == space2->xyz().getFloat(0, 0));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(0, 1) == space2->xyz().getFloat(0, 1));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(0, 2) == space2->xyz().getFloat(0, 2));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(0, 3) == space2->xyz().getFloat(0, 3));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(1, 0) == space2->xyz().getFloat(1, 0));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(1, 1) == space2->xyz().getFloat(1, 1));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(1, 2) == space2->xyz().getFloat(1, 2));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(1, 3) == space2->xyz().getFloat(1, 3));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(2, 0) == space2->xyz().getFloat(2, 0));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(2, 1) == space2->xyz().getFloat(2, 1));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(2, 2) == space2->xyz().getFloat(2, 2));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(2, 3) == space2->xyz().getFloat(2, 3));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(3, 0) == space2->xyz().getFloat(3, 0));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(3, 1) == space2->xyz().getFloat(3, 1));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(3, 2) == space2->xyz().getFloat(3, 2));
-    REPORTER_ASSERT(r, space1->xyz().getFloat(3, 3) == space2->xyz().getFloat(3, 3));
-}
-
 static void test_serialize(skiatest::Reporter* r, SkColorSpace* space, bool isNamed) {
     sk_sp<SkData> data1 = space->serialize();
 
@@ -206,10 +187,8 @@ static void test_serialize(skiatest::Reporter* r, SkColorSpace* space, bool isNa
         REPORTER_ASSERT(r, space == newSpace1.get());
         REPORTER_ASSERT(r, space == newSpace2.get());
     } else {
-        REPORTER_ASSERT(r, space->gammaNamed() == newSpace1->gammaNamed());
-        REPORTER_ASSERT(r, space->gammaNamed() == newSpace2->gammaNamed());
-        matrix_equals(r, space, newSpace1.get());
-        matrix_equals(r, space, newSpace2.get());
+        REPORTER_ASSERT(r, SkColorSpace::Equals(space, newSpace1.get()));
+        REPORTER_ASSERT(r, SkColorSpace::Equals(space, newSpace2.get()));
     }
 }
 
@@ -226,6 +205,10 @@ DEF_TEST(ColorSpace_Serialize, r) {
     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);
+
+    const float gammas[] = { 1.1f, 1.2f, 1.7f, };
+    SkMatrix44 toXYZ(SkMatrix44::kIdentity_Constructor);
+    test_serialize(r, SkColorSpace_Base::NewRGB(gammas, toXYZ).get(), false);
 }
 
 DEF_TEST(ColorSpace_Equals, r) {
@@ -240,6 +223,12 @@ DEF_TEST(ColorSpace_Equals, r) {
     sk_sp<SkColorSpace> upperLeft = SkColorSpace::NewICC(data->data(), data->size());
     data = SkData::MakeFromFileName(GetResourcePath("icc_profiles/upperRight.icc").c_str());
     sk_sp<SkColorSpace> upperRight = SkColorSpace::NewICC(data->data(), data->size());
+    const float gammas1[] = { 1.1f, 1.2f, 1.3f, };
+    const float gammas2[] = { 1.1f, 1.2f, 1.7f, };
+    SkMatrix44 toXYZ(SkMatrix44::kIdentity_Constructor);
+    sk_sp<SkColorSpace> rgb1 = SkColorSpace_Base::NewRGB(gammas1, toXYZ);
+    sk_sp<SkColorSpace> rgb2 = SkColorSpace_Base::NewRGB(gammas2, toXYZ);
+    sk_sp<SkColorSpace> rgb3 = SkColorSpace_Base::NewRGB(gammas1, toXYZ);
 
     REPORTER_ASSERT(r, SkColorSpace::Equals(nullptr, nullptr));
     REPORTER_ASSERT(r, SkColorSpace::Equals(srgb.get(), srgb.get()));
@@ -248,6 +237,8 @@ DEF_TEST(ColorSpace_Equals, r) {
     REPORTER_ASSERT(r, SkColorSpace::Equals(z32.get(), z32.get()));
     REPORTER_ASSERT(r, SkColorSpace::Equals(upperLeft.get(), upperLeft.get()));
     REPORTER_ASSERT(r, SkColorSpace::Equals(upperRight.get(), upperRight.get()));
+    REPORTER_ASSERT(r, SkColorSpace::Equals(rgb1.get(), rgb1.get()));
+    REPORTER_ASSERT(r, SkColorSpace::Equals(rgb1.get(), rgb3.get()));
 
     REPORTER_ASSERT(r, !SkColorSpace::Equals(nullptr, srgb.get()));
     REPORTER_ASSERT(r, !SkColorSpace::Equals(srgb.get(), nullptr));
@@ -258,4 +249,5 @@ DEF_TEST(ColorSpace_Equals, r) {
     REPORTER_ASSERT(r, !SkColorSpace::Equals(upperLeft.get(), upperRight.get()));
     REPORTER_ASSERT(r, !SkColorSpace::Equals(z30.get(), upperRight.get()));
     REPORTER_ASSERT(r, !SkColorSpace::Equals(upperRight.get(), adobe.get()));
+    REPORTER_ASSERT(r, !SkColorSpace::Equals(rgb1.get(), rgb2.get()));
 }