libgdx / libgdx

Desktop/Android/HTML5/iOS Java game development framework
http://www.libgdx.com/
Apache License 2.0
23.32k stars 6.44k forks source link

Problems with a BitmapFont generated by Hiero #4570

Open LennartH opened 7 years ago

LennartH commented 7 years ago

I tried to change the font of an existing skin and repeatedly got an exception like the one below, after adding some lines to a modified TextArea inside a ScrollPane. I assumed that the BitmapFont generated by Hiero caused the exception, so I built an SSCCE with the default skin and replaced the font of the TextArea with the desired one and I could reproduce the error.

Edit 1: I tried to generate the font at runtime with the FreeTypeFontGenerator and it executed without exception. But I can't use the generator in my productive code, since I'm targeting web.

Edit 2: The TextArea modifications have nothing to do with the error. I simplified the test and could reproduce the exception.

Edit 3: Fonts generated with the FreeTypeFontGenerator that are stored with the BitmapFontWriter also result in an excpetion. But fonts generated with BMFont work error free. This was the used configuration.

Reproduction steps

The SSCCE can be found in my test repository. To reproduce the error run com.upseil.prototype.TextAreaHieroFontTest and press the button Add new line until the error occurs (usually once).

Assets and code

The used font is assets/font/lucida-console-15.fnt and was generated using the settings:

Code of the test class:

import com.badlogic.gdx.ApplicationAdapter;
import com.badlogic.gdx.Gdx;
import com.badlogic.gdx.backends.lwjgl.LwjglApplication;
import com.badlogic.gdx.backends.lwjgl.LwjglApplicationConfiguration;
import com.badlogic.gdx.graphics.GL20;
import com.badlogic.gdx.graphics.g2d.BitmapFont;
import com.badlogic.gdx.scenes.scene2d.Actor;
import com.badlogic.gdx.scenes.scene2d.Stage;
import com.badlogic.gdx.scenes.scene2d.ui.Button;
import com.badlogic.gdx.scenes.scene2d.ui.Skin;
import com.badlogic.gdx.scenes.scene2d.ui.Table;
import com.badlogic.gdx.scenes.scene2d.ui.TextArea;
import com.badlogic.gdx.scenes.scene2d.ui.TextButton;
import com.badlogic.gdx.scenes.scene2d.utils.ChangeListener;
import com.badlogic.gdx.utils.viewport.ScreenViewport;

public class TextAreaHieroFontTest extends ApplicationAdapter {
    Stage stage;
    int lineCounter;

    public void create () {
        stage = new Stage(new ScreenViewport());
        Skin skin = new Skin(Gdx.files.internal("skin/default/uiskin.json"));
        Gdx.input.setInputProcessor(stage);

        Table container = new Table();
        stage.addActor(container);
        container.setFillParent(true);
        container.pad(10).defaults().expandX().fillX().space(4);

        final TextArea textArea = new TextArea("Initial line.", skin);
        textArea.setDisabled(true);
        textArea.getStyle().font = new BitmapFont(Gdx.files.internal("font/lucida-console-15.fnt"));
        // Fonts generated with BMFont also work
//        textArea.getStyle().font = new BitmapFont(Gdx.files.internal("font/BMF-15/lucida-console-15.fnt"));
        // This works
//        FreeTypeFontGenerator fontGenerator = new FreeTypeFontGenerator(Gdx.files.internal("font/lucida-console.ttf"));
//        FreeTypeFontParameter fontParameter = new FreeTypeFontParameter();
//        fontParameter.size = 15;
//        textArea.getStyle().font = fontGenerator.generateFont(fontParameter);
//        fontGenerator.dispose();

        Button addLineButton = new TextButton("Add new line", skin);
        addLineButton.addListener(new ChangeListener() {
            @Override
            public void changed(ChangeEvent event, Actor actor) {
                textArea.appendText("\nLine " + lineCounter++);
//                textArea.setCursorPosition(0); // This prevents the error
            }
        });
        container.add(addLineButton).colspan(2);

        container.row().height(350);
        container.add(textArea);
        container.debugAll();
    }

    public void render () {
        Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT);
        stage.act(Gdx.graphics.getDeltaTime());
        stage.draw();
    }

    public void resize (int width, int height) {
        // Pass false to not modify the camera position.
        stage.getViewport().update(width, height, true);
    }

    public static void main (String[] args) throws Exception {
        LwjglApplicationConfiguration config = new LwjglApplicationConfiguration();
        config.width = 1024;
        config.height = 768;
        new LwjglApplication(new TextAreaHieroFontTest(), config);
    }
}

Versions

LibGDX: 1.9.6-SNAPSHOT and 1.9.5 LibGDX-Tools: 1.9.5

Stacktrace

Exception in thread "LWJGL Application" java.lang.ArrayIndexOutOfBoundsException: 19
    at com.badlogic.gdx.scenes.scene2d.ui.TextField.calculateOffsets(TextField.java:229)
    at com.badlogic.gdx.scenes.scene2d.ui.TextArea.calculateOffsets(TextArea.java:263)
    at com.badlogic.gdx.scenes.scene2d.ui.TextField.draw(TextField.java:318)
    at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110)
    at com.badlogic.gdx.scenes.scene2d.ui.ScrollPane.draw(ScrollPane.java:588)
    at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:123)
    at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57)
    at com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup.draw(WidgetGroup.java:163)
    at com.badlogic.gdx.scenes.scene2d.ui.Table.draw(Table.java:119)
    at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110)
    at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57)
    at com.badlogic.gdx.scenes.scene2d.Stage.draw(Stage.java:128)
    at com.upseil.prototype.AutoScrollingTextAreaFontTest.render(AutoScrollingTextAreaFontTest.java:73)
    at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:225)
    at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:126)

Affected platforms

I assume all, but was only tested on Windows

LennartH commented 7 years ago

While working on my project, I found another work around for this problem, by setting the text areas curser position to 0, after adding the text. I adjusted the minimal example. This is also a workaround for #4561.

ttencate commented 7 years ago

I also ran into this, and found a better workaround. Open up the .fnt file generated by Hiero, delete the character definitions for chars 0 and 10, and reduce chars count accordingly.

In my case, I went from this...

info face="Perfect DOS VGA 437" size=16 bold=0 italic=0 charset="" unicode=0 stretchH=100 smooth=1 aa=1 padding=1,1,1,1 spacing=-2,-2
common lineHeight=16 base=12 scaleW=512 scaleH=512 pages=1 packed=0
page id=0 file="vga437.png"
chars count=98
char id=0       x=0    y=0    width=0    height=0    xoffset=-1   yoffset=0    xadvance=9    page=0    chnl=0 
char id=10      x=0    y=0    width=0    height=0    xoffset=-1   yoffset=0    xadvance=0    page=0    chnl=0 
char id=32      x=0    y=0    width=0    height=0    xoffset=-1   yoffset=0    xadvance=9    page=0    chnl=0 
...

... to this:

info face="Perfect DOS VGA 437" size=16 bold=0 italic=0 charset="" unicode=0 stretchH=100 smooth=1 aa=1 padding=1,1,1,1 spacing=-2,-2
common lineHeight=16 base=12 scaleW=512 scaleH=512 pages=1 packed=0
page id=0 file="vga437.png"
chars count=96
char id=32      x=0    y=0    width=0    height=0    xoffset=-1   yoffset=0    xadvance=9    page=0    chnl=0
...

So this is deleting both the null character (\0) and the newline character (\n). Deleting only one of them is not enough. Maybe that provides a clue about this issue.

ttencate commented 7 years ago

I think the problem is in TextField#updateDisplayText.

In a single-line TextField, the glyphPositions array holds the x position where each glyph starts, and this array has the same length as the text of the field.

In a multi-line TextArea, however, I'm not sure what glyphPositions should even mean anymore. In practice, it holds the x position where each glyph starts, but ignoring line breaks, and only for glyphs in the first run:

    if (layout.runs.size > 0) {
        GlyphRun run = layout.runs.first();
        FloatArray xAdvances = run.xAdvances;
        fontOffset = xAdvances.first();
        for (int i = 1, n = xAdvances.size; i < n; i++) {
            glyphPositions.add(x);
            x += xAdvances.get(i);
        }
            }

With the BMFont-generated font, all glyphs end up in a single run, and everything is fine. But with the font generated by Hiero, the two lines end up in different runs, so glyphPositions contains only the first line. So no wonder we get OutOfBoundsExceptions everywhere, because glyphPositions is assumed to be as long as text in several places. The "need" for commit adaf504f0267ef7adf5106e5ea7d65c8c0276e3b might also have been a symptom of this.

I suppose this could be fixed by going through all runs, instead of processing just the first. The numbers in glyphPositions won't really make sense, but they already don't, and apparently that's fine.

So why does this depend on the font? I.e. why do some fonts generate just one GlyphRun and others generate one per line? With the BMFont font, TextField#updateDisplayText replaces the \n with a space, because it's not in the font. The GlyphLayout class never even gets to see the line break. With the Hiero font, however, data.hasGlyph('\n') returns true, because character 10 actually is in the BitmapFont. And if we remove it, BitmapFont can still fall back to any characters ≤ 0, which provide BitmapFontData#missingGlyph. So that explains why I needed to remove both character 0 and character 10.

For the actual fix, I'll let @NathanSweet chime in, because this code is rather finicky and I'm afraid I'd break more than I'd fix.

wheelerruss commented 7 years ago

I have just come across this very problem, but I am using FreeTypeFontGenerator, so I'm not sure how I can remove characters 0 and 10.

Is there any other workaround available?

data.hasGlyph('\n') actually returns false for me, but it still has the error above. also when I look at data.glyphs[0][0] and data.glyphs[0][10], 10 is null, but 0 has something in it (two unicode ? that I can't tell what they are)

Thanks for your insightful work on this @ttencate

ttencate commented 7 years ago

You could try fixing it by copying TextArea to your own project, renaming it to PatchedTextArea (keep it in the same package though), and overriding TextField#updateDisplayText like this:

@Override
void updateDisplayText () {
    BitmapFont font = style.font;
    BitmapFontData data = font.getData();
    String text = this.text;
    int textLength = text.length();

    StringBuilder buffer = new StringBuilder();
    for (int i = 0; i < textLength; i++) {
        char c = text.charAt(i);
        buffer.append(data.hasGlyph(c) ? c : ' ');
    }
    String newDisplayText = buffer.toString();

    if (passwordMode && data.hasGlyph(passwordCharacter)) {
        if (passwordBuffer == null) passwordBuffer = new StringBuilder(newDisplayText.length());
        if (passwordBuffer.length() > textLength)
            passwordBuffer.setLength(textLength);
        else {
            for (int i = passwordBuffer.length(); i < textLength; i++)
                passwordBuffer.append(passwordCharacter);
        }
        displayText = passwordBuffer;
    } else
        displayText = newDisplayText;

    layout.setText(font, displayText);
    glyphPositions.clear();
    float x = 0;

    // BEGIN CHANGES

    if (layout.runs.size > 0) {
        fontOffset = layout.runs.first().xAdvances.first();
        for (GlyphRun run : layout.runs) {
            FloatArray xAdvances = run.xAdvances;
            for (int i = 1, n = xAdvances.size; i < n; i++) {
                glyphPositions.add(x);
                x += xAdvances.get(i);
            }
        }
    } else
        fontOffset = 0;

    // END CHANGES

    glyphPositions.add(x);

    if (selectionStart > newDisplayText.length()) selectionStart = textLength;
}

I haven't tested it but that's the general idea. If it works, you could even make a pull request for it (it should probably be fixed directly in TextField then though).

wheelerruss commented 7 years ago

Thanks for answering so quickly, I'll see if I can get it working during my lunch hour.

wheelerruss commented 7 years ago

Just to answer, I still can't get this to work at all, even with ttencates help.