Defer keyframe resolution#2448
Conversation
mattgperry
force-pushed
the
fix/async-animation
branch
2 times, most recently
from
7d7fd64 to
442a1bf
Compare
mattgperry
force-pushed
the
fix/async-animation
branch
from
9417c50 to
f795e33
Compare
mattgperry
force-pushed
the
fix/async-animation
branch
3 times, most recently
from
f22a711 to
76580f0
Compare
mattgperry
force-pushed
the
fix/async-animation
branch
from
3cf6299 to
2ca44fc
Compare
mattgperry
force-pushed
the
fix/async-animation
branch
from
51b90c0 to
e3036c3
Compare
| @@ -28,15 +28,15 @@ | |||
| } | |||
There was a problem hiding this comment.
Can ignore the /benchmarks
| chain = chain | ||
| .trigger("pointermove", 360, baseY + y, { force: true }) | ||
| .wait(50) | ||
| .wait(100) |
| { duration: 1, transform: { duration: 2 } } | ||
| ) | ||
|
|
||
| await nextFrame() |
There was a problem hiding this comment.
WAAPI nextFrame calls generally added as the WAAPI animation is only initialised after keyframes are resolved. We could add more custom logic here to say if something is WAAPI we can resolve fewer keyframes as it can handle CSS vars etc itself
| visualElement.values.forEach((value) => value.stop()) | ||
| } | ||
|
|
||
| function setVariants(visualElement: VisualElement, variantLabels: string[]) { |
There was a problem hiding this comment.
Moved in from utils/setters as only used in this file.
| let { elapsed = 0 } = transition | ||
| elapsed = elapsed - secondsToMilliseconds(delay) | ||
|
|
||
| const keyframes = getKeyframes( |
There was a problem hiding this comment.
Moved to can-animate
| } | ||
|
|
||
| readValueFromInstance(instance: SVGElement, key: string) { | ||
| // console.log("reading", key, "from", instance) |
There was a problem hiding this comment.
| // console.log("reading", key, "from", instance) |
| export function makeNoneKeyframesAnimatable( | ||
| unresolvedKeyframes: UnresolvedKeyframes<string | number>, | ||
| noneKeyframeIndexes: number[], | ||
| name?: string | ||
| ) { | ||
| /** | ||
| * If we detected "none"-equivalent keyframes, we need to find a template | ||
| */ | ||
| let i = 0 | ||
| let animatableTemplate: string | undefined = undefined | ||
| while (i < unresolvedKeyframes.length && !animatableTemplate) { | ||
| if ( | ||
| typeof unresolvedKeyframes[i] === "string" && | ||
| unresolvedKeyframes[i] !== "none" && | ||
| unresolvedKeyframes[i] !== "0" | ||
| ) { | ||
| animatableTemplate = unresolvedKeyframes[i] as string | ||
| } | ||
| i++ | ||
| } | ||
|
|
||
| if (animatableTemplate && name) { | ||
| for (const noneIndex of noneKeyframeIndexes) { | ||
| unresolvedKeyframes[noneIndex] = getAnimatableNone( | ||
| name, | ||
| animatableTemplate | ||
| ) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm having a hard time understanding what this is doing. What is the template, just some keyframe value like a color string or numeric value?
There was a problem hiding this comment.
Let me add a comment - but to explain given a bunch of keyframes
[0, null, "#fff 300px 300px"]
We're looking for one that can serve as a template. In this case we want the animation to output something like the last keyframe so we take that and turn each value into a zero, currently "#fff0 0px 0px" and use that in place of 0.
adamseckel
left a comment
adamseckel
left a comment
There was a problem hiding this comment.
I wonder if before merging this, it would be worthwhile to create a FramerStudio branch with this alpha, and see what tests pass/fail as an indicator of if we are missing anything critical?
| ) as any | ||
|
|
||
| if (name === "height" && this.suspendedScrollY !== undefined) { | ||
| window.scrollTo(0, this.suspendedScrollY) |
There was a problem hiding this comment.
Why do we need to scroll the window to 0?
There was a problem hiding this comment.
We're scrolling it to the value it was before measurement - the measurement can fuck with page scroll
* Refactor * Latest * Latest * Latest * Latest * Latest * Latest * Latest * Latest * Fixing tests * Latest * Fixing tests
mattgperry
force-pushed
the
fix/async-animation
branch
from
777eb3a to
1175f07
Compare
2 participants
When keyframes aren't inherently mixable, we make them mixable by measuring the DOM or resolving values from it.
This is currently done synchronously per
animate()call /motioncomponent animation which can lead to layout thrashing when many are performed and/or when mixed in amongst other layout-measuring effects.Solution
This PR defers keyframe resolution until the following animation frame. All reads and writes are batched so we have at max 1 style recalculation/ layout reflow.
It also refactors JS and WAAPI animations into a
MainThreadAnimationandAcceleratedAnimationclass inheriting fromBaseAnimationto share as much of this new logic as possible.Animations still return synchronously-available animation controls like
.play()/.timeetc. As much of this is computed or otherwise relies on resolved keyframes, they internally usethis.resolvedwhich is behind a getter. This getter will synchronously flush all pending animations as a de-opt when accessed. Some methods have already been changed to allow pending operations but this can be improved further. In the absolute worst case, where animations are created/accessedPerformance is the same as today. But in the best case, cold start performance is 2.5x faster and unit conversion performance is 6x faster, and much of that work is lifted out of the effect and into the next animation frame.
Bonus:
Fixes #2168
Fixes #2309
Fixes #2339