Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1932393002:
Turn off IOSurface implementation for Canvas2D. (Closed)
|
Created:
4 years, 7 months ago by erikchen Modified:
4 years, 7 months ago CC:
chromium-reviews, krit, pdr+graphicswatchlist_chromium.org, drott+blinkwatch_chromium.org, blink-reviews-platform-graphics_chromium.org, ajuma+watch-canvas_chromium.org, dshwang, jbroman, danakj+watch_chromium.org, Rik, f(malita), blink-reviews, kinuko+watch, Stephen Chennney, rwlbuis Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTurn off IOSurface implementation for Canvas2D.
IOSurface texures can't be reused because the compositor returns them while they
are still in use by the Window Server, and the SyncToken isn't relevant.
BUG=608026
Committed: https://crrev.com/64d59f650c4f3a957f28943858fbb6b60754b98b
Cr-Commit-Position: refs/heads/master@{#391043}
Patch Set 1 #Patch Set 2 : Comments from ccameron. #Messages
Total messages: 24 (10 generated)
The CQ bit was checked by erikchen@chromium.org to run a CQ dry run
erikchen@chromium.org changed reviewers: + ccameron@chromium.org, junov@chromium.org
junov, ccameron: Please review.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932393002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932393002/1
lgtm -- but will this have performance issues with lots of alloc-and-free -- if so, should we also disable IOSurface use?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/04/29 22:21:32, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. Sent this CL to the perf trybots: https://codereview.chromium.org/1936743002 Depending on those results, we can decide whether to turn this off entirely, or have lots of alloc-and-free.
On 2016/04/29 22:29:14, erikchen wrote: > On 2016/04/29 22:21:32, commit-bot: I haz the power wrote: > > Dry run: This issue passed the CQ dry run. > > Sent this CL to the perf trybots: > https://codereview.chromium.org/1936743002 > > Depending on those results, we can decide whether to turn this off entirely, or > have lots of alloc-and-free. Sounds fair. We should also check power of an "animating canvas thing" before+after (not sure what's on the perf tryjobs) -- I'd be surprised if repeated alloc+free didn't have a big hit.
Description was changed from ========== Don't reuse IOSurfaces for Canvas2D. Disable IOSurface texures reuse, because the compositor returns them while they are still in use, and the SyncToken isn't relevant. It is still better to use IOSurfaces than plain textures, since this allows the use of the Core Animation compositor. BUG=608026 ========== to ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ==========
Description was changed from ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ========== to ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ==========
I updated the CL to turn off IOSurfaces for Canvas2D.
The CQ bit was checked by erikchen@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932393002/20001
lgtm, sadly. Hopefully we can get the IsInUse hooked up to the browser reasonably quickly.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/04/30 01:14:19, commit-bot: I haz the power wrote: > Dry run: This issue passed the CQ dry run. lgtm
The CQ bit was checked by erikchen@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1932393002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1932393002/20001
Message was sent while issue was closed.
Description was changed from ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ========== to ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 ========== to ========== Turn off IOSurface implementation for Canvas2D. IOSurface texures can't be reused because the compositor returns them while they are still in use by the Window Server, and the SyncToken isn't relevant. BUG=608026 Committed: https://crrev.com/64d59f650c4f3a957f28943858fbb6b60754b98b Cr-Commit-Position: refs/heads/master@{#391043} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/64d59f650c4f3a957f28943858fbb6b60754b98b Cr-Commit-Position: refs/heads/master@{#391043} |
