Skip to content

Commit 70f655a

Browse files
authored
Only keep a cloned node's layout when its children's layout results match
Summary: When a host node is cloned with its children passed in (`passChildrenWhenCloningPersistedNodes`), `YogaLayoutableShadowNode::updateYogaChildren` keeps the clone's layout clean if every new child is clean and styled like the child it replaces. That misses children that are clean and styled the same but carry a different layout. Yoga then reuses the parent's cached layout, never visits those children, and they keep their stale layout. Runtime shadow node reference updates produce exactly such children when `<Activity>` hides a subtree: 1. The renderer commits a `display: none` clone H of the hidden subtree's host root V. H gets its own runtime reference; the renderer keeps V, which still holds the layout and layout cache from when it was visible. 2. Laying out H zeroes out clones of V's children. Those clones inherit the children's runtime references, so the renderer now holds the zeroed revisions of V's children while it still holds the non-zeroed V. 3. Any re-render inside the subtree clones those zeroed children and then V with the new children. Every check passes, so V's clone keeps V's cached layout. 4. When the subtree is revealed, Yoga reuses that cache and the content stays at 0x0. With the children appended one by one instead, the append always dirties the parent, so this only shows up with the flag on. Also require each new child's layout results (`yoga::LayoutResults`: computed layout and layout cache) to match those of the child it replaces before keeping the clone clean. A parent's cached layout is only valid for the layouts its children had when it was computed. A child cloned without layout changes copies its layout results, so the common case keeps the clone clean. `configVersion` is left out of the comparison: every clone gets its own Yoga config, and `Node::setConfig()` re-aligns the layout's `configVersion` with it. Comparing it would, for example, dirty a `display: none` parent's ancestors on every commit that clones it, which `YogaDirtyFlagTest.dirtyChildOfHiddenNodeMustNotDirtyIt` guards against. Changelog: [General][Fixed] - Fix content laid out at zero size after revealing an `<Activity>` subtree when children are passed into clones Reviewed By: javache Differential Revision: D131502045 Pull Request resolved: #59984
1 parent ad2e8f2 commit 70f655a

3 files changed

Lines changed: 82 additions & 3 deletions

File tree

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,17 @@ void YogaLayoutableShadowNode::updateYogaChildrenOwnersIfNeeded() {
347347
}
348348
}
349349

350+
// Every clone gets its own Yoga config, and `setConfig()` re-aligns the
351+
// layout's `configVersion` with it, so that field says nothing about whether
352+
// two nodes carry the same layout. Everything else has to match.
353+
static bool hasSameLayoutResults(
354+
const yoga::Node& yogaNode,
355+
const yoga::Node& otherYogaNode) {
356+
auto layout = yogaNode.getLayout();
357+
layout.configVersion = otherYogaNode.getLayout().configVersion;
358+
return layout == otherYogaNode.getLayout();
359+
}
360+
350361
void YogaLayoutableShadowNode::updateYogaChildren() {
351362
if (getTraits().check(ShadowNodeTraits::Trait::LeafYogaNode)) {
352363
return;
@@ -386,8 +397,11 @@ void YogaLayoutableShadowNode::updateYogaChildren() {
386397
auto& newYogaChildNode =
387398
yogaLayoutableChildren_.at(yogaChildIndex)->yogaNode_;
388399

400+
// A clean child styled the same can still carry another layout, e.g. a
401+
// revision zeroed out under a `display: none` clone of this node.
389402
isClean = isClean && !newYogaChildNode.isDirty() &&
390-
(newYogaChildNode.style() == oldYogaChildNode.style());
403+
(newYogaChildNode.style() == oldYogaChildNode.style()) &&
404+
hasSameLayoutResults(newYogaChildNode, oldYogaChildNode);
391405
}
392406
}
393407
}

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,59 @@ TEST_F(YogaCloneTest, newChildrenCloneReflectsNewList) {
559559
100);
560560
}
561561

562+
// A clone that is handed its children must not keep its layout when a child,
563+
// although clean and styled the same, carries a different computed layout.
564+
// That is the case for a child revision zeroed out under a `display: none`
565+
// clone of the parent, which React can hold through runtime shadow node
566+
// reference updates while it keeps cloning from the visible parent.
567+
TEST_F(YogaCloneTest, newChildrenWithDifferentLayoutDirtyTheClone) {
568+
auto hiddenRoot = rootShadowNode_->cloneTree(
569+
parentShadowNode_->getFamily(), [](const ShadowNode& oldShadowNode) {
570+
auto hiddenProps = std::make_shared<ViewProps>();
571+
auto& yogaStyle = hiddenProps->yogaStyle;
572+
yogaStyle.setFlexDirection(yoga::FlexDirection::Row);
573+
yogaStyle.setDimension(
574+
yoga::Dimension::Width, yoga::StyleSizeLength::points(300));
575+
yogaStyle.setDimension(
576+
yoga::Dimension::Height, yoga::StyleSizeLength::points(100));
577+
yogaStyle.setDisplay(yoga::Display::None);
578+
return oldShadowNode.clone({.props = hiddenProps});
579+
});
580+
static_cast<RootShadowNode&>(*hiddenRoot).layoutIfNeeded();
581+
582+
auto zeroedChildA = hiddenRoot->getChildren()[0]->getChildren()[0];
583+
ASSERT_EQ(
584+
yogaNodeOf(*zeroedChildA).getLayout().dimension(yoga::Dimension::Width),
585+
0);
586+
ASSERT_FALSE(yogaNodeOf(*zeroedChildA).isDirty());
587+
588+
auto newChildren =
589+
std::make_shared<const std::vector<std::shared_ptr<const ShadowNode>>>(
590+
std::vector<std::shared_ptr<const ShadowNode>>{
591+
zeroedChildA->clone({}),
592+
parentShadowNode_->getChildren()[1],
593+
parentShadowNode_->getChildren()[2]});
594+
595+
auto clonedParent = parentShadowNode_->clone(
596+
{.props = ShadowNodeFragment::propsPlaceholder(),
597+
.children = newChildren});
598+
599+
EXPECT_FALSE(
600+
static_cast<const YogaLayoutableShadowNode&>(*clonedParent)
601+
.getIsLayoutClean());
602+
603+
auto newRootShadowNode = rootShadowNode_->cloneTree(
604+
parentShadowNode_->getFamily(),
605+
[&](const ShadowNode& /*oldShadowNode*/) { return clonedParent; });
606+
static_cast<RootShadowNode&>(*newRootShadowNode).layoutIfNeeded();
607+
608+
const auto& laidOutChildA =
609+
yogaNodeOf(*newRootShadowNode->getChildren()[0]->getChildren()[0])
610+
.getLayout();
611+
EXPECT_EQ(laidOutChildA.dimension(yoga::Dimension::Width), 100);
612+
EXPECT_EQ(laidOutChildA.dimension(yoga::Dimension::Height), 100);
613+
}
614+
562615
// Successive prop-only clones (the animation commit pattern) must each
563616
// produce a self-consistent layout. Validates that the fast path's vector
564617
// copy yields a vector the next clone can again copy from.

‎packages/react-native/ReactCommon/react/renderer/core/__docs__/passChildrenWhenCloning.md‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,17 @@ of mutation.
4848

4949
The clone implementation of the YogaLayoutableShadowNode will compare the
5050
previous set of children with the new set. If both sets have the same size and
51-
each node has equivalent styling, the cloned node is not being dirtied for
52-
layout. This allows for improving the layout data re-use on subsequent renders.
51+
each new child is clean, has equivalent styling and carries the same layout
52+
results (computed layout and layout cache) as the child it replaces, the cloned
53+
node is not being dirtied for layout. This allows for improving the layout data
54+
re-use on subsequent renders.
55+
56+
The layout results have to be compared as well: a child can be clean and styled
57+
the same while carrying a layout that the parent's cached layout was not
58+
computed with. Runtime shadow node reference updates can point the renderer at
59+
such a revision. For example, when `<Activity>` hides a subtree the renderer
60+
commits a `display: none` clone of the subtree's host root. Laying out that
61+
clone zeroes out clones of its children, and those clones inherit the children's
62+
runtime references, while the renderer keeps cloning from the visible host root.
63+
Without the layout check, the next clone of that host root would keep its cached
64+
layout and Yoga would never revisit the zeroed children.

0 commit comments

Comments
 (0)