Skip to content

Commit 8e2b33f

Browse files
zeyapfacebook-github-bot
authored andcommitted
keep track of last direct manipulation props (#52842)
Summary: Pull Request resolved: #52842 ## Changelog: [Internal] [Fixed] - keep track of last direct manipulation props * so that at next time there's a ShadowTree commit (regardless of which thread), they're merged into the update mutation * the props will stay until the view is disconnected from props node via Animated API or the view is removed/deleted. This should be expected behavior, because in RN we expect that once Animated changes a prop, subsequent react commits should not change the value. Reviewed By: sammy-SC Differential Revision: D78702843 fbshipit-source-id: b5e6e01a7a4f6caeea4cc1eeafaef9c3c7e51691
1 parent f47da61 commit 8e2b33f

5 files changed

Lines changed: 71 additions & 26 deletions

File tree

packages/react-native/ReactCxxPlatform/react/renderer/animated/AnimatedMountingOverrideDelegate.cpp

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
#include "AnimatedMountingOverrideDelegate.h"
9+
#include "NativeAnimatedNodesManager.h"
910

1011
#include <react/renderer/componentregistry/ComponentDescriptorRegistry.h>
1112
#include <react/renderer/components/view/ViewProps.h>
@@ -16,14 +17,17 @@
1617
namespace facebook::react {
1718

1819
AnimatedMountingOverrideDelegate::AnimatedMountingOverrideDelegate(
19-
std::function<folly::dynamic(Tag)> getAnimatedManagedProps,
20+
NativeAnimatedNodesManager& animatedManager,
2021
const Scheduler& scheduler)
2122
: MountingOverrideDelegate(),
22-
getAnimatedManagedProps_(std::move(getAnimatedManagedProps)),
23+
animatedManager_(&animatedManager),
2324
scheduler_(&scheduler){};
2425

2526
bool AnimatedMountingOverrideDelegate::shouldOverridePullTransaction() const {
26-
return getAnimatedManagedProps_ != nullptr;
27+
if (animatedManager_ != nullptr) {
28+
return animatedManager_->hasManagedProps();
29+
}
30+
return false;
2731
}
2832

2933
std::optional<MountingTransaction>
@@ -32,19 +36,16 @@ AnimatedMountingOverrideDelegate::pullTransaction(
3236
MountingTransaction::Number transactionNumber,
3337
const TransactionTelemetry& telemetry,
3438
ShadowViewMutationList mutations) const {
35-
if (!getAnimatedManagedProps_) {
36-
return MountingTransaction{
37-
surfaceId, transactionNumber, std::move(mutations), telemetry};
38-
}
39-
4039
std::unordered_map<Tag, folly::dynamic> animatedManagedProps;
4140
for (const auto& mutation : mutations) {
4241
if (mutation.type == ShadowViewMutation::Update) {
4342
const auto tag = mutation.newChildShadowView.tag;
44-
auto props = getAnimatedManagedProps_(tag);
43+
auto props = animatedManager_->managedProps(tag);
4544
if (!props.isNull()) {
4645
animatedManagedProps.insert({tag, std::move(props)});
4746
}
47+
} else if (mutation.type == ShadowViewMutation::Delete) {
48+
animatedManager_->onManagedPropsRemoved(mutation.oldChildShadowView.tag);
4849
}
4950
}
5051

packages/react-native/ReactCxxPlatform/react/renderer/animated/AnimatedMountingOverrideDelegate.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,12 @@
1717
namespace facebook::react {
1818

1919
class Scheduler;
20+
class NativeAnimatedNodesManager;
2021

2122
class AnimatedMountingOverrideDelegate : public MountingOverrideDelegate {
2223
public:
2324
AnimatedMountingOverrideDelegate(
24-
std::function<folly::dynamic(Tag)> getAnimatedManagedProps,
25+
NativeAnimatedNodesManager& animatedManager,
2526
const Scheduler& scheduler);
2627

2728
bool shouldOverridePullTransaction() const override;
@@ -33,7 +34,7 @@ class AnimatedMountingOverrideDelegate : public MountingOverrideDelegate {
3334
ShadowViewMutationList mutations) const override;
3435

3536
private:
36-
std::function<folly::dynamic(Tag)> getAnimatedManagedProps_;
37+
mutable NativeAnimatedNodesManager* animatedManager_;
3738

3839
const Scheduler* scheduler_;
3940
};

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

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,8 @@ void NativeAnimatedNodesManager::disconnectAnimatedNodeFromView(
201201
connectedAnimatedNodes_.erase(viewTag);
202202
}
203203
updatedNodeTags_.insert(node->tag());
204+
205+
onManagedPropsRemoved(viewTag);
204206
} else {
205207
LOG(WARNING)
206208
<< "Cannot DisconnectAnimatedNodeToView, animated node has to be props type";
@@ -711,18 +713,50 @@ bool NativeAnimatedNodesManager::onAnimationFrame(double timestamp) {
711713
return commitProps();
712714
}
713715

714-
folly::dynamic NativeAnimatedNodesManager::managedProps(Tag tag) noexcept {
716+
folly::dynamic NativeAnimatedNodesManager::managedProps(
717+
Tag tag) const noexcept {
715718
std::lock_guard<std::mutex> lock(connectedAnimatedNodesMutex_);
716-
const auto iter = connectedAnimatedNodes_.find(tag);
717-
if (iter != connectedAnimatedNodes_.end()) {
719+
if (const auto iter = connectedAnimatedNodes_.find(tag);
720+
iter != connectedAnimatedNodes_.end()) {
718721
if (const auto node = getAnimatedNode<PropsAnimatedNode>(iter->second)) {
719722
return node->props();
720723
}
724+
} else {
725+
std::lock_guard<std::mutex> lockUnsyncedDirectViewProps(
726+
unsyncedDirectViewPropsMutex_);
727+
if (auto it = unsyncedDirectViewProps_.find(tag);
728+
it != unsyncedDirectViewProps_.end()) {
729+
return it->second;
730+
}
721731
}
722732

723733
return nullptr;
724734
}
725735

736+
bool NativeAnimatedNodesManager::hasManagedProps() const noexcept {
737+
{
738+
std::lock_guard<std::mutex> lock(connectedAnimatedNodesMutex_);
739+
if (!connectedAnimatedNodes_.empty()) {
740+
return true;
741+
}
742+
}
743+
{
744+
std::lock_guard<std::mutex> lock(unsyncedDirectViewPropsMutex_);
745+
if (!unsyncedDirectViewProps_.empty()) {
746+
return true;
747+
}
748+
}
749+
return false;
750+
}
751+
752+
void NativeAnimatedNodesManager::onManagedPropsRemoved(Tag tag) noexcept {
753+
std::lock_guard<std::mutex> lock(unsyncedDirectViewPropsMutex_);
754+
if (auto iter = unsyncedDirectViewProps_.find(tag);
755+
iter != unsyncedDirectViewProps_.end()) {
756+
unsyncedDirectViewProps_.erase(iter);
757+
}
758+
}
759+
726760
bool NativeAnimatedNodesManager::isOnRenderThread() const noexcept {
727761
return isOnRenderThread_;
728762
}
@@ -771,6 +805,10 @@ void NativeAnimatedNodesManager::schedulePropsCommit(
771805
mergeObjects(updateViewPropsDirect_[viewTag], props);
772806
} else if (directManipulationCallback_ != nullptr) {
773807
mergeObjects(updateViewPropsDirect_[viewTag], props);
808+
{
809+
std::lock_guard<std::mutex> lock(unsyncedDirectViewPropsMutex_);
810+
mergeObjects(unsyncedDirectViewProps_[viewTag], props);
811+
}
774812
}
775813
}
776814

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

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,11 @@ class NativeAnimatedNodesManager {
175175
void updateNodes(
176176
const std::set<int>& finishedAnimationValueNodes = {}) noexcept;
177177

178-
folly::dynamic managedProps(Tag tag) noexcept;
178+
folly::dynamic managedProps(Tag tag) const noexcept;
179+
180+
bool hasManagedProps() const noexcept;
181+
182+
void onManagedPropsRemoved(Tag tag) noexcept;
179183

180184
bool isOnRenderThread() const noexcept;
181185

@@ -209,7 +213,7 @@ class NativeAnimatedNodesManager {
209213
eventDrivers_;
210214
std::unordered_set<Tag> updatedNodeTags_;
211215

212-
std::mutex connectedAnimatedNodesMutex_;
216+
mutable std::mutex connectedAnimatedNodesMutex_;
213217

214218
std::mutex uiTasksMutex_;
215219
std::vector<UiTask> operations_;
@@ -238,6 +242,16 @@ class NativeAnimatedNodesManager {
238242
std::unordered_map<Tag, folly::dynamic> updateViewProps_{};
239243
std::unordered_map<Tag, folly::dynamic> updateViewPropsDirect_{};
240244

245+
/*
246+
* Sometimes a view is not longer connected to a PropsAnimatedNode, but
247+
* NativeAnimated has previously changed the view's props via direct
248+
* manipulation, we use unsyncedDirectViewProps_ to keep track of those
249+
* props, to make sure later Fabric commits will not override direct
250+
* manipulation result on this view.
251+
*/
252+
mutable std::mutex unsyncedDirectViewPropsMutex_;
253+
std::unordered_map<Tag, folly::dynamic> unsyncedDirectViewProps_{};
254+
241255
int animatedGraphBFSColor_ = 0;
242256
#ifdef REACT_NATIVE_DEBUG
243257
bool warnedAboutGraphTraversal_ = false;

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

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -88,16 +88,7 @@ NativeAnimatedNodesManagerProvider::getOrCreate(jsi::Runtime& runtime) {
8888
auto* scheduler = (Scheduler*)uiManager->getDelegate();
8989
animatedMountingOverrideDelegate_ =
9090
std::make_shared<AnimatedMountingOverrideDelegate>(
91-
[nativeAnimatedNodesManager =
92-
std::weak_ptr<NativeAnimatedNodesManager>(
93-
nativeAnimatedNodesManager_)](Tag tag) -> folly::dynamic {
94-
if (auto nativeAnimatedNodesManagerStrong =
95-
nativeAnimatedNodesManager.lock()) {
96-
return nativeAnimatedNodesManagerStrong->managedProps(tag);
97-
}
98-
return nullptr;
99-
},
100-
*scheduler);
91+
*nativeAnimatedNodesManager_, *scheduler);
10192

10293
// Register on existing surfaces
10394
uiManager->getShadowTreeRegistry().enumerate(

0 commit comments

Comments
 (0)