Closed Bug 1232563 Opened 9 years ago Closed 9 years ago

Move restyle requests to the animation effect

Categories

(Core :: DOM: Animation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: birtles, Assigned: birtles)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

This covers the second part of bug 1230040: * Move request restyle down to the effect * Store previous progress and don't request restyle if it hasn't changed * Post a layer update whenever an Animation is newly finished
Blocks: 1232577
When requesting restyles we take special care to detect when an animation has newly finished so we perform the necessary restyle to represent the fill state. However, we should really explicitly pull the animation off the layer at this point by requesting a layer update. (That is, when an animation is newly-finished we should use RestyleType::Layer instead of RestyleType::Standard. Currently we just use RestyleType::Standard.) In this bug we plan to move restyle requests down the effect (since it is the *effect* that is restyled). However, only the Animation has the notion of "finished" or not so we detect this particular case in the Animation and request the layer update there. We already request layer updates in the Animation for other situations such as pausing so doing *layer* updates in the Animation and regular restyles in the effect is not inconsistent. This patch also tweaks test_animations_omta.html since it was previously erroneously testing that a finished animation was still running on the compositor.
Attachment #8701712 - Flags: review?(cam)
Attachment #8701714 - Flags: review?(cam)
Blocks: 1235002
Comment on attachment 8701714 [details] [diff] [review] part 2 - Move RequestRestyle calls to the effect This patch actually introduces a fairly significant change. In the case of the effect having no properties or not being "in effect", rather than simply requesting a throttled restyle, it actually causes us to not request a restyle at all. This is actually not correct in all cases and so the next two patches in the series correct this. As a result, this next patch is not strictly atomic but needs to be landed together with parts 3 and 4. I normally try to avoid this but in this case I think it's probably ok since there are only 4 parts to this bug.
This is because rather than simply requesting a throttled restyle when there were no properties, as of the previous patch, we no longer request a restyle at all in this case. We should be able to restore this optimization in bug 1235002 when we properly encapsulate the properties of a keyframe effect.
Attachment #8701715 - Flags: review?(cam)
Now that restyle requests are handled by the effect, we can more easily detect cases where we don't need to trigger a style update by looking for when the output of the effect could actually differ. Currently, any changes that require updates where the progress does *not* change (e.g. pausing) are triggered by the Animation. The exception is when we update timing properties (e.g. animation-iteration-count) from CSS but current nsAnimationManager takes care to adjust the animation generation in this case.
Attachment #8701716 - Flags: review?(cam)
Comment on attachment 8701712 [details] [diff] [review] part 1 - Request a layer update if an animation is newly finished Review of attachment 8701712 [details] [diff] [review]: ----------------------------------------------------------------- s/down/down to/ in the commit message.
Attachment #8701712 - Flags: review?(cam) → review+
Attachment #8701714 - Flags: review?(cam) → review+
Attachment #8701715 - Flags: review?(cam) → review+
Attachment #8701716 - Flags: review?(cam) → review+
Blocks: 1237453
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: