Fix measure-func leaf bug#2
Open
sedrew wants to merge 2 commits into
Open
Conversation
… width, not parent The measure func received parent's full width instead of the child's resolved percent width, preventing wrapping. 4 test cases.
|
Hi! The description (and test header) say the fix is |
…t/ cqi/cqmin/calc) and pass it to the measure func instead of parent's availCross. Additionally, after flex distribution re-measure a row measure-func leaf at the actual flex-distributed width so wrapping reflects the post-grow size. Both together prevent height=1 when the leaf has a percent/point cross-axis size.
Author
|
Thanks neftaly. In my first fix I tried to change like this. But after that I decided change the const passWidthToChild =
isRow && mainIsAutoChild && !flexGrowHasDefiniteMainBudget && !flexDistChanged && !hasMeasureLeaf
? NaN
: hasMeasureLeaf // ← new hasMeasureLeaf
? mainSizeToPass // ← measure: childWidth 30%
: !isRow && crossIsAutoForLayoutCall && !parentHasDefiniteCross && !crossIsFitContent
? NaN
: isRow && mainIsPercentForLayoutCall
? mainAxisSize
: !isRow && crossIsPercentForLayoutCall
? crossAxisSize
: !isRow && crossIsFitContent
? crossAxisSize
: isRow
? mainSizeToPass
: childWidth |
|
My LLM still has problems with this (I basically just started a fresh prompt with "Review #2, tell me what the comment should be, be consise"). Can you please make a commit that is just a failing fuzzer, and a few tests (1-3, 1-2 of them failing)? And no changes to the actual codebase. |
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.
I use my own text layout engine for rendering. When a measure-function leaf has an explicit cross-axis size (e.g.
width="30%"in a column orheight="30%"in a row), Flexily should resolve that size and pass the resolved value to the measure function so the text is measured under the correct constraint. Instead, it was passing the parent's available cross-axis size, causing the text to be measured as if it had the parent's full width. As a result, text didn't wrap correctly and the measured height could incorrectly remain1.Root cause
In
layout-zero.tsPhase 8 (passWidthToChild), measure-function leaves always receivedavailCrossas their cross-axis constraint, even when they had an explicitpoint,percent,cqi,cqmin, orcalc()cross-axis size. The child's own cross-axis dimension was never resolved before calling the measure function.Fix
Resolve the measure leaf's own cross-axis size before invoking the measure function:
point→ use the absolute value.percent→ resolve against the parent's cross-axis size.cqi,cqmin, andcalc()→ resolve viaresolveValue().Pass this resolved value to
cachedMeasure()instead of the parent'savailCross.Additionally, if
flexGrowchanges a row measure leaf's width, re-measure it after flex distribution using the final flex-distributed width so text wrapping and the resulting height reflect the actual width.Test coverage
Four test cases cover the fix:
flexGrowchanges the leaf width → the leaf is re-measured at the final width and wrapping is updated.width="30%"resolves to30pxin a100pxparent instead of measuring at the parent's full width.point,percent,cqi,cqmin, andcalc()values are correctly resolved before measurement.All tests fail without the fix and pass with it.
Bug
because take parent width
Fix