Repository navigation
Conversation
Summary: `Style::operator==` compares every style field except `boxSizing_`, so two styles that only differ in box sizing compare equal. `YGNodeCopyStyle` then skips the copy and leaves the destination node with the old box sizing, and callers that compare styles to decide whether a node needs relayout miss the change. Compare `boxSizing_` together with the other enum fields. Reviewed By: cortinico Differential Revision: D125427467
Summary: Update the Fantom regression test to reflect Yoga style equality including `boxSizing`. Assert that native content-box updates invalidate layout on the first rerender, and remove the stale follow-up rerender. Changelog: [Internal] Differential Revision: D125660351
meta-cla
Bot
added
the
CLA Signed
|
@cortinico has exported this pull request. If you are a Meta employee, you can view the originating Diff in D125660351. |
|
This pull request has been merged in 32131bf. |
ide-agent
pushed a commit
to expo/react-native
that referenced
this pull request
Oct 20, 2026
Why === Two guards on the cascade's path asked a node's seal, and a seal exists only in debug builds. In release the configure pass's anonymous-box loop wrote the cascade into atomic inlines a committed revision still held, and the layout pass's defence against a shared child never cloned one. The configure pass's main loop guarded writing a child in place with `doesOwn(child)` alone, a fact about the Yoga layout tree: a cloned container adopts its unchanged children into its Yoga tree, so the owner pointer said "mine" while those children were still the ones the committed revision held, and the loop wrote `receivedTextAttributes_`, `listDepth_` and `markDirtyAndPropagate()` into shared state. `ShadowTree::tryCommit` builds and lays out a candidate with no lock, so that speculative phase mutating another revision's nodes is the whole hazard. Anonymous inline boxes are not in `children_`, so nothing sealed or published them. The clone constructor also sets the cost of an animation. `AnimationBackend` commits the shadow tree on every frame of a native-driven animation, and `cloneMultipleRecursive` copies each ancestor's children vector and swaps one entry, after which `updateYogaChildren()` scanned every child for foreign Yoga owners, detached the whole list and rebuilt it, so an animation's cost scaled with how many siblings the animated node had. How === `ShadowNode::isPublished()` is the release-visible copy-on-write fact: set when a revision commits, so a node created or cloned in the current commit is unpublished until then. The anonymous-box loop hands the cascade only to a child that is the revision's own. An atomic inline takes the cascade where the renderer already clones it: `layoutInlineAttachments` hands each attachment clone the cascade (`receiveCascade`), folded from the box's stored cascade through the inheritable props of the inline boxes on the way, so a consumer publishes from the node the revision mounts. The layout pass's defence asks `isPublished()`. The configure pass's guard asks both questions, `doesOwn(child) && !child.isPublished()`, so a published child takes the `cloneChildInPlace` path already in the else-branch. It cannot race: a commit reaches nodes only through a published revision and everything in one is published, so an unpublished node belongs to this commit alone. `YogaLayoutableShadowNode` overrides `sealOwnedContentRecursive` to publish its anonymous boxes, and the runs under them, with it. A clone whose new children list is the same length and differs only by replacement updates its Yoga children in place, on the Yoga side only: `replaceChild` would route through `ShadowNode::replaceChild` and copy the children vector back to write a child `children_` already holds. The path is taken only where it is provably equivalent: no anonymous text boxes (they are generated from the children, so a replacement can change which runs exist), equal lengths, and every child layoutable so the two index spaces line up. A replacement child whose Yoga node another node already owns is left to the rebuild, which clones it through `adoptYogaChild`, and so is one that becomes inline-level or stops being one, because the rebuild builds the runs it joins or leaves. The node is dirtied unconditionally, since reaching the path means the caller replaced the children list and `replaceChild` does not dirty, and `SubtreeHasCascadeDependents` is recomputed. Anything else rebuilds as before, and the probe is a pointer walk. The register's `eager-yoga-node` performance entry gains its marker in `YogaLayoutableShadowNode.h`. Test Plan ========= `yarn fantom` over `DynamicRangeLimitInheritance`, `DynamicRangeLimitOnViews`, `CascadeBoundary`, `StringChildren`, `StringChildrenBehavior` and the expo-intrinsics inline-image suites passes against the rebuilt tester; the inline `<img>` case that proves the attachment path lives with the HTML elements branch. Upstream's `View-flexBasisFitContent-itest` (react#59137) expects the native `boxSizing` width, 92 (83 plus 6 of padding and 3 of border), from the commit that carries it; layout that writes through to children the committed revision still holds reads the stale 83 through that commit and the next render. Sampled on a 100,000-message chat transcript during a send, without the in-place path: AnimationBackend::commitUpdates 1779 samples ShadowTree::commit -> tryCommit 1336 YogaLayoutableShadowNode ctor 1331 updateYogaChildren() 1285 replaceChild() 1284 With it, the send's flight at 100,000 messages takes 1677ms instead of 6061ms. Co-authored-by: ide <379606+ide@users.noreply.github.com>
ide-agent
pushed a commit
to expo/react-native
that referenced
this pull request
Oct 20, 2026
Why === Two guards on the cascade's path asked a node's seal, and a seal exists only in debug builds. In release the configure pass's anonymous-box loop wrote the cascade into atomic inlines a committed revision still held, and the layout pass's defence against a shared child never cloned one. The configure pass's main loop guarded writing a child in place with `doesOwn(child)` alone, a fact about the Yoga layout tree: a cloned container adopts its unchanged children into its Yoga tree, so the owner pointer said "mine" while those children were still the ones the committed revision held, and the loop wrote `receivedTextAttributes_`, `listDepth_` and `markDirtyAndPropagate()` into shared state. `ShadowTree::tryCommit` builds and lays out a candidate with no lock, so that speculative phase mutating another revision's nodes is the whole hazard. Anonymous inline boxes are not in `children_`, so nothing sealed or published them. The clone constructor also sets the cost of an animation. `AnimationBackend` commits the shadow tree on every frame of a native-driven animation, and `cloneMultipleRecursive` copies each ancestor's children vector and swaps one entry, after which `updateYogaChildren()` scanned every child for foreign Yoga owners, detached the whole list and rebuilt it, so an animation's cost scaled with how many siblings the animated node had. How === `ShadowNode::isPublished()` is the release-visible copy-on-write fact: set when a revision commits, so a node created or cloned in the current commit is unpublished until then. The anonymous-box loop hands the cascade only to a child that is the revision's own. An atomic inline takes the cascade where the renderer already clones it: `layoutInlineAttachments` hands each attachment clone the cascade (`receiveCascade`), folded from the box's stored cascade through the inheritable props of the inline boxes on the way, so a consumer publishes from the node the revision mounts. The layout pass's defence asks `isPublished()`. The configure pass's guard asks both questions, `doesOwn(child) && !child.isPublished()`, so a published child takes the `cloneChildInPlace` path already in the else-branch. It cannot race: a commit reaches nodes only through a published revision and everything in one is published, so an unpublished node belongs to this commit alone. `YogaLayoutableShadowNode` overrides `sealOwnedContentRecursive` to publish its anonymous boxes, and the runs under them, with it. A clone whose new children list is the same length and differs only by replacement updates its Yoga children in place, on the Yoga side only: `replaceChild` would route through `ShadowNode::replaceChild` and copy the children vector back to write a child `children_` already holds. The path is taken only where it is provably equivalent: no anonymous text boxes (they are generated from the children, so a replacement can change which runs exist), equal lengths, and every child layoutable so the two index spaces line up. A replacement child whose Yoga node another node already owns is left to the rebuild, which clones it through `adoptYogaChild`, and so is one that becomes inline-level or stops being one, because the rebuild builds the runs it joins or leaves. The node is dirtied unconditionally, since reaching the path means the caller replaced the children list and `replaceChild` does not dirty, and `SubtreeHasCascadeDependents` is recomputed. Anything else rebuilds as before, and the probe is a pointer walk. The register's `eager-yoga-node` performance entry gains its marker in `YogaLayoutableShadowNode.h`. Test Plan ========= `yarn fantom` over `DynamicRangeLimitInheritance`, `DynamicRangeLimitOnViews`, `CascadeBoundary`, `StringChildren`, `StringChildrenBehavior` and the expo-intrinsics inline-image suites passes against the rebuilt tester; the inline `<img>` case that proves the attachment path lives with the HTML elements branch. Upstream's `View-flexBasisFitContent-itest` (react#59137) expects the native `boxSizing` width, 92 (83 plus 6 of padding and 3 of border), from the commit that carries it; layout that writes through to children the committed revision still holds reads the stale 83 through that commit and the next render. Sampled on a 100,000-message chat transcript during a send, without the in-place path: AnimationBackend::commitUpdates 1779 samples ShadowTree::commit -> tryCommit 1336 YogaLayoutableShadowNode ctor 1331 updateYogaChildren() 1285 replaceChild() 1284 With it, the send's flight at 100,000 messages takes 1677ms instead of 6061ms. Co-authored-by: ide <379606+ide@users.noreply.github.com>
ide-agent
pushed a commit
to expo/react-native
that referenced
this pull request
Oct 20, 2026
Why === Two guards on the cascade's path asked a node's seal, and a seal exists only in debug builds. In release the configure pass's anonymous-box loop wrote the cascade into atomic inlines a committed revision still held, and the layout pass's defence against a shared child never cloned one. The configure pass's main loop guarded writing a child in place with `doesOwn(child)` alone, a fact about the Yoga layout tree: a cloned container adopts its unchanged children into its Yoga tree, so the owner pointer said "mine" while those children were still the ones the committed revision held, and the loop wrote `receivedTextAttributes_`, `listDepth_` and `markDirtyAndPropagate()` into shared state. `ShadowTree::tryCommit` builds and lays out a candidate with no lock, so that speculative phase mutating another revision's nodes is the whole hazard. Anonymous inline boxes are not in `children_`, so nothing sealed or published them. The clone constructor also sets the cost of an animation. `AnimationBackend` commits the shadow tree on every frame of a native-driven animation, and `cloneMultipleRecursive` copies each ancestor's children vector and swaps one entry, after which `updateYogaChildren()` scanned every child for foreign Yoga owners, detached the whole list and rebuilt it, so an animation's cost scaled with how many siblings the animated node had. How === `ShadowNode::isPublished()` is the release-visible copy-on-write fact: set when a revision commits, so a node created or cloned in the current commit is unpublished until then. The anonymous-box loop hands the cascade only to a child that is the revision's own. An atomic inline takes the cascade where the renderer already clones it: `layoutInlineAttachments` hands each attachment clone the cascade (`receiveCascade`), folded from the box's stored cascade through the inheritable props of the inline boxes on the way, so a consumer publishes from the node the revision mounts. The layout pass's defence asks `isPublished()`. The configure pass's guard asks both questions, `doesOwn(child) && !child.isPublished()`, so a published child takes the `cloneChildInPlace` path already in the else-branch. It cannot race: a commit reaches nodes only through a published revision and everything in one is published, so an unpublished node belongs to this commit alone. `YogaLayoutableShadowNode` overrides `sealOwnedContentRecursive` to publish its anonymous boxes, and the runs under them, with it. A clone whose new children list is the same length and differs only by replacement updates its Yoga children in place, on the Yoga side only: `replaceChild` would route through `ShadowNode::replaceChild` and copy the children vector back to write a child `children_` already holds. The path is taken only where it is provably equivalent: no anonymous text boxes (they are generated from the children, so a replacement can change which runs exist), equal lengths, and every child layoutable so the two index spaces line up. A replacement child whose Yoga node another node already owns is left to the rebuild, which clones it through `adoptYogaChild`, and so is one that becomes inline-level or stops being one, because the rebuild builds the runs it joins or leaves. The node is dirtied unconditionally, since reaching the path means the caller replaced the children list and `replaceChild` does not dirty, and `SubtreeHasCascadeDependents` is recomputed. Anything else rebuilds as before, and the probe is a pointer walk. The register's `eager-yoga-node` performance entry gains its marker in `YogaLayoutableShadowNode.h`. Test Plan ========= `yarn fantom` over `DynamicRangeLimitInheritance`, `DynamicRangeLimitOnViews`, `CascadeBoundary`, `StringChildren`, `StringChildrenBehavior` and the expo-intrinsics inline-image suites passes against the rebuilt tester; the inline `<img>` case that proves the attachment path lives with the HTML elements branch. Upstream's `View-flexBasisFitContent-itest` (react#59137) expects the native `boxSizing` width, 92 (83 plus 6 of padding and 3 of border), from the commit that carries it; layout that writes through to children the committed revision still holds reads the stale 83 through that commit and the next render. Sampled on a 100,000-message chat transcript during a send, without the in-place path: AnimationBackend::commitUpdates 1779 samples ShadowTree::commit -> tryCommit 1336 YogaLayoutableShadowNode ctor 1331 updateYogaChildren() 1285 replaceChild() 1284 With it, the send's flight at 100,000 messages takes 1677ms instead of 6061ms. Co-authored-by: ide <379606+ide@users.noreply.github.com>
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
1 participant
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:
Update the Fantom regression test to reflect Yoga style equality including
boxSizing. Assert that native content-box updates invalidate layout on the first rerender, and remove the stale follow-up rerender.Changelog: [Internal]
Differential Revision: D125660351