Skip to content

Commit fe0df65

Browse files
mozziusmeta-codesync[bot]
authored andcommitted
Don't dirty display: none nodes because of their children (#59113)
Summary: `YogaLayoutableShadowNode::updateYogaChildren` marks a cloned node dirty if any of its children is dirty. For a `display: none` node that's never needed: Yoga lays it out with zero size whatever its children are. It's also actively costly. When Yoga lays out a hidden node's parent, `zeroOutLayoutRecursively` resets the hidden subtree and the hidden node is marked clean, but its descendants keep their dirty flags. Fabric's `layout()` doesn't recurse into `display: none` nodes either. So a child that was created or changed while its parent was hidden stays dirty until the parent is shown. Every later commit that clones the hidden node sees that child, dirties the hidden node and every ancestor up to the root, and `RootShadowNode::layoutIfNeeded` re-runs `layoutTree`, including `roundLayoutResultsToPixelGrid` over every node in the surface. React commits lay out from the root anyway, so this costs the commits that don't go through React: `setNativeProps`, JS-driven `Animated` (which uses `setNativeProps` on Fabric), native state updates and animation libraries that commit to the ShadowTree. The clearest case in core is a JS-driven `Animated.loop` on a view inside a `display: 'none'` container, which re-lays out the whole surface on every frame. Content hidden by `<Activity mode="hidden">` ends up in the same state, since the renderer hides it by cloning the top-level host nodes with `display: 'none'`. This change skips the children comparison for `display: none` nodes. It keeps the node's own dirty check and the child count check. Showing the node again changes its style, which dirties it in `updateYogaProps`, and the children that changed while it was hidden are laid out then. Fixes #59112 ## Changelog: [GENERAL] [FIXED] - Commits under a `display: 'none'` view no longer re-run layout over the whole surface when something under it was created or changed while it was hidden Pull Request resolved: #59113 Test Plan: **Unit tests:** two new `YogaDirtyFlagTest` cases in `ViewTest.cpp`: - `dirtyChildOfHiddenNodeMustNotDirtyIt`: hides the inner node, changes a child's width while it's hidden, then changes the child's opacity. Neither commit may need layout. - `showingHiddenNodeMustDirtyItAndLayOutItsChildren`: hides the inner node, changes a child's width while hidden, then shows the node again. That commit must need layout, and the child must come out 42 wide. There's no OSS runner for these, so I compiled the `YogaDirtyFlagTest` cases from this branch into the repro app, against RN 0.87.1 built from source, with a minimal stand-in for the gtest macros. With this change, all 10 cases pass. Without it, `dirtyChildOfHiddenNodeMustNotDirtyIt` fails both of its `EXPECT_FALSE` checks and the other 9 pass. I'm still relying on CI to run them properly. **End to end:** with the repro from #59112 (https://github.com/mozzius/hidden-subtree-relayout-repro). Its `fixed` branch applies this exact change to RN 0.87.1 built from source. iOS 26.5 simulator, Release, 1000 rows on screen; per-commit time on the JS thread, median / p90 over 9 runs of 120 commits: | | stock | with this change | |---|---:|---:| | `setNativeProps` on a view created inside a `display: 'none'` parent | 1.06 / 2.02 ms | 0.15 / 0.21 ms | | … after React re-rendered a sibling while the parent was hidden | 1.01 / 2.06 ms | 0.16 / 0.20 ms | | … inside a hidden `<Activity>`, after a re-render while hidden | 1.18 / 2.03 ms | 0.16 / 0.21 ms | | JS-driven `Animated.loop` on a view inside a `display: 'none'` parent | 1.09 / 1.95 ms | 0.13 / 0.17 ms | | control: `setNativeProps` on a view in a visible parent | 0.19 / 0.23 ms | 0.19 / 0.24 ms | Counting with lldb, stock ran `YogaLayoutableShadowNode::layoutTree` for every commit in the first four rows, and this change runs it for none. A `yoga::Node::setDirty` breakpoint on stock shows each of those commits starting with the hidden node going from clean to dirty in `updateYogaChildren ← ShadowNode::cloneTree ← UIManager::setNativeProps_DEPRECATED`. **Showing hidden views again:** the repro's `-scenario unhide` hides three containers, re-renders a label in each and changes each child 30 times with `setNativeProps` while hidden. It then shows them again with `setNativeProps({style: {display: 'flex'}})`, with a React state change, and by switching an `<Activity>` back to visible. All three lay out the same as a never-hidden reference, with and without this change. **Context:** we found this in the Bluesky app, where a Reanimated `useAnimatedStyle` on a hidden spinner in the profile header committed on every scroll frame, and each commit re-ran layout for the whole screen. We worked around it in app code (bluesky-social/social-app#12000). It's a sibling of #59100, fixed in #59101: the same symptom with a different cause, and the two changes are independent. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed By: christophpurrer Differential Revision: D125454304 Pulled By: javache fbshipit-source-id: 1952fa3d9798e1fb5d2405fbb4c47a346008834b
1 parent e04aaf6 commit fe0df65

2 files changed

Lines changed: 106 additions & 3 deletions

File tree

‎packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -357,8 +357,17 @@ void YogaLayoutableShadowNode::updateYogaChildren() {
357357
bool isClean = !YGNodeIsDirty(&yogaNode_) &&
358358
getChildren().size() == YGNodeGetChildCount(&yogaNode_);
359359

360-
auto oldYogaChildren =
361-
isClean ? yogaNode_.getChildren() : std::vector<yoga::Node*>{};
360+
// A `display: none` node is laid out with zero size whatever its children
361+
// are, so its children can't make its layout stale. Yoga never cleans the
362+
// descendants of a hidden node either, so a child that was created or
363+
// changed while the node was hidden stays dirty until the node is shown.
364+
// Comparing children here would dirty the hidden node, and every ancestor
365+
// up to the root, on every commit that clones it. Showing the node again
366+
// changes its style, which dirties it in `updateYogaProps`.
367+
bool isHidden = yogaNode_.style().display() == yoga::Display::None;
368+
369+
auto oldYogaChildren = isClean && !isHidden ? yogaNode_.getChildren()
370+
: std::vector<yoga::Node*>{};
362371

363372
yogaNode_.setChildren({});
364373
yogaLayoutableChildren_.clear();
@@ -371,7 +380,7 @@ void YogaLayoutableShadowNode::updateYogaChildren() {
371380
appendYogaChild(yogaLayoutableChild);
372381
adoptYogaChild(i);
373382

374-
if (isClean) {
383+
if (isClean && !isHidden) {
375384
auto yogaChildIndex = yogaLayoutableChildren_.size() - 1;
376385
auto& oldYogaChildNode = *oldYogaChildren.at(yogaChildIndex);
377386
auto& newYogaChildNode =

‎packages/react-native/ReactCommon/react/renderer/components/view/tests/ViewTest.cpp‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,100 @@ TEST_F(YogaDirtyFlagTest, updatingStateForScrollViewMistNotDirtyYogaNode) {
232232
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded());
233233
}
234234

235+
TEST_F(YogaDirtyFlagTest, dirtyChildOfHiddenNodeMustNotDirtyIt) {
236+
/*
237+
* A `display: none` node is laid out with zero size whatever its children
238+
* are, and Yoga never cleans the descendants of a hidden node. A child that
239+
* changed while its parent was hidden must *not* dirty the parent, otherwise
240+
* every commit that clones the parent re-runs layout from the root.
241+
*/
242+
auto& childFamily = innerShadowNode_->getChildren().at(0)->getFamily();
243+
244+
auto hiddenRootShadowNode = rootShadowNode_->cloneTree(
245+
innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
246+
auto viewProps = std::make_shared<ViewProps>();
247+
viewProps->yogaStyle.setDisplay(yoga::Display::None);
248+
return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
249+
});
250+
251+
/*
252+
* Hiding the node *must* dirty it.
253+
*/
254+
EXPECT_TRUE(
255+
static_cast<RootShadowNode&>(*hiddenRootShadowNode).layoutIfNeeded());
256+
257+
/*
258+
* Changing the layout of a child of the hidden node dirties the child, but
259+
* not the hidden node.
260+
*/
261+
auto changedRootShadowNode = hiddenRootShadowNode->cloneTree(
262+
childFamily, [](const ShadowNode& oldShadowNode) {
263+
auto viewProps = std::make_shared<ViewProps>();
264+
viewProps->yogaStyle.setDimension(
265+
yoga::Dimension::Width, yoga::StyleSizeLength::points(42));
266+
return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
267+
});
268+
269+
EXPECT_FALSE(
270+
static_cast<RootShadowNode&>(*changedRootShadowNode).layoutIfNeeded());
271+
272+
/*
273+
* The child is still dirty, because the layout pass didn't visit it. A
274+
* later change to its non-layout props must not dirty the hidden node
275+
* either.
276+
*/
277+
auto newRootShadowNode = changedRootShadowNode->cloneTree(
278+
childFamily, [](const ShadowNode& oldShadowNode) {
279+
auto viewProps = std::make_shared<ViewProps>();
280+
viewProps->yogaStyle.setDimension(
281+
yoga::Dimension::Width, yoga::StyleSizeLength::points(42));
282+
viewProps->opacity = 0.5;
283+
return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
284+
});
285+
286+
EXPECT_FALSE(
287+
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded());
288+
}
289+
290+
TEST_F(YogaDirtyFlagTest, showingHiddenNodeMustDirtyItAndLayOutItsChildren) {
291+
/*
292+
* Showing a hidden node again *must* dirty it, and the children that
293+
* changed while it was hidden must be laid out.
294+
*/
295+
auto& childFamily = innerShadowNode_->getChildren().at(0)->getFamily();
296+
297+
auto hiddenRootShadowNode = rootShadowNode_->cloneTree(
298+
innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
299+
auto viewProps = std::make_shared<ViewProps>();
300+
viewProps->yogaStyle.setDisplay(yoga::Display::None);
301+
return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
302+
});
303+
static_cast<RootShadowNode&>(*hiddenRootShadowNode).layoutIfNeeded();
304+
305+
auto changedRootShadowNode = hiddenRootShadowNode->cloneTree(
306+
childFamily, [](const ShadowNode& oldShadowNode) {
307+
auto viewProps = std::make_shared<ViewProps>();
308+
viewProps->yogaStyle.setDimension(
309+
yoga::Dimension::Width, yoga::StyleSizeLength::points(42));
310+
return oldShadowNode.clone(ShadowNodeFragment{.props = viewProps});
311+
});
312+
static_cast<RootShadowNode&>(*changedRootShadowNode).layoutIfNeeded();
313+
314+
auto shownRootShadowNode = changedRootShadowNode->cloneTree(
315+
innerShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
316+
return oldShadowNode.clone(
317+
ShadowNodeFragment{.props = std::make_shared<ViewProps>()});
318+
});
319+
320+
EXPECT_TRUE(
321+
static_cast<RootShadowNode&>(*shownRootShadowNode).layoutIfNeeded());
322+
323+
auto child = std::static_pointer_cast<const ViewShadowNode>(
324+
shownRootShadowNode->getChildren().at(1)->getChildren().at(0));
325+
EXPECT_EQ(child->getLayoutMetrics().displayType, DisplayType::Flex);
326+
EXPECT_EQ(child->getLayoutMetrics().frame.size.width, 42);
327+
}
328+
235329
TEST_F(YogaDirtyFlagTest, clonedPropsPreserveAspectRatio) {
236330
ContextContainer contextContainer{};
237331
PropsParserContext parserContext{-1, contextContainer};

0 commit comments

Comments
 (0)