Keep non-layout animations on the synchronous path while another view animates layout (#58772)

Summary:
Pull Request resolved: #58772

The shared animation backend decided per surface whether a frame's animated props go through a shadow tree commit or straight to the mounted views. As soon as one view animated a layout prop, every animated view on that surface went through the commit path for the whole animation, including views that only animate `transform` or `opacity`. Besides the extra commit work, on Android this moved those views from `updatePropsSynchronously` to regular mounts. There the synchronous mount props cache (`overrideBySynchronousMountPropsAtMountingAndroid`) replaced the incoming transform with its last synchronously written value, so the view froze while the other view's layout animation ran and jumped when it ended.

The decision is now made per view. `SurfaceUpdates` holds a surface's mutations keyed by view tag, and `applySurfaceUpdates` splits them into views with layout updates, which go through `commitUpdates`, and the rest, which are applied synchronously. `AnimationMutation` and `AnimationMutations` move to `AnimationMutation.h`, still included by `AnimationBackend.h`. `AnimatedPropsRegistry::update` reads the frame's batches instead of the merged per-surface map. When the same view receives several mutations in one frame, they are merged, with later values winning, so the mounted view and the registry stay in sync.

## Changelog:

[General] [Fixed] - A view animating only non-layout props on the shared animation backend no longer freezes while another view on the surface animates a layout prop

Differential Revision: D122570616

Author: Bartlomiej Bloniarz

packages/react-native/Libraries/Animated/__tests__/AnimatedBackend-itest.js

@@ -77,6 +77,69 @@ test('animate marginLeft layout prop', () => {
   );
 });
 
