SkColorSpace::NewICC() warn instead of fail on bad input

src/core/SkColorSpace.cpp

Index: src/core/SkColorSpace.cpp
diff --git a/src/core/SkColorSpace.cpp b/src/core/SkColorSpace.cpp
index 3f93f162331f188175b51e84dc529efb215e535b..3f4c44d902bcae9c6d54838d803e921ecd44c59e 100644
--- a/src/core/SkColorSpace.cpp
+++ b/src/core/SkColorSpace.cpp
@@ -201,12 +201,13 @@ static constexpr size_t kICCHeaderSize = 132;
 // Contains a signature (4), offset (4), and size (4).
 static constexpr size_t kICCTagTableEntrySize = 12;
 
-static constexpr uint32_t kRGB_ColorSpace  = SkSetFourByteTag('R', 'G', 'B', ' ');
-static constexpr uint32_t kDisplay_Profile = SkSetFourByteTag('m', 'n', 't', 'r');
-static constexpr uint32_t kInput_Profile   = SkSetFourByteTag('s', 'c', 'n', 'r');
-static constexpr uint32_t kOutput_Profile  = SkSetFourByteTag('p', 'r', 't', 'r');
-static constexpr uint32_t kXYZ_PCSSpace    = SkSetFourByteTag('X', 'Y', 'Z', ' ');
-static constexpr uint32_t kACSP_Signature  = SkSetFourByteTag('a', 'c', 's', 'p');
+static constexpr uint32_t kRGB_ColorSpace     = SkSetFourByteTag('R', 'G', 'B', ' ');
+static constexpr uint32_t kDisplay_Profile    = SkSetFourByteTag('m', 'n', 't', 'r');
+static constexpr uint32_t kInput_Profile      = SkSetFourByteTag('s', 'c', 'n', 'r');
+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 kACSP_Signature     = SkSetFourByteTag('a', 'c', 's', 'p');
 
 struct ICCProfileHeader {
     uint32_t fSize;
@@ -269,12 +270,13 @@ struct ICCProfileHeader {
         uint8_t majorVersion = fVersion >> 24;
         return_if_false(majorVersion <= 4, "Unsupported version");
 
-        // These are the three basic classes of profiles that we might expect to see embedded
-        // in images.  Four additional classes exist, but they generally are used as a convenient
+        // These are the four basic classes of profiles that we might expect to see embedded
+        // in images.  Additional classes exist, but they generally are used as a convenient
         // way for CMMs to store calculated transforms.
         return_if_false(fProfileClass == kDisplay_Profile ||
                         fProfileClass == kInput_Profile ||
-                        fProfileClass == kOutput_Profile,
+                        fProfileClass == kOutput_Profile ||
+                        fProfileClass == kColorSpace_Profile,
                         "Unsupported profile");
 
         // TODO (msarett):
@@ -291,7 +293,12 @@ struct ICCProfileHeader {
         // Should we treat different rendering intents differently?
         // Valid rendering intents include kPerceptual (0), kRelative (1),
         // kSaturation (2), and kAbsolute (3).
-        return_if_false(fRenderingIntent <= 3, "Bad rendering intent");
+        if (fRenderingIntent <= 3) {
+            // I think it makes to warn rather than fail here.  I've seen a
+            // few profiles that specify 0x01000000 as the rendering intent.
+            // It's likely that they're just confused about endianness.

[reed1, 2016/06/25 01:16:37]

What do other parsers do on these? Can you tell what Apple or Adobe do?

[msarett, 2016/07/14 13:45:04]

I'm fairly confident that this is the right decision.  My test images have
perfectly normal sRGB profiles (minus the weird rendering intent).

Safari treats these images as sRGB (but that's not too informative because
Safari also treats unmarked images as sRGB).

Linux command line tool (exiftool) parses the profile just fine.

I'm working on getting Adobe Photoshop (finally) but I don't think it'll change
my opinion.

+            SkColorSpacePrintf("Warning, bad rendering intent.\n");
+        }
 
         return_if_false(color_space_almost_equal(SkFixedToFloat(fIlluminantXYZ[0]), 0.96420f) &&
                         color_space_almost_equal(SkFixedToFloat(fIlluminantXYZ[1]), 1.00000f) &&