Revert of Refactor parsing and storage of SkGammas (patchset #8 id:280001 of https...
authormsarett <msarett@google.com>
Wed, 20 Jul 2016 23:14:16 +0000 (16:14 -0700)
committerCommit bot <commit-bot@chromium.org>
Wed, 20 Jul 2016 23:14:17 +0000 (16:14 -0700)
Reason for revert:
Tests failing

Original issue's description:
> Refactor parsing and storage of SkGammas
>
> Benefits:
> (1) Parses and stores gamma tags in a single allocation.
> (2) Recognizes equal gamma tags to skip parsing work and
>     save memory.
>
> Non-Benefits:
> (1) Not less complicated.
>
> BUG=skia:
> GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2117773002
>
> Committed: https://skia.googlesource.com/skia/+/2ea944c2b710caf29d4795ac953bad14224796f7
> Committed: https://skia.googlesource.com/skia/+/959ccc1f3f49e1ddeb51c32c30ac4a2d94653856

TBR=reed@google.com,brianosman@google.com,mtklein@google.com
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:

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

src/core/SkColorSpace.cpp
src/core/SkColorSpacePriv.h
src/core/SkColorSpaceXform.cpp
src/core/SkColorSpaceXform.h
src/core/SkColorSpace_Base.h
src/core/SkColorSpace_ICC.cpp
tests/ColorSpaceXformTest.cpp

index ba278df..894c4b9 100644 (file)
@@ -22,10 +22,9 @@ SkColorSpace_Base::SkColorSpace_Base(GammaNamed gammaNamed, const SkMatrix44& to
     , fProfileData(nullptr)
 {}
 
-SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, GammaNamed gammaNamed,
-                                     sk_sp<SkGammas> gammas, const SkMatrix44& toXYZD50,
-                                     sk_sp<SkData> profileData)
-    : INHERITED(gammaNamed, toXYZD50, kUnknown_Named)
+SkColorSpace_Base::SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, sk_sp<SkGammas> gammas,
+                                     const SkMatrix44& toXYZD50, sk_sp<SkData> profileData)
+    : INHERITED(kNonStandard_GammaNamed, toXYZD50, kUnknown_Named)
     , fColorLUT(std::move(colorLUT))
     , fGammas(std::move(gammas))
     , fProfileData(std::move(profileData))
@@ -69,31 +68,16 @@ static bool xyz_almost_equal(const SkMatrix44& toXYZD50, const float* standard)
 }
 
 sk_sp<SkColorSpace> SkColorSpace_Base::NewRGB(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]) &&
-            color_space_almost_equal(1.0f, values[1]) &&
-            color_space_almost_equal(1.0f, values[2])) {
-        gammaNamed = kLinear_GammaNamed;
-    }
+    SkGammaCurve curves[3];
+    set_gamma_value(&curves[0], values[0]);
+    set_gamma_value(&curves[1], values[1]);
+    set_gamma_value(&curves[2], values[2]);
 
+    GammaNamed gammaNamed = SkGammas::Named(curves);
     if (kNonStandard_GammaNamed == gammaNamed) {
-        sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new SkGammas());
-        gammas->fRedType = SkGammas::Type::kValue_Type;
-        gammas->fGreenType = SkGammas::Type::kValue_Type;
-        gammas->fBlueType = SkGammas::Type::kValue_Type;
-        gammas->fRedData.fValue = values[0];
-        gammas->fGreenData.fValue = values[1];
-        gammas->fBlueData.fValue = values[2];
-        return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, kNonStandard_GammaNamed, gammas,
-                                                         toXYZD50, nullptr));
+        sk_sp<SkGammas> gammas(new SkGammas(std::move(curves[0]), std::move(curves[1]),
+                                            std::move(curves[2])));
+        return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, gammas, toXYZD50, nullptr));
     }
 
     return SkColorSpace_Base::NewRGB(gammaNamed, toXYZD50);
index e7c8aaa..7c7b9d0 100644 (file)
 inline bool color_space_almost_equal(float a, float b) {
     return SkTAbs(a - b) < 0.01f;
 }
+
+inline void set_gamma_value(SkGammaCurve* gamma, float value) {
+    if (color_space_almost_equal(2.2f, value)) {
+        gamma->fNamed = SkColorSpace::k2Dot2Curve_GammaNamed;
+    } else if (color_space_almost_equal(1.0f, value)) {
+        gamma->fNamed = SkColorSpace::kLinear_GammaNamed;
+    } else if (color_space_almost_equal(0.0f, value)) {
+        SkColorSpacePrintf("Treating invalid zero gamma as linear.");
+        gamma->fNamed = SkColorSpace::kLinear_GammaNamed;
+    } else {
+        gamma->fValue = value;
+    }
+}
index 1b5d7e6..8b23d18 100644 (file)
@@ -264,11 +264,11 @@ static uint8_t clamp_normalized_float_to_byte(float v) {
     }
 }
 
-static void build_table_linear_to_gamma(uint8_t* outTable, float exponent) {
+static void build_table_linear_to_gamma(uint8_t* outTable, int outTableSize, float exponent) {
     float toGammaExp = 1.0f / exponent;
 
-    for (int i = 0; i < SkDefaultXform::kDstGammaTableSize; i++) {
-        float x = ((float) i) * (1.0f / ((float) (SkDefaultXform::kDstGammaTableSize - 1)));
+    for (int i = 0; i < outTableSize; i++) {
+        float x = ((float) i) * (1.0f / ((float) (outTableSize - 1)));
         outTable[i] = clamp_normalized_float_to_byte(powf(x, toGammaExp));
     }
 }
@@ -276,7 +276,7 @@ static void build_table_linear_to_gamma(uint8_t* outTable, float exponent) {
 // Inverse table lookup.  Ex: what index corresponds to the input value?  This will
 // have strange results when the table is non-increasing.  But any sane gamma
 // function will be increasing.
-static float inverse_interp_lut(float input, const float* table, int tableSize) {
+static float inverse_interp_lut(float input, float* table, int tableSize) {
     if (input <= table[0]) {
         return table[0];
     } else if (input >= table[tableSize - 1]) {
@@ -299,10 +299,10 @@ static float inverse_interp_lut(float input, const float* table, int tableSize)
     return 0.0f;
 }
 
-static void build_table_linear_to_gamma(uint8_t* outTable, const float* inTable,
+static void build_table_linear_to_gamma(uint8_t* outTable, int outTableSize, float* inTable,
                                         int inTableSize) {
-    for (int i = 0; i < SkDefaultXform::kDstGammaTableSize; i++) {
-        float x = ((float) i) * (1.0f / ((float) (SkDefaultXform::kDstGammaTableSize - 1)));
+    for (int i = 0; i < outTableSize; i++) {
+        float x = ((float) i) * (1.0f / ((float) (outTableSize - 1)));
         float y = inverse_interp_lut(x, inTable, inTableSize);
         outTable[i] = clamp_normalized_float_to_byte(y);
     }
@@ -339,10 +339,10 @@ static float inverse_parametric(float x, float g, float a, float b, float c, flo
     return (powf(x - c, 1.0f / g) - b) / a;
 }
 
-static void build_table_linear_to_gamma(uint8_t* outTable, float g, float a,
+static void build_table_linear_to_gamma(uint8_t* outTable, int outTableSize, float g, float a,
                                         float b, float c, float d, float e, float f) {
-    for (int i = 0; i < SkDefaultXform::kDstGammaTableSize; i++) {
-        float x = ((float) i) * (1.0f / ((float) (SkDefaultXform::kDstGammaTableSize - 1)));
+    for (int i = 0; i < outTableSize; i++) {
+        float x = ((float) i) * (1.0f / ((float) (outTableSize - 1)));
         float y = inverse_parametric(x, g, a, b, c, d, e, f);
         outTable[i] = clamp_normalized_float_to_byte(y);
     }
@@ -350,102 +350,6 @@ static void build_table_linear_to_gamma(uint8_t* outTable, float g, float a,
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
-template <typename T>
-struct GammaFns {
-    const T* fSRGBTable;
-    const T* f2Dot2Table;
-
-    void (*fBuildFromValue)(T*, float);
-    void (*fBuildFromTable)(T*, const float*, int);
-    void (*fBuildFromParam)(T*, float, float, float, float, float, float, float);
-};
-
-static const GammaFns<float> kToLinear {
-    sk_linear_from_srgb,
-    sk_linear_from_2dot2,
-    &build_table_linear_from_gamma,
-    &build_table_linear_from_gamma,
-    &build_table_linear_from_gamma,
-};
-
-static const GammaFns<uint8_t> kFromLinear {
-    linear_to_srgb,
-    linear_to_2dot2,
-    &build_table_linear_to_gamma,
-    &build_table_linear_to_gamma,
-    &build_table_linear_to_gamma,
-};
-
-// Build tables to transform src gamma to linear.
-template <typename T>
-static void build_gamma_tables(const T* outGammaTables[3], T* gammaTableStorage, int gammaTableSize,
-                               const sk_sp<SkColorSpace>& space, const GammaFns<T>& fns) {
-    switch (space->gammaNamed()) {
-        case SkColorSpace::kSRGB_GammaNamed:
-            outGammaTables[0] = outGammaTables[1] = outGammaTables[2] = fns.fSRGBTable;
-            break;
-        case SkColorSpace::k2Dot2Curve_GammaNamed:
-            outGammaTables[0] = outGammaTables[1] = outGammaTables[2] = fns.f2Dot2Table;
-            break;
-        case SkColorSpace::kLinear_GammaNamed:
-            (*fns.fBuildFromValue)(gammaTableStorage, 1.0f);
-            outGammaTables[0] = outGammaTables[1] = outGammaTables[2] = gammaTableStorage;
-            break;
-        default: {
-            const SkGammas* gammas = as_CSB(space)->gammas();
-            SkASSERT(gammas);
-
-            for (int i = 0; i < 3; i++) {
-                if (i > 0) {
-                    // Check if this curve matches the first curve.  In this case, we can
-                    // 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)) {
-                        outGammaTables[i] = outGammaTables[0];
-                        continue;
-                    }
-                }
-
-                if (gammas->isNamed(i)) {
-                    switch (gammas->data(i).fNamed) {
-                        case SkColorSpace::kSRGB_GammaNamed:
-                            outGammaTables[i] = fns.fSRGBTable;
-                            break;
-                        case SkColorSpace::k2Dot2Curve_GammaNamed:
-                            outGammaTables[i] = fns.f2Dot2Table;
-                            break;
-                        case SkColorSpace::kLinear_GammaNamed:
-                            (*fns.fBuildFromValue)(&gammaTableStorage[i * gammaTableSize], 1.0f);
-                            outGammaTables[i] = &gammaTableStorage[i * gammaTableSize];
-                            break;
-                        default:
-                            SkASSERT(false);
-                            break;
-                    }
-                } else if (gammas->isValue(i)) {
-                    (*fns.fBuildFromValue)(&gammaTableStorage[i * gammaTableSize],
-                                           gammas->data(i).fValue);
-                    outGammaTables[i] = &gammaTableStorage[i * gammaTableSize];
-                } else if (gammas->isTable(i)) {
-                    (*fns.fBuildFromTable)(&gammaTableStorage[i * gammaTableSize], gammas->table(i),
-                                           gammas->data(i).fTable.fSize);
-                    outGammaTables[i] = &gammaTableStorage[i * gammaTableSize];
-                } else {
-                    SkASSERT(gammas->isParametric(i));
-                    const SkGammas::Params& params = gammas->params(i);
-                    (*fns.fBuildFromParam)(&gammaTableStorage[i * gammaTableSize], params.fG,
-                                           params.fA, params.fB, params.fC, params.fD, params.fE,
-                                           params.fF);
-                    outGammaTables[i] = &gammaTableStorage[i * gammaTableSize];
-                }
-            }
-        }
-    }
-}
-
-///////////////////////////////////////////////////////////////////////////////////////////////////
-
 std::unique_ptr<SkColorSpaceXform> SkColorSpaceXform::New(const sk_sp<SkColorSpace>& srcSpace,
                                                           const sk_sp<SkColorSpace>& dstSpace) {
     if (!srcSpace || !dstSpace) {
@@ -516,9 +420,150 @@ SkFastXform<Dst>::SkFastXform(const sk_sp<SkColorSpace>& srcSpace, const SkMatri
                               const sk_sp<SkColorSpace>& dstSpace)
 {
     build_src_to_dst(fSrcToDst, srcToDst);
-    build_gamma_tables(fSrcGammaTables, fSrcGammaTableStorage, 256, srcSpace, kToLinear);
-    build_gamma_tables(fDstGammaTables, fDstGammaTableStorage, SkDefaultXform::kDstGammaTableSize,
-                       dstSpace, kFromLinear);
+
+    // Build tables to transform src gamma to linear.
+    switch (srcSpace->gammaNamed()) {
+        case SkColorSpace::kSRGB_GammaNamed:
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = sk_linear_from_srgb;
+            break;
+        case SkColorSpace::k2Dot2Curve_GammaNamed:
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = sk_linear_from_2dot2;
+            break;
+        case SkColorSpace::kLinear_GammaNamed:
+            build_table_linear_from_gamma(fSrcGammaTableStorage, 1.0f);
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = fSrcGammaTableStorage;
+            break;
+        default: {
+            const SkGammas* gammas = as_CSB(srcSpace)->gammas();
+            SkASSERT(gammas);
+
+            for (int i = 0; i < 3; i++) {
+                const SkGammaCurve& curve = (*gammas)[i];
+
+                if (i > 0) {
+                    // Check if this curve matches the first curve.  In this case, we can
+                    // share the same table pointer.  Logically, 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.
+                    // TODO (msarett):
+                    // This comparison won't catch the case where each gamma curve has a
+                    // pointer to its own look-up table, but the tables actually match.
+                    // Should we perform a deep compare of gamma tables here?  Or should
+                    // we catch this when parsing the profile?  Or should we not worry
+                    // about a bit of redundant work?
+                    if (curve.quickEquals((*gammas)[0])) {
+                        fSrcGammaTables[i] = fSrcGammaTables[0];
+                        continue;
+                    }
+                }
+
+                if (curve.isNamed()) {
+                    switch (curve.fNamed) {
+                        case SkColorSpace::kSRGB_GammaNamed:
+                            fSrcGammaTables[i] = sk_linear_from_srgb;
+                            break;
+                        case SkColorSpace::k2Dot2Curve_GammaNamed:
+                            fSrcGammaTables[i] = sk_linear_from_2dot2;
+                            break;
+                        case SkColorSpace::kLinear_GammaNamed:
+                            build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], 1.0f);
+                            fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                            break;
+                        default:
+                            SkASSERT(false);
+                            break;
+                    }
+                } else if (curve.isValue()) {
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], curve.fValue);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                } else if (curve.isTable()) {
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256],
+                                                  curve.fTable.get(), curve.fTableSize);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                } else {
+                    SkASSERT(curve.isParametric());
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], curve.fG,
+                                                  curve.fA, curve.fB, curve.fC, curve.fD, curve.fE,
+                                                  curve.fF);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                }
+            }
+        }
+    }
+
+    // Build tables to transform linear to dst gamma.
+    // FIXME (msarett):
+    // Should we spend all of this time bulding the dst gamma tables when the client only
+    // wants to convert to F16?
+    switch (dstSpace->gammaNamed()) {
+        case SkColorSpace::kSRGB_GammaNamed:
+        case SkColorSpace::k2Dot2Curve_GammaNamed:
+            break;
+        case SkColorSpace::kLinear_GammaNamed:
+            build_table_linear_to_gamma(fDstGammaTableStorage, kDstGammaTableSize, 1.0f);
+            fDstGammaTables[0] = fDstGammaTables[1] = fDstGammaTables[2] = fDstGammaTableStorage;
+            break;
+        default: {
+            const SkGammas* gammas = as_CSB(dstSpace)->gammas();
+            SkASSERT(gammas);
+
+            for (int i = 0; i < 3; i++) {
+                const SkGammaCurve& curve = (*gammas)[i];
+
+                if (i > 0) {
+                    // Check if this curve matches the first curve.  In this case, we can
+                    // share the same table pointer.  Logically, 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.
+                    // TODO (msarett):
+                    // This comparison won't catch the case where each gamma curve has a
+                    // pointer to its own look-up table (but the tables actually match).
+                    // Should we perform a deep compare of gamma tables here?  Or should
+                    // we catch this when parsing the profile?  Or should we not worry
+                    // about a bit of redundant work?
+                    if (curve.quickEquals((*gammas)[0])) {
+                        fDstGammaTables[i] = fDstGammaTables[0];
+                        continue;
+                    }
+                }
+
+                if (curve.isNamed()) {
+                    switch (curve.fNamed) {
+                        case SkColorSpace::kSRGB_GammaNamed:
+                            fDstGammaTables[i] = linear_to_srgb;
+                            break;
+                        case SkColorSpace::k2Dot2Curve_GammaNamed:
+                            fDstGammaTables[i] = linear_to_2dot2;
+                            break;
+                        case SkColorSpace::kLinear_GammaNamed:
+                            build_table_linear_to_gamma(
+                                    &fDstGammaTableStorage[i * kDstGammaTableSize],
+                                    kDstGammaTableSize, 1.0f);
+                            fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                            break;
+                        default:
+                            SkASSERT(false);
+                            break;
+                    }
+                } else if (curve.isValue()) {
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fValue);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                } else if (curve.isTable()) {
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fTable.get(),
+                                                curve.fTableSize);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                } else {
+                    SkASSERT(curve.isParametric());
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fG, curve.fA, curve.fB,
+                                                curve.fC, curve.fD, curve.fE, curve.fF);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                }
+            }
+        }
+    }
 }
 
 template <>
@@ -556,9 +601,149 @@ SkDefaultXform::SkDefaultXform(const sk_sp<SkColorSpace>& srcSpace, const SkMatr
     : fColorLUT(sk_ref_sp((SkColorLookUpTable*) as_CSB(srcSpace)->colorLUT()))
     , fSrcToDst(srcToDst)
 {
-    build_gamma_tables(fSrcGammaTables, fSrcGammaTableStorage, 256, srcSpace, kToLinear);
-    build_gamma_tables(fDstGammaTables, fDstGammaTableStorage, SkDefaultXform::kDstGammaTableSize,
-                       dstSpace, kFromLinear);
+    // Build tables to transform src gamma to linear.
+    switch (srcSpace->gammaNamed()) {
+        case SkColorSpace::kSRGB_GammaNamed:
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = sk_linear_from_srgb;
+            break;
+        case SkColorSpace::k2Dot2Curve_GammaNamed:
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = sk_linear_from_2dot2;
+            break;
+        case SkColorSpace::kLinear_GammaNamed:
+            build_table_linear_from_gamma(fSrcGammaTableStorage, 1.0f);
+            fSrcGammaTables[0] = fSrcGammaTables[1] = fSrcGammaTables[2] = fSrcGammaTableStorage;
+            break;
+        default: {
+            const SkGammas* gammas = as_CSB(srcSpace)->gammas();
+            SkASSERT(gammas);
+
+            for (int i = 0; i < 3; i++) {
+                const SkGammaCurve& curve = (*gammas)[i];
+
+                if (i > 0) {
+                    // Check if this curve matches the first curve.  In this case, we can
+                    // share the same table pointer.  Logically, 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.
+                    // TODO (msarett):
+                    // This comparison won't catch the case where each gamma curve has a
+                    // pointer to its own look-up table, but the tables actually match.
+                    // Should we perform a deep compare of gamma tables here?  Or should
+                    // we catch this when parsing the profile?  Or should we not worry
+                    // about a bit of redundant work?
+                    if (curve.quickEquals((*gammas)[0])) {
+                        fSrcGammaTables[i] = fSrcGammaTables[0];
+                        continue;
+                    }
+                }
+
+                if (curve.isNamed()) {
+                    switch (curve.fNamed) {
+                        case SkColorSpace::kSRGB_GammaNamed:
+                            fSrcGammaTables[i] = sk_linear_from_srgb;
+                            break;
+                        case SkColorSpace::k2Dot2Curve_GammaNamed:
+                            fSrcGammaTables[i] = sk_linear_from_2dot2;
+                            break;
+                        case SkColorSpace::kLinear_GammaNamed:
+                            build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], 1.0f);
+                            fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                            break;
+                        default:
+                            SkASSERT(false);
+                            break;
+                    }
+                } else if (curve.isValue()) {
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], curve.fValue);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                } else if (curve.isTable()) {
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256],
+                                                  curve.fTable.get(), curve.fTableSize);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                } else {
+                    SkASSERT(curve.isParametric());
+                    build_table_linear_from_gamma(&fSrcGammaTableStorage[i * 256], curve.fG,
+                                                  curve.fA, curve.fB, curve.fC, curve.fD, curve.fE,
+                                                  curve.fF);
+                    fSrcGammaTables[i] = &fSrcGammaTableStorage[i * 256];
+                }
+            }
+        }
+    }
+
+    // Build tables to transform linear to dst gamma.
+    switch (dstSpace->gammaNamed()) {
+        case SkColorSpace::kSRGB_GammaNamed:
+            fDstGammaTables[0] = fDstGammaTables[1] = fDstGammaTables[2] = linear_to_srgb;
+            break;
+        case SkColorSpace::k2Dot2Curve_GammaNamed:
+            fDstGammaTables[0] = fDstGammaTables[1] = fDstGammaTables[2] = linear_to_2dot2;
+            break;
+        case SkColorSpace::kLinear_GammaNamed:
+            build_table_linear_to_gamma(fDstGammaTableStorage, kDstGammaTableSize, 1.0f);
+            fDstGammaTables[0] = fDstGammaTables[1] = fDstGammaTables[2] = fDstGammaTableStorage;
+            break;
+        default: {
+            const SkGammas* gammas = as_CSB(dstSpace)->gammas();
+            SkASSERT(gammas);
+
+            for (int i = 0; i < 3; i++) {
+                const SkGammaCurve& curve = (*gammas)[i];
+
+                if (i > 0) {
+                    // Check if this curve matches the first curve.  In this case, we can
+                    // share the same table pointer.  Logically, 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.
+                    // TODO (msarett):
+                    // This comparison won't catch the case where each gamma curve has a
+                    // pointer to its own look-up table (but the tables actually match).
+                    // Should we perform a deep compare of gamma tables here?  Or should
+                    // we catch this when parsing the profile?  Or should we not worry
+                    // about a bit of redundant work?
+                    if (curve.quickEquals((*gammas)[0])) {
+                        fDstGammaTables[i] = fDstGammaTables[0];
+                        continue;
+                    }
+                }
+
+                if (curve.isNamed()) {
+                    switch (curve.fNamed) {
+                        case SkColorSpace::kSRGB_GammaNamed:
+                            fDstGammaTables[i] = linear_to_srgb;
+                            break;
+                        case SkColorSpace::k2Dot2Curve_GammaNamed:
+                            fDstGammaTables[i] = linear_to_2dot2;
+                            break;
+                        case SkColorSpace::kLinear_GammaNamed:
+                            build_table_linear_to_gamma(
+                                    &fDstGammaTableStorage[i * kDstGammaTableSize],
+                                    kDstGammaTableSize, 1.0f);
+                            fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                            break;
+                        default:
+                            SkASSERT(false);
+                            break;
+                    }
+                } else if (curve.isValue()) {
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fValue);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                } else if (curve.isTable()) {
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fTable.get(),
+                                                curve.fTableSize);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                } else {
+                    SkASSERT(curve.isParametric());
+                    build_table_linear_to_gamma(&fDstGammaTableStorage[i * kDstGammaTableSize],
+                                                kDstGammaTableSize, curve.fG, curve.fA, curve.fB,
+                                                curve.fC, curve.fD, curve.fE, curve.fF);
+                    fDstGammaTables[i] = &fDstGammaTableStorage[i * kDstGammaTableSize];
+                }
+            }
+        }
+    }
 }
 
 static float byte_to_float(uint8_t byte) {
index 061bc36..16b8cfa 100644 (file)
@@ -76,11 +76,12 @@ public:
     void applyTo8888(SkPMColor* dst, const RGBA32* src, int len) const override;
     void applyToF16(RGBAF16* dst, const RGBA32* src, int len) const override;
 
-    static constexpr int kDstGammaTableSize = 1024;
 private:
     SkDefaultXform(const sk_sp<SkColorSpace>& srcSpace, const SkMatrix44& srcToDst,
                    const sk_sp<SkColorSpace>& dstSpace);
 
+    static constexpr int      kDstGammaTableSize = 1024;
+
     sk_sp<SkColorLookUpTable> fColorLUT;
 
     // May contain pointers into storage or pointers into precomputed tables.
index d18b631..6289b09 100644 (file)
 #include "SkData.h"
 #include "SkTemplates.h"
 
-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 {
-        kNone_Type,
-        kNamed_Type,
-        kValue_Type,
-        kTable_Type,
-        kParam_Type,
-    };
-
-    // Contains information for a gamma table.
-    struct Table {
-        size_t fOffset;
-        int    fSize;
-
-        const float* table(const SkGammas* base) const {
-            return SkTAddOffset<const float>(base, sizeof(SkGammas) + fOffset);
-        }
-    };
-
-    // Contains the parameters for a parametric curve.
-    struct Params {
-        //     Y = (aX + b)^g + c  for X >= d
-        //     Y = eX + f          otherwise
-        float                    fG;
-        float                    fA;
-        float                    fB;
-        float                    fC;
-        float                    fD;
-        float                    fE;
-        float                    fF;
-    };
-
-    // Contains the actual gamma curve information.  Should be interpreted
-    // based on the type of the gamma curve.
-    union Data {
-        Data()
-            : fTable{ 0, 0 }
-        {}
-
-        inline bool operator==(const Data& that) const {
-            return this->fTable.fOffset == that.fTable.fOffset &&
-                   this->fTable.fSize == that.fTable.fSize;
-        }
-
-        SkColorSpace::GammaNamed fNamed;
-        float                    fValue;
-        Table                    fTable;
-        size_t                   fParamOffset;
-
-        const Params& params(const SkGammas* base) const {
-            return *SkTAddOffset<const Params>(base, sizeof(SkGammas) + fParamOffset);
-        }
-    };
-
-    bool isNamed(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kNamed_Type;
+struct SkGammaCurve {
+    bool isNamed() const {
+       bool result = (SkColorSpace::kNonStandard_GammaNamed != fNamed);
+       SkASSERT(!result || (0.0f == fValue));
+       SkASSERT(!result || (0 == fTableSize));
+       SkASSERT(!result || (0.0f == fG && 0.0f == fE));
+       return result;
     }
 
-    bool isValue(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kValue_Type;
+    bool isValue() const {
+        bool result = (0.0f != fValue);
+        SkASSERT(!result || SkColorSpace::kNonStandard_GammaNamed == fNamed);
+        SkASSERT(!result || (0 == fTableSize));
+        SkASSERT(!result || (0.0f == fG && 0.0f == fE));
+        return result;
     }
 
-    bool isTable(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kTable_Type;
+    bool isTable() const {
+        bool result = (0 != fTableSize);
+        SkASSERT(!result || SkColorSpace::kNonStandard_GammaNamed == fNamed);
+        SkASSERT(!result || (0.0f == fValue));
+        SkASSERT(!result || (0.0f == fG && 0.0f == fE));
+        SkASSERT(!result || fTable);
+        return result;
     }
 
-    bool isParametric(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedType)[i] == Type::kParam_Type;
+    bool isParametric() const {
+        bool result = (0.0f != fG || 0.0f != fE);
+        SkASSERT(!result || SkColorSpace::kNonStandard_GammaNamed == fNamed);
+        SkASSERT(!result || (0.0f == fValue));
+        SkASSERT(!result || (0 == fTableSize));
+        return result;
     }
 
-    const Data& data(int i) const {
-        SkASSERT(0 <= i && i < 3);
-        return (&fRedData)[i];
+    // We have four different ways to represent gamma.
+    // (1) A known, named type:
+    SkColorSpace::GammaNamed fNamed;
+
+    // (2) A single value:
+    float                    fValue;
+
+    // (3) A lookup table:
+    uint32_t                 fTableSize;
+    std::unique_ptr<float[]> fTable;
+
+    // (4) Parameters for a curve:
+    //     Y = (aX + b)^g + c  for X >= d
+    //     Y = eX + f          otherwise
+    float                    fG;
+    float                    fA;
+    float                    fB;
+    float                    fC;
+    float                    fD;
+    float                    fE;
+    float                    fF;
+
+    SkGammaCurve()
+        : fNamed(SkColorSpace::kNonStandard_GammaNamed)
+        , fValue(0.0f)
+        , fTableSize(0)
+        , fTable(nullptr)
+        , fG(0.0f)
+        , fA(0.0f)
+        , fB(0.0f)
+        , fC(0.0f)
+        , fD(0.0f)
+        , fE(0.0f)
+        , fF(0.0f)
+    {}
+
+    bool quickEquals(const SkGammaCurve& that) const {
+        return (this->fNamed == that.fNamed) && (this->fValue == that.fValue) &&
+                (this->fTableSize == that.fTableSize) && (this->fTable == that.fTable) &&
+                (this->fG == that.fG) && (this->fA == that.fA) && (this->fB == that.fB) &&
+                (this->fC == that.fC) && (this->fD == that.fD) && (this->fE == that.fE) &&
+                (this->fF == that.fF);
     }
+};
 
-    const float* table(int i) const {
-        SkASSERT(isTable(i));
-        return (&fRedData)[i].fTable.table(this);
+struct SkGammas : public SkRefCnt {
+public:
+    static SkColorSpace::GammaNamed Named(SkGammaCurve curves[3]) {
+        if (SkColorSpace::kLinear_GammaNamed == curves[0].fNamed &&
+            SkColorSpace::kLinear_GammaNamed == curves[1].fNamed &&
+            SkColorSpace::kLinear_GammaNamed == curves[2].fNamed)
+        {
+            return SkColorSpace::kLinear_GammaNamed;
+        }
+
+        if (SkColorSpace::kSRGB_GammaNamed == curves[0].fNamed &&
+            SkColorSpace::kSRGB_GammaNamed == curves[1].fNamed &&
+            SkColorSpace::kSRGB_GammaNamed == curves[2].fNamed)
+        {
+            return SkColorSpace::kSRGB_GammaNamed;
+        }
+
+        if (SkColorSpace::k2Dot2Curve_GammaNamed == curves[0].fNamed &&
+            SkColorSpace::k2Dot2Curve_GammaNamed == curves[1].fNamed &&
+            SkColorSpace::k2Dot2Curve_GammaNamed == curves[2].fNamed)
+        {
+            return SkColorSpace::k2Dot2Curve_GammaNamed;
+        }
+
+        return SkColorSpace::kNonStandard_GammaNamed;
     }
 
-    const Params& params(int i) const {
-        SkASSERT(isParametric(i));
-        return (&fRedData)[i].params(this);
+    const SkGammaCurve& operator[](int i) const {
+        SkASSERT(0 <= i && i < 3);
+        return (&fRed)[i];
     }
 
-    SkGammas()
-        : fRedType(Type::kNone_Type)
-        , fGreenType(Type::kNone_Type)
-        , fBlueType(Type::kNone_Type)
+    const SkGammaCurve fRed;
+    const SkGammaCurve fGreen;
+    const SkGammaCurve fBlue;
+
+    SkGammas(SkGammaCurve red, SkGammaCurve green, SkGammaCurve blue)
+        : fRed(std::move(red))
+        , fGreen(std::move(green))
+        , fBlue(std::move(blue))
     {}
 
-    // These fields should only be modified when initializing the struct.
-    Data fRedData;
-    Data fGreenData;
-    Data fBlueData;
-    Type fRedType;
-    Type fGreenType;
-    Type fBlueType;
-
-    // Objects of this type are sometimes created in a custom fashion using
-    // sk_malloc_throw and therefore must be sk_freed.  We overload new to
-    // also call sk_malloc_throw so that memory can be unconditionally released
-    // using sk_free in an overloaded delete. Overloading regular new means we
-    // must also overload placement new.
-    void* operator new(size_t size) { return sk_malloc_throw(size); }
-    void* operator new(size_t, void* p) { return p; }
-    void operator delete(void* p) { sk_free(p); }
+    SkGammas() {}
+
+    friend class SkColorSpace;
 };
 
 struct SkColorLookUpTable : public SkRefCnt {
@@ -163,8 +173,8 @@ private:
 
     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);
+    SkColorSpace_Base(sk_sp<SkColorLookUpTable> colorLUT, sk_sp<SkGammas> gammas,
+                      const SkMatrix44& toXYZ, sk_sp<SkData> profileData);
 
     sk_sp<SkColorLookUpTable> fColorLUT;
     sk_sp<SkGammas>           fGammas;
index c428009..5585fbb 100644 (file)
@@ -238,384 +238,287 @@ static bool load_xyz(float dst[3], const uint8_t* src, size_t len) {
 static constexpr uint32_t kTAG_CurveType     = SkSetFourByteTag('c', 'u', 'r', 'v');
 static constexpr uint32_t kTAG_ParaCurveType = SkSetFourByteTag('p', 'a', 'r', 'a');
 
-static SkGammas::Type set_gamma_value(SkGammas::Data* data, float value) {
-    if (color_space_almost_equal(2.2f, value)) {
-        data->fNamed = SkColorSpace::k2Dot2Curve_GammaNamed;
-        return SkGammas::Type::kNamed_Type;
-    }
-
-    if (color_space_almost_equal(1.0f, value)) {
-        data->fNamed = SkColorSpace::kLinear_GammaNamed;
-        return SkGammas::Type::kNamed_Type;
-    }
-
-    if (color_space_almost_equal(0.0f, value)) {
-        return SkGammas::Type::kNone_Type;
-    }
-
-    data->fValue = value;
-    return SkGammas::Type::kValue_Type;
-}
-
-static float read_big_endian_16_dot_16(const uint8_t buf[4]) {
-    // It just so happens that SkFixed is also 16.16!
-    return SkFixedToFloat(read_big_endian_int(buf));
-}
-
-/**
- *  @param outData     Set to the appropriate value on success.  If we have table or
- *                     parametric gamma, it is the responsibility of the caller to set
- *                     fOffset.
- *  @param outParams   If this is a parametric gamma, this is set to the appropriate
- *                     parameters on success.
- *  @param outTagBytes Will be set to the length of the tag on success.
- *  @src               Pointer to tag data.
- *  @len               Length of tag data in bytes.
- *
- *  @return            kNone_Type on failure, otherwise the type of the gamma tag.
- */
-static SkGammas::Type parse_gamma(SkGammas::Data* outData, SkGammas::Params* outParams,
-                                      size_t* outTagBytes, const uint8_t* src, size_t len) {
-    if (len < 12) {
-        SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-        return SkGammas::Type::kNone_Type;
-    }
-
-    // In the case of consecutive gamma tags, we need to count the number of bytes in the
-    // tag, so that we can move on to the next tag.
-    size_t tagBytes;
-
-    uint32_t type = read_big_endian_uint(src);
-    // Bytes 4-7 are reserved and should be set to zero.
-    switch (type) {
-        case kTAG_CurveType: {
-            uint32_t count = read_big_endian_uint(src + 8);
-
-            // tagBytes = 12 + 2 * count
-            // We need to do safe addition here to avoid integer overflow.
-            if (!safe_add(count, count, &tagBytes) ||
-                !safe_add((size_t) 12, tagBytes, &tagBytes))
-            {
-                SkColorSpacePrintf("Invalid gamma count");
-                return SkGammas::Type::kNone_Type;
-            }
-
-            if (len < tagBytes) {
-                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                return SkGammas::Type::kNone_Type;
-            }
-            *outTagBytes = tagBytes;
-
-            if (0 == count) {
-                // Some tags require a gamma curve, but the author doesn't actually want
-                // to transform the data.  In this case, it is common to see a curve with
-                // a count of 0.
-                outData->fNamed = SkColorSpace::kLinear_GammaNamed;
-                return SkGammas::Type::kNamed_Type;
-            }
-
-            const uint16_t* table = (const uint16_t*) (src + 12);
-            if (1 == count) {
-                // The table entry is the gamma (with a bias of 256).
-                float value = (read_big_endian_short((const uint8_t*) table)) / 256.0f;
-                SkColorSpacePrintf("gamma %g\n", value);
-
-                return set_gamma_value(outData, value);
-            }
+static bool load_gammas(SkGammaCurve* gammas, uint32_t numGammas, const uint8_t* src, size_t len) {
+    for (uint32_t i = 0; i < numGammas; i++) {
+        if (len < 12) {
+            // FIXME (msarett):
+            // We could potentially return false here after correctly parsing *some* of the
+            // gammas correctly.  Should we somehow try to indicate a partial success?
+            SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
+            return false;
+        }
 
-            // Check for frequently occurring sRGB curves.
-            // We do this by sampling a few values and see if they match our expectation.
-            // A more robust solution would be to compare each value in this curve against
-            // an sRGB curve to see if we remain below an error threshold.  At this time,
-            // we haven't seen any images in the wild that make this kind of
-            // calculation necessary.  We encounter identical gamma curves over and
-            // over again, but relatively few variations.
-            if (1024 == count) {
-                // The magic values were chosen because they match both the very common
-                // HP sRGB gamma table and the less common Canon sRGB gamma table (which use
-                // different rounding rules).
-                if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
-                        3366 == read_big_endian_short((const uint8_t*) &table[257]) &&
-                        14116 == read_big_endian_short((const uint8_t*) &table[513]) &&
-                        34318 == read_big_endian_short((const uint8_t*) &table[768]) &&
-                        65535 == read_big_endian_short((const uint8_t*) &table[1023])) {
-                    outData->fNamed = SkColorSpace::kSRGB_GammaNamed;
-                    return SkGammas::Type::kNamed_Type;
-                }
-            }
+        // We need to count the number of bytes in the tag, so we are able to move to the
+        // next tag on the next loop iteration.
+        size_t tagBytes;
 
-            if (26 == count) {
-                // The magic values were chosen because they match a very common LCMS sRGB
-                // gamma table.
-                if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
-                        3062 == read_big_endian_short((const uint8_t*) &table[6]) &&
-                        12824 == read_big_endian_short((const uint8_t*) &table[12]) &&
-                        31237 == read_big_endian_short((const uint8_t*) &table[18]) &&
-                        65535 == read_big_endian_short((const uint8_t*) &table[25])) {
-                    outData->fNamed = SkColorSpace::kSRGB_GammaNamed;
-                    return SkGammas::Type::kNamed_Type;
-                }
-            }
+        uint32_t type = read_big_endian_uint(src);
+        switch (type) {
+            case kTAG_CurveType: {
+                uint32_t count = read_big_endian_uint(src + 8);
 
-            if (4096 == count) {
-                // The magic values were chosen because they match Nikon, Epson, and
-                // LCMS sRGB gamma tables (all of which use different rounding rules).
-                if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
-                        950 == read_big_endian_short((const uint8_t*) &table[515]) &&
-                        3342 == read_big_endian_short((const uint8_t*) &table[1025]) &&
-                        14079 == read_big_endian_short((const uint8_t*) &table[2051]) &&
-                        65535 == read_big_endian_short((const uint8_t*) &table[4095])) {
-                    outData->fNamed = SkColorSpace::kSRGB_GammaNamed;
-                    return SkGammas::Type::kNamed_Type;
+                // tagBytes = 12 + 2 * count
+                // We need to do safe addition here to avoid integer overflow.
+                if (!safe_add(count, count, &tagBytes) ||
+                    !safe_add((size_t) 12, tagBytes, &tagBytes))
+                {
+                    SkColorSpacePrintf("Invalid gamma count");
+                    return false;
                 }
-            }
 
-            // Otherwise, we will represent gamma with a table.
-            outData->fTable.fSize = count;
-            return SkGammas::Type::kTable_Type;
-        }
-        case kTAG_ParaCurveType: {
-            enum ParaCurveType {
-                kExponential_ParaCurveType = 0,
-                kGAB_ParaCurveType         = 1,
-                kGABC_ParaCurveType        = 2,
-                kGABDE_ParaCurveType       = 3,
-                kGABCDEF_ParaCurveType     = 4,
-            };
-
-            // Determine the format of the parametric curve tag.
-            uint16_t format = read_big_endian_short(src + 8);
-            if (format > kGABCDEF_ParaCurveType) {
-                SkColorSpacePrintf("Unsupported gamma tag type %d\n", type);
-                return SkGammas::Type::kNone_Type;
-            }
-
-            if (kExponential_ParaCurveType == format) {
-                tagBytes = 12 + 4;
-                if (len < tagBytes) {
+                if (0 == count) {
+                    // Some tags require a gamma curve, but the author doesn't actually want
+                    // to transform the data.  In this case, it is common to see a curve with
+                    // a count of 0.
+                    gammas[i].fNamed = SkColorSpace::kLinear_GammaNamed;
+                    break;
+                } else if (len < tagBytes) {
                     SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                    return SkGammas::Type::kNone_Type;
+                    return false;
                 }
 
-                // Y = X^g
-                float g = read_big_endian_16_dot_16(src + 12);
-
-                *outTagBytes = tagBytes;
-                return set_gamma_value(outData, g);
-            }
-
-            // Here's where the real parametric gammas start.  There are many
-            // permutations of the same equations.
-            //
-            // Y = (aX + b)^g + c  for X >= d
-            // Y = eX + f          otherwise
-            //
-            // We will fill in with zeros as necessary to always match the above form.
-            if (len < 24) {
-                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                return SkGammas::Type::kNone_Type;
-            }
-            float g = read_big_endian_16_dot_16(src + 12);
-            float a = read_big_endian_16_dot_16(src + 16);
-            float b = read_big_endian_16_dot_16(src + 20);
-            float c = 0.0f, d = 0.0f, e = 0.0f, f = 0.0f;
-            switch(format) {
-                case kGAB_ParaCurveType:
-                    tagBytes = 12 + 12;
-
-                    // Y = (aX + b)^g  for X >= -b/a
-                    // Y = 0           otherwise
-                    d = -b / a;
+                const uint16_t* table = (const uint16_t*) (src + 12);
+                if (1 == count) {
+                    // The table entry is the gamma (with a bias of 256).
+                    float value = (read_big_endian_short((const uint8_t*) table)) / 256.0f;
+                    set_gamma_value(&gammas[i], value);
+                    SkColorSpacePrintf("gamma %g\n", value);
                     break;
-                case kGABC_ParaCurveType:
-                    tagBytes = 12 + 16;
-                    if (len < tagBytes) {
-                        SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                        return SkGammas::Type::kNone_Type;
-                    }
+                }
 
-                    // Y = (aX + b)^g + c  for X >= -b/a
-                    // Y = c               otherwise
-                    c = read_big_endian_16_dot_16(src + 24);
-                    d = -b / a;
-                    f = c;
-                    break;
-                case kGABDE_ParaCurveType:
-                    tagBytes = 12 + 20;
-                    if (len < tagBytes) {
-                        SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                        return SkGammas::Type::kNone_Type;
+                // Check for frequently occurring sRGB curves.
+                // We do this by sampling a few values and see if they match our expectation.
+                // A more robust solution would be to compare each value in this curve against
+                // an sRGB curve to see if we remain below an error threshold.  At this time,
+                // we haven't seen any images in the wild that make this kind of
+                // calculation necessary.  We encounter identical gamma curves over and
+                // over again, but relatively few variations.
+                if (1024 == count) {
+                    // The magic values were chosen because they match a very common sRGB
+                    // gamma table and the less common Canon sRGB gamma table (which use
+                    // different rounding rules).
+                    if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
+                            3366 == read_big_endian_short((const uint8_t*) &table[257]) &&
+                            14116 == read_big_endian_short((const uint8_t*) &table[513]) &&
+                            34318 == read_big_endian_short((const uint8_t*) &table[768]) &&
+                            65535 == read_big_endian_short((const uint8_t*) &table[1023])) {
+                        gammas[i].fNamed = SkColorSpace::kSRGB_GammaNamed;
+                        break;
                     }
+                } else if (26 == count) {
+                    // The magic values were chosen because they match a very common sRGB
+                    // gamma table.
+                    if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
+                            3062 == read_big_endian_short((const uint8_t*) &table[6]) &&
+                            12824 == read_big_endian_short((const uint8_t*) &table[12]) &&
+                            31237 == read_big_endian_short((const uint8_t*) &table[18]) &&
+                            65535 == read_big_endian_short((const uint8_t*) &table[25])) {
+                        gammas[i].fNamed = SkColorSpace::kSRGB_GammaNamed;
+                        break;
+                    }
+                } else if (4096 == count) {
+                    // The magic values were chosen because they match Nikon, Epson, and
+                    // LCMS sRGB gamma tables (all of which use different rounding rules).
+                    if (0 == read_big_endian_short((const uint8_t*) &table[0]) &&
+                            950 == read_big_endian_short((const uint8_t*) &table[515]) &&
+                            3342 == read_big_endian_short((const uint8_t*) &table[1025]) &&
+                            14079 == read_big_endian_short((const uint8_t*) &table[2051]) &&
+                            65535 == read_big_endian_short((const uint8_t*) &table[4095])) {
+                        gammas[i].fNamed = SkColorSpace::kSRGB_GammaNamed;
+                        break;
+                    }
+                }
 
-                    // Y = (aX + b)^g  for X >= d
-                    // Y = eX          otherwise
-                    d = read_big_endian_16_dot_16(src + 28);
-
-                    // Not a bug!  We define |e| to always be the coefficient on X in the
-                    // second equation.  The spec calls this |c| in this particular equation.
-                    // We don't follow their convention because then |c| would have a
-                    // different meaning in each of our cases.
-                    e = read_big_endian_16_dot_16(src + 24);
-                    break;
-                case kGABCDEF_ParaCurveType:
-                    tagBytes = 12 + 28;
+                // Otherwise, fill in the interpolation table.
+                gammas[i].fTableSize = count;
+                gammas[i].fTable = std::unique_ptr<float[]>(new float[count]);
+                for (uint32_t j = 0; j < count; j++) {
+                    gammas[i].fTable[j] =
+                            (read_big_endian_short((const uint8_t*) &table[j])) / 65535.0f;
+                }
+                break;
+            }
+            case kTAG_ParaCurveType: {
+                enum ParaCurveType {
+                    kExponential_ParaCurveType = 0,
+                    kGAB_ParaCurveType         = 1,
+                    kGABC_ParaCurveType        = 2,
+                    kGABDE_ParaCurveType       = 3,
+                    kGABCDEF_ParaCurveType     = 4,
+                };
+
+                // Determine the format of the parametric curve tag.
+                uint16_t format = read_big_endian_short(src + 8);
+                if (kExponential_ParaCurveType == format) {
+                    tagBytes = 12 + 4;
                     if (len < tagBytes) {
                         SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
-                        return SkGammas::Type::kNone_Type;
+                        return false;
                     }
 
+                    // Y = X^g
+                    int32_t g = read_big_endian_int(src + 12);
+                    set_gamma_value(&gammas[i], SkFixedToFloat(g));
+                } else {
+                    // Here's where the real parametric gammas start.  There are many
+                    // permutations of the same equations.
+                    //
                     // Y = (aX + b)^g + c  for X >= d
                     // Y = eX + f          otherwise
-                    // NOTE: The ICC spec writes "cX" in place of "eX" but I think
-                    //       it's a typo.
-                    c = read_big_endian_16_dot_16(src + 24);
-                    d = read_big_endian_16_dot_16(src + 28);
-                    e = read_big_endian_16_dot_16(src + 32);
-                    f = read_big_endian_16_dot_16(src + 36);
-                    break;
-                default:
-                    SkASSERT(false);
-                    return SkGammas::Type::kNone_Type;
-            }
-
-            // Recognize and simplify a very common parametric representation of sRGB gamma.
-            if (color_space_almost_equal(0.9479f, a) &&
-                    color_space_almost_equal(0.0521f, b) &&
-                    color_space_almost_equal(0.0000f, c) &&
-                    color_space_almost_equal(0.0405f, d) &&
-                    color_space_almost_equal(0.0774f, e) &&
-                    color_space_almost_equal(0.0000f, f) &&
-                    color_space_almost_equal(2.4000f, g)) {
-                outData->fNamed = SkColorSpace::kSRGB_GammaNamed;
-                return SkGammas::Type::kNamed_Type;
-            }
-
-            // Fail on invalid gammas.
-            if (SkScalarIsNaN(d)) {
-                return SkGammas::Type::kNone_Type;
-            }
+                    //
+                    // We will fill in with zeros as necessary to always match the above form.
+                    float g = 0.0f, a = 0.0f, b = 0.0f, c = 0.0f, d = 0.0f, e = 0.0f, f = 0.0f;
+                    switch(format) {
+                        case kGAB_ParaCurveType: {
+                            tagBytes = 12 + 12;
+                            if (len < tagBytes) {
+                                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
+                                return false;
+                            }
+
+                            // Y = (aX + b)^g  for X >= -b/a
+                            // Y = 0           otherwise
+                            g = SkFixedToFloat(read_big_endian_int(src + 12));
+                            a = SkFixedToFloat(read_big_endian_int(src + 16));
+                            if (0.0f == a) {
+                                return false;
+                            }
+
+                            b = SkFixedToFloat(read_big_endian_int(src + 20));
+                            d = -b / a;
+                            break;
+                        }
+                        case kGABC_ParaCurveType:
+                            tagBytes = 12 + 16;
+                            if (len < tagBytes) {
+                                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
+                                return false;
+                            }
+
+                            // Y = (aX + b)^g + c  for X >= -b/a
+                            // Y = c               otherwise
+                            g = SkFixedToFloat(read_big_endian_int(src + 12));
+                            a = SkFixedToFloat(read_big_endian_int(src + 16));
+                            if (0.0f == a) {
+                                return false;
+                            }
+
+                            b = SkFixedToFloat(read_big_endian_int(src + 20));
+                            c = SkFixedToFloat(read_big_endian_int(src + 24));
+                            d = -b / a;
+                            f = c;
+                            break;
+                        case kGABDE_ParaCurveType:
+                            tagBytes = 12 + 20;
+                            if (len < tagBytes) {
+                                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
+                                return false;
+                            }
+
+                            // Y = (aX + b)^g  for X >= d
+                            // Y = cX          otherwise
+                            g = SkFixedToFloat(read_big_endian_int(src + 12));
+                            a = SkFixedToFloat(read_big_endian_int(src + 16));
+                            b = SkFixedToFloat(read_big_endian_int(src + 20));
+                            d = SkFixedToFloat(read_big_endian_int(src + 28));
+                            e = SkFixedToFloat(read_big_endian_int(src + 24));
+                            break;
+                        case kGABCDEF_ParaCurveType:
+                            tagBytes = 12 + 28;
+                            if (len < tagBytes) {
+                                SkColorSpacePrintf("gamma tag is too small (%d bytes)", len);
+                                return false;
+                            }
+
+                            // Y = (aX + b)^g + c  for X >= d
+                            // Y = eX + f          otherwise
+                            // NOTE: The ICC spec writes "cX" in place of "eX" but I think
+                            //       it's a typo.
+                            g = SkFixedToFloat(read_big_endian_int(src + 12));
+                            a = SkFixedToFloat(read_big_endian_int(src + 16));
+                            b = SkFixedToFloat(read_big_endian_int(src + 20));
+                            c = SkFixedToFloat(read_big_endian_int(src + 24));
+                            d = SkFixedToFloat(read_big_endian_int(src + 28));
+                            e = SkFixedToFloat(read_big_endian_int(src + 32));
+                            f = SkFixedToFloat(read_big_endian_int(src + 36));
+                            break;
+                        default:
+                            SkColorSpacePrintf("Invalid parametric curve type\n");
+                            return false;
+                    }
 
-            if (d <= 0.0f) {
-                // Y = (aX + b)^g + c  for always
-                if (0.0f == a || 0.0f == g) {
-                    SkColorSpacePrintf("A or G is zero, constant gamma function "
-                                       "is nonsense");
-                    return SkGammas::Type::kNone_Type;
-                }
-            }
+                    // Recognize and simplify a very common parametric representation of sRGB gamma.
+                    if (color_space_almost_equal(0.9479f, a) &&
+                            color_space_almost_equal(0.0521f, b) &&
+                            color_space_almost_equal(0.0000f, c) &&
+                            color_space_almost_equal(0.0405f, d) &&
+                            color_space_almost_equal(0.0774f, e) &&
+                            color_space_almost_equal(0.0000f, f) &&
+                            color_space_almost_equal(2.4000f, g)) {
+                        gammas[i].fNamed = SkColorSpace::kSRGB_GammaNamed;
+                    } else {
+                        // Fail on invalid gammas.
+                        if (d <= 0.0f) {
+                            // Y = (aX + b)^g + c  for always
+                            if (0.0f == a || 0.0f == g) {
+                                SkColorSpacePrintf("A or G is zero, constant gamma function "
+                                                   "is nonsense");
+                                return false;
+                            }
+                        } else if (d >= 1.0f) {
+                            // Y = eX + f          for always
+                            if (0.0f == e) {
+                                SkColorSpacePrintf("E is zero, constant gamma function is "
+                                                   "nonsense");
+                                return false;
+                            }
+                        } else if ((0.0f == a || 0.0f == g) && 0.0f == e) {
+                            SkColorSpacePrintf("A or G, and E are zero, constant gamma function "
+                                               "is nonsense");
+                            return false;
+                        }
 
-            if (d >= 1.0f) {
-                // Y = eX + f          for always
-                if (0.0f == e) {
-                    SkColorSpacePrintf("E is zero, constant gamma function is "
-                                       "nonsense");
-                    return SkGammas::Type::kNone_Type;
+                        gammas[i].fG = g;
+                        gammas[i].fA = a;
+                        gammas[i].fB = b;
+                        gammas[i].fC = c;
+                        gammas[i].fD = d;
+                        gammas[i].fE = e;
+                        gammas[i].fF = f;
+                    }
                 }
-            }
 
-            if ((0.0f == a || 0.0f == g) && 0.0f == e) {
-                SkColorSpacePrintf("A or G, and E are zero, constant gamma function "
-                                   "is nonsense");
-                return SkGammas::Type::kNone_Type;
+                break;
             }
-
-            *outTagBytes = tagBytes;
-
-            outParams->fG = g;
-            outParams->fA = a;
-            outParams->fB = b;
-            outParams->fC = c;
-            outParams->fD = d;
-            outParams->fE = e;
-            outParams->fF = f;
-            return SkGammas::Type::kParam_Type;
+            default:
+                SkColorSpacePrintf("Unsupported gamma tag type %d\n", type);
+                return false;
         }
-        default:
-            SkColorSpacePrintf("Unsupported gamma tag type %d\n", type);
-            return SkGammas::Type::kNone_Type;
-    }
-}
-
-/**
- *  Returns the additional size in bytes needed to store the gamma tag.
- */
-static size_t gamma_alloc_size(SkGammas::Type type, const SkGammas::Data& data) {
-    switch (type) {
-        case SkGammas::Type::kNamed_Type:
-        case SkGammas::Type::kValue_Type:
-            return 0;
-        case SkGammas::Type::kTable_Type:
-            return sizeof(float) * data.fTable.fSize;
-        case SkGammas::Type::kParam_Type:
-            return sizeof(SkGammas::Params);
-        default:
-            SkASSERT(false);
-            return 0;
-    }
-}
 
-/**
- *  Sets invalid gamma to the default value.
- */
-static void handle_invalid_gamma(SkGammas::Type* type, SkGammas::Data* data) {
-    if (SkGammas::Type::kNone_Type == *type) {
-        *type = SkGammas::Type::kNamed_Type;
-        data->fNamed = SkColorSpace::kSRGB_GammaNamed;
-    }
-}
+        // Ensure that we have successfully read a gamma representation.
+        SkASSERT(gammas[i].isNamed() || gammas[i].isValue() || gammas[i].isTable() ||
+                 gammas[i].isParametric());
 
-/**
- *  Finish loading the gammas, now that we have allocated memory for the SkGammas struct.
- *
- *  There's nothing to do for the simple cases, but for table gammas we need to actually
- *  read the table into heap memory.  And for parametric gammas, we need to copy over the
- *  parameter values.
- *
- *  @param memory Pointer to start of the SkGammas memory block
- *  @param offset Bytes of memory (after the SkGammas struct) that are already in use.
- *  @param data   In-out variable.  Will fill in the offset to the table or parameters
- *                if necessary.
- *  @param params Parameters for gamma curve.  Only initialized/used when we have a
- *                parametric gamma.
- *  @param src    Pointer to start of the gamma tag.
- *
- *  @return       Additional bytes of memory that are being used by this gamma curve.
- */
-static size_t load_gammas(void* memory, size_t offset, SkGammas::Type type,
-                        SkGammas::Data* data, const SkGammas::Params& params,
-                        const uint8_t* src) {
-    void* storage = SkTAddOffset<void>(memory, offset + sizeof(SkGammas));
-
-    switch (type) {
-        case SkGammas::Type::kNamed_Type:
-        case SkGammas::Type::kValue_Type:
-            // Nothing to do here.
-            return 0;
-        case SkGammas::Type::kTable_Type: {
-            data->fTable.fOffset = offset;
-
-            float* outTable = (float*) storage;
-            const uint16_t* inTable = (const uint16_t*) (src + 12);
-            for (int i = 0; i < data->fTable.fSize; i++) {
-                outTable[i] = read_big_endian_16_dot_16((const uint8_t*) &inTable[i]);
+        // Adjust src and len if there is another gamma curve to load.
+        if (i != numGammas - 1) {
+            // Each curve is padded to 4-byte alignment.
+            tagBytes = SkAlign4(tagBytes);
+            if (len < tagBytes) {
+                return false;
             }
 
-            return sizeof(float) * data->fTable.fSize;
+            src += tagBytes;
+            len -= tagBytes;
         }
-        case SkGammas::Type::kParam_Type:
-            data->fTable.fOffset = offset;
-            memcpy(storage, &params, sizeof(SkGammas::Params));
-            return sizeof(SkGammas::Params);
-        default:
-            SkASSERT(false);
-            return 0;
     }
+
+    return true;
 }
 
 static constexpr uint32_t kTAG_AtoBType = SkSetFourByteTag('m', 'A', 'B', ' ');
 
-static bool load_color_lut(SkColorLookUpTable* colorLUT, uint32_t inputChannels,
-                           uint32_t outputChannels, const uint8_t* src, size_t len) {
+bool load_color_lut(SkColorLookUpTable* colorLUT, uint32_t inputChannels, uint32_t outputChannels,
+                    const uint8_t* src, size_t len) {
     // 16 bytes reserved for grid points, 2 for precision, 2 for padding.
     // The color LUT data follows after this header.
     static constexpr uint32_t kColorLUTHeaderSize = 20;
@@ -684,7 +587,7 @@ static bool load_color_lut(SkColorLookUpTable* colorLUT, uint32_t inputChannels,
     return true;
 }
 
-static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
+bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
     if (len < 48) {
         SkColorSpacePrintf("Matrix tag is too small (%d bytes).", len);
         return false;
@@ -713,8 +616,8 @@ static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
     return true;
 }
 
-static bool load_a2b0(SkColorLookUpTable* colorLUT, SkColorSpace::GammaNamed* gammaNamed,
-                      sk_sp<SkGammas>* gammas, SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
+bool load_a2b0(SkColorLookUpTable* colorLUT, SkGammaCurve* gammas, SkMatrix44* toXYZ,
+               const uint8_t* src, size_t len) {
     if (len < 32) {
         SkColorSpacePrintf("A to B tag is too small (%d bytes).", len);
         return false;
@@ -762,77 +665,11 @@ static bool load_a2b0(SkColorLookUpTable* colorLUT, SkColorSpace::GammaNamed* ga
 
     uint32_t offsetToMCurves = read_big_endian_int(src + 20);
     if (0 != offsetToMCurves && offsetToMCurves < len) {
-        const uint8_t* rTagPtr = src + offsetToMCurves;
-        size_t tagLen = len - offsetToMCurves;
-
-        SkGammas::Data rData;
-        SkGammas::Params rParams;
-
-        // On an invalid first gamma, tagBytes remains set as zero.  This causes the two
-        // subsequent to be treated as identical (which is what we want).
-        size_t tagBytes = 0;
-        SkGammas::Type rType = parse_gamma(&rData, &rParams, &tagBytes, rTagPtr, tagLen);
-        handle_invalid_gamma(&rType, &rData);
-        size_t alignedTagBytes = SkAlign4(tagBytes);
-
-        if ((3 * alignedTagBytes <= tagLen) &&
-            !memcmp(rTagPtr, rTagPtr + 1 * alignedTagBytes, tagBytes) &&
-            !memcmp(rTagPtr, rTagPtr + 2 * alignedTagBytes, tagBytes))
-        {
-            if (SkGammas::Type::kNamed_Type == rType) {
-                *gammaNamed = rData.fNamed;
-            } else {
-                size_t allocSize = sizeof(SkGammas) + gamma_alloc_size(rType, rData);
-                void* memory = sk_malloc_throw(allocSize);
-                *gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-                load_gammas(memory, 0, rType, &rData, rParams, rTagPtr);
-
-                (*gammas)->fRedType = rType;
-                (*gammas)->fGreenType = rType;
-                (*gammas)->fBlueType = rType;
-
-                (*gammas)->fRedData = rData;
-                (*gammas)->fGreenData = rData;
-                (*gammas)->fBlueData = rData;
-            }
-        } else {
-            const uint8_t* gTagPtr = rTagPtr + alignedTagBytes;
-            tagLen = tagLen > alignedTagBytes ? tagLen - alignedTagBytes : 0;
-            SkGammas::Data gData;
-            SkGammas::Params gParams;
-            tagBytes = 0;
-            SkGammas::Type gType = parse_gamma(&gData, &gParams, &tagBytes, gTagPtr,
-                                                       tagLen);
-            handle_invalid_gamma(&gType, &gData);
-
-            alignedTagBytes = SkAlign4(tagBytes);
-            const uint8_t* bTagPtr = gTagPtr + alignedTagBytes;
-            tagLen = tagLen > alignedTagBytes ? tagLen - alignedTagBytes : 0;
-            SkGammas::Data bData;
-            SkGammas::Params bParams;
-            SkGammas::Type bType = parse_gamma(&bData, &bParams, &tagBytes, bTagPtr,
-                                                       tagLen);
-            handle_invalid_gamma(&bType, &bData);
-
-            size_t allocSize = sizeof(SkGammas) + gamma_alloc_size(rType, rData)
-                                                + gamma_alloc_size(gType, gData)
-                                                + gamma_alloc_size(bType, bData);
-            void* memory = sk_malloc_throw(allocSize);
-            *gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-
-            uint32_t offset = 0;
-            (*gammas)->fRedType = rType;
-            offset += load_gammas(memory, offset, rType, &rData, rParams, rTagPtr);
-
-            (*gammas)->fGreenType = gType;
-            offset += load_gammas(memory, offset, gType, &gData, gParams, gTagPtr);
-
-            (*gammas)->fBlueType = bType;
-            load_gammas(memory, offset, bType, &bData, bParams, bTagPtr);
-
-            (*gammas)->fRedData = rData;
-            (*gammas)->fGreenData = gData;
-            (*gammas)->fBlueData = bData;
+        if (!load_gammas(gammas, outputChannels, src + offsetToMCurves, len - offsetToMCurves)) {
+            SkColorSpacePrintf("Failed to read M curves from A to B tag.  Using linear gamma.\n");
+            gammas[0].fNamed = SkColorSpace::kLinear_GammaNamed;
+            gammas[1].fNamed = SkColorSpace::kLinear_GammaNamed;
+            gammas[2].fNamed = SkColorSpace::kLinear_GammaNamed;
         }
     }
 
@@ -847,22 +684,6 @@ static bool load_a2b0(SkColorLookUpTable* colorLUT, SkColorSpace::GammaNamed* ga
     return true;
 }
 
-static bool tag_equals(const ICCTag* a, const ICCTag* b, const uint8_t* base) {
-    if (!a || !b) {
-        return a == b;
-    }
-
-    if (a->fLength != b->fLength) {
-        return false;
-    }
-
-    if (a->fOffset == b->fOffset) {
-        return true;
-    }
-
-    return !memcmp(a->addr(base), b->addr(base), a->fLength);
-}
-
 sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
     if (!input || len < kICCHeaderSize) {
         return_null("Data is null or not large enough to contain an ICC profile");
@@ -872,8 +693,8 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
     void* memory = sk_malloc_throw(len);
     memcpy(memory, input, len);
     sk_sp<SkData> data = SkData::MakeFromMalloc(memory, len);
-    const uint8_t* base = data->bytes();
-    const uint8_t* ptr = base;
+    const void* base = data->data();
+    const uint8_t* ptr = (const uint8_t*) base;
 
     // Read the ICC profile header and check to make sure that it is valid.
     ICCProfileHeader header;
@@ -919,112 +740,44 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
             const ICCTag* b = ICCTag::Find(tags.get(), tagCount, kTAG_bXYZ);
             if (r && g && b) {
                 float toXYZ[9];
-                if (!load_xyz(&toXYZ[0], r->addr(base), r->fLength) ||
-                    !load_xyz(&toXYZ[3], g->addr(base), g->fLength) ||
-                    !load_xyz(&toXYZ[6], b->addr(base), b->fLength))
+                if (!load_xyz(&toXYZ[0], r->addr((const uint8_t*) base), r->fLength) ||
+                    !load_xyz(&toXYZ[3], g->addr((const uint8_t*) base), g->fLength) ||
+                    !load_xyz(&toXYZ[6], b->addr((const uint8_t*) base), b->fLength))
                 {
                     return_null("Need valid rgb tags for XYZ space");
                 }
                 SkMatrix44 mat(SkMatrix44::kUninitialized_Constructor);
                 mat.set3x3RowMajorf(toXYZ);
 
+                // It is not uncommon to see missing or empty gamma tags.  This indicates
+                // that we should use unit gamma.
+                SkGammaCurve curves[3];
                 r = ICCTag::Find(tags.get(), tagCount, kTAG_rTRC);
                 g = ICCTag::Find(tags.get(), tagCount, kTAG_gTRC);
                 b = ICCTag::Find(tags.get(), tagCount, kTAG_bTRC);
-
-                // If some, but not all, of the gamma tags are missing, assume that all
-                // gammas are meant to be the same.  This behavior is an arbitrary guess,
-                // but it simplifies the code below.
-                if ((!r || !g || !b) && (r || g || b)) {
-                    if (!r) {
-                        r = g ? g : b;
-                    }
-
-                    if (!g) {
-                        g = r ? r : b;
-                    }
-
-                    if (!b) {
-                        b = r ? r : g;
-                    }
+                if (!r || !load_gammas(&curves[0], 1, r->addr((const uint8_t*) base), r->fLength))
+                {
+                    SkColorSpacePrintf("Failed to read R gamma tag.\n");
+                    curves[0].fNamed = SkColorSpace::kLinear_GammaNamed;
                 }
-
-                GammaNamed gammaNamed = kNonStandard_GammaNamed;
-                sk_sp<SkGammas> gammas = nullptr;
-                size_t tagBytes;
-                if (r && g && b) {
-                    if (tag_equals(r, g, base) && tag_equals(g, b, base)) {
-                        SkGammas::Data data;
-                        SkGammas::Params params;
-                        SkGammas::Type Type =
-                                parse_gamma(&data, &params, &tagBytes, r->addr(base), r->fLength);
-                        handle_invalid_gamma(&Type, &data);
-
-                        if (SkGammas::Type::kNamed_Type == Type) {
-                            gammaNamed = data.fNamed;
-                        } else {
-                            size_t allocSize = sizeof(SkGammas) + gamma_alloc_size(Type, data);
-                            void* memory = sk_malloc_throw(allocSize);
-                            gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-                            load_gammas(memory, 0, Type, &data, params, r->addr(base));
-
-                            gammas->fRedType = Type;
-                            gammas->fGreenType = Type;
-                            gammas->fBlueType = Type;
-
-                            gammas->fRedData = data;
-                            gammas->fGreenData = data;
-                            gammas->fBlueData = data;
-                        }
-                    } else {
-                        SkGammas::Data rData;
-                        SkGammas::Params rParams;
-                        SkGammas::Type rType =
-                                parse_gamma(&rData, &rParams, &tagBytes, r->addr(base), r->fLength);
-                        handle_invalid_gamma(&rType, &rData);
-
-                        SkGammas::Data gData;
-                        SkGammas::Params gParams;
-                        SkGammas::Type gType =
-                                parse_gamma(&gData, &gParams, &tagBytes, g->addr(base), g->fLength);
-                        handle_invalid_gamma(&gType, &gData);
-
-                        SkGammas::Data bData;
-                        SkGammas::Params bParams;
-                        SkGammas::Type bType =
-                                parse_gamma(&bData, &bParams, &tagBytes, b->addr(base), b->fLength);
-                        handle_invalid_gamma(&bType, &bData);
-
-                        size_t allocSize = sizeof(SkGammas) + gamma_alloc_size(rType, rData)
-                                                            + gamma_alloc_size(gType, gData)
-                                                            + gamma_alloc_size(bType, bData);
-                        void* memory = sk_malloc_throw(allocSize);
-                        gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-
-                        uint32_t offset = 0;
-                        gammas->fRedType = rType;
-                        offset += load_gammas(memory, offset, rType, &rData, rParams,
-                                              r->addr(base));
-
-                        gammas->fGreenType = gType;
-                        offset += load_gammas(memory, offset, gType, &gData, gParams,
-                                              g->addr(base));
-
-                        gammas->fBlueType = bType;
-                        load_gammas(memory, offset, bType, &bData, bParams, b->addr(base));
-
-                        gammas->fRedData = rData;
-                        gammas->fGreenData = gData;
-                        gammas->fBlueData = bData;
-                    }
-                } else {
-                    gammaNamed = kLinear_GammaNamed;
+                if (!g || !load_gammas(&curves[1], 1, g->addr((const uint8_t*) base), g->fLength))
+                {
+                    SkColorSpacePrintf("Failed to read G gamma tag.\n");
+                    curves[1].fNamed = SkColorSpace::kLinear_GammaNamed;
+                }
+                if (!b || !load_gammas(&curves[2], 1, b->addr((const uint8_t*) base), b->fLength))
+                {
+                    SkColorSpacePrintf("Failed to read B gamma tag.\n");
+                    curves[2].fNamed = SkColorSpace::kLinear_GammaNamed;
                 }
 
+                GammaNamed gammaNamed = SkGammas::Named(curves);
                 if (kNonStandard_GammaNamed == gammaNamed) {
-                    return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, gammaNamed,
-                                                                     std::move(gammas), mat,
-                                                                     std::move(data)));
+                    sk_sp<SkGammas> gammas = sk_make_sp<SkGammas>(std::move(curves[0]),
+                                                                  std::move(curves[1]),
+                                                                  std::move(curves[2]));
+                    return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, std::move(gammas),
+                                                                     mat, std::move(data)));
                 } else {
                     return SkColorSpace_Base::NewRGB(gammaNamed, mat);
                 }
@@ -1033,20 +786,24 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
             // Recognize color profile specified by A2B0 tag.
             const ICCTag* a2b0 = ICCTag::Find(tags.get(), tagCount, kTAG_A2B0);
             if (a2b0) {
-                GammaNamed gammaNamed = kNonStandard_GammaNamed;
-                sk_sp<SkGammas> gammas = nullptr;
                 sk_sp<SkColorLookUpTable> colorLUT = sk_make_sp<SkColorLookUpTable>();
+                SkGammaCurve curves[3];
                 SkMatrix44 toXYZ(SkMatrix44::kUninitialized_Constructor);
-                if (!load_a2b0(colorLUT.get(), &gammaNamed, &gammas, &toXYZ, a2b0->addr(base),
+                if (!load_a2b0(colorLUT.get(), curves, &toXYZ, a2b0->addr((const uint8_t*) base),
                                a2b0->fLength)) {
                     return_null("Failed to parse A2B0 tag");
                 }
 
+                GammaNamed gammaNamed = SkGammas::Named(curves);
                 colorLUT = colorLUT->fTable ? colorLUT : nullptr;
                 if (colorLUT || kNonStandard_GammaNamed == gammaNamed) {
+                    sk_sp<SkGammas> gammas = sk_make_sp<SkGammas>(std::move(curves[0]),
+                                                                  std::move(curves[1]),
+                                                                  std::move(curves[2]));
+
                     return sk_sp<SkColorSpace>(new SkColorSpace_Base(std::move(colorLUT),
-                                                                     gammaNamed, std::move(gammas),
-                                                                     toXYZ, std::move(data)));
+                                                                     std::move(gammas), toXYZ,
+                                                                     std::move(data)));
                 } else {
                     return SkColorSpace_Base::NewRGB(gammaNamed, toXYZ);
                 }
@@ -1188,6 +945,23 @@ static void write_trc_tag(uint32_t* ptr, float value) {
     ptr16[1] = 0;
 }
 
+static float get_gamma_value(const SkGammaCurve* curve) {
+    switch (curve->fNamed) {
+        case SkColorSpace::kSRGB_GammaNamed:
+            // FIXME (msarett):
+            // kSRGB cannot be represented by a value.  Here we fall through to 2.2f,
+            // which is a close guess.  To be more accurate, we need to represent sRGB
+            // gamma with a parametric curve.
+        case SkColorSpace::k2Dot2Curve_GammaNamed:
+            return 2.2f;
+        case SkColorSpace::kLinear_GammaNamed:
+            return 1.0f;
+        default:
+            SkASSERT(curve->isValue());
+            return curve->fValue;
+    }
+}
+
 sk_sp<SkData> SkColorSpace_Base::writeToICC() const {
     // Return if this object was created from a profile, or if we have already serialized
     // the profile.
@@ -1231,13 +1005,11 @@ sk_sp<SkData> SkColorSpace_Base::writeToICC() const {
     // Write TRC tags
     GammaNamed gammaNamed = this->gammaNamed();
     if (kNonStandard_GammaNamed == gammaNamed) {
-        // FIXME (msarett):
-        // Write the correct gamma representation rather than 2.2f.
-        write_trc_tag((uint32_t*) ptr, 2.2f);
+        write_trc_tag((uint32_t*) ptr, get_gamma_value(&as_CSB(this)->fGammas->fRed));
         ptr += SkAlign4(kTAG_TRC_Bytes);
-        write_trc_tag((uint32_t*) ptr, 2.2f);
+        write_trc_tag((uint32_t*) ptr, get_gamma_value(&as_CSB(this)->fGammas->fGreen));
         ptr += SkAlign4(kTAG_TRC_Bytes);
-        write_trc_tag((uint32_t*) ptr, 2.2f);
+        write_trc_tag((uint32_t*) ptr, get_gamma_value(&as_CSB(this)->fGammas->fBlue));
         ptr += SkAlign4(kTAG_TRC_Bytes);
     } else {
         switch (gammaNamed) {
index 23b08ce..7091946 100644 (file)
@@ -17,8 +17,7 @@ class ColorSpaceXformTest {
 public:
     static std::unique_ptr<SkColorSpaceXform> CreateIdentityXform(const sk_sp<SkGammas>& gammas) {
         // Logically we can pass any matrix here.  For simplicty, pass I(), i.e. D50 XYZ gamut.
-        sk_sp<SkColorSpace> space(new SkColorSpace_Base(
-                nullptr, SkColorSpace::kNonStandard_GammaNamed, gammas, SkMatrix::I(), nullptr));
+        sk_sp<SkColorSpace> space(new SkColorSpace_Base(nullptr, gammas, SkMatrix::I(), nullptr));
         return SkColorSpaceXform::New(space, space);
     }
 };
@@ -55,99 +54,53 @@ static void test_identity_xform(skiatest::Reporter* r, const sk_sp<SkGammas>& ga
 
 DEF_TEST(ColorSpaceXform_TableGamma, r) {
     // Lookup-table based gamma curves
+    SkGammaCurve red, green, blue;
     constexpr size_t tableSize = 10;
-    void* memory = sk_malloc_throw(sizeof(SkGammas) + sizeof(float) * tableSize);
-    sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-    gammas->fRedType = gammas->fGreenType = gammas->fBlueType = SkGammas::Type::kTable_Type;
-    gammas->fRedData.fTable.fSize = gammas->fGreenData.fTable.fSize =
-            gammas->fBlueData.fTable.fSize = tableSize;
-    gammas->fRedData.fTable.fOffset = gammas->fGreenData.fTable.fOffset =
-            gammas->fBlueData.fTable.fOffset = 0;
-    float* table = SkTAddOffset<float>(memory, sizeof(SkGammas));
-
-    table[0] = 0.00f;
-    table[1] = 0.05f;
-    table[2] = 0.10f;
-    table[3] = 0.15f;
-    table[4] = 0.25f;
-    table[5] = 0.35f;
-    table[6] = 0.45f;
-    table[7] = 0.60f;
-    table[8] = 0.75f;
-    table[9] = 1.00f;
+    red.fTable = std::unique_ptr<float[]>(new float[tableSize]);
+    green.fTable = std::unique_ptr<float[]>(new float[tableSize]);
+    blue.fTable = std::unique_ptr<float[]>(new float[tableSize]);
+    red.fTableSize = green.fTableSize = blue.fTableSize = 10;
+    red.fTable[0] = green.fTable[0] = blue.fTable[0] = 0.00f;
+    red.fTable[1] = green.fTable[1] = blue.fTable[1] = 0.05f;
+    red.fTable[2] = green.fTable[2] = blue.fTable[2] = 0.10f;
+    red.fTable[3] = green.fTable[3] = blue.fTable[3] = 0.15f;
+    red.fTable[4] = green.fTable[4] = blue.fTable[4] = 0.25f;
+    red.fTable[5] = green.fTable[5] = blue.fTable[5] = 0.35f;
+    red.fTable[6] = green.fTable[6] = blue.fTable[6] = 0.45f;
+    red.fTable[7] = green.fTable[7] = blue.fTable[7] = 0.60f;
+    red.fTable[8] = green.fTable[8] = blue.fTable[8] = 0.75f;
+    red.fTable[9] = green.fTable[9] = blue.fTable[9] = 1.00f;
+    sk_sp<SkGammas> gammas =
+            sk_make_sp<SkGammas>(std::move(red), std::move(green), std::move(blue));
     test_identity_xform(r, gammas);
 }
 
 DEF_TEST(ColorSpaceXform_ParametricGamma, r) {
     // Parametric gamma curves
-    void* memory = sk_malloc_throw(sizeof(SkGammas) + sizeof(SkGammas::Params));
-    sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-    gammas->fRedType = gammas->fGreenType = gammas->fBlueType = SkGammas::Type::kParam_Type;
-    gammas->fRedData.fParamOffset = gammas->fGreenData.fParamOffset =
-            gammas->fBlueData.fParamOffset = 0;
-    SkGammas::Params* params = SkTAddOffset<SkGammas::Params>(memory, sizeof(SkGammas));
+    SkGammaCurve red, green, blue;
 
     // Interval, switch xforms at 0.0031308f
-    params->fD = 0.04045f;
+    red.fD = green.fD = blue.fD = 0.04045f;
 
     // First equation:
-    params->fE = 1.0f / 12.92f;
-    params->fF = 0.0f;
+    red.fE = green.fE = blue.fE = 1.0f / 12.92f;
 
     // Second equation:
     // Note that the function is continuous (it's actually sRGB).
-    params->fA = 1.0f / 1.055f;
-    params->fB = 0.055f / 1.055f;
-    params->fC = 0.0f;
-    params->fG = 2.4f;
+    red.fA = green.fA = blue.fA = 1.0f / 1.055f;
+    red.fB = green.fB = blue.fB = 0.055f / 1.055f;
+    red.fC = green.fC = blue.fC = 0.0f;
+    red.fG = green.fG = blue.fG = 2.4f;
+    sk_sp<SkGammas> gammas =
+            sk_make_sp<SkGammas>(std::move(red), std::move(green), std::move(blue));
     test_identity_xform(r, gammas);
 }
 
 DEF_TEST(ColorSpaceXform_ExponentialGamma, r) {
     // Exponential gamma curves
-    sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new SkGammas());
-    gammas->fRedType = gammas->fGreenType = gammas->fBlueType = SkGammas::Type::kValue_Type;
-    gammas->fRedData.fValue = gammas->fGreenData.fValue = gammas->fBlueData.fValue = 1.4f;
-    test_identity_xform(r, gammas);
-}
-
-DEF_TEST(ColorSpaceXform_NonMatchingGamma, r) {
-    constexpr size_t tableSize = 10;
-    void* memory = sk_malloc_throw(sizeof(SkGammas) + sizeof(float) * tableSize +
-                                   sizeof(SkGammas::Params));
-    sk_sp<SkGammas> gammas = sk_sp<SkGammas>(new (memory) SkGammas());
-
-    float* table = SkTAddOffset<float>(memory, sizeof(SkGammas));
-    table[0] = 0.00f;
-    table[1] = 0.15f;
-    table[2] = 0.20f;
-    table[3] = 0.25f;
-    table[4] = 0.35f;
-    table[5] = 0.45f;
-    table[6] = 0.55f;
-    table[7] = 0.70f;
-    table[8] = 0.85f;
-    table[9] = 1.00f;
-
-    SkGammas::Params* params = SkTAddOffset<SkGammas::Params>(memory, sizeof(SkGammas) +
-                                                              sizeof(float) * tableSize);
-    params->fA = 1.0f / 1.055f;
-    params->fB = 0.055f / 1.055f;
-    params->fC = 0.0f;
-    params->fD = 0.04045f;
-    params->fE = 1.0f / 12.92f;
-    params->fF = 0.0f;
-    params->fG = 2.4f;
-
-    gammas->fRedType = SkGammas::Type::kValue_Type;
-    gammas->fRedData.fValue = 1.2f;
-
-    gammas->fGreenType = SkGammas::Type::kTable_Type;
-    gammas->fGreenData.fTable.fSize = tableSize;
-    gammas->fGreenData.fTable.fOffset = 0;
-
-    gammas->fBlueType = SkGammas::Type::kParam_Type;
-    gammas->fBlueData.fParamOffset = sizeof(float) * tableSize;
-
+    SkGammaCurve red, green, blue;
+    red.fValue = green.fValue = blue.fValue = 1.4f;
+    sk_sp<SkGammas> gammas =
+            sk_make_sp<SkGammas>(std::move(red), std::move(green), std::move(blue));
     test_identity_xform(r, gammas);
 }