Make Canvas2DLayerBridge use SkColorSpaceXformCanvas

third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

Index: third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
diff --git a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
index cf7ef93a9808be5ba8a74d1d40058b64c998be26..e34bfb236654ddcd7393ee0808b5fc743447d478 100644
--- a/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
+++ b/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp
@@ -48,6 +48,7 @@
 #include "public/platform/WebScheduler.h"
 #include "public/platform/WebTraceLocation.h"
 #include "skia/ext/texture_handle.h"
+#include "third_party/skia/include/core/SkColorSpaceXformCanvas.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkSurface.h"
 #include "third_party/skia/include/gpu/GrContext.h"
@@ -779,8 +780,16 @@ void Canvas2DLayerBridge::FlushRecordingOnly() {
 
   if (have_recorded_draw_commands_ && GetOrCreateSurface()) {
     TRACE_EVENT0("cc", "Canvas2DLayerBridge::flushRecordingOnly");
-    recorder_->finishRecordingAsPicture()->playback(
-        GetOrCreateSurface()->getCanvas());
+
+    SkCanvas* canvas = GetOrCreateSurface()->getCanvas();
+    std::unique_ptr<SkCanvas> color_transform_canvas;
+    if (!sk_surfaces_use_color_space_) {

[ccameron, 2017/04/15 01:15:49]

Is there a reason why we can't plumb the CanvasColorSpace and CanvasPixelFormat
parameters from CanvasRenderingContext directly to here?

As it is, we derive the gfx::ColorSpace, sk_surfaces_use_color_space_, and
SkColorType arguments from them -- I'd rather just add helper functions to
convert from CanvasColorSpace+CanvasPixelFormat the various parameters we need.

[Justin Novosad, 2017/04/17 14:41:21]

We could. I think that we originally thought gfx::ColorSpace and SkColorType
would be the most convenient forms for passing this info, but we weren't really
sure where the code was heading when we wrote that. 

[ccameron, 2017/04/17 18:27:17]

Okay -- I may add a CanvasColorSpace.h with some helpers & structures to plumb
these throughout.

+      color_transform_canvas =
+          SkCreateColorSpaceXformCanvas(canvas, color_space_.ToSkColorSpace());

[Justin Novosad, 2017/04/17 14:41:21]

Why is this necessary? In CreateSkSurface (above), we create a surface that is
tagged with the right color space.  Instead of patching through an intermediate
canvas here, could we just set up the surface correctly in CreateSkSurface for
the legacy case?

[ccameron, 2017/04/17 18:27:17]

Tagging a surface with a color space opts us in to linear blending. Getting
color conversion without linear blending requires running the draw command
through a ColorSpaceXformCanvas.

[msarett1, 2017/04/17 18:42:46]

+1, this is the recommended approach.

+      canvas = color_transform_canvas.get();
+    }
+
+    recorder_->finishRecordingAsPicture()->playback(canvas);
     if (is_deferral_enabled_)
       StartRecording();
     have_recorded_draw_commands_ = false;