Closed Bug 1230040 Opened 9 years ago Closed 9 years ago

Use EffectCompositor to do style rule generation

Categories

(Core :: DOM: Animation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox45 --- affected
firefox46 --- fixed

People

(Reporter: birtles, Assigned: birtles)

References

Details

(Keywords: meta)

Attachments

(1 file)

We need to make our style updating include *all* animations not just those in AnimationCollections. This bug tracks that. This is possibly the last part of bug 1190235 but it's massive so I might split off more parts yet.
Attached patch WIP patch v1 (deleted) — Splinter Review
WIP patch of roughly what I intend to do. This patch doesn't actually work (it compiles and runs, but animations are broken) but it outlines the structure of what I think this will end being.
I'm going to try to break this up into steps that can be verified and landed individually since this is pretty tricky code full of all sorts of dirty flags and timestamps just begging for regressions. At very least, if I can avoid too many big changes we can bisect any regressions that do pop up down to something fairly easy to understand. Goal: Add ComposeStyleRule that works off the effect set * Add AnimationLevel to EffectCompositor (use with AppliesToTransitionsLevel) * Move mStyleRule to the EffectSet * Add static ComposeStyleRule to EffectCompositor and call from EnsureStyleRule -- convert as many call sites as possible to using ComposeStyleRule directly Goal: Get rid of mStyleChanging (and fix the layer update problem at the same time) * Move request restyle down to the effect * Store previous progress and don't request restyle if it hasn't changed * Try removing mStyleChanging * Post a layer update whenever an Animation is newly finished (Rename Animation::PostUpdate?) VERIFY: We are still skipping style updates for compositor animations Goal [[BIG]]: Get rid of the style rule refresh time * Add a compositor per pres context * Define a hashset keyed on RefPtr<Element> and pseudo-type Add a hashset: mElementsToRestyle and add to it from QUESTION: Do we need to add two maps here? One for each level? * In RequestRestyle::Standard, add elements to the hashset. Try to drop mHasPendingAnimationRestyle flag at this point by using the IsEmpty status of the map instead * Instead of checking the rule refresh time, check if it is in the set or not. If it's not, it doesn't need updating. Once we've updated the rule, remove the animation from the hashset. * Move RequestRestyle to EffectCompositor * Update end of UpdateCascadeResults to call the local RequestRestyle VERIFY: We are still skipping style updates for compositor animations Goal: Drop EnsureStyleRule and just call ComposeStyleRule directly Do the checks inside ComposeStyleRule? Call it UpdateStyleRule? VERIFY: We are still skipping style updates for compositor animations Goal: Move GetAnimationRule to EffectCompositor Goal: Track throttled elements in the compositor * Add another hashset for throttled animations? (One for each level?) * Update hashset in RequestRestyle VERIFY: This updating works as expected * Implement AddThrottledStyleUpdates * Use AddThrottledStyleUpdates * Drop old AddStyleUpdates VERIFY: We only add the throttled updates Goal: Move FlushAnimations to the compositor (Need to work out what it is actually doing!) Goal: Move style rule processors to the compositor Goal: Tidy up * Move ClearIsRunningOnCompositor to EffectCompositor? * Redo ContentOrAncestorHasAnimation * Remove any other redundant methods (including empty NeedRefreshes()) * Remove linked-list from collections? * Move AnimStyleValue stuff to dom/animation? * Rename nsAnimationManager -> mozilla::CSSAnimationBuilder * Likewise nsTransitionManager -> mozilla::CSSTransitionBuilder * Separate out files
Do you have a plan to expose the status which indicates the animation is finished but not composed the last frame yet? https://bugzilla.mozilla.org/attachment.cgi?id=8695113&action=diff#a/dom/animation/Animation.cpp_sec3 If we expose the status, we can check it in AddStyleUpdatesTo to skip unnecessary styling processes in bug1219236.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #3) > Do you have a plan to expose the status which indicates the animation is > finished but not composed the last frame yet? > > https://bugzilla.mozilla.org/attachment.cgi?id=8695113&action=diff#a/dom/ > animation/Animation.cpp_sec3 > > If we expose the status, we can check it in AddStyleUpdatesTo to skip > unnecessary styling processes in bug1219236. The idea in that patch is that AddStyleUpdatesTo is replaced with AddThrottledStyleUpdatesTo. The EffectCompositor maintains a list of elements with throttled. When AddThrottledStyleUpdatesTo is called it adds only the elements in that list and then clears the list. It also clears that list whenever RequestRestyle > Standard is called on the element. That's the idea, anyway, I'm not sure if it really works. But if it does it would mean we never add unnecessary style updates in AddStyleUpdatesTo.
Depends on: 1232561
Depends on: 1232563
Depends on: 1232577
Depends on: 1235112
Keywords: meta
All dependent bugs are now fixed, marking resolved.
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.

Attachment

General

Created:
Updated:
Size: