Do not retain a flex basis measured during a previous layout - #2020
Open
mozzius wants to merge 1 commit into
Open
Do not retain a flex basis measured during a previous layout#2020mozzius wants to merge 1 commit into
mozzius wants to merge 1 commit into
Conversation
`computedFlexBasis` survives across layout calls, and the resolved-basis branch of `computeFlexBasisForChild` keeps whatever is stored so a size measured by the max-content pass carries into the definite-size pass. That guard checks whether a value exists, not which layout produced it: when an ancestor's measurement cache answers for a whole subtree the skipped max-content pass leaves a basis measured against a different available size, and a `flex: 1` node keeps its landscape height after rotating back to portrait. Track whether a stored basis was measured, and re-measure rather than retain a stale one from an earlier layout. Fixes react#2019
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
Fixes #2019.
layout.computedFlexBasispersists acrossYGNodeCalculateLayoutcalls and is only cleared bymarkDirtyAndPropagateon that exact node. The resolved-flex-basis branch ofcomputeFlexBasisForChildkeeps whatever value is already stored, which is what lets a max-content measurement made earlier in the same layout survive into the definite-size pass (and is whyflex: 1inside an auto-height column means "content height"). The guard only checks whether a value exists, not which layout produced it.When a later layout at a different root size hits an ancestor's measurement cache during the max-content pass, that subtree is skipped, the child's
computedFlexBasisis never refreshed, and the definite pass retains a size measured against the previous available size. In React Native this shows up as aflex: 1Textthat keeps its landscape height after rotating back to portrait (react/react-native#58294).Fix
Record whether a stored
computedFlexBasiscame from measuring the child's content (computedFlexBasisIsMeasured). In the resolved branch, a stored value that is measured and stamped with a previous generation is treated as stale: the child takes the measure path again, which does exactly what the skipped max-content pass would have done and normally hits the child's own measurement cache. A retained resolved basis is unchanged, so "definite from the start" containers still resolveflex: 1to a basis of 0 as before, and nothing changes within a single layout.Clearing the value instead (or enabling
WebFlexBasis) is not a fix: the resolved branch then writes 0 with no measurement to follow and the child collapses to 0.Test plan
tests/YGRelayoutTest.cpp: an 8-node tree (scroll container,flex: 1column with a fixed-height sibling, two plain wrappers, aflex: 1node with a text-like measure function) laid out at 400x800, 800x400, 400x800. Asserts the measured node returns to its first height. Fails onmain, passes with this change../unit_tests Debug: all 843 tests pass. Onmainthe new test fails (portraitHeightis 100, the relaid-out node is 60) and the other 842 pass.yoga-harness/run.sh) built against this branch: two and three wrapper levels now return to their first height (they stay at the landscape height onmain); one wrapper level passes on both.