Skip to content

Commit 04164e6

Browse files
Bartlomiej Bloniarzfacebook-github-bot
Bartlomiej Bloniarz
authored and
facebook-github-bot
committed
Move ShadowNodeFamily to PropsAnimatedNode (#54879)
Summary: This diff is a part of the process of getting the Animated-itest to work with Animation Backend. During testing I found that sometimes the disconnect method would cleanup `tagToShadowNodeFamily_` when there were more than one PropsAnimatedNode for a view, so we would wrongly skip an animation. By storing the family pointer on the props node we can avoid that. Differential Revision: D89042963
1 parent 89aeabc commit 04164e6

File tree

4 files changed

+94
-76
lines changed

4 files changed

+94
-76
lines changed

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

Lines changed: 60 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -244,8 +244,7 @@ void NativeAnimatedNodesManager::connectAnimatedNodeToShadowNodeFamily(
244244
react_native_assert(propsNodeTag);
245245
auto node = getAnimatedNode<PropsAnimatedNode>(propsNodeTag);
246246
if (node != nullptr && family != nullptr) {
247-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
248-
tagToShadowNodeFamily_[family->getTag()] = family;
247+
node->connectToShadowNodeFamily(family);
249248
} else {
250249
LOG(WARNING)
251250
<< "Cannot ConnectAnimatedNodeToShadowNodeFamily, animated node has to be props type";
@@ -265,10 +264,6 @@ void NativeAnimatedNodesManager::disconnectAnimatedNodeFromView(
265264
std::lock_guard<std::mutex> lock(connectedAnimatedNodesMutex_);
266265
connectedAnimatedNodes_.erase(viewTag);
267266
}
268-
{
269-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
270-
tagToShadowNodeFamily_.erase(viewTag);
271-
}
272267
updatedNodeTags_.insert(node->tag());
273268

274269
onManagedPropsRemoved(viewTag);
@@ -907,16 +902,17 @@ void NativeAnimatedNodesManager::schedulePropsCommit(
907902
Tag viewTag,
908903
const folly::dynamic& props,
909904
bool layoutStyleUpdated,
910-
bool forceFabricCommit) noexcept {
905+
bool forceFabricCommit,
906+
ShadowNodeFamily::Weak shadowNodeFamily) noexcept {
911907
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
912908
if (forceFabricCommit) {
913909
shouldRequestAsyncFlush_ = true;
914910
}
915-
if (layoutStyleUpdated) {
916-
mergeObjects(updateViewProps_[viewTag], props);
917-
} else {
918-
mergeObjects(updateViewPropsDirect_[viewTag], props);
919-
}
911+
auto& current = layoutStyleUpdated
912+
? updateViewPropsForBackend_[viewTag]
913+
: updateViewPropsDirectForBackend_[viewTag];
914+
current.first = std::move(shadowNodeFamily);
915+
mergeObjects(current.second, props);
920916
return;
921917
}
922918

@@ -1010,38 +1006,36 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
10101006
}
10111007
}
10121008

1013-
{
1014-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
1015-
for (auto& [tag, props] : updateViewPropsDirect_) {
1016-
auto weakFamily = tagToShadowNodeFamily_[tag];
1017-
1018-
if (auto family = weakFamily.lock()) {
1019-
propsBuilder.storeDynamic(props);
1020-
mutations.batch.push_back(
1021-
AnimationMutation{
1022-
.tag = tag,
1023-
.family = family,
1024-
.props = propsBuilder.get(),
1025-
});
1026-
}
1027-
containsChange = true;
1009+
for (auto& [tag, update] : updateViewPropsDirectForBackend_) {
1010+
auto weakFamily = update.first;
1011+
1012+
if (auto family = weakFamily.lock()) {
1013+
propsBuilder.storeDynamic(update.second);
1014+
mutations.batch.push_back(
1015+
AnimationMutation{
1016+
.tag = tag,
1017+
.family = family,
1018+
.props = propsBuilder.get(),
1019+
});
10281020
}
1029-
for (auto& [tag, props] : updateViewProps_) {
1030-
auto weakFamily = tagToShadowNodeFamily_[tag];
1031-
1032-
if (auto family = weakFamily.lock()) {
1033-
propsBuilder.storeDynamic(props);
1034-
mutations.batch.push_back(
1035-
AnimationMutation{
1036-
.tag = tag,
1037-
.family = family,
1038-
.props = propsBuilder.get(),
1039-
.hasLayoutUpdates = true,
1040-
});
1041-
}
1042-
containsChange = true;
1021+
containsChange = true;
1022+
}
1023+
for (auto& [tag, update] : updateViewPropsForBackend_) {
1024+
auto weakFamily = update.first;
1025+
1026+
if (auto family = weakFamily.lock()) {
1027+
propsBuilder.storeDynamic(update.second);
1028+
mutations.batch.push_back(
1029+
AnimationMutation{
1030+
.tag = tag,
1031+
.family = family,
1032+
.props = propsBuilder.get(),
1033+
.hasLayoutUpdates = true,
1034+
});
10431035
}
1036+
containsChange = true;
10441037
}
1038+
10451039
if (containsChange) {
10461040
updateViewPropsDirect_.clear();
10471041
updateViewProps_.clear();
@@ -1071,36 +1065,34 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
10711065

10721066
isEventAnimationInProgress_ = false;
10731067

1074-
{
1075-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
1076-
for (auto& [tag, props] : updateViewPropsDirect_) {
1077-
auto weakFamily = tagToShadowNodeFamily_[tag];
1078-
1079-
if (auto family = weakFamily.lock()) {
1080-
propsBuilder.storeDynamic(props);
1081-
mutations.batch.push_back(
1082-
AnimationMutation{
1083-
.tag = tag,
1084-
.family = family,
1085-
.props = propsBuilder.get(),
1086-
});
1087-
}
1068+
for (auto& [tag, update] : updateViewPropsDirectForBackend_) {
1069+
auto weakFamily = update.first;
1070+
1071+
if (auto family = weakFamily.lock()) {
1072+
propsBuilder.storeDynamic(update.second);
1073+
mutations.batch.push_back(
1074+
AnimationMutation{
1075+
.tag = tag,
1076+
.family = family,
1077+
.props = propsBuilder.get(),
1078+
});
10881079
}
1089-
for (auto& [tag, props] : updateViewProps_) {
1090-
auto weakFamily = tagToShadowNodeFamily_[tag];
1091-
1092-
if (auto family = weakFamily.lock()) {
1093-
propsBuilder.storeDynamic(props);
1094-
mutations.batch.push_back(
1095-
AnimationMutation{
1096-
.tag = tag,
1097-
.family = family,
1098-
.props = propsBuilder.get(),
1099-
.hasLayoutUpdates = true,
1100-
});
1101-
}
1080+
}
1081+
for (auto& [tag, update] : updateViewPropsForBackend_) {
1082+
auto weakFamily = update.first;
1083+
1084+
if (auto family = weakFamily.lock()) {
1085+
propsBuilder.storeDynamic(update.second);
1086+
mutations.batch.push_back(
1087+
AnimationMutation{
1088+
.tag = tag,
1089+
.family = family,
1090+
.props = propsBuilder.get(),
1091+
.hasLayoutUpdates = true,
1092+
});
11021093
}
11031094
}
1095+
11041096
updateViewProps_.clear();
11051097
updateViewPropsDirect_.clear();
11061098
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ class NativeAnimatedNodesManager {
153153
Tag viewTag,
154154
const folly::dynamic &props,
155155
bool layoutStyleUpdated,
156-
bool forceFabricCommit) noexcept;
156+
bool forceFabricCommit,
157+
ShadowNodeFamily::Weak shadowNodeFamily = {}) noexcept;
157158

158159
/**
159160
* Commits all pending animated property updates to their respective views.
@@ -260,11 +261,10 @@ class NativeAnimatedNodesManager {
260261

261262
std::unordered_map<Tag, folly::dynamic> updateViewProps_{};
262263
std::unordered_map<Tag, folly::dynamic> updateViewPropsDirect_{};
264+
std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsForBackend_{};
265+
std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsDirectForBackend_{};
263266
bool shouldRequestAsyncFlush_{false};
264267

265-
mutable std::mutex tagToShadowNodeFamilyMutex_;
266-
std::unordered_map<Tag, std::weak_ptr<const ShadowNodeFamily>> tagToShadowNodeFamily_{};
267-
268268
/*
269269
* Sometimes a view is not longer connected to a PropsAnimatedNode, but
270270
* NativeAnimated has previously changed the view's props via direct

‎packages/react-native/ReactCommon/react/renderer/animated/nodes/PropsAnimatedNode.cpp‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ PropsAnimatedNode::PropsAnimatedNode(
5555
}
5656
}
5757

58+
void PropsAnimatedNode::connectToShadowNodeFamily(
59+
ShadowNodeFamily::Weak shadowNodeFamily) {
60+
shadowNodeFamily_ = std::move(shadowNodeFamily);
61+
}
62+
5863
void PropsAnimatedNode::connectToView(Tag viewTag) {
5964
react_native_assert(
6065
connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier &&
@@ -67,15 +72,25 @@ void PropsAnimatedNode::disconnectFromView(Tag viewTag) {
6772
connectedViewTag_ == viewTag &&
6873
"Attempting to disconnect view that has not been connected with the given animated node.");
6974
connectedViewTag_ = animated::undefinedAnimatedNodeIdentifier;
75+
shadowNodeFamily_.reset();
7076
}
7177

7278
// restore the value to whatever the value was on the ShadowNode instead of in
7379
// the View hierarchy
7480
void PropsAnimatedNode::restoreDefaultValues() {
7581
// If node is already disconnected from View, we cannot restore default values
7682
if (connectedViewTag_ != animated::undefinedAnimatedNodeIdentifier) {
77-
manager_->schedulePropsCommit(
78-
connectedViewTag_, folly::dynamic::object(), false, false);
83+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
84+
manager_->schedulePropsCommit(
85+
connectedViewTag_,
86+
folly::dynamic::object(),
87+
false,
88+
false,
89+
shadowNodeFamily_);
90+
} else {
91+
manager_->schedulePropsCommit(
92+
connectedViewTag_, folly::dynamic::object(), false, false);
93+
}
7994
}
8095
}
8196

@@ -147,8 +162,17 @@ void PropsAnimatedNode::update(bool forceFabricCommit) {
147162

148163
layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
149164

150-
manager_->schedulePropsCommit(
151-
connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
165+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
166+
manager_->schedulePropsCommit(
167+
connectedViewTag_,
168+
props_,
169+
layoutStyleUpdated_,
170+
forceFabricCommit,
171+
shadowNodeFamily_);
172+
} else {
173+
manager_->schedulePropsCommit(
174+
connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
175+
}
152176
}
153177

154178
} // namespace facebook::react

‎packages/react-native/ReactCommon/react/renderer/animated/nodes/PropsAnimatedNode.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ namespace facebook::react {
2121
class PropsAnimatedNode final : public AnimatedNode {
2222
public:
2323
PropsAnimatedNode(Tag tag, const folly::dynamic &config, NativeAnimatedNodesManager &manager);
24+
void connectToShadowNodeFamily(ShadowNodeFamily::Weak shadowNodeFamily);
2425
void connectToView(Tag viewTag);
2526
void disconnectFromView(Tag viewTag);
2627
void restoreDefaultValues();
@@ -51,6 +52,7 @@ class PropsAnimatedNode final : public AnimatedNode {
5152
bool layoutStyleUpdated_{false};
5253

5354
Tag connectedViewTag_{animated::undefinedAnimatedNodeIdentifier};
55+
ShadowNodeFamily::Weak shadowNodeFamily_;
5456

5557
// Needed for PlatformColor resolver
5658
SurfaceId connectedRootTag_{animated::undefinedAnimatedNodeIdentifier};

0 commit comments

Comments
 (0)