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

Issue 1491033002: Create RenderSurface on Effect Tree (Closed)

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

Description

Create RenderSurface on Effect Tree Move RenderSurface creation reason to effect tree. Update LayerImpl's API to include SetForceRenderSurface. Update unittests. R=enne BUG=557160 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://crrev.com/4374c2cc231a7e5b6ae65d0e0814b5dccf445200 Cr-Commit-Position: refs/heads/master@{#363544} Committed: https://crrev.com/c154ce2efa59dbaecb3cb038adb247426f7c6988 Cr-Commit-Position: refs/heads/master@{#363967}

Patch Set 1 #

Patch Set 2 : rebase on master #

Total comments: 7

Patch Set 3 : address review comments #

Total comments: 13

Patch Set 4 : address review comments #

Total comments: 2

Patch Set 5 : merge dup code to function based on review comments #

Patch Set 6 : rebase #

Patch Set 7 : rebase #

Patch Set 8 : rebase #

Patch Set 9 : fix for crash #

Unified diffs Side-by-side diffs Delta from patch set Stats (+631 lines, -502 lines) Patch
M cc/layers/delegated_renderer_layer_impl_unittest.cc View 12 chunks +16 lines, -16 lines 0 comments Download
M cc/layers/layer.h View 1 2 3 4 5 6 7 2 chunks +4 lines, -4 lines 0 comments Download
M cc/layers/layer.cc View 1 2 3 4 5 6 7 2 chunks +2 lines, -2 lines 0 comments Download
M cc/layers/layer_impl.h View 1 2 3 4 5 6 7 4 chunks +6 lines, -1 line 0 comments Download
M cc/layers/layer_impl.cc View 1 2 3 4 5 6 7 8 6 chunks +22 lines, -2 lines 0 comments Download
M cc/layers/layer_impl_unittest.cc View 1 2 3 4 5 6 7 2 chunks +10 lines, -4 lines 0 comments Download
M cc/layers/layer_iterator_unittest.cc View 4 chunks +3 lines, -6 lines 0 comments Download
M cc/layers/nine_patch_layer_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/layers/picture_layer_impl_perftest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/layers/picture_layer_impl_unittest.cc View 1 3 chunks +3 lines, -3 lines 0 comments Download
M cc/layers/render_surface_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/layers/solid_color_layer_impl_unittest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M cc/layers/ui_resource_layer_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/layers/video_layer_impl_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/test/layer_test_common.h View 1 chunk +9 lines, -0 lines 0 comments Download
M cc/test/layer_tree_host_common_test.h View 4 chunks +19 lines, -0 lines 0 comments Download
M cc/test/layer_tree_host_common_test.cc View 1 2 3 4 5 6 5 chunks +20 lines, -8 lines 0 comments Download
M cc/tiles/tile_manager_perftest.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M cc/trees/damage_tracker_unittest.cc View 14 chunks +14 lines, -15 lines 0 comments Download
M cc/trees/draw_property_utils.cc View 1 2 3 4 5 6 6 chunks +65 lines, -2 lines 0 comments Download
M cc/trees/layer_tree_host.cc View 1 2 3 4 5 6 7 1 chunk +1 line, -5 lines 0 comments Download
M cc/trees/layer_tree_host_common.h View 1 chunk +0 lines, -8 lines 0 comments Download
M cc/trees/layer_tree_host_common.cc View 1 2 3 4 5 6 7 12 chunks +14 lines, -199 lines 0 comments Download
M cc/trees/layer_tree_host_common_unittest.cc View 1 2 3 4 5 6 7 25 chunks +182 lines, -139 lines 0 comments Download
M cc/trees/layer_tree_host_impl.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M cc/trees/layer_tree_host_impl.cc View 1 2 3 4 5 1 chunk +7 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_host_impl_unittest.cc View 1 2 3 4 5 44 chunks +45 lines, -46 lines 0 comments Download
M cc/trees/layer_tree_impl.h View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M cc/trees/layer_tree_impl.cc View 1 2 3 4 5 6 1 chunk +8 lines, -0 lines 0 comments Download
M cc/trees/layer_tree_impl_unittest.cc View 1 5 chunks +9 lines, -4 lines 0 comments Download
M cc/trees/occlusion_tracker_perftest.cc View 1 chunk +1 line, -1 line 0 comments Download
M cc/trees/occlusion_tracker_unittest.cc View 13 chunks +14 lines, -15 lines 0 comments Download
M cc/trees/property_tree_builder.cc View 1 2 3 4 5 6 12 chunks +146 lines, -13 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 32 (11 generated)
weiliangc
Spirit child of my previous patch on move RS creation to impl thread (crrev.com/1449193002). Cleaned ...
1 year, 9 months ago (2015-12-02 01:07:28 UTC) #2
ajuma
https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc File cc/trees/layer_tree_host_common_unittest.cc (right): https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc#newcode8785 cc/trees/layer_tree_host_common_unittest.cc:8785: EXPECT_EQ(gfx::Rect(22, 21), test_layer->visible_rect_from_property_trees()); Is this change because layer_clips_subtree has ...
1 year, 9 months ago (2015-12-02 14:54:21 UTC) #4
weiliangc
Updated according to review comments. https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc File cc/trees/layer_tree_host_common_unittest.cc (right): https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc#newcode8785 cc/trees/layer_tree_host_common_unittest.cc:8785: EXPECT_EQ(gfx::Rect(22, 21), test_layer->visible_rect_from_property_trees()); On ...
1 year, 9 months ago (2015-12-02 18:16:39 UTC) #5
ajuma
Thanks, lgtm % enne. https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc File cc/trees/layer_tree_host_common_unittest.cc (right): https://codereview.chromium.org/1491033002/diff/20001/cc/trees/layer_tree_host_common_unittest.cc#newcode8785 cc/trees/layer_tree_host_common_unittest.cc:8785: EXPECT_EQ(gfx::Rect(22, 21), test_layer->visible_rect_from_property_trees()); On 2015/12/02 ...
1 year, 9 months ago (2015-12-02 18:25:08 UTC) #6
enne (OOO)
Looking great in general. A few general comments. https://codereview.chromium.org/1491033002/diff/40001/cc/layers/layer_impl.cc File cc/layers/layer_impl.cc (right): https://codereview.chromium.org/1491033002/diff/40001/cc/layers/layer_impl.cc#newcode1813 cc/layers/layer_impl.cc:1813: void ...
1 year, 9 months ago (2015-12-02 21:59:51 UTC) #7
weiliangc
https://codereview.chromium.org/1491033002/diff/40001/cc/trees/draw_property_utils.cc File cc/trees/draw_property_utils.cc (right): https://codereview.chromium.org/1491033002/diff/40001/cc/trees/draw_property_utils.cc#newcode465 cc/trees/draw_property_utils.cc:465: void UpdateRenderSurfacesWithEffectTree(EffectTree* effect_tree, On 2015/12/02 at 21:59:51, enne wrote: ...
1 year, 9 months ago (2015-12-04 00:54:54 UTC) #8
ajuma
https://codereview.chromium.org/1491033002/diff/40001/cc/trees/property_tree_builder.cc File cc/trees/property_tree_builder.cc (right): https://codereview.chromium.org/1491033002/diff/40001/cc/trees/property_tree_builder.cc#newcode566 cc/trees/property_tree_builder.cc:566: data_for_children->compound_transform_since_render_target *= On 2015/12/04 00:54:54, weiliangc wrote: > On ...
1 year, 9 months ago (2015-12-04 14:38:38 UTC) #9
ajuma
https://codereview.chromium.org/1491033002/diff/40001/cc/trees/property_tree_builder.cc File cc/trees/property_tree_builder.cc (right): https://codereview.chromium.org/1491033002/diff/40001/cc/trees/property_tree_builder.cc#newcode566 cc/trees/property_tree_builder.cc:566: data_for_children->compound_transform_since_render_target *= On 2015/12/04 14:38:38, ajuma wrote: > On ...
1 year, 9 months ago (2015-12-04 15:17:01 UTC) #10
enne (OOO)
lgtm > Hmm, it turns out we used to do something pessimistic before (checking if ...
1 year, 9 months ago (2015-12-04 17:39:30 UTC) #11
weiliangc
https://codereview.chromium.org/1491033002/diff/60001/cc/trees/draw_property_utils.cc File cc/trees/draw_property_utils.cc (right): https://codereview.chromium.org/1491033002/diff/60001/cc/trees/draw_property_utils.cc#newcode487 cc/trees/draw_property_utils.cc:487: } else if (effect_node->owner_id == layer->id() && On 2015/12/04 ...
1 year, 9 months ago (2015-12-04 20:28:12 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1491033002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1491033002/100001
1 year, 9 months ago (2015-12-04 21:52:12 UTC) #15
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/125244)
1 year, 9 months ago (2015-12-04 22:01:18 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1491033002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1491033002/120001
1 year, 9 months ago (2015-12-07 16:42:09 UTC) #20
commit-bot: I haz the power
Committed patchset #7 (id:120001)
1 year, 9 months ago (2015-12-07 19:26:11 UTC) #21
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/4374c2cc231a7e5b6ae65d0e0814b5dccf445200 Cr-Commit-Position: refs/heads/master@{#363544}
1 year, 9 months ago (2015-12-07 19:27:32 UTC) #23
weiliangc
A revert of this CL (patchset #7 id:120001) has been created in https://codereview.chromium.org/1505243003/ by weiliangc@chromium.org. ...
1 year, 9 months ago (2015-12-08 21:53:46 UTC) #24
weiliangc
Quick fix for the crash. Test: 1. Go to http://www.google.com/intl/en/chromecast/ 2. Click on ‘BUY NOW’ ...
1 year, 9 months ago (2015-12-09 00:50:28 UTC) #25
enne (OOO)
Fix lgtm, thanks.
1 year, 9 months ago (2015-12-09 00:53:04 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1491033002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1491033002/160001
1 year, 9 months ago (2015-12-09 01:01:48 UTC) #29
commit-bot: I haz the power
Committed patchset #9 (id:160001)
1 year, 9 months ago (2015-12-09 03:39:36 UTC) #30
commit-bot: I haz the power
1 year, 9 months ago (2015-12-09 03:40:37 UTC) #32
Message was sent while issue was closed.
Patchset 9 (id:??) landed as
https://crrev.com/c154ce2efa59dbaecb3cb038adb247426f7c6988
Cr-Commit-Position: refs/heads/master@{#363967}
Sign in to reply to this message.

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