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

Author: Bartlomiej Bloniarz

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

@@ -244,8 +244,7 @@ void NativeAnimatedNodesManager::connectAnimatedNodeToShadowNodeFamily(
   react_native_assert(propsNodeTag);
   auto node = getAnimatedNode<PropsAnimatedNode>(propsNodeTag);
   if (node != nullptr && family != nullptr) {
-    std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
-    tagToShadowNodeFamily_[family->getTag()] = family;
+    node->connectToShadowNodeFamily(family);
   } else {
     LOG(WARNING)
         << "Cannot ConnectAnimatedNodeToShadowNodeFamily, animated node has to be props type";
@@ -265,10 +264,6 @@ void NativeAnimatedNodesManager::disconnectAnimatedNodeFromView(
       std::lock_guard<std::mutex> lock(connectedAnimatedNodesMutex_);
       connectedAnimatedNodes_.erase(viewTag);
     }
-    {
-      std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
-      tagToShadowNodeFamily_.erase(viewTag);
-    }
     updatedNodeTags_.insert(node->tag());
 
     onManagedPropsRemoved(viewTag);
@@ -907,13 +902,14 @@ void NativeAnimatedNodesManager::schedulePropsCommit(
     Tag viewTag,
     const folly::dynamic& props,
     bool layoutStyleUpdated,
-    bool forceFabricCommit) noexcept {
+    bool forceFabricCommit,
+    ShadowNodeFamily::Weak shadowNodeFamily) noexcept {
   if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
-    if (layoutStyleUpdated) {
-      mergeObjects(updateViewProps_[viewTag], props);
-    } else {
-      mergeObjects(updateViewPropsDirect_[viewTag], props);
-    }
+    auto& current = layoutStyleUpdated
+        ? updateViewPropsForBackend_[viewTag]
+        : updateViewPropsDirectForBackend_[viewTag];
+    current.first = std::move(shadowNodeFamily);
+    mergeObjects(current.second, props);
     return;
   }
 
@@ -1007,38 +1003,36 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
         }
       }
 
-      {
-        std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
-        for (auto& [tag, props] : updateViewPropsDirect_) {
-          auto weakFamily = tagToShadowNodeFamily_[tag];
-
-          if (auto family = weakFamily.lock()) {
-            propsBuilder.storeDynamic(props);
-            mutations.batch.push_back(
-                AnimationMutation{
-                    .tag = tag,
-                    .family = family,
-                    .props = propsBuilder.get(),
-                });
-          }
-          containsChange = true;
+      for (auto& [tag, update] : updateViewPropsDirectForBackend_) {
+        auto weakFamily = update.first;
+
+        if (auto family = weakFamily.lock()) {
+          propsBuilder.storeDynamic(update.second);
+          mutations.batch.push_back(
+              AnimationMutation{
+                  .tag = tag,
+                  .family = family,
+                  .props = propsBuilder.get(),
+              });
         }
-        for (auto& [tag, props] : updateViewProps_) {
-          auto weakFamily = tagToShadowNodeFamily_[tag];
-
-          if (auto family = weakFamily.lock()) {
-            propsBuilder.storeDynamic(props);
-            mutations.batch.push_back(
-                AnimationMutation{
-                    .tag = tag,
-                    .family = family,
-                    .props = propsBuilder.get(),
-                    .hasLayoutUpdates = true,
-                });
-          }
-          containsChange = true;
+        containsChange = true;
+      }
+      for (auto& [tag, update] : updateViewPropsForBackend_) {
+        auto weakFamily = update.first;
+
+        if (auto family = weakFamily.lock()) {
+          propsBuilder.storeDynamic(update.second);
+          mutations.batch.push_back(
+              AnimationMutation{
+                  .tag = tag,
+                  .family = family,
+                  .props = propsBuilder.get(),
+                  .hasLayoutUpdates = true,
+              });
         }
+        containsChange = true;
       }
+
       if (containsChange) {
         updateViewPropsDirect_.clear();
         updateViewProps_.clear();
@@ -1068,36 +1062,34 @@ AnimationMutations NativeAnimatedNodesManager::pullAnimationMutations() {
 
       isEventAnimationInProgress_ = false;
 
-      {
-        std::lock_guard<std::mutex> lock(tagToShadowNodeFamilyMutex_);
-        for (auto& [tag, props] : updateViewPropsDirect_) {
-          auto weakFamily = tagToShadowNodeFamily_[tag];
-
-          if (auto family = weakFamily.lock()) {
-            propsBuilder.storeDynamic(props);
-            mutations.batch.push_back(
-                AnimationMutation{
-                    .tag = tag,
-                    .family = family,
-                    .props = propsBuilder.get(),
-                });
-          }
+      for (auto& [tag, update] : updateViewPropsDirectForBackend_) {
+        auto weakFamily = update.first;
+
+        if (auto family = weakFamily.lock()) {
+          propsBuilder.storeDynamic(update.second);
+          mutations.batch.push_back(
+              AnimationMutation{
+                  .tag = tag,
+                  .family = family,
+                  .props = propsBuilder.get(),
+              });
         }
-        for (auto& [tag, props] : updateViewProps_) {
-          auto weakFamily = tagToShadowNodeFamily_[tag];
-
-          if (auto family = weakFamily.lock()) {
-            propsBuilder.storeDynamic(props);
-            mutations.batch.push_back(
-                AnimationMutation{
-                    .tag = tag,
-                    .family = family,
-                    .props = propsBuilder.get(),
-                    .hasLayoutUpdates = true,
-                });
-          }
+      }
+      for (auto& [tag, update] : updateViewPropsForBackend_) {
+        auto weakFamily = update.first;
+
+        if (auto family = weakFamily.lock()) {
+          propsBuilder.storeDynamic(update.second);
+          mutations.batch.push_back(
+              AnimationMutation{
+                  .tag = tag,
+                  .family = family,
+                  .props = propsBuilder.get(),
+                  .hasLayoutUpdates = true,
+              });
         }
       }
+
       updateViewProps_.clear();
       updateViewPropsDirect_.clear();
     }

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

@@ -153,7 +153,8 @@ class NativeAnimatedNodesManager {
       Tag viewTag,
       const folly::dynamic &props,
       bool layoutStyleUpdated,
-      bool forceFabricCommit) noexcept;
+      bool forceFabricCommit,
+      ShadowNodeFamily::Weak shadowNodeFamily = {}) noexcept;
 
   /**
    * Commits all pending animated property updates to their respective views.
@@ -260,9 +261,8 @@ class NativeAnimatedNodesManager {
 
   std::unordered_map<Tag, folly::dynamic> updateViewProps_{};
   std::unordered_map<Tag, folly::dynamic> updateViewPropsDirect_{};
-
-  mutable std::mutex tagToShadowNodeFamilyMutex_;
-  std::unordered_map<Tag, std::weak_ptr<const ShadowNodeFamily>> tagToShadowNodeFamily_{};
+  std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsForBackend_{};
+  std::unordered_map<Tag, std::pair<ShadowNodeFamily::Weak, folly::dynamic>> updateViewPropsDirectForBackend_{};
 
   /*
    * Sometimes a view is not longer connected to a PropsAnimatedNode, but

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

@@ -12,6 +12,7 @@
 #include "PropsAnimatedNode.h"
 
 #include <react/debug/react_native_assert.h>
+#include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/renderer/animated/NativeAnimatedNodesManager.h>
 #include <react/renderer/animated/nodes/ColorAnimatedNode.h>
 #include <react/renderer/animated/nodes/ObjectAnimatedNode.h>
@@ -55,6 +56,11 @@ PropsAnimatedNode::PropsAnimatedNode(
   }
 }
 
+void PropsAnimatedNode::connectToShadowNodeFamily(
+    ShadowNodeFamily::Weak shadowNodeFamily) {
+  shadowNodeFamily_ = std::move(shadowNodeFamily);
+}
+
 void PropsAnimatedNode::connectToView(Tag viewTag) {
   react_native_assert(
       connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier &&
@@ -67,15 +73,25 @@ void PropsAnimatedNode::disconnectFromView(Tag viewTag) {
       connectedViewTag_ == viewTag &&
       "Attempting to disconnect view that has not been connected with the given animated node.");
   connectedViewTag_ = animated::undefinedAnimatedNodeIdentifier;
+  shadowNodeFamily_.reset();
 }
 
 // restore the value to whatever the value was on the ShadowNode instead of in
 // the View hierarchy
 void PropsAnimatedNode::restoreDefaultValues() {
   // If node is already disconnected from View, we cannot restore default values
   if (connectedViewTag_ != animated::undefinedAnimatedNodeIdentifier) {
-    manager_->schedulePropsCommit(
-        connectedViewTag_, folly::dynamic::object(), false, false);
+    if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
+      manager_->schedulePropsCommit(
+          connectedViewTag_,
+          folly::dynamic::object(),
+          false,
+          false,
+          shadowNodeFamily_);
+    } else {
+      manager_->schedulePropsCommit(
+          connectedViewTag_, folly::dynamic::object(), false, false);
+    }
   }
 }
 
@@ -147,8 +163,17 @@ void PropsAnimatedNode::update(bool forceFabricCommit) {
 
   layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
 
-  manager_->schedulePropsCommit(
-      connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
+  if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
+    manager_->schedulePropsCommit(
+        connectedViewTag_,
+        props_,
+        layoutStyleUpdated_,
+        forceFabricCommit,
+        shadowNodeFamily_);
+  } else {
+    manager_->schedulePropsCommit(
+        connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
+  }
 }
 
 } // namespace facebook::react

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

@@ -21,6 +21,7 @@ namespace facebook::react {
 class PropsAnimatedNode final : public AnimatedNode {
  public:
   PropsAnimatedNode(Tag tag, const folly::dynamic &config, NativeAnimatedNodesManager &manager);
+  void connectToShadowNodeFamily(ShadowNodeFamily::Weak shadowNodeFamily);
   void connectToView(Tag viewTag);
   void disconnectFromView(Tag viewTag);
   void restoreDefaultValues();
@@ -51,6 +52,7 @@ class PropsAnimatedNode final : public AnimatedNode {
   bool layoutStyleUpdated_{false};
 
   Tag connectedViewTag_{animated::undefinedAnimatedNodeIdentifier};
+  ShadowNodeFamily::Weak shadowNodeFamily_;
 
   // Needed for PlatformColor resolver
   SurfaceId connectedRootTag_{animated::undefinedAnimatedNodeIdentifier};