Skip to content

Commit 2079cb2

Browse files
sammy-SCfacebook-github-bot
authored andcommitted
Fix AnimationDriverTests and align with android on rounding (facebook#51922)
Summary: Pull Request resolved: facebook#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
1 parent 79354eb commit 2079cb2

4 files changed

Lines changed: 7 additions & 8 deletions

File tree

packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/AnimationDriver.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ void AnimationDriver::runAnimationStep(double renderingTime) {
6969
return;
7070
}
7171

72-
// ticks are 100 nanoseconds, divide by 10000 to get milliseconds.
7372
const auto frameTimeMs = renderingTime;
7473
auto restarting = false;
7574
if (startFrameTimeMs_ < 0) {

packages/react-native/ReactCxxPlatform/react/renderer/animated/drivers/FrameAnimationDriver.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ bool FrameAnimationDriver::update(double timeDeltaMs, bool /*restarting*/) {
5858
}
5959

6060
const auto startIndex =
61-
static_cast<size_t>(std::ceil(timeDeltaMs / SingleFrameIntervalMs));
61+
static_cast<size_t>(std::round(timeDeltaMs / SingleFrameIntervalMs));
6262
assert(startIndex >= 0);
6363
const auto nextIndex = startIndex + 1;
6464

packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationDriverTests.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,21 @@ TEST_F(AnimationDriverTests, framesAnimation) {
4040
"toValue", toValue),
4141
std::nullopt);
4242

43-
const auto startTimeInTick = 12345;
43+
const double startTimeInTick = 12345;
4444

4545
runAnimationFrame(startTimeInTick);
4646
EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 0);
4747

48-
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 2.5 * TicksPerMs);
48+
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 2.5);
4949
EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 65);
5050

51-
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 3 * TicksPerMs);
51+
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 3);
5252
EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), 90);
5353

54-
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 4 * TicksPerMs);
54+
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 4);
5555
EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), toValue);
5656

57-
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 10 * TicksPerMs);
57+
runAnimationFrame(startTimeInTick + SingleFrameIntervalMs * 10);
5858
EXPECT_EQ(round(nodesManager_->getValue(valueNodeTag).value()), toValue);
5959
}
6060

packages/react-native/ReactCxxPlatform/react/renderer/animated/tests/AnimationTestsBase.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class AnimationTestsBase : public testing::Test {
4242
}
4343

4444
void runAnimationFrame(double timestamp) {
45-
nodesManager_->onAnimationFrame(static_cast<uint64_t>(timestamp));
45+
nodesManager_->onAnimationFrame(timestamp);
4646
}
4747

4848
std::shared_ptr<NativeAnimatedNodesManager> nodesManager_;

0 commit comments

Comments
 (0)