New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 718149 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature



Sign in to add a comment

Remove cc::LayerImpl::GetRenderSurface

Project Member Reported by chrishtr@chromium.org, May 3 2017

Issue description

This appears to be the last blocker to removing dummy layers for
effect nodes in blink::PaintArtifactCompositor

There are 5 non-test callsites:

Getting the root layer's render surface. These can be replaced by the render surface of the root effect node.

https://cs.chromium.org/chromium/src/cc/trees/layer_tree_host_common.cc?l=585
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?l=265
https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?l=1063

Getting the render target of a clip node. These can go through the clip
node instead.

https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?l=1849

Getting the screen space transform:

https://cs.chromium.org/chromium/src/cc/trees/layer_tree_impl.cc?l=1804

 
And the last callsite is called from the second-to last.
Project Member

Comment 3 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/51b96b368bc1ebb4b952465a4220f093f6850528

commit 51b96b368bc1ebb4b952465a4220f093f6850528
Author: chrishtr <chrishtr@chromium.org>
Date: Wed May 03 23:06:06 2017

Delete code in PointIsClippedByAncestorClipNode which checked render surface clip.

Since all actual semantic clips are represented by the clip tree, this code
was only necessary to check for cases when a surface exceeded the maximum
texture size. Since the rendering would already be broken in such cases,
it seems ok to not count that as a clip, which is correct w.r.t. the semantics
of the web page, even if it disagrees with the apparent rendering.

Note: the relevant code was added in https://codereview.chromium.org/1551333003.

BUG= 718149 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2858053003
Cr-Commit-Position: refs/heads/master@{#469183}

[modify] https://crrev.com/51b96b368bc1ebb4b952465a4220f093f6850528/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/51b96b368bc1ebb4b952465a4220f093f6850528/cc/trees/layer_tree_impl_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, May 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e3aaf256e6d56290701b3d4b7d982139d8441da

commit 7e3aaf256e6d56290701b3d4b7d982139d8441da
Author: chrishtr <chrishtr@chromium.org>
Date: Thu May 04 15:04:29 2017

Delete LayerImpl::GetRenderSurface.

(and its callsites) The only remaining callsites are in tests.

BUG= 718149 , 557160 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2857343002
Cr-Commit-Position: refs/heads/master@{#469335}

[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/effect_tree_layer_list_iterator_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/layer_impl.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/layer_impl.h
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/layer_position_constraint_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/render_surface_impl_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/layers/render_surface_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/test/layer_test_common.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/test/layer_test_common.h
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_unittest_damage.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_unittest_occlusion.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_host_unittest_video.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/7e3aaf256e6d56290701b3d4b7d982139d8441da/cc/trees/occlusion_tracker_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment