Rearrange ICC profile parsing

None of the small details have changed, just some high level
reorganization:
(1) Check for XYZ spaces before A2B.
(2) If we fail to parse the XYZ space, fallback by trying to
    parse the A2B space.

This should cause no image diffs on Gold.

There is an image from the ICC website that is *supposed* to
test that we parse the A2B tag before the XYZ tag.  Our
behavior on this image will actually not change - the XYZ
tag is invalid (non-D50 matrix), so we fall back to A2B
anyway.  I think this behavior is ok.

BUG:674584

Change-Id: I271fd990937268e03e98f5037a0837a574e775ef
Reviewed-on: https://skia-review.googlesource.com/6143
Reviewed-by: Robert Aftias <raftias@google.com>
Commit-Queue: Matt Sarett <msarett@google.com>

src/core/SkColorSpace_A2B.h

@@ -166,10 +166,10 @@ public:
 
     InputColorFormat inputColorFormat() const { return fInputColorFormat; }
 
-private:
     SkColorSpace_A2B(InputColorFormat inputColorFormat, std::vector<Element> elements, PCS pcs,
                      sk_sp<SkData> profileData);
 
+private:
     InputColorFormat     fInputColorFormat;
     std::vector<Element> fElements;
     PCS                  fPCS;

src/core/SkColorSpace_Base.h

@@ -183,6 +183,8 @@ public:
     static sk_sp<SkColorSpace> MakeICC(const void* input, size_t len,
                                        InputColorFormat inputColorFormat);
 
+    static sk_sp<SkColorSpace> MakeRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50);
+
 protected:
     SkColorSpace_Base(sk_sp<SkData> profileData);
 
@@ -197,8 +199,6 @@ private:
      */
     sk_sp<SkData> writeToICC() const;
 
-    static sk_sp<SkColorSpace> MakeRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50);
-
     SkColorSpace_Base(SkGammaNamed gammaNamed, const SkMatrix44& toXYZ);
 
     sk_sp<SkData> fProfileData;

src/core/SkColorSpace_ICC.cpp

@@ -1322,109 +1322,20 @@ static inline bool is_close_to_d50(const SkMatrix44& matrix) {
            (SkTAbs(Z - kD50_WhitePoint[2]) <= 0.04f);
 }
 
-sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) {
+static sk_sp<SkColorSpace> make_xyz(const ICCProfileHeader& header, ICCTag* tags, int tagCount,
-    return SkColorSpace_Base::MakeICC(input, len, SkColorSpace_Base::InputColorFormat::kRGB);
+                                    const uint8_t* base, sk_sp<SkData> profileData) {
-}
+    if (kLAB_PCSSpace == header.fPCS) {
-
-sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
-                                               InputColorFormat inputColorFormat) {
-    if (!input || len < kICCHeaderSize) {
-        return_null("Data is null or not large enough to contain an ICC profile");
-    }
-
-    // Create our own copy of the input.
-    void* memory = sk_malloc_throw(len);
-    memcpy(memory, input, len);
-    sk_sp<SkData> profileData = SkData::MakeFromMalloc(memory, len);
-    const uint8_t* base = profileData->bytes();
-    const uint8_t* ptr = base;
-
-    // Read the ICC profile header and check to make sure that it is valid.
-    ICCProfileHeader header;
-    header.init(ptr, len);
-    if (!header.valid()) {
         return nullptr;
     }
 
-    switch (inputColorFormat) {
-        case InputColorFormat::kRGB:
-            if (header.fInputColorSpace != kRGB_ColorSpace) {
-                return_null("Provided input color format (RGB) does not match profile.\n");
-            }
-            break;
-        case InputColorFormat::kCMYK:
-            if (header.fInputColorSpace != kCMYK_ColorSpace) {
-                return_null("Provided input color format (CMYK) does not match profile.\n");
-            }
-            break;
-        case InputColorFormat::kGray:
-            if (header.fInputColorSpace != kGray_ColorSpace) {
-                return_null("Provided input color format (Gray) does not match profile.\n");
-            }
-            break;
-        default:
-            return_null("Provided input color format not supported");
-    }
-
-    // Adjust ptr and len before reading the tags.
-    if (len < header.fSize) {
-        SkColorSpacePrintf("ICC profile might be truncated.\n");
-    } else if (len > header.fSize) {
-        SkColorSpacePrintf("Caller provided extra data beyond the end of the ICC profile.\n");
-        len = header.fSize;
-    }
-    ptr += kICCHeaderSize;
-    len -= kICCHeaderSize;
-
-    // Parse tag headers.
-    uint32_t tagCount = header.fTagCount;
-    SkColorSpacePrintf("ICC profile contains %d tags.\n", tagCount);
-    if (len < kICCTagTableEntrySize * tagCount) {
-        return_null("Not enough input data to read tag table entries");
-    }
-
-    SkAutoTArray<ICCTag> tags(tagCount);
-    for (uint32_t i = 0; i < tagCount; i++) {
-        ptr = tags[i].init(ptr);
-        SkColorSpacePrintf("[%d] %c%c%c%c %d %d\n", i, (tags[i].fSignature >> 24) & 0xFF,
-                (tags[i].fSignature >> 16) & 0xFF, (tags[i].fSignature >>  8) & 0xFF,
-                (tags[i].fSignature >>  0) & 0xFF, tags[i].fOffset, tags[i].fLength);
-
-        if (!tags[i].valid(kICCHeaderSize + len)) {
-            return_null("Tag is too large to fit in ICC profile");
-        }
-    }
-
-    // Recognize color profile specified by A2B0 tag.
-    // this must be done before XYZ profile checking, as a profile can have both
-    // in which case we should use the A2B case to be accurate
-    // (XYZ is there as a fallback / quick preview)
-    const ICCTag* a2b0 = ICCTag::Find(tags.get(), tagCount, kTAG_A2B0);
-    if (a2b0) {
-        const SkColorSpace_A2B::PCS pcs = kXYZ_PCSSpace == header.fPCS
-                                        ? SkColorSpace_A2B::PCS::kXYZ
-                                        : SkColorSpace_A2B::PCS::kLAB;
-        std::vector<SkColorSpace_A2B::Element> elements;
-        if (load_a2b0(&elements, a2b0->addr(base), a2b0->fLength, pcs, inputColorFormat)) {
-            return sk_sp<SkColorSpace>(new SkColorSpace_A2B(inputColorFormat, std::move(elements),
-                                                            pcs, std::move(profileData)));
-        }
-        SkColorSpacePrintf("Ignoring malformed A2B0 tag.\n");
-    }
-
-    // Lab PCS means the profile is required to be an n-component LUT-based
-    // profile, so monochrome/3-component matrix-based profiles can only have an XYZ PCS
-    if (kLAB_PCSSpace == header.fPCS) {
-        return_null("Invalid PCS. PCSLAB only supported on A2B0 profiles.");
-    }
-    SkASSERT(kXYZ_PCSSpace == header.fPCS);
-
-    if (kRGB_ColorSpace == header.fInputColorSpace) {
     // Recognize the rXYZ, gXYZ, and bXYZ tags.
-        const ICCTag* r = ICCTag::Find(tags.get(), tagCount, kTAG_rXYZ);
+    const ICCTag* r = ICCTag::Find(tags, tagCount, kTAG_rXYZ);
-        const ICCTag* g = ICCTag::Find(tags.get(), tagCount, kTAG_gXYZ);
+    const ICCTag* g = ICCTag::Find(tags, tagCount, kTAG_gXYZ);
-        const ICCTag* b = ICCTag::Find(tags.get(), tagCount, kTAG_bXYZ);
+    const ICCTag* b = ICCTag::Find(tags, tagCount, kTAG_bXYZ);
-        if (r && g && b) {
+    if (!r || !g || !b) {
+        return nullptr;
+    }
+
     float toXYZ[9];
     if (!load_xyz(&toXYZ[0], r->addr(base), r->fLength) ||
         !load_xyz(&toXYZ[3], g->addr(base), g->fLength) ||
@@ -1437,29 +1348,21 @@ sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
                toXYZ[3], toXYZ[4], toXYZ[5],
                toXYZ[6], toXYZ[7], toXYZ[8]);
     if (!is_close_to_d50(mat)) {
-                // QCMS treats these profiles as "bogus".  I'm not sure if that's
+        return_null("XYZ matrix is not D50");
-                // correct, but we certainly do not handle non-D50 matrices
-                // correctly.  So I'll disable this for now.
-                SkColorSpacePrintf("Matrix is not close to D50");
-                return nullptr;
     }
 
-            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,
+    // gammas are meant to be the same.
-            // but it simplifies the code below.
+    r = ICCTag::Find(tags, tagCount, kTAG_rTRC);
-            if ((!r || !g || !b) && (r || g || b)) {
+    g = ICCTag::Find(tags, tagCount, kTAG_gTRC);
+    b = ICCTag::Find(tags, tagCount, kTAG_bTRC);
+    if ((!r || !g || !b)) {
         if (!r) {
             r = g ? g : b;
         }
-
         if (!g) {
             g = r ? r : b;
         }
-
         if (!b) {
             b = r ? r : g;
         }
@@ -1555,8 +1458,14 @@ sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
 
     return SkColorSpace_Base::MakeRGB(gammaNamed, mat);
 }
-    } else if (kGray_ColorSpace == header.fInputColorSpace) {
+
-        const ICCTag* grayTRC = ICCTag::Find(tags.get(), tagCount, kTAG_kTRC);
+static sk_sp<SkColorSpace> make_gray(const ICCProfileHeader& header, ICCTag* tags, int tagCount,
+                                     const uint8_t* base, sk_sp<SkData> profileData) {
+    if (kLAB_PCSSpace == header.fPCS) {
+        return nullptr;
+    }
+
+    const ICCTag* grayTRC = ICCTag::Find(tags, tagCount, kTAG_kTRC);
     if (!grayTRC) {
         return_null("grayTRC tag required for monochrome profiles.");
     }
@@ -1582,15 +1491,121 @@ sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
         gammas->fData[0] = data;
         elements.push_back(SkColorSpace_A2B::Element(std::move(gammas)));
     }
-        return sk_sp<SkColorSpace>(new SkColorSpace_A2B(inputColorFormat,
+    return sk_sp<SkColorSpace>(new SkColorSpace_A2B(SkColorSpace_Base::InputColorFormat::kGray,
                                                     std::move(elements),
                                                     SkColorSpace_A2B::PCS::kXYZ,
                                                     std::move(profileData)));
 }
 
+static sk_sp<SkColorSpace> make_a2b(SkColorSpace_Base::InputColorFormat inputColorFormat, const ICCProfileHeader& header, ICCTag* tags, int tagCount, const uint8_t* base, sk_sp<SkData> profileData) {
+    const ICCTag* a2b0 = ICCTag::Find(tags, tagCount, kTAG_A2B0);
+    if (a2b0) {
+        const SkColorSpace_A2B::PCS pcs = kXYZ_PCSSpace == header.fPCS
+                                        ? SkColorSpace_A2B::PCS::kXYZ
+                                        : SkColorSpace_A2B::PCS::kLAB;
+        std::vector<SkColorSpace_A2B::Element> elements;
+        if (load_a2b0(&elements, a2b0->addr(base), a2b0->fLength, pcs, inputColorFormat)) {
+            return sk_sp<SkColorSpace>(new SkColorSpace_A2B(inputColorFormat, std::move(elements),
+                                                            pcs, std::move(profileData)));
+        }
+    }
+
+    return nullptr;
+}
+
+sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) {
+    return SkColorSpace_Base::MakeICC(input, len, SkColorSpace_Base::InputColorFormat::kRGB);
+}
+
+sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
+                                               InputColorFormat inputColorFormat) {
+    if (!input || len < kICCHeaderSize) {
+        return_null("Data is null or not large enough to contain an ICC profile");
+    }
+
+    // Create our own copy of the input.
+    void* memory = sk_malloc_throw(len);
+    memcpy(memory, input, len);
+    sk_sp<SkData> profileData = SkData::MakeFromMalloc(memory, len);
+    const uint8_t* base = profileData->bytes();
+    const uint8_t* ptr = base;
+
+    // Read the ICC profile header and check to make sure that it is valid.
+    ICCProfileHeader header;
+    header.init(ptr, len);
+    if (!header.valid()) {
+        return nullptr;
+    }
+
+    // Adjust ptr and len before reading the tags.
+    if (len < header.fSize) {
+        SkColorSpacePrintf("ICC profile might be truncated.\n");
+    } else if (len > header.fSize) {
+        SkColorSpacePrintf("Caller provided extra data beyond the end of the ICC profile.\n");
+        len = header.fSize;
+    }
+    ptr += kICCHeaderSize;
+    len -= kICCHeaderSize;
+
+    // Parse tag headers.
+    uint32_t tagCount = header.fTagCount;
+    SkColorSpacePrintf("ICC profile contains %d tags.\n", tagCount);
+    if (len < kICCTagTableEntrySize * tagCount) {
+        return_null("Not enough input data to read tag table entries");
+    }
+
+    SkAutoTArray<ICCTag> tags(tagCount);
+    for (uint32_t i = 0; i < tagCount; i++) {
+        ptr = tags[i].init(ptr);
+        SkColorSpacePrintf("[%d] %c%c%c%c %d %d\n", i, (tags[i].fSignature >> 24) & 0xFF,
+                (tags[i].fSignature >> 16) & 0xFF, (tags[i].fSignature >>  8) & 0xFF,
+                (tags[i].fSignature >>  0) & 0xFF, tags[i].fOffset, tags[i].fLength);
+
+        if (!tags[i].valid(kICCHeaderSize + len)) {
+            return_null("Tag is too large to fit in ICC profile");
+        }
+    }
+
+    switch (header.fInputColorSpace) {
+        case kRGB_ColorSpace: {
+            if (InputColorFormat::kRGB != inputColorFormat) {
+                return_null("Provided input color format (RGB) does not match profile.");
+            }
+
+            sk_sp<SkColorSpace> colorSpace =
+                    make_xyz(header, tags.get(), tagCount, base, profileData);
+            if (colorSpace) {
+                return colorSpace;
+            }
+
+            break;
+        }
+        case kGray_ColorSpace: {
+            if (InputColorFormat::kGray != inputColorFormat) {
+                return_null("Provided input color format (Gray) does not match profile.");
+            }
+
+            sk_sp<SkColorSpace> colorSpace =
+                    make_gray(header, tags.get(), tagCount, base, profileData);
+            if (colorSpace) {
+                return colorSpace;
+            }
+
+            break;
+        }
+        case kCMYK_ColorSpace:
+            if (InputColorFormat::kCMYK != inputColorFormat) {
+                return_null("Provided input color format (CMYK) does not match profile.");
+            }
+
+            break;
+        default:
             return_null("ICC profile contains unsupported colorspace");
     }
 
+    return make_a2b(inputColorFormat, header, tags.get(), tagCount, base, profileData);
+}
+
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 // We will write a profile with the minimum nine required tags.

src/core/SkColorSpace_XYZ.h

@@ -34,12 +34,12 @@ public:
 
     void toDstGammaTables(const uint8_t* tables[3], sk_sp<SkData>* storage, int numTables) const;
 
-private:
     SkColorSpace_XYZ(SkGammaNamed gammaNamed, const SkMatrix44& toXYZ);
 
     SkColorSpace_XYZ(SkGammaNamed gammaNamed, sk_sp<SkGammas> gammas,
                      const SkMatrix44& toXYZ, sk_sp<SkData> profileData);
 
+private:
     const SkGammaNamed     fGammaNamed;
     sk_sp<SkGammas>        fGammas;
     const SkMatrix44       fToXYZD50;