Skip to content

Conversation

@KirillSerogodsky
Copy link
Contributor

MeasureString incorrectly calculates space width.

GameService.Content.DefaultFont16.MeasureString(" ").Width = 3
there's no problem with
GameService.Content.DefaultFont16.MeasureString("_").Width = 21

Discussion Reference

No

Is this a breaking change?

No

@KirillSerogodsky
Copy link
Contributor Author

KirillSerogodsky commented Jan 10, 2025

before
image
image
after
image
image

It's not perfect, but it works better

@dlamkins
Copy link
Member

Could you please share some more comparison screenshots of other wrapped text in Blish HUD (and perhaps in a module or two)? I want to make sure this doesn't look weird with anything existing.

@KirillSerogodsky
Copy link
Contributor Author

I found a better solution

DefaultFont12
image
DefaultFont14
image
DefaultFont16
image
DefaultFont18
image
DefaultFont32
image

@KirillSerogodsky
Copy link
Contributor Author

Module list before
image
image

Mystic Crafting before
image
image
image

Module info before
image

Pathing markers before
image

@KirillSerogodsky
Copy link
Contributor Author

Module list after
image
image

Mystic Crafting after
image
image
image

Module info after
image

Pathing markers after
image

@KirillSerogodsky
Copy link
Contributor Author

Legendary crafting before
image
after
image

@KirillSerogodsky
Copy link
Contributor Author

Farming Tracker before
image
after
image

@KirillSerogodsky KirillSerogodsky changed the title Fix incorrect calc space width in WrapTextSegment Fix incorrect calc width in WrapTextSegment Jan 12, 2025
@Flyga-M
Copy link
Contributor

Flyga-M commented Jan 12, 2025

While this seems to fix the issue well enough, I think it does make sense to implement a custom MeasureString(..) method (in the future?) that uses XAdvance instead of Width to determine glyph sizes.

MeasureString(..) is just used in so many places, that a change like that would affect many places in Core.

@dlamkins
Copy link
Member

dlamkins commented Feb 4, 2026

@KirillSerogodsky Thank you for this! I do think that this is an important thing for us to address. I was looking into this as well and I think @Flyga-M is right about a custom MeasureString method. I did some testing with a custom extension that utilizes XAdvance instead of visual bounds.

I think it also eliminates the built-up errors mentioned in #998. Would you be open to letting me push a commit to your branch with these changes (I still want your name on the PR).

And thanks @Flyga-M for the detailed breakdown in #998!

public static Vector2 MeasureStringLogical(this BitmapFont font, string text) {
    if (string.IsNullOrEmpty(text))
        return Vector2.Zero;

    var size = Vector2.Zero;
    var currentX = 0f;

    for (var i = 0; i < text.Length; i++) {
        var c = text[i];

        // Handle Newline
        if (c == '\n') {
            size.X = Math.Max(size.X, currentX);
            size.Y += font.LineHeight;
            currentX = 0;
            continue;
        }

        var region = font.GetCharacterRegion(c);
        if (region == null)
            continue;

        // Use XAdvance (logical width) instead of TextureRegion.Width (visual width)
        currentX += region.XAdvance + font.LetterSpacing;

        if (BitmapFont.UseKernings && i < text.Length - 1) {
            if (region.Kernings.TryGetValue(text[i + 1], out var kerning)) {
                currentX += kerning;
            }
        }
    }

    size.X = Math.Max(size.X, currentX);
    size.Y += font.LineHeight; // Ensure at least one line height

    return size;
}

I'm especially excited because, in my testing, it fixed numerous alignment issues with our textbox controls! In fact, the textbox controls currently have some built-in alignment offsets to try to compensate for the previous errors and now those offsets can be removed once this is implemented! 😄

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.

3 participants