fix: pass run_mode through layout#932
Draft
jkelleyrtp wants to merge 3 commits into
Draft
Conversation
Root cause: Taffy's block layout had two places where it called PerformLayout on children when it only needed sizing information, causing descendant layouts to be overwritten with values computed at the wrong width. Fix 1 — determine_content_based_container_width (line 632): Changed tree.perform_child_layout to tree.measure_child_size. This function only needs the child's width to determine the container's intrinsic width — it was needlessly doing full PerformLayout, which cascaded down and overwrote correct layouts. Fix 2 — perform_final_layout_on_in_flow_children (line 846): Propagated the parent's run_mode to children instead of hardcoding PerformLayout. When a parent block is in ComputeSize mode, children now also get ComputeSize, preventing them from storing intermediate layout results. Also guarded set_unrounded_layout to only run in PerformLayout mode. Result: node=31 (version selector) now correctly gets h=41 (matching Chrome), and the nav starts at y=49 instead of y=82, eliminating the 33px gap. All 4,380 Taffy tests pass.
nicoburns
changed the title
Member
|
@jkelleyrtp Counter proposal: #935. Does that PR also fix the issue you were seeing? |
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
2 participants
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.
Root cause: Taffy's block layout had two places where it called PerformLayout on children when it only needed sizing information, causing descendant layouts to be overwritten with values computed at the wrong width.
Fix 1 — determine_content_based_container_width (line 632): Changed tree.perform_child_layout to tree.measure_child_size. This function only needs the child's width to determine the container's intrinsic width — it was needlessly doing full PerformLayout, which cascaded down and overwrote correct layouts.
Fix 2 — perform_final_layout_on_in_flow_children (line 846): Propagated the parent's run_mode to children instead of hardcoding PerformLayout. When a parent block is in ComputeSize mode, children now also get ComputeSize, preventing them from storing intermediate layout results. Also guarded set_unrounded_layout to only run in PerformLayout mode.
Result: node=31 (version selector) now correctly gets h=41 (matching Chrome), and the nav starts at y=49 instead of y=82, eliminating the 33px gap. All 4,380 Taffy tests pass.