+test('non-layout props stay on the direct path while another view animates layout', () => {
+  const movingRef = createRef<HostInstance>();
+
+  let _translateX;
+  let _translateXAnimation;
+  let _siblingHeight;
+  let _siblingHeightAnimation;
+
+  function MyApp() {
+    const translateX = useAnimatedValue(0);
+    const siblingHeight = useAnimatedValue(10);
+    _translateX = translateX;
+    _siblingHeight = siblingHeight;
+    return (
+      <View collapsable={false}>
+        <Animated.View
+          ref={movingRef}
+          style={{width: 100, height: 100, transform: [{translateX}]}}
+        />
+        <Animated.View style={{width: 100, height: siblingHeight}} />
+      </View>
+    );
+  }
+
+  const root = Fantom.createRoot();
+
+  Fantom.runTask(() => {
+    root.render(<MyApp />);
+  });
+
+  Fantom.runTask(() => {
+    _translateXAnimation = Animated.timing(_translateX, {
+      toValue: 100,
+      duration: 200,
+      useNativeDriver: true,
+    }).start();
+    _siblingHeightAnimation = Animated.timing(_siblingHeight, {
+      toValue: 110,
+      duration: 200,
+      useNativeDriver: true,
+    }).start();
+  });
+
+  Fantom.unstable_produceFramesForDuration(100);
+
+  // The sibling's height went through a commit; the transform did not.
+  expect(root.getRenderedOutput({props: ['height']}).toJSX()).toEqual(
+    <rn-view>
+      <rn-view key={0} height="100" />
+      <rn-view key={1} height="60" />
+    </rn-view>,
+  );
+  expect(
+    Fantom.unstable_getDirectManipulationProps(nullthrows(movingRef.current))
+      .transform,
+  ).toEqual([{translateX: 50}]);
+
+  Fantom.runTask(() => {
+    _translateXAnimation?.stop();
+    _siblingHeightAnimation?.stop();
+  });
+});
+
 test('animated opacity', () => {
   let _opacity;
   let _opacityAnimation;

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimatedPropsRegistry.cpp

@@ -6,31 +6,28 @@
  */
 
 #include "AnimatedPropsRegistry.h"
+#include <react/debug/react_native_assert.h>
 #include <react/renderer/core/PropsParserContext.h>
 #include "AnimatedProps.h"
 
 namespace facebook::react {
 
 void AnimatedPropsRegistry::update(
-    const std::unordered_map<SurfaceId, SurfaceUpdates>& surfaceUpdates) {
+    const std::vector<AnimationMutations>& batches) {
   auto lock = std::lock_guard(mutex_);
-  for (const auto& [surfaceId, updates] : surfaceUpdates) {
-    auto contextIt = surfaceContexts_.find(surfaceId);
-    if (contextIt == surfaceContexts_.end()) {
-      continue;
-    }
-    auto& surfaceContext = contextIt->second;
-    auto& pendingMap = surfaceContext.pendingMap;
-    auto& pendingFamilies = surfaceContext.pendingFamilies;
-
-    auto& updatesMap = updates.propsMap;
-    auto& updatesFamilies = updates.families;
-
-    for (auto& family : updatesFamilies) {
-      pendingFamilies.insert(family);
-    }
-
-    for (auto& [tag, animatedProps] : updatesMap) {
+  for (const auto& mutations : batches) {
+    for (const auto& mutation : mutations.batch) {
+      const auto& family = mutation.family;
+      react_native_assert(family != nullptr);
+      auto contextIt = surfaceContexts_.find(family->getSurfaceId());
+      if (contextIt == surfaceContexts_.end()) {
+        continue;
+      }
+      auto& surfaceContext = contextIt->second;
+      auto& pendingMap = surfaceContext.pendingMap;
+      surfaceContext.pendingFamilies.insert(family);
+      const auto tag = mutation.tag;
+      const auto& animatedProps = mutation.props;
       auto it = pendingMap.find(tag);
       if (it == pendingMap.end()) {
         it = pendingMap.insert_or_assign(tag, std::make_unique<PropsSnapshot>())

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimatedPropsRegistry.h

@@ -15,6 +15,7 @@
 #include <react/renderer/uimanager/UIManager.h>
 #include <react/renderer/uimanager/UIManagerCommitHook.h>
 #include "AnimatedProps.h"
+#include "AnimationMutation.h"
 
 namespace facebook::react {
 
@@ -29,17 +30,11 @@ struct SurfaceContext {
   std::unordered_set<std::shared_ptr<const ShadowNodeFamily>> pendingFamilies, families;
 };
 
-struct SurfaceUpdates {
-  std::unordered_set<std::shared_ptr<const ShadowNodeFamily>> families;
-  std::unordered_map<Tag, AnimatedProps> propsMap;
-  bool hasLayoutUpdates{false};
-};
-
 using SnapshotMap = std::unordered_map<Tag, std::unique_ptr<PropsSnapshot>>;
 
 class AnimatedPropsRegistry {
  public:
-  void update(const std::unordered_map<SurfaceId, SurfaceUpdates> &surfaceUpdates);
+  void update(const std::vector<AnimationMutations> &batches);
   void initializeSurface(SurfaceId surfaceId);
   void clear(SurfaceId surfaceId);
   void clearOnSurfaceStop(SurfaceId surfaceId);

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.cpp

@@ -12,6 +12,7 @@
 #include <react/featureflags/ReactNativeFeatureFlags.h>
 #include <react/renderer/animationbackend/AnimatedPropsSerializer.h>
 #include <react/renderer/graphics/Color.h>
+#include <algorithm>
 #include <chrono>
 #include <utility>
 
@@ -48,6 +49,26 @@ static inline Props::Shared cloneProps(
   return newProps;
 }
 
+// Applies `incoming` on top of `existing` for the same view, later values win.
+static void mergeMutation(
+    AnimationMutation& existing,
+    AnimationMutation&& incoming) {
+  auto& props = existing.props;
+  for (auto& animatedProp : incoming.props.props) {
+    props.props.push_back(std::move(animatedProp));
+  }
+  if (incoming.props.rawProps) {
+    if (props.rawProps) {
+      auto merged = props.rawProps->toDynamic();
+      merged.merge_patch(incoming.props.rawProps->toDynamic());
+      props.rawProps = std::make_unique<RawProps>(std::move(merged));
+    } else {
+      props.rawProps = std::move(incoming.props.rawProps);
+    }
+  }
+  existing.hasLayoutUpdates |= incoming.hasLayoutUpdates;
+}
+
 AnimationBackend::AnimationBackend(
     std::shared_ptr<AnimationChoreographer> animationChoreographer,
     std::shared_ptr<UIManager> uiManager)
@@ -80,14 +101,13 @@ void AnimationBackend::unpackMutations(
     std::unordered_map<SurfaceId, SurfaceUpdates>& surfaceUpdates,
     std::set<SurfaceId>& asyncFlushSurfaces) {
   for (auto& mutation : mutations.batch) {
-    const auto family = mutation.family;
-    react_native_assert(family != nullptr);
-
-    auto& [families, updates, hasLayoutUpdates] =
-        surfaceUpdates[family->getSurfaceId()];
-    hasLayoutUpdates |= mutation.hasLayoutUpdates;
-    families.insert(family);
-    updates[mutation.tag] = std::move(mutation.props);
+    auto& updates = surfaceUpdates[mutation.family->getSurfaceId()];
+    const auto tag = mutation.tag;
+    if (auto it = updates.find(tag); it != updates.end()) {
+      mergeMutation(it->second, std::move(mutation));
+    } else {
+      updates.emplace(tag, std::move(mutation));
+    }
   }
 
   asyncFlushSurfaces.merge(mutations.asyncFlushSurfaces);
@@ -96,23 +116,34 @@ void AnimationBackend::unpackMutations(
 void AnimationBackend::applySurfaceUpdates(
     std::unordered_map<SurfaceId, SurfaceUpdates>& surfaceUpdates,
     const std::set<SurfaceId>& asyncFlushSurfaces) {
-  animatedPropsRegistry_->update(surfaceUpdates);
-
   for (auto& [surfaceId, updates] : surfaceUpdates) {
-    if (updates.hasLayoutUpdates) {
-      commitUpdates(surfaceId, updates);
-    } else {
-      synchronouslyUpdateProps(updates.propsMap);
+    SurfaceUpdates layoutUpdates;
+    std::unordered_map<Tag, AnimatedProps> directProps;
+    for (auto& [tag, mutation] : updates) {
+      if (mutation.hasLayoutUpdates) {
+        layoutUpdates.emplace(tag, std::move(mutation));
+      } else {
+        directProps.emplace(tag, std::move(mutation.props));
+      }
+    }
+    if (!layoutUpdates.empty()) {
+      commitUpdates(surfaceId, layoutUpdates);
+    }
+    if (!directProps.empty()) {
+      synchronouslyUpdateProps(directProps);
     }
   }
 
   requestAsyncFlushForSurfaces(asyncFlushSurfaces);
 }
 
-void AnimationBackend::applyMutations(AnimationMutations mutations) {
+void AnimationBackend::applyMutations(std::vector<AnimationMutations> batches) {
+  animatedPropsRegistry_->update(batches);
   std::unordered_map<SurfaceId, SurfaceUpdates> surfaceUpdates;
   std::set<SurfaceId> asyncFlushSurfaces;
-  unpackMutations(mutations, surfaceUpdates, asyncFlushSurfaces);
+  for (auto& mutations : batches) {
+    unpackMutations(mutations, surfaceUpdates, asyncFlushSurfaces);
+  }
   applySurfaceUpdates(surfaceUpdates, asyncFlushSurfaces);
 }
 
@@ -124,13 +155,17 @@ void AnimationBackend::onAnimationFrame(AnimationTimestamp timestamp) {
     callbacksCopy = callbacks;
   }
 
-  std::unordered_map<SurfaceId, SurfaceUpdates> surfaceUpdates;
-  std::set<SurfaceId> asyncFlushSurfaces;
-  for (auto& callbackWithId : callbacksCopy) {
-    auto mutations = callbackWithId.callback(timestamp);
-    unpackMutations(mutations, surfaceUpdates, asyncFlushSurfaces);
-  }
-  applySurfaceUpdates(surfaceUpdates, asyncFlushSurfaces);
+  // Sized up front rather than grown: MSVC's std::set move isn't noexcept, so
+  // growing a vector of AnimationMutations would try to copy move-only props.
+  std::vector<AnimationMutations> batches(callbacksCopy.size());
+  std::transform(
+      callbacksCopy.begin(),
+      callbacksCopy.end(),
+      batches.begin(),
+      [timestamp](const CallbackWithId& callbackWithId) {
+        return callbackWithId.callback(timestamp);
+      });
+  applyMutations(std::move(batches));
 }
 
 CallbackId AnimationBackend::start(const Callback& callback) {
@@ -169,8 +204,9 @@ void AnimationBackend::trigger() {
 
 void AnimationBackend::pushAnimationMutations(const Callback& callback) {
   auto timestamp = animationChoreographer_->now();
-  auto mutations = callback(timestamp);
-  applyMutations(std::move(mutations));
+  std::vector<AnimationMutations> batches(1);
+  batches[0] = callback(timestamp);
+  applyMutations(std::move(batches));
 }
 
 void AnimationBackend::commitUpdates(
@@ -181,24 +217,28 @@ void AnimationBackend::commitUpdates(
     return;
   }
 
-  auto& surfaceFamilies = surfaceUpdates.families;
-  auto& updates = surfaceUpdates.propsMap;
+  std::unordered_set<std::shared_ptr<const ShadowNodeFamily>> surfaceFamilies;
+  for (const auto& [tag, mutation] : surfaceUpdates) {
+    surfaceFamilies.insert(mutation.family);
+  }
 
   uiManager->getShadowTreeRegistry().visit(
-      surfaceId, [&surfaceFamilies, &updates](const ShadowTree& shadowTree) {
+      surfaceId,
+      [&surfaceFamilies, &surfaceUpdates](const ShadowTree& shadowTree) {
         shadowTree.commit(
             [&surfaceFamilies,
-             &updates](const RootShadowNode& oldRootShadowNode) {
+             &surfaceUpdates](const RootShadowNode& oldRootShadowNode) {
               return std::static_pointer_cast<RootShadowNode>(
                   oldRootShadowNode.cloneMultiple(
                       surfaceFamilies,
-                      [&surfaceFamilies, &updates](
+                      [&surfaceFamilies, &surfaceUpdates](
                           const ShadowNode& shadowNode,
                           const ShadowNodeFragment& fragment) {
                         auto newProps = ShadowNodeFragment::propsPlaceholder();
                         if (surfaceFamilies.contains(
                                 shadowNode.getFamilyShared())) {
-                          auto& animatedProps = updates.at(shadowNode.getTag());
+                          auto& animatedProps =
+                              surfaceUpdates.at(shadowNode.getTag()).props;
                           newProps = cloneProps(animatedProps, shadowNode);
                         }
                         return shadowNode.clone(

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationBackend.h

@@ -21,22 +21,13 @@
 #include "AnimatedPropsRegistry.h"
 #include "AnimationBackendCommitHook.h"
 #include "AnimationChoreographer.h"
+#include "AnimationMutation.h"
 
 namespace facebook::react {
 
 class AnimationBackend;
 
-struct AnimationMutation {
-  Tag tag;
-  std::shared_ptr<const ShadowNodeFamily> family;
-  AnimatedProps props;
-  bool hasLayoutUpdates{false};
-};
-
-struct AnimationMutations {
-  std::vector<AnimationMutation> batch;
-  std::set<SurfaceId> asyncFlushSurfaces;
-};
+using SurfaceUpdates = std::unordered_map<Tag, AnimationMutation>;
 
 using Callback = std::function<AnimationMutations(AnimationTimestamp)>;
 
@@ -74,7 +65,7 @@ class AnimationBackend : public UIManagerAnimationBackend {
   void applySurfaceUpdates(
       std::unordered_map<SurfaceId, SurfaceUpdates> &surfaceUpdates,
       const std::set<SurfaceId> &asyncFlushSurfaces);
-  void applyMutations(AnimationMutations mutations);
+  void applyMutations(std::vector<AnimationMutations> batches);
   std::vector<CallbackWithId> callbacks;
   std::shared_ptr<AnimatedPropsRegistry> animatedPropsRegistry_;
   std::shared_ptr<AnimationChoreographer> animationChoreographer_;

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimationMutation.h

@@ -0,0 +1,32 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+#pragma once
+
+#include <react/cxxstableapi/FrameworksGuard.h>
+
+#include <React/RendererCore.h>
+#include <memory>
+#include <set>
+#include <vector>
+#include "AnimatedProps.h"
+
+namespace facebook::react {
+
+struct AnimationMutation {
+  Tag tag;
+  std::shared_ptr<const ShadowNodeFamily> family;
+  AnimatedProps props;
+  bool hasLayoutUpdates{false};
+};
+
+struct AnimationMutations {
+  std::vector<AnimationMutation> batch;
+  std::set<SurfaceId> asyncFlushSurfaces;
+};
+
+} // namespace facebook::react