Refactored SkColorSpace_A2B to allow arbitrary ordering of elements

This is essential for representing non-lutAtoBType A2B tags such as
lut16Type, lut8Type, mpet. Parsing of A2B0 tags was also moved ahead
of the TRC/XYZ-matrix parsing, as profiles examined with both tags
either had the TRC/XYZ tags as a fall-back or were incorrectly displayed
if only the TRC/XYZ tags were used.

This was submitted alone to reduce CL size. Tests that will use these changes will be introduced in the subsequent CLs that add on lut8/16Type A2B0 parsing. We already have lut16Type test images and these have been tested locally, but require additional code not submitted yet for lut16Type ICC profile parsing and A2B colorspace xforms.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2444553002

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

4 files changed, 180 insertions, 118 deletions

diff --git a/gm/labpcsdemo.cpp b/gm/labpcsdemo.cpp
index f9fffe1209..d2a4ba9535 100644
--- a/gm/labpcsdemo.cpp
+++ b/gm/labpcsdemo.cpp
@@ -175,14 +175,27 @@ protected:
         if (convertLabToXYZ) {
             SkASSERT(SkColorSpace_Base::Type::kA2B == as_CSB(colorSpace)->type());
             SkColorSpace_A2B& cs = *static_cast<SkColorSpace_A2B*>(colorSpace.get());
+            const SkColorLookUpTable* colorLUT = nullptr;
             bool printConversions = false;
-            SkASSERT(cs.colorLUT());
             // We're skipping evaluating the TRCs and the matrix here since they aren't
             // in the ICC profile initially used here.
-            SkASSERT(kLinear_SkGammaNamed == cs.aCurveNamed());
-            SkASSERT(kLinear_SkGammaNamed == cs.mCurveNamed());
-            SkASSERT(kLinear_SkGammaNamed == cs.bCurveNamed());
-            SkASSERT(cs.matrix().isIdentity());
+            for (size_t e = 0; e < cs.count(); ++e) {
+                switch (cs.element(e).type()) {
+                    case SkColorSpace_A2B::Element::Type::kGammaNamed:
+                        SkASSERT(kLinear_SkGammaNamed == cs.element(e).gammaNamed());
+                        break;
+                    case SkColorSpace_A2B::Element::Type::kGammas:
+                        SkASSERT(false);
+                        break;
+                    case SkColorSpace_A2B::Element::Type::kCLUT:
+                        colorLUT = &cs.element(e).colorLUT();
+                        break;
+                    case SkColorSpace_A2B::Element::Type::kMatrix:
+                        SkASSERT(cs.element(e).matrix().isIdentity());
+                        break;
+                }
+            }
+            SkASSERT(colorLUT);
             for (int y = 0; y < imageHeight; ++y) {
                 for (int x = 0; x < imageWidth; ++x) {
                     uint32_t& p = *bitmap.getAddr32(x, y);
@@ -195,7 +208,7 @@ protected:
 
                     float lab[4] = { r * (1.f/255.f), g * (1.f/255.f), b * (1.f/255.f), 1.f };
                     
-                    interp_3d_clut(lab, lab, cs.colorLUT());
+                    interp_3d_clut(lab, lab, colorLUT);
                     
                     // Lab has ranges [0,100] for L and [-128,127] for a and b
                     // but the ICC profile loader stores as [0,1]. The ICC
diff --git a/src/core/SkColorSpace_A2B.cpp b/src/core/SkColorSpace_A2B.cpp
index f67da48259..9fed82379a 100644
--- a/src/core/SkColorSpace_A2B.cpp
+++ b/src/core/SkColorSpace_A2B.cpp
@@ -7,20 +7,9 @@
 
 #include "SkColorSpace_A2B.h"
 
-SkColorSpace_A2B::SkColorSpace_A2B(SkGammaNamed aCurveNamed, sk_sp<SkGammas> aCurve,
-                                   sk_sp<SkColorLookUpTable> colorLUT,
-                                   SkGammaNamed mCurveNamed, sk_sp<SkGammas> mCurve,
-                                   const SkMatrix44& matrix,
-                                   SkGammaNamed bCurveNamed, sk_sp<SkGammas> bCurve,
-                                   PCS pcs, sk_sp<SkData> profileData)
+SkColorSpace_A2B::SkColorSpace_A2B(PCS pcs, sk_sp<SkData> profileData,
+                                   std::vector<Element> elements)
     : INHERITED(std::move(profileData))
-    , fACurveNamed(aCurveNamed)
-    , fACurve(std::move(aCurve))
-    , fColorLUT(std::move(colorLUT))
-    , fMCurveNamed(mCurveNamed)
-    , fMCurve(std::move(mCurve))
-    , fMatrix(matrix)
-    , fBCurveNamed(bCurveNamed)
-    , fBCurve(std::move(bCurve))
     , fPCS(pcs)
+    , fElements(std::move(elements))
 {}
diff --git a/src/core/SkColorSpace_A2B.h b/src/core/SkColorSpace_A2B.h
index a0bd4a0319..844800588b 100644
--- a/src/core/SkColorSpace_A2B.h
+++ b/src/core/SkColorSpace_A2B.h
@@ -10,6 +10,8 @@
 
 #include "SkColorSpace_Base.h"
 
+#include <vector>
+
 // An alternative SkColorSpace that represents all the color space data that
 // is stored in an A2B0 ICC tag. This allows us to use alternative profile
 // connection spaces (CIELAB instead of just CIEXYZ), use color-lookup-tables
@@ -18,9 +20,8 @@
 // the potential to allow conversion from input color spaces with a different
 // number of channels such as CMYK (4) or GRAY (1), but that is not supported yet.
 //
-// Evaluation is done: A-curve => CLUT => M-curve => Matrix => B-curve
-//
-// There are also multi-processing-elements in the A2B0 tag which allow you to
+// Currently AtoBType A2B0 tag types are supported. There are also lut8Type,
+// lut16Type and MPET (multi-processing-elements) A2B0 tags which allow you to
 // combine these 3 primitives (TRC, CLUT, matrix) in any order/quantitiy,
 // but support for that is not implemented.
 class SkColorSpace_A2B : public SkColorSpace_Base {
@@ -55,21 +56,72 @@ public:
         return false;
     }
 
-    SkGammaNamed aCurveNamed() const { return fACurveNamed; }
-
-    const SkGammas* aCurve() const { return fACurve.get(); }
-
-    const SkColorLookUpTable* colorLUT() const { return fColorLUT.get(); }
+    Type type() const override { return Type::kA2B; }
 
-    SkGammaNamed mCurveNamed() const { return fMCurveNamed; }
+    class Element {
+    public:
+        explicit Element(SkGammaNamed gammaNamed)
+            : fType(Type::kGammaNamed)
+            , fGammaNamed(gammaNamed)
+            , fMatrix(SkMatrix44::kUninitialized_Constructor)
+        {}
+
+        explicit Element(sk_sp<SkGammas> gammas)
+            : fType(Type::kGammas)
+            , fGammas(std::move(gammas))
+            , fMatrix(SkMatrix44::kUninitialized_Constructor)  
+        {}
+
+        explicit Element(sk_sp<SkColorLookUpTable> colorLUT)
+            : fType(Type::kCLUT)
+            , fCLUT(std::move(colorLUT))
+            , fMatrix(SkMatrix44::kUninitialized_Constructor)
+        {}
+
+        explicit Element(const SkMatrix44& matrix)
+            : fType(Type::kMatrix)
+            , fMatrix(matrix)
+        {}
     
-    const SkGammas* mCurve() const { return fMCurve.get(); }
-
-    const SkMatrix44& matrix() const { return fMatrix; }
-
-    SkGammaNamed bCurveNamed() const { return fBCurveNamed; }
+        enum class Type {
+            kGammaNamed,
+            kGammas,
+            kCLUT,
+            kMatrix
+        };
+
+        Type type() const { return fType; }
+
+        SkGammaNamed gammaNamed() const {
+            SkASSERT(Type::kGammaNamed == fType);
+            return fGammaNamed;
+        }
+
+        const SkGammas& gammas() const {
+            SkASSERT(Type::kGammas == fType);
+            return *fGammas;
+        }
+
+        const SkColorLookUpTable& colorLUT() const {
+            SkASSERT(Type::kCLUT == fType);
+            return *fCLUT;
+        }
+
+        const SkMatrix44& matrix() const {
+            SkASSERT(Type::kMatrix == fType);
+            return fMatrix;
+        }
+
+    private:
+        Type                      fType;
+        SkGammaNamed              fGammaNamed;
+        sk_sp<SkGammas>           fGammas;
+        sk_sp<SkColorLookUpTable> fCLUT;
+        SkMatrix44                fMatrix;
+    };
+    const Element& element(size_t i) const { return fElements[i]; }
     
-    const SkGammas* bCurve() const { return fBCurve.get(); }
+    size_t count() const { return fElements.size(); }
 
     // the intermediate profile connection space that this color space
     // represents the transformation to
@@ -80,26 +132,12 @@ public:
     
     PCS pcs() const { return fPCS; }
 
-    Type type() const override { return Type::kA2B; }
-
 private:
-    SkColorSpace_A2B(SkGammaNamed aCurveNamed, sk_sp<SkGammas> aCurve,
-                     sk_sp<SkColorLookUpTable> colorLUT,
-                     SkGammaNamed mCurveNamed, sk_sp<SkGammas> mCurve,
-                     const SkMatrix44& matrix,
-                     SkGammaNamed bCurveNamed, sk_sp<SkGammas> bCurve,
-                     PCS pcs, sk_sp<SkData> profileData);
-
-    const SkGammaNamed        fACurveNamed;
-    sk_sp<SkGammas>           fACurve;
-    sk_sp<SkColorLookUpTable> fColorLUT;
-    const SkGammaNamed        fMCurveNamed;
-    sk_sp<SkGammas>           fMCurve;
-    SkMatrix44                fMatrix;
-    const SkGammaNamed        fBCurveNamed;
-    sk_sp<SkGammas>           fBCurve;
-    PCS                       fPCS;
-    
+    SkColorSpace_A2B(PCS pcs, sk_sp<SkData> profileData, std::vector<Element> elements);
+
+    PCS                  fPCS;
+    std::vector<Element> fElements;
+
     friend class SkColorSpace;
     typedef SkColorSpace_Base INHERITED;
 };
diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp
index 4a58cd682c..b0563c8459 100644
--- a/src/core/SkColorSpace_ICC.cpp
+++ b/src/core/SkColorSpace_ICC.cpp
@@ -131,9 +131,18 @@ struct ICCProfileHeader {
         // All the profiles we've tested so far use RGB as the input color space.
         return_if_false(fInputColorSpace == kRGB_ColorSpace, "Unsupported color space");
 
-        // TODO (msarett):
-        // All the profiles we've tested so far use XYZ as the profile connection space.
-        return_if_false(fPCS == kXYZ_PCSSpace || fPCS == kLAB_PCSSpace, "Unsupported PCS space");
+        switch (fPCS) {
+            case kXYZ_PCSSpace:
+                SkColorSpacePrintf("XYZ PCS\n");
+                break;
+            case kLAB_PCSSpace:
+                SkColorSpacePrintf("Lab PCS\n");
+                break;
+            default:
+                // ICC currently (V4.3) only specifices XYZ and Lab PCS spaces
+                SkColorSpacePrintf("Unsupported PCS space\n");
+                return false;
+        }
 
         return_if_false(fSignature == kACSP_Signature, "Bad signature");
 
@@ -724,6 +733,8 @@ static bool parse_and_load_gamma(SkGammaNamed* gammaNamed, sk_sp<SkGammas>* gamm
     SkGammas::Data rData;
     SkColorSpaceTransferFn rParams;
 
+    *gammaNamed = kNonStandard_SkGammaNamed;
+
     // 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;
@@ -807,27 +818,12 @@ static bool parse_and_load_gamma(SkGammaNamed* gammaNamed, sk_sp<SkGammas>* gamm
     return true;
 }
 
-static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT,
-                      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;
-    }
-
-    uint32_t type = read_big_endian_u32(src);
-    if (kTAG_AtoBType != type) {
-        // FIXME (msarett): Need to support lut8Type and lut16Type.
-        SkColorSpacePrintf("Unsupported A to B tag type.\n");
-        return false;
-    }
-
-    // Read the number of channels.  The four bytes that we skipped are reserved and
+bool load_a2b0_a_to_b_type(std::vector<SkColorSpace_A2B::Element>* elements, const uint8_t* src,
+                           size_t len) {
+    // Read the number of channels.  The four bytes (4-7) that we skipped are reserved and
     // must be zero.
-    uint8_t inputChannels = src[8];
-    uint8_t outputChannels = src[9];
+    const uint8_t inputChannels = src[8];
+    const uint8_t outputChannels = src[9];
     if (3 != inputChannels || SkColorLookUpTable::kOutputChannels != outputChannels) {
         // We only handle (supposedly) RGB inputs and RGB outputs.  The numbers of input
         // channels and output channels both must be 3.
@@ -836,52 +832,98 @@ static bool load_a2b0(sk_sp<SkColorLookUpTable>* colorLUT,
         SkColorSpacePrintf("Input and output channels must equal 3 in A to B tag.\n");
         return false;
     }
+    
+    // It is important that these are loaded in the order of application, as the
+    // order you construct an A2B color space's elements is the order it is applied
 
     // If the offset is non-zero it indicates that the element is present.
-    uint32_t offsetToACurves = read_big_endian_i32(src + 28);
+    const uint32_t offsetToACurves = read_big_endian_i32(src + 28);
     if (0 != offsetToACurves && offsetToACurves < len) {
         const size_t tagLen = len - offsetToACurves;
-        if (!parse_and_load_gamma(aCurveNamed, aCurve, src + offsetToACurves, tagLen)) {
+        SkGammaNamed gammaNamed;
+        sk_sp<SkGammas> gammas;
+        if (!parse_and_load_gamma(&gammaNamed, &gammas, src + offsetToACurves, tagLen)) {
             return false;
         }
+        if (gammas) {
+            elements->push_back(SkColorSpace_A2B::Element(std::move(gammas)));
+        } else {
+            elements->push_back(SkColorSpace_A2B::Element(gammaNamed));
+        }
     }
 
-    uint32_t offsetToColorLUT = read_big_endian_i32(src + 24);
+    const uint32_t offsetToColorLUT = read_big_endian_i32(src + 24);
     if (0 != offsetToColorLUT && offsetToColorLUT < len) {
-        if (!load_color_lut(colorLUT, inputChannels, src + offsetToColorLUT,
+        sk_sp<SkColorLookUpTable> colorLUT;
+        if (!load_color_lut(&colorLUT, inputChannels, src + offsetToColorLUT,
                             len - offsetToColorLUT)) {
             SkColorSpacePrintf("Failed to read color LUT from A to B tag.\n");
             return false;
         }
+        elements->push_back(SkColorSpace_A2B::Element(std::move(colorLUT)));
     }
 
-    uint32_t offsetToMCurves = read_big_endian_i32(src + 20);
+    const uint32_t offsetToMCurves = read_big_endian_i32(src + 20);
     if (0 != offsetToMCurves && offsetToMCurves < len) {
         const size_t tagLen = len - offsetToMCurves;
-        if (!parse_and_load_gamma(mCurveNamed, mCurve, src + offsetToMCurves, tagLen)) {
+        SkGammaNamed gammaNamed;
+        sk_sp<SkGammas> gammas;
+        if (!parse_and_load_gamma(&gammaNamed, &gammas, src + offsetToMCurves, tagLen)) {
             return false;
         }
+        if (gammas) {
+            elements->push_back(SkColorSpace_A2B::Element(std::move(gammas)));
+        } else {
+            elements->push_back(SkColorSpace_A2B::Element(gammaNamed));
+        }
     }
 
-    uint32_t offsetToMatrix = read_big_endian_i32(src + 16);
+    const uint32_t offsetToMatrix = read_big_endian_i32(src + 16);
     if (0 != offsetToMatrix && offsetToMatrix < len) {
-        if (!load_matrix(matrix, src + offsetToMatrix, len - offsetToMatrix)) {
+        SkMatrix44 matrix(SkMatrix44::kUninitialized_Constructor);
+        if (!load_matrix(&matrix, src + offsetToMatrix, len - offsetToMatrix)) {
             SkColorSpacePrintf("Failed to read matrix from A to B tag.\n");
-            matrix->setIdentity();
+        } else {
+            elements->push_back(SkColorSpace_A2B::Element(matrix));
         }
     }
     
-    uint32_t offsetToBCurves = read_big_endian_i32(src + 12);
+    const 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)) {
+        SkGammaNamed gammaNamed;
+        sk_sp<SkGammas> gammas;
+        if (!parse_and_load_gamma(&gammaNamed, &gammas, src + offsetToBCurves, tagLen)) {
             return false;
         }
+        if (gammas) {
+            elements->push_back(SkColorSpace_A2B::Element(std::move(gammas)));
+        } else {
+            elements->push_back(SkColorSpace_A2B::Element(gammaNamed));
+        }
     }
 
     return true;
 }
 
+static bool load_a2b0(std::vector<SkColorSpace_A2B::Element>* elements, const uint8_t* src,
+                      size_t len) {
+    const uint32_t type = read_big_endian_u32(src);
+    switch (type) {
+        case kTAG_AtoBType:
+            if (len < 32) {
+                SkColorSpacePrintf("A to B tag is too small (%d bytes).", len);
+                return false;
+            }
+            SkColorSpacePrintf("A2B0 tag is of type lutAtoBType\n");
+            return load_a2b0_a_to_b_type(elements, src, len);
+        default:
+            SkColorSpacePrintf("Unsupported A to B tag type: %c%c%c%c\n", (type>>24)&0xFF,
+                               (type>>16)&0xFF, (type>>8)&0xFF, type&0xFF);
+    }
+    return false;
+}
+
 static bool tag_equals(const ICCTag* a, const ICCTag* b, const uint8_t* base) {
     if (!a || !b) {
         return a == b;
@@ -952,12 +994,9 @@ sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) {
             const ICCTag* r = ICCTag::Find(tags.get(), tagCount, kTAG_rXYZ);
             const ICCTag* g = ICCTag::Find(tags.get(), tagCount, kTAG_gXYZ);
             const ICCTag* b = ICCTag::Find(tags.get(), tagCount, kTAG_bXYZ);
-            if (r && g && b) {
-                // Lab PCS means the profile is required to be an n-component LUT-based
-                // profile, so 3-component matrix-based profiles can only have an XYZ PCS
-                if (kXYZ_PCSSpace != header.fPCS) {
-                    return_null("Unsupported PCS space");
-                }
+            // Lab PCS means the profile is required to be an n-component LUT-based
+            // profile, so 3-component matrix-based profiles can only have an XYZ PCS
+            if (r && g && b && kXYZ_PCSSpace == header.fPCS) {
                 float toXYZ[9];
                 if (!load_xyz(&toXYZ[0], r->addr(base), r->fLength) ||
                     !load_xyz(&toXYZ[3], g->addr(base), g->fLength) ||
@@ -1089,32 +1128,15 @@ sk_sp<SkColorSpace> SkColorSpace::MakeICC(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) {
-                // default to Linear transforms for when the curves are not
-                // in the profile (which is legal behavior for a profile)
-                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 matrix(SkMatrix44::kUninitialized_Constructor);
-                if (!load_a2b0(&colorLUT, &aCurveNamed, &aCurve, &mCurveNamed, &mCurve,
-                               &bCurveNamed, &bCurve, &matrix, a2b0->addr(base), a2b0->fLength)) {
+                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)) {
                     return_null("Failed to parse A2B0 tag");
                 }
-
-                SkColorSpace_A2B::PCS pcs = SkColorSpace_A2B::PCS::kLAB;
-                if (header.fPCS == kXYZ_PCSSpace) {
-                    pcs = SkColorSpace_A2B::PCS::kXYZ;
-                }
-
-                return sk_sp<SkColorSpace>(new SkColorSpace_A2B(aCurveNamed, std::move(aCurve),
-                                                                std::move(colorLUT),
-                                                                mCurveNamed, std::move(mCurve),
-                                                                matrix,
-                                                                bCurveNamed, std::move(bCurve),
-                                                                pcs, std::move(data)));
+                return sk_sp<SkColorSpace>(new SkColorSpace_A2B(pcs, std::move(data),
+                                                                std::move(elements)));
             }
         }
         default: