Closed
Bug 1232563
Opened 9 years ago
Closed 9 years ago
Move restyle requests to the animation effect
Categories
(Core :: DOM: Animation, defect)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: birtles, Assigned: birtles)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
heycam
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•9 years ago
|
||
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)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8701714 -
Flags: review?(cam)
Assignee | ||
Comment 3•9 years ago
|
||
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.
Assignee | ||
Comment 4•9 years ago
|
||
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)
Assignee | ||
Comment 5•9 years ago
|
||
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 6•9 years ago
|
||
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+
Updated•9 years ago
|
Attachment #8701714 -
Flags: review?(cam) → review+
Updated•9 years ago
|
Attachment #8701715 -
Flags: review?(cam) → review+
Updated•9 years ago
|
Attachment #8701716 -
Flags: review?(cam) → review+
Comment 8•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/aeaec67959aa
https://hg.mozilla.org/mozilla-central/rev/86b2881f5527
https://hg.mozilla.org/mozilla-central/rev/b101f84e3953
https://hg.mozilla.org/mozilla-central/rev/3a75759624a0
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in
before you can comment on or make changes to this bug.
Description
•