Chromium Code Reviews
Help | Chromium Project | Sign in
(23)

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 years ago by enne
Modified:
2 years 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
2 years 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 ago (2015-04-16 21:58:15 UTC) #4
enne
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 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 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 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 ago (2015-04-17 15:33:31 UTC) #11
enne
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 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 ago (2015-04-17 22:53:11 UTC) #13
enne
That seems reasonable. Just curious what y'all thought about that tradeoff. I'll commit this then, ...
2 years 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 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 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 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 ago (2015-04-20 20:04:03 UTC) #23
commit-bot: I haz the power
Committed patchset #4 (id:60001)
2 years ago (2015-04-20 20:35:59 UTC) #24
commit-bot: I haz the power
2 years 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 cc6ac46