Skip to content

Commit cf84371

Browse files
Mark Verlingierifacebook-github-bot
authored andcommitted
Step time-based animations only on frames with the shared animated backend (#59565)
Summary: Fixes native-driven animations stepping backward for a frame when the shared animated backend is on and a native event update (e.g. an `Animated.event` `onScroll`) runs between frames. Without the shared backend this does not happen. With the shared backend, an event update also steps every active timing, spring and decay animation, at the time of the update. Frames step them at the platform frame time (on Android, the Choreographer vsync time), which is often earlier: 1. An event update steps an animation at time T. 2. The next frame steps it at T - 8..25 ms. 3. The animation moves backward for that frame; a timing animation started by the event overshoots past its start value. Event updates now only propagate the event values, and time-based animations step only on frames. This aligns the shared backend with the implementations it replaces: the Kotlin `NativeAnimatedNodesManager` updates nodes on events without stepping animations, and the C++ path without the shared backend steps everything on one clock. As on Kotlin, a timing animation whose `toValue` tracks an event-driven value restarts on each event and advances only on frames between events. Changelog: [General][Fixed] - Fix native-driven animations stepping backward when a native event update runs between frames with the shared animated backend Reviewed By: zeyap Differential Revision: D127496079
1 parent 29636a2 commit cf84371

7 files changed

Lines changed: 235 additions & 14 deletions

File tree

‎packages/react-native/Libraries/Animated/__tests__/AnimatedBackend-itest.js‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -912,3 +912,103 @@ test('animate width with memo and rerender (js sync test)', () => {
912912
<rn-view width="200" />,
913913
);
914914
});
915+
916+
// A native scroll event pushes its values outside the frame loop. It must not
917+
// step running time-based animations: frames step them with the frame time, so
918+
// stepping them at the push's own time as well would move them whenever the
919+
// two times differ.
920+
test('native scroll event does not step a running timing animation', () => {
921+
const scrollViewRef = createRef<HostInstance>();
922+
const viewRef = createRef<HostInstance>();
923+
const scrollFollowerRef = createRef<HostInstance>();
924+
let _translateX;
925+
926+
component MyApp() {
927+
const scrollY = useAnimatedValue(0);
928+
const translateX = useAnimatedValue(0);
929+
_translateX = translateX;
930+
return (
931+
<View style={{flex: 1}}>
932+
<Animated.View
933+
ref={viewRef}
934+
style={{width: 10, height: 10, transform: [{translateX}]}}
935+
/>
936+
<Animated.View
937+
ref={scrollFollowerRef}
938+
style={{width: 10, height: 10, transform: [{translateY: scrollY}]}}
939+
/>
940+
<Animated.ScrollView
941+
ref={scrollViewRef}
942+
onScroll={Animated.event(
943+
[{nativeEvent: {contentOffset: {y: scrollY}}}],
944+
{useNativeDriver: true},
945+
)}>
946+
<View style={{height: 1000, width: 100}} />
947+
</Animated.ScrollView>
948+
</View>
949+
);
950+
}
951+
952+
// Stamp pushes far later than the frame clock, as on device, where a push
953+
// between frames runs later than the next frame's vsync time.
954+
const mockClock = Fantom.installHighResTimeStampMock();
955+
mockClock.setTime(1e12);
956+
let animation;
957+
try {
958+
const root = Fantom.createRoot();
959+
Fantom.runTask(() => {
960+
root.render(<MyApp />);
961+
});
962+
const scrollViewElement = nullthrows(scrollViewRef.current);
963+
const viewElement = nullthrows(viewRef.current);
964+
const scrollFollowerElement = nullthrows(scrollFollowerRef.current);
965+
966+
const getTransformValue = (element: HostInstance, key: string): number => {
967+
const transform =
968+
Fantom.unstable_getDirectManipulationProps(element).transform;
969+
const first = Array.isArray(transform) ? transform[0] : null;
970+
if (
971+
first == null ||
972+
typeof first !== 'object' ||
973+
typeof first[key] !== 'number'
974+
) {
975+
throw new Error(`Expected a numeric ${key} transform`);
976+
}
977+
return first[key];
978+
};
979+
const getTranslateX = (): number =>
980+
getTransformValue(viewElement, 'translateX');
981+
982+
Fantom.runTask(() => {
983+
animation = Animated.timing(nullthrows(_translateX), {
984+
toValue: 100,
985+
duration: 1000,
986+
easing: Easing.linear,
987+
useNativeDriver: true,
988+
});
989+
animation.start();
990+
});
991+
Fantom.unstable_produceFramesForDuration(500);
992+
993+
const beforeScroll = getTranslateX();
994+
expect(beforeScroll).toBeGreaterThan(0);
995+
expect(beforeScroll).toBeLessThan(100);
996+
997+
Fantom.scrollTo(scrollViewElement, {x: 0, y: 10});
998+
// The scroll event reached the native graph...
999+
expect(getTransformValue(scrollFollowerElement, 'translateY')).toBe(10);
1000+
// ...and left the running timing animation where the frames put it.
1001+
expect(getTranslateX()).toBe(beforeScroll);
1002+
1003+
Fantom.unstable_produceFramesForDuration(100);
1004+
expect(getTranslateX()).toBeGreaterThan(beforeScroll);
1005+
expect(getTranslateX()).toBeLessThan(100);
1006+
} finally {
1007+
Fantom.runTask(() => {
1008+
animation?.stop();
1009+
});
1010+
// Flush the work queued by stopping the animation.
1011+
Fantom.runWorkLoop();
1012+
mockClock.uninstall();
1013+
}
1014+
});

‎packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.cpp‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,9 +555,14 @@ void NativeAnimatedNodesManager::handleAnimatedEvent(
555555
// frames.
556556
if (useSharedAnimatedBackend_) {
557557
if (auto animationBackend = animationBackend_.lock()) {
558+
// Pass stepAnimations false so this push only propagates event values.
559+
// Frames step time-based drivers with the vsync time, which can be
560+
// earlier than this push's time, so stepping them here would move them
561+
// backward on the next frame.
558562
animationBackend->pushAnimationMutations(
559563
[this](AnimationTimestamp timestamp) -> AnimationMutations {
560-
return pullAnimationMutations(timestamp);
564+
return pullAnimationMutations(
565+
timestamp, /* stepAnimations */ false);
561566
});
562567
}
563568
} else {
@@ -1038,14 +1043,17 @@ void NativeAnimatedNodesManager::insertMutations(
10381043

10391044
AnimationMutations NativeAnimatedNodesManager::onAnimationFrameForBackend(
10401045
AnimatedPropsBuilder& propsBuilder,
1041-
AnimationTimestamp timestamp) {
1046+
AnimationTimestamp timestamp,
1047+
bool stepAnimations) {
10421048
AnimationMutations mutations{};
10431049
auto timestampMs = timestamp.count();
10441050
// Run all active animations
10451051
auto hasFinishedAnimations = false;
10461052
std::set<int> finishedAnimationValueNodes;
10471053
for (const auto& [_id, driver] : activeAnimations_) {
1048-
driver->runAnimationStep(timestampMs);
1054+
if (stepAnimations) {
1055+
driver->runAnimationStep(timestampMs);
1056+
}
10491057

10501058
if (driver->getIsComplete()) {
10511059
hasFinishedAnimations = true;
@@ -1083,7 +1091,8 @@ AnimationMutations NativeAnimatedNodesManager::onAnimationFrameForBackend(
10831091
}
10841092

10851093
AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations(
1086-
AnimationTimestamp timestamp) {
1094+
AnimationTimestamp timestamp,
1095+
bool stepAnimations) {
10871096
if (!useSharedAnimatedBackend_) {
10881097
return {};
10891098
}
@@ -1113,7 +1122,8 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations(
11131122
// Step through the animation loop
11141123
if (isAnimationUpdateNeeded()) {
11151124
AnimatedPropsBuilder propsBuilder;
1116-
mutations = onAnimationFrameForBackend(propsBuilder, timestamp);
1125+
mutations =
1126+
onAnimationFrameForBackend(propsBuilder, timestamp, stepAnimations);
11171127

11181128
if (mutations.batch.empty()) {
11191129
// The last animation tick didn't result in any changes to the UI.

‎packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManager.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,11 @@ class NativeAnimatedNodesManager : public std::enable_shared_from_this<NativeAni
133133
AnimationMutations &mutations,
134134
AnimatedPropsBuilder &propsBuilder,
135135
bool hasLayoutUpdates = false);
136-
AnimationMutations onAnimationFrameForBackend(AnimatedPropsBuilder &propsBuilder, AnimationTimestamp timestamp);
137-
AnimationMutations pullAnimationMutations(AnimationTimestamp timestamp);
136+
AnimationMutations onAnimationFrameForBackend(
137+
AnimatedPropsBuilder &propsBuilder,
138+
AnimationTimestamp timestamp,
139+
bool stepAnimations = true);
140+
AnimationMutations pullAnimationMutations(AnimationTimestamp timestamp, bool stepAnimations = true);
138141

139142
#pragma mark - Drivers
140143

‎packages/react-native/ReactCommon/react/renderer/animated/tests/EventAnimationDriverTests.cpp‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,83 @@
77

88
#include "AnimationTestsBase.h"
99

10+
#include <react/renderer/animated/drivers/AnimationDriverUtils.h>
1011
#include <react/renderer/components/scrollview/ScrollEvent.h>
1112
#include <react/renderer/core/ReactRootViewTagGenerator.h>
1213

1314
namespace facebook::react {
1415

15-
class EventAnimationDriverTests : public AnimationTestsBase {};
16+
namespace {
17+
18+
// Runs event-driven updates at a timestamp the test sets, like a platform
19+
// whose current time is later than the vsync time of the next frame.
20+
class FakeAnimationBackend : public UIManagerAnimationBackend {
21+
public:
22+
AnimationTimestamp pushTimestamp{0};
23+
24+
void onAnimationFrame(AnimationTimestamp /*timestamp*/) override {}
25+
CallbackId start(const Callback& /*callback*/) override {
26+
return 0;
27+
}
28+
void stop(CallbackId /*callbackId*/) override {}
29+
void clearRegistry(SurfaceId /*surfaceId*/) override {}
30+
void clearRegistryOnSurfaceStop(SurfaceId /*surfaceId*/) override {}
31+
void trigger() override {}
32+
void pushAnimationMutations(const Callback& callback) override {
33+
callback(pushTimestamp);
34+
}
35+
void registerJSInvoker(std::shared_ptr<CallInvoker> /*jsInvoker*/) override {}
36+
};
37+
38+
} // namespace
39+
40+
class EventAnimationDriverTests : public AnimationTestsBase {
41+
protected:
42+
static constexpr double kStartTime = 1000;
43+
44+
std::shared_ptr<FakeAnimationBackend> backend_ =
45+
std::make_shared<FakeAnimationBackend>();
46+
Tag tag_{getNextRootViewTag()};
47+
Tag viewTag_{++tag_};
48+
Tag scrollValueTag_{++tag_};
49+
Tag animatedValueTag_{++tag_};
50+
51+
// A shared-backend manager with a scroll event mapped to scrollValueTag_ and
52+
// the given animation running on animatedValueTag_.
53+
void startSharedBackendAnimation(const folly::dynamic& animationConfig) {
54+
// Marks this thread as the render thread.
55+
initNodesManager();
56+
nodesManager_ = std::make_shared<NativeAnimatedNodesManager>(backend_);
57+
58+
const folly::dynamic valueNodeConfig =
59+
folly::dynamic::object("type", "value")("value", 0)("offset", 0);
60+
nodesManager_->createAnimatedNode(scrollValueTag_, valueNodeConfig);
61+
nodesManager_->addAnimatedEventToView(
62+
viewTag_,
63+
"scroll",
64+
folly::dynamic::object("animatedValueTag", scrollValueTag_)(
65+
"nativeEventPath", folly::dynamic::array("contentOffset", "y")));
66+
nodesManager_->createAnimatedNode(animatedValueTag_, valueNodeConfig);
67+
nodesManager_->startAnimatingNode(
68+
1, animatedValueTag_, animationConfig, std::nullopt);
69+
}
70+
71+
void runBackendFrame(double timestamp) {
72+
nodesManager_->pullAnimationMutations(AnimationTimestamp{timestamp});
73+
}
74+
75+
void dispatchScrollAt(double timestamp, double offsetY) {
76+
backend_->pushTimestamp = AnimationTimestamp{timestamp};
77+
auto scrollEvent = std::make_shared<ScrollEvent>();
78+
scrollEvent->contentOffset = {.x = 0, .y = static_cast<Float>(offsetY)};
79+
(*nodesManager_->getEventEmitterListener())(
80+
viewTag_, "scroll", *scrollEvent);
81+
}
82+
83+
double animatedValue() {
84+
return nodesManager_->getValue(animatedValueTag_).value();
85+
}
86+
};
1687

1788
TEST_F(EventAnimationDriverTests, subscribeToViewEvent) {
1889
initNodesManager();
@@ -67,4 +138,41 @@ TEST_F(EventAnimationDriverTests, subscribeToViewEvent) {
67138
EXPECT_EQ(nodesManager_->getValue(animatedValueTag2), 11);
68139
}
69140

141+
TEST_F(
142+
EventAnimationDriverTests,
143+
sharedBackendEventDoesNotStepTimingAnimationAheadOfFrames) {
144+
startSharedBackendAnimation(
145+
folly::dynamic::object("type", "frames")(
146+
"frames", folly::dynamic::array(0.0, 0.25, 0.5, 0.75, 1.0))(
147+
"toValue", 100));
148+
149+
runBackendFrame(kStartTime);
150+
runBackendFrame(kStartTime + SingleFrameIntervalMs);
151+
const auto valueBeforeEvent = animatedValue();
152+
EXPECT_GT(valueBeforeEvent, 0);
153+
154+
// The event is handled later than the next frame's vsync time.
155+
dispatchScrollAt(kStartTime + SingleFrameIntervalMs * 3, 4);
156+
EXPECT_EQ(nodesManager_->getValue(scrollValueTag_), 4);
157+
EXPECT_EQ(animatedValue(), valueBeforeEvent);
158+
159+
runBackendFrame(kStartTime + SingleFrameIntervalMs * 2);
160+
EXPECT_GT(animatedValue(), valueBeforeEvent);
161+
}
162+
163+
TEST_F(EventAnimationDriverTests, sharedBackendEventDoesNotEndDecayAnimation) {
164+
startSharedBackendAnimation(
165+
folly::dynamic::object("type", "decay")("velocity", 1.0)(
166+
"deceleration", 0.998)("iterations", 1));
167+
168+
runBackendFrame(kStartTime);
169+
runBackendFrame(kStartTime + SingleFrameIntervalMs);
170+
dispatchScrollAt(kStartTime + SingleFrameIntervalMs * 2, 4);
171+
runBackendFrame(kStartTime + SingleFrameIntervalMs * 2);
172+
const auto valueAfterEvent = animatedValue();
173+
174+
runBackendFrame(kStartTime + SingleFrameIntervalMs * 3);
175+
EXPECT_GT(animatedValue(), valueAfterEvent);
176+
}
177+
70178
} // namespace facebook::react

‎scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2839,8 +2839,8 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
28392839
public bool hasManagedProps() const noexcept;
28402840
public bool isOnRenderThread() const noexcept;
28412841
public bool useSharedAnimatedBackend() const noexcept;
2842-
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
2843-
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
2842+
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
2843+
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
28442844
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;
28452845
public facebook::react::NativeAnimatedNodesManager& operator=(facebook::react::NativeAnimatedNodesManager&&) = delete;
28462846
public folly::dynamic getManagedProps(facebook::react::Tag tag) const noexcept;

‎scripts/cxx-api/api-snapshots/ReactAppleFrameworksCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5038,8 +5038,8 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
50385038
public bool hasManagedProps() const noexcept;
50395039
public bool isOnRenderThread() const noexcept;
50405040
public bool useSharedAnimatedBackend() const noexcept;
5041-
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
5042-
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
5041+
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
5042+
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
50435043
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;
50445044
public facebook::react::NativeAnimatedNodesManager& operator=(facebook::react::NativeAnimatedNodesManager&&) = delete;
50455045
public folly::dynamic getManagedProps(facebook::react::Tag tag) const noexcept;

‎scripts/cxx-api/api-snapshots/ReactCommonFrameworksCxx.api‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,8 +1703,8 @@ class facebook::react::NativeAnimatedNodesManager : public std::enable_shared_fr
17031703
public bool hasManagedProps() const noexcept;
17041704
public bool isOnRenderThread() const noexcept;
17051705
public bool useSharedAnimatedBackend() const noexcept;
1706-
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp);
1707-
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp);
1706+
public facebook::react::AnimationMutations onAnimationFrameForBackend(facebook::react::AnimatedPropsBuilder& propsBuilder, facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
1707+
public facebook::react::AnimationMutations pullAnimationMutations(facebook::react::AnimationTimestamp timestamp, bool stepAnimations = true);
17081708
public facebook::react::NativeAnimatedNodesManager& operator=(const facebook::react::NativeAnimatedNodesManager&) = delete;
17091709
public facebook::react::NativeAnimatedNodesManager& operator=(facebook::react::NativeAnimatedNodesManager&&) = delete;
17101710
public folly::dynamic getManagedProps(facebook::react::Tag tag) const noexcept;

0 commit comments

Comments
 (0)