Skip to content

textmeasure: prevent Unicode markdown labels from clipping#2780

Open
ihopenre-eng wants to merge 1 commit into
terrastruct:masterfrom
ihopenre-eng:fix/markdown-unicode-width
Open

textmeasure: prevent Unicode markdown labels from clipping#2780
ihopenre-eng wants to merge 1 commit into
terrastruct:masterfrom
ihopenre-eng:fix/markdown-unicode-width

Conversation

@ihopenre-eng

Copy link
Copy Markdown

Summary

  • measure supported Unicode runes with their original TTF glyph advances instead of the fixed atlas replacement glyph
  • add unit and visual regression coverage for the long Markdown heading from title bug #2647
  • add the Cloud Run egress diagram to docs/examples as requested

Root cause

The fixed text-measurement atlas does not include the em dash. It therefore measured the heading with the replacement glyph (672 font units) instead of the em dash (800 units). The generated font subset renders the real glyph, so the browser needed 1021.45 px while the foreignObject was only 1018 px wide. The final word wrapped onto a second line and was clipped by the 51 px-high box.

The updated Unicode path uses raw glyph advances from the original TTF whenever every rune is supported, while retaining the existing grapheme-width fallback for unsupported glyphs.

Verification

  • go test ./lib/textmeasure -count=1
  • go vet ./lib/textmeasure
  • go test ./d2graph -count=1
  • go test ./d2lib -count=1
  • go test ./d2renderers/d2svg -count=1
  • go test ./e2etests -run 'TestE2E/stable/unicode_markdown_title$' -count=1
  • rendered the full example and verified in Chrome after fonts loaded: foreignObject 1022 x 51, content scroll size 1022 x 51

go test -race was unavailable because this environment has CGO_ENABLED=0.

Result

Fixed Cloud Run egress example

Fixes #2647

Signed-off-by: ihopenre-eng <247072151+ihopenre-eng@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

title bug

1 participant