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

Spaces cause wrapping to create an extra line for each space #13

Open
lucas-viva opened this issue Aug 19, 2023 · 9 comments
Open

Spaces cause wrapping to create an extra line for each space #13

lucas-viva opened this issue Aug 19, 2023 · 9 comments

Comments

@lucas-viva
Copy link

Hi!

When rendering a wrapped TextraLabel that has a lot of spaces, each space will result in an extra line.

Example:

image

Code:

public class MyGdxGame extends ApplicationAdapter {

  private Stage stage;
  private Skin skin;

  @Override
  public void create () {
    stage = new Stage(new ScreenViewport());

    skin = new Skin(Gdx.files.internal("uiskin.json"));

    int fitOneLineCount = 25;
    int extraLines = 10;

    StringBuilder text = new StringBuilder();
    for (int count = 0; count < fitOneLineCount + extraLines; count++) {
      text.append(' ');
    }

    TextraLabel textraLabel = new TextraLabel(text.toString(), skin);
    textraLabel.setWrap(true);

    // Runs in the next render thread so the layout is ready.
    Gdx.app.postRunnable(() -> System.out.println("Lines: " + textraLabel.layout.lines()));

    Table table = new Table();
    table.debug();
    table.add(textraLabel).prefWidth(100).row();

    Stack stack = new Stack(table);
    stack.setFillParent(true);

    stage.addActor(stack);
  }

  @Override
  public void render () {
    ScreenUtils.clear(0, 0, 0, 1);
    stage.act();
    stage.draw();
  }

  @Override
  public void dispose () {
    stage.dispose();
    skin.dispose();
  }
}

In the code above, there will be a whole line with spaces only. When it reaches the size of the table row preferred width, each extra space (defined by extraLines in the code) will create a new line.

Let me know if you need more information!

Regards.

@lucas-viva
Copy link
Author

Note: I was playing around with the code above and doing some experiments.

I noticed that if you replace font.regenerateLayout by font.markup at TextraLabel.layout, it starts working (rollback 9087144).

@tommyettinger
Copy link
Owner

I know markup() is an option, unfortunately it is quite a bit slower (and allocates more) than regenerateLayout() if it is run every frame. I think this bug is fairly old -- I have projects that depend on specific versions in the versions/ folder, and I believe the latest version can reproduce this bug, so I'll be able to check it with several older versions. This bug isn't isolated to spaces; it can also affect single words that get

placed
on
individual
lines

like this. I'll see what I can do to fix this when I'm changing word wrap to match libGDX behavior (related to #11 ).

tommyettinger added a commit that referenced this issue Aug 19, 2023
The bug causes each word to wrap onto its own line, somehow -- it looks like https://i.imgur.com/gN3LsoU.png .
tommyettinger added a commit that referenced this issue Aug 20, 2023
I have at least the non-space case fixed, where words would take up an entire line regardless of width: https://i.imgur.com/QVL9bU0.png . I'll try out the repro case in issue 13 next.
tommyettinger added a commit that referenced this issue Aug 20, 2023
This can be seen as a temporary fix, since I need to rewrite large parts of the word wrap API to match libGDX behavior, but it does make it so multiple spaces are kept (except for the last one on a line, which becomes a newline and can be turned back into a space by repeated wrap changes).
@tommyettinger
Copy link
Owner

This should be mostly fixed by 0f821d7, but it would be nice to know if it is fixed in your app's/game's case, @lucas-kakele . Can you try a JitPack dependency on the latest commit? https://jitpack.io/#tommyettinger/textratypist/0f821d7390

I'm not entirely sure the multiple-space logic is going to be how people want it, but now it definitely doesn't add a newline after every space. As normal, it converts one trailing space (the last one on a line) to a newline (\n char, though I think the code might then transform it to \r, since earlier versions used \r to indicate a soft wrap). If there are many spaces, it only converts the trailing spaces to newlines now, and leaves the rest in-place.

@lucas-viva
Copy link
Author

Thank you Tommy for promptly working on a fix!

I'm traveling this week and will be able to test it next week. I will keep you posted.

Regards.

@lucas-viva
Copy link
Author

Hi Tommy.

I tested your latest commit both in my game and in the example above.

It looks like text bounds are not working properly now.

Gradle:

api "com.github.tommyettinger:textratypist:0f821d7390"

Demo code with this is a normal text test test test! text (let me know if you need more string examples):

image

tommyettinger added a commit that referenced this issue Aug 29, 2023
It still seems to be working when making proper use of scene2d.ui's "preferred size" inside a Table cell or Container.
@tommyettinger
Copy link
Owner

Ugh, the sizing logic for anything involving scene2d.ui is just maddening. I tried this in Issue13Test in this commit: 513c6b8 and it measures the width and height correctly there, for either TextraLabel or TypingLabel. I am guessing something is different with your example, but I'm not sure what. Wrap could be turned off, which would be strange because it is wrapping... The label might not be in a Table cell or Container, which would make its preferred size not necessarily match its actual size. Something might need to be pack()-ed or invalidate()-ed; I can never tell when scene2d.ui needs that, though I don't need it in the linked test.

java_l1RANU2wUN

@lucas-viva
Copy link
Author

I'm not sure what are the color differences (red/green) when enabling scene2d.ui debug, but for the Issue13Test code, the height is still wrong if you print the label height.

If you remove stage.setDebugAll(true) and use table.debug, it will draw the height accordingly (in red).

TypingLabel is working correctly for me.

@tommyettinger
Copy link
Owner

tommyettinger commented Aug 31, 2023

I'll take a look again... I was pretty sure debugAll() just set the debug mode on everything in the stage. It's also possible that the height starts incorrect and becomes correct by the time it's displayed, which would explain the print but not the red outline.

EDIT: Yeah, the height starts at one line (20 pixels) even though the preferred height is the correct 4 lines (80 pixels). Calling table.pack() will make the preferred size calculate properly for everything in the table, and after that a TextraLabel's getHeight() will be correct. I think TypingLabel may do some of what pack() does, internally. I can't possibly claim to understand all of scene2d.ui, but I know calling pack() is often an answer to some problems.

tommyettinger added a commit that referenced this issue Sep 6, 2023
Now a TextraLabel will show the correct debug bounds even without calling pack(), which is the same behavior as TypingLabel. It shows the correct actual height in the println() call once pack() has been called.
@lucas-viva
Copy link
Author

Thank you for your work so far in this issue!

I was testing the changes, for the following versions:

libGDX: 1.12.2-SNAPSHOT
textratypist: 6406d6f (last commit)

And the following code:

package com.mygdx.game;

import com.badlogic.gdx.ApplicationAdapter;
import com.badlogic.gdx.Gdx;
import com.badlogic.gdx.scenes.scene2d.Stage;
import com.badlogic.gdx.scenes.scene2d.ui.Skin;
import com.badlogic.gdx.scenes.scene2d.ui.Stack;
import com.badlogic.gdx.scenes.scene2d.ui.Table;
import com.badlogic.gdx.utils.Align;
import com.badlogic.gdx.utils.ScreenUtils;
import com.badlogic.gdx.utils.viewport.ScreenViewport;
import com.github.tommyettinger.textra.TextraLabel;

public class MyGdxGame extends ApplicationAdapter {

  private Stage stage;
  private Skin skin;

  @Override
  public void create () {
    stage = new Stage(new ScreenViewport());

    skin = new Skin(Gdx.files.internal("uiskin.json"));

    TextraLabel label = new TextraLabel("This is a test. This is a test. This is a test. This is a test. This is a test.", skin) {
      @Override
      public void layout() {
        float width = getWidth();
        if (style != null && style.background != null) {
          width = (width - (style.background.getLeftWidth() + style.background.getRightWidth()));
        }
        if (wrap && layout.getTargetWidth() != width) {
          if(width != 0)
            layout.setTargetWidth(width);
          font.regenerateLayout(layout);
          //invalidateHierarchy(); // Uncomment this line to fix the issue.
        }
      }
    };
    label.setWrap(true);
    label.setAlignment(Align.left);

    Table table = new Table();
    table.add(label).prefWidth(300).row();
    table.add().growY();

    Stack stack = new Stack(table);
    stack.setFillParent(true);

    stage.addActor(stack);
  }

  @Override
  public void render () {
    ScreenUtils.clear(0, 0, 0, 1);
    stage.act();
    stage.draw();
  }

  @Override
  public void dispose () {
    stage.dispose();
    skin.dispose();
  }
}

Will result in this:

image

You can see the top of the label gets cut out.

Uncommenting the line at the TextraLabel layout method (invalidateHierarchy();) will fix it. I'm not sure if this is the actual solution, but I couldn't reproduce the error on other labels and compositions of UI (like label inside a table inside a scrollpane, etc):

image

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