-
Notifications
You must be signed in to change notification settings - Fork 28.2k
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 SpringSimulation formula for underdamping #165017
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
I trust your math :)
Unfortunately this PR has caused some Google internal failures. I'm going to fix them (but probably not very soon since I have a lot on my hand). Also this change will probably count as breaking changes and I'll write a migration guide. Therefore don't expect this PR land very soon. |
Why will it count as breaking change? I don't see any changes in the API itself |
@dkwingsmt this should not be a breaking change. yes, updating scroll physics will cause some golden files to change heavily, but fixing this is more important than keeping those goldens the same. |
I think the idea is that developers do not expect their apps to behave differently after a version update. Silent changes like this are even worse than compile errors. A migration guide should be provided to let developers know how to calculate new values to achieve the previous effect. |
No that is absolutely not our policy. We need to be able to update things like scroll physics |
By our policy,
This PR breaks a few tests in Google tests. Shouldn't it count? |
If you changed something unrelated and a bunch of goldens broke, it is a good sign you introduced a bug yes. But in this case, you are intentionally fixing the spring simulation formula - so its expected that golden tests which perform something like a fling will change. Arguably these tests should not depend on the exact physics, because at any rate we must be able to make updates to physics to match the platform. We are absolutely allowed to update golden files. Unless it is a major change (i.e. light colors become dark) we do not need to flag it. |
Fix #163858
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.