Color correct ImageBitmap(HTMLImageElement*) constructor

third_party/WebKit/Source/core/frame/ImageBitmap.cpp

Index: third_party/WebKit/Source/core/frame/ImageBitmap.cpp
diff --git a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
index c1cb64a471998ea3f1508f3306a180c6237de77a..6529daa3e5d69cde7e7c2f3be40fa3c0921f437f 100644
--- a/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
+++ b/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
@@ -31,10 +31,9 @@ struct ParsedOptions {
   unsigned resizeHeight = 0;
   IntRect cropRect;
   SkFilterQuality resizeQuality = kLow_SkFilterQuality;
-  // This value should be changed in the future when we support
-  // createImageBitmap with higher bit depth, in the parseOptions() function.
-  // For now, it is always 4.
   int bytesPerPixel = 4;
+  sk_sp<SkColorSpace> dstColorSpace = nullptr;
+  SkColorType dstColorType = SkColorType::kUnknown_SkColorType;
 };
 
 // The following two functions are helpers used in cropImage
@@ -63,6 +62,35 @@ ParsedOptions parseOptions(const ImageBitmapOptions& options,
            options.premultiplyAlpha() == "premultiply");
   }
 
+  if (options.colorSpaceConversion() != "none") {
+    if (!RuntimeEnabledFeatures::experimentalCanvasFeaturesEnabled() ||
+        !RuntimeEnabledFeatures::colorCorrectRenderingEnabled()) {
+      DCHECK_EQ(options.colorSpaceConversion(), "default");
+      // TODO(zakerinasab): Replace sRGB with a call to
+      // ImageDecoder::globalTargetColorSpace() when the crash problem on Mac
+      // is fixed. crbug.com/668546.
+      parsedOptions.dstColorSpace =
+          SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+      parsedOptions.dstColorType = SkColorType::kN32_SkColorType;
+    } else {
+      if (options.colorSpaceConversion() == "default" ||
+          options.colorSpaceConversion() == "srgb") {
+        parsedOptions.dstColorSpace =
+            SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
+        parsedOptions.dstColorType = SkColorType::kN32_SkColorType;
+      } else if (options.colorSpaceConversion() == "linear-rgb") {
+        parsedOptions.bytesPerPixel = 8;

[Justin Novosad, 2016/11/28 16:30:43]

avoid hard-coded values.  IMHO, we should remove 'bytesPerPixel' completely
since this information is redundant with dstColorSpace.  Just call
SkColorTypeBytesPerPixel() to get the byte per pixel for a given color type.

[zakerinasab, 2016/11/28 19:12:31]

Done.

+        parsedOptions.dstColorSpace =
+            SkColorSpace::MakeNamed(SkColorSpace::kSRGBLinear_Named);
+        parsedOptions.dstColorType = SkColorType::kRGBA_F16_SkColorType;
+      } else {
+        NOTREACHED()
+            << "Invalid ImageBitmap creation attribute colorSpaceConversion: "
+            << options.colorSpaceConversion();
+      }
+    }
+  }
+
   int sourceWidth = sourceSize.width();
   int sourceHeight = sourceSize.height();
   if (!cropRect) {
@@ -232,6 +260,20 @@ static sk_sp<SkImage> unPremulSkImageToPremul(SkImage* input) {
       static_cast<unsigned>(input->width()) * info.bytesPerPixel());
 }
 
+static sk_sp<SkImage> applyColorSpaceConversion(
+    sk_sp<SkImage> image,
+    const ParsedOptions& parsedOptions) {
+  if (!parsedOptions.dstColorSpace)
+    return image;
+
+  SkImageInfo imageInfo = SkImageInfo::Make(
+      image->width(), image->height(), parsedOptions.dstColorType,
+      image->alphaType(), parsedOptions.dstColorSpace);
+  sk_sp<SkSurface> surface = SkSurface::MakeRaster(imageInfo);
+  surface->getCanvas()->drawImage(image, 0, 0);

[Justin Novosad, 2016/11/28 16:30:42]

This works, but it would be even better to have a path that can do color
conversions in-place, especially for cases that require no color type
conversion.  It is possible that Skia has no API for this right now though (not
sure).  Please create a new crbug for this and add a TODO comment right here in
the code, associated with the bug.

[zakerinasab, 2016/11/28 19:12:31]

Done.

+  return surface->makeImageSnapshot();
+}
+
 sk_sp<SkImage> ImageBitmap::getSkImageFromDecoder(
     std::unique_ptr<ImageDecoder> decoder) {
   if (!decoder->frameCount())
@@ -322,6 +364,9 @@ static PassRefPtr<StaticBitmapImage> cropImage(
 
   if (parsedOptions.cropRect == srcRect && !parsedOptions.shouldScaleInput) {
     sk_sp<SkImage> croppedSkImage = skiaImage->makeSubset(srcRect);
+    if (parsedOptions.dstColorSpace) {
+      croppedSkImage = applyColorSpaceConversion(croppedSkImage, parsedOptions);
+    }
     if (parsedOptions.flipY)
       return StaticBitmapImage::create(flipSkImageVertically(
           croppedSkImage.get(), parsedOptions.premultiplyAlpha
@@ -368,6 +413,8 @@ static PassRefPtr<StaticBitmapImage> cropImage(
     surface->getCanvas()->drawImage(skiaImage, dstLeft, dstTop);
   }
   skiaImage = surface->makeImageSnapshot();
+  if (parsedOptions.dstColorSpace)
+    skiaImage = applyColorSpaceConversion(skiaImage, parsedOptions);
 
   if (parsedOptions.premultiplyAlpha) {
     if (imageFormat == DontPremultiplyAlpha)
@@ -388,13 +435,10 @@ ImageBitmap::ImageBitmap(HTMLImageElement* image,
   if (dstBufferSizeHasOverflow(parsedOptions))
     return;
 
-  if (options.colorSpaceConversion() == "none") {
-    m_image = cropImage(input.get(), parsedOptions, PremultiplyAlpha,
-                        ImageDecoder::ColorSpaceIgnored);
-  } else {
-    m_image = cropImage(input.get(), parsedOptions, PremultiplyAlpha,
-                        ImageDecoder::ColorSpaceApplied);
-  }
+  m_image =
+      cropImage(input.get(), parsedOptions, PremultiplyAlpha,
+                parsedOptions.dstColorSpace ? ImageDecoder::ColorSpaceApplied
+                                            : ImageDecoder::ColorSpaceIgnored);
   if (!m_image)
     return;
   // In the case where the source image is lazy-decoded, m_image may not be in