From 2079cb295f2ea6267f4a0ce4876b6ea696ce98da Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 10 Jun 2025 08:03:44 -0700 Subject: [PATCH] Fix AnimationDriverTests and align with android on rounding (#51922) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51922 changelog: [internal] fix existing C++ Animated tests and align with Android on how to go from current time to applied frame. On iOS [floor](https://fburl.com/code/7zy5e5ul) is used to decide which frame to apply. On Android, [round](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/animated/FrameBasedAnimationDriver.kt#L65) is used. In D75813200 I chose to use `std::ceil` as I wanted to have a predictable behaviour in tests. This is not wrong but it is better to align at least with one of the existing implementations. Let's go with Android as it strikes the balance of what we want to see in tests (an animation that is running for 1000ms should finish after 1000ms, not 1000ms + one frame) and C++ Animated is closer to at least one of the existing implementations. Reviewed By: christophpurrer Differential Revision: D76337384 fbshipit-source-id: 444c94d88c2fa60bb4f0649f57e0e42f5cd27626 --- .../renderer/animated/drivers/AnimationDriver.cpp | 1 - .../renderer/animated/drivers/FrameAnimationDriver.cpp | 2 +- .../renderer/animated/tests/AnimationDriverTests.cpp | 10 +++++----- .../react/renderer/animated/tests/AnimationTestsBase.h | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/AnimationDriver.cpp b/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/AnimationDriver.cpp index 086c09a169a..edb0463e15f 100644 --- a/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/AnimationDriver.cpp +++ b/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/AnimationDriver.cpp @@ -69,7 +69,6 @@ void AnimationDriver::runAnimationStep(double renderingTime) { return; } - // ticks are 100 nanoseconds, divide by 10000 to get milliseconds. const auto frameTimeMs = renderingTime; auto restarting = false; if (startFrameTimeMs_ < 0) { diff --git a/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/FrameAnimationDriver.cpp b/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/FrameAnimationDriver.cpp index 7bcec731ec2..5398648ea13 100644 --- a/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/FrameAnimationDriver.cpp +++ b/packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/FrameAnimationDriver.cpp @@ -58,7 +58,7 @@ bool FrameAnimationDriver::update(double timeDeltaMs, bool /*restarting*/) { } const auto startIndex = - static_cast(std::ceil(timeDeltaMs / SingleFrameIntervalMs)); + static_cast(std::round(timeDeltaMs / SingleFrameIntervalMs)); assert(startIndex >= 0); const auto nextIndex = startIndex + 1; diff --git a/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationDriverTests.cpp b/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationDriverTests.cpp index 456edf22943..5fd98c4bd2a 100644 --- a/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationDriverTests.cpp +++ b/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationDriverTests.cpp @@ -40,21 +40,21 @@ TEST_F(AnimationDriverTests, framesAnimation) { "toValue", toValue), std::nullopt); - const auto startTimeInTick = 12345; + const double startTimeInTick = 12345; runAnimationFrame(startTimeInTick); EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 0); - runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 2.5 * TicksPerMs); + runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 2.5); EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 65); - runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 3 * TicksPerMs); + runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 3); EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 90); - runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 4 * TicksPerMs); + runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 4); EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), toValue); - runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 10 * TicksPerMs); + runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 10); EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), toValue); } diff --git a/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationTestsBase.h b/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationTestsBase.h index 8e582908b55..97ff2be3743 100644 --- a/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationTestsBase.h +++ b/packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationTestsBase.h @@ -42,7 +42,7 @@ class AnimationTestsBase : public testing::Test { } void runAnimationFrame(double timestamp) { - nodesManager_->onAnimationFrame(static_cast(timestamp)); + nodesManager_->onAnimationFrame(timestamp); } std::shared_ptr nodesManager_;