Skip to content

Commit d1c5ee0

Browse files
GrahamCampbellmeta-codesync[bot]
GrahamCampbell
authored and
meta-codesync[bot]
committed
Fix native animated mount flash (#57391)
Summary: The C++ native animated backend (`cxxNativeAnimatedEnabled`) prevents the `useNativeDriver` first-frame flash by having `AnimatedMountingOverrideDelegate` re-merge a view's live animated props (`getManagedProps`) onto mount `Update` mutations, so a stale JS re-render can't reach the screen. There is a residual at connect time: `connectAnimatedNodeToView` registers the view (the override starts overriding it) but never runs the props node, so `getManagedProps()` returns an empty object until the next animation frame. If a Fabric mount transaction is pulled in that window the override has nothing to merge and the un-driven default value flashes for one frame, most visibly when a brand-new view is connected to an already mid-flight shared `Animated.Value`. This seeds the props node with `node->update()` at the end of `connectAnimatedNodeToView` so `getManagedProps()` is live the instant the view becomes managed. No manager mutex is held at that point and `update()` takes only the node-local props mutex, so it is lock-safe; it is idempotent with the per-frame update and adds no extra Fabric commit because it only stages. It closes the connect-while-`Update`-in-flight case and mitigates the brand-new-view (`Insert`) first-paint case; the override only rewrites `Update`/`Delete` mutations today, so fully closing `Insert` is a follow-up. It also adds the missing unit coverage for the `getManagedProps` / `hasManagedProps` seam the override depends on. ## Changelog: [INTERNAL] [FIXED] - Seed managed props when a view connects to the native animated backend so they are live before the first frame, shrinking the useNativeDriver mount flash Pull Request resolved: #57391 Test Plan: Adds `ManagedPropsMountingOverrideTests.cpp` to the ReactCommon animated unit tests. `getManagedPropsReflectsLiveValueAcrossFrames`, `getManagedPropsNullForUnconnectedView`, `hasManagedPropsTracksConnectAndDisconnect` and `getManagedPropsIsolatedPerView` pin the seam and pass regardless of the change. `getManagedPropsLiveImmediatelyOnConnect` and `getManagedPropsLiveOnConnectWhileValueMidFlight` are the regression cases: they fail without the seed (empty props at connect) and pass with it. Run via the animated C++ unit test target. On-device verification (iOS and Android with `cxxNativeAnimatedEnabled` on, frame-stepped capture of a popover/menu open) is still recommended for the user-visible flash. Reviewed By: sammy-SC Differential Revision: D110323404 Pulled By: zeyap fbshipit-source-id: 485e31e1c874548da1ee7339481164faa763b2d9
1 parent 34ccf4f commit d1c5ee0

13 files changed

Lines changed: 246 additions & 10 deletions

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ void NativeAnimatedNodesManager::connectAnimatedNodeToView(
229229
connectedAnimatedNodes_.insert({viewTag, propsNodeTag});
230230
}
231231
updatedNodeTags_.insert(node->tag());
232+
// Seed props_ so getManagedProps() is live at mount, without staging a
233+
// commit.
234+
node->collectProps();
232235
} else {
233236
LOG(WARNING)
234237
<< "Cannot ConnectAnimatedNodeToView, animated node has to be props type";

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -97,18 +97,9 @@ void PropsAnimatedNode::restoreDefaultValues() {
9797
}
9898
}
9999

100-
void PropsAnimatedNode::update() {
101-
return update(false);
102-
}
103-
104-
void PropsAnimatedNode::update(bool forceFabricCommit) {
105-
if (connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier) {
106-
return;
107-
}
108-
100+
void PropsAnimatedNode::collectPropsLocked() {
109101
// TODO: T190192206 consolidate shared update logic between
110102
// Props/StyleAnimatedNode
111-
std::lock_guard<std::mutex> lock(propsMutex_);
112103
const auto& configProps = getConfig()["props"];
113104
for (const auto& entry : configProps.items()) {
114105
auto propName = entry.first.asString();
@@ -164,6 +155,28 @@ void PropsAnimatedNode::update(bool forceFabricCommit) {
164155
}
165156

166157
layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
158+
}
159+
160+
void PropsAnimatedNode::collectProps() {
161+
if (connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier) {
162+
return;
163+
}
164+
165+
std::lock_guard<std::mutex> lock(propsMutex_);
166+
collectPropsLocked();
167+
}
168+
169+
void PropsAnimatedNode::update() {
170+
return update(false);
171+
}
172+
173+
void PropsAnimatedNode::update(bool forceFabricCommit) {
174+
if (connectedViewTag_ == animated::undefinedAnimatedNodeIdentifier) {
175+
return;
176+
}
177+
178+
std::lock_guard<std::mutex> lock(propsMutex_);
179+
collectPropsLocked();
167180

168181
if (manager_->useSharedAnimatedBackend()) {
169182
manager_->schedulePropsCommit(

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,18 @@ class PropsAnimatedNode final : public AnimatedNode {
4646
return props_;
4747
}
4848

49+
// Recompute props_ from the connected nodes WITHOUT scheduling a view
50+
// commit, so getManagedProps() is live at connect with no commit side effect.
51+
void collectProps();
52+
4953
void update() override;
5054

5155
void update(bool forceFabricCommit);
5256

5357
private:
58+
// Caller must hold propsMutex_.
59+
void collectPropsLocked();
60+
5461
std::mutex propsMutex_;
5562
folly::dynamic props_;
5663
bool layoutStyleUpdated_{false};

‎packages/react-native/ReactCommon/react/renderer/animated/tests/ManagedPropsMountingOverrideTests.cpp‎

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include "AnimationTestsBase.h"
9+
10+
#include <react/renderer/animated/drivers/AnimationDriverUtils.h>
11+
#include <react/renderer/core/ReactRootViewTagGenerator.h>
12+
13+
namespace facebook::react {
14+
15+
// Pins the getManagedProps / hasManagedProps seam re-merged by
16+
// AnimatedMountingOverrideDelegate to prevent the useNativeDriver mount flash.
17+
class ManagedPropsMountingOverrideTests : public AnimationTestsBase {
18+
protected:
19+
struct OpacityGraph {
20+
Tag opacityTag;
21+
Tag styleTag;
22+
Tag propsTag;
23+
Tag viewTag;
24+
};
25+
26+
// value -> style -> props graph for an animated opacity; view left
27+
// unconnected.
28+
OpacityGraph buildOpacityGraph(float initialOpacity) {
29+
const auto opacityTag = ++rootTag_;
30+
nodesManager_->createAnimatedNode(
31+
opacityTag,
32+
folly::dynamic::object("type", "value")("value", initialOpacity)(
33+
"offset", 0));
34+
35+
const auto styleTag = ++rootTag_;
36+
nodesManager_->createAnimatedNode(
37+
styleTag,
38+
folly::dynamic::object("type", "style")(
39+
"style", folly::dynamic::object("opacity", opacityTag)));
40+
nodesManager_->connectAnimatedNodes(opacityTag, styleTag);
41+
42+
const auto propsTag = ++rootTag_;
43+
nodesManager_->createAnimatedNode(
44+
propsTag,
45+
folly::dynamic::object("type", "props")(
46+
"props", folly::dynamic::object("style", styleTag)));
47+
nodesManager_->connectAnimatedNodes(styleTag, propsTag);
48+
49+
const auto viewTag = ++rootTag_;
50+
return OpacityGraph{
51+
.opacityTag = opacityTag,
52+
.styleTag = styleTag,
53+
.propsTag = propsTag,
54+
.viewTag = viewTag};
55+
}
56+
57+
Tag rootTag_{getNextRootViewTag()};
58+
};
59+
60+
TEST_F(
61+
ManagedPropsMountingOverrideTests,
62+
getManagedPropsNullForUnconnectedView) {
63+
initNodesManager();
64+
65+
const auto unknownTag = ++rootTag_;
66+
EXPECT_TRUE(nodesManager_->getManagedProps(unknownTag).isNull());
67+
EXPECT_FALSE(nodesManager_->hasManagedProps());
68+
}
69+
70+
// Managed props mirror the live committed value every frame, never the frozen
71+
// start value nor the 1.0 default.
72+
TEST_F(
73+
ManagedPropsMountingOverrideTests,
74+
getManagedPropsReflectsLiveValueAcrossFrames) {
75+
initNodesManager();
76+
77+
auto g = buildOpacityGraph(/*initialOpacity=*/0.0f);
78+
nodesManager_->connectAnimatedNodeToView(g.propsTag, g.viewTag);
79+
80+
const auto frames = folly::dynamic::array(0.0f, 0.25f, 0.5f, 0.75f, 1.0f);
81+
nodesManager_->startAnimatingNode(
82+
/*animationId=*/1,
83+
g.opacityTag,
84+
folly::dynamic::object("type", "frames")("frames", frames)(
85+
"toValue", 1.0f),
86+
std::nullopt);
87+
88+
const double startTimeInTick = 1000;
89+
for (int frame = 0; frame <= 4; ++frame) {
90+
runAnimationFrame(startTimeInTick + frame * SingleFrameIntervalMs);
91+
92+
auto props = nodesManager_->getManagedProps(g.viewTag);
93+
ASSERT_FALSE(props.isNull());
94+
ASSERT_TRUE(props.isObject());
95+
ASSERT_EQ(props.count("opacity"), 1u);
96+
97+
const double live = nodesManager_->getValue(g.opacityTag).value();
98+
EXPECT_DOUBLE_EQ(props["opacity"].asDouble(), live);
99+
100+
if (frame == 0) {
101+
EXPECT_DOUBLE_EQ(live, 0.0);
102+
}
103+
}
104+
105+
EXPECT_DOUBLE_EQ(nodesManager_->getValue(g.opacityTag).value(), 1.0);
106+
auto finalProps = nodesManager_->getManagedProps(g.viewTag);
107+
ASSERT_EQ(finalProps.count("opacity"), 1u);
108+
EXPECT_DOUBLE_EQ(finalProps["opacity"].asDouble(), 1.0);
109+
}
110+
111+
// Without the connect-time seed props_ is empty at connect, so the override has
112+
// nothing to merge: the opacity assertion fails pre-hardening, passes post.
113+
TEST_F(
114+
ManagedPropsMountingOverrideTests,
115+
getManagedPropsLiveImmediatelyOnConnect) {
116+
initNodesManager();
117+
118+
auto g = buildOpacityGraph(/*initialOpacity=*/0.8f);
119+
nodesManager_->connectAnimatedNodeToView(g.propsTag, g.viewTag);
120+
// No runAnimationFrame: the value must be live the instant the view is
121+
// managed.
122+
123+
auto props = nodesManager_->getManagedProps(g.viewTag);
124+
EXPECT_FALSE(props.isNull());
125+
ASSERT_TRUE(props.isObject());
126+
ASSERT_EQ(props.count("opacity"), 1u);
127+
EXPECT_DOUBLE_EQ(props["opacity"].asDouble(), static_cast<double>(0.8f));
128+
}
129+
130+
// A brand-new view connected to an already mid-flight shared value, with no
131+
// frame in between: fails pre-hardening, passes post.
132+
TEST_F(
133+
ManagedPropsMountingOverrideTests,
134+
getManagedPropsLiveOnConnectWhileValueMidFlight) {
135+
initNodesManager();
136+
137+
auto g = buildOpacityGraph(/*initialOpacity=*/0.0f);
138+
nodesManager_->connectAnimatedNodeToView(g.propsTag, g.viewTag);
139+
140+
const auto frames = folly::dynamic::array(0.0f, 0.25f, 0.5f, 0.75f, 1.0f);
141+
nodesManager_->startAnimatingNode(
142+
/*animationId=*/1,
143+
g.opacityTag,
144+
folly::dynamic::object("type", "frames")("frames", frames)(
145+
"toValue", 1.0f),
146+
std::nullopt);
147+
runAnimationFrame(1000);
148+
runAnimationFrame(1000 + 2 * SingleFrameIntervalMs);
149+
150+
const double live = nodesManager_->getValue(g.opacityTag).value();
151+
ASSERT_GT(live, 0.0);
152+
ASSERT_LT(live, 1.0);
153+
154+
const auto styleTag2 = ++rootTag_;
155+
nodesManager_->createAnimatedNode(
156+
styleTag2,
157+
folly::dynamic::object("type", "style")(
158+
"style", folly::dynamic::object("opacity", g.opacityTag)));
159+
nodesManager_->connectAnimatedNodes(g.opacityTag, styleTag2);
160+
const auto propsTag2 = ++rootTag_;
161+
nodesManager_->createAnimatedNode(
162+
propsTag2,
163+
folly::dynamic::object("type", "props")(
164+
"props", folly::dynamic::object("style", styleTag2)));
165+
nodesManager_->connectAnimatedNodes(styleTag2, propsTag2);
166+
const auto viewTag2 = ++rootTag_;
167+
nodesManager_->connectAnimatedNodeToView(propsTag2, viewTag2);
168+
169+
auto props = nodesManager_->getManagedProps(viewTag2);
170+
EXPECT_FALSE(props.isNull());
171+
ASSERT_EQ(props.count("opacity"), 1u);
172+
EXPECT_DOUBLE_EQ(props["opacity"].asDouble(), live);
173+
}
174+
175+
TEST_F(
176+
ManagedPropsMountingOverrideTests,
177+
hasManagedPropsTracksConnectAndDisconnect) {
178+
initNodesManager();
179+
180+
auto g = buildOpacityGraph(/*initialOpacity=*/0.5f);
181+
182+
EXPECT_FALSE(nodesManager_->hasManagedProps());
183+
EXPECT_TRUE(nodesManager_->getManagedProps(g.viewTag).isNull());
184+
185+
nodesManager_->connectAnimatedNodeToView(g.propsTag, g.viewTag);
186+
EXPECT_TRUE(nodesManager_->hasManagedProps());
187+
EXPECT_FALSE(nodesManager_->getManagedProps(g.viewTag).isNull());
188+
189+
nodesManager_->disconnectAnimatedNodeFromView(g.propsTag, g.viewTag);
190+
EXPECT_FALSE(nodesManager_->hasManagedProps());
191+
EXPECT_TRUE(nodesManager_->getManagedProps(g.viewTag).isNull());
192+
}
193+
194+
TEST_F(ManagedPropsMountingOverrideTests, getManagedPropsIsolatedPerView) {
195+
initNodesManager();
196+
197+
auto g = buildOpacityGraph(/*initialOpacity=*/0.3f);
198+
nodesManager_->connectAnimatedNodeToView(g.propsTag, g.viewTag);
199+
200+
EXPECT_FALSE(nodesManager_->getManagedProps(g.viewTag).isNull());
201+
EXPECT_TRUE(nodesManager_->getManagedProps(g.viewTag + 1000).isNull());
202+
}
203+
204+
} // namespace facebook::react

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4146,6 +4146,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
41464146
public facebook::react::Tag connectedViewTag() const;
41474147
public folly::dynamic props();
41484148
public virtual void update() override;
4149+
public void collectProps();
41494150
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
41504151
public void connectToView(facebook::react::Tag viewTag);
41514152
public void disconnectFromShadowNodeFamily();

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4000,6 +4000,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
40004000
public facebook::react::Tag connectedViewTag() const;
40014001
public folly::dynamic props();
40024002
public virtual void update() override;
4003+
public void collectProps();
40034004
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
40044005
public void connectToView(facebook::react::Tag viewTag);
40054006
public void disconnectFromShadowNodeFamily();

‎scripts/cxx-api/api-snapshots/ReactAndroidReleaseCxx.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4143,6 +4143,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
41434143
public facebook::react::Tag connectedViewTag() const;
41444144
public folly::dynamic props();
41454145
public virtual void update() override;
4146+
public void collectProps();
41464147
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
41474148
public void connectToView(facebook::react::Tag viewTag);
41484149
public void disconnectFromShadowNodeFamily();

‎scripts/cxx-api/api-snapshots/ReactAppleDebugCxx.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6329,6 +6329,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
63296329
public facebook::react::Tag connectedViewTag() const;
63306330
public folly::dynamic props();
63316331
public virtual void update() override;
6332+
public void collectProps();
63326333
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
63336334
public void connectToView(facebook::react::Tag viewTag);
63346335
public void disconnectFromShadowNodeFamily();

‎scripts/cxx-api/api-snapshots/ReactAppleNewarchCxx.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6211,6 +6211,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
62116211
public facebook::react::Tag connectedViewTag() const;
62126212
public folly::dynamic props();
62136213
public virtual void update() override;
6214+
public void collectProps();
62146215
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
62156216
public void connectToView(facebook::react::Tag viewTag);
62166217
public void disconnectFromShadowNodeFamily();

‎scripts/cxx-api/api-snapshots/ReactAppleReleaseCxx.api‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6326,6 +6326,7 @@ class facebook::react::PropsAnimatedNode : public facebook::react::AnimatedNode
63266326
public facebook::react::Tag connectedViewTag() const;
63276327
public folly::dynamic props();
63286328
public virtual void update() override;
6329+
public void collectProps();
63296330
public void connectToShadowNodeFamily(facebook::react::ShadowNodeFamily::Weak shadowNodeFamily);
63306331
public void connectToView(facebook::react::Tag viewTag);
63316332
public void disconnectFromShadowNodeFamily();

0 commit comments

Comments
 (0)