color: Convert BitmapImage to sRGB ahead of time

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

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/frame/ImageBitmap.h"
 
 #include <memory>
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLVideoElement.h"
 #include "core/html/ImageData.h"
 #include "core/offscreencanvas/OffscreenCanvas.h"
 #include "platform/graphics/CanvasColorParams.h"
 #include "platform/graphics/skia/SkiaUtils.h"
 #include "platform/image-decoders/ImageDecoder.h"
 #include "platform/wtf/CheckedNumeric.h"
 #include "platform/wtf/PtrUtil.h"
 #include "platform/wtf/RefPtr.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/core/SkColorSpaceXformCanvas.h"
 #include "third_party/skia/include/core/SkImageInfo.h"
 #include "third_party/skia/include/core/SkSurface.h"
 
 namespace blink {
 
 constexpr const char* kImageOrientationFlipY = "flipY";
 constexpr const char* kImageBitmapOptionNone = "none";
 constexpr const char* kImageBitmapOptionDefault = "default";
 constexpr const char* kImageBitmapOptionPremultiply = "premultiply";
 constexpr const char* kImageBitmapOptionResizeQualityHigh = "high";
 constexpr const char* kImageBitmapOptionResizeQualityMedium = "medium";
 constexpr const char* kImageBitmapOptionResizeQualityPixelated = "pixelated";
 constexpr const char* kSRGBImageBitmapColorSpaceConversion = "srgb";
 constexpr const char* kLinearRGBImageBitmapColorSpaceConversion = "linear-rgb";
 constexpr const char* kP3ImageBitmapColorSpaceConversion = "p3";
 constexpr const char* kRec2020ImageBitmapColorSpaceConversion = "rec2020";
 
 namespace {
 
 struct ParsedOptions {
   bool flip_y = false;
   bool premultiply_alpha = true;
   bool should_scale_input = false;
   unsigned resize_width = 0;
   unsigned resize_height = 0;
   IntRect crop_rect;
   SkFilterQuality resize_quality = kLow_SkFilterQuality;
   CanvasColorParams color_params;
   bool color_canvas_extensions_enabled = false;
+  bool convert_to_srgb = false;

[Justin Novosad, 2017/06/09 14:08:34]

This new member is confusing IMHO because it potentially conflicts with the
color space stored in color_params, and it may not be clear which of the two is
the ground truth.  I think the cleaner solution is to tweak the code that sets
the color space in color_params.  That might be the only code you need to touch
to fix this issue.

[ccameron, 2017/06/09 17:01:33]

I removed this and replaced it with "color_params.UsesOutputSpaceBlending &&
ColorCorrectRenderingEnabled".

 };
 
 ParsedOptions DefaultOptions() {
   return ParsedOptions();
 }
 
 // The following two functions are helpers used in cropImage
 static inline IntRect NormalizeRect(const IntRect& rect) {
   return IntRect(std::min(rect.X(), rect.MaxX()),
                  std::min(rect.Y(), rect.MaxY()),
@@ skipping 13 matching lines @@
   }
   if (options.premultiplyAlpha() == kImageBitmapOptionNone) {
     parsed_options.premultiply_alpha = false;
   } else {
     parsed_options.premultiply_alpha = true;
     DCHECK(options.premultiplyAlpha() == kImageBitmapOptionDefault ||
            options.premultiplyAlpha() == kImageBitmapOptionPremultiply);
   }
 
   if (options.colorSpaceConversion() != kImageBitmapOptionNone) {
-    parsed_options.color_canvas_extensions_enabled =
-        RuntimeEnabledFeatures::ColorCanvasExtensionsEnabled();
+    if (RuntimeEnabledFeatures::ColorCanvasExtensionsEnabled()) {
+      parsed_options.color_canvas_extensions_enabled = true;
+    } else {
+      // When canvas color extensions are not enabled, all images may be
+      // converted ahead of time to sRGB, since that is what their destination
+      // will always be sRGB.
+      parsed_options.convert_to_srgb =
+          RuntimeEnabledFeatures::ColorCorrectRenderingEnabled();
+    }
+
     if (!parsed_options.color_canvas_extensions_enabled) {
       DCHECK_EQ(options.colorSpaceConversion(), kImageBitmapOptionDefault);
     } else {
       if (options.colorSpaceConversion() == kImageBitmapOptionDefault ||
           options.colorSpaceConversion() ==
               kSRGBImageBitmapColorSpaceConversion) {
         parsed_options.color_params.SetCanvasColorSpace(kSRGBCanvasColorSpace);
       } else if (options.colorSpaceConversion() ==
                  kLinearRGBImageBitmapColorSpaceConversion) {
         parsed_options.color_params.SetCanvasColorSpace(kSRGBCanvasColorSpace);
@@ skipping 202 matching lines @@
   RefPtr<Uint8Array> dst_pixels = CopySkImageData(input, info);
   if (!dst_pixels)
     return nullptr;
   return NewSkImageFromRaster(
       info, std::move(dst_pixels),
       static_cast<unsigned>(input->width()) * info.bytesPerPixel());
 }
 
 static void ApplyColorSpaceConversion(sk_sp<SkImage>& image,
                                       ParsedOptions& options) {
+  if (options.convert_to_srgb) {
+    image = image->makeColorSpace(SkColorSpace::MakeSRGB(),
+                                  SkTransferFunctionBehavior::kIgnore);
+  }
   if (!options.color_canvas_extensions_enabled)
     return;
 
   sk_sp<SkColorSpace> dst_color_space = nullptr;
   SkColorType dst_color_type = kN32_SkColorType;
   dst_color_space = options.color_params.GetSkColorSpace();
   dst_color_type = options.color_params.GetSkColorType();
   if (SkColorSpace::Equals(image->colorSpace(), dst_color_space.get()))
     return;
 
@@ skipping 151 matching lines @@
     // Special case: The first parameter image is unpremul but we need to turn
     // it into premul.
     if (parsed_options.premultiply_alpha &&
         image_format == kDontPremultiplyAlpha) {
       return StaticBitmapImage::Create(
           UnPremulSkImageToPremul(cropped_sk_image.get()));
     }
     return StaticBitmapImage::Create(std::move(cropped_sk_image));
   }
 
   sk_sp<SkSurface> surface = SkSurface::MakeRasterN32Premul(

[Justin Novosad, 2017/06/09 14:08:34]

I think it would be better to just specify a color space for this SkSurface.
Then we would not need the ColorSpaceXformCanvas below.

[ccameron, 2017/06/09 17:01:33]

Doing that will opt us in to linear blending, which has a couple of
side-effects. One is that it will not match previous behavior. The other, more
serious effect is that that path doesn't have as wide support -- it depends on
the GL sRGB extensions (which aren't always there).

       parsed_options.resize_width, parsed_options.resize_height);
   if (!surface)
     return nullptr;
   if (src_rect.IsEmpty())
     return StaticBitmapImage::Create(surface->makeImageSnapshot());
 
+  SkCanvas* canvas = surface->getCanvas();
+  std::unique_ptr<SkCanvas> color_transform_canvas;
+  if (parsed_options.convert_to_srgb) {
+    color_transform_canvas =
+        SkCreateColorSpaceXformCanvas(canvas, SkColorSpace::MakeSRGB());
+    canvas = color_transform_canvas.get();
+  }
+
   SkScalar dst_left = std::min(0, -parsed_options.crop_rect.X());
   SkScalar dst_top = std::min(0, -parsed_options.crop_rect.Y());
   if (parsed_options.crop_rect.X() < 0)
     dst_left = -parsed_options.crop_rect.X();
   if (parsed_options.crop_rect.Y() < 0)
     dst_top = -parsed_options.crop_rect.Y();
   if (parsed_options.flip_y) {
-    surface->getCanvas()->translate(0, surface->height());
-    surface->getCanvas()->scale(1, -1);
+    canvas->translate(0, surface->height());
+    canvas->scale(1, -1);
   }
   if (parsed_options.should_scale_input) {
     SkRect draw_src_rect = SkRect::MakeXYWH(
         parsed_options.crop_rect.X(), parsed_options.crop_rect.Y(),
         parsed_options.crop_rect.Width(), parsed_options.crop_rect.Height());
     SkRect draw_dst_rect = SkRect::MakeXYWH(0, 0, parsed_options.resize_width,
                                             parsed_options.resize_height);
     SkPaint paint;
     paint.setFilterQuality(parsed_options.resize_quality);
-    surface->getCanvas()->drawImageRect(skia_image, draw_src_rect,
-                                        draw_dst_rect, &paint);
+    canvas->drawImageRect(skia_image, draw_src_rect, draw_dst_rect, &paint);
   } else {
-    surface->getCanvas()->drawImage(skia_image, dst_left, dst_top);
+    canvas->drawImage(skia_image, dst_left, dst_top);
   }
   skia_image = surface->makeImageSnapshot();
   ApplyColorSpaceConversion(skia_image, parsed_options);
 
   if (parsed_options.premultiply_alpha) {
     if (image_format == kDontPremultiplyAlpha)
       return StaticBitmapImage::Create(
           UnPremulSkImageToPremul(skia_image.get()));
     return StaticBitmapImage::Create(std::move(skia_image));
   }
@@ skipping 28 matching lines @@
   SkPixmap pixmap;
   if (!sk_image->isTextureBacked() && !sk_image->peekPixels(&pixmap)) {
     sk_sp<SkColorSpace> dst_color_space = nullptr;
     SkColorType dst_color_type = kN32_SkColorType;
     if (parsed_options.color_canvas_extensions_enabled) {
       dst_color_space = parsed_options.color_params.GetSkColorSpace();
       dst_color_type = parsed_options.color_params.GetSkColorType();
     }
     SkImageInfo image_info =
         SkImageInfo::Make(sk_image->width(), sk_image->height(), dst_color_type,
                           kPremul_SkAlphaType, dst_color_space);

[Justin Novosad, 2017/06/09 14:08:34]

We should just tweak the dst_color_space used here instead of setting up a
ColorSpaceXformCanvas below.

[ccameron, 2017/06/09 17:01:33]

Updated this to still use the xform canvas, but to also use
parsed_options.color_params.GetSkColorSpace, since there's no reason to
hard-code sRGB.

     sk_sp<SkSurface> surface = SkSurface::MakeRaster(image_info);
-    surface->getCanvas()->drawImage(sk_image, 0, 0);
+    SkCanvas* canvas = surface->getCanvas();
+    std::unique_ptr<SkCanvas> color_transform_canvas;
+    if (parsed_options.convert_to_srgb) {
+      color_transform_canvas =
+          SkCreateColorSpaceXformCanvas(canvas, SkColorSpace::MakeSRGB());
+      canvas = color_transform_canvas.get();
+    }
+    canvas->drawImage(sk_image, 0, 0);
     image_ = StaticBitmapImage::Create(surface->makeImageSnapshot());
   }
   if (!image_)
     return;
   image_->SetOriginClean(
       !image->WouldTaintOrigin(document->GetSecurityOrigin()));
   image_->SetPremultiplied(parsed_options.premultiply_alpha);
 }
 
 ImageBitmap::ImageBitmap(HTMLVideoElement* video,
@@ skipping 332 matching lines @@
                        src_rect, dst_point);
   sk_sp<SkImage> sk_image =
       buffer->NewSkImageSnapshot(kPreferNoAcceleration, kSnapshotReasonUnknown);
   if (parsed_options.flip_y)
     sk_image = FlipSkImageVertically(sk_image.get(), kEnforceAlphaPremultiply);
   if (!sk_image)
     return;
   if (parsed_options.should_scale_input) {
     sk_sp<SkSurface> surface = SkSurface::MakeRaster(SkImageInfo::MakeN32Premul(
         parsed_options.resize_width, parsed_options.resize_height,
         data->GetSkColorSpace()));

[Justin Novosad, 2017/06/09 14:08:34]

data->GetColorSpace() should return return srgb if that is the desired color
space.  Then the ColorSpaceXformCanvas would not be needed.

[ccameron, 2017/06/09 17:01:33]

Same here.

     if (!surface)
       return;
     SkPaint paint;
     paint.setFilterQuality(parsed_options.resize_quality);
     SkRect dst_draw_rect = SkRect::MakeWH(parsed_options.resize_width,
                                           parsed_options.resize_height);
-    surface->getCanvas()->drawImageRect(sk_image, dst_draw_rect, &paint);
+    SkCanvas* canvas = surface->getCanvas();
+    std::unique_ptr<SkCanvas> color_transform_canvas;
+    if (parsed_options.convert_to_srgb) {
+      color_transform_canvas =
+          SkCreateColorSpaceXformCanvas(canvas, SkColorSpace::MakeSRGB());
+      canvas = color_transform_canvas.get();
+    }
+    canvas->drawImageRect(sk_image, dst_draw_rect, &paint);
     sk_image = surface->makeImageSnapshot();
   }
   ApplyColorSpaceConversion(sk_image, parsed_options);
   image_ = StaticBitmapImage::Create(std::move(sk_image));
 }
 
 ImageBitmap::ImageBitmap(ImageBitmap* bitmap,
                          Optional<IntRect> crop_rect,
                          const ImageBitmapOptions& options) {
   RefPtr<Image> input = bitmap->BitmapImage();
@@ skipping 189 matching lines @@
 void ImageBitmap::AdjustDrawRects(FloatRect* src_rect,
                                   FloatRect* dst_rect) const {}
 
 FloatSize ImageBitmap::ElementSize(const FloatSize&) const {
   return FloatSize(width(), height());
 }
 
 DEFINE_TRACE(ImageBitmap) {}
 
 }  // namespace blink