Refactored SkColorSpace and added in a Lab PCS GM

src/core/SkColorSpace_ICC.cpp

Index: src/core/SkColorSpace_ICC.cpp
diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp
index 4ef9f2b0a83146e3a39818e4e388640b5953913a..6e660489602e1bcb5dc98370fa6eb07e541c8f2d 100755
--- a/src/core/SkColorSpace_ICC.cpp
+++ b/src/core/SkColorSpace_ICC.cpp
@@ -7,6 +7,8 @@
 
 #include "SkColorSpace.h"
 #include "SkColorSpace_Base.h"
+#include "SkColorSpace_A2B0.h"

[msarett, 2016/10/11 13:40:31]

nit: Alphabetical order

+#include "SkColorSpace_XYZTRC.h"
 #include "SkColorSpacePriv.h"
 #include "SkEndian.h"
 #include "SkFixed.h"
@@ -52,6 +54,7 @@ static constexpr uint32_t kInput_Profile      = SkSetFourByteTag('s', 'c', 'n',
 static constexpr uint32_t kOutput_Profile     = SkSetFourByteTag('p', 'r', 't', 'r');
 static constexpr uint32_t kColorSpace_Profile = SkSetFourByteTag('s', 'p', 'a', 'c');
 static constexpr uint32_t kXYZ_PCSSpace       = SkSetFourByteTag('X', 'Y', 'Z', ' ');
+static constexpr uint32_t kLAB_PCSSpace       = SkSetFourByteTag('L', 'a', 'b', ' ');
 static constexpr uint32_t kACSP_Signature     = SkSetFourByteTag('a', 'c', 's', 'p');
 
 struct ICCProfileHeader {
@@ -130,7 +133,7 @@ struct ICCProfileHeader {
 
         // TODO (msarett):
         // All the profiles we've tested so far use XYZ as the profile connection space.
-        return_if_false(fPCS == kXYZ_PCSSpace, "Unsupported PCS space");
+        return_if_false(fPCS == kXYZ_PCSSpace || fPCS == kLAB_PCSSpace, "Unsupported PCS space");
 
         return_if_false(fSignature == kACSP_Signature, "Bad signature");
 
@@ -689,14 +692,12 @@ static bool load_color_lut(sk_sp<SkColorLookUpTable>* colorLUT, uint32_t inputCh
     return true;
 }
 
-static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
+static bool load_matrix(SkMatrix44* toXYZ, const uint8_t* src, size_t len, float scale = 1.f) {

[msarett, 2016/10/11 13:40:31]

I think you've got the scale logic correct (in that Lab doesn't need it).

But, let's require this parameter and make sure to pass in the correct value?

     if (len < 48) {
         SkColorSpacePrintf("Matrix tag is too small (%d bytes).", len);
         return false;
     }
 
-    // For this matrix to behave like our "to XYZ D50" matrices, it needs to be scaled.
-    constexpr float scale = 65535.0 / 32768.0;
     float array[16];
     array[ 0] = scale * SkFixedToFloat(read_big_endian_i32(src));
     array[ 1] = scale * SkFixedToFloat(read_big_endian_i32(src + 4));
@@ -729,9 +730,115 @@ static inline SkGammaNamed is_named(const sk_sp<SkGammas>& gammas) {
     return kNonStandard_SkGammaNamed;
 }
 
+/**
+ *  Parse and load an entire stored curve. Handles invalid gammas as well.
+ *
+ *  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 gammaNamed Out-variable. The named gamma curve.
+ *  @param gammas     Out-variable. The stored gamma curve information. Can be null if
+ *                    gammaNamed is a named curve
+ *  @param rTagPtr    Pointer to start of the gamma tag.
+ *  @param taglen     The size in bytes of the tag
+ *
+ *  @return           false on failure, true on success
+ */
+static bool parse_and_load_gamma(SkGammaNamed* gammaNamed, sk_sp<SkGammas>* gammas,

[msarett, 2016/10/11 13:40:31]

Thanks for sharing this code :)

+                                 const uint8_t* rTagPtr, size_t tagLen)
+{
+    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);
+            return_if_false(safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize),
+                            "SkGammas struct is too large to allocate");
+            void* memory = sk_malloc_throw(allocSize);
+            *gammas = sk_sp<SkGammas>(new (memory) SkGammas());
+            load_gammas(memory, 0, rType, &rData, rParams, rTagPtr);
 
-static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT, SkGammaNamed* gammaNamed,
-                      sk_sp<SkGammas>* gammas, SkMatrix44* toXYZ, const uint8_t* src, size_t len) {
+            (*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);
+        return_if_false(safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize),
+                        "SkGammas struct is too large to allocate");
+        return_if_false(safe_add(allocSize, gamma_alloc_size(gType, gData), &allocSize),
+                        "SkGammas struct is too large to allocate");
+        return_if_false(safe_add(allocSize, gamma_alloc_size(bType, bData), &allocSize),
+                        "SkGammas struct is too large to allocate");
+        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 (kNonStandard_SkGammaNamed == *gammaNamed) {
+        *gammaNamed = is_named(*gammas);
+        if (kNonStandard_SkGammaNamed != *gammaNamed) {
+            // No need to keep the gammas struct, the enum is enough.
+            *gammas = nullptr;
+        }
+    }
+    return true;
+}
+
+static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT,

