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

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());
+            for (size_t e = 0; e < cs.count(); ++e) {
-            SkASSERT(kLinear_SkGammaNamed == cs.mCurveNamed());
+                switch (cs.element(e).type()) {
-            SkASSERT(kLinear_SkGammaNamed == cs.bCurveNamed());
+                    case SkColorSpace_A2B::Element::Type::kGammaNamed:
-            SkASSERT(cs.matrix().isIdentity());
+                        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

src/core/SkColorSpace_A2B.cpp

@@ -7,20 +7,9 @@
 
 #include "SkColorSpace_A2B.h"
 
-SkColorSpace_A2B::SkColorSpace_A2B(SkGammaNamed aCurveNamed, sk_sp<SkGammas> aCurve,
+SkColorSpace_A2B::SkColorSpace_A2B(PCS pcs, sk_sp<SkData> profileData,
-                                   sk_sp<SkColorLookUpTable> colorLUT,
+                                   std::vector<Element> elements)
-                                   SkGammaNamed mCurveNamed, sk_sp<SkGammas> mCurve,
-                                   const SkMatrix44& matrix,
-                                   SkGammaNamed bCurveNamed, sk_sp<SkGammas> bCurve,
-                                   PCS pcs, sk_sp<SkData> profileData)
     : 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))
 {}

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
+// Currently AtoBType A2B0 tag types are supported. There are also lut8Type,
-//
+// lut16Type and MPET (multi-processing-elements) A2B0 tags which allow you to
-// There are also multi-processing-elements in the A2B0 tag 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; }
+    Type type() const override { return Type::kA2B; }
 
-    const SkGammas* aCurve() const { return fACurve.get(); }
+    class Element {
+    public:
+        explicit Element(SkGammaNamed gammaNamed)
+            : fType(Type::kGammaNamed)
+            , fGammaNamed(gammaNamed)
+            , fMatrix(SkMatrix44::kUninitialized_Constructor)
+        {}
 
-    const SkColorLookUpTable* colorLUT() const { return fColorLUT.get(); }
+        explicit Element(sk_sp<SkGammas> gammas)
+            : fType(Type::kGammas)
+            , fGammas(std::move(gammas))
+            , fMatrix(SkMatrix44::kUninitialized_Constructor)  
+        {}
 
-    SkGammaNamed mCurveNamed() const { return fMCurveNamed; }
+        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(); }
+        enum class Type {
+            kGammaNamed,
+            kGammas,
+            kCLUT,
+            kMatrix
+        };
 
-    const SkMatrix44& matrix() const { return fMatrix; }
+        Type type() const { return fType; }
 
-    SkGammaNamed bCurveNamed() const { return fBCurveNamed; }
+        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,
+    SkColorSpace_A2B(PCS pcs, sk_sp<SkData> profileData, std::vector<Element> elements);
-                     sk_sp<SkColorLookUpTable> colorLUT,
+
-                     SkGammaNamed mCurveNamed, sk_sp<SkGammas> mCurve,
+    PCS                  fPCS;
-                     const SkMatrix44& matrix,
+    std::vector<Element> fElements;
-                     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;
-    
     friend class SkColorSpace;
     typedef SkColorSpace_Base INHERITED;
 };

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):
+        switch (fPCS) {
-        // All the profiles we've tested so far use XYZ as the profile connection space.
+            case kXYZ_PCSSpace:
-        return_if_false(fPCS == kXYZ_PCSSpace || fPCS == kLAB_PCSSpace, "Unsupported PCS space");
+                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,
+bool load_a2b0_a_to_b_type(std::vector<SkColorSpace_A2B::Element>* elements, const uint8_t* src,
-                      SkGammaNamed* aCurveNamed, sk_sp<SkGammas>* aCurve,
+                           size_t len) {
-                      SkGammaNamed* mCurveNamed, sk_sp<SkGammas>* mCurve,
+    // Read the number of channels.  The four bytes (4-7) that we skipped are reserved and
-                      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
     // must be zero.
-    uint8_t inputChannels = src[8];
+    const uint8_t inputChannels = src[8];
-    uint8_t outputChannels = src[9];
+    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
-                // 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
-                // profile, so 3-component matrix-based profiles can only have an XYZ PCS
+            if (r && g && b && kXYZ_PCSSpace == header.fPCS) {
-                if (kXYZ_PCSSpace != header.fPCS) {
-                    return_null("Unsupported PCS space");
-                }
                 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
+                const SkColorSpace_A2B::PCS pcs = kXYZ_PCSSpace == header.fPCS
-                // in the profile (which is legal behavior for a profile)
+                                                ? SkColorSpace_A2B::PCS::kXYZ
-                SkGammaNamed aCurveNamed = kLinear_SkGammaNamed;
+                                                : SkColorSpace_A2B::PCS::kLAB;
-                SkGammaNamed mCurveNamed = kLinear_SkGammaNamed;
+                std::vector<SkColorSpace_A2B::Element> elements;
-                SkGammaNamed bCurveNamed = kLinear_SkGammaNamed;
+                if (!load_a2b0(&elements, a2b0->addr(base), a2b0->fLength)) {
-                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)) {
                     return_null("Failed to parse A2B0 tag");
                 }
-
+                return sk_sp<SkColorSpace>(new SkColorSpace_A2B(pcs, std::move(data),
-                SkColorSpace_A2B::PCS pcs = SkColorSpace_A2B::PCS::kLAB;
+                                                                std::move(elements)));
-                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)));
             }
         }
         default: