Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(7)

Issue 2820823004: Make Canvas2DLayerBridge use SkColorSpaceXformCanvas (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 month, 2 weeks ago by ccameron
Modified:
1 month, 1 week ago
CC:
chromium-reviews, krit, drott+blinkwatch_chromium.org, ajuma+watch-canvas_chromium.org, dshwang, pdr+graphicswatchlist_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, fmalita+watch_chromium.org, blink-reviews, kinuko+watch, ajuma+watch_chromium.org, Stephen Chennney, rwlbuis
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Make Canvas2DLayerBridge use SkColorSpaceXformCanvas This makes legacy mode canvases render into sRGB (transforming images as needed). BUG=709695 Review-Url: https://codereview.chromium.org/2820823004 Cr-Commit-Position: refs/heads/master@{#465069} Committed: https://chromium.googlesource.com/chromium/src/+/2639037b06c07f5613640e1a5f5a98c5932cb8a0

Patch Set 1 #

Total comments: 6

Patch Set 2 : Put behind flag #

Total comments: 2

Patch Set 3 : Add comment #

Messages

Total messages: 19 (6 generated)
ccameron
https://codereview.chromium.org/2820823004/diff/1/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/2820823004/diff/1/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode786 third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:786: if (!sk_surfaces_use_color_space_) { Is there a reason why we ...
1 month, 2 weeks ago (2017-04-15 01:15:49 UTC) #1
ccameron
Adding msarett@ for content, pdr@ for OWNERs. I'd like to plumb the CanvasColorSpace and CanvasPixelFormat ...
1 month, 1 week ago (2017-04-16 08:51:48 UTC) #3
Justin Novosad
This changes current shipping behavior in a significant way. Requires an Intent To Ship IMHO. ...
1 month, 1 week ago (2017-04-17 14:41:21 UTC) #5
Justin Novosad
On 2017/04/17 14:41:21, junov - OoO - back April 24 wrote: > This changes current ...
1 month, 1 week ago (2017-04-17 14:46:23 UTC) #6
ccameron
Yes, this was supposed to be behind a RuntimeEnabledFeatures::colorCorrectRenderingEnabled. Updated. https://codereview.chromium.org/2820823004/diff/1/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/2820823004/diff/1/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode786 ...
1 month, 1 week ago (2017-04-17 18:27:18 UTC) #7
msarett1
> Yes, this was supposed to be behind a RuntimeEnabledFeatures::colorCorrectRenderingEnabled. Updated. Did you mean to ...
1 month, 1 week ago (2017-04-17 18:42:46 UTC) #8
ccameron
On 2017/04/17 18:42:46, msarett1 wrote: > > Yes, this was supposed to be behind a ...
1 month, 1 week ago (2017-04-17 18:45:23 UTC) #9
msarett1
lgtm https://codereview.chromium.org/2820823004/diff/20001/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/2820823004/diff/20001/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode789 third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:789: SkCreateColorSpaceXformCanvas(canvas, color_space_.ToSkColorSpace()); Just curious, is color_space_ always sRGB?
1 month, 1 week ago (2017-04-17 18:57:50 UTC) #10
ccameron
Thanks! https://codereview.chromium.org/2820823004/diff/20001/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp File third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp (right): https://codereview.chromium.org/2820823004/diff/20001/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp#newcode789 third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp:789: SkCreateColorSpaceXformCanvas(canvas, color_space_.ToSkColorSpace()); On 2017/04/17 18:57:50, msarett1 wrote: > ...
1 month, 1 week ago (2017-04-17 19:35:13 UTC) #11
Justin Novosad
lgtm. But please add a comment that explains why the ColorSpaceXformCanvas is required since it ...
1 month, 1 week ago (2017-04-17 20:34:13 UTC) #12
ccameron
Thanks!
1 month, 1 week ago (2017-04-17 21:57:42 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2820823004/40001
1 month, 1 week ago (2017-04-17 21:58:21 UTC) #16
commit-bot: I haz the power
1 month, 1 week ago (2017-04-18 00:06:30 UTC) #19
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2639037b06c07f5613640e1a5f5a...
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06