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. `applySurfaceUpdates` splits a surface's mutations, keyed by view tag, into views with layout updates, which go through `commitUpdates`, and the rest, which are applied synchronously. Views with layout updates are also written synchronously, as the non-shared Animated path already does, so a view that starts animating a layout prop while animating `opacity` doesn't freeze its opacity behind the cache either. `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, so the mounted view and the registry stay in sync. Raw animated props are now merged per key, like the animation drivers merge them, instead of with merge-patch, which dropped `null` values meant to reset a prop. `RawProps::getDynamic` exposes a dynamic source without copying it.

## Changelog:

[General] [Fixed] - Non-layout props animated on the shared animation backend no longer freeze while the same view or another view on the surface animates a layout prop

Differential Revision: D122570616

Author: Bartlomiej Bloniarz

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

@@ -17,7 +17,7 @@ import * as Fantom from '@react-native/fantom';
 import nullthrows from 'nullthrows';
 import * as React from 'react';
 import {Component, createRef, memo, useEffect, useMemo, useState} from 'react';
-import {Animated, View, useAnimatedValue} from 'react-native';
+import {Animated, Easing, View, useAnimatedValue} from 'react-native';
 
 // marginLeft (and the other margin props) are only on the native animated
 // allowlist when the shared backend is enabled. This verifies the prop is
@@ -77,6 +77,196 @@ 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('direct-path props survive a re-render while another view animates layout', () => {
+  let _opacity;
+  let _opacityAnimation;
+  let _siblingHeight;
+  let _siblingHeightAnimation;
+  let _setWidth;
+
+  function MyApp() {
+    const opacity = useAnimatedValue(0);
+    const siblingHeight = useAnimatedValue(10);
+    const [width, setWidth] = useState(100);
+    _opacity = opacity;
+    _siblingHeight = siblingHeight;
+    _setWidth = setWidth;
+    return (
+      <View collapsable={false}>
+        <Animated.View style={{width, height: 100, opacity}} />
+        <Animated.View style={{width: 100, height: siblingHeight}} />
+      </View>
+    );
+  }
+
+  const root = Fantom.createRoot();
+
+  Fantom.runTask(() => {
+    root.render(<MyApp />);
+  });
+
+  Fantom.runTask(() => {
+    _opacityAnimation = Animated.timing(_opacity, {
+      toValue: 0.5,
+      duration: 1000,
+      useNativeDriver: true,
+    }).start();
+    _siblingHeightAnimation = Animated.timing(_siblingHeight, {
+      toValue: 110,
+      duration: 1000,
+      useNativeDriver: true,
+    }).start();
+  });
+
+  Fantom.unstable_produceFramesForDuration(500);
+
+  Fantom.runTask(() => {
+    _setWidth(150);
+  });
+
+  expect(
+    root.getRenderedOutput({props: ['opacity', 'width', 'height']}).toJSX(),
+  ).toEqual(
+    <rn-view>
+      <rn-view key={0} opacity="0.25" width="150" height="100" />
+      <rn-view key={1} width="100" height="60" />
+    </rn-view>,
+  );
+
+  Fantom.runTask(() => {
+    _opacityAnimation?.stop();
+    _siblingHeightAnimation?.stop();
+  });
+});
+
+test('a view keeps writing its props directly after it starts animating layout', () => {
+  const viewRef = createRef<HostInstance>();
+
+  let _opacity;
+  let _opacityAnimation;
+  let _height;
+  let _heightAnimation;
+
+  function MyApp() {
+    const opacity = useAnimatedValue(0);
+    const height = useAnimatedValue(10);
+    _opacity = opacity;
+    _height = height;
+    return (
+      <Animated.View ref={viewRef} style={{width: 100, height, opacity}} />
+    );
+  }
+
+  const root = Fantom.createRoot();
+
+  Fantom.runTask(() => {
+    root.render(<MyApp />);
+  });
+
+  const viewElement = nullthrows(viewRef.current);
+
+  Fantom.runTask(() => {
+    _opacityAnimation = Animated.timing(_opacity, {
+      toValue: 0.5,
+      duration: 1000,
+      easing: Easing.linear,
+      useNativeDriver: true,
+    }).start();
+  });
+
+  Fantom.unstable_produceFramesForDuration(500);
+
+  expect(
+    Fantom.unstable_getDirectManipulationProps(viewElement).opacity,
+  ).toBeCloseTo(0.25, 0.001);
+
+  Fantom.runTask(() => {
+    _heightAnimation = Animated.timing(_height, {
+      toValue: 110,
+      duration: 1000,
+      easing: Easing.linear,
+      useNativeDriver: true,
+    }).start();
+  });
+
+  Fantom.unstable_produceFramesForDuration(250);
+
+  expect(root.getRenderedOutput({props: ['height']}).toJSX()).toEqual(
+    <rn-view height="35" />,
+  );
+  expect(
+    Fantom.unstable_getDirectManipulationProps(viewElement).opacity,
+  ).toBeCloseTo(0.375, 0.001);
+
+  Fantom.runTask(() => {
+    _opacityAnimation?.stop();
+    _heightAnimation?.stop();
+  });
+});
+
 test('animated opacity', () => {
   let _opacity;
   let _opacityAnimation;

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

@@ -6,31 +6,40 @@
  */
 
 #include "AnimatedPropsRegistry.h"
+#include <react/debug/react_native_assert.h>
 #include <react/renderer/core/PropsParserContext.h>
 #include "AnimatedProps.h"
 
 namespace facebook::react {
 
+void mergeAnimatedRawProps(
+    folly::dynamic& target,
+    const folly::dynamic& source) {
+  if (!target.isObject() || !source.isObject()) {
+    target = source;
+    return;
+  }
+  for (const auto& [key, value] : source.items()) {
+    target[key] = value;
+  }
+}
+
 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>())
@@ -44,8 +53,7 @@ void AnimatedPropsRegistry::update(
         auto& currentRawProps = snapshot->rawProps;
 
         if (currentRawProps) {
-          auto newRawPropsDynamic = newRawProps.toDynamic();
-          currentRawProps->merge_patch(newRawPropsDynamic);
+          mergeAnimatedRawProps(*currentRawProps, newRawProps.toDynamic());
         } else {
           currentRawProps =
               std::make_unique<folly::dynamic>(newRawProps.toDynamic());
@@ -83,7 +91,8 @@ AnimatedPropsRegistry::getMap(SurfaceId surfaceId) {
       auto& currentSnapshot = currentIt->second;
       if (propsSnapshot->rawProps) {
         if (currentSnapshot->rawProps) {
-          currentSnapshot->rawProps->merge_patch(*propsSnapshot->rawProps);
+          mergeAnimatedRawProps(
+              *currentSnapshot->rawProps, *propsSnapshot->rawProps);
         } else {
           currentSnapshot->rawProps = std::move(propsSnapshot->rawProps);
         }

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,17 @@ 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>>;
 
+/*
+ * Merges raw animated props per key, like the props merged by the animation
+ * drivers. A null value is kept, so it still resets the prop on commit.
+ */
+void mergeAnimatedRawProps(folly::dynamic &target, const folly::dynamic &source);
+
 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);