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

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

@@ -77,6 +77,72 @@ test('animate marginLeft layout prop', () => {
   );
 });
 
+// A layout animation on one view must not push the other views of the
+// surface through a shadow tree commit: their non-layout props keep taking
+// the direct path to the mounted views.
+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

@@ -12,25 +12,20 @@
 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;
+      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

@@ -80,14 +80,10 @@ 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);
+    react_native_assert(mutation.family != nullptr);
+    auto& updates = surfaceUpdates[mutation.family->getSurfaceId()];
+    const auto tag = mutation.tag;
+    updates.insert_or_assign(tag, std::move(mutation));
   }
 
   asyncFlushSurfaces.merge(mutations.asyncFlushSurfaces);
@@ -96,23 +92,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 +131,13 @@ 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);
+  // 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());
+  for (size_t i = 0; i < callbacksCopy.size(); ++i) {
+    batches[i] = callbacksCopy[i].callback(timestamp);
   }
-  applySurfaceUpdates(surfaceUpdates, asyncFlushSurfaces);
+  applyMutations(std::move(batches));
 }
 
 CallbackId AnimationBackend::start(const Callback& callback) {
@@ -169,20 +176,23 @@ 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(
     SurfaceId surfaceId,
-    SurfaceUpdates& surfaceUpdates) {
+    SurfaceUpdates& updates) {
   auto uiManager = uiManager_.lock();
   if (!uiManager) {
     return;
   }
 
-  auto& surfaceFamilies = surfaceUpdates.families;
-  auto& updates = surfaceUpdates.propsMap;
+  std::unordered_set<std::shared_ptr<const ShadowNodeFamily>> surfaceFamilies;
+  for (const auto& [tag, mutation] : updates) {
+    surfaceFamilies.insert(mutation.family);
+  }
 
   uiManager->getShadowTreeRegistry().visit(
       surfaceId, [&surfaceFamilies, &updates](const ShadowTree& shadowTree) {
@@ -198,7 +208,8 @@ void AnimationBackend::commitUpdates(
                         auto newProps = ShadowNodeFragment::propsPlaceholder();
                         if (surfaceFamilies.contains(
                                 shadowNode.getFamilyShared())) {
-                          auto& animatedProps = updates.at(shadowNode.getTag());
+                          auto& animatedProps =
+                              updates.at(shadowNode.getTag()).props;
                           newProps = cloneProps(animatedProps, shadowNode);
                         }
                         return shadowNode.clone(

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

@@ -21,22 +21,16 @@
 #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;
-};
+// A frame's mutations on one surface, by view. Views with layout updates go
+// through a shadow tree commit, the rest is applied directly to the mounted
+// views.
+using SurfaceUpdates = std::unordered_map<Tag, AnimationMutation>;
 
 using Callback = std::function<AnimationMutations(AnimationTimestamp)>;
 
@@ -74,7 +68,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,33 @@
+/*
+ * 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/renderer/core/ReactPrimitives.h>
+#include <react/renderer/core/ShadowNodeFamily.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

scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api

@@ -710,6 +710,7 @@ using facebook::react::SizeAsTuple = std::tuple<facebook::react::Float, facebook
 using facebook::react::SnapshotMap = std::unordered_map<facebook::react::Tag, std::unique_ptr<facebook::react::PropsSnapshot>>;
 using facebook::react::StatePipe = std::function<void(const facebook::react::StateUpdate& stateUpdate)>;
 using facebook::react::SurfaceId = int32_t;
+using facebook::react::SurfaceUpdates = std::unordered_map<facebook::react::Tag, facebook::react::AnimationMutation>;
 using facebook::react::Tag = int32_t;
 using facebook::react::TelemetryClock = std::chrono::steady_clock;
 using facebook::react::TelemetryDuration = std::chrono::nanoseconds;
@@ -1580,7 +1581,7 @@ class facebook::react::AnimatedPropsRegistry {
   public void clear(facebook::react::SurfaceId surfaceId);
   public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
   public void initializeSurface(facebook::react::SurfaceId surfaceId);
-  public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
+  public void update(const std::vector<facebook::react::AnimationMutations>& batches);
 }
 
 class facebook::react::AnimationBackend : public facebook::react::UIManagerAnimationBackend {
@@ -8112,12 +8113,6 @@ struct facebook::react::SurfaceContext {
   public std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>> pendingFamilies;
 }
 
-struct facebook::react::SurfaceUpdates {
-  public bool hasLayoutUpdates;
-  public std::unordered_map<facebook::react::Tag, facebook::react::AnimatedProps> propsMap;
-  public std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>> families;
-}
-
 struct facebook::react::SystraceSection : public facebook::react::DummyTraceSection {
   template <typename... ConvertsToStringPiece>
   public SystraceSection(const char* name, ConvertsToStringPiece &&... args);

scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api

@@ -709,6 +709,7 @@ using facebook::react::SizeAsTuple = std::tuple<facebook::react::Float, facebook
 using facebook::react::SnapshotMap = std::unordered_map<facebook::react::Tag, std::unique_ptr<facebook::react::PropsSnapshot>>;
 using facebook::react::StatePipe = std::function<void(const facebook::react::StateUpdate& stateUpdate)>;
 using facebook::react::SurfaceId = int32_t;
+using facebook::react::SurfaceUpdates = std::unordered_map<facebook::react::Tag, facebook::react::AnimationMutation>;
 using facebook::react::Tag = int32_t;
 using facebook::react::TelemetryClock = std::chrono::steady_clock;
 using facebook::react::TelemetryDuration = std::chrono::nanoseconds;
@@ -1575,7 +1576,7 @@ class facebook::react::AnimatedPropsRegistry {
   public void clear(facebook::react::SurfaceId surfaceId);
   public void clearOnSurfaceStop(facebook::react::SurfaceId surfaceId);
   public void initializeSurface(facebook::react::SurfaceId surfaceId);
-  public void update(const std::unordered_map<facebook::react::SurfaceId, facebook::react::SurfaceUpdates>& surfaceUpdates);
+  public void update(const std::vector<facebook::react::AnimationMutations>& batches);
 }
 
 class facebook::react::AnimationBackend : public facebook::react::UIManagerAnimationBackend {
@@ -7872,12 +7873,6 @@ struct facebook::react::SurfaceContext {
   public std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>> pendingFamilies;
 }
 
-struct facebook::react::SurfaceUpdates {
-  public bool hasLayoutUpdates;
-  public std::unordered_map<facebook::react::Tag, facebook::react::AnimatedProps> propsMap;
-  public std::unordered_set<std::shared_ptr<const facebook::react::ShadowNodeFamily>> families;
-}
-
 struct facebook::react::SystraceSection : public facebook::react::DummyTraceSection {
   template <typename... ConvertsToStringPiece>
   public SystraceSection(const char* name, ConvertsToStringPiece &&... args);