color: Perform color transform in ImageBufferSurface

color: Perform color transform in ImageBufferSurface

When we draw a canvas element to a AcceleratedImageBufferSurface or
to a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2904143002
Cr-Commit-Position: refs/heads/master@{#475412}
Committed: https://chromium.googlesource.com/chromium/src/+/30a7f83076be71351b71dfcb6cefee6ec2a9faca

[ccameron, 2017-05-25 22:06:07]

Description was changed from

==========
color: Perform color transform in ImageBufferSurface

When draw a canvas element to a AcceleratedImageBufferSurface or to
a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
==========

to

==========
color: Perform color transform in ImageBufferSurface

When draw a canvas element to a AcceleratedImageBufferSurface or to
a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-05-25 22:06:41]

Description was changed from

==========
color: Perform color transform in ImageBufferSurface

When draw a canvas element to a AcceleratedImageBufferSurface or to
a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
color: Perform color transform in ImageBufferSurface

When we draw a canvas element to a AcceleratedImageBufferSurface or
to a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ccameron, 2017-05-25 22:06:41]

ccameron@chromium.org changed reviewers:
+ enne@chromium.org, junov@chromium.org

[ccameron, 2017-05-25 22:08:10]

This fixes offscreen and non-display-list-un-accelerated canvases.

[ccameron, 2017-05-26 17:21:58]

ping on this

[Justin Novosad, 2017-05-29 14:45:35]

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp
(right):

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp:62:
RuntimeEnabledFeatures::colorCorrectRenderingEnabled() &&
Any tests for this?  Does it affect layout tests under
virtual/color_space/fast/canvas/color-space?  If not we probably need an
additional test.

As a drive-by feel free to convert that to a "physical test suite".

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp
(right):

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp:68:
RuntimeEnabledFeatures::colorCorrectRenderingEnabled() &&
Why is this needed? This calss is not used for rastering. It is only used as a
vehicle for WebGL snapshots.  If you are looking for the place where accelerated
2D canvases are rasterized, that place is Canvas2DLayerBridge.  IMHO, you should
create a gpu-accelerated variant of the color_space virtual test suite.

[ccameron, 2017-05-29 15:27:17]

This patch is to fix tests that are already failing with
--enable-color-correct-rendering, so that I can enable a physical and virtual
test suite for color correct rendering.

The tests in color_space are just for the canvas color extensions.

This work is for --enable-color-correct-rendering, and is shipping in M61
(needed for Android O DR), which is long before we will make the canvas
extensions non-experimental.

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp
(right):

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/UnacceleratedImageBufferSurface.cpp:62:
RuntimeEnabledFeatures::colorCorrectRenderingEnabled() &&
On 2017/05/29 14:45:35, Justin Novosad wrote:
> Any tests for this?  Does it affect layout tests under
> virtual/color_space/fast/canvas/color-space?  If not we probably need an
> additional test.
> 
> As a drive-by feel free to convert that to a "physical test suite".

This patch is part of the "fix the pile of bugs that are preventing us from
enabling layout tests with --enable-color-correct-rendering".

The CL to enable the tests (which still has lots of bugs like this that are
preventing it from getting committed) is:
https://codereview.chromium.org/2889313002/

The goal of that patch is to have images/
- run with --enable-color-correct-rendering
- run with both CPU and GPU raster (as a virtual test suite for GPU)

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp
(right):

https://codereview.chromium.org/2904143002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/graphics/gpu/AcceleratedImageBufferSurface.cpp:68:
RuntimeEnabledFeatures::colorCorrectRenderingEnabled() &&
On 2017/05/29 14:45:35, Justin Novosad wrote:
> Why is this needed? This calss is not used for rastering. It is only used as a
> vehicle for WebGL snapshots.  If you are looking for the place where
accelerated
> 2D canvases are rasterized, that place is Canvas2DLayerBridge.  IMHO, you
should
> create a gpu-accelerated variant of the color_space virtual test suite.

This looked like the right behavior for offscreen canvas:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/offscreen...

I don't remember offhand if this was required to fix a particular layout test,
or if this was speculative. Given the SkSurface creation properties, this
definitely feels right.

[Justin Novosad, 2017-05-29 16:54:35]

> This looked like the right behavior for offscreen canvas:

Oh right, OffscreenCanvas! Of course!


Okay, you've convinced me that you know what your doing here.
lgtm.

[enne (OOO), 2017-05-29 23:35:07]

lgtm

[ccameron, 2017-05-30 00:45:28]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2017-05-30 00:45:39]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 03:10:17]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1496105128738470, "parent_rev":
"8d639e9e2ac1811b38e6bacc13e73049b27750f4", "commit_rev":
"30a7f83076be71351b71dfcb6cefee6ec2a9faca"}

[commit-bot: I haz the power, 2017-05-30 03:10:24]

Description was changed from

==========
color: Perform color transform in ImageBufferSurface

When we draw a canvas element to a AcceleratedImageBufferSurface or
to a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
color: Perform color transform in ImageBufferSurface

When we draw a canvas element to a AcceleratedImageBufferSurface or
to a UnacceleratedImageBufferSurface, we must transform the inputs to
the color space specified by the CanvasColorParams it was initialized
with.

To do this, make the SkiaPaintCanvas wrap the SkCanvas in a
SkColorSpaceXformCanvas.

BUG=725372
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2904143002
Cr-Commit-Position: refs/heads/master@{#475412}
Committed:
https://chromium.googlesource.com/chromium/src/+/30a7f83076be71351b71dfcb6cef...
==========

[commit-bot: I haz the power, 2017-05-30 03:10:25]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/30a7f83076be71351b71dfcb6cef...