Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: String Width Calculation Issues #998

Open
Flyga-M opened this issue Jan 11, 2025 · 2 comments
Open

Bug: String Width Calculation Issues #998

Flyga-M opened this issue Jan 11, 2025 · 2 comments

Comments

@Flyga-M
Copy link
Contributor

Flyga-M commented Jan 11, 2025

The title is so vague, because there are multiple issues with the string width calculations. I wanted to document them in one place. Feel free to change it to a more appropriate name.

Prompted by the recent discussion on discord about the Label wrap not calculating the text width correctly, I delved a bit into the source to look for the issue.

Latest discussion: https://discord.com/channels/531175899588984842/599270434642460753/1327399531393978440
Previous discussion: https://discord.com/channels/531175899588984842/536970543736291346/1152261761467162848

Problems

MeasureString(..)

It turns out the MonoGame.Extended.BitmapFonts.BitmapFont.MeasureString(..) on it's own works mostly as intended.

It uses the StringGlyphEnumerable to calculate the glyph (characters) positions based on the provided FontRegion and the LetterSpacing. For the width it chooses the highest value of the glyphs width added to their respective positions. The height is calculated by multiplying the amount of lines by the LineHeight.

size_calculation

SpriteBatch.DrawStringOnCtrl() calls MonoGame.Extended.BitmapFonts.BitmapFontExtensions.DrawString(..), which uses the same StringGlyphEnumerable to draw the glyphs onto the SpriteBatch. If you measure the whole string you want to draw, the MeasureWidth(..) gives the exact result.

The only thing that MeasureString(..) is missing: Most, if not all, first characters in a line start at a negative x position. Since MeasureString(..) only calculates position + width, the calculated width is missing the offset to the left.

This on it's own is not a cause for the wrap issue, because the calculation fits the visual width of the rendered text (relative to the Labels (0, 0)), because it's also rendered offset to the left.

DrawUtil.WrapText(..)

The WrapText(..) method (more specifically the WrapTextSegment(..) method) that is used all over Blish HUD separates the string into words, calculates their respective width and then glues them back together.

It calculates the width of the space and the words using the MeasureString(..) method mentioned above.

Now the small issue with MeasureString(..) is amplified a lot. Every single word and every space is 'losing' the negative offset from the first character.

Adding the calculated width of the words and spaces together is in most cases not the same as calculating the width of the whole string.

Note

Example DefaultFont14

text: "mm mm mm mm mm mm mm mm mm mm mm mm mm"
measured width as a whole (MeasureString(..)): 373
measured width as single words and then glued together (WrapTextSegment(..)): 361

Default Fonts

There are some issues with the way the default fonts were generated.

Space

No matter the font size, the width is always set to 5. The actual spacing is accomplished by making the xadvance bigger for bigger fonts. This works visually well, but since using MeasureString(..) for a single character only cares about a glyphs width, the aforementioned issue is amplified again by a lot.

For some reason the xoffset for space is -2 which seems to be a lot and rather unneeded.

space_placement
This (the small green rectangle) is where the space character is actually rendered (and measured).

Other Characters

This probably applies to other special characters, though I have not tested that.

Solutions

WrapText(..)

Regardless of other modifications, WrapText(..) need to be fixed to measure the resulting line as a whole and not word by word. As far as I can tell, this is the only way to ensure that the measured value is always the same as the rendered text.

Caution

Although this is possible to fix and a much needed fix at that, I would advise against a quick fix like #997 or even a proper fix until other PRs are merged that affect this area (namely #984).

Default Fonts

It would be possible to rebuild the default fonts with values that are chosen to mitigate the issue with the implementation of MeasureString(..). I'm not sure if that would be desireable, or otherwise useful.

MeasureString(..)

It would be possible to write an internal MeasureString(..) method that takes the way the default fonts are built into account. How that would affect other non-default fonts I have no idea.

@KirillSerogodsky
Copy link

PR with fix #997

@KirillSerogodsky
Copy link

KirillSerogodsky commented Jan 12, 2025

Some test

DefaultFont12

Initial Text: 2926
Clear Text: 2566
Spaces: 360
Spaces + Text: 2926
Rows Sum: 2928

DefaultFont14

Initial Text: 3489
Clear Text: 3039
Spaces: 450
Spaces + Text: 3489
Rows Sum: 3492

DefaultFont16

Initial Text: 3960
Clear Text: 3420
Spaces: 540
Spaces + Text: 3960
Rows Sum: 3965

DefaultFont18

Initial Text: 4182
Clear Text: 3912
Spaces: 270
Spaces + Text: 4182
Rows Sum: 4244

DefaultFont32

Initial Text: 7596
Clear Text: 6606
Spaces: 990
Spaces + Text: 7596
Rows Sum: 7626

No issue with calc space width, but word calculation not perfect. If the sum of all lines is greater than the original, the text is moved a little earlier. No visual issues.

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

No branches or pull requests

2 participants