This patch adjusts GetComputedTimingAt to set the time fraction and current
iteration fields of the output computed timing correctly for animations with
zero iteration duration. Care must be taken to handle cases such as animations
that have zero duration but repeat infinitely.
The code is significantly re-arranged to more closely align with the naming and
algorithms defined in Web Animations.
A couple of tests in test_animations.html have been tweaked to account for
floating-point error. This is not because the new code is less precise but
actually the opposite. These tests fall on the transition point of step-timing
functions. The new code uses the closest possible floating-point representation
of these times which happens to cause them to fall on the opposite side of the
transition point.
For example, in evaluating a point 3s into a reversed interval the old code
would give us an intermediate time fraction of:
0.29999999999999982
When we reverse that by subtracting from 1.0 we get: 0.70000000000000018
With the code in this patch we get an intermediate time fraction of:
0.29999999999999999
When we reverse that by subtracting from 1.0 we get: 0.69999999999999996
Hence we fall on the opposite side of the transition boundary.
This patch also makes ElementAnimation::ActiveDuration a static method that
takes timing parameters as an argument. This is so that this method can be
used within ElementAnimations::GetComputedTimingAt (a static method) in a
future patch.
We could also make ActiveDuration() a method of AnimationTiming. I suspect
this logic belongs together in ElementAnimation however.
In a future patch we could also add the active duration to the ComputedTiming
struct which would simplify the only other place this is currently used
which is ElementAnimations::GetEventsAt.
viewport-units-rounding-1.html fails without the patch and passes with
the patch.
viewport-units-rounding-2.html fails with an early version of the patch
but not without the patch or with the final version.
Also shuffle the initialization of members in
nsAnimationManager::BuildAnimations to roughly match the order in which they
are declared (with the exception that mPlayState needs to be set before calling
IsPaused() which is used to set mPauseStart).
This was only needed when we were inspecting the returned time fraction but now
that we inspect the phase it's not necessary to force the fill mode to "both".
This patch simply moves the code from ElementAnimations to ElementAnimation so
that it can later be used in transitions code and so we can later move
EnsureStyleRuleFor to ElementAnimation.
This patch shuffles the code in ElementAnimations::GetEventsAt to make it easier
to follow.
It also removes a check for whether or not the animation is paused.
Previously we would not dispatch events if the animation was paused and in its
active phase (but we would if the animation had finished). There doesn't seem to
be any reason for this. If the animation was paused between the last sample and
the current sample and the boundary of an iteration also occurred in that time
then I expect we should dispatch that event. Removing this check for the pause
state does not cause any tests fail.
Separating out the event logic here makes it clear that we do not dispatch start
events in the situation where one sample falls before the active interval and
one sample falls after it (filed as bug 1004361). This patch adds a comment to
this effect.
This patch simply shifts the event-related code from GetPositionInIteration to
GetEventsAt. Although there are simplifications that could be done to
GetEventsAt, they are deferred to a subsequent patch so as not to obscure the
translation of code from one function to another.
As a result of moving event-related handling from GetPositionInIteration it no
longer needs to support different main-thread vs compositor modes.
This patch makes ElementAnimations::GetPositionInIteration return
a ComputedTiming object instead of just a time portion (time fraction).
Since the ComputedTiming object includes phase information, we can fix those
parts of EnsureStyleRule and GetEventsAt that were temporarily using the time
portion to guess if the animation might have finished or not.
This patch moves the FillsForwards/FillsBackwards methods previously defined on
ElementAnimations to the structure contain the fill mode: AnimationTiming. It
also changes GetPositionInIteration to use these methods.
Introduces a struct to store timing parameters for passing to
GetPositionInIteration. In future this struct is expected to be expanded to
include other timing parameters as well (based roughly on Web Animations'
"Timing" interface, hence the name AnimationTiming).
This patch moves event queuing out of EnsureStyleRuleFor into a separate method.
This is a preparatory step towards making GetPositionInIteration into a more
generic method for calculating the current time fraction.
In order to achieve this, GetPositionInIteration needs to be able to calculate
the correct time portion for times outside the range [0, 1] even when it is not
passed a ElementAnimation object. Specifically, it needs the fill mode of the
animation to be passed in.
(Rather than using FillForwards/FillBackwards this patch just compares the
NS_STYLE_ANIMATION_FILL_MODE_* values directly but FillForwards/FillBackwards
are restored in a subsequent patch when they are added to the struct used to
lump the timing parameters together.)
There are a number of places where positionInIteration is used to determine if
the current sample occurs in the active phase or after. This is sub-optimal but
is fixed in a subsequent patch in this series.
The actual work of removing event queuing from GetPositionInIteration is
deferred to a subsequent patch in order to keep the changes as small as
possible. This patch simply makes separate calls to GetPositionInIteration for
interpolating and for event queuing.
This patch adds tests for triggering animations based on the length of the
animation-name property as well as tests for dynamic changes to style rules.
These tests are based on tests in test_animations.html but for directed at
animations that run on the compositor thread.
This patch adds tests for the cascanding of keyframes rules based on those in
test_animations.html but for animations that run on the compositor thread.
This patch adds tests for the interaction of animation and restyling (Bug
686656) based on those in test_animations.html but for animations that run on
the compositor thread.
The implementation here current expects BOTH the following to fail:
- The comparison between the OMTA value and the expected value
- The comparison between the OMTA value and the computed value
This generally tends to be the case since the computed value and expected value
normally match unless we have a bug that affects all CSS animations. If we need
to mark tests where the computed value is also wrong we'll need to modify the
behavior here at that time.
This patch also applies this new function to the author !important test that was
previously commented-out because it currently fails.
This patch also ensures that when we have an animation running on the compositor
when it should not that we still compare the values produced on the compositor
and on the main thread so that the visual result is correct even if the
performance characteristics are not.
This patch adds tests for the handling of author !important rules and animations
based on similar tests in test_animations.html but for animations that run on
the compositor thread.
Due to bug 847287, these tests don't pass and are partly disabled. Subsequent
patches add todo_is tests for this.
This patch adds a version of tests in test_animations.html for bug 651456 which
covers multiple samples with the same timestamp. The version here, however,
tests animations that run on the compositor thread.
This patch adds a version of tests in test_animations.html for keyframe
animations with multiple properties where some properties are present in only
some keyframes. The version here, however, tests animations that run on the
compositor thread.
This patch adds a version of tests in test_animations.html under the heading,
"css3-animation: 3.8 The 'animation-delay' Property", for animations
that run on the compositor thread.
This patch adds a version of tests in test_animations.html under the heading,
"css3-animation: 3.7 The 'animation-play-state' Property", for animations
that run on the compositor thread.
This patch adds a version of tests in test_animations.html under the heading,
"css3-animation: 3.6 The 'animation-direction' Property, for animations
that run on the compositor thread.
This patch addresses and issue where the OMTA style and computed style were not
comparing equal in one particular case.
In this case AddTransformTranslate in nsStyleAnimation would give us
a translate-y value of 94.331673 in both cases (i.e. when calculating the
animated value on the compositor thread or when fetching computed style).
For the OMTA case, however, after we apply additional layer transformations and
then reverse them (so we can query the CSS value) we'd end up with 94.331642,
a difference of 0.000031. The reversing procedure is only used for testing so
the actual error introduced here by the additional layer transformations is
probably less.
Unfortunately, when we pass 94.331642 this along to MatrixToCSSValue we get back
matrix(1, 0, 0, 1, 94.3316) since it only outputs 6 digits of precision.
On the other hand, on the computed style end we'd pass 94.331673 to
MatrixToCSSValue which gives us matrix(1, 0, 0, 1, 94.3317), so the error swells
from 0.000031 to 0.0001.
Then when we subtract 94.3316 from 94.3317 in Javascript we get
0.00010000000000331966 due to floating-point precision issues which compares
greater than the default tolerance of 0.0001.
This patch simply adjusts the default tolerance to 0.00011 to accommodate
these floating-point differences.
This patch adds a version of tests in test_animations.html under the heading,
"css3-animation: 3.5 The 'animation-iteration-count' Property" for animations
that run on the compositor thread.
These tests surface an issue where in some cases precision errors lead to
discrepencies between the OMTA style and computed style. This is fixed in
a subsequent patch in this series.
This patch adds a version of the tests in test_animations.html under the
heading, "css3-animations: 3.2. The 'animation-name' Property" that tests the
same features when animations are running on the compositor thread.
The test harness code for normalizing transform inputs to a standard form for
comparison fails to detect the case where the input is a string such as
{ tx: "20px" }
instead of:
{ tx: 20 }
When we go to compare matrix components we fail if:
Math.abs(a.comp - b.comp) > tolerance
But if a.comp or b.comp is a string, we'll get NaN on the LHS and
"NaN > tolerance" will return false so we'll skip the failure handling and
continue onto the next component. That means if we have input { tx: "30px" } and
we get "20" as the x-translation component we'll pass the test.
This patch fixes this condition to check for isNaN.
We *could* also just drop a few .map(parseFloat) calls into
convertObjectTo3dMatrix and convertArrayTo3dMatrix to ensure "20px" becomes 20
but there may be situations where that masks bugs (since "20px" and "20em" turn
into the same thing) so for now this minimal fix should be enough.
This patch converts the tests in test_animations.html under the heading,
"css3-animations: 3.1. Timing functions for keyframes" to an equivalent version
for testing animations that run on the compositor thread.
These functions get invoked as event listeners, so we'll automatically get the
proper |this|. The reason for the existing shenanigans was to work around
bug 872772, which has now been fixed.
As a result, transitions are now stored using a pointer to the base class,
mozilla::ElementAnimation. We downcast to a transition only when necessary. No
error-checking of the result of AsTransition is performed since we only ever
call it on the mAnimations member of ElementTransitions.
Add a method for downcasting from an ElementAnimation to an
ElementPropertyTransition (when the underlying object is an
ElementPropertyTransition).
This, unfortunately, adds a vtable to ElementAnimation but in the long term
I hope we will be able to isolate transition-specific code to a specific kind of
TransitionEffect that hangs off ElementAnimation and put the vtable on
AnimationEffect instead. (The AnimationEffect concept is part of the Web
Animations API.)
We currently have mozilla::StyleAnimation as well as nsStyleAnimation. This
patch renames StyleAnimation back to ElementAnimation.
Although ElementAnimation is very similar to ElementAnimations, in the near
future we expect to retire ElementAnimations and replace it with a common
AnimationSet-like structure that is covers the features of ElementAnimations and
ElementTransitions.
This patch takes StyleAnimation and makes it ref-counted heap object. This
should allow us to store StyleAnimation and its subclasses (transitions only
currently) in a consistent fashion (an array of base-class pointers).
Furthermore, this will be helpful if we want these things to be pointed to
from Javascript objects that may, for example, preserve their lifetime beyond
that of the element that currently owns them.
This patch also introduces a typedef for an array of refptrs to StyleAnimation
objects (and similarly for the subclass ElementPropertyTransition) to simplify
the code somewhat.
- OverflowChangedTracker::AddFrame now accepts an enumerated type parameter to
indicate if the overflow areas of children have changed (CHILDREN_CHANGED),
the overflow areas of the children have changed and the parent have changed
(CHILDREN_AND_PARENT_CHANGED), or if only the transform has changed
(TRANSFORM_CHANGED).
- OverflowChangedTracker::Flush no longer falls back to calling
nsIFrame::UpdateOverflow when a frame lacks a PreTransformOverflowAreas
property.
- Added an additional change hint, nsChangeHint_ChildrenOnlyTransform, which
results in TRANSFORM_CHANGED being passed in to
OverflowChangedTracker::AddFrame.
- In nsIFrame::FinishAndStoreOverflow, the passed in overflow is now stored as
the InitialTransformProperty for elements that are IsTransformed().
- Partially corrected Bug 926155, by only calling
OverflowChangedTracker::AddFrame on parents of the sticky element during
StickyScrollContainer::UpdatePositions, using CHILDREN_CHANGED.
This patch was mostly generated with the following command:
find . -name "*.h" -o -name "*.cpp" | xargs sed -e '/WrapObject(JSContext/ {; N; s/\(WrapObject(JSContext *\* *a\{0,1\}[Cc]x\),\n\{0,1\} *JS::Handle<JSObject\*> a\{0,1\}[sS]cope/\1/ ; }' -i ""
and then reverting the changes that made to
dom/bindings/BindingUtils.h, since those WrapObject methods are not
the ones we're trying to change here, plus a bunch of manual fixups
for cases that this command did not catch (including all the callsites
of WrapObject()).
This patch was mostly generated with this command:
find . -name "*.h" -o -name "*.cpp" | xargs sed -e 's/Binding::Wrap(aCx, aScope, this/Binding::Wrap(aCx, this/' -e 's/Binding_workers::Wrap(aCx, aScope, this/Binding_workers::Wrap(aCx, this/' -e 's/Binding::Wrap(cx, scope, this/Binding::Wrap(cx, this/' -i ""
plus a few manual fixes to dom/bindings/Codegen.py, js/xpconnect/src/event_impl_gen.py, and a few C++ files that were not caught in the search-and-replace above.
- OverflowChangedTracker::AddFrame now accepts an enumerated type parameter to
indicate if the overflow areas of children have changed (CHILDREN_CHANGED) or
if the transform has changed (TRANSFORM_CHANGED).
- OverflowChangedTracker::Flush no longer falls back to calling
nsIFrame::UpdateOverflow when a frame lacks a PreTransformOverflowAreas
property.
- Added an additional change hint, nsChangeHint_ChildrenOnlyTransform, which
results in TRANSFORM_CHANGED being passed in to
OverflowChangedTracker::AddFrame.
- In nsIFrame::FinishAndStoreOverflow, the passed in overflow is now stored as
the InitialTransformProperty for elements that are IsTransformed().
- Partially corrected Bug 926155, by only calling
OverflowChangedTracker::AddFrame on parents of the sticky element during
StickyScrollContainer::UpdatePositions, using CHILDREN_CHANGED.
Tests for animation list handling on the compositor thread. Some checks are
currently marked todo_is because they depend on Bug 980769 in order to pass.
This patch takes the compareTransform utility methods and makes them more
generic so they can be used for testing opacity too (the other property
currently animated on the compositor thread).
The naming omta_is and omta_is_approx is intended to mirror is and is_approx in
test_animations.html.
This patch adds an additional mochitest for specifically targetting CSS
Animations that run on the compositor thread. The content of the test mimicks
test_animations.html but using properties whose animations are expected to run
on the compositor thread.
Since off-main thread animation (OMTA) is not available on all platforms we
define a common wrapper function that runs OMTA tests only when available. This
patch further performs an internal check of basic OMTA operation so that
only a single error is produced if OMTA is unexpectedly unavailable.
Typical usage is:
SimpleTest.waitForExplicitFinish();
runOMTATest(function() {
... test code ...
SimpleTest.finish();
},
SimpleTest.finish);
This can be easily wrapped with promises if needed but does not require using
promises.
The calls to waitForExplicitFinish and finish are not performed automatically
since this function may be integrated with test suites that do other work
outside the call to runOMTATest.
Two comments in AnimationCommon.h refer to 'mFlushCount' which was presumably
the old name for mAnimationGeneration. Also, one comment says
nsCSSFrameConstructor tracks this. This patch adjusts the comments to refer
to mAnimationGeneration and RestyleManager.
(The reference to nsTransitionManager::UpdateAllThrottleStyles() is still valid
since there is useful documentation accompanying that method despite the fact
that the relevant code is mostly contained in AnimationCommon.h since bug
914847. Eventually we will unify the structures of transitions and
animations to the the point that we can replace the
IMPL_UPDATE_ALL_THROTTLED_STYLES_INTERNAL macro in AnimationCommon.h with an
actual method. At that point we can move the documentation accompanying
nsTransitionManager::UpdateAllThrottleStyles and its references to
AnimationCommon.)
We need a basic representation of animations from which we can derive subclasses
to represent specific cases such as transitions. For now we will retrofit
ElementAnimation for that purpose hence renaming it to StyleAnimation.
This patch removes the "using namespace mozilla::layers" line from
AnimationCommon.cpp since the unified build system concatenates several files
together before compiling making using declarations like this leak into other
files potentially creating ambiguities. Previously, when we were calling
ElementAnimation, 'Animation', there were ambiguities between
mozilla::layers::Animation and this new 'Animation' class. In general, it is
probably a good idea to limit the scope of these using declarations so I've kept
that change.
This patch relocates ElementAnimation from nsAnimationManager.{h,cpp} to
AnimationCommon.{h,cpp} and in the process moves it into the mozilla::css
namespace.
ElementAnimation::HasAnimationOfProperty doesn't seem to be overridden anywhere.
I suspect it was a copy-paste mistake because the methods of the same name on
ElementAnimations, ElementTransitions, and CommonElementAnimationData are
virtual.
Now that ElementTransitionProperty inherits from ElementAnimation,
ElementTransitions::HasAnimationOfProperty can re-use
ElementAnimation::HasAnimationOfProperty in its definition of
ElementTransitions::HasAnimationOfProperty.
Similarly, in nsDisplayList::AddAnimationsAndTransitionsToLayer we can use this
method rather than drilling down to the appropriate segment by hand.
Both ElementPropertyTransition and ElementAnimation specify an IsRunningAt
method which have the same purpose but with two subtle differences:
a) ElementPropertyTransition::IsRunningAt checks if the transition is a removed
sentinel and if so returns false. This patch adds a check for a null start time
to IsRunningAt since I think in future we will want to allow null times in
various places to represent, for example, animations that are not connected to
a timeline. (However, ultimately we will probably not allow start times on
*animations* to be null, only on their associated player.)
Should we later use a different mechanism for marking sentinel transitions (e.g.
a boolean flag) this method should still be correct as it checks if aTime is
inside the transition interval before returning true.
b) ElementPropertyTransition::IsRunningAt returns false if the transition is in
the delay phase, that is, waiting to start. This patch changes this behavior so
that transitions are considered running even if they are in the delay phase.
This brings their behavior into line with animations and removes the need for
the ElementPropertyTransition::mIsRunningOnCompositor since it is only used to
determine when a transition in the delay phase has begun.
ElementAnimation::IsRunningAt also handles pause state and iterations but this
logic should still be correct for transitions which, in this area, only use
a subset of the functionality of animations since their pause state is always
playing and their iteration count is 1.
As part of moving towards more shared data structures for animation, this patch
makes ElementPropertyTransition inherit from ElementAnimation. At the same time
we switch from storing the target property, start/end values, start time, delay,
and timing function on the transition to the corresponding location in
ElementAnimation.
Since nsDisplayList::AddAnimationsAndTransitionsToLayer was already doing this
conversion in order to create animations to pass to the compositor thread, we
can remove the conversion code from there and just use the ElementAnimation data
structures as-is.
A number of assertions are added to verify that transitions are set up as
expected (namely, they have only a single property-animation with a single
segment). As we move to more generic handling of animations and transitions
these assertions should disappear.
As a first step towards making CSS animations and CSS transitions use the same
data structures, this patch aligns their behavior with regards to start time and
delay handling.
Previously, ElementAnimation objects maintained separate mStartTime and mDelay
members whilst ElementPropertyTransition objects maintained a single mStartTime
property that incorporated the delay. This patch adds an mDelay member to
ElementPropertyTransition and stores the delay and start time separately.
Calculations involving ElementPropertyTransition::mStartTime are adjusted to
incorporate mDelay.
I confirmed that the crashtest crashes in the harness without the patch.
--HG--
rename : layout/reftests/backgrounds/blue-32x32.png => layout/style/crashtests/blue-32x32.png
This adds a check that is currently present in most but not all
codepaths leading to this point, but which patch 4 will remove from many
of those codepaths.
Had patch 3 not been present, this would be needed to prevent a test
failure with patch 4 (which removes the ExpectEndProperty check from the
CSS_PROPERTY_PARSE_VALUE case in
CSSParserImpl::ParseProperty(nsCSSProperty) since its callers handle
checking for appropriate endings), since the way this function returns
success for empty values leads var() functions alone inside
font-variant-alternates to be rejected, since
CSSParserImpl::ParseProperty(nsCSSProperty) won't try reparsing with
variables.
Values that are already in invalidGradientAndElementValues don't also
need to be listed in an array that has invalidGradientAndElementValues
appended to it.
* Refuse to serialize some combinations of values that the shorthand
can not represent: 'grid-template-areas: (not none)' combined with
'grid-template-rows: subgrid' or 'grid-template-columns: subgrid'.
(The former used to cause an assertion failure.)
* Remove an extraneous trailing space that occured when a <track-list>
was last. (ie. followed by an omitted <line-names>)
* Add tests for the result of this serialization.
This patch adds an extra check for paint suppression when waiting for paint
events. This is because on some platforms (notably B2G) we can think all paints
have completed because paint suppression is in effect and as a result call the
callback too soon.
This patch use window.setTimeout(..., 0) to wait for paint suppression to finish
before preceding to check for pending paint events.
When the refresh driver is under test control, if we detect that paint events
are pending we need to force a refresh driver tick. This patch adds that tick.
I suppose we had previously never hit this situation before and never noticed
this.
This patch also rearranges the main loop so that early returns appear first and
calling the callback appears at the end.
... and a bit of grid-auto-flow, for the grid shorthand.
* Avoid using CheckEndProperty() when possible
* Distinguish between "error" and "not a <track-size>"
* Split out parsing of a <track-list>, given its initial [<line-names>?]
When we have a backwards fill and we sample at *exactly* the start of the
animation on the next refresh driver tick, when we get to
RestyleManager::ComputeStyleChangeFor (or more specifically
ElementRestyler::CaptureChange) we notice that the style hasn't changed (since
the first frame of the animation produces the same value as the backwards fill)
and end up with an empty change list. As a result we never schedule a view
manager flush and rebuild the layer. Hence, the animation never gets sent to the
compositor thread. On the next tick we're already throttling the main thread.
This patch fixes this by applying the same approach as is used for transitions,
that is, explicitly marking which animations are running on the compositor
thread so we know if we need to trigger a layer transaction or not. This should
not only be more robust than the previous code but also facilitate aligning
animations and transitions code (bug 880596).
This test reproduces a bug where we don't send an animation to the compositor
thread. The important step is the sample precisely at the start of the animation
interval.
Adds a test for transform animations with a delay where there is already
a transform specified on the element.
This test used to fail but bug 828173 fixed it. It is included here as
a regression test.
(Prior to bug 828173 we were able to fix this by triggering
ForceLayerRerendering inside nsAnimationManager::nsFlushAnimations whenever we
detected *different* style rules but canThrottleTick=true)
The issue stemmed from the fact that when an element has a transform property,
LayerIsPrerenderedDataKey would get set on the layer because in
nsDisplayTransform::ShouldPrerenderTransformedContent we would only check for
the *presence* of an animation, not whether it is running. Then in
RestyleManager::DoApplyRenderingChangeToTree we wouldn't do an invalidating
paint because TryUpdateTransformOnly() returns true since it looks at
LayerIsPrerenderedDataKey.
Bug 828173 fixes this, at least in part, by checking if an animation is running.
This bug may resurface if we put animations with a delay on the compositor
thread hence it is worth including here.
Animations with a delay are not put on the compositor thread until the end of
the delay phase. However, there is currently nothing that explicitly triggers
this transaction. It may occur due to flushes that arise from UI events but it
is not guaranteed.
This patch detects the end of a delay phase and turns off throttling for that
sample. It re-uses the mLastNotification member which is not ideal but
a subsequent patch in this queue removes this and replaces it with the approach
used for transitions.
In the parsing code for grid-template-areas:
* Move some of the processing into the helper function,
so that the grid-template will not have to duplicate it.
* Store the number of parsed columns (and rows),
as they will affect the size of the "explicit grid".
* Use a hash table instead of a linear scan to avoid O(n²)-ish complexity.
This changes the behavior of the CanPerformOnCompositorThread methods of
both ElementAnimations and ElementTransitions to check that the
respective animations or transitions are actually running. This is ok
because:
- The main caller is nsLayoutUtils::HasAnimationsForCompositor, and all
of its callers pretty clearly want the more restricted behavior (they're
concerned with layer activity)
- The only other callers of these functions are
nsAnimationManager::FlushAnimations and
nsTransitionManager::FlushTransitions (determining when to do
throttling), nsAnimationManager::GetAnimationsForCompositor (whose
only caller,
nsDisplayListBuilder::AddAnimationsAndTransitionsToLayer, also checks
IsRunningAt). I think these also all want or are fine with having
the IsRunningAt check.
As to the actual changes:
- In the animation manager, I think it's a mistake that
ElementAnimation::IsRunningAt didn't already check
mIterationDuration, since we throw out animations with a bad
iteration-duration in ElementAnimations::EnsureStyleRuleFor. So this
makes that change as well.
- In the transition manager, IsRunningAt already checks
!IsRemovedSentinel().
I've confirmed in gdb on a device that this fixes the repeated
nsIFrame::SchedulePaint calls that were the symptom of this bug.
I believe this patch also makes it so that a short animation of a
property that can't be animated on the compositor doesn't prevent the
entire duration of the animation of a property that can from being
throttled (having the main thread style updates suppressed).
I've been wanting to remove this code for a while. I think this code is
problematic for three reasons:
(1) It's in the middle of code where it doesn't belong, and which ought
to be handling purely-style-system things. (This is blocking me
from reusing that code elsewhere, e.g., in bug 977991 and
bug 960465, both of which could use it in some form.)
(2) It defeats the optimization from bug 790505 whenever we do a
miniflush (in other words, whenever we have any style change,
whether or not it's related)
(3) It means the conditions for when we decide to ship a new set of
animation data to a layer doesn't cover all the cases the layer
needs it. In particular, we only run this miniflush code when we
have a currently running animation or transition that's running on
the compositor thread. On the other hand, the UpdateTransformLayer
style change handling in DoApplyRenderingChangeToTree depends on
whether the frame currently has a transform layer, which can
continue to be true for a bit after the animation stops. So if we
need to send animations to the layer because of a transform style
change that happens soon after an animation completes, our style
change handling will find the existing layer and call its
SetBaseTransformForNextTransaction method but never do anything
that triggers layer construction. The style throttling code, in
turn, will never stop doing main thread updates because the
animation generation on the layer is out-of-date, and these main
thread updates will keep the layer active, but they'll never show
up because the stale animation data overrides the new transform
that we've been setting. (At least, I think that's what was
happening; it makes sense to me and matches the behavior I was
observing. I didn't verify which main thread updates and which
layer updates were actually happening, though.)
This shows up, for example, in the animation in attachment 8384813
just halting at a corner if I'm careful not to disturb it. (I'm
testing on Linux, with both accelerated layers and OMT animations
explicitly enabled.)
I think there are probably some other things that can be removed as
followups to removing this code, because I think we made some boundary
conditions intentionally incorrect so that problem (3) above wouldn't be
as bad as it otherwise would have been.
The first provides a reasonable default (failure to parse) for the case
of an unknown value type (which should never happen).
The second reorders two failure checks so that the one that returns
early when units is uninitialized happens before the one that looks at
units.
In this crash test we restore the refresh driver after manually advancing it.
This causes a situation where a layer has an animation that has yet to start.
Prior to modifying ElementAnimations::GetPositionIteration this test case would
trip an assertion there that rejected negative elapsed times when called from
the compositor thread.
Rename NS_STYLE_CLEAR_LEFT_AND_RIGHT to NS_STYLE_CLEAR_BOTH. Also fix whitespace and remove unnecessary parens in a few places and enumerate the possible break types in an assertion so that it doesn't make assumptions on the actual property values.
