Skip to content

Commit 19a6722

Browse files
kwy404meta-codesync[bot]
authored andcommitted
Fix stale layout when a child switches to display: contents after insertion (#58721)
Summary: Pull Request resolved: #58721 Each node caches how many of its children use `display: contents` (`contentsChildrenCount_`, added in react/yoga#1726). The count is updated in `insertChild`, `removeChild`, `replaceChild` and `setChildren`, but not when the display of a child that is already attached changes. So if `YGNodeStyleSetDisplay(child, YGDisplayContents)` is called after the child was inserted, the owner still reports `hasContentsChildren() == false`. The layout algorithm then skips `cleanupContentsNodesRecursively` for it, and the contents node keeps `isDirty == true` after layout. When a descendant of that node changes later, `markDirtyAndPropagate` stops at the contents node because it is already dirty, the root is never marked dirty, and the next `YGNodeCalculateLayout` returns the cached layout. Minimal repro: ```cpp YGNodeRef root = YGNodeNew(); YGNodeStyleSetWidth(root, 100); YGNodeStyleSetHeight(root, 100); YGNodeRef child = YGNodeNew(); YGNodeInsertChild(root, child, 0); YGNodeRef grandchild = YGNodeNew(); YGNodeStyleSetWidth(grandchild, 10); YGNodeStyleSetHeight(grandchild, 10); YGNodeInsertChild(child, grandchild, 0); YGNodeStyleSetDisplay(child, YGDisplayContents); YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); // YGNodeIsDirty(child) is still true YGNodeStyleSetWidth(grandchild, 20); // YGNodeIsDirty(root) is false YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR); // YGNodeLayoutGetWidth(grandchild) is 10, expected 20 ``` This affects any binding that mutates nodes in place (the JS and Java bindings both go through `YGNodeStyleSetDisplay`). Setting the display before inserting the child works, which is what the existing tests do. The fix recomputes the owner's count in `YGNodeStyleSetDisplay` when a node that has an owner switches to or from `display: contents`. Changelog: [General][Fixed] - Fix stale layout when a child switches to `display: contents` after being inserted X-link: react/yoga#2028 Test Plan: Added `dirty_propagation_through_child_set_to_display_contents` to `tests/YGDirtyMarkingTest.cpp`. Without the fix, all three of its assertions fail (the contents node stays dirty, the root is not marked dirty, and the grandchild keeps width 10). With the fix it passes. Ran the full C++ suite (`unit_tests.bat`, MSVC 2022 + Ninja): 855 tests passed. Checked formatting of the changed files with the repo's clang-format version (21.1.2): `clang-format --dry-run --Werror yoga/YGNodeStyle.cpp tests/YGDirtyMarkingTest.cpp` reports no changes. Reviewed By: javache Differential Revision: D122122520 Pulled By: cipolleschi fbshipit-source-id: 79433cfb8112c9e8596f1ae604de56055860d814
1 parent 7ca1b42 commit 19a6722

1 file changed

Lines changed: 9 additions & 0 deletions

File tree

‎packages/react-native/ReactCommon/yoga/yoga/YGNodeStyle.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,16 @@ YGOverflow YGNodeStyleGetOverflow(const YGNodeConstRef node) {
152152
}
153153

154154
void YGNodeStyleSetDisplay(const YGNodeRef node, const YGDisplay display) {
155+
const bool wasContents =
156+
resolveRef(node)->style().display() == Display::Contents;
155157
updateStyle<&Style::display, &Style::setDisplay>(node, scopedEnum(display));
158+
159+
// The owner caches how many of its children use display: contents, so it
160+
// has to be recomputed when an attached child switches to or from it.
161+
auto owner = resolveRef(node)->getOwner();
162+
if (owner != nullptr && wasContents != (display == YGDisplayContents)) {
163+
owner->setChildren(owner->getChildren());
164+
}
156165
}
157166

158167
YGDisplay YGNodeStyleGetDisplay(const YGNodeConstRef node) {

0 commit comments

Comments
 (0)