Use SkColorSpaceXform to handle color conversions in decoders

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 9f3aeab110589bd7936797be2924085778974665..d56d9a597917cc14bfc752ba1ca7e2589b9d83a3 100644
--- a/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
+++ b/third_party/WebKit/Source/platform/image-decoders/ImageDecoder.cpp
@@ -35,17 +35,6 @@
 
 namespace blink {
 
-#if USE(QCMSLIB)
-struct QCMSProfileDeleter {
-  void operator()(qcms_profile* profile) {
-    if (profile)
-      qcms_profile_release(profile);
-  }
-};
-
-using QCMSProfileUniquePtr = std::unique_ptr<qcms_profile, QCMSProfileDeleter>;
-#endif  // USE(QCMSLIB)
-
 inline bool matchesJPEGSignature(const char* contents) {
   return !memcmp(contents, "\xFF\xD8\xFF", 3);
 }
@@ -341,73 +330,33 @@ size_t ImagePlanes::rowBytes(int i) const {
   return m_rowBytes[i];
 }
 
-namespace {
-
-#if USE(QCMSLIB)
-
-const unsigned kIccColorProfileHeaderLength = 128;
-
-bool rgbColorProfile(const char* profileData, unsigned profileLength) {
-  DCHECK_GE(profileLength, kIccColorProfileHeaderLength);
-
-  return !memcmp(&profileData[16], "RGB ", 4);
-}
-
-bool inputDeviceColorProfile(const char* profileData, unsigned profileLength) {
-  DCHECK_GE(profileLength, kIccColorProfileHeaderLength);
-
-  return !memcmp(&profileData[12], "mntr", 4) ||
-         !memcmp(&profileData[12], "scnr", 4);
-}
-
-// The output device color profile is global and shared across multiple threads.
-SpinLock gTargetColorProfileLock;
-qcms_profile* gTargetColorProfile = nullptr;
-
-#endif  // USE(QCMSLIB)
-
-}  // namespace
+// The output device color space is global and shared across multiple threads.
+SpinLock gTargetColorSpaceLock;

[scroggo_chromium, 2016/10/19 18:26:25]

Why not leave these in an anonymous namespace?

[msarett, 2016/10/19 19:55:43]

Didn't mean to remove this, adding it back.

+SkColorSpace* gTargetColorSpace = nullptr;
 
 // static
 void ImageDecoder::setTargetColorProfile(const WebVector<char>& profile) {
-#if USE(QCMSLIB)
   if (profile.isEmpty())
     return;
 
   // Take a lock around initializing and accessing the global device color
   // profile.
-  SpinLock::Guard guard(gTargetColorProfileLock);
+  SpinLock::Guard guard(gTargetColorSpaceLock);
 
   // Layout tests expect that only the first call will take effect.
-  if (gTargetColorProfile)
-    return;
-
-  {
-    sk_sp<SkColorSpace> colorSpace =
-        SkColorSpace::NewICC(profile.data(), profile.size());
-    BitmapImageMetrics::countGamma(colorSpace.get());
-  }
-
-  // FIXME: Add optional ICCv4 support and support for multiple monitors.
-  gTargetColorProfile =
-      qcms_profile_from_memory(profile.data(), profile.size());
-  if (!gTargetColorProfile)
+  if (gTargetColorSpace)
     return;
 
-  if (qcms_profile_is_bogus(gTargetColorProfile)) {
-    qcms_profile_release(gTargetColorProfile);
-    gTargetColorProfile = nullptr;
-    return;
-  }
+  gTargetColorSpace =
+      SkColorSpace::NewICC(profile.data(), profile.size()).release();

[scroggo_chromium, 2016/10/19 18:26:25]

nit: Can this fit on one line?

[msarett, 2016/10/19 19:55:43]

Autoformatter actually keeps forcing me to less than 80 chars.

 
-  qcms_profile_precache_output_transform(gTargetColorProfile);
-#endif  // USE(QCMSLIB)
+  // UMA statistics.
+  BitmapImageMetrics::countGamma(gTargetColorSpace);
 }
 
-void ImageDecoder::setColorProfileAndComputeTransform(const char* iccData,
-                                                      unsigned iccLength,
-                                                      bool hasAlpha,
-                                                      bool useSRGB) {
+void ImageDecoder::setColorSpaceAndComputeTransform(const char* iccData,
+                                                    unsigned iccLength,
+                                                    bool useSRGB) {
   // Sub-classes should not call this if they were instructed to ignore embedded
   // color profiles.
   DCHECK(!m_ignoreGammaAndColorProfile);
@@ -415,56 +364,42 @@ void ImageDecoder::setColorProfileAndComputeTransform(const char* iccData,
   m_colorProfile.assign(iccData, iccLength);
   m_hasColorProfile = true;
 
-  // With color correct rendering, we use Skia instead of QCMS to color correct
-  // images.
+  // With color correct rendering, we do not transform to the output color space
+  // at decode time.  Instead, we tag the raw image pixels and pass the tagged
+  // SkImage to Skia.
   if (RuntimeEnabledFeatures::colorCorrectRenderingEnabled())
     return;
 
-#if USE(QCMSLIB)
-  m_sourceToOutputDeviceColorTransform.reset();
+  m_sourceToOutputDeviceColorTransform = nullptr;
 
-  // Create the input profile
-  QCMSProfileUniquePtr inputProfile;
+  // Create the input profile.
+  sk_sp<SkColorSpace> srcSpace = nullptr;
   if (useSRGB) {
-    inputProfile.reset(qcms_profile_sRGB());
+    srcSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
   } else {
-    // Only accept RGB color profiles from input class devices.
-    if (iccLength < kIccColorProfileHeaderLength)
-      return;
-    if (!rgbColorProfile(iccData, iccLength))
-      return;
-    if (!inputDeviceColorProfile(iccData, iccLength))
-      return;
-    inputProfile.reset(qcms_profile_from_memory(iccData, iccLength));
+    srcSpace = SkColorSpace::NewICC(iccData, iccLength);
   }
-  if (!inputProfile)
-    return;
 
-  // We currently only support color profiles for RGB profiled images.
-  ASSERT(rgbData == qcms_profile_get_color_space(inputProfile.get()));
+  if (!srcSpace)
+    return;
 
   // Take a lock around initializing and accessing the global device color
   // profile.
-  SpinLock::Guard guard(gTargetColorProfileLock);
+  SpinLock::Guard guard(gTargetColorSpaceLock);
 
   // Initialize the output device profile to sRGB if it has not yet been
   // initialized.
-  if (!gTargetColorProfile) {
-    gTargetColorProfile = qcms_profile_sRGB();
-    qcms_profile_precache_output_transform(gTargetColorProfile);
+  if (!gTargetColorSpace) {
+    gTargetColorSpace =

[scroggo_chromium, 2016/10/19 18:26:25]

nit: Can this fit on one line?

[msarett, 2016/10/19 19:55:43]

Autoformatter puts it on two lines.

+        SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named).release();
   }
 
-  if (qcms_profile_match(inputProfile.get(), gTargetColorProfile))
+  if (SkColorSpace::Equals(srcSpace.get(), gTargetColorSpace)) {
     return;
+  }
 
-  qcms_data_type dataFormat = hasAlpha ? QCMS_DATA_RGBA_8 : QCMS_DATA_RGB_8;
-
-  // FIXME: Don't force perceptual intent if the image profile contains an
-  // intent.
-  m_sourceToOutputDeviceColorTransform.reset(
-      qcms_transform_create(inputProfile.get(), dataFormat, gTargetColorProfile,
-                            QCMS_DATA_RGBA_8, QCMS_INTENT_PERCEPTUAL));
-#endif  // USE(QCMSLIB)
+  m_sourceToOutputDeviceColorTransform =
+      SkColorSpaceXform::New(srcSpace.get(), gTargetColorSpace);
 }
 
 }  // namespace blink