Fix Equals and serialization for rare pngs

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 "SkColorSpacePriv.h"
@@ skipping 50 matching lines @@
            color_space_almost_equal(toXYZD50.getFloat(2, 2), standard[8]) &&
            color_space_almost_equal(toXYZD50.getFloat(0, 3), 0.0f) &&
            color_space_almost_equal(toXYZD50.getFloat(1, 3), 0.0f) &&
            color_space_almost_equal(toXYZD50.getFloat(2, 3), 0.0f) &&
            color_space_almost_equal(toXYZD50.getFloat(3, 0), 0.0f) &&
            color_space_almost_equal(toXYZD50.getFloat(3, 1), 0.0f) &&
            color_space_almost_equal(toXYZD50.getFloat(3, 2), 0.0f) &&
            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;
     }
 
     GammaNamed gammaNamed = kNonStandard_GammaNamed;
     if (color_space_almost_equal(2.2f, values[0]) &&
             color_space_almost_equal(2.2f, values[1]) &&
             color_space_almost_equal(2.2f, values[2])) {
         gammaNamed = k2Dot2Curve_GammaNamed;
     } else if (color_space_almost_equal(1.0f, values[0]) &&
@@ skipping 75 matching lines @@
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 enum Version {
     k0_Version, // Initial version, header + flags for matrix and profile
 };
 
 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) {
         ColorSpaceHeader header;
 
         SkASSERT(k0_Version == version);
         header.fVersion = (uint8_t) version;
 
         SkASSERT(named <= SkColorSpace::kAdobeRGB_Named);
         header.fNamed = (uint8_t) named;
 
         SkASSERT(gammaNamed <= SkColorSpace::kNonStandard_GammaNamed);
         header.fGammaNamed = (uint8_t) gammaNamed;
 
-        SkASSERT(flags <= kICC_Flag);
+        SkASSERT(flags <= kFloatGamma_Flag);
         header.fFlags = flags;
         return header;
     }
 
     uint8_t fVersion;    // Always zero
     uint8_t fNamed;      // Must be a SkColorSpace::Named
     uint8_t fGammaNamed; // Must be a SkColorSpace::GammaNamed
     uint8_t fFlags;      // Some combination of the flags listed above
 };
 
@@ skipping 23 matching lines @@
                 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);
             }
             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);
         }
     }
 
     // Otherwise, serialize the ICC data.
     size_t profileSize = as_CSB(this)->fProfileData->size();
     if (SkAlign4(profileSize) != (uint32_t) SkAlign4(profileSize)) {
         return 0;
     }
 
     if (memory) {
@@ skipping 47 matching lines @@
             }
 
             SkMatrix44 toXYZ(SkMatrix44::kUninitialized_Constructor);
             toXYZ.set4x3ColMajorf((const float*) data);
             return NewRGB((GammaNamed) header.fGammaNamed, toXYZ);
         }
         default:
             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;
+            }
+
+            return NewICC(data, profileSize);
+        }
+        case ColorSpaceHeader::kFloatGamma_Flag: {
+            if (ColorSpaceHeader::kFloatGamma_Flag != header.fFlags || length < 15 * sizeof(float))

[Brian Osman, 2016/08/22 14:00:02]

The first condition on the 'if' is strange - we're in a case block that just
ensured that header.fFlags will always be kFloatGamma.

[msarett, 2016/08/22 14:10:24]

Yeah thanks...  Removing that condition.

+            {
+                return nullptr;
+            }
+
+            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;
     }
-
-    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);
 }
 
 bool SkColorSpace::Equals(const SkColorSpace* src, const SkColorSpace* dst) {
     if (src == dst) {
         return true;
     }
 
     if (!src || !dst) {
         return false;
     }
@@ skipping 21 matching lines @@
     }
 
     // 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 (src->fGammaNamed) {
         case kSRGB_GammaNamed:
         case k2Dot2Curve_GammaNamed:
         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());
     }
 }
 
 bool SkColorSpace::gammasAreMatching() const {
     const SkGammas* gammas = as_CSB(this)->gammas();
     SkASSERT(gammas);
     return gammas->fRedType == gammas->fGreenType && gammas->fGreenType == gammas->fBlueType &&
            gammas->fRedData == gammas->fGreenData && gammas->fGreenData == gammas->fBlueData;
 }
 
@@ skipping 21 matching lines @@
            gammas->fBlueType == SkGammas::Type::kTable_Type;
 }
 
 bool SkColorSpace::gammasAreParams() const {
     const SkGammas* gammas = as_CSB(this)->gammas();
     SkASSERT(gammas);
     return gammas->fRedType == SkGammas::Type::kParam_Type &&
            gammas->fGreenType == SkGammas::Type::kParam_Type &&
            gammas->fBlueType == SkGammas::Type::kParam_Type;
 }