From 2027a6133907d16d2256a2139f5fd09de00f3447 Mon Sep 17 00:00:00 2001 From: Martin Sherburn Date: Thu, 10 Oct 2019 05:58:21 -0700 Subject: [PATCH] Fix TimingAnimation rounding error issue (Take 2) Summary: This fixes a bug where the frames array can contain a duplicate entry at the end. For example, suppose the duration is 1000.0 then it would create an array with the following: ``` [ 0, 0.0010119824303159884, 0.00391003997863186, 0.00851330482578147, 0.01466951184383165, 0.022249135687575607, 0.03114100006836614, 0.041248932923769244, 0.05248918022066495, 0.06478838042267626, 0.07808196049642172, 0.09231285402599128, 0.1074304693764467, 0.12338985513375342, 0.14015102395653428, 0.15767840628626964, 0.17594041329987542, 0.1949090949486824, 0.21455988464815853, 0.23487142789035506, 0.25582549864491233, 0.27740701626433145, 0.2996041891505173, 0.3224088345090182, 0.34581696665965683, 0.36982983491413496, 0.394455794287552, 0.4197139228812336, 0.44564199741037275, 0.4723190090623474, 0.5000000572130084, 0.5276809909376533, 0.5543580025896278, 0.5802860771187669, 0.6055442057124484, 0.6301701650858652, 0.6541830333403433, 0.6775911654909819, 0.7003958108494828, 0.7225929837356684, 0.7441745013550876, 0.7651285721096447, 0.785440115351841, 0.8050909050513173, 0.8240595867001241, 0.84232159371373, 0.8598489760434653, 0.876610144866246, 0.8925695306235529, 0.9076871459740083, 0.9219180395035779, 0.9352116195773232, 0.9475108197793346, 0.9587510670762303, 0.9688589999316335, 0.9777508643124241, 0.9853304881561681, 0.9914866951742183, 0.996089960021368, 0.9989880175696839, 1, 1 ] ``` With this change, it now generates the following array: ``` [ 0, 0.0010119824303159884, 0.00391003997863186, 0.00851330482578147, 0.01466951184383165, 0.022249135687575607, 0.03114100006836614, 0.041248932923769244, 0.05248918022066495, 0.06478838042267626, 0.07808196049642172, 0.09231285402599128, 0.1074304693764467, 0.12338985513375342, 0.14015102395653428, 0.15767840628626964, 0.17594041329987542, 0.1949090949486824, 0.21455988464815853, 0.23487142789035506, 0.25582549864491233, 0.27740701626433145, 0.2996041891505173, 0.3224088345090182, 0.34581696665965683, 0.36982983491413496, 0.394455794287552, 0.4197139228812336, 0.44564199741037275, 0.4723190090623474, 0.5000000572130084, 0.5276809909376533, 0.5543580025896278, 0.5802860771187669, 0.6055442057124484, 0.6301701650858652, 0.6541830333403433, 0.6775911654909819, 0.7003958108494828, 0.7225929837356684, 0.7441745013550876, 0.7651285721096447, 0.785440115351841, 0.8050909050513173, 0.8240595867001241, 0.84232159371373, 0.8598489760434653, 0.876610144866246, 0.8925695306235529, 0.9076871459740083, 0.9219180395035779, 0.9352116195773232, 0.9475108197793346, 0.9587510670762303, 0.9688589999316335, 0.9777508643124241, 0.9853304881561681, 0.9914866951742183, 0.996089960021368, 0.9989880175696839, 1 ] ``` Note that the duplicate 1 at the end is now gone. This is because previously when it accumulated dt for 60 frames. dt wasn't quite exactly 1000, it was instead 999.9999999999999 and so didn't break out of the loop when it should have. This adds a tolerance so that it does break out of the loop. Reviewed By: dimach1977 Differential Revision: D17828204 fbshipit-source-id: 4483303de852071436cf9a82e50296baf3392329 --- .../src/__tests__/TimingAnimation-test.js | 32 +++++++++++++++++++ .../src/animations/TimingAnimation.js | 5 +-- 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 Libraries/Animated/src/__tests__/TimingAnimation-test.js diff --git a/Libraries/Animated/src/__tests__/TimingAnimation-test.js b/Libraries/Animated/src/__tests__/TimingAnimation-test.js new file mode 100644 index 00000000000000..d63f44b3928514 --- /dev/null +++ b/Libraries/Animated/src/__tests__/TimingAnimation-test.js @@ -0,0 +1,32 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @emails oncall+react_native + */ + +'use strict'; + +const TimingAnimation = require('../animations/TimingAnimation'); + +describe('Timing Animation', () => { + it('should return array of 61 items', () => { + const timingAnim = new TimingAnimation({duration: 1000}); + const config = timingAnim.__getNativeAnimationConfig(); + + expect(config.frames.length).toBe(61); + expect(config.frames[60]).toBe(1); + expect(config.frames[59]).toBeLessThan(1); + }); + + it('should cope with zero duration', () => { + const timingAnim = new TimingAnimation({duration: 0}); + const config = timingAnim.__getNativeAnimationConfig(); + + expect(config.frames.length).toBe(1); + expect(config.frames[0]).toBe(1); + }); +}); diff --git a/Libraries/Animated/src/animations/TimingAnimation.js b/Libraries/Animated/src/animations/TimingAnimation.js index b73794f1333e44..8458ef9712f678 100644 --- a/Libraries/Animated/src/animations/TimingAnimation.js +++ b/Libraries/Animated/src/animations/TimingAnimation.js @@ -66,8 +66,9 @@ class TimingAnimation extends Animation { __getNativeAnimationConfig(): any { const frameDuration = 1000.0 / 60.0; const frames = []; - for (let dt = 0.0; dt < this._duration; dt += frameDuration) { - frames.push(this._easing(dt / this._duration)); + const numFrames = Math.round(this._duration / frameDuration); + for (let frame = 0; frame < numFrames; frame++) { + frames.push(this._easing(frame / numFrames)); } frames.push(this._easing(1)); return {