[msarett, 2016/10/11 13:40:31]

I'm pleasantly surprised by your changes to this function.  If anything I find
it easier to understand :).

+                      SkGammaNamed* aCurveNamed, sk_sp<SkGammas>* aCurve,
+                      SkGammaNamed* mCurveNamed, sk_sp<SkGammas>* mCurve,
+                      SkGammaNamed* bCurveNamed, sk_sp<SkGammas>* bCurve,
+                      SkMatrix44* matrix, const uint8_t* src, size_t len) {
     if (len < 32) {
         SkColorSpacePrintf("A to B tag is too small (%d bytes).", len);
         return false;
@@ -757,18 +864,13 @@ static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT, SkGammaNamed* gammaNa
         return false;
     }
 
-    // Read the offsets of each element in the A to B tag.  With the exception of A curves and
-    // B curves (which we do not yet support), we will handle these elements in the order in
-    // which they should be applied (rather than the order in which they occur in the tag).
     // If the offset is non-zero it indicates that the element is present.
     uint32_t offsetToACurves = read_big_endian_i32(src + 28);
-    uint32_t offsetToBCurves = read_big_endian_i32(src + 12);
-    if ((0 != offsetToACurves) || (0 != offsetToBCurves)) {
-        // FIXME (msarett): Handle A and B curves.
-        // Note that the A curve is technically required in order to have a color LUT.
-        // However, all the A curves I have seen so far have are just placeholders that
-        // don't actually transform the data.
-        SkColorSpacePrintf("Ignoring A and/or B curve.  Output may be wrong.\n");
+    if (0 != offsetToACurves && offsetToACurves < len) {
+        const size_t tagLen = len - offsetToACurves;
+        if (!parse_and_load_gamma(aCurveNamed, aCurve, src + offsetToACurves, tagLen)) {
+            return false;
+        }
     }
 
     uint32_t offsetToColorLUT = read_big_endian_i32(src + 24);
@@ -776,107 +878,31 @@ static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT, SkGammaNamed* gammaNa
         if (!load_color_lut(colorLUT, inputChannels, src + offsetToColorLUT,
                             len - offsetToColorLUT)) {
             SkColorSpacePrintf("Failed to read color LUT from A to B tag.\n");
+            return false;
         }
     }
 
     uint32_t offsetToMCurves = read_big_endian_i32(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);
-                return_if_false(safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize),
-                                "SkGammas struct is too large to allocate");
-                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);
-            return_if_false(safe_add(allocSize, gamma_alloc_size(rType, rData), &allocSize),
-                            "SkGammas struct is too large to allocate");
-            return_if_false(safe_add(allocSize, gamma_alloc_size(gType, gData), &allocSize),
-                            "SkGammas struct is too large to allocate");
-            return_if_false(safe_add(allocSize, gamma_alloc_size(bType, bData), &allocSize),
-                            "SkGammas struct is too large to allocate");
-            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;
-        }
-    } else {
-        // Guess sRGB if the chunk is missing a transfer function.
-        *gammaNamed = kSRGB_SkGammaNamed;
-    }
-
-    if (kNonStandard_SkGammaNamed == *gammaNamed) {
-        *gammaNamed = is_named(*gammas);
-        if (kNonStandard_SkGammaNamed != *gammaNamed) {
-            // No need to keep the gammas struct, the enum is enough.
-            *gammas = nullptr;
+        const size_t tagLen = len - offsetToMCurves;
+        if (!parse_and_load_gamma(mCurveNamed, mCurve, src + offsetToMCurves, tagLen)) {
+            return false;
         }
     }
 
     uint32_t offsetToMatrix = read_big_endian_i32(src + 16);
     if (0 != offsetToMatrix && offsetToMatrix < len) {
-        if (!load_matrix(toXYZ, src + offsetToMatrix, len - offsetToMatrix)) {
+        if (!load_matrix(matrix, src + offsetToMatrix, len - offsetToMatrix)) {

[msarett, 2016/10/11 13:40:31]

Pass in encodingFactor here?

Reading the spec, it looks to me like it should be 32768/65535 for XYZ and 1.0
for Lab.

             SkColorSpacePrintf("Failed to read matrix from A to B tag.\n");
-            toXYZ->setIdentity();
+            matrix->setIdentity();
+        }
+    }
+    
+    uint32_t offsetToBCurves = read_big_endian_i32(src + 12);
+    if (0 != offsetToBCurves && offsetToBCurves < len) {
+        const size_t tagLen = len - offsetToBCurves;
+        if (!parse_and_load_gamma(bCurveNamed, bCurve, src + offsetToBCurves, tagLen)) {
+            return false;
         }
     }
 
@@ -1071,11 +1097,31 @@ sk_sp<SkColorSpace> SkColorSpace::NewICC(const void* input, size_t len) {
                     // It's possible that we'll initially detect non-matching gammas, only for
                     // them to evaluate to the same named gamma curve.
                     gammaNamed = is_named(gammas);
-                    if (kNonStandard_SkGammaNamed == gammaNamed) {
-                        return sk_sp<SkColorSpace>(new SkColorSpace_Base(nullptr, gammaNamed,
-                                                                         std::move(gammas), mat,
-                                                                         std::move(data)));
-                    }
+                }
+                    
+                    
+                // if we're a LAB profile, but specified as a 3-component matrix-based profile

[msarett, 2016/10/11 13:40:31]

Can we hold off on this until we're sure that we need it?

Maybe just fail for now (closer to the top of this fn).  Then we can investigate
the right thing to do for XYZ/A2B/Lab profiles in another CL...

+                // then we should just construct an A2B0 profile from it, as it will support PCS
+                // conversions during decoding - however, it is actually against the ICC specs
+                // for a 3-component matrix-based profile to have a non-XYZ PCS
+                if (header.fPCS == kLAB_PCSSpace) {
+                    return sk_sp<SkColorSpace>(new SkColorSpace_A2B0(kLinear_SkGammaNamed, nullptr,
+                                                                     nullptr,
+                                                                     gammaNamed, std::move(gammas),
+                                                                     mat,
+                                                                     kLinear_SkGammaNamed, nullptr,
+                                                                     SkColorSpace_A2B0::PCS::kLAB,
+                                                                     std::move(data)));
+                }
+                
+                // Handle XYZ PCS cases of the XYZTRC profiles now.
+                // However, Lab/XYZ are the only specified ICC PCS spaces right now
+                SkASSERT(header.fPCS == kXYZ_PCSSpace);
+                
+                if (kNonStandard_SkGammaNamed == gammaNamed) {
+                    return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(gammaNamed,
+                                                                       std::move(gammas),
+                                                                       mat, std::move(data)));
                 }
 
                 return SkColorSpace_Base::NewRGB(gammaNamed, mat);
@@ -1084,22 +1130,45 @@ 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) {
-                SkGammaNamed gammaNamed = kNonStandard_SkGammaNamed;
-                sk_sp<SkGammas> gammas = nullptr;
+                // default to Linear transforms in case curves are not there
+                SkGammaNamed aCurveNamed = kLinear_SkGammaNamed;
+                SkGammaNamed mCurveNamed = kLinear_SkGammaNamed;
+                SkGammaNamed bCurveNamed = kLinear_SkGammaNamed;
+                sk_sp<SkGammas> aCurve = nullptr;
+                sk_sp<SkGammas> mCurve = nullptr;
+                sk_sp<SkGammas> bCurve = nullptr;
                 sk_sp<SkColorLookUpTable> colorLUT = nullptr;
-                SkMatrix44 toXYZ(SkMatrix44::kUninitialized_Constructor);
-                if (!load_a2b0(&colorLUT, &gammaNamed, &gammas, &toXYZ, a2b0->addr(base),
-                               a2b0->fLength)) {
+                SkMatrix44 matrix(SkMatrix44::kUninitialized_Constructor);
+                if (!load_a2b0(&colorLUT, &aCurveNamed, &aCurve, &mCurveNamed, &mCurve,
+                               &bCurveNamed, &bCurve, &matrix, a2b0->addr(base), a2b0->fLength)) {
                     return_null("Failed to parse A2B0 tag");
                 }
 
-                if (colorLUT || kNonStandard_SkGammaNamed == gammaNamed) {
-                    return sk_sp<SkColorSpace>(new SkColorSpace_Base(std::move(colorLUT),
-                                                                     gammaNamed, std::move(gammas),
-                                                                     toXYZ, std::move(data)));
+                SkColorSpace_A2B0::PCS pcs = SkColorSpace_A2B0::PCS::kLAB;
+                if (header.fPCS == kXYZ_PCSSpace) {

[msarett, 2016/10/11 13:40:31]

Can we delete this too?  Do we need it anywhere to pass tests?  There are two
images that will look wrong for a bit, but that's fine.  Will remind us to
finish implementing.

+                    pcs = SkColorSpace_A2B0::PCS::kXYZ;
+                    // TODO (raftias): remove this return branch once A2B0-aware colorxforms
+                    // exist. This is just here to avoid breaking existing functionality where
+                    // A2B0 profiles were used but only the PCS matrix/b-curve were used
+                    // which is something that you can store in a SkColorSpace_XYZTRC
+                    SkColorSpacePrintf("Treating A2B0 profile as one specified as XYZ matrices/TRCs\n");

[msarett, 2016/10/11 13:40:31]

nit: 100 chars

+                    SkColorSpacePrintf("Output might not be correct.\n");
+                    SkMatrix44 scale(SkMatrix44::kIdentity_Constructor);
+                    // to match the XYZ matrices extracted earlier
+                    scale.setScale((float) (65535.0 / 32768.0));
+                    SkMatrix44 scaledMatrix(matrix, scale);
+                    return sk_sp<SkColorSpace>(new SkColorSpace_XYZTRC(mCurveNamed,
+                                                                       std::move(mCurve),
+                                                                       scaledMatrix,
+                                                                       std::move(data)));
                 }
-
-                return SkColorSpace_Base::NewRGB(gammaNamed, toXYZ);
+              
+                return sk_sp<SkColorSpace>(new SkColorSpace_A2B0(aCurveNamed, std::move(aCurve),
+                                                                 std::move(colorLUT),
+                                                                 mCurveNamed, std::move(mCurve),
+                                                                 matrix,
+                                                                 bCurveNamed, std::move(bCurve),
+                                                                 pcs, std::move(data)));
             }
         }
         default:
@@ -1244,13 +1313,17 @@ sk_sp<SkData> SkColorSpace_Base::writeToICC() const {
     if (fProfileData) {
         return fProfileData;
     }
+    // Profile Data is be mandatory for A2B0 Color Spaces
+    SkASSERT(type() == Type::kXYZTRC);
 
     // The client may create an SkColorSpace using an SkMatrix44, but currently we only
     // support writing profiles with 3x3 matrices.
     // TODO (msarett): Fix this!
-    if (0.0f != fToXYZD50.getFloat(3, 0) || 0.0f != fToXYZD50.getFloat(3, 1) ||
-        0.0f != fToXYZD50.getFloat(3, 2) || 0.0f != fToXYZD50.getFloat(0, 3) ||
-        0.0f != fToXYZD50.getFloat(1, 3) || 0.0f != fToXYZD50.getFloat(2, 3))
+    const SkColorSpace_XYZTRC* thisXYZ = static_cast<const SkColorSpace_XYZTRC*>(this);
+    const SkMatrix44& toXYZD50 = *thisXYZ->toXYZD50();
+    if (0.0f != toXYZD50.getFloat(3, 0) || 0.0f != toXYZD50.getFloat(3, 1) ||
+        0.0f != toXYZD50.getFloat(3, 2) || 0.0f != toXYZD50.getFloat(0, 3) ||
+        0.0f != toXYZD50.getFloat(1, 3) || 0.0f != toXYZD50.getFloat(2, 3))
     {
         return nullptr;
     }
@@ -1271,15 +1344,15 @@ sk_sp<SkData> SkColorSpace_Base::writeToICC() const {
     ptr += sizeof(gEmptyTextTag);
 
     // Write XYZ tags
-    write_xyz_tag((uint32_t*) ptr, fToXYZD50, 0);
+    write_xyz_tag((uint32_t*) ptr, toXYZD50, 0);
     ptr += kTAG_XYZ_Bytes;
-    write_xyz_tag((uint32_t*) ptr, fToXYZD50, 1);
+    write_xyz_tag((uint32_t*) ptr, toXYZD50, 1);
     ptr += kTAG_XYZ_Bytes;
-    write_xyz_tag((uint32_t*) ptr, fToXYZD50, 2);
+    write_xyz_tag((uint32_t*) ptr, toXYZD50, 2);
     ptr += kTAG_XYZ_Bytes;
 
     // Write TRC tags
-    SkGammaNamed gammaNamed = this->gammaNamed();
+    SkGammaNamed gammaNamed = thisXYZ->gammaNamed();
     if (kNonStandard_SkGammaNamed == gammaNamed) {
         // FIXME (msarett):
         // Write the correct gamma representation rather than 2.2f.