Simplify SkColorSpace::MakeICC

Rather than restricting the supported ICC types in MakeICC, create any
ICC type that we support, and make the client reject them as necessary
by querying the SkColorSpace::Type.

Remove ICCTypeFlag and replace uses of it with SkColorSpace::Type.

This depends on a change in Chromium
(https://chromium-review.googlesource.com/c/chromium/src/+/741843).
Without that, this change will start allowing non-CMYK images to use
CMYK profiles.

Bug: 727128
Change-Id: I085b4665e49bc80083264496d864cc4cd62ae914
Reviewed-on: https://skia-review.googlesource.com/64841
Commit-Queue: Leon Scroggins <scroggo@google.com>
Reviewed-by: Mike Klein <mtklein@chromium.org>

11 files changed, 58 insertions, 72 deletions

diff --git a/src/codec/SkHeifCodec.cpp b/src/codec/SkHeifCodec.cpp
index 5fef5114a0..78f418f8c5 100644
--- a/src/codec/SkHeifCodec.cpp
+++ b/src/codec/SkHeifCodec.cpp
@@ -11,7 +11,6 @@
 #include "SkCodec.h"
 #include "SkCodecPriv.h"
 #include "SkColorData.h"
-#include "SkColorSpace_Base.h"
 #include "SkEndian.h"
 #include "SkStream.h"
 #include "SkHeifCodec.h"
@@ -140,11 +139,10 @@ std::unique_ptr<SkCodec> SkHeifCodec::MakeFromStream(
 
     sk_sp<SkColorSpace> colorSpace = nullptr;
     if ((frameInfo.mIccSize > 0) && (frameInfo.mIccData != nullptr)) {
-        SkColorSpace_Base::ICCTypeFlag iccType = SkColorSpace_Base::kRGB_ICCTypeFlag;
-        colorSpace = SkColorSpace_Base::MakeICC(
-                frameInfo.mIccData.get(), frameInfo.mIccSize, iccType);
+        colorSpace = SkColorSpace::MakeICC(frameInfo.mIccData.get(),
+                                           frameInfo.mIccSize);
     }
-    if (!colorSpace) {
+    if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
         colorSpace = SkColorSpace::MakeSRGB();
     }
 
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 062f05bd01..a6c2549141 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -122,7 +122,7 @@ static bool is_icc_marker(jpeg_marker_struct* marker) {
  *     (1) Discover all ICC profile markers and verify that they are numbered properly.
  *     (2) Copy the data from each marker into a contiguous ICC profile.
  */
-static sk_sp<SkData> get_icc_profile(jpeg_decompress_struct* dinfo) {
+static sk_sp<SkColorSpace> read_color_space(jpeg_decompress_struct* dinfo) {
     // Note that 256 will be enough storage space since each markerIndex is stored in 8-bits.
     jpeg_marker_struct* markerSequence[256];
     memset(markerSequence, 0, sizeof(markerSequence));
@@ -182,7 +182,7 @@ static sk_sp<SkData> get_icc_profile(jpeg_decompress_struct* dinfo) {
         dst = SkTAddOffset<void>(dst, bytes);
     }
 
-    return iccData;
+    return SkColorSpace::MakeICC(iccData->data(), iccData->size());
 }
 
 SkCodec::Result SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
@@ -228,23 +228,27 @@ SkCodec::Result SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
         SkEncodedInfo info = SkEncodedInfo::Make(color, SkEncodedInfo::kOpaque_Alpha, 8);
 
         SkEncodedOrigin orientation = get_exif_orientation(decoderMgr->dinfo());
-        sk_sp<SkData> iccData = get_icc_profile(decoderMgr->dinfo());
-        sk_sp<SkColorSpace> colorSpace = nullptr;
-        if (iccData) {
-            SkColorSpace_Base::ICCTypeFlag iccType = SkColorSpace_Base::kRGB_ICCTypeFlag;
+        sk_sp<SkColorSpace> colorSpace = read_color_space(decoderMgr->dinfo());
+        if (colorSpace) {
             switch (decoderMgr->dinfo()->jpeg_color_space) {
                 case JCS_CMYK:
                 case JCS_YCCK:
-                    iccType = SkColorSpace_Base::kCMYK_ICCTypeFlag;
+                    if (colorSpace->type() != SkColorSpace::kCMYK_Type) {
+                        colorSpace = nullptr;
+                    }
                     break;
                 case JCS_GRAYSCALE:
-                    // Note the "or equals".  We will accept gray or rgb profiles for gray images.
-                    iccType |= SkColorSpace_Base::kGray_ICCTypeFlag;
-                    break;
+                    if (colorSpace->type() != SkColorSpace::kGray_Type &&
+                        colorSpace->type() != SkColorSpace::kRGB_Type)
+                    {
+                        colorSpace = nullptr;
+                    }
                 default:
+                    if (colorSpace->type() != SkColorSpace::kRGB_Type) {
+                        colorSpace = nullptr;
+                    }
                     break;
             }
-            colorSpace = SkColorSpace_Base::MakeICC(iccData->data(), iccData->size(), iccType);
         }
         if (!colorSpace) {
             colorSpace = defaultColorSpace;
diff --git a/src/codec/SkPngCodec.cpp b/src/codec/SkPngCodec.cpp
index 1c88b4780f..e9fff97add 100644
--- a/src/codec/SkPngCodec.cpp
+++ b/src/codec/SkPngCodec.cpp
@@ -343,8 +343,7 @@ static float png_inverted_fixed_point_to_float(png_fixed_point x) {
 // Returns a colorSpace object that represents any color space information in
 // the encoded data.  If the encoded data contains an invalid/unsupported color space,
 // this will return NULL. If there is no color space information, it will guess sRGB
-sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr,
-                                     SkColorSpace_Base::ICCTypeFlag iccType) {
+sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr) {
 
 #if (PNG_LIBPNG_VER_MAJOR > 1) || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 6)
 
@@ -361,7 +360,7 @@ sk_sp<SkColorSpace> read_color_space(png_structp png_ptr, png_infop info_ptr,
     int compression;
     if (PNG_INFO_iCCP == png_get_iCCP(png_ptr, info_ptr, &name, &compression, &profile,
             &length)) {
-        return SkColorSpace_Base::MakeICC(profile, length, iccType);
+        return SkColorSpace::MakeICC(profile, length);
     }
 
     // Second, check for sRGB.
@@ -911,11 +910,23 @@ void AutoCleanPng::infoCallback(size_t idatLength) {
 
     if (fOutCodec) {
         SkASSERT(nullptr == *fOutCodec);
-        SkColorSpace_Base::ICCTypeFlag iccType = SkColorSpace_Base::kRGB_ICCTypeFlag;
-        if (SkEncodedInfo::kGray_Color == color || SkEncodedInfo::kGrayAlpha_Color == color) {
-            iccType |= SkColorSpace_Base::kGray_ICCTypeFlag;
+        sk_sp<SkColorSpace> colorSpace = read_color_space(fPng_ptr, fInfo_ptr);
+        if (colorSpace) {
+            switch (colorSpace->type()) {
+                case SkColorSpace::kCMYK_Type:
+                    colorSpace = nullptr;
+                    break;
+                case SkColorSpace::kGray_Type:
+                    if (SkEncodedInfo::kGray_Color != color &&
+                        SkEncodedInfo::kGrayAlpha_Color != color)
+                    {
+                        colorSpace = nullptr;
+                    }
+                    break;
+                case SkColorSpace::kRGB_Type:
+                    break;
+            }
         }
-        sk_sp<SkColorSpace> colorSpace = read_color_space(fPng_ptr, fInfo_ptr, iccType);
         if (!colorSpace) {
             // Treat unsupported/invalid color spaces as sRGB.
             colorSpace = SkColorSpace::MakeSRGB();
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index baf3468e30..6287617373 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -97,7 +97,7 @@ std::unique_ptr<SkCodec> SkWebpCodec::MakeFromStream(std::unique_ptr<SkStream> s
     if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
         colorSpace = SkColorSpace::MakeICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
     }
-    if (!colorSpace) {
+    if (!colorSpace || colorSpace->type() != SkColorSpace::kRGB_Type) {
         colorSpace = SkColorSpace::MakeSRGB();
     }
 
diff --git a/src/core/SkColorSpaceXform_A2B.cpp b/src/core/SkColorSpaceXform_A2B.cpp
index cc80145af1..3543ba40a3 100644
--- a/src/core/SkColorSpaceXform_A2B.cpp
+++ b/src/core/SkColorSpaceXform_A2B.cpp
@@ -112,10 +112,10 @@ SkColorSpaceXform_A2B::SkColorSpaceXform_A2B(SkColorSpace_A2B* srcSpace,
 #endif
     int currentChannels;
     switch (srcSpace->iccType()) {
-        case SkColorSpace_Base::kRGB_ICCTypeFlag:
+        case SkColorSpace::kRGB_Type:
             currentChannels = 3;
             break;
-        case SkColorSpace_Base::kCMYK_ICCTypeFlag: {
+        case SkColorSpace::kCMYK_Type: {
             currentChannels = 4;
             // CMYK images from JPEGs (the only format that supports it) are actually
             // inverted CMYK, so we need to invert every channel.
diff --git a/src/core/SkColorSpace_A2B.cpp b/src/core/SkColorSpace_A2B.cpp
index 7e097ffa80..b5d8d37747 100644
--- a/src/core/SkColorSpace_A2B.cpp
+++ b/src/core/SkColorSpace_A2B.cpp
@@ -7,12 +7,12 @@
 
 #include "SkColorSpace_A2B.h"
 
-SkColorSpace_A2B::SkColorSpace_A2B(ICCTypeFlag iccType, std::vector<Element> elements,
+SkColorSpace_A2B::SkColorSpace_A2B(SkColorSpace::Type iccType, std::vector<Element> elements,
                                    PCS pcs, sk_sp<SkData> profileData)
     : INHERITED(std::move(profileData))
     , fICCType(iccType)
     , fElements(std::move(elements))
     , fPCS(pcs)
 {
-    SkASSERT(kRGB_ICCTypeFlag == iccType || kCMYK_ICCTypeFlag == iccType);
+    SkASSERT(SkColorSpace::kRGB_Type == iccType || SkColorSpace::kCMYK_Type == iccType);
 }
diff --git a/src/core/SkColorSpace_A2B.h b/src/core/SkColorSpace_A2B.h
index 5f2800796a..08784c23bb 100644
--- a/src/core/SkColorSpace_A2B.h
+++ b/src/core/SkColorSpace_A2B.h
@@ -52,7 +52,7 @@ public:
     bool onGammaIsLinear() const override { return false; }
     bool onIsNumericalTransferFn(SkColorSpaceTransferFn* coeffs) const override { return false; }
 
-    bool onIsCMYK() const override { return kCMYK_ICCTypeFlag == fICCType; }
+    bool onIsCMYK() const override { return SkColorSpace::kCMYK_Type == fICCType; }
 
     sk_sp<SkColorSpace> makeLinearGamma() const override {
         // TODO: Analyze the extrema of our projection into XYZ and use suitable primaries?
@@ -161,13 +161,13 @@ public:
 
     PCS pcs() const { return fPCS; }
 
-    ICCTypeFlag iccType() const { return fICCType; }
+    SkColorSpace::Type iccType() const { return fICCType; }
 
-    SkColorSpace_A2B(ICCTypeFlag iccType, std::vector<Element> elements, PCS pcs,
+    SkColorSpace_A2B(SkColorSpace::Type iccType, std::vector<Element> elements, PCS pcs,
                      sk_sp<SkData> profileData);
 
 private:
-    ICCTypeFlag          fICCType;
+    SkColorSpace::Type   fICCType;
     std::vector<Element> fElements;
     PCS                  fPCS;
 
diff --git a/src/core/SkColorSpace_Base.h b/src/core/SkColorSpace_Base.h
index 015e95f8a1..32e12b4717 100644
--- a/src/core/SkColorSpace_Base.h
+++ b/src/core/SkColorSpace_Base.h
@@ -187,13 +187,6 @@ public:
 
     virtual Type type() const = 0;
 
-    typedef uint8_t ICCTypeFlag;
-    static constexpr ICCTypeFlag kRGB_ICCTypeFlag  = 1 << 0;
-    static constexpr ICCTypeFlag kCMYK_ICCTypeFlag = 1 << 1;
-    static constexpr ICCTypeFlag kGray_ICCTypeFlag = 1 << 2;
-
-    static sk_sp<SkColorSpace> MakeICC(const void* input, size_t len, ICCTypeFlag type);
-
     static sk_sp<SkColorSpace> MakeRGB(SkGammaNamed gammaNamed, const SkMatrix44& toXYZD50);
 
     enum Named : uint8_t {
diff --git a/src/core/SkColorSpace_ICC.cpp b/src/core/SkColorSpace_ICC.cpp
index 72592d6444..78318f6a56 100644
--- a/src/core/SkColorSpace_ICC.cpp
+++ b/src/core/SkColorSpace_ICC.cpp
@@ -1180,11 +1180,11 @@ bool load_a2b0_lutn_type(std::vector<SkColorSpace_A2B::Element>* elements, const
     return true;
 }
 
-static inline int icf_channels(SkColorSpace_Base::ICCTypeFlag iccType) {
+static inline int icf_channels(SkColorSpace::Type iccType) {
     switch (iccType) {
-        case SkColorSpace_Base::kRGB_ICCTypeFlag:
+        case SkColorSpace::kRGB_Type:
             return 3;
-        case SkColorSpace_Base::kCMYK_ICCTypeFlag:
+        case SkColorSpace::kCMYK_Type:
             return 4;
         default:
             SkASSERT(false);
@@ -1194,7 +1194,7 @@ static inline int icf_channels(SkColorSpace_Base::ICCTypeFlag iccType) {
 
 static bool load_a2b0(std::vector<SkColorSpace_A2B::Element>* elements, const uint8_t* src,
                       size_t len, SkColorSpace_A2B::PCS pcs,
-                      SkColorSpace_Base::ICCTypeFlag iccType) {
+                      SkColorSpace::Type iccType) {
     if (len < 4) {
         return false;
     }
@@ -1472,7 +1472,7 @@ static sk_sp<SkColorSpace> make_gray(const ICCProfileHeader& header, ICCTag* tag
                                                     toXYZD50, std::move(profileData)));
 }
 
-static sk_sp<SkColorSpace> make_a2b(SkColorSpace_Base::ICCTypeFlag iccType,
+static sk_sp<SkColorSpace> make_a2b(SkColorSpace::Type iccType,
                                     const ICCProfileHeader& header, ICCTag* tags, int tagCount,
                                     const uint8_t* base, sk_sp<SkData> profileData) {
     const ICCTag* a2b0 = ICCTag::Find(tags, tagCount, kTAG_A2B0);
@@ -1491,11 +1491,6 @@ static sk_sp<SkColorSpace> make_a2b(SkColorSpace_Base::ICCTypeFlag iccType,
 }
 
 sk_sp<SkColorSpace> SkColorSpace::MakeICC(const void* input, size_t len) {
-    return SkColorSpace_Base::MakeICC(input, len, SkColorSpace_Base::kRGB_ICCTypeFlag);
-}
-
-sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
-                                               ICCTypeFlag desiredType) {
     if (!input || len < kICCHeaderSize) {
         return_null("Data is null or not large enough to contain an ICC profile");
     }
@@ -1543,38 +1538,25 @@ sk_sp<SkColorSpace> SkColorSpace_Base::MakeICC(const void* input, size_t len,
         }
     }
 
+    Type a2b_type = kRGB_Type;
     switch (header.fInputColorSpace) {
         case kRGB_ColorSpace: {
-            if (!(kRGB_ICCTypeFlag & desiredType)) {
-                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;
             }
-
-            desiredType = kRGB_ICCTypeFlag;
             break;
         }
         case kGray_ColorSpace: {
-            if (!(kGray_ICCTypeFlag & desiredType)) {
-                return_null("Provided input color format (Gray) does not match profile.");
-            }
-
             return make_gray(header, tags.get(), tagCount, base, profileData);
         }
         case kCMYK_ColorSpace:
-            if (!(kCMYK_ICCTypeFlag & desiredType)) {
-                return_null("Provided input color format (CMYK) does not match profile.");
-            }
-
-            desiredType = kCMYK_ICCTypeFlag;
+            a2b_type = kCMYK_Type;
             break;
         default:
             return_null("ICC profile contains unsupported colorspace");
     }
 
-    return make_a2b(desiredType, header, tags.get(), tagCount, base, profileData);
+    return make_a2b(a2b_type, header, tags.get(), tagCount, base, profileData);
 }
diff --git a/tests/ColorSpaceXformTest.cpp b/tests/ColorSpaceXformTest.cpp
index 83317d95e4..041796c5c4 100644
--- a/tests/ColorSpaceXformTest.cpp
+++ b/tests/ColorSpaceXformTest.cpp
@@ -50,7 +50,6 @@ public:
         srcElements.push_back(SkColorSpace_A2B::Element(arbitraryMatrix));
         auto srcSpace =
                 ColorSpaceXformTest::CreateA2BSpace(SkColorSpace_A2B::PCS::kXYZ,
-                                                    SkColorSpace_Base::kRGB_ICCTypeFlag,
                                                     std::move(srcElements));
         sk_sp<SkColorSpace> dstSpace(new SkColorSpace_XYZ(gammaNamed, gammas, arbitraryMatrix,
                                                           nullptr));
@@ -60,9 +59,9 @@ public:
     }
 
     static sk_sp<SkColorSpace> CreateA2BSpace(SkColorSpace_A2B::PCS pcs,
-                                              SkColorSpace_Base::ICCTypeFlag iccType,
                                               std::vector<SkColorSpace_A2B::Element> elements) {
-        return sk_sp<SkColorSpace>(new SkColorSpace_A2B(iccType, std::move(elements),
+        return sk_sp<SkColorSpace>(new SkColorSpace_A2B(SkColorSpace::kRGB_Type,
+                                                        std::move(elements),
                                                         pcs, nullptr));
     }
 };
@@ -309,7 +308,6 @@ DEF_TEST(ColorSpaceXform_A2BCLUT, r) {
     std::vector<SkColorSpace_A2B::Element> srcElements;
     srcElements.push_back(SkColorSpace_A2B::Element(std::move(colorLUT)));
     auto srcSpace = ColorSpaceXformTest::CreateA2BSpace(SkColorSpace_A2B::PCS::kXYZ,
-                                                        SkColorSpace_Base::kRGB_ICCTypeFlag,
                                                         std::move(srcElements));
     // dst space is entirely identity
     auto dstSpace = SkColorSpace::MakeRGB(SkColorSpace::kLinear_RenderTargetGamma, SkMatrix44::I());
diff --git a/tools/colorspaceinfo.cpp b/tools/colorspaceinfo.cpp
index 3e09a9aeeb..0002e961cf 100644
--- a/tools/colorspaceinfo.cpp
+++ b/tools/colorspaceinfo.cpp
@@ -565,13 +565,13 @@ int main(int argc, char** argv) {
         SkColorSpace_A2B* a2b = static_cast<SkColorSpace_A2B*>(colorSpace.get());
         SkDebugf("Conversion type: ");
         switch (a2b->iccType()) {
-            case SkColorSpace_Base::kRGB_ICCTypeFlag:
+            case SkColorSpace::kRGB_Type:
                 SkDebugf("RGB");
                 break;
-            case SkColorSpace_Base::kCMYK_ICCTypeFlag:
+            case SkColorSpace::kCMYK_Type:
                 SkDebugf("CMYK");
                 break;
-            case SkColorSpace_Base::kGray_ICCTypeFlag:
+            case SkColorSpace::kGray_Type:
                 SkDebugf("Gray");
                 break;
             default: