1300472 - Drop cascade level for animation rule's refresh time

Status: RESOLVED INVALID

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

Description

[Hiroyuki Ikezoe (:hiro)]

EffectSet has mAnimationRuleRefreshTime for transition and animation respectively
but in current implementation we update all of animations (i.e. CSS transitions,
CSS animations and script animations) on the transform layer for an element
whenever we update the transform layer.  Even if there are a CSS transtion
and a CSS animation on an element, the refresh times for the transition and
the animation will be the same value on the first unthrottled frame.  So we
don't need to magage the refresh time respectively.

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

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1300472 - Drop cascade level for animation rule's refresh time.

In current implementation we update all of animations (i.e. CSS transitions,
CSS animations and script animations) on the transform layer for an element
whenever we update the transform layer.  Even if there are a CSS transtion
and a CSS animation on an element, the refresh times for the transition and
the animation will be the same value on the first unthrottled frame.  So we
don't need to magage the refresh time respectively.

Review commit: https://reviewboard.mozilla.org/r/76666/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/76666/

Comment 2

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

I'm not so sure about this. It seems risky. If we later add a code path which only updates the transitions rule, for example, we'll update the refresh time and fail to update the animations rule when we later attempt to do so.

What's the advantage of doing this?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8788080 [details]
Bug 1300472 - Drop cascade level for animation rule's refresh time.

(In reply to Brian Birtles (:birtles) from comment #2)
> I'm not so sure about this. It seems risky. If we later add a code path
> which only updates the transitions rule, for example, we'll update the
> refresh time and fail to update the animations rule when we later attempt to
> do so.

I think such case won't happen, it shouldn't be or it isn't a problem.  The case we are talking is that an element has both of transition and animation.  In other cases a single refresh time is sufficient anyway.

There are two cases I can think of.

a) a transition and an animation(replace)
b) a transition and an animation(additive)

In case of b), we should update both of rules, so it's not the case.
In case of a), we only update the animation rule. (we actually don't need to update the transition rule)
Yes, there need a timing case in a).


a1) a 2s transition and a 1s animation(replace) and each start time is slightly different.

Yes, in this case, an unthrottled time after the animation is finished will be shifted to the amount of the start time difference (actually it will be min(the difference, 200ms)).  But is it really a problem?

> What's the advantage of doing this?

I am planning to remove cascade level from EffectSet to implement additive animations.

That being said,  I am clearing review request for now.  I will reconsider how to implement additive animations in more detail.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

I don't think this is necessary now.