Repository navigation
Conversation
A display: none node is laid out with zero size whatever its children are, and Yoga never cleans the descendants of a hidden node. updateYogaChildren dirtied a hidden node whenever it was cloned with a dirty child, so a child created or changed while the node was hidden made every later commit under it re-run layout from the root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D125454304. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
YogaLayoutableShadowNode::updateYogaChildrenmarks a cloned node dirty if any of its children is dirty. For adisplay: nonenode 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,
zeroOutLayoutRecursivelyresets the hidden subtree and the hidden node is marked clean, but its descendants keep their dirty flags. Fabric'slayout()doesn't recurse intodisplay: nonenodes 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, andRootShadowNode::layoutIfNeededre-runslayoutTree, includingroundLayoutResultsToPixelGridover 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-drivenAnimated(which usessetNativePropson Fabric), native state updates and animation libraries that commit to the ShadowTree. The clearest case in core is a JS-drivenAnimated.loopon a view inside adisplay: '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 withdisplay: 'none'.This change skips the children comparison for
display: nonenodes. It keeps the node's own dirty check and the child count check. Showing the node again changes its style, which dirties it inupdateYogaProps, 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 hiddenTest Plan:
Unit tests: two new
YogaDirtyFlagTestcases inViewTest.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
YogaDirtyFlagTestcases 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,dirtyChildOfHiddenNodeMustNotDirtyItfails both of itsEXPECT_FALSEchecks 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
fixedbranch 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:setNativePropson a view created inside adisplay: 'none'parent<Activity>, after a re-render while hiddenAnimated.loopon a view inside adisplay: 'none'parentsetNativePropson a view in a visible parentCounting with lldb, stock ran
YogaLayoutableShadowNode::layoutTreefor every commit in the first four rows, and this change runs it for none. Ayoga::Node::setDirtybreakpoint on stock shows each of those commits starting with the hidden node going from clean to dirty inupdateYogaChildren ← ShadowNode::cloneTree ← UIManager::setNativeProps_DEPRECATED.Showing hidden views again: the repro's
-scenario unhidehides three containers, re-renders a label in each and changes each child 30 times withsetNativePropswhile hidden. It then shows them again withsetNativeProps({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
useAnimatedStyleon 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