Respect colorSpace in DecodingImageGenerator::onGetPixels()

third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp

Index: third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
diff --git a/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
index 440e16a3e3af68ffdd1031bc7fd9eb7803dc3732..bba70c81d3e61039d8dd4ea4ccc2494264cb5ffd 100644
--- a/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
@@ -522,19 +522,30 @@ SkColorSpaceXform* ImageDecoder::colorTransform() {
   m_sourceToTargetColorTransformNeedsUpdate = false;
   m_sourceToTargetColorTransform = nullptr;
 
-  if (!m_colorBehavior.isTransformToTargetColorSpace())
+  if (m_colorBehavior.isIgnore()) {
     return nullptr;
+  }
 
-  sk_sp<SkColorSpace> srcColorSpace = m_embeddedColorSpace;
-  if (!srcColorSpace) {
-    if (RuntimeEnabledFeatures::colorCorrectRenderingEnabled())
-      srcColorSpace = SkColorSpace::MakeSRGB();
-    else
+  sk_sp<SkColorSpace> srcColorSpace = nullptr;
+  sk_sp<SkColorSpace> dstColorSpace = nullptr;
+  if (m_colorBehavior.isTransformToTargetColorSpace()) {
+    if (!m_embeddedColorSpace) {
       return nullptr;
-  }
+    }
 
-  sk_sp<SkColorSpace> dstColorSpace =
-      m_colorBehavior.targetColorSpace().ToSkColorSpace();
+    srcColorSpace = m_embeddedColorSpace;
+    dstColorSpace = m_colorBehavior.targetColorSpace().ToSkColorSpace();
+  } else {
+    DCHECK(m_colorBehavior.isTag());

[scroggo_chromium, 2017/04/07 18:06:06]

Alternatively, you could make this a switch statement, and then no need for the
DCHECK.

D'oh! Type is not public, so you cannot do that. NVM.

[msarett1, 2017/04/10 14:42:46]

Acknowledged.

+    srcColorSpace = m_embeddedColorSpace;
+    if (!srcColorSpace) {
+      srcColorSpace = SkColorSpace::MakeSRGB();
+    }
+
+    // This will most likely be equal to the |srcColorSpace|.
+    // In that case, we skip the xform when we check for equality below.
+    dstColorSpace = colorSpaceForSkImages();
+  }
 
   if (SkColorSpace::Equals(srcColorSpace.get(), dstColorSpace.get())) {
     return nullptr;
@@ -549,8 +560,26 @@ sk_sp<SkColorSpace> ImageDecoder::colorSpaceForSkImages() const {
   if (!m_colorBehavior.isTag())
     return nullptr;
 
-  if (m_embeddedColorSpace)
-    return m_embeddedColorSpace;
+  if (m_embeddedColorSpace) {
+    SkColorSpaceTransferFn fn;
+    if (m_embeddedColorSpace->isNumericalTransferFn(&fn)) {
+      // The embedded color space is supported by Skia.
+      return m_embeddedColorSpace;
+    }
+
+    // In the rare case that the embedded color space is unsupported, xform at
+    // decode time.
+    SkMatrix44 toXYZD50(SkMatrix44::kUninitialized_Constructor);
+    if (m_embeddedColorSpace->toXYZD50(&toXYZD50)) {
+      // Preserve the gamut, but convert to a standard transfer function.
+      return SkColorSpace::MakeRGB(SkColorSpace::kSRGB_RenderTargetGamma,
+                                   toXYZD50);
+    }
+
+    // For color spaces without an identifiable gamut, just fall through to
+    // sRGB.
+  }
+
   return SkColorSpace::MakeSRGB();
 }