fix(text): baseline-anchored layout, include lineGap, trim trailing letterSpacing#23
Merged
Merged
Conversation
…etterSpacing Aligns SDF and Canvas text layout with CSS line-box semantics so that multiple fonts at the same size share a baseline and default line spacing matches web 'normal'. - TextLayoutEngine: bareLineHeight now includes lineGap (CSS normal = asc + lineGap - desc). line[4] is the alphabetic baseline Y rather than the line-box top. Renamed halfDelta -> halfLeading for clarity. - TextLayoutEngine: trim one trailing letterSpacing from each non-empty line's reported width so textAlign center/right and effectiveMaxWidth no longer over-count by one letter-spacing. Wrap decisions still use the un-trimmed accumulators. - SdfTextRenderer: anchor glyphs to the alphabetic baseline via common.base (glyph.y = baselineY + yoffset - atlasBase). Fonts whose BMFont 'base' differs (e.g. Ubuntu 32.6 vs Noto 44.9 at design size 42) now sit on the same line. Dropped the SDF-specific designLineHeight fallback so the engine's lineHeight*bareLineHeight is used uniformly with Canvas. - SdfTextRenderer: include fontStyle in the layout cache key. - CanvasTextRenderer: switch both contexts from textBaseline 'hanging' to 'alphabetic' so fillText draws the baseline at line[4]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d532b39 to
efde96c
Compare
3 tasks
…ents Each page(i) call in the viewport-events automation kicks off a multi- frame cascade: position mutation -> bounds intersection recompute -> inBounds / inViewport / outOfBounds events -> status-text mutations in handlers -> text re-layout and SDF atlas update -> final render. settings.snapshot() only sleeps a flat 200ms after the mutation, which is usually enough at 60Hz but is not deterministic - under load or when the cascade takes an extra frame the snapshot lands mid-update and produces a sub-pixel diff against the certified image. Added a local waitForIdle helper that awaits the renderer's 'idle' event (fired from WebPlatform once stage.hasSceneUpdates() returns false - i.e. every dirty node has actually rendered) with a 500ms safety timeout in case a mutation happens to be a no-op. Called before every snapshot including the initial one (so font load + first bounds cascade are also flushed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
After re-certifying snapshots in Docker, viewport-events and other text-heavy tests still drifted by a small handful of glyph-edge pixels. The Chromium flags that suppress text-AA variance only affect Canvas text rendering; SDF glyphs go through our own WebGL pipeline where sub-pixel float-precision differences across Docker rebuilds land as isolated mis-classified anti-aliasing pixels at glyph boundaries. pixelmatch's `threshold` is the wrong knob for this - it controls per- pixel color distance, and lowering it would make these failures worse. The standard graphics-test answer is a small absolute pixel-count tolerance. Added MAX_DIFF_RATIO = 0.05% of total frame pixels (about 460 px for a 1280x720 capture, ~1000 px for 1920x1080). Tight enough that a missing word, a swapped color, or a shifted rect would still trip the check; loose enough that residual AA jitter on character outlines doesn't. The failure message now reports both the actual count and the allowed tolerance so future drift expansions are obvious in the log. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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
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
Aligns SDF and Canvas text layout with CSS line-box semantics. Multiple fonts at the same size now share a baseline, default line spacing matches the web's
normalline-height, and trailing letter-spacing no longer skews alignment.Changes
TextLayoutEngine.mapTextLayoutbareLineHeightnow includeslineGap(CSSnormal=asc + lineGap - desc).line[4]is the alphabetic baseline Y in screen px, not the line-box top.halfDelta→halfLeading; computedfirstBaselineY = halfLeading + ascender.letterSpacingis trimmed from each non-empty line's reported width soeffectiveMaxWidthandtextAligncenter/right offsets no longer over-count by one letter-spacing. Wrap decisions still use the un-trimmed accumulators (no wrap-behavior regressions).SdfTextRenderercommon.base:glyph.y = baselineY + yoffset − atlasBase. Fonts whose BMFontbasediffers (e.g. Ubuntu 32.6 vs Noto 44.9 at design size 42) now sit on the same line at the samefontSize.designLineHeightfallback so the engine'slineHeight × bareLineHeight(incl.lineGap) is used uniformly with Canvas.fontStyleadded to the layout cache key.CanvasTextRenderertextBaseline = 'hanging'to'alphabetic', matching the newline[4]semantics.Why
Previous code:
lineGapfrom default line height (~6% tighter than the browser for Ubuntu).common.basecould not share an on-screen baseline.letterSpacingto each line width, shifting center/right alignment byletterSpacingpx.'hanging'vs ad-hocyoffsetplacement), so the two backends could not produce identical layouts for the same input.Reviewer notes
letterSpacingshifts by ~½ letterSpacing toward the correct CSS position.pnpm test:visual:updatewill need to be re-run and certified snapshots reviewed.verticalAlignis not implemented in this PR — it's the next change. A handful of related items (e.g. gatingmaxHeighttruncation oncontain) are intentionally deferred to land with that work.lineHeight <= 3unitless-multiplier heuristic is kept as-is (API contract).Test plan
pnpm test --run(148/148 pass)pnpm buildcleanpnpm test:visual:updateand review certified snapshot diffs before merge🤖 Generated with Claude Code