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

Rare NPE in GlyphLayout #7582

Open
upost opened this issue Jan 31, 2025 · 7 comments
Open

Rare NPE in GlyphLayout #7582

upost opened this issue Jan 31, 2025 · 7 comments

Comments

@upost
Copy link

upost commented Jan 31, 2025

I have an Android app in the Play Store.
Rarely I see exceptions like this in the Google Play Console:

Exception java.lang.NullPointerException:
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:176)
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:95)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.computePrefSize (Label.java:162)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.scaleAndComputePrefSize (Label.java:147)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.getPrefWidth (Label.java:244)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.<init> (Label.java:75)

or:

(RuntimeException root cause)
Caused by java.lang.NullPointerException:
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.calculateWidths (GlyphLayout.java:280)
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:266)
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:95)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.computePrefSize (Label.java:162)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.scaleAndComputePrefSize (Label.java:147)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.getPrefWidth (Label.java:244)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.<init> (Label.java:75)

or, maybe related:

Exception java.lang.IndexOutOfBoundsException:
  at com.badlogic.gdx.utils.Array.get (Array.java:155)
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:215)
  at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText (GlyphLayout.java:101)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.computePrefSize (Label.java:160)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.scaleAndComputePrefSize (Label.java:147)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.getPrefHeight (Label.java:253)
  at com.badlogic.gdx.scenes.scene2d.ui.Label.layout (Label.java:175)
  at com.badlogic.gdx.scenes.scene2d.ui.Widget.validate (Widget.java:88)
  at com.badlogic.gdx.scenes.scene2d.ui.Stack.layout (Stack.java:107)
  at com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup.validate (WidgetGroup.java:104)
  at com.badlogic.gdx.scenes.scene2d.ui.VerticalGroup.layout (VerticalGroup.java:201)
  at com.badlogic.gdx.scenes.scene2d.ui.WidgetGroup.validate (WidgetGroup.java:104)
  at com.badlogic.gdx.scenes.scene2d.ui.ScrollPane.layout (ScrollPane.java:498)

It happens in different labels, but only in labels, and I'm not able to reproduce it.

libgdx version: 1.12.1

Any idea?

@Tom-Ski
Copy link
Member

Tom-Ski commented Jan 31, 2025

Most likely this is related to threading. Are you doing any multithreading/http requests and mutating state/labels without making sure those mutations are running on main thread?

@upost
Copy link
Author

upost commented Jan 31, 2025

Yes there is stuff happening in the background but I am running UI related code only on the main thread because I know that libgdx is not threadsafe.
At least I hope so. I spent some time to find situations where this could go wrong but found nothing.
I also looked if there's a workaround because this seems to be related to GlyphLayout and nothing else, maybe overriding/implementing an own class which just catches the exception, because it is so rare. It just should not crash the app - if a label is empty once in a lifetime, that'd be acceptable.

@Tom-Ski
Copy link
Member

Tom-Ski commented Jan 31, 2025

Yeah, it definitely sounds like a multithreading issue. It can be super rare.

I would verify you aren't mutating label states from another thread, you can do this even without getting a crash.
This is a handy util I use when making sure methods are being called correctly.

package com.rockbite.engine.threadutils;

import com.badlogic.gdx.utils.GdxRuntimeException;
import lombok.Getter;
import lombok.Setter;

public class ThreadUtils {
	@Getter
	@Setter
	private static Thread gdxThread;

	public static void gdxThreadSafetyCheck () {
		if (!isOnRightThread()) {
			throw new GdxRuntimeException("Trying to use on non-gdx thread");
		}
	}

	public static void gdxThreadSafetyCheck (Thread accessThread) {
		if (Thread.currentThread() != accessThread) {
			throw new GdxRuntimeException("Trying to use on non-gdx thread");
		}
	}

	public static boolean isOnRightThread () {
		return Thread.currentThread() == gdxThread;
	}
}

You can just ThreadUtils.setGdxThread(Thread.currentThread()), in your create method of application listener. Then use gdxThreadSafetyCheck() anywhere you suspect it may be happening.

You can do this in conditional breakpoints on libgdx methods too with your debugger if you have lots of different access points.

@upost
Copy link
Author

upost commented Jan 31, 2025

Looks interesting, I will certainly look into it, thank you.

@LobbyDivinus
Copy link

LobbyDivinus commented Feb 3, 2025

I want to advocate for more methods that allow use in a parallel context. E.g. FontCache text can be set using an explicit GlyphLayout instance preventing access to the non thread safe static GlyphLayout pool. However, GlyphLayout does not offer such access to give the user control over how GlyphRuns get pooled.

An option could be to let each GlyphLayout store it's own reference to a glyph pool that can be set by the user.
Another one, less invasive one, would be the use of overrideable obtain and free methods for GlyphRun within GlyphLayout. This general pattern is already used all over libGDX to allow easy modification.

If you wonder why this is useful in the first place. I am generating a lot of mesh data, including ones for text, in some worker threads. Having to do this explicitly in the main thread would be a bummer.

@tommyettinger
Copy link
Member

Smiles, nods, and points at TextraTypist...

TextraTypist doesn't use any static pools, though I'd stop shy of calling it fully-thread-safe at this point. It uses a different technique for storing text internally, and this allows lots of additional markup to be used for various styling. For example, libGDX can do [BLUE]This text is blue![] This text is not! and TextraTypist can do [lighter dull blue][/][_]This text is a light grayish blue, with an oblique slant and underline![ ] This text is none of those! There's a TextraLabel that should be pretty close to drop-in compatible with Label, though because TextraTypist doesn't use BitmapFont in most places, a different LabelStyle class is used, and loaded by a subclass of Skin (FWSkin). All .fnt files can be loaded by TextraTypist with no changes, and FreeType can be used by depending on FreeTypist or just copying its 2-3 files into your code. FWSkin also allows loading the custom format used by FontWriter, hence the FW in the name, and it tends to work really well with these, but it doesn't require them.

I've been working on version 1.2.0 for a while now, and I'm not 100% sure when it can be released, because I've been thinking and saying "soon" for a month now or more. I might mark it 2.0.0, because it's a big improvement to usability in general, and some config from 1.1.0 needs changing (mostly because special config is needed less often).

It isn't a tiny change to go from a game that uses GlyphLayout and BitmapFont internals to one that uses TextraTypist, so obviously this isn't something to try just before you release! The docs could also use work for the patterns of class use and interactions between classes, but the JavaDocs are rather thorough at least. If you decide to use TextraTypist, let me know if there are features I can add!

@LobbyDivinus
Copy link

Thanks, TextraTypist definitely looks cool. Though, as you said, in my case it's just easier to copy libGDX' code to make the necessary changes and I don't want to add just another library for this seemingly small requirement. Besides that, I don't think it provides raw text mesh data like BitmapFontCache.getVertices().

LobbyDivinus added a commit to LobbyDivinus/libgdx that referenced this issue Feb 7, 2025
This PR basically serves as a solution to libgdx#7582 by allowing the user to implement custom pooling mechanic that could for example allow for thread safe use
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

4 participants