Skip to content

Commit 7822032

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. Reviewed By: zeyap Differential Revision: D89042963
1 parent 294a961 commit 7822032

4 files changed

Lines changed: 109 additions & 96 deletions

File tree

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

Lines changed: 67 additions & 88 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";
@@ -261,14 +260,13 @@ void NativeAnimatedNodesManager::disconnectAnimatedNodeFromView(
261260
auto node = getAnimatedNode<PropsAnimatedNode>(propsNodeTag);
262261
if (node != nullptr) {
263262
node->disconnectFromView(viewTag);
263+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
264+
node->disconnectFromShadowNodeFamily();
265+
}
264266
{
265267
std::lock_guard<std::mutex> lock(connectedAnimatedNodesMutex_);
266268
connectedAnimatedNodes_.erase(viewTag);
267269
}
268-
{
269-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
270-
tagToShadowNodeFamily_.erase(viewTag);
271-
}
272270
updatedNodeTags_.insert(node->tag());
273271

274272
onManagedPropsRemoved(viewTag);
@@ -907,13 +905,14 @@ void NativeAnimatedNodesManager::schedulePropsCommit(
907905
Tag viewTag,
908906
const folly::dynamic& props,
909907
bool layoutStyleUpdated,
910-
bool forceFabricCommit) noexcept {
908+
bool forceFabricCommit,
909+
ShadowNodeFamily::Weak shadowNodeFamily) noexcept {
911910
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
912-
if (layoutStyleUpdated) {
913-
mergeObjects(updateViewProps_[viewTag], props);
914-
} else {
915-
mergeObjects(updateViewPropsDirect_[viewTag], props);
916-
}
911+
auto& current = layoutStyleUpdated
912+
? updateViewPropsForBackend_[viewTag]
913+
: updateViewPropsDirectForBackend_[viewTag];
914+
current.first = std::move(shadowNodeFamily);
915+
mergeObjects(current.second, props);
917916
return;
918917
}
919918

@@ -1007,49 +1006,39 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
10071006
}
10081007
}
10091008

1010-
{
1011-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
1012-
for (auto& [tag, props] : updateViewPropsDirect_) {
1013-
auto familyIt = tagToShadowNodeFamily_.find(tag);
1014-
if (familyIt == tagToShadowNodeFamily_.end()) {
1015-
continue;
1016-
}
1017-
1018-
auto weakFamily = familyIt->second;
1019-
if (auto family = weakFamily.lock()) {
1020-
propsBuilder.storeDynamic(props);
1021-
mutations.batch.push_back(
1022-
AnimationMutation{
1023-
.tag = tag,
1024-
.family = family,
1025-
.props = propsBuilder.get(),
1026-
});
1027-
}
1028-
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+
});
10291020
}
1030-
for (auto& [tag, props] : updateViewProps_) {
1031-
auto familyIt = tagToShadowNodeFamily_.find(tag);
1032-
if (familyIt == tagToShadowNodeFamily_.end()) {
1033-
continue;
1034-
}
1035-
1036-
auto weakFamily = familyIt->second;
1037-
if (auto family = weakFamily.lock()) {
1038-
propsBuilder.storeDynamic(props);
1039-
mutations.batch.push_back(
1040-
AnimationMutation{
1041-
.tag = tag,
1042-
.family = family,
1043-
.props = propsBuilder.get(),
1044-
.hasLayoutUpdates = true,
1045-
});
1046-
}
1047-
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+
});
10481035
}
1036+
containsChange = true;
10491037
}
1038+
10501039
if (containsChange) {
1051-
updateViewPropsDirect_.clear();
1052-
updateViewProps_.clear();
1040+
updateViewPropsDirectForBackend_.clear();
1041+
updateViewPropsForBackend_.clear();
10531042
}
10541043
}
10551044

@@ -1076,46 +1065,36 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
10761065

10771066
isEventAnimationInProgress_ = false;
10781067

1079-
{
1080-
std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
1081-
for (auto& [tag, props] : updateViewPropsDirect_) {
1082-
auto familyIt = tagToShadowNodeFamily_.find(tag);
1083-
if (familyIt == tagToShadowNodeFamily_.end()) {
1084-
continue;
1085-
}
1086-
1087-
auto weakFamily = familyIt->second;
1088-
if (auto family = weakFamily.lock()) {
1089-
propsBuilder.storeDynamic(props);
1090-
mutations.batch.push_back(
1091-
AnimationMutation{
1092-
.tag = tag,
1093-
.family = family,
1094-
.props = propsBuilder.get(),
1095-
});
1096-
}
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+
});
10971079
}
1098-
for (auto& [tag, props] : updateViewProps_) {
1099-
auto familyIt = tagToShadowNodeFamily_.find(tag);
1100-
if (familyIt == tagToShadowNodeFamily_.end()) {
1101-
continue;
1102-
}
1103-
1104-
auto weakFamily = familyIt->second;
1105-
if (auto family = weakFamily.lock()) {
1106-
propsBuilder.storeDynamic(props);
1107-
mutations.batch.push_back(
1108-
AnimationMutation{
1109-
.tag = tag,
1110-
.family = family,
1111-
.props = propsBuilder.get(),
1112-
.hasLayoutUpdates = true,
1113-
});
1114-
}
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+
});
11151093
}
11161094
}
1117-
updateViewProps_.clear();
1118-
updateViewPropsDirect_.clear();
1095+
1096+
updateViewPropsForBackend_.clear();
1097+
updateViewPropsDirectForBackend_.clear();
11191098
}
11201099
} else {
11211100
// There is no active animation. Stop the render callback.

‎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,9 +261,8 @@ class NativeAnimatedNodesManager {
260261

261262
std::unordered_map<Tag, folly::dynamic> updateViewProps_{};
262263
std::unordered_map<Tag, folly::dynamic> updateViewPropsDirect_{};
263-
264-
mutable std::mutex tagToShadowNodeFamilyMutex_;
265-
std::unordered_map<Tag, std::weak_ptr<const ShadowNodeFamily>> tagToShadowNodeFamily_{};
264+
std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsForBackend_{};
265+
std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsDirectForBackend_{};
266266

267267
/*
268268
* Sometimes a view is not longer connected to a PropsAnimatedNode, but

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

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include "PropsAnimatedNode.h"
1313

1414
#include <react/debug/react_native_assert.h>
15+
#include <react/featureflags/ReactNativeFeatureFlags.h>
1516
#include <react/renderer/animated/NativeAnimatedNodesManager.h>
1617
#include <react/renderer/animated/nodes/ColorAnimatedNode.h>
1718
#include <react/renderer/animated/nodes/ObjectAnimatedNode.h>
@@ -55,6 +56,15 @@ PropsAnimatedNode::PropsAnimatedNode(
5556
}
5657
}
5758

59+
void PropsAnimatedNode::connectToShadowNodeFamily(
60+
ShadowNodeFamily::Weak shadowNodeFamily) {
61+
shadowNodeFamily_ = std::move(shadowNodeFamily);
62+
}
63+
64+
void PropsAnimatedNode::disconnectFromShadowNodeFamily() {
65+
shadowNodeFamily_.reset();
66+
}
67+
5868
void PropsAnimatedNode::connectToView(Tag viewTag) {
5969
react_native_assert(
6070
connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier &&
@@ -74,8 +84,17 @@ void PropsAnimatedNode::disconnectFromView(Tag viewTag) {
7484
void PropsAnimatedNode::restoreDefaultValues() {
7585
// If node is already disconnected from View, we cannot restore default values
7686
if (connectedViewTag_ != animated::undefinedAnimatedNodeIdentifier) {
77-
manager_->schedulePropsCommit(
78-
connectedViewTag_, folly::dynamic::object(), false, false);
87+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
88+
manager_->schedulePropsCommit(
89+
connectedViewTag_,
90+
folly::dynamic::object(),
91+
false,
92+
false,
93+
shadowNodeFamily_);
94+
} else {
95+
manager_->schedulePropsCommit(
96+
connectedViewTag_, folly::dynamic::object(), false, false);
97+
}
7998
}
8099
}
81100

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

148167
layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
149168

150-
manager_->schedulePropsCommit(
151-
connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
169+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
170+
manager_->schedulePropsCommit(
171+
connectedViewTag_,
172+
props_,
173+
layoutStyleUpdated_,
174+
forceFabricCommit,
175+
shadowNodeFamily_);
176+
} else {
177+
manager_->schedulePropsCommit(
178+
connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
179+
}
152180
}
153181

154182
} // namespace facebook::react

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ namespace facebook::react {
2121
class PropsAnimatedNode final : public AnimatedNode {
2222
public:
2323
PropsAnimatedNode(Tag tag, const folly::dynamic &config, NativeAnimatedNodesManager &manager);
24+
25+
// Only called when `useSharedAnimatedBackend`==true
26+
void connectToShadowNodeFamily(ShadowNodeFamily::Weak shadowNodeFamily);
27+
void disconnectFromShadowNodeFamily();
28+
2429
void connectToView(Tag viewTag);
2530
void disconnectFromView(Tag viewTag);
2631
void restoreDefaultValues();
@@ -51,6 +56,7 @@ class PropsAnimatedNode final : public AnimatedNode {
5156
bool layoutStyleUpdated_{false};
5257

5358
Tag connectedViewTag_{animated::undefinedAnimatedNodeIdentifier};
59+
ShadowNodeFamily::Weak shadowNodeFamily_;
5460

5561
// Needed for PlatformColor resolver
5662
SurfaceId connectedRootTag_{animated::undefinedAnimatedNodeIdentifier};

0 commit comments

Comments
 (0)