cc: Derive draw opacity from property trees

cc: Derive draw opacity from property trees

This adds an opacity tree, computes draw opacity
using the property tree, and verifies that the computed
draw opacity matches what's computed by CDP.

BUG=386810
Committed: https://crrev.com/fc856e96e24438a2ea0fba2473e156b091a1fe3c
Cr-Commit-Position: refs/heads/master@{#318093}

[ajuma, 2015-02-24 21:21:08]

ajuma@chromium.org changed reviewers:
+ vollick@chromium.org

[Ian Vollick, 2015-02-24 21:25:37]

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
Does this capture impl-only opacity animations? These seem to be used for PZ
scrollbars.

[ajuma, 2015-02-24 21:30:38]

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
On 2015/02/24 21:25:36, vollick wrote:
> Does this capture impl-only opacity animations? These seem to be used for PZ
> scrollbars.

Afaik, PZ scrollbars no longer use impl-only animations, despite their layers
overriding OpacityCanAnimateOnImplThread.

The only current use of impl-only animations that I know of is for input-driven
smooth scrolling (and that does't involve opacity).

[danakj, 2015-02-24 21:34:48]

On Tue, Feb 24, 2015 at 1:30 PM, <ajuma@chromium.org> wrote:

>
> https://codereview.chromium.org/953093002/diff/1/cc/trees/
> property_tree_builder.cc
> File cc/trees/property_tree_builder.cc (right):
>
> https://codereview.chromium.org/953093002/diff/1/cc/trees/
> property_tree_builder.cc#newcode235
> cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
> On 2015/02/24 21:25:36, vollick wrote:
>
>> Does this capture impl-only opacity animations? These seem to be used
>>
> for PZ
>
>> scrollbars.
>>
>
> Afaik, PZ scrollbars no longer use impl-only animations, despite their
> layers overriding OpacityCanAnimateOnImplThread.
>

Should we make them not do that then?


>
> The only current use of impl-only animations that I know of is for
> input-driven smooth scrolling (and that does't involve opacity).
>
> https://codereview.chromium.org/953093002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[ajuma, 2015-02-24 21:45:57]

On 2015/02/24 21:34:48, danakj wrote:
> On Tue, Feb 24, 2015 at 1:30 PM, <mailto:ajuma@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/953093002/diff/1/cc/trees/
> > property_tree_builder.cc
> > File cc/trees/property_tree_builder.cc (right):
> >
> > https://codereview.chromium.org/953093002/diff/1/cc/trees/
> > property_tree_builder.cc#newcode235
> > cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
> > On 2015/02/24 21:25:36, vollick wrote:
> >
> >> Does this capture impl-only opacity animations? These seem to be used
> >>
> > for PZ
> >
> >> scrollbars.
> >>
> >
> > Afaik, PZ scrollbars no longer use impl-only animations, despite their
> > layers overriding OpacityCanAnimateOnImplThread.
> >
> 
> Should we make them not do that then?

Actually, I take it back. They don't use the impl-only animation machinery from
LayerAnimationController, but ScrollbarAnimationController does change opacity
on the compositor thread. So I'll update this CL to create nodes for these
layers.

[ajuma, 2015-02-24 21:53:29]

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
File cc/trees/property_tree_builder.cc (right):

https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
On 2015/02/24 21:25:36, vollick wrote:
> Does this capture impl-only opacity animations? These seem to be used for PZ
> scrollbars.

Updated to also create nodes for layers that can animate opacity on impl-only.

[Ian Vollick, 2015-02-25 00:46:53]

On 2015/02/24 21:53:29, ajuma wrote:
>
https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
> File cc/trees/property_tree_builder.cc (right):
> 
>
https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
> cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
> On 2015/02/24 21:25:36, vollick wrote:
> > Does this capture impl-only opacity animations? These seem to be used for PZ
> > scrollbars.
> 
> Updated to also create nodes for layers that can animate opacity on impl-only.

Thanks! The implementation seems super straightforward, the only concern is perf
(as we were chatting about offline). Could you please try running with my
telemetry benchmark and see if this is actually a concern?
(crrev.com/918633003).

[ajuma, 2015-02-25 17:05:25]

On 2015/02/25 00:46:53, vollick wrote:
> On 2015/02/24 21:53:29, ajuma wrote:
> >
>
https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
> > File cc/trees/property_tree_builder.cc (right):
> > 
> >
>
https://codereview.chromium.org/953093002/diff/1/cc/trees/property_tree_build...
> > cc/trees/property_tree_builder.cc:235: Animation::OPACITY);
> > On 2015/02/24 21:25:36, vollick wrote:
> > > Does this capture impl-only opacity animations? These seem to be used for
PZ
> > > scrollbars.
> > 
> > Updated to also create nodes for layers that can animate opacity on
impl-only.
> 
> Thanks! The implementation seems super straightforward, the only concern is
perf
> (as we were chatting about offline). Could you please try running with my
> telemetry benchmark and see if this is actually a concern?
> (crrev.com/918633003).

An initial run with your benchmark on the top_25 pageset showed no change in the
average cost of property trees. But there was a huge variation across individual
pages -- some got significantly worse, some were significantly better (and the
latter is clearly bogus). I'm going to try again with a larger repeat count
before drawing firm conclusions.

[ajuma, 2015-02-25 18:29:28]

I've updated this so that opacity tree construction can be skipped by passing a
null opacity tree to BuildPropertyTrees. The idea is that we can leave opacity
tree construction on for testing correctness, but turn it off for perf tests or
for shipping property trees on the main thread only (since opacity trees aren't
needed on the main thread).

[Ian Vollick, 2015-02-25 18:56:09]

On 2015/02/25 18:29:28, ajuma wrote:
> I've updated this so that opacity tree construction can be skipped by passing
a
> null opacity tree to BuildPropertyTrees. The idea is that we can leave opacity
> tree construction on for testing correctness, but turn it off for perf tests
or
> for shipping property trees on the main thread only (since opacity trees
aren't
> needed on the main thread).

Thanks, that puts my mind at ease. lgtm.

[ajuma, 2015-02-25 18:57:05]

The CQ bit was checked by ajuma@chromium.org

[commit-bot: I haz the power, 2015-02-25 18:58:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/953093002/40001

[commit-bot: I haz the power, 2015-02-25 19:54:49]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-02-25 19:55:44]

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