Repository navigation
fix(render): pass letter-spacing through to the glyph path builder - #25
Open
leshchenko1979 wants to merge 1 commit into
Open
leshchenko1979 wants to merge 1 commit into
leshchenko1979 wants to merge 1 commit into
Conversation
letter-spacing was measured but never drawn: the text layout reserved the wider box while the glyph paths were built with a hardcoded spacing of 0, so the rendered text kept its unspaced width and the reserved space stayed empty. Reproduced on main (32f3fad), font-size 20px, "ABCDEFGHIJ": letter-spacing 0px text width 253.7 letter-spacing 10px text width 253.7 <- identical SVG font.Measurer.StringWidth did apply the spacing, so measure and render disagreed by exactly spacing x (n-1). Changes: - font/glyph.go: TextToPathVariationsSpaced, ShapedTextToPathVariationsSpaced, ShapedRunToPathSpaced and shapedRunToPathVariationsSpaced accept a spacing and insert it between glyphs, matching the (n-1) convention already used by the measurer. The original functions delegate with 0, so existing callers are unaffected. - render/text.go, render/svg.go: all four glyph-path call sites now pass cs.LetterSpacing (shadows, renderTextSegment, appendGlyphClip, renderTextAt). - render/png.go: rasterizeShapedPath no longer short-circuits when spacing is non-zero and forwards the value, so the PNG path matches the SVG path. Verified against Chromium with the same DejaVu Sans file for both engines: spacing 0px 84px (Chromium 94px) spacing 10px 96px (Chromium 106px) The remaining 10px offset is a font-metric difference that is present with spacing off (84 vs 94) and is not caused by this bug; the widening itself now matches exactly (+12px here, +90px on the 10-glyph case, both engines). Tests added: - font/letterspacing_test.go: zero spacing reproduces the old paths byte for byte, spacing widens by (n-1) x spacing, a single glyph is unaffected. - ogre_letter_spacing_e2e_test.go: through the public Render, 10px widens by 90px, and 0.5em resolves to the same width as 10px. Fixes Macawls#22
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.
Problem
letter-spacingis parsed, stored, and even applied when measuring text — but the glyph paths are built with a hardcoded spacing of0, so the drawn text keeps its unspaced width while layout reserves the wider box.Reproduced on
main(32f3fad),font-size: 20px, textABCDEFGHIJ:Chromium gives
94pxand184pxfor the same declarations.Cause
font.Measurer.StringWidthdoes applyletter-spacing, so measurement and rendering disagree by exactlyspacing × (n-1). The render side never received the value:font/glyph.go—TextToPathVariationsandShapedTextToPathVariationspassed a literal0asletterSpacingdown totextToPathVariable/textToPathCached.render/png.go—rasterizeShapedPathhad no spacing parameter at all, so the PNG output would have diverged from the SVG output even after fixing the SVG path.Fix
font/glyph.go: newTextToPathVariationsSpaced,ShapedTextToPathVariationsSpaced,ShapedRunToPathSpacedandshapedRunToPathVariationsSpacedtake a spacing and insert it between glyphs, using the same(n-1)convention the measurer already uses. The existing functions delegate with0, so current callers keep their behaviour byte for byte.render/text.go,render/svg.go: all four glyph-path call sites now passcs.LetterSpacing(text shadows,renderTextSegment,appendGlyphClip,renderTextAt).render/png.go:rasterizeShapedPathno longer short-circuits when spacing is non-zero and forwards the value.Verification
Same DejaVu Sans file handed to both engines:
letter-spacing0px10pxThe 10px constant offset exists with spacing off too (84 vs 94), so it is a font-metric difference and not part of this bug. The widening itself now matches:
+12pxon this sample,+90pxon a 10-glyph sample — identical in both engines.Tests added:
font/letterspacing_test.go— zero spacing reproduces the old paths byte for byte; spacing widens the path by(n-1) × spacing; a single glyph is unaffected.ogre_letter_spacing_e2e_test.go— through the publicRender:10pxwidens by 90px, and0.5ematfont-size: 20pxyields the same width as10px.go build ./...,go vet ./...andgo test ./...pass.Fixes #22