1479173 - Auto-generate an nsCSSPropertyID array for properties that can run on the compositor and use it in applicable places

Status: RESOLVED FIXED

(Core :: DOM: Animation, enhancement, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

I think this is the first step to make more properties run on the compositor.

There are some places that we iterate properties in LayerAnimationInfo and use the count of LayerAnimationInfo, those places can be rewritten with the new nsCSSPropertyIDSet.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7e50c2c232da3d6175e4d45031e74818ca34f38

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Add a static constexpr function returns an nsCSSPropertyIDSet being consist of CSS properties set can be run on the compositor. r?heycam!,birtles!

Also add a script to generate the CSS properties set by looking at
CanAnimateOnCompositor flag in servo's property definitions.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Add an assertion checking the properties in nsCSSPropertyIDSet::CompositorAnimatable equals to the properies in LayerAnimationInfo::sRecords. r?birtles!

Depends on D10687

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Use nsCSSPropertyIDSet::CompositorAnimatableCount() for LayerAnimationInfo::kRecords. r?birtles!

Depends on D10688

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Rename LayerAnimationInto::mLayerType to LayerAnimationInfo::mDisplayitemType. r?birtles!

In the case of WebRender there is no layers, but actually we'd been using it for
WebRender too, that's confusing.

Depends on D10689

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Replace LayerAnimationInfo::kRecords with nsCSSPropertyIDSet::CompositorAnimatableCount. r?birtles!

Depends on D10690

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Use nsCSSPropertyIDSet::CompositorAnimatable and HasCompositorAnimatableProperty in EffectCompositor::UpdateCascadeResults. r?birtles!

Depends on D10691

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Check mIsRunningOnCompositor flag before iterating LayerAnimationInfo. r?birtles!

The comment there was wrong.  We just bail out from there only if
mIsRunningCompositor is false, so it doesn't matter whatever the layer
generation check results.  (i.e., we don't bail out in the case where
mIsRunningCompositor is true).

Also, we iterate over mProperties in the LayerAnimationInfo::sRecords loop
through HasEffectiveAnimationOfProperty, so it doesn't matter that we iterate
mProperties before the loop either.  We will avoid the iteration in the sRecords
loop in a subsequent patch in this series.

Depends on D10692

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Call EffectSet::GetEffectSet in CanThrottle just once. r?birtles!

Depends on D10693

Comment 10

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1479173 - Check animation generation change in the mProperties loop and drop LayerAnimationInfo::sRecords loop. r?birtles!

If mIsRunningOnCompositor is true, the property is effective state because
CanThrottle() is called in advance of a restyle for the effect so that we can
drop the check and drop skipping in the case of non-effective properties.

Depends on D10694

Comment 11

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Sorry, it might take me another day or two to get to this -- I'm off sick today and possibly tomorrow.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

No worries!  I think I found an underlying issue on compositor runnable properties while I was debugging patches for bug 1504065.  It actually doesn't affect patches for this bug, but I have to write a test case to confirm that.

Comment 13

[Hiroyuki Ikezoe (:hiro)]

A final try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3e4deb2b4d8bda198557b7743f376ad712cd35f

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cf0ad1af067
Add a static constexpr function returns an nsCSSPropertyIDSet being consist of CSS properties set can be run on the compositor. r=heycam,birtles
https://hg.mozilla.org/integration/autoland/rev/94f0ae94a02c
Add an assertion checking the properties in nsCSSPropertyIDSet::CompositorAnimatable equals to the properies in LayerAnimationInfo::sRecords. r=birtles
https://hg.mozilla.org/integration/autoland/rev/43c6a7863536
Use nsCSSPropertyIDSet::CompositorAnimatableCount() for LayerAnimationInfo::kRecords. r=birtles
https://hg.mozilla.org/integration/autoland/rev/efcbbb9daa39
Rename LayerAnimationInto::mLayerType to LayerAnimationInfo::mDisplayitemType. r=birtles
https://hg.mozilla.org/integration/autoland/rev/c330e7e1eb1d
Replace LayerAnimationInfo::kRecords with nsCSSPropertyIDSet::CompositorAnimatableCount. r=birtles
https://hg.mozilla.org/integration/autoland/rev/ed1c344ccf0d
Use nsCSSPropertyIDSet::CompositorAnimatable and HasCompositorAnimatableProperty in EffectCompositor::UpdateCascadeResults. r=birtles
https://hg.mozilla.org/integration/autoland/rev/89dbc6f7f959
Check mIsRunningOnCompositor flag before iterating LayerAnimationInfo. r=birtles
https://hg.mozilla.org/integration/autoland/rev/73aba16a223f
Call EffectSet::GetEffectSet in CanThrottle just once. r=birtles
https://hg.mozilla.org/integration/autoland/rev/439ac5cfbced
Check animation generation change in the mProperties loop and drop LayerAnimationInfo::sRecords loop. r=birtles

Comment 15

[Eliza Balazs [:ebalazs_]]

Backed out 9 changesets (Bug 1479173) for build failures in src/dom/animation/EffectCompositor.cpp CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=439ac5cfbcedb23c1587c703af3e66cda06ed5cf&selectedJob=210006328

Failure log:
https://treeherder.mozilla.org/logviewer.html#?job_id=210006328&repo=autoland&lineNumber=19793
https://treeherder.mozilla.org/logviewer.html#?job_id=210006322&repo=autoland&lineNumber=20555

Backout push: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=944be4a5056f5a54e606d9cda71064c087157304

Comment 16

[Hiroyuki Ikezoe (:hiro)]

It seems MSVC doesn't like 'auto list = COMPOSITOR_ANIMATABLE_PROPERTY_LIST;' in CompositorAnimatableCount(). :/

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ead2368dc078
Add a static constexpr function returns an nsCSSPropertyIDSet being consist of CSS properties set can be run on the compositor. r=heycam,birtles
https://hg.mozilla.org/integration/autoland/rev/426beb45b7d4
Add an assertion checking the properties in nsCSSPropertyIDSet::CompositorAnimatable equals to the properies in LayerAnimationInfo::sRecords. r=birtles
https://hg.mozilla.org/integration/autoland/rev/3e986c035887
Use nsCSSPropertyIDSet::CompositorAnimatableCount() for LayerAnimationInfo::kRecords. r=birtles
https://hg.mozilla.org/integration/autoland/rev/b2857fb4156f
Rename LayerAnimationInto::mLayerType to LayerAnimationInfo::mDisplayitemType. r=birtles
https://hg.mozilla.org/integration/autoland/rev/eb673eb63998
Replace LayerAnimationInfo::kRecords with nsCSSPropertyIDSet::CompositorAnimatableCount. r=birtles
https://hg.mozilla.org/integration/autoland/rev/42bd72076aca
Use nsCSSPropertyIDSet::CompositorAnimatable and HasCompositorAnimatableProperty in EffectCompositor::UpdateCascadeResults. r=birtles
https://hg.mozilla.org/integration/autoland/rev/389edd54ab0f
Check mIsRunningOnCompositor flag before iterating LayerAnimationInfo. r=birtles
https://hg.mozilla.org/integration/autoland/rev/a3dfabd0a69a
Call EffectSet::GetEffectSet in CanThrottle just once. r=birtles
https://hg.mozilla.org/integration/autoland/rev/774f5ab895af
Check animation generation change in the mProperties loop and drop LayerAnimationInfo::sRecords loop. r=birtles

Comment 18

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/ead2368dc078
https://hg.mozilla.org/mozilla-central/rev/426beb45b7d4
https://hg.mozilla.org/mozilla-central/rev/3e986c035887
https://hg.mozilla.org/mozilla-central/rev/b2857fb4156f
https://hg.mozilla.org/mozilla-central/rev/eb673eb63998
https://hg.mozilla.org/mozilla-central/rev/42bd72076aca
https://hg.mozilla.org/mozilla-central/rev/389edd54ab0f
https://hg.mozilla.org/mozilla-central/rev/a3dfabd0a69a
https://hg.mozilla.org/mozilla-central/rev/774f5ab895af

Comment 19

[Boris Chiou [:boris]]

\o/