Edit site: WAAPIfy canvas moving animation #62435
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -16.1 kB (-0.92%) Total Size: 1.73 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 590c54e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9438501143
|
stokesman
changed the title
stokesman
added
[Type] Performance
|
🎰 |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
It’s not recommended in the spec and unnecessary for the particular animation case.
|
Are there any libraries around |
motion and @okikio/animate. Motion One is quite small and it’d probably be worth a try for this. I'm not sure that it supports custom easing like the animation currently uses. I suppose that may not be a hard requirement though. If not, the implementation here might also be simplified to such a degree that maintenance isn’t a concern. |
|
I think @youknowriad moved it from Motion to React Spring, but I could be recalling it incorrectly. |
I believe you're correct. Just that it was moved from |
youknowriad
reviewed
Jun 11, 2024
| * @param {number} target The target progression — `1` is end, `0` is beginning. | ||
| * @param {(() => void) | undefined} callback | ||
| */ | ||
| const animate = ( element, config, target = 1, callback ) => { |
There was a problem hiding this comment.
Is this some kind of generic utility you wrote on your own? Do we expect this to be reused elsewhere (block animation maybe as I just copied that one and adapted it to create this file)
There was a problem hiding this comment.
I pared down Svelte’s internal animate. It would serve for reuse, the block animation could pass a different config (one that operates only on transfrom which should make that one run on the GPU). I’d thought we’d want to try that if this worked well in this instance.
|
Are we able to notice perf improvements or less lags or something with this PR? |
|
In my testing, the trunk animation feels better (less jumpy). For this one for instance I can see the edge of the frame move left than right in some occasions. |
Ideally there’s no difference in best-case scenarios where both animations aren’t impacted by main thread slowness.
I think that’s possible. This way of running the animation keeps the duration close to specified, so if it stutters it’s a bigger stutter because the motion is compensated to keep the duration accurate.
I didn’t notice that but I’m curious and will try to repro. Thanks for testing! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Refactors canvas moving animation so
it doesn’t run on the main threadits timing is less impacted by jank.Why?
How?
Computes the animation keyframes before starting it and passes them to
animate.More
The idea and the (adapted) implementation are borrowed from Svelte. This generates the keyframes needed for an animation beforehand and that enables
Element’sanimatemethod to handle the rest(off the main thread 🎉). That’s in contrast to the current implementation that on every animation frame computes the keyframe and applies it to the element imperatively.Update: I’ve learned that because the animation is changing the element’s
widthandheightthe animation can still stutter if the main thread gets busy. However, the duration remains close to what it’s supposed to be unlike the current implementation where the duration gets stretched out. So it’s still a slight improvement for the worst case scenarios.Testing Instructions