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

Issue 1097583002: cc: Commit property trees to the compositor thread (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years, 5 months ago by enne (OOO)
Modified:
2 years, 5 months ago
Reviewers:
Ian Vollick, ajuma
CC:
cc-bugs_chromium.org, chromium-reviews, weiliangc
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

cc: Commit property trees to the compositor thread This patch is ugly plumbing and sad template boilerplate. It pushes property trees and and property tree-derived values (like tree indices and transform offsets) to the compositor trees. It also calculates (but does not verify) visible rects using property trees on the compositor thread. In order to not crash, this patch also has to build property trees for tests that only have a LayerImpl-side structure and don't have property trees that get committed from a Layer tree. These property trees cannot be verified yet as they are not updated from compositor-side changes like scrolling or animation. BUG=386793 Committed: https://crrev.com/e95b154d48067914e65a091aaee0b78359f7f08f Cr-Commit-Position: refs/heads/master@{#325904}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Now with more push properties #

Patch Set 3 : Rebased on top of needs_rebuild patch #

Total comments: 2

Patch Set 4 : Win bool conversion compile fix #

Unified diffs Side-by-side diffs Delta from patch set Stats (+577 lines, -254 lines) Patch
M cc/layers/layer.h View 1 2 2 chunks +28 lines, -9 lines 0 comments Download
M cc/layers/layer.cc View 1 2 2 chunks +6 lines, -75 lines 0 comments Download
M cc/layers/layer_impl.h View 1 2 3 7 chunks +54 lines, -0 lines 0 comments Download
M cc/layers/layer_impl.cc View 4 chunks +11 lines, -0 lines 0 comments Download
M cc/trees/draw_property_utils.h View 2 chunks +44 lines, -5 lines 0 comments Download
M cc/trees/draw_property_utils.cc View 8 chunks +197 lines, -32 lines 0 comments Download
M cc/trees/layer_tree_host.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cc/trees/layer_tree_host_common.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_host_common.cc View 1 2 3 chunks +105 lines, -75 lines 0 comments Download
M cc/trees/layer_tree_host_unittest.cc View 1 2 4 chunks +20 lines, -11 lines 0 comments Download
M cc/trees/layer_tree_impl.h View 1 2 3 chunks +6 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_impl.cc View 1 2 3 chunks +3 lines, -4 lines 0 comments Download
M cc/trees/property_tree_builder.h View 1 chunk +7 lines, -0 lines 0 comments Download
M cc/trees/property_tree_builder.cc View 1 2 13 chunks +93 lines, -43 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 25 (10 generated)
enne (OOO)
2 years, 5 months ago (2015-04-16 21:36:45 UTC) #2
ajuma
lgtm https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h File cc/layers/layer.h (right): https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480 cc/layers/layer.h:480: // TODO(enne): should these set needs push properties? ...
2 years, 5 months ago (2015-04-16 21:58:15 UTC) #4
enne (OOO)
https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h File cc/layers/layer.h (right): https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480 cc/layers/layer.h:480: // TODO(enne): should these set needs push properties? On ...
2 years, 5 months ago (2015-04-16 22:16:54 UTC) #5
Ian Vollick
On 2015/04/16 22:16:54, enne wrote: > https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h > File cc/layers/layer.h (right): > > https://codereview.chromium.org/1097583002/diff/1/cc/layers/layer.h#newcode480 > ...
2 years, 5 months ago (2015-04-17 13:30:07 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1097583002/20001
2 years, 5 months ago (2015-04-17 15:26:05 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_32_ng/builds/43798) ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, ...
2 years, 5 months ago (2015-04-17 15:33:31 UTC) #11
enne (OOO)
https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_host_unittest.cc File cc/trees/layer_tree_host_unittest.cc (left): https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_host_unittest.cc#oldcode3448 cc/trees/layer_tree_host_unittest.cc:3448: child_->SetPosition(gfx::Point(1, 1)); This code used to adjust the offset ...
2 years, 5 months ago (2015-04-17 20:59:36 UTC) #12
ajuma
https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_host_unittest.cc File cc/trees/layer_tree_host_unittest.cc (left): https://codereview.chromium.org/1097583002/diff/40001/cc/trees/layer_tree_host_unittest.cc#oldcode3448 cc/trees/layer_tree_host_unittest.cc:3448: child_->SetPosition(gfx::Point(1, 1)); On 2015/04/17 20:59:36, enne wrote: > This ...
2 years, 5 months ago (2015-04-17 22:53:11 UTC) #13
enne (OOO)
That seems reasonable. Just curious what y'all thought about that tradeoff. I'll commit this then, ...
2 years, 5 months ago (2015-04-17 23:00:06 UTC) #14
ajuma
On 2015/04/17 23:00:06, enne wrote: > That seems reasonable. Just curious what y'all thought about ...
2 years, 5 months ago (2015-04-17 23:09:54 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1097583002/40001
2 years, 5 months ago (2015-04-17 23:14:39 UTC) #18
commit-bot: I haz the power
Try jobs failed on following builders: win8_chromium_rel on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_rel/builds/74965)
2 years, 5 months ago (2015-04-18 00:46:42 UTC) #20
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1097583002/60001
2 years, 5 months ago (2015-04-20 20:04:03 UTC) #23
commit-bot: I haz the power
Committed patchset #4 (id:60001)
2 years, 5 months ago (2015-04-20 20:35:59 UTC) #24
commit-bot: I haz the power
2 years, 5 months ago (2015-04-20 20:37:30 UTC) #25
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/e95b154d48067914e65a091aaee0b78359f7f08f
Cr-Commit-Position: refs/heads/master@{#325904}
Sign in to reply to this message.

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