Skip to content

Commit 43f464b

Browse files
zeyapmeta-codesync[bot]
authored andcommitted
pass down isAsync arg to start/stopOnRenderCallback to indicate thread (#54252)
Summary: Pull Request resolved: #54252 ## Changelog: [Internal] [Added] - pass down isAsync arg to start/stopOnRenderCallback to indicate thread make it more explicit where start/stopOnRenderCallback is invoked so we can handle it on each platform in a more thread safe way Reviewed By: lenaic Differential Revision: D85365058 fbshipit-source-id: 5f0ee1343547fe3466fcd23282e0f27e129a2e7d
1 parent 642f086 commit 43f464b

File tree

9 files changed

+90
-59
lines changed

9 files changed

+90
-59
lines changed

packages/react-native/ReactApple/RCTAnimatedModuleProvider/RCTAnimatedModuleProvider.mm

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,30 +40,49 @@ - (void)_onDisplayLinkTick
4040
if (name == facebook::react::AnimatedModule::kModuleName) {
4141
__weak RCTAnimatedModuleProvider *weakSelf = self;
4242
auto provider = std::make_shared<facebook::react::NativeAnimatedNodesManagerProvider>(
43-
[weakSelf](std::function<void()> &&onRender) {
44-
RCTAnimatedModuleProvider *strongSelf = weakSelf;
45-
if (strongSelf) {
46-
strongSelf->_onRender = onRender;
47-
if (strongSelf->_displayLink == nil) {
43+
[weakSelf](std::function<void()> &&onRender, bool isAsync) {
44+
const auto start_render = [weakSelf, onRender]() {
45+
RCTAnimatedModuleProvider *strongSelf = weakSelf;
46+
if (strongSelf) {
47+
strongSelf->_onRender = onRender;
48+
if (strongSelf->_displayLink == nil) {
4849
#if TARGET_OS_OSX
49-
strongSelf->_displayLink = [RCTPlatformDisplayLink displayLinkWithTarget:strongSelf
50-
selector:@selector(_onDisplayLinkTick)];
50+
strongSelf->_displayLink =
51+
[RCTPlatformDisplayLink displayLinkWithTarget:strongSelf selector:@selector(_onDisplayLinkTick)];
5152
#else
52-
strongSelf->_displayLink = [CADisplayLink displayLinkWithTarget:strongSelf
53-
selector:@selector(_onDisplayLinkTick)];
53+
strongSelf->_displayLink = [CADisplayLink displayLinkWithTarget:strongSelf
54+
selector:@selector(_onDisplayLinkTick)];
5455
#endif
55-
[strongSelf->_displayLink addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
56+
[strongSelf->_displayLink addToRunLoop:[NSRunLoop mainRunLoop] forMode:NSRunLoopCommonModes];
57+
}
5658
}
59+
};
60+
if (isAsync) {
61+
dispatch_async(dispatch_get_main_queue(), ^{
62+
start_render();
63+
});
64+
} else {
65+
start_render();
5766
}
5867
},
59-
[weakSelf]() {
60-
RCTAnimatedModuleProvider *strongSelf = weakSelf;
61-
if (strongSelf) {
62-
if (strongSelf->_displayLink != nil) {
63-
[strongSelf->_displayLink invalidate];
64-
strongSelf->_displayLink = nil;
65-
strongSelf->_onRender = nullptr;
68+
[weakSelf](bool isAsync) {
69+
const auto stop_render = [weakSelf]() {
70+
RCTAnimatedModuleProvider *strongSelf = weakSelf;
71+
if (strongSelf) {
72+
if (strongSelf->_displayLink != nil) {
73+
[strongSelf->_displayLink invalidate];
74+
strongSelf->_displayLink = nil;
75+
strongSelf->_onRender = nullptr;
76+
}
6677
}
78+
};
79+
80+
if (isAsync) {
81+
dispatch_async(dispatch_get_main_queue(), ^{
82+
stop_render();
83+
});
84+
} else {
85+
stop_render();
6786
}
6887
});
6988
return std::make_shared<facebook::react::AnimatedModule>(std::move(jsInvoker), std::move(provider));

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ NativeAnimatedNodesManager::NativeAnimatedNodesManager(
100100
: animationBackend_(animationBackend) {}
101101

102102
NativeAnimatedNodesManager::~NativeAnimatedNodesManager() noexcept {
103-
stopRenderCallbackIfNeeded();
103+
stopRenderCallbackIfNeeded(true);
104104
}
105105

106106
std::optional<double> NativeAnimatedNodesManager::getValue(Tag tag) noexcept {
@@ -491,7 +491,7 @@ void NativeAnimatedNodesManager::handleAnimatedEvent(
491491
// when there are no active animations. Calling
492492
// `startRenderCallbackIfNeeded` will call platform specific code to
493493
// register UI tick listener.
494-
startRenderCallbackIfNeeded();
494+
startRenderCallbackIfNeeded(false);
495495
// Calling startOnRenderCallback_ will register a UI tick listener.
496496
// The UI ticker listener will not be called until the next frame.
497497
// That's why, in case this is called from the UI thread, we need to
@@ -516,12 +516,14 @@ NativeAnimatedNodesManager::ensureEventEmitterListener() noexcept {
516516
return eventEmitterListener_;
517517
}
518518

519-
void NativeAnimatedNodesManager::startRenderCallbackIfNeeded() {
519+
void NativeAnimatedNodesManager::startRenderCallbackIfNeeded(bool isAsync) {
520520
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
521521
#ifdef RN_USE_ANIMATION_BACKEND
522522
if (auto animationBackend = animationBackend_.lock()) {
523523
std::static_pointer_cast<AnimationBackend>(animationBackend)
524-
->start([this](float /*f*/) { return pullAnimationMutations(); });
524+
->start(
525+
[this](float /*f*/) { return pullAnimationMutations(); },
526+
isAsync);
525527
}
526528
#endif
527529

@@ -539,14 +541,15 @@ void NativeAnimatedNodesManager::startRenderCallbackIfNeeded() {
539541
}
540542

541543
if (startOnRenderCallback_) {
542-
startOnRenderCallback_();
544+
startOnRenderCallback_(isAsync);
543545
}
544546
}
545547

546-
void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded() noexcept {
548+
void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded(
549+
bool isAsync) noexcept {
547550
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
548551
if (auto animationBackend = animationBackend_.lock()) {
549-
animationBackend->stop();
552+
animationBackend->stop(isAsync);
550553
}
551554
return;
552555
}
@@ -558,7 +561,7 @@ void NativeAnimatedNodesManager::stopRenderCallbackIfNeeded() noexcept {
558561

559562
if (isRenderCallbackStarted) {
560563
if (stopOnRenderCallback_) {
561-
stopOnRenderCallback_();
564+
stopOnRenderCallback_(isAsync);
562565
}
563566
}
564567
}
@@ -1008,7 +1011,7 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
10081011
}
10091012
} else {
10101013
// There is no active animation. Stop the render callback.
1011-
stopRenderCallbackIfNeeded();
1014+
stopRenderCallbackIfNeeded(false);
10121015
}
10131016
return mutations;
10141017
}
@@ -1083,7 +1086,7 @@ void NativeAnimatedNodesManager::onRender() {
10831086
}
10841087
} else {
10851088
// There is no active animation. Stop the render callback.
1086-
stopRenderCallbackIfNeeded();
1089+
stopRenderCallbackIfNeeded(false);
10871090
}
10881091
}
10891092

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ class NativeAnimatedNodesManager {
6161
std::function<void(Tag, const folly::dynamic&)>;
6262
using FabricCommitCallback =
6363
std::function<void(std::unordered_map<Tag, folly::dynamic>&)>;
64-
using StartOnRenderCallback = std::function<void()>;
65-
using StopOnRenderCallback = std::function<void()>;
64+
using StartOnRenderCallback = std::function<void(bool isAsync)>;
65+
using StopOnRenderCallback = std::function<void(bool isAsync)>;
6666

6767
explicit NativeAnimatedNodesManager(
6868
DirectManipulationCallback&& directManipulationCallback,
@@ -183,12 +183,12 @@ class NativeAnimatedNodesManager {
183183
// Whenever a batch is flushed to the UI thread, start the onRender
184184
// callbacks to guarantee they run at least once. E.g., to execute
185185
// setValue calls.
186-
startRenderCallbackIfNeeded();
186+
startRenderCallbackIfNeeded(true);
187187
}
188188

189189
void onRender();
190190

191-
void startRenderCallbackIfNeeded();
191+
void startRenderCallbackIfNeeded(bool isAsync);
192192

193193
void updateNodes(
194194
const std::set<int>& finishedAnimationValueNodes = {}) noexcept;
@@ -202,7 +202,7 @@ class NativeAnimatedNodesManager {
202202
bool isOnRenderThread() const noexcept;
203203

204204
private:
205-
void stopRenderCallbackIfNeeded() noexcept;
205+
void stopRenderCallbackIfNeeded(bool isAsync) noexcept;
206206

207207
bool onAnimationFrame(double timestamp);
208208

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManagerProvider.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ NativeAnimatedNodesManagerProvider::NativeAnimatedNodesManagerProvider(
4242
frameRateListenerCallback_(std::move(frameRateListenerCallback)),
4343
startOnRenderCallback_(std::move(startOnRenderCallback)) {
4444
if (frameRateListenerCallback_) {
45-
stopOnRenderCallback_ = [this, stopOnRenderCallback]() {
45+
stopOnRenderCallback_ = [this, stopOnRenderCallback](bool isAsync) {
4646
if (stopOnRenderCallback) {
47-
stopOnRenderCallback();
47+
stopOnRenderCallback(isAsync);
4848
}
4949
if (frameRateListenerCallback_) {
5050
frameRateListenerCallback_(false);
@@ -106,17 +106,19 @@ NativeAnimatedNodesManagerProvider::getOrCreate(
106106

107107
uiManager->unstable_setAnimationBackend(animationBackend_);
108108
} else {
109-
auto startOnRenderCallback = [this,
110-
startOnRenderCallbackFn =
111-
std::move(startOnRenderCallback_)]() {
112-
if (startOnRenderCallbackFn) {
113-
startOnRenderCallbackFn([this]() {
114-
if (nativeAnimatedDelegate_) {
115-
nativeAnimatedDelegate_->runAnimationFrame();
109+
auto startOnRenderCallback =
110+
[this, startOnRenderCallbackFn = std::move(startOnRenderCallback_)](
111+
bool isAsync) {
112+
if (startOnRenderCallbackFn) {
113+
startOnRenderCallbackFn(
114+
[this]() {
115+
if (nativeAnimatedDelegate_) {
116+
nativeAnimatedDelegate_->runAnimationFrame();
117+
}
118+
},
119+
isAsync);
116120
}
117-
});
118-
}
119-
};
121+
};
120122
nativeAnimatedNodesManager_ =
121123
std::make_shared<NativeAnimatedNodesManager>(
122124
std::move(directManipulationCallback),

packages/react-native/ReactCommon/react/renderer/animated/NativeAnimatedNodesManagerProvider.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ class NativeAnimatedNodesManagerProvider {
1919
public:
2020
using FrameRateListenerCallback =
2121
std::function<void(bool /* shouldEnableListener */)>;
22-
using StartOnRenderCallback = std::function<void(std::function<void()>&&)>;
22+
// when isAsync is true, it means StartOnRenderCallback is invoked from js
23+
// thread, otherwise from main thread
24+
using StartOnRenderCallback =
25+
std::function<void(std::function<void()>&&, bool /* isAsync */)>;
2326
using StopOnRenderCallback = NativeAnimatedNodesManager::StopOnRenderCallback;
2427

2528
NativeAnimatedNodesManagerProvider(
@@ -49,6 +52,7 @@ class NativeAnimatedNodesManagerProvider {
4952
animatedMountingOverrideDelegate_;
5053

5154
FrameRateListenerCallback frameRateListenerCallback_;
55+
5256
StartOnRenderCallback startOnRenderCallback_;
5357
StopOnRenderCallback stopOnRenderCallback_;
5458

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.cpp

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,18 @@ void AnimationBackend::onAnimationFrame(double timestamp) {
105105
}
106106
}
107107

108-
void AnimationBackend::start(const Callback& callback) {
108+
void AnimationBackend::start(const Callback& callback, bool isAsync) {
109109
callbacks.push_back(callback);
110110
// TODO: startOnRenderCallback_ should provide the timestamp from the platform
111-
startOnRenderCallback_([this]() {
112-
onAnimationFrame(
113-
std::chrono::steady_clock::now().time_since_epoch().count() / 1000);
114-
});
111+
startOnRenderCallback_(
112+
[this]() {
113+
onAnimationFrame(
114+
std::chrono::steady_clock::now().time_since_epoch().count() / 1000);
115+
},
116+
isAsync);
115117
}
116-
void AnimationBackend::stop() {
117-
stopOnRenderCallback_();
118+
void AnimationBackend::stop(bool isAsync) {
119+
stopOnRenderCallback_(isAsync);
118120
callbacks.clear();
119121
}
120122

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,9 @@ using AnimationMutations = std::vector<AnimationMutation>;
2929
class AnimationBackend : public UIManagerAnimationBackend {
3030
public:
3131
using Callback = std::function<AnimationMutations(float)>;
32-
using StartOnRenderCallback = std::function<void(std::function<void()>&&)>;
33-
using StopOnRenderCallback = std::function<void()>;
32+
using StartOnRenderCallback =
33+
std::function<void(std::function<void()>&&, bool /* isAsync */)>;
34+
using StopOnRenderCallback = std::function<void(bool /* isAsync */)>;
3435
using DirectManipulationCallback =
3536
std::function<void(Tag, const folly::dynamic&)>;
3637
using FabricCommitCallback =
@@ -56,7 +57,7 @@ class AnimationBackend : public UIManagerAnimationBackend {
5657
const std::unordered_map<Tag, AnimatedProps>& updates);
5758

5859
void onAnimationFrame(double timestamp) override;
59-
void start(const Callback& callback);
60-
void stop() override;
60+
void start(const Callback& callback, bool isAsync);
61+
void stop(bool isAsync) override;
6162
};
6263
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/uimanager/UIManagerAnimationBackend.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class UIManagerAnimationBackend {
1818

1919
virtual void onAnimationFrame(double timestamp) = 0;
2020
// TODO: T240293839 Move over start() function and mutation types
21-
virtual void stop() = 0;
21+
virtual void stop(bool isAsync) = 0;
2222
};
2323

2424
} // namespace facebook::react

private/react-native-fantom/tester/src/TesterAppDelegate.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,10 @@ TesterAppDelegate::TesterAppDelegate(
110110
g_setNativeAnimatedNowTimestampFunction(StubClock::now);
111111

112112
auto provider = std::make_shared<NativeAnimatedNodesManagerProvider>(
113-
[this](std::function<void()>&& onRender) {
113+
[this](std::function<void()>&& onRender, bool /*isAsync*/) {
114114
onAnimationRender_ = std::move(onRender);
115115
},
116-
[this]() { onAnimationRender_ = nullptr; });
116+
[this](bool /*isAsync*/) { onAnimationRender_ = nullptr; });
117117

118118
reactHost_ = std::make_unique<ReactHost>(
119119
reactInstanceConfig,

0 commit comments

Comments
 (0)