make SkGammas less bad

Data's existing operator== is terribly broken.
Instead replace it with what everyone really
wants to know, allChannelsSame().

I'm not quite ready to get rid of SkGammas yet,
but we're getting close.

Bug: chromium:799834

Change-Id: Ibcfd252e380b15fba53eb707f58304a50e90ce9f
Reviewed-on: https://skia-review.googlesource.com/100320
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Mike Klein <mtklein@chromium.org>

src/core/SkColorSpaceXform.cpp

@@ -337,14 +337,9 @@ static inline int num_tables(SkColorSpace_XYZ* space) {
             const SkGammas* gammas = space->gammas();
             SkASSERT(gammas);
 
-            bool gammasAreMatching = (gammas->type(0) == gammas->type(1)) &&
-                                     (gammas->data(0) == gammas->data(1)) &&
-                                     (gammas->type(0) == gammas->type(2)) &&
-                                     (gammas->data(0) == gammas->data(2));
-
             // It's likely that each component will have the same gamma.  In this case,
             // we only need to build one table.
-            return gammasAreMatching ? 1 : 3;
+            return gammas->allChannelsSame() ? 1 : 3;
         }
     }
 }

src/core/SkColorSpace_XYZ.cpp

@@ -65,7 +65,7 @@ bool SkColorSpace_XYZ::onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) c
     }
 
     SkASSERT(fGammas);
-    if (fGammas->data(0) != fGammas->data(1) || fGammas->data(0) != fGammas->data(2)) {
+    if (!fGammas->allChannelsSame()) {
         return false;
     }
 

src/core/SkGammas.h

@@ -16,7 +16,7 @@ struct SkGammas : SkRefCnt {
 
     // There are four possible representations for gamma curves.  kNone_Type is used
     // as a placeholder until the struct is initialized.  It is not a valid value.
-    enum class Type : uint8_t {
+    enum class Type {
         kNone_Type,
         kNamed_Type,
         kValue_Type,
@@ -37,18 +37,7 @@ struct SkGammas : SkRefCnt {
     // Contains the actual gamma curve information.  Should be interpreted
     // based on the type of the gamma curve.
     union Data {
-        Data()
+        Data() : fTable{0, 0} {}
-            : fTable{ 0, 0 }
-        {}
-
-        inline bool operator==(const Data& that) const {
-            return this->fTable.fOffset == that.fTable.fOffset &&
-                   this->fTable.fSize == that.fTable.fSize;
-        }
-
-        inline bool operator!=(const Data& that) const {
-            return !(*this == that);
-        }
 
         SkGammaNamed fNamed;
         float        fValue;
@@ -56,26 +45,47 @@ struct SkGammas : SkRefCnt {
         size_t       fParamOffset;
 
         const SkColorSpaceTransferFn& params(const SkGammas* base) const {
-            return *SkTAddOffset<const SkColorSpaceTransferFn>(
+            return *SkTAddOffset<const SkColorSpaceTransferFn>(base,
-                    base, sizeof(SkGammas) + fParamOffset);
+                                                               sizeof(SkGammas) + fParamOffset);
         }
     };
 
-    bool isNamed(int i) const {
+    bool allChannelsSame() const {
-        return Type::kNamed_Type == this->type(i);
+        // All channels are the same type?
+        Type type = this->type(0);
+        for (int i = 1; i < this->channels(); i++) {
+            if (type != this->type(i)) {
+                return false;
+            }
         }
 
-    bool isValue(int i) const {
+        // All data the same?
-        return Type::kValue_Type == this->type(i);
+        auto& first = this->data(0);
+        for (int i = 1; i < this->channels(); i++) {
+            auto& data = this->data(i);
+            switch (type) {
+                case Type:: kNone_Type:                                                    break;
+                case Type::kNamed_Type: if (first.fNamed != data.fNamed) { return false; } break;
+                case Type::kValue_Type: if (first.fValue != data.fValue) { return false; } break;
+                case Type::kTable_Type:
+                    if (first.fTable.fOffset != data.fTable.fOffset) { return false; }
+                    if (first.fTable.fSize   != data.fTable.fSize  ) { return false; }
+                    break;
+                case Type::kParam_Type:
+                    if (0 != memcmp(&first.params(this), &data.params(this),
+                                    sizeof(SkColorSpaceTransferFn))) {
+                        return false;
+                    }
+                    break;
+            }
+        }
+        return true;
     }
 
-    bool isTable(int i) const {
+    bool isNamed     (int i) const { return Type::kNamed_Type == this->type(i); }
-        return Type::kTable_Type == this->type(i);
+    bool isValue     (int i) const { return Type::kValue_Type == this->type(i); }
-    }
+    bool isTable     (int i) const { return Type::kTable_Type == this->type(i); }
-
+    bool isParametric(int i) const { return Type::kParam_Type == this->type(i); }
-    bool isParametric(int i) const {
-        return Type::kParam_Type == this->type(i);
-    }
 
     const Data& data(int i) const {
         SkASSERT(i >= 0 && i < fChannels);
@@ -83,17 +93,17 @@ struct SkGammas : SkRefCnt {
     }
 
     const float* table(int i) const {
-        SkASSERT(isTable(i));
+        SkASSERT(this->isTable(i));
         return this->data(i).fTable.table(this);
     }
 
     int tableSize(int i) const {
-        SkASSERT(isTable(i));
+        SkASSERT(this->isTable(i));
         return this->data(i).fTable.fSize;
     }
 
     const SkColorSpaceTransferFn& params(int i) const {
-        SkASSERT(isParametric(i));
+        SkASSERT(this->isParametric(i));
         return this->data(i).params(this);
     }
 
@@ -102,17 +112,17 @@ struct SkGammas : SkRefCnt {
         return fType[i];
     }
 
-    uint8_t channels() const { return fChannels; }
+    int channels() const { return fChannels; }
 
-    SkGammas(uint8_t channels) : fChannels(channels) {
+    SkGammas(int channels) : fChannels(channels) {
-        SkASSERT(channels <= SK_ARRAY_COUNT(fType));
+        SkASSERT(channels <= (int)SK_ARRAY_COUNT(fType));
         for (Type& t : fType) {
             t = Type::kNone_Type;
         }
     }
 
     // These fields should only be modified when initializing the struct.
-    uint8_t fChannels;
+    int  fChannels;
     Data fData[4];
     Type fType[4];
 

src/core/SkICC.cpp

@@ -77,7 +77,7 @@ bool SkICC::rawTransferFnData(Tables* tables) const {
 
     const SkGammas* gammas = colorSpace->gammas();
     SkASSERT(gammas);
-    if (gammas->data(0) == gammas->data(1) && gammas->data(0) == gammas->data(2)) {
+    if (gammas->allChannelsSame()) {
         SkASSERT(gammas->isTable(0));
         tables->fStorage = SkData::MakeUninitialized(gammas->tableSize(0) * sizeof(float));
         copy_to_table((float*) tables->fStorage->writable_data(), gammas, 0);