Support animation on the compositor for individual transform, including animation test cases
Categories
(Core :: DOM: Animation, enhancement, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox68 | --- | fixed |
People
(Reporter: u459114, Assigned: boris)
References
(Blocks 2 open bugs)
Details
Attachments
(9 files, 4 obsolete files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
No description provided.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 3•6 years ago
|
||
Moving this to Core::Animation. This should depend on bug 1479173.
Assignee | ||
Updated•6 years ago
|
Comment 4•6 years ago
|
||
I did innocently use nsCSSPropertyIDSet::CompositorAnimatableCount() for LayerAnimationInfo::sDisplayItemTypes, but it should be the number of |nsCSSPropertyIDSet::CompositorAnimatableCount() - the number of individual transform properties|.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 5•6 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #4) > I did innocently use nsCSSPropertyIDSet::CompositorAnimatableCount() for > LayerAnimationInfo::sDisplayItemTypes, but it should be the number of > |nsCSSPropertyIDSet::CompositorAnimatableCount() - the number of individual > transform properties|. Oh, yes. Because all of them use DisplayItemType::Transform. Thanks for this reminder. :)
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Boris Chiou [:boris] (UTC-7) from comment #5) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #4) > > I did innocently use nsCSSPropertyIDSet::CompositorAnimatableCount() for > > LayerAnimationInfo::sDisplayItemTypes, but it should be the number of > > |nsCSSPropertyIDSet::CompositorAnimatableCount() - the number of individual > > transform properties|. > > Oh, yes. Because all of them use DisplayItemType::Transform. Thanks for this > reminder. :) And this also applies to motion-path.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Comment 7•6 years ago
•
|
||
It seems we "hardcode" `eCSSProperty_transform` in a lot of functions, so even though I add new properties in the compositor animation list, we cannot set AnimationProperty::mRunningOnCompositor=true for the properties which also use nsDisplayTransfrom. :( We have to make nsDisplayTransform more general, for all the transformation properties (i.e. transform function list, individual transforms, and motion-path).
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 8•5 years ago
|
||
The major implementation. FIXME: Add comments.
Assignee | ||
Comment 9•5 years ago
|
||
LayerAnimationInfo knows the transform-like properties we support on
the compositor thread, so we should use it in HasTransformAnimation().
Depends on D19634
Assignee | ||
Comment 10•5 years ago
|
||
These functions set frame bits or update change hint for layer
animations on transform-like properties, so we should also take
other transform-like properties into account.
Updated•5 years ago
|
Assignee | ||
Comment 11•5 years ago
|
||
Sometimes, we want to set the performance warning by a property set, so
add a new function for it.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 12•5 years ago
|
||
It seems this function uses some FFIs to generate the AnimationValue,
We could move it into AnimationValue class, although what we really need is
RefPtr<ServoAnimationValue>.
Maybe we should use AnimationValue everywhere, instead of the Servo type.
This could be done by other patches.
Assignee | ||
Comment 13•5 years ago
|
||
Both layers and web-render use this function, so we factor it out.
Depends on D22562
Assignee | ||
Comment 14•5 years ago
|
||
I separate two parts while implementing compositor animations for
individual transforms. The first part is on the sender side of transactions.
Basically, we have to convert the individual transforms to the proper
types in layers::Animations. So this patch is focus on SetAnimatable and
the definition in LayersMessages.
Depends on D22563
Assignee | ||
Comment 15•5 years ago
|
||
This is the 2nd part of the implementation. We focus on the compositor side
(i.e. received side of transactions). Basically, we convert the list of
layers::Animation into a list of SampleAnimationData
, which is an
intermediate value. And then use this list to do interpolation for each
property in SampleAnimationForEachNode
, which will return a list of
RefPtr<RawServoAnimationValue>
.
Depends on D22564
Updated•5 years ago
|
Assignee | ||
Comment 16•5 years ago
|
||
Now, its time to let individual transform run on the compositor thread.
Depends on D19636
Assignee | ||
Comment 17•5 years ago
|
||
Drop the hack which prevents individual transform running on the
compositor thread.
Depends on D22566
Assignee | ||
Comment 18•5 years ago
|
||
This also adds the missing preference in test_transition_per_property.html.
Depends on D22567
Updated•5 years ago
|
Updated•5 years ago
|
Comment 19•5 years ago
|
||
Comment on attachment 9046822 [details]
Bug 1425837 - Part 9: Set performance warning by a property set.
Revision D19633 was moved to bug 1533594. Setting attachment 9046822 [details] to obsolete.
Assignee | ||
Comment 20•5 years ago
|
||
This is the pre-patch for the next patch (i.e. Part 3-3).
The original implementation about "setting animations" is a little bit hard
to read. In SetAnimations()
, we create a new intermediate data,
AnimData
, and we mutate the original animations. And then iterate this
mutated animations & intermediate data for sampling. In this bug, we are
planning to group the AnimData as a useful data structure for supporting
multiple properties transform-like animations, so it seems the structure
of original animations may be hard to use after that. Therefore,
we decide to do some reworks on this:
First, we do renames,
- InfalliableTArray to nsTArray. (They are the same.)
- AnimData to PropertyAnimation.
- SetAnimations() to ExtractAnimations(), which returns
nsTArray<PropertyAnimationGroup>. Each entry in the array is for one
property. In this patch, there is only one entry. We will extend this
to multiple entries in the next patch.
And then rework ExtractAnimations()
, which stores all the necessary data
in PropertyAnimationGroup
. For WR, we store this in
CompositorAnimationStorage
. For non-WR, we store it in AnimationInfo
.
So we can just use this organized data structure for supporting multiple
properties animations. (See the next patch.)
Depends on D22564
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 21•5 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c3a636faee85571c0c96599b96b297bfb7f8b35a
Will land these patches after the soft freeze.
Thanks for your reviews, Brian and Hiro. :)
Assignee | ||
Comment 22•5 years ago
|
||
Comment 23•5 years ago
|
||
Pushed by bchiou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/07a1ab95598e Part 1: Move ToAnimationValue into AnimationValue. r=hiro https://hg.mozilla.org/integration/autoland/rev/5d416b07c9b9 Part 2: Factor out the conversion from ServoAnimationValue into Matrix4x4. r=hiro https://hg.mozilla.org/integration/autoland/rev/6c55b82deb9b Part 3: Don't store AnimationArray (a.k.a. AnimationInfo::mAnimations) on the compositor thread. r=birtles https://hg.mozilla.org/integration/autoland/rev/b27d18cccc62 Part 4: Implement compositor animations on translate/rotate/scale. r=hiro,birtles https://hg.mozilla.org/integration/autoland/rev/067a9dac440a Part 5: AnimationInfo::HasTransformAnimation should check other OMTA transform-like properties r=hiro https://hg.mozilla.org/integration/autoland/rev/066e7dffe044 Part 6: Add individual transform properties into compositor animation list. r=hiro https://hg.mozilla.org/integration/autoland/rev/af39dc228d19 Part 7: Drop the hard-code disabling compositor animations on individual transforms. r=hiro,birtles https://hg.mozilla.org/integration/autoland/rev/9fe743aa6c94 Part 8: Test that individual transforms run on the compositor thread. r=hiro,birtles https://hg.mozilla.org/integration/autoland/rev/08126c1f53fe Part 9: Throttle transform-like properties animations without 0% or 100% keyframe. r=hiro
Comment 24•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/07a1ab95598e
https://hg.mozilla.org/mozilla-central/rev/5d416b07c9b9
https://hg.mozilla.org/mozilla-central/rev/6c55b82deb9b
https://hg.mozilla.org/mozilla-central/rev/b27d18cccc62
https://hg.mozilla.org/mozilla-central/rev/067a9dac440a
https://hg.mozilla.org/mozilla-central/rev/066e7dffe044
https://hg.mozilla.org/mozilla-central/rev/af39dc228d19
https://hg.mozilla.org/mozilla-central/rev/9fe743aa6c94
https://hg.mozilla.org/mozilla-central/rev/08126c1f53fe
Comment 25•5 years ago
|
||
Pushed by boris.chiou@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/487ede41ade2 Drop the redundant animation debug log. r=me
Comment 26•5 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Description
•