Skip to content

Commit 101f13a

Browse files
javachefacebook-github-bot
authored andcommitted
Split scheduler commit and flush delegate methods (#44188)
Summary: The current approach used for `batchRenderingUpdatesInEventLoop` is not compatible with Android due to limitations in its props processing model. The raw props changeset is passed through to Android, and must be available for the Android mounting layer to correctly apply changes. We have some logic to merge these payloads when multiple ShadowNode clones take place but were previously assuming that a ShadowTree commit was a safe state to synchronize. In the current implementation this means that two commits driven from layout effects (triggering states A → B → C) may cause Android to observe only the B → C props change, and miss out on any props changed in A → B. Changelog: [Android][Fixed] Cascading renders were not mounting correctly when `batchRenderingUpdatesInEventLoop` is enabled. Differential Revision: D56414689
1 parent d59de54 commit 101f13a

File tree

9 files changed

+82
-8
lines changed

9 files changed

+82
-8
lines changed

packages/react-native/React/Fabric/RCTScheduler.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ NS_ASSUME_NONNULL_BEGIN
2828

2929
- (void)schedulerDidFinishTransaction:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
3030

31+
- (void)schedulerShouldRenderTransactions:(facebook::react::MountingCoordinator::Shared)mountingCoordinator;
32+
3133
- (void)schedulerDidDispatchCommand:(const facebook::react::ShadowView &)shadowView
3234
commandName:(const std::string &)commandName
3335
args:(const folly::dynamic &)args;

packages/react-native/React/Fabric/RCTScheduler.mm

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@ void schedulerDidFinishTransaction(const MountingCoordinator::Shared &mountingCo
3131
[scheduler.delegate schedulerDidFinishTransaction:mountingCoordinator];
3232
}
3333

34+
void schedulerShouldRenderTransactions(const MountingCoordinator::Shared &mountingCoordinator) override
35+
{
36+
RCTScheduler *scheduler = (__bridge RCTScheduler *)scheduler_;
37+
[scheduler.delegate schedulerShouldRenderTransactions:mountingCoordinator];
38+
}
39+
3440
void schedulerDidRequestPreliminaryViewAllocation(SurfaceId surfaceId, const ShadowNode &shadowNode) override
3541
{
3642
// Does nothing.

packages/react-native/React/Fabric/RCTSurfacePresenter.mm

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,16 @@ - (void)_applicationWillTerminate
334334

335335
- (void)schedulerDidFinishTransaction:(MountingCoordinator::Shared)mountingCoordinator
336336
{
337-
[_mountingManager scheduleTransaction:mountingCoordinator];
337+
if (!ReactNativeFeatureFlags::batchRenderingUpdatesInEventLoop()) {
338+
[_mountingManager scheduleTransaction:mountingCoordinator];
339+
}
340+
}
341+
342+
- (void)schedulerShouldRenderTransactions:(MountingCoordinator::Shared)mountingCoordinator
343+
{
344+
if (ReactNativeFeatureFlags::batchRenderingUpdatesInEventLoop()) {
345+
[_mountingManager scheduleTransaction:mountingCoordinator];
346+
}
338347
}
339348

340349
- (void)schedulerDidDispatchCommand:(const ShadowView &)shadowView

packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -462,16 +462,38 @@ std::shared_ptr<FabricMountingManager> Binding::getMountingManager(
462462

463463
void Binding::schedulerDidFinishTransaction(
464464
const MountingCoordinator::Shared& mountingCoordinator) {
465-
auto mountingManager = getMountingManager("schedulerDidFinishTransaction");
466-
if (!mountingManager) {
465+
auto mountingTransaction = mountingCoordinator->pullTransaction();
466+
if (!mountingTransaction.has_value()) {
467467
return;
468468
}
469469

470-
auto mountingTransaction = mountingCoordinator->pullTransaction();
471-
if (!mountingTransaction.has_value()) {
470+
auto pendingTransaction = std::find_if(
471+
pendingTransactions_.begin(),
472+
pendingTransactions_.end(),
473+
[&](const auto& transaction) {
474+
return transaction.getSurfaceId() ==
475+
mountingTransaction->getSurfaceId();
476+
});
477+
478+
if (pendingTransaction != pendingTransactions_.end()) {
479+
pendingTransaction->mergeWith(std::move(*mountingTransaction));
480+
} else {
481+
pendingTransactions_.push_back(std::move(*mountingTransaction));
482+
}
483+
}
484+
485+
void Binding::schedulerShouldRenderTransactions(
486+
const MountingCoordinator::Shared& /*mountingCoordinator*/) {
487+
auto mountingManager =
488+
getMountingManager("schedulerShouldRenderTransactions");
489+
if (!mountingManager) {
472490
return;
473491
}
474-
mountingManager->executeMount(*mountingTransaction);
492+
493+
for (auto& transaction : pendingTransactions_) {
494+
mountingManager->executeMount(transaction);
495+
}
496+
pendingTransactions_.clear();
475497
}
476498

477499
void Binding::schedulerDidRequestPreliminaryViewAllocation(

packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,9 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
101101
void schedulerDidFinishTransaction(
102102
const MountingCoordinator::Shared& mountingCoordinator) override;
103103

104+
void schedulerShouldRenderTransactions(
105+
const MountingCoordinator::Shared& mountingCoordinator) override;
106+
104107
void schedulerDidRequestPreliminaryViewAllocation(
105108
const SurfaceId surfaceId,
106109
const ShadowNode& shadowNode) override;
@@ -146,6 +149,9 @@ class Binding : public jni::HybridClass<Binding, JBinding>,
146149
std::shared_mutex
147150
surfaceHandlerRegistryMutex_; // Protects `surfaceHandlerRegistry_`.
148151

152+
// Track pending transactions, one per surfaceId
153+
std::vector<MountingTransaction> pendingTransactions_;
154+
149155
float pointScaleFactor_ = 1;
150156

151157
std::shared_ptr<const ReactNativeConfig> reactNativeConfig_{nullptr};

packages/react-native/ReactCommon/react/renderer/mounting/MountingTransaction.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,17 @@ Number MountingTransaction::getNumber() const {
4141
return number_;
4242
}
4343

44+
void MountingTransaction::mergeWith(MountingTransaction&& transaction) {
45+
react_native_assert(transaction.getSurfaceId() == surfaceId_);
46+
number_ = transaction.getNumber();
47+
mutations_.insert(
48+
mutations_.end(),
49+
std::make_move_iterator(transaction.mutations_.begin()),
50+
std::make_move_iterator(transaction.mutations_.end()));
51+
52+
// TODO T186641819: Telemetry for merged transactions is not supported, use
53+
// the latest instance
54+
telemetry_ = std::move(transaction.telemetry_);
55+
}
56+
4457
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/mounting/MountingTransaction.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ class MountingTransaction final {
7676
*/
7777
Number getNumber() const;
7878

79+
/*
80+
* Merges the given transaction in the current transaction, so they
81+
* can be executed atomatically as a single transaction.
82+
*/
83+
void mergeWith(MountingTransaction&& transaction);
84+
7985
private:
8086
SurfaceId surfaceId_;
8187
Number number_;

packages/react-native/ReactCommon/react/renderer/scheduler/Scheduler.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,8 @@ void Scheduler::uiManagerDidFinishTransaction(
291291
SystraceSection s("Scheduler::uiManagerDidFinishTransaction");
292292

293293
if (delegate_ != nullptr) {
294+
delegate_->schedulerDidFinishTransaction(mountingCoordinator);
295+
294296
auto weakRuntimeScheduler =
295297
contextContainer_->find<std::weak_ptr<RuntimeScheduler>>(
296298
"RuntimeScheduler");
@@ -301,13 +303,14 @@ void Scheduler::uiManagerDidFinishTransaction(
301303
runtimeScheduler->scheduleRenderingUpdate(
302304
[delegate = delegate_,
303305
mountingCoordinator = std::move(mountingCoordinator)]() {
304-
delegate->schedulerDidFinishTransaction(mountingCoordinator);
306+
delegate->schedulerShouldRenderTransactions(mountingCoordinator);
305307
});
306308
} else {
307-
delegate_->schedulerDidFinishTransaction(mountingCoordinator);
309+
delegate_->schedulerShouldRenderTransactions(mountingCoordinator);
308310
}
309311
}
310312
}
313+
311314
void Scheduler::uiManagerDidCreateShadowNode(const ShadowNode& shadowNode) {
312315
SystraceSection s("Scheduler::uiManagerDidCreateShadowNode");
313316

packages/react-native/ReactCommon/react/renderer/scheduler/SchedulerDelegate.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ class SchedulerDelegate {
2828
virtual void schedulerDidFinishTransaction(
2929
const MountingCoordinator::Shared& mountingCoordinator) = 0;
3030

31+
/*
32+
* Called when the runtime scheduler decides that one-or-more previously
33+
* finished transactions should now be flushed to the screen (atomically).
34+
*/
35+
virtual void schedulerShouldRenderTransactions(
36+
const MountingCoordinator::Shared& mountingCoordinator) = 0;
37+
3138
/*
3239
* Called right after a new ShadowNode was created.
3340
*/

0 commit comments

Comments
 (0)