cc : Add node and ancestors are animated or invertible to transform tree

cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

BUG=601088
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113
Cr-Commit-Position: refs/heads/master@{#386514}

[jaydasika, 2016-04-11 18:12:28]

Description was changed from

==========
cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

BUG=601088
==========

to

==========
cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

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

[jaydasika, 2016-04-11 18:14:47]

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

[jaydasika, 2016-04-11 18:14:47]

PTAL

[ajuma, 2016-04-11 20:21:44]

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:1091: parent_node->data.ancestors_are_invertible)
Is it sufficient to just use the value of node->data.ancestors_are_invertible
here (which, despite its name, really is about the invertibility of the node as
well as its ancestors)? That is, this is true iff both the screen space
transform and draw transform are invertible. So we don't need to additionally
check node->data.is_invertible and parent_node->data.ancestors_are_invertible.

[jaydasika, 2016-04-11 20:31:28]

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:1091: parent_node->data.ancestors_are_invertible)
On 2016/04/11 20:21:44, ajuma wrote:
> Is it sufficient to just use the value of node->data.ancestors_are_invertible
> here (which, despite its name, really is about the invertibility of the node
as
> well as its ancestors)? That is, this is true iff both the screen space
> transform and draw transform are invertible. So we don't need to additionally
> check node->data.is_invertible and parent_node->data.ancestors_are_invertible.

For the case, A->B, A has animating singular transform, B's
ancestors_are_invertible will be false, but we want
node_and_ancestors_are_animated_or_invertible to be true for B

[ajuma, 2016-04-11 20:35:58]

lgtm

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:1091: parent_node->data.ancestors_are_invertible)
On 2016/04/11 20:31:28, jaydasika wrote:
> On 2016/04/11 20:21:44, ajuma wrote:
> > Is it sufficient to just use the value of
node->data.ancestors_are_invertible
> > here (which, despite its name, really is about the invertibility of the node
> as
> > well as its ancestors)? That is, this is true iff both the screen space
> > transform and draw transform are invertible. So we don't need to
additionally
> > check node->data.is_invertible and
parent_node->data.ancestors_are_invertible.
> 
> For the case, A->B, A has animating singular transform, B's
> ancestors_are_invertible will be false, but we want
> node_and_ancestors_are_animated_or_invertible to be true for B 

Ah, good point, thanks. If we don't have a test case with this structure, please
add one.

[jaydasika, 2016-04-11 20:42:38]

One more change, PTAL

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc
File cc/trees/property_tree.cc (right):

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:1079:
DCHECK(node->data.node_and_ancestors_are_animated_or_invertible);
I just realized, this shouldn't be a DCHECK, we should be checking the node's
transform.

https://codereview.chromium.org/1876903003/diff/1/cc/trees/property_tree.cc#n...
cc/trees/property_tree.cc:1091: parent_node->data.ancestors_are_invertible)
On 2016/04/11 20:35:58, ajuma wrote:
> On 2016/04/11 20:31:28, jaydasika wrote:
> > On 2016/04/11 20:21:44, ajuma wrote:
> > > Is it sufficient to just use the value of
> node->data.ancestors_are_invertible
> > > here (which, despite its name, really is about the invertibility of the
node
> > as
> > > well as its ancestors)? That is, this is true iff both the screen space
> > > transform and draw transform are invertible. So we don't need to
> additionally
> > > check node->data.is_invertible and
> parent_node->data.ancestors_are_invertible.
> > 
> > For the case, A->B, A has animating singular transform, B's
> > ancestors_are_invertible will be false, but we want
> > node_and_ancestors_are_animated_or_invertible to be true for B 
> 
> Ah, good point, thanks. If we don't have a test case with this structure,
please
> add one.

There's a test already :
LayerTreeHostCommonTest.VisibleContentRectForAnimatedLayerWithSingularTransform

[ajuma, 2016-04-11 21:25:52]

Still lgtm

[jaydasika, 2016-04-11 21:26:48]

The CQ bit was checked by jaydasika@chromium.org

[commit-bot: I haz the power, 2016-04-11 21:27:39]

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

[commit-bot: I haz the power, 2016-04-11 23:30:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-11 23:32:09]

Description was changed from

==========
cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

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

to

==========
cc : Add node and ancestors are animated or invertible to transform tree

This is needed to decide if a layer can be skipped during visible rect
and RSLL computation.

BUG=601088
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/f187b06b55a08ca8efb94361282428d7f8532113
Cr-Commit-Position: refs/heads/master@{#386514}
==========

[commit-bot: I haz the power, 2016-04-11 23:32:11]

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