From ca017f74acc0cdd99ba5ec684378a26aa1292dfb Mon Sep 17 00:00:00 2001 From: Wojciech Lewicki Date: Thu, 13 Jun 2024 16:27:40 +0200 Subject: [PATCH 1/3] fix: make it possible to add multiple mountingOverrideDelegates --- .../renderer/mounting/MountingCoordinator.cpp | 54 ++++++++++--------- .../renderer/mounting/MountingCoordinator.h | 4 +- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 77cc62690b9f10..9ae478dfbe07c7 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -104,31 +104,35 @@ std::optional MountingCoordinator::pullTransaction() } // Override case - auto mountingOverrideDelegate = mountingOverrideDelegate_.lock(); - auto shouldOverridePullTransaction = mountingOverrideDelegate && - mountingOverrideDelegate->shouldOverridePullTransaction(); - - if (shouldOverridePullTransaction) { - SystraceSection section2("MountingCoordinator::overridePullTransaction"); - - auto mutations = ShadowViewMutation::List{}; - auto telemetry = TransactionTelemetry{}; - - if (transaction.has_value()) { - mutations = transaction->getMutations(); - telemetry = transaction->getTelemetry(); - } else { - number_++; - telemetry.willLayout(); - telemetry.didLayout(); - telemetry.willCommit(); - telemetry.didCommit(); - telemetry.willDiff(); - telemetry.didDiff(); + for(auto it = mountingOverrideDelegates_.begin(); it < mountingOverrideDelegates_.end(); it++) + { + auto &delegate = *it; + auto mountingOverrideDelegate = delegate.lock(); + auto shouldOverridePullTransaction = mountingOverrideDelegate && + mountingOverrideDelegate->shouldOverridePullTransaction(); + + if (shouldOverridePullTransaction) { + SystraceSection section2("MountingCoordinator::overridePullTransaction"); + + auto mutations = ShadowViewMutation::List{}; + auto telemetry = TransactionTelemetry{}; + + if (transaction.has_value()) { + mutations = transaction->getMutations(); + telemetry = transaction->getTelemetry(); + } else { + number_++; + telemetry.willLayout(); + telemetry.didLayout(); + telemetry.willCommit(); + telemetry.didCommit(); + telemetry.willDiff(); + telemetry.didDiff(); + } + + transaction = mountingOverrideDelegate->pullTransaction( + surfaceId_, number_, telemetry, std::move(mutations)); } - - transaction = mountingOverrideDelegate->pullTransaction( - surfaceId_, number_, telemetry, std::move(mutations)); } #ifdef RN_SHADOW_TREE_INTROSPECTION @@ -203,7 +207,7 @@ ShadowTreeRevision MountingCoordinator::getBaseRevision() const { void MountingCoordinator::setMountingOverrideDelegate( std::weak_ptr delegate) const { std::scoped_lock lock(mutex_); - mountingOverrideDelegate_ = std::move(delegate); + mountingOverrideDelegates_.insert(mountingOverrideDelegates_.end(), std::move(delegate)); } } // namespace facebook::react diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h index 87078034076b6e..6b2bca898a8118 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -119,8 +119,8 @@ class MountingCoordinator final { mutable std::optional lastRevision_{}; mutable MountingTransaction::Number number_{0}; mutable std::condition_variable signal_; - mutable std::weak_ptr - mountingOverrideDelegate_; + mutable std::vector> + mountingOverrideDelegates_; TelemetryController telemetryController_; From c8693e4a4fa45f628accbe30f83ca4fbaab19159 Mon Sep 17 00:00:00 2001 From: Wojciech Lewicki Date: Thu, 13 Jun 2024 16:48:01 +0200 Subject: [PATCH 2/3] feat: suggestion from review --- .../react/renderer/mounting/MountingCoordinator.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 9ae478dfbe07c7..14944943b7c55d 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -104,9 +104,8 @@ std::optional MountingCoordinator::pullTransaction() } // Override case - for(auto it = mountingOverrideDelegates_.begin(); it < mountingOverrideDelegates_.end(); it++) + for(const auto &delegate : mountingOverrideDelegates_) { - auto &delegate = *it; auto mountingOverrideDelegate = delegate.lock(); auto shouldOverridePullTransaction = mountingOverrideDelegate && mountingOverrideDelegate->shouldOverridePullTransaction(); From 563c69aebf46a3abc42ab92e2bfb02100181c575 Mon Sep 17 00:00:00 2001 From: Wojciech Lewicki Date: Fri, 14 Jun 2024 12:42:19 +0200 Subject: [PATCH 3/3] chore: change from set to add --- packages/react-native/React/Fabric/RCTScheduler.mm | 2 +- .../ReactAndroid/src/main/jni/react/fabric/Binding.cpp | 4 ++-- .../react/renderer/mounting/MountingCoordinator.cpp | 2 +- .../ReactCommon/react/renderer/mounting/MountingCoordinator.h | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-native/React/Fabric/RCTScheduler.mm b/packages/react-native/React/Fabric/RCTScheduler.mm index 79d2b11e4dcc37..bd1d143ddbf6eb 100644 --- a/packages/react-native/React/Fabric/RCTScheduler.mm +++ b/packages/react-native/React/Fabric/RCTScheduler.mm @@ -172,7 +172,7 @@ - (const ComponentDescriptor *)findComponentDescriptorByHandle_DO_NOT_USE_THIS_I - (void)setupAnimationDriver:(const facebook::react::SurfaceHandler &)surfaceHandler { - surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate(_animationDriver); + surfaceHandler.getMountingCoordinator()->addMountingOverrideDelegate(_animationDriver); } - (void)onAnimationStarted diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp index 280ae463d33c84..70e57414fd6435 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/fabric/Binding.cpp @@ -129,7 +129,7 @@ void Binding::startSurface( surfaceHandler.start(); - surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate( + surfaceHandler.getMountingCoordinator()->addMountingOverrideDelegate( animationDriver_); { @@ -197,7 +197,7 @@ void Binding::startSurfaceWithConstraints( surfaceHandler.start(); - surfaceHandler.getMountingCoordinator()->setMountingOverrideDelegate( + surfaceHandler.getMountingCoordinator()->addMountingOverrideDelegate( animationDriver_); { diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp index 14944943b7c55d..339f81d41ffc0f 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp @@ -203,7 +203,7 @@ ShadowTreeRevision MountingCoordinator::getBaseRevision() const { return baseRevision_; } -void MountingCoordinator::setMountingOverrideDelegate( +void MountingCoordinator::addMountingOverrideDelegate( std::weak_ptr delegate) const { std::scoped_lock lock(mutex_); mountingOverrideDelegates_.insert(mountingOverrideDelegates_.end(), std::move(delegate)); diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h index 6b2bca898a8118..0759d2efabc2a8 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h +++ b/packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.h @@ -89,7 +89,7 @@ class MountingCoordinator final { void updateBaseRevision(const ShadowTreeRevision& baseRevision) const; void resetLatestRevision() const; - void setMountingOverrideDelegate( + void addMountingOverrideDelegate( std::weak_ptr delegate) const; /*