From 6c78e36ccb3d65b6082d6f451c397cd9329992f4 Mon Sep 17 00:00:00 2001 From: Simon Friis Vindum Date: Wed, 21 Feb 2024 01:36:08 +0100 Subject: [PATCH] Fix initialization of time in repeat on AnimationController (#142887) This PR fixes #142885. The issue is that in `_RepeatingSimulation` the initial time is calculated as follows: ``` (initialValue / (max - min)) * (period.inMicroseconds / Duration.microsecondsPerSecond) ``` This calculation does not work in general. For instance, if `max` is 300, `min` is 100, and `initialValue` is 100 then `initialValue / (max - min)` is 1/2 when it should be 0 The current tests work by happenstance because the numbers used happen to work. To reveal the bug I've added some more tests similar to the existing ones but with different numbers. A "side-effect" of the incorrect calculation is that if `initialValue` is 0, then the animation will always start from `min` no matter what. For instance, in one of the tests, an `AnimationController` with the value 0 is told to `repeat` between 0.5 and 1.0, and this starts the animation from 0.5. To preserve this behavior, and to more generally handle the case where the initial value is out of bounds, this PR clamps the initial value to be within the lower and upper bounds of the repetition. Just for reference, this calculation was introduced at https://github.com/flutter/flutter/pull/25125. --- .../src/animation/animation_controller.dart | 2 +- .../animation/animation_controller_test.dart | 45 ++++++++++++++++++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/packages/flutter/lib/src/animation/animation_controller.dart b/packages/flutter/lib/src/animation/animation_controller.dart index a0ec2c8e7568..70b303ba9842 100644 --- a/packages/flutter/lib/src/animation/animation_controller.dart +++ b/packages/flutter/lib/src/animation/animation_controller.dart @@ -910,7 +910,7 @@ typedef _DirectionSetter = void Function(_AnimationDirection direction); class _RepeatingSimulation extends Simulation { _RepeatingSimulation(double initialValue, this.min, this.max, this.reverse, Duration period, this.directionSetter) : _periodInSeconds = period.inMicroseconds / Duration.microsecondsPerSecond, - _initialT = (max == min) ? 0.0 : (initialValue / (max - min)) * (period.inMicroseconds / Duration.microsecondsPerSecond) { + _initialT = (max == min) ? 0.0 : ((clampDouble(initialValue, min, max) - min) / (max - min)) * (period.inMicroseconds / Duration.microsecondsPerSecond) { assert(_periodInSeconds > 0.0); assert(_initialT >= 0.0); } diff --git a/packages/flutter/test/animation/animation_controller_test.dart b/packages/flutter/test/animation/animation_controller_test.dart index 725a44fb7aa2..c2112762946d 100644 --- a/packages/flutter/test/animation/animation_controller_test.dart +++ b/packages/flutter/test/animation/animation_controller_test.dart @@ -763,8 +763,8 @@ void main() { ); test( - 'calling repeat with specified min and max values makes the animation ' - 'alternate between min and max values on each repeat', + 'calling repeat with specified min and max values between 0 and 1 makes ' + 'the animation alternate between min and max values on each repeat', () { final AnimationController controller = AnimationController( duration: const Duration(milliseconds: 100), @@ -799,6 +799,47 @@ void main() { tick(Duration.zero); tick(const Duration(milliseconds: 125)); expect(controller.value, 1.0); + + controller.reset(); + controller.value = 0.2; + expect(controller.value, 0.2); + + controller.repeat(reverse: true, min: 0.2, max: 0.6); + tick(Duration.zero); + tick(const Duration(milliseconds: 50)); + expect(controller.value, 0.4); + controller.dispose(); + }, + ); + + test( + 'calling repeat with negative min value and positive max value makes the ' + 'animation alternate between min and max values on each repeat', + () { + final AnimationController controller = AnimationController( + duration: const Duration(milliseconds: 100), + value: 1.0, + lowerBound: -1, + upperBound: 3, + vsync: const TestVSync(), + ); + + expect(controller.value, 1.0); + + controller.repeat(min: 1, max: 3); + tick(Duration.zero); + expect(controller.value, 1); + tick(const Duration(milliseconds: 50)); + expect(controller.value, 2); + + controller.reset(); + controller.value = 0.0; + + controller.repeat(min: -1, max: 3); + tick(Duration.zero); + expect(controller.value, 0); + tick(const Duration(milliseconds: 25)); + expect(controller.value, 1); controller.dispose(); }, );