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

Issue 594675 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature


Sign in to add a comment

Clean up clip tree logic for cc property trees

Project Member Reported by weiliangc@chromium.org, Mar 14 2016

Issue description

1. fix logic where we no longer need to match CDP
2. simplify how clip_rect and visible_rect are calculated
3. move caching off clip nodes

 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 14 2016

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

commit 0b41eafc7e1265b86c8be6f559e1d33a31b3621f
Author: weiliangc <weiliangc@chromium.org>
Date: Mon Mar 14 21:35:56 2016

cc: Correctly inherit clip parent's clip in target space

When a clip is created by a render surface, it uses only parent clip.
When this render surface doesn't apply clip, this clip node should
inherit parent clip's |clip_in_target_space| adjusted to current target
space.

R=ajuma
BUG= 594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1802753002

Cr-Commit-Position: refs/heads/master@{#381080}

[modify] https://crrev.com/0b41eafc7e1265b86c8be6f559e1d33a31b3621f/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/0b41eafc7e1265b86c8be6f559e1d33a31b3621f/cc/trees/layer_tree_host_common_unittest.cc

Blocking: 481592
Blocking: 597734
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 11 2016

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

commit 189c1a15ccd8cb50b9c1c21f786120b1e8ad1077
Author: weiliangc <weiliangc@chromium.org>
Date: Mon Apr 11 16:16:25 2016

cc: Move RenderTarget Information to Effect Tree

Move render target related information to effect tree and clean up
render target logic.

This CL's major change includes:
1. Effect node's target_id is updated every frame
2. Effect node's target_id always points to closest ancestor that has a
    render surface, and never points to itself.
3. LayerImpl's render target returns the RenderSurfaceImpl that the
    layer contributes to. It is possible the LayerImpl owns that
    RenderSurfaceImpl.
4. RenderSurfaceImpl's render target returns the RenderSurfaceImpl that
    the render surface contributes to.

Resulting from this CL, effect tree can be walked upwards using
target_id, and render target information can be queried from effect
tree.

R=ajuma, enne
BUG=504464,  594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Review URL: https://codereview.chromium.org/1868003002

Cr-Commit-Position: refs/heads/master@{#386399}

[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/draw_properties.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/draw_properties.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/layer_impl.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/layer_impl.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/nine_patch_layer_impl_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/render_surface_impl.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/render_surface_impl.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/render_surface_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/solid_color_layer_impl_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/layers/ui_resource_layer_impl_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/damage_tracker_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/layer_tree_host_unittest_occlusion.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/occlusion_tracker.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/occlusion_tracker.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/property_tree.cc
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/property_tree.h
[modify] https://crrev.com/189c1a15ccd8cb50b9c1c21f786120b1e8ad1077/cc/trees/property_tree_builder.cc

Project Member

Comment 5 by bugdroid1@chromium.org, May 12 2016

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

commit 365a6fe2125dd821db7df698778fc05f3099bcff
Author: weiliangc <weiliangc@chromium.org>
Date: Thu May 12 19:32:31 2016

cc: Directly use property trees to calculate clip rect

Clip tree at this point uses top down recursion with caching to
calculate clips. This matches the behavior old CDP. Now that CDP is
gone, clip rect can be calculated directly from property trees without
recursion.

This CL is only turned on for cc_unittests.

This CL is a proof of concept. There is no caching so this CL would
not be as performance. This CL is behind a flag, and only switched
to after caching and visible rect calculations are finished.

R=enne
BUG= 594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/BUILD.gn
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/cc_tests.gyp
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/layers/layer_impl_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/layers/picture_layer_impl_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/layers/picture_layer_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/test/fake_layer_tree_host_impl.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/test/layer_test_common.cc
[add] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/test/layer_tree_settings_for_testing.cc
[add] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/test/layer_tree_settings_for_testing.h
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/test/layer_tree_test.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/tiles/tile_manager_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/draw_property_utils.h
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_host_common.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_host_common.h
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_host_common_perftest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_impl.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_impl_unittest.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_settings.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/layer_tree_settings.h
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/property_tree.cc
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/property_tree.h
[modify] https://crrev.com/365a6fe2125dd821db7df698778fc05f3099bcff/cc/trees/property_tree_builder.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 6 2016

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

commit a6dee63104bb2e8cc99bd8146dfdee0bdd3323db
Author: weiliangc <weiliangc@chromium.org>
Date: Mon Jun 06 18:39:10 2016

cc: Separate computation of clip rect and visible rect to two functions

Separate clip rect and visible rect calculation so visible rect logic
would be easier to follow.

R=ajuma
BUG= 594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/a6dee63104bb2e8cc99bd8146dfdee0bdd3323db/cc/trees/draw_property_utils.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 7 2016

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

commit d15784430af415d27d64ff504cfc8cc4c86f2f29
Author: weiliangc <weiliangc@chromium.org>
Date: Tue Jun 07 17:57:33 2016

cc: Visible Rect Calculation use RectF between transformations

Currently visible rect calculation expands to enclosing rect in target
space, and then transform to layer space, and expand to enclosing rect
in layer space again. Remove the first expansion to rect.

R=ajuma
BUG= 594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/d15784430af415d27d64ff504cfc8cc4c86f2f29/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/d15784430af415d27d64ff504cfc8cc4c86f2f29/cc/trees/layer_tree_host_common_unittest.cc

To move forward, first need to clean up current visible rect calculation:
- inside non root copy request: combine clips from layer up to copy request.
- all other cases: combine clips up to screen.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 15 2016

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

commit de7e0c3e9bf26b7c3bad40c6aa8883eb47159989
Author: weiliangc <weiliangc@chromium.org>
Date: Wed Jun 15 15:02:41 2016

cc: Calculate visible rect inside non root copy request

Modifies how visible rect calculation is done inside non root copy
request. Before the layer with the copy request would be fully visible.
Any other layers that is also inside the copy request would have some
combination of clips applied depending on tree structure. This CL
unifies how layers contributing to non root copy request has clips
applied: clips up to the copy request are applied, clips between copy
request and screen are ignored.

R=ajuma
BUG= 594675 
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

[modify] https://crrev.com/de7e0c3e9bf26b7c3bad40c6aa8883eb47159989/cc/trees/draw_property_utils.cc
[modify] https://crrev.com/de7e0c3e9bf26b7c3bad40c6aa8883eb47159989/cc/trees/layer_tree_host_common_unittest.cc
[modify] https://crrev.com/de7e0c3e9bf26b7c3bad40c6aa8883eb47159989/cc/trees/property_tree.cc
[modify] https://crrev.com/de7e0c3e9bf26b7c3bad40c6aa8883eb47159989/cc/trees/property_tree.h

What is the status of this bug?
Blockedon: 642054
Blockedon: 642578
Blockedon: 642581
Blockedon: 642584
I have separate out remaining tasks into smaller chunks:
- Calculate and verify visible rect
  - Need to identify when mismatch from unittest whether need to update test or actual edge case
- Calculate and verify is_clipped (should be straightforward using clip_rect)
- Cache intermediate values

And not blocking remove old logic but nice clean up:
- Stop creating clip node from Render Surface

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 3 2016

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

commit 1fc2fd1adf7c5af04f7d8e2fe752b0a31e0dde7c
Author: jaydasika <jaydasika@chromium.org>
Date: Thu Nov 03 23:19:38 2016

cc: Set clip rect to empty rect when layer's is_clipped is false

And also verify is_clipped calculated dynamically matches the current
is_clipped value, also verify clip rects even when is_clipped is false

BUG= 594675 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[modify] https://crrev.com/1fc2fd1adf7c5af04f7d8e2fe752b0a31e0dde7c/cc/trees/draw_property_utils.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Dec 9 2016

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

commit 1a837b8028867b9ef99248fe67acdd6aa66a4701
Author: jaydasika <jaydasika@chromium.org>
Date: Fri Dec 09 03:16:29 2016

cc : Seperate adding clip node and setting render surface is_clipped

Computing surface_is_clipped on effect node depends on creating a clip
node for render surface. This CL puts the code that computes
surface_is_clipped into its own method in such a way that it can be
called even from the path where we don't create clip nodes for render
surfaces

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

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

[modify] https://crrev.com/1a837b8028867b9ef99248fe67acdd6aa66a4701/cc/trees/property_tree_builder.cc

Cc: -jaydasika@chromium.org weiliangc@chromium.org
Owner: jaydasika@chromium.org
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 27 2017

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

commit a9661a4d68e451c3d351e4a0a50a830b7ab7803c
Author: jaydasika <jaydasika@chromium.org>
Date: Fri Jan 27 05:07:16 2017

cc : Remove usage of combined_clip_in_target_space during hit testing.

This is equivalent to the current logic because, if a point is clipped
by combined_clip stored on clip node, it should be clipped by the local
clip of some ancestor clip node. This CL is needed to delete
combined_clip from clip nodes.

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

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

[modify] https://crrev.com/a9661a4d68e451c3d351e4a0a50a830b7ab7803c/cc/trees/layer_tree_impl.cc

Blocking: -581844
Blockedon: 702010
Status: Fixed (was: Started)

Sign in to add a comment