Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: more predictable spring behaviour #7011

Open
wants to merge 3 commits into
base: svelte-4
Choose a base branch
from

Conversation

robertadamsonsmith
Copy link

Fix for bug described here: #7010

The proposed solution is that 'tick' should be added to the spring options. If the elapsed frame time is greater than this, then multiple tick calculations are carried out instead of the usual 1.

To prevent the number of rounds of calculations in a frame from increasing high enough to cause high cpu load (principally from when the user switches tab during a spring animation), it is proposed that there should be a configurable 'limit', which sets a limit on the number of ticks calculated per frame.

As this issue effects all springs, and is unlikely to cause issues with any existing usage, I think it may be appropriate for this to be the standard behaviour with sensible defaults.

I don't know if the proposed variable names are appropriate (it is hard to reduce a complex idea to a simple memorable term without losing some nuance.)

I haven't indented the existing code that should be indented to be in a loop, in order to make the changes clear. I've also not attempted to write any tests until I know that it would be worthwhile. This is my first attempt at submitting a pull request, so please let me know if I'm going about this at all wrong.

@robertadamsonsmith
Copy link
Author

I've just noticed two more issues with the current spring implementation.

Firstly, the acceleration is not being multiplied by ctx.dt, which makes changes to acceleration incorrectly calculated when the frame rate is anything other than exactly 60fps. I think that this is an important and trivial change (the changes in this branch make it easy to see this issue, by setting the 'tick' setting to a low number).

Secondly, there is an assumption that the elapsed time between the last_value and current_value, is the same as between the current_value and next_value. They will usually be similar, which is why the bug isn't obvious, but in those cases where the frame times can vary, it introduces a considerable source of error.

I will update this branch to fix both issues, but I think that this all needs some good unit tests to make the intended and actual behaviour clearer. Hopefully there is a good way to mock the 'now' and 'loop' functions.

@benmccann benmccann changed the title [fix] Erratic spring behaviour fix: more predictable spring behaviour Mar 14, 2023
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Mar 30, 2023

@Rich-Harris is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Mar 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
svelte-dev-2 ❌ Failed (Inspect) Mar 31, 2023 at 3:01AM (UTC)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants