Make Canvas2DLayerBridge use SkColorSpaceXformCanvas

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

 /*
  * Copyright (C) 2012 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 30 matching lines @@
 #include "platform/graphics/gpu/SharedContextRateLimiter.h"
 #include "platform/graphics/paint/PaintCanvas.h"
 #include "platform/instrumentation/tracing/TraceEvent.h"
 #include "platform/wtf/PtrUtil.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebCompositorSupport.h"
 #include "public/platform/WebGraphicsContext3DProvider.h"
 #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"
 #include "third_party/skia/include/gpu/GrTexture.h"
 #include "third_party/skia/include/gpu/gl/GrGLTypes.h"
 
 namespace {
 enum {
   InvalidMailboxIndex = -1,
   MaxCanvasAnimationBacklog = 2,  // Make sure the the GPU is never more than
@@ skipping 711 matching lines @@
     if (rate_limiter_)
       rate_limiter_->Reset();
   }
 }
 
 void Canvas2DLayerBridge::FlushRecordingOnly() {
   DCHECK(!destruction_in_progress_);
 
   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;
   }
 }
 
 void Canvas2DLayerBridge::Flush() {
   if (!did_draw_since_last_flush_)
     return;
   TRACE_EVENT0("cc", "Canvas2DLayerBridge::flush");
@@ skipping 323 matching lines @@
     default;
 
 void Canvas2DLayerBridge::Logger::ReportHibernationEvent(
     HibernationEvent event) {
   DEFINE_STATIC_LOCAL(EnumerationHistogram, hibernation_histogram,
                       ("Canvas.HibernationEvents", kHibernationEventCount));
   hibernation_histogram.Count(event);
 }
 
 }  // namespace blink