cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5f7a7b8deec6cadbc43c56773e804031d7b4be17
Cr-Commit-Position: refs/heads/master@{#385271}

[jaydasika, 2016-04-04 22:51:58]

Description was changed from

==========
cc : Unify layer skipping logic in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon

BUG=
==========

to

==========
cc : Unify layer skipping logic in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-04 22:57:03]

jaydasika@chromium.org changed reviewers:
+ ajuma@chromium.org, enne@chromium.org, vollick@chromium.org,
weiliangc@chromium.org

[jaydasika, 2016-04-04 22:57:03]

PTAL

[ajuma, 2016-04-05 15:16:46]

https://codereview.chromium.org/1855373003/diff/20001/cc/tiles/tile_manager_u...
File cc/tiles/tile_manager_unittest.cc (right):

https://codereview.chromium.org/1855373003/diff/20001/cc/tiles/tile_manager_u...
cc/tiles/tile_manager_unittest.cc:1030:
pending_child_layer->SetForceRenderSurface(true);
Why do we need to force a render surface here? Is it just so that this layer
gets an effect node?

https://codereview.chromium.org/1855373003/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1855373003/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:540:
draw_property_utils::LayerShouldBeSkipped(
Hmm. This is still effectively skipping a subtree, since if we early-out here we
don't recurse on children below.

And the code below that assigns render targets assumes that a layer's parent has
a valid render target (which won't necessarily be true if we're skipping
individual layers instead of subtrees).

So I think this logic needs a bit more re-working (+ tests!).

[jaydasika, 2016-04-05 17:24:22]

https://codereview.chromium.org/1855373003/diff/20001/cc/tiles/tile_manager_u...
File cc/tiles/tile_manager_unittest.cc (right):

https://codereview.chromium.org/1855373003/diff/20001/cc/tiles/tile_manager_u...
cc/tiles/tile_manager_unittest.cc:1030:
pending_child_layer->SetForceRenderSurface(true);
On 2016/04/05 15:16:46, ajuma wrote:
> Why do we need to force a render surface here? Is it just so that this layer
> gets an effect node?

Yes.

https://codereview.chromium.org/1855373003/diff/20001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common.cc (right):

https://codereview.chromium.org/1855373003/diff/20001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common.cc:540:
draw_property_utils::LayerShouldBeSkipped(
On 2016/04/05 15:16:46, ajuma wrote:
> Hmm. This is still effectively skipping a subtree, since if we early-out here
we
> don't recurse on children below.
> 
> And the code below that assigns render targets assumes that a layer's parent
has
> a valid render target (which won't necessarily be true if we're skipping
> individual layers instead of subtrees).
> 
> So I think this logic needs a bit more re-working (+ tests!).

Agreed. But in this CL, I am just trying to call the draw_property_utils
SubtreeShouldBeSkippped instead of the one here, so the return is still there.
What you said needs to be done when we remove the return and recurse into the
subtree which I think can be done in a different CL ?

[jaydasika, 2016-04-05 18:53:15]

Description was changed from

==========
cc : Unify layer skipping logic in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon

BUG=
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-05 18:54:00]

Added a test and updated description.

[jaydasika, 2016-04-05 19:03:27]

Description was changed from

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in DrawPropertyUtils. This Cl
also clears render targets 
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-05 19:04:24]

Description was changed from

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in DrawPropertyUtils. This Cl
also clears render targets 
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ajuma, 2016-04-05 19:05:36]

Thanks, lgtm.

https://codereview.chromium.org/1855373003/diff/60001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/1855373003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:515: // should e reset.
typo: 'be'

[jaydasika, 2016-04-05 19:05:52]

Description was changed from

==========
cc : Use same layer skipping functions  in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[jaydasika, 2016-04-05 19:19:03]

https://codereview.chromium.org/1855373003/diff/60001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_common_unittest.cc (right):

https://codereview.chromium.org/1855373003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_common_unittest.cc:515: // should e reset.
On 2016/04/05 19:05:35, ajuma wrote:
> typo: 'be'

Done.

[jaydasika, 2016-04-05 19:19:23]

The CQ bit was checked by jaydasika@chromium.org

[jaydasika, 2016-04-05 19:19:24]

The patchset sent to the CQ was uploaded after l-g-t-m from ajuma@chromium.org
Link to the patchset: https://codereview.chromium.org/1855373003/#ps80001
(title: "Rebase + typo in comment")

[commit-bot: I haz the power, 2016-04-05 19:19:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1855373003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1855373003/80001

[commit-bot: I haz the power, 2016-04-05 20:24:42]

Description was changed from

==========
cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-04-05 20:24:43]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-04-05 20:26:08]

Description was changed from

==========
cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
cc : Use same layer skipping functions in DrawPropertyUtils and LTHC

LTHC = LayerTreeHostCommon
LTHC still skips subtree but it uses functions in
DrawPropertyUtils. This CL also clears render targets
to account for any mismatches in what is skipped in LTHC
and DrawPropertyUtils.

BUG=581939
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/5f7a7b8deec6cadbc43c56773e804031d7b4be17
Cr-Commit-Position: refs/heads/master@{#385271}
==========

[commit-bot: I haz the power, 2016-04-05 20:26:09]

Patchset 5 (id:??) landed as
https://crrev.com/5f7a7b8deec6cadbc43c56773e804031d7b4be17
Cr-Commit-Position: refs/heads/master@{#385271}