LibGfx+LibWeb: Apply word-spacing to word separator advances - #11066
Conversation
Tests and their references can list the same resource. Each duplicate entry re-applied the same substitution, corrupting the rewritten path.
Previously, trimming trailing whitespace from a line recomputed each removed character's width from font metrics and added letter-spacing by hand. That duplicated the advance computation in shaping and drifted from it for tab characters. Shaping now records the advance of a run's trailing whitespace, line box fragments keep the record up to date as runs merge, and trimming subtracts the recorded advance. Removal is the exact inverse of shaping.
Previously, `word-spacing` was only consulted by the `tab-size` advance formula. Space glyphs never received it. Text shaping now adds the computed `word-spacing` to the advance of each word separator glyph. The space and no-break space are word separators. The line feed is treated the same way, because whitespace collapsing can leave it in shaped text, where it behaves as a space.
📝 WalkthroughWalkthroughThe PR adds word-spacing to native and Rust text shaping. It applies spacing to word separators and includes it in shaping-cache keys. Shaping now returns trailing-whitespace length and advance. Inline layout stores this metadata in items and fragments, then trims trailing whitespace from recorded metrics. New WPT tests cover word-spacing across inline, block, table, percentage, preserved-whitespace, and negative-value cases. Sequence Diagram(s)sequenceDiagram
participant Layout
participant shape_text_with_font
participant ladybird_gfx_shape_text
participant GfxShapeText
Layout->>shape_text_with_font: request text shaping with word_spacing
shape_text_with_font->>ladybird_gfx_shape_text: pass word_spacing
ladybird_gfx_shape_text->>GfxShapeText: shape text and request trailing-whitespace metrics
GfxShapeText-->>ladybird_gfx_shape_text: return glyphs and metrics
ladybird_gfx_shape_text-->>shape_text_with_font: return shaped run
shape_text_with_font-->>Layout: store glyphs and trailing-whitespace data
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/Rust/src/layout/line_box.rs`:
- Around line 191-201: Update the trailing-whitespace trimming logic around
last_fragment_index so it continues inspecting preceding fragments after a
whitespace-only fragment is reduced to zero. Accumulate and remove eligible
trailing whitespace across inline-node or font boundaries, while retaining
required zero-length inline fragments and keeping
trailing_whitespace_inline_size() accurate.
In
`@Tests/LibWeb/Ref/expected/wpt-import/css/css-text/word-spacing/word-spacing-001-ref.html`:
- Line 25: Close all four nested span elements opened in the reference markup by
adding the corresponding closing tags before the closing div, preserving the
existing text and nesting order.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a59d5915-b3a5-4314-827a-c30658595a50
📒 Files selected for processing (36)
Libraries/LibGfx/Font/Font.hLibraries/LibGfx/Rust/src/text_layout.rsLibraries/LibGfx/TextLayout.cppLibraries/LibGfx/TextLayout.hLibraries/LibWeb/Painting/DisplayListRecorder.cppLibraries/LibWeb/Rust/src/layout/font.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_level_iterator.rsLibraries/LibWeb/Rust/src/layout/line_box.rsLibraries/LibWeb/Rust/src/layout/line_box_fragment.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsMeta/import-wpt-test.pyTests/LibWeb/Ref/expected/wpt-import/css/CSS2/text/word-spacing-applies-to-001-ref.xhtTests/LibWeb/Ref/expected/wpt-import/css/CSS2/text/word-spacing-applies-to-007-ref.xhtTests/LibWeb/Ref/expected/wpt-import/css/CSS2/text/word-spacing-applies-to-012-ref.xhtTests/LibWeb/Ref/expected/wpt-import/css/css-text/word-spacing/reference/ref-filled-green-100px-square.xhtTests/LibWeb/Ref/expected/wpt-import/css/css-text/word-spacing/word-spacing-001-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-text/word-spacing/word-spacing-002-ref.htmlTests/LibWeb/Ref/expected/wpt-import/css/css-text/word-spacing/word-spacing-003-ref.htmlTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-001.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-002.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-005.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-006.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-007.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-008.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-009.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-010.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-011.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-012.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-013.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-014.xhtTests/LibWeb/Ref/input/wpt-import/css/CSS2/text/word-spacing-applies-to-015.xhtTests/LibWeb/Ref/input/wpt-import/css/css-text/word-spacing/word-spacing-001.htmlTests/LibWeb/Ref/input/wpt-import/css/css-text/word-spacing/word-spacing-002.htmlTests/LibWeb/Ref/input/wpt-import/css/css-text/word-spacing/word-spacing-003.htmlTests/LibWeb/Ref/input/wpt-import/css/css-text/word-spacing/word-spacing-negative-value-001.html
c0fa102
into
LadybirdBrowser:master
Previously,
word-spacingwas only consulted by thetab-sizeadvance formula. Space glyphs never received it.Text shaping now adds the computed
word-spacingto the advance of each word separator glyph. The space and no-break space are word separators. The line feed is treated the same way, because whitespace collapsing can leave it in shaped text, where it behaves as a space.I've included a fix to the WPT test importer, which previously mangled the path to resources that were included in multiple files (i.e: the test file and the reference file).