999922 - Share more code between ElementAnimations / ElementTransitions / CommonElementAnimationData

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

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

I have a rough plan for this below. The basic idea is to end up with a single AnimationSet class that tracks a list of animations specific to an element and has methods for managing them, querying them, styling them etc.

Possible steps:

Phase 1: Store animations on base class

* Rename ElementPropertyTransition to StyleTransition (this is just a cosmetic thing that makes sense to me)
* Make StyleAnimation ref-counted and store as an array of nsRefPtrs
  Apart from allowing us to make the storage of these things common (next step) it will be helpful when we want to point to these things from Javascript objects.
* Add StyleAnimation::AsTransition virtual method
* Store animations in CommonElementAnimationData as an array of StyleAnimation nsRefPtrs.
  nsTransitionManager::EnsureStyleRuleFor etc. will use AsTransition to downcast but hopefully further down we can eventually limit downcasting.

Phase 2: Remove ElementAnimations::GetPositionInIteration

* Create a Timing struct to store timing parameters: iteration duration, start delay, fill mode, iteration count and use this in StyleAnimation
* Create static ComputedTiming StyleAnimation::GetComputedTimingFor(Timestamp, const &Timing) based on ElementAnimations::PositionInIteration but without the event handling. It would be good to try and line up with the Web Animations naming and procedures in this implementation.
  Member include:
     mTimeFraction (=timePortion)
     mCurrentIteration
     mPhase (enum: before|active|after|null)
  Also IsNull() which returns true if mPhase is null. Default ctor initializes to this.
* Make ElementAnimations::EnsureStyleRuleFor use GetTimeFraction etc. and do event handling in FlushAnimations like the transition manager does
  That would mean we don't dispatch events when calling CheckAnimationRule or UpdateThrottledStyle. Is that a problem?
  -- At the point in ElementAnimations::EnsureStyleRuleFor where we check if the animation is newly finished by looking at mLastNotification we do this by storing the last phase and comparing last phase vs new phase.
* Make ElementTransitions::ValuePortionFor use StyleAnimation::GetComputedTimingFor (probably will need to override fill mode to 'both' to get 1 capping or else just replace null with 1)
* Make Compositor thread call StyleAnimation::GetComputedTimingFor (make it store timing in a Timing struct?)
* Remove ElementAnimations::GetPositionInIteration

Phase 3: Phase out ElementTransitions

* Move event dispatch to FlushAnimations
  -- The main issue with dispatching events as a separate step is it means we iterate over animations twice for each call to FlushAnimations. That might be ok. Alternatively, we could get EnsureStyleRule to return an abstract list of events or just store it in mPendingEvents and re-use that in transitions too.
     -- The thing is, iteration events are only needed for CSS animations (and SVG animations) so it might be good to skip them otherwise? Perhaps we could pass in an array that, if present, gets filled in, otherwise it gets skipped. We'd also only update mLastNotification on the animation if we filled in that list.
* Move EnsureStyleRule to CommonElementAnimationData
  -- We can skip sentinel values by making GetComputedTiming return a null value for something that has no time (including sentinel values) and then skipping them
* Move HasAnimationOfProperty to CommonElementAnimationData
  -- With regards to skipping sentinel values I think we should just make it skip items with no start time (and document this dependency)
    -- In future we will probably change this area considerably to, for example, return a list of "current" animations or "playing" animations since in a lot of the call sites that's what we're really interested in. So we'd actually have HasAnimationOfPropertyAt and pass in a time. For now, I think the dependency is ok
* Move CanPerformOnCompositorThread to common base class
   We should make notifying the active layer tracker happen at the call sites. Specifically, probably nsAnimationManager::FlushAnimations and nsTransitionManager::FlushTransitions is enough.
* Rename CommonElementAnimationData to AnimationSet
* Remove ElementTransitions (it should be empty by now)

Phase 4: Phase out ElementAnimations

* Move ElementAnimations::IsForElement and PseudoElement to AnimationSet
* Move PostRestyleForAnimation to base class -- not sure how to stop calling this for transitions yet. Add a flag to AnimatorSet, mRestylesAnimation?
* Remove ElementAnimations and just use AnimationSet
* Fix nsDisplayList to remove redundant processing of animations and transitions

Comment 1

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

There are a lot of steps in comment 0 so I'm splitting off separate bugs to cover each phase. In particular I want to land phases 1 and 2 soon so I can more easily fix bug 1004361, bug 1004365, and bug 1004377.  That should also help with phases 3 and 4.

Comment 2

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

As part of this work I'd also like to rework ElementAnimation::GetComputedTimingAt to incorporate the calculation of the elapsed duration. This would mean:

* AnimationTiming includes *all* input timing parameters (not just a random subset)
* Call sites to GetComputedTimingAt are simplified
* Less redundant code (currently AsyncCompositionManager.cpp does its own GetElapsedDurationAt calculation)
* Easier to align with Web Animations algorithms and naming

So perhaps this can be Phase 5.

Comment 3

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

All dependent bugs have been resolved, so resolving this too.