Skip to content

Use actual glyph width, not the guessed spaceWidth, to determine the width of the space character in VLW fonts - #159

Merged
lovyan03 merged 2 commits into
m5stack:developfrom
kirberich:fix-space-width
Aug 16, 2026
Merged

Use actual glyph width, not the guessed spaceWidth, to determine the width of the space character in VLW fonts#159
lovyan03 merged 2 commits into
m5stack:developfrom
kirberich:fix-space-width

Conversation

@kirberich

Copy link
Copy Markdown

I was hitting an issue where textWidth would give me results that were slightly off from the actual width of a string drawn to the display when using a VLW font today. After a bunch of digging, it seems that the problem is that drawChar uses a special case for the space character where it uses this->spaceWidth (which according to the comment in the code is just a guess), instead of the width of the space glyph, but textWidth doesn't use this special case.

I'm not sure why this special case was there, and I'm not sure if this would break for some other fonts, but for my case at least the rendering looks correct, and textWidth and drawChar now agree.

Note: the test was done with a custom font, Bookerly 24pt, created using the M5 font converter
Also note: I pointed this at develop as that's what i'm using, let me know if that's wrong

@lovyan03

Copy link
Copy Markdown
Collaborator

Hello, @kirberich
First and foremost, it's important to note that the width of drawChar and the width of textWidth are not necessarily the same.
drawChar renders a single character, but it can vary depending on negative offsets and various other conditions.

We need to investigate how this pull request affects other existing fonts, but this will take time, so it's difficult to merge it immediately.

Some VLW exporters (e.g. the Processing font converter) omit the space
glyph from the file. Falling through to drawCharDummy would render the
missing-glyph box for every space, so restore the previous silent
guessed-width behavior for that case only. Fonts that do contain a
space glyph keep using the actual glyph metrics.
@ainyan03

Copy link
Copy Markdown
Contributor

Thank you for the fix, and sorry it took so long to get back to this.

The analysis here is correct: updateFontMetric (used by textWidth) already prefers the actual glyph metrics for U+0020, so drawChar forcing the guessed spaceWidth made the two disagree for fonts that contain a space glyph.

One case needed an additional guard, though: some VLW exporters (e.g. the Processing font converter) omit the space glyph entirely — the font.vlw bundled with the VlwFont example is one of these (glyphs 0x21-0x7E only). With the special case removed, a missing space fell through to drawCharDummy, which draws the missing-glyph box, so every space would render as a visible rectangle with such fonts. I pushed a follow-up commit to this branch that keeps the guessed-width silent behavior only when the font has no space glyph; fonts that do contain one use the actual glyph metrics as intended by this PR.

Verified on real hardware with both cases:

  • bundled font.vlw (no space glyph): textWidth(" ") == drawn advance == guessed width, no box rendered
  • a VLW containing a space glyph whose xAdvance differs from the guess: both paths use the actual xAdvance, and textWidth("A A") matches the drawn advance exactly

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates VLW rendering so space characters use their glyph’s actual advance width when available.

Changes:

  • Uses the space glyph’s stored metrics during rendering.
  • Retains guessed-width fallback when the glyph is absent.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@lovyan03
lovyan03 merged commit b51ea64 into m5stack:develop Aug 16, 2026
27 checks passed
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.

4 participants