Skip to content

Commit 2bacf51

Browse files
kwy404facebook-github-bot
authored andcommitted
Fix stale layout when a child switches to display: contents after insertion
Summary: 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 Reviewed By: javache Differential Revision: D122122520 Pulled By: cipolleschi
1 parent 7ca1b42 commit 2bacf51

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)