Unverified Commit 97938859 authored by Greg Price's avatar Greg Price Committed by GitHub

Fix buggy formula for critically-damped springs (#120488)

Fixes #109675.

This formula would produce an initial velocity quite different
from the one specified as an argument.

To update the test, I computed the expected results separately
by using the physical formula.

Happily, the framework by default never ends up actually exercising
this code.  Of the four SpringDescription call sites within the
framework, two are explicitly overdamped; the other two are by
design critically damped, but due to rounding they end up being
treated as (very slightly) overdamped too.  Details here:
  https://github.com/flutter/flutter/issues/109675#issuecomment-1423674855

So the only way an app could be affected by this bug is if it called
a SpringDescription constructor itself, and managed to create a spring
description where the distinguishing formula in _SpringSolution comes
out exactly equal to zero.  It's likely nobody has ever shipped such
an app, because the behavior this produces would be so wildly wrong
that it'd be hard to miss when exercised.
Co-authored-by: 's avatarKate Lovett <katelovett@google.com>
parent 8080beca
...@@ -176,7 +176,7 @@ class _CriticalSolution implements _SpringSolution { ...@@ -176,7 +176,7 @@ class _CriticalSolution implements _SpringSolution {
) { ) {
final double r = -spring.damping / (2.0 * spring.mass); final double r = -spring.damping / (2.0 * spring.mass);
final double c1 = distance; final double c1 = distance;
final double c2 = velocity / (r * distance); final double c2 = velocity - (r * distance);
return _CriticalSolution.withArgs(r, c1, c2); return _CriticalSolution.withArgs(r, c1, c2);
} }
......
...@@ -167,19 +167,19 @@ void main() { ...@@ -167,19 +167,19 @@ void main() {
expect(crit.isDone(0.0), false); expect(crit.isDone(0.0), false);
expect(crit.x(0.0), 0.0); expect(crit.x(0.0), 0.0);
expect(crit.dx(0.0), 5000.0); expect(crit.dx(0.0), 0.0);
expect(crit.x(0.25).floor(), 458.0); expect(crit.x(0.25).floor(), 356);
expect(crit.x(0.50).floor(), 496.0); expect(crit.x(0.50).floor(), 479);
expect(crit.x(0.75).floor(), 499.0); expect(crit.x(0.75).floor(), 497);
expect(crit.dx(0.25).floor(), 410); expect(crit.dx(0.25).floor(), 1026);
expect(crit.dx(0.50).floor(), 33); expect(crit.dx(0.50).floor(), 168);
expect(crit.dx(0.75).floor(), 2); expect(crit.dx(0.75).floor(), 20);
expect(crit.isDone(1.50), true);
expect(crit.x(1.5) > 499.0 && crit.x(1.5) < 501.0, true); expect(crit.x(1.5) > 499.0 && crit.x(1.5) < 501.0, true);
expect(crit.dx(1.5) < 0.1, true /* basically within tolerance */); expect(crit.dx(1.5) < 0.1, true /* basically within tolerance */);
expect(crit.isDone(1.60), true);
}); });
test('overdamped_spring', () { test('overdamped_spring', () {
...@@ -195,6 +195,7 @@ void main() { ...@@ -195,6 +195,7 @@ void main() {
expect(over.isDone(0.0), false); expect(over.isDone(0.0), false);
expect(over.x(0.0), 0.0); expect(over.x(0.0), 0.0);
expect(over.dx(0.0), moreOrLessEquals(0.0));
expect(over.x(0.5).floor(), 445.0); expect(over.x(0.5).floor(), 445.0);
expect(over.x(1.0).floor(), 495.0); expect(over.x(1.0).floor(), 495.0);
...@@ -216,6 +217,8 @@ void main() { ...@@ -216,6 +217,8 @@ void main() {
expect(under.type, SpringType.underDamped); expect(under.type, SpringType.underDamped);
expect(under.isDone(0.0), false); expect(under.isDone(0.0), false);
expect(under.x(0.0), moreOrLessEquals(0.0));
expect(under.dx(0.0), moreOrLessEquals(0.0));
// Overshot with negative velocity // Overshot with negative velocity
expect(under.x(1.0).floor(), 325); expect(under.x(1.0).floor(), 325);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment