We have already resolved to make calling Finish() clear the pause state (see
https://lists.w3.org/Archives/Public/public-fx/2015AprJun/0038.html, item 2).
Doing that involves resolving the start time when the animation is paused.
Furthermore, as a separate change, we resolved to make the finished promise not
resolve when the animation is paused. That suggests making UpdateFinishedState()
only resolve the finished promise when PlayState() == Finished rather than using
IsFinished() which returns true even if the animation is paused.
However, if we compare PlayState() == Finished in UpdateFinishedState() then we
will *not* resolve the finished promise when the animation is play-pending since
PlayState() == Pending in that case (pause-pending is ok since the call to
SetCurrentTime will cause a transition to the Paused state). Furthermore, the
existing call to cancel the pending play task will effectively leave this
animation forever pending. Hence, in this patch we unconditionally fill in the
start time.
--HG--
extra : rebase_source : 499ad0530eac0ee62c8ed2df41360c45abc34816
This is much easier to read at call sites and prevents accidentally using the
default value when another value might be more suitable.
--HG--
extra : rebase_source : b1c05d8bf7b97744e53f2ecc676561f3a4a80888
The point of making pausing async is to allow time to sync up the current time
with the compositor. Setting the current time manually should simply force it to
the specified time and complete the pause action, not abort it. (We do a similar
thing for a pending play. For a pending play we're waiting to establish
a suitable start time. Manually setting the start time in that case simply
forces the start time to the specified time and completes the play operation.)
--HG--
extra : rebase_source : 614ed9ef01204e4137783c0d48e975eb8febbe2a
Animation::ResumeAt contains an assertion that, when we exit the play-pending
state, checks we either have a resolved start time or a resolved hold time.
That's normally true but if we are aborting a pause on animation that is
finished we can end up with a resolved start time (since we don't clear the
start time when we're aborting a pause) and a resolved hold time (either
because the regular finishing behavior set one, or, because play() applied
auto-rewinding behavior and set it).
In that case we should actually respect the hold time and update the start time
when resuming the animation. However, ResumeAt won't update the start time if it
is already set.
This patch fixes that by clearing the start time in DoPlay in the case where we
are aborting a pause and have a hold time.
--HG--
extra : rebase_source : 83f980d6cbc34375274f30f6527992b4fec7f639
This patch makes Cancel() call PostUpdate which clobbers certain state in style
so that animated style is correctly flushed when an animation is cancelled.
The main difficulty with this is that we *don't* want to call this when we're
cancelling an animation as a result of a style update or else we'll trigger
needless work. The pattern elsewhere has been to define a *FromStyle() method
for this case (e.g. CSSAnimation::PlayFromStyle, PauseFromStyle). This isn't
ideal because there's always the danger we will forget to call the appropriate
*FromStyle method. It is, however, consistent. Hopefully in bug 1151731 we'll
find a better way of expressing this.
This patch also updates the method names on PendingAnimationTracker but leaves
a number of local variables which will be fixed in a subsequent patch.
--HG--
rename : dom/animation/PendingPlayerTracker.cpp => dom/animation/PendingAnimationTracker.cpp
rename : dom/animation/PendingPlayerTracker.h => dom/animation/PendingAnimationTracker.h
This patch is a fairly minimal rename of the AnimationPlayer interface. It
leaves a bunch of local variables and helper classes still using the word
"player". These will be addressed in subsequent patches that don't require DOM
peer review.
--HG--
rename : dom/animation/AnimationPlayer.cpp => dom/animation/Animation.cpp
rename : dom/animation/AnimationPlayer.h => dom/animation/Animation.h
rename : dom/webidl/AnimationPlayer.webidl => dom/webidl/Animation.webidl
We define KeyframeEffectReadonly in KeyframeEffect.cpp since Web Animations also
defines KeyframeEffect and when we come to implement that I expect we'll define
it in the same class, maybe even using the same object.
This patch also adds a few missing includes in places where
KeyframeEffectReadonly is used so that we're not just cargo-culting it in.
--HG--
rename : dom/animation/Animation.cpp => dom/animation/KeyframeEffect.cpp
rename : dom/animation/Animation.h => dom/animation/KeyframeEffect.h
rename : dom/animation/test/css-animations/test_animation-name.html => dom/animation/test/css-animations/test_effect-name.html
rename : dom/animation/test/css-animations/test_animation-target.html => dom/animation/test/css-animations/test_effect-target.html
rename : dom/animation/test/css-transitions/test_animation-name.html => dom/animation/test/css-transitions/test_effect-name.html
rename : dom/animation/test/css-transitions/test_animation-target.html => dom/animation/test/css-transitions/test_effect-target.html
rename : dom/webidl/Animation.webidl => dom/webidl/KeyframeEffect.webidl
Most of this is fairly obvious. However, the addition of 'override' to
ElementPropertyTransition::Name() is not strictly necessary. It was simply added
because while making these changes I accidentally dropped the 'virtual' keyword
from the method in the superclass and the compiler didn't complain. Adding this
will hopefully make it harder to create the same bug in the future.
--HG--
rename : dom/animation/test/css-animations/test_animation-effect-name.html => dom/animation/test/css-animations/test_animation-name.html
rename : dom/animation/test/css-transitions/test_animation-effect-name.html => dom/animation/test/css-transitions/test_animation-name.html
This patch (after stepping through the call graph) affects the following
places:
* CommonAnimationManager::GetAnimationsForCompositor, which is used
only by nsDisplayListBuilder::AddAnimationsAndTransitionsToLayer,
which already checks the individual animations (so really no change)
* AnimationPlayerCollection::CanThrottleAnimation
* ActiveLayerTracker::IsStyleAnimated
* nsLayoutUtils::HasAnimationsForCompositor
* nsLayoutUtils::HasAnimations (which is used only to check whether we
can make the 0-opacity optimization)
I believe it makes sense to change all of these locations (although in
the long term we want to throttle (or similar) more animations).
Without this patch, I believe we're forcing the creation of an opacity
layer because we think we have animations to send to it.
This patch adds a method for testing if an animation is "in play" which is
a term defined in the Web Animations spec. This is in preparation for removing
some slightly redundant code in IsRunning and aligning better with the spec.
In preparation for introducing IsInPlay (where "in play" is a term in the Web
Animations spec), this patch aligns the existing IsCurrent with the definition
in the spec that says an animation effect is only current if it is attached
to an animation (player in our current naming) that is not finished. In order
to ensure that we need to pass the animation/player into the method.
This actually changes the behavior of IsCurrent since now we will return false
for animations that are finished. As far as I can tell, all the call sites that
are requesting current animations should only be concerned with animations that
are actually running. If not, they need to be adjusted to look for animations
that are either current or in effect.
This patch extracts the logic for calculating animation styles from
AnimationPlayerCollection and puts the bulk of it into the Animation objects.
Some of the initial logic surrounding the animation player state (e.g. is it
paused or not, etc.) is put into AnimationPlayer.
In future we may shift this logic even further down to the AnimationEffect
objects but currently we don't create such objects unless necessary.
This patch adds a utility method to return if an animation is "in effect" or not
as defined by Web Animations:
http://w3c.github.io/web-animations/#in-effect
It also moves the utility method for querying if an animation is "current"
(IsCurrent) to the .cpp file since it is fairly long. (Bug 1046055 makes one of
the callers of IsCurrent inline-able which should offset any cost introduced by
this no longer being inline-able.)
This patch takes the StickyTimeDuration defined in the previous patch and
uses it within the calculation of animation timing for parameters that are
expected to be +/- Forever.
As the second step in dividing functionality between AnimationPlayer and
Animation, this patch moves the AnimationTiming member from AnimationPlayer to
Animation.
Most of this patch is simply moving code around. However, one significant
functional difference is that Animation::GetLocalTime() uses the mParentTime
member which is set when the Animation is updated by the player it is attached
to.
Other less significant differences are:
* AnimationPlayer::GetLocalTime is renamed to GetCurrentTimeDuration
In Web Animations, animation players have a (writeable) "current time" and
animations have a (read-only) "local time".
We would call the method simply "GetCurrentTime" (instead of
"GetCurrentTimeDuration") but GetCurrentTime is the name of the method used in
the content-facing API where it returns a double.
* "IsCurrent" is defined on both AnimationPlayer and Animation with the version
in AnimationPlayer serving mostly as a convenience shortcut to the version on
Animation.
* Animation::GetComputedTiming (previously on AnimationPlayer) now makes the
timing parameter optional since most of the time it is not needed.
As the first step in dividing the functionality currently contained in
AnimationPlayer between AnimationPlayer and Animation this patch moves the set
of keyframe properties to the Animation.
These properties are returned from the Animation by a couple of Properties()
methods that provide direct access to the member variable. In future it is
anticipated that the non-const version will be replaced with an appropriate
setter function. This will likely happen when we implement a separate
KeyframeEffect object as defined by the Web Animations API.
With regards to error checking, nsAnimationManager checks the result of
AnimationPlayer::GetSource() and handles the case where it is nullptr.
nsTransitionManager, however, simply asserts that GetSource() is never null much
like it also asserts that there is only one property with one segment in the
animation. Eventually this code should be made more generic which will probably
happen in bug 999927.
This patch makes AnimationPlayers pass their current time down to the Animation
they are playing.
Since all Animations need from their players is their time, this avoids adding
a pointer back to their AnimationPlayer.
Brian Birtles
Andrew McCreight
Carsten "Tomcat" Book
Jonathan Watt
Markus Stange
L. David Baron
Boris Zbarsky
Nicholas Nethercote
Andrew McCreight
David Zbarsky