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

BitmapFont.setFixedWidthGlyphs() does not fix width of first and last characters of text #3239

Closed
hyvas opened this issue Jun 28, 2015 · 6 comments

Comments

@hyvas
Copy link

@hyvas hyvas commented Jun 28, 2015

When using BitmapFont.setFixedWidthGlyphs() the first and last characters of the text being rendered are not drawn with a fixed width. This appears to have been broken since the changes related to #3074 in libGDX 1.6.0.

@NathanSweet
Copy link
Member

@NathanSweet NathanSweet commented Jun 28, 2015

Please show what is not rendering correctly.

@hyvas
Copy link
Author

@hyvas hyvas commented Jun 30, 2015

I still see this issue with the latest nightlies.

Here's a bare-bones example which illustrates the problem.

public class FixedWidthGlyphTest extends ApplicationAdapter {
    SpriteBatch batch;
    BitmapFont font;
    BitmapFontCache fontCache;
    float counter;

    @Override
    public void create () {
        batch = new SpriteBatch();

        font = new BitmapFont();
        font.getData().setScale(2);
        font.setFixedWidthGlyphs("0123456789");

        fontCache = new BitmapFontCache(font);
    }

    @Override
    public void render () {
        Gdx.gl.glClearColor(0, 0, 0, 1);
        Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);

        counter += Gdx.graphics.getDeltaTime() * 10;
        String text = String.valueOf((int)counter);

        GlyphLayout layout = fontCache.setText(text, Gdx.graphics.getWidth() / 2, Gdx.graphics.getHeight() / 2, 50, Align.right, false);
        Gdx.app.log("FixedWidthGlyphTest", "Layout width of " + text + " is " + layout.width);

        batch.begin();
        fontCache.draw(batch);
        batch.end();
    }
}

All glyphs used are set to have a fixed width. However, as shown in the log, the width of text with the same number of characters varies. This causes the text to appear to move around.

The width, and therefore the position, of the text depends on the width of the last character, which is variable despite the call to setFixedWidthGlyphs(). The width of the first character is also variable so a similar effect would be seen if the text were left-aligned.

@NathanSweet NathanSweet reopened this Jul 1, 2015
@NathanSweet
Copy link
Member

@NathanSweet NathanSweet commented Jul 3, 2015

Thanks.

The problem is in BitmapFont getGlyphs. Here is a "fix":

public void getGlyphs (GlyphRun run, CharSequence str, int start, int end) {
    boolean markupEnabled = this.markupEnabled;
    float scaleX = this.scaleX;
    Array<Glyph> glyphs = run.glyphs;
    FloatArray xAdvances = run.xAdvances;

    Glyph lastGlyph = null;
    while (start < end) {
        char ch = str.charAt(start++);
        Glyph glyph = getGlyph(ch);
        if (glyph == null) continue;
        glyphs.add(glyph);

        if (lastGlyph == null)
            xAdvances.add(0); // First glyph. // This changed!
        else
            xAdvances.add((lastGlyph.xadvance + lastGlyph.getKerning(ch)) * scaleX);
        lastGlyph = glyph;

        // "[[" is an escaped left square bracket, skip second character.
        if (markupEnabled && ch == '[' && start < end && str.charAt(start) == '[') start++;
    }
    if (lastGlyph != null) xAdvances.add(lastGlyph.xadvance * scaleX - padRight); // This changed!
}

See the two lines noted. Those lines (before being fixed as above) made the first glyph flush with the left edge and made the last xadvance flush with the right edge. This is nice for variable width fonts because it gives tight fitting bounds for the glyphs. For a fixed width font, it is not so good because you lose the fixed width for the first and last glyph, as you found.

I'm not sure what the fix is. BitmapFont getGlyphs is intended for customization by subclasses, eg to combine multiple characters into a single glyph. Needing to copy/paste it for a fixed width font isn't good. Adding a font field boolean fixedWidth is also not good, since it adds a special case that needs to be handled in ever getGlyphs implementation. It may be the only way though.

Thoughts?

@kubaork
Copy link

@kubaork kubaork commented Nov 1, 2015

Is it going to be fixed soon?
I am trying to make a digital clock with this font http://www.fonts2u.com/digital-7-mono.font
This font have fixed width (for numbers, I checked this in MS Word).
If I render for example "11" there will not be space on beggining, but there will be space bettwen these two characters.

EDIT: I downgraded libgdx to 1.5.6. It works fine. But please repair it :-)

hyvas pushed a commit to hyvas/libgdx that referenced this issue Nov 15, 2015
@hyvas
Copy link
Author

@hyvas hyvas commented Nov 15, 2015

Thanks very much for the investigation and explanation, and apologies for the delay in replying.

The updated getGlyphs works for me. I see what you mean about the difficulty of a fix. Since setFixedWidthGlyphs takes a list of glyphs rather than fixed width being applied to the whole font, a boolean fixedWidth field in Glyph seems to make most sense. Also better to have to deal with a special case in a custom getGlyphs implementation than the more common case of using setFixedWidthGlyphs. See what you think of the pull request.

@NathanSweet
Copy link
Member

@NathanSweet NathanSweet commented Nov 23, 2015

Closed by #3564.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.