yellowstonegames / SquidLib

Useful tools for roguelike, role-playing, strategy, and other grid-based games in Java. Feedback is welcome!
Other
448 stars 46 forks source link

Exception/Screen gets destroyed if game window looses focus #177

Closed coldwarrl closed 6 years ago

coldwarrl commented 7 years ago

If the game window looses focus (or I switch to another application on the desktop) either the screen looses its content / it shows some weird artifacts or (mostly) I get following exception:

Exception in thread "LWJGL Application" java.lang.NullPointerException at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addGlyph(BitmapFontCache.java:389) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addToCache(BitmapFontCache.java:380) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:512) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:506) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:480) at com.badlogic.gdx.graphics.g2d.BitmapFont.draw(BitmapFont.java:201) at squidpony.squidgrid.gui.gdx.TextCellFactory.draw(TextCellFactory.java:1002) at squidpony.squidgrid.gui.gdx.SquidPanel.draw(SquidPanel.java:657) at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110) at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57) at squidpony.squidgrid.gui.gdx.SquidLayers.draw(SquidLayers.java:1049) 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.ui.WidgetGroup.draw(WidgetGroup.java:163) 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)

(or similar).

I get an instance of the textCellFactory by: val factory = new TextCellFactory() factory.font(DefaultResources.getSquareSmoothFont)

Do i need to do sth if the game window/fullscreen looses focus ?

coldwarrl commented 7 years ago

It happens even without changing the focus but only rarely

tommyettinger commented 7 years ago

Seems like some annoying junk related to libGDX's resume() and pause() methods as they are used in DefaultResources, though the issue would probably be present in any code that was resuming/pausing with GL resources involved. It looks like this is Scala code? I'll try to reproduce in Java, but it's possible the libGDX lifecycle (the system that calls pause() when the window loses focus and resume() when it regains focus) needs some special attention in Scala. It's also very possible I forgot something in dispose().

tommyettinger commented 7 years ago

Hm, I can't reproduce with a simple test using that font in Java. Could you be starting a libGDX application in a non-standard way? If the Gdx.app, Gdx.graphics, and similar static fields aren't accessible at some point, there might be some issue with how your code is handling the application lifecycle. DefaultResources relies on the behavior of the libGDX lifecycle so it can work at all on Android. Standard libGDX advice of "make sure to implement ApplicationListener (or extend ApplicationAdapter or Game), and when starting your application, create that ApplicationListener in the appropriate way for your platform" still applies. Is your code available anywhere for me to test? I think I can get Scala running pretty quickly; I've written part of a game in it before.

tommyettinger commented 7 years ago

There's also the possibility that you're calling dispose() somewhere when it shouldn't be. Generally you almost never need to call dispose() yourself, and DefaultResources should be handling disposal. If you dispose a resource and then try to use it, that's a bad thing, and you will get NullPointerExceptions at best and segfaults at worst.

coldwarrl commented 7 years ago

Thanks for your quick investigation. Really strange, since I've a backup screen without Squidlib and there are no issues.

I'll check your example and recheck my coding and come back. Yes it's Scala but I don't guess that this is the issue.

coldwarrl commented 7 years ago

So if I get an instance instead by

val factory = DefaultResources.getStretchableSquareFont (or any other font by using this approach)

all is fine...have not found the issue so far but I guess it's somehow related how the stuff is buffered in DefaultResources

BTW: I have not implement any dispose()

tommyettinger commented 7 years ago

Yeah, it is puzzling. There's the libGDX class BitmapFont, which doesn't add any special traits to a font and uses one size unless roughly scaled, and the SquidLib class TextCellFactory, which wraps BitmapFont and sometimes adds traits that allow it to be smoothly scaled (it uses a shader and a specially prepared font image). The getStretchableThingFont methods return a copy of a TextCellFactory, not the original, while the earlier methods like getSquareSmoothFont return a BitmapFont directly. I suspect the original issue is related to SquidLib failing to copy the BitmapFont, while it does copy the TextCellFactory and I think its internal BitmapFont as well. I still can't test what's causing the issue though. Are you on a recent version of SquidLib? There's instructions for getting the latest dependency, including for SBT, at https://jitpack.io/#SquidPony/SquidLib/99542ec0fe . I'll see if I can make a small Scala demo to test on.

coldwarrl commented 7 years ago

with the latest commit, I get still null pointer exceptions, especially if another window gets the focus. val factory = DefaultResources.getStretchableSquareFont seems working fine

tommyettinger commented 7 years ago

I just fixed it, not long after your last comment. I have a Scala demo here, https://github.com/tommyettinger/SquidLib-Demos/tree/master/ScalaDemo , and the behavior was actually related to SquidLib calling dispose() when the application is paused. Something in the Scala version (I directly ported a working Java example) behaved differently about the OpenGL context, and the disposed values were not getting re-assigned useful font values. Now, the code doesn't ever dispose BitmapFonts on pause, only disposing TextCellFactories since those worked before. I do think that the stretchable distance field fonts look better, especially if the window can resize, so I'd encourage you to use one of those if you find one you like. They can stretch a lot horizontally to make any stretchable font have a square aspect ratio; you would call width() and height() on the TextCellFactory or use SquidLayers' cell sizes in the constructor (which call width() and height() internally). I'll wait to hear if the latest commit, on JitPack it's e311887dfc, fixed it for you before I close.

coldwarrl commented 7 years ago

Yup, works and thanks 👍

coldwarrl commented 7 years ago

Sorry to report that, but I still face this issue, although it occurs only rarely and I cannot reproduce it deterministically . Sometimes it does not happen, sometimes it happens after the first focus change (and once it happened even during the first startup).

Could you check it on your side: Run it in the IDE and switch back/from it a couple of times...sometimes you get again those weird NullPointer in

at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addGlyph(BitmapFontCache.java:389) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addToCache(BitmapFontCache.java:380) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:512) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:506) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:480) at com.badlogic.gdx.graphics.g2d.BitmapFont.draw(BitmapFont.java:201) at squidpony.squidgrid.gui.gdx.TextCellFactory.draw(TextCellFactory.java:995) at squidpony.squidgrid.gui.gdx.SquidPanel.draw(SquidPanel.java:657) at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110) at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57) at squidpony.squidgrid.gui.gdx.SquidLayers.draw(SquidLayers.java:1049)

coldwarrl commented 7 years ago

In the debugger I've seen that https://github.com/SquidPony/SquidLib/blob/master/squidlib/src/main/java/squidpony/squidgrid/gui/gdx/TextCellFactory.java, function dispose()

gets called if the application window looses focus. Is this the intended behavior, since then the Bitmap font gets disposed as well ?

tommyettinger commented 7 years ago

I'm not sure if my calling dispose() on BitmapFonts in my code is correct, now that I've seen the improvement gained by not disposing BitmapFonts elsewhere. Are you using a BitmapFont now or are you using a TextCellFactory from DefaultResources?

tommyettinger commented 7 years ago

What's more strange is that TextCellFactory.dispose() should only be called by libGDX when the application as a whole is shutting down, or manually if someone wants to dispose of its resources. As far as I can tell, the only calls to TextCellFactory.dispose() are in DefaultResources, and I just committed some code that moves the calls to dispose() into DefaultResources.dispose() instead of DefaultResources.pause(). I'll test this out in my Scala demo as well. Current JitPack dependency version is 47fbfc35a6.

coldwarrl commented 7 years ago

no, using defaultResources; yes it's strange, even more strange is that it even sometimes happens if I do not loose the focus...ok, will try it out when I come home...

coldwarrl commented 7 years ago

What I can say with the latest commit is that now dispose() @TextFactory seems not be called anymore.

I did some tests and from time to time I still get Exception in thread "LWJGL Application" java.lang.NullPointerException at com.badlogic.gdx.graphics.g2d.GlyphLayout.setText(GlyphLayout.java:141) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:505) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:480) at com.badlogic.gdx.graphics.g2d.BitmapFont.draw(BitmapFont.java:201) at squidpony.squidgrid.gui.gdx.TextCellFactory.draw(TextCellFactory.java:995) at squidpony.squidgrid.gui.gdx.SquidPanel.draw(SquidPanel.java:657) at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110) at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57) at squidpony.squidgrid.gui.gdx.SquidLayers.draw(SquidLayers.java:1146) 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.Group.drawChildren(Group.java:110) 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.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 ui.main.LibgdxAdapter.render(LibgdxAdapter.scala:81) at com.badlogic.gdx.backends.lwjgl.LwjglApplication.mainLoop(LwjglApplication.java:225) at com.badlogic.gdx.backends.lwjgl.LwjglApplication$1.run(LwjglApplication.java:126)

If it would be always reproducible :-(

tommyettinger commented 7 years ago

Is it possible there's some concurrency in your game/application? I'm going over the lines in the stacktrace and the only causes I can see for line 141 of GlyphLayout throwing the Exception it does are related to either some code that's at fault in libGDX or some code concurrently modifying (maybe indirectly) the Pool of GlyphLayout.GlyphRun objects used in GlyphLayout. The code I have for the relevant lines:

                    GlyphRun run = glyphRunPool.obtain();
                    run.color.set(color); // Exception happens on this line

Here, run could maybe be null if there's concurrent freeAll calls to the Pool of GlyphLayout.GlyphRun objects, while run.color should never be null in Java, but could somehow maybe be null in Scala (it's a final field and is always initialized to a new Color() that is mutated). It seems possible that this is related to libGDX's Array class not being thread-safe and some section of Scala code (maybe library code) is expecting that collections are thread-safe, doing some concurrent operation on SquidPanels, and depleting the Pool (setting all GlyphRuns to null) while another thread tries to get a value from it, and the other thread gets the Exception. Concurrent usage of GUI widgets shouldn't be expected to work in any language, due to OpenGL being single-threaded. There doesn't need to be any explicit calls to freeing GlyphRuns in the Pool in concurrent code for this to be a problem, since freeAll is called in GlyphLayout.

Another possibility is that you're using TextPanel or LinesPanel (or some other widget that uses the same BitmapFont internally as the one used by a TextPanel or LinesPanel), and using the char [ in a way that isn't working with libGDX's Color markup. TextPanel and LinesPanel, in their default configuration, use libGDX's markup for colors to handle color inside Strings, and this reserves [ as the start of color markup. It needs to set this on the BitmapFont itself, which seems strange to me. If you have [ on its own and use TextPanel or LinesPanel, first try copying any TextCellFactory that's shared between a TextPanel or LinesPanel and a SquidPanel/SquidLayers so they get their TextCellFactory objects from different calls to DefaultResources.getStretchableSquareFont() or whatever font you use, and if that doesn't work, keep going and change the markup flag for the SquidLayers' fonts (not the ones used with the TextPanel or LinesPanel) with myTextCellFactory.font().markupEnabled = false

tommyettinger commented 7 years ago

Oh, and also: what OS are you running this on, and if it applies, what version of OS and/or desktop/windowing toolkit if you use one? I'm wondering if I may have trouble reproducing the issue because foreground and background windows aren't treated the same on my OS, Windows 7, relative to yours.

coldwarrl commented 7 years ago

I'm working on Windows 10 but could also test it on Ubuntu.

I do use concurrency but not in the UI layer and I use for that (backend) the akka framework, which program model ensures that you do not have the usual Thread 'pifalls'. And Scala has its own special custom concurrency collections, so the standard ones do not expect that a collection is thread-safe.

tried markupEnabled but this does not help either.

Sometimes, instead of a nullpointer, I get also a

Exception in thread "LWJGL Application" java.lang.ArrayIndexOutOfBoundsException: 60 at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addGlyph(BitmapFontCache.java:409) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addToCache(BitmapFontCache.java:380) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:512) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:506) at com.badlogic.gdx.graphics.g2d.BitmapFontCache.addText(BitmapFontCache.java:480) at com.badlogic.gdx.graphics.g2d.BitmapFont.draw(BitmapFont.java:201) at squidpony.squidgrid.gui.gdx.TextCellFactory.draw(TextCellFactory.java:995) at squidpony.squidgrid.gui.gdx.SquidPanel.draw(SquidPanel.java:657) at com.badlogic.gdx.scenes.scene2d.Group.drawChildren(Group.java:110) at com.badlogic.gdx.scenes.scene2d.Group.draw(Group.java:57) at squidpony.squidgrid.gui.gdx.SquidLayers.draw(SquidLayers.java:1146) 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.Group.drawChildren(Group.java:110) 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.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) Currently, I'm using only SquidLayers, wrapped in my customs views (which inherit from a group). Getting instances of the (same, since cached) TextCellfactory and using them in different squidLayers should be fine, shouldn't it ?

EDIT: I've deleted my remark if it's related to IntelliJ's IDE debug mode because now I got the issue even after restarting the complete IDE

tommyettinger commented 7 years ago

Hmm. Calling DefaultResources.getStretchableSquareFont() will return a copy of the cached TextCellFactory, which has in the past helped avoid issues where size changes to a shared TextCellFactory affected all text with that font, even when unintended. It's still absolutely possible and sometimes desirable to share a reference to a TextCellFactory, but it won't be done automatically by DefaultResources. It shouldn't be a problem to have multiple SquidLayers; I've actually used multiple Stages before, each with a SquidLayers, to handle separate camera views. That code used one style of font, at two different sizes (so two TextCellFactory objects), across four SquidLayers objects. Have you gotten an impression of what causes the ArrayIndexOutOfBoundsException, or when it tends to happen? The index 60 seems to be what's out of bounds, but I don't know what array it's having trouble with, or what length it should have. I'll try to go through BitmapFontCache and see what might be happening.

coldwarrl commented 7 years ago

seen the issue

So, GlyphLayout public class GlyphLayout implements **Poolable** { **public final Array<GlyphRun> runs = new Array();**

So the 'runs' seem get inconsistent and the GlyphLayout is poolable...again I'm not an expert when it comes to libgdx...but maybe the objects go back to the pool although they shouldn't ? If the stage gets redrawn (in the game loop)...is the idea if the screen has not changed that all the stuff is not rendered or is it always re-rendered but the objects are pooled so no java memory leak occurs (or the garbage collector is too often triggered)

I'm too tired today :/ but I guess it would help that I re-run the game outside of the IDE a dozens of time if it also happens then (hm, should have had that idea earlier ;))..will test it and come back

tommyettinger commented 7 years ago

I haven't really used hot redeployment of code, mostly because I'm used to working without it, but it does seem like hot-swapping logic parts of a game shouldn't affect the internals of BitmapFont and GlyphLayout. Hot-swapping a TextCellFactory, SquidPanel, SquidLayers, or other visual object might, though, if parts are shared with things being drawn at that time. If the screen has not changed it still should usually draw because the screen is usually cleared each frame, which prevents buildup artifacts with transparency. How the pooling seems to work in libGDX is by setting fields to all null in a no-longer-needed pooled object (like a GlyphRun), then reassigning values to the all-null object when it would normally construct a new GlyphRun. Can you post some or all of what your render() function looks like in the class where the Exceptions are starting? I'm decent at reading Scala, though things like implicitly or most ScalaZ lib code still don't make sense.

tommyettinger commented 7 years ago

I finally got the Scala debugger to work, and the place where your code is getting the ArrayIndexOutOfBoundsException is very strange.

        final int page = glyph.page;
        int idx = this.idx[page];
        this.idx[page] += 20;

        if (pageGlyphIndices != null) pageGlyphIndices[page].add(glyphCount++);

        final float[] vertices = pageVertices[page];
        vertices[idx++] = x;         // Exception is thrown here
        vertices[idx++] = y;
        vertices[idx++] = color;
        vertices[idx++] = u;
        vertices[idx++] = v;

It should only have a value for pageVertices[page], and thus vertices.length, of 20 when it is drawing one char at a time, so since the Exception has an index of 60 being used in what is probably an array with length 20 or possibly 40, that explains part of the problem. The line when the exception is thrown involves SquidPanel at line 657, which only draws one char with TextCellFactory, so vertices.length should only ever be 20 at that point unless I'm missing something while debugging. It is possible that int idx = this.idx[page]; is having problems with hot-swapping or maaaaybe concurrency when the line after it, this.idx[page] += 20;, has already run more times than expected.

coldwarrl commented 7 years ago

so, checked it and unfortunately it occurs also outside the IDE.

My render function is pretty simple

` import com.badlogic.gdx.Gdx import com.badlogic.gdx.graphics.GL2

Gdx.gl.glClearColor(0, 0, 0, 1) Gdx.gl.glClear(GL20.GL_COLOR_BUFFER_BIT)

stage.act() stage.draw()`

Not sure if you meant this...

there isn't probably an easy method to prevent pooling, that is to force libgdx to create always new objects to see if this is the issue ?

tommyettinger commented 7 years ago

Hm, that looks fine. It looks like a any calls to SquidLayers.put() are happening in other code, but that shouldn't be a problem unless those calls are somehow happening in another thread while render() is in the middle of running. I may post a libGDX issue if I can figure out what causes this -- if your project is open source, can you link to any code that might show the problem, or if it isn't open source, can you post a .jar that I can run to try to see what might be related?

EDIT: no, I don't know of any way to prevent libGDX from pooling internally. If there is a bug, they may be able to fix it, but we don't really know what the cause is at this point.

coldwarrl commented 7 years ago

Since I've used the adapter pattern and not used Squidlib directly, it was pretty easy to implement a very simple version of an AsciiPanel by myself.

And I run into the same issue :(. So my issue is related to libgdx directly.

I've put all my render (put methods) code directly into the libgdx render method, so there is no chance that a seperate thread makes trouble. I guess I ask the libgdx guys if they have any clue what's going on.

Anyways, thx a lot for your help.

coldwarrl commented 7 years ago

-> http://www.badlogicgames.com/forum/viewtopic.php?f=11&t=26715

tommyettinger commented 7 years ago

Yeah, this is a bizarre issue... I'll comment on the forum if I find any further information. I guess I should close this since it isn't an issue with SquidLib (since it happens with just a custom AsciiPanel without any Squid involved), but I'll wait to see if my code can be updated with a workaround for any possible libGDX bugs. This may have already been fixed in a nightly version of libGDX, and I can see if I can make at least one commit of SquidLib that depends on a libGDX nightly so you can see if it has different behavior. When libGDX releases something after 1.9.6, the nightly dependency will break, so I'll only change it to the nightly dependency when testing something for this bug.

coldwarrl commented 7 years ago

so, as described @badlogic forum the stuff runs with my own implementation...unfortunately still not with SquidPony :(...

I guess we close the issue now..I guess it's related to my app/Scala somehow, although it's a pity not using your great library :(

tommyettinger commented 7 years ago

Hm, if it works with your own implementation then maybe I can try to incorporate the same working techniques into SquidLib. I'll check the forum.

EDIT: Checked forum.

Was this a libGDX OrderedMap that you found worked for your glyphMap? Not squidlib-util's version, I'm guessing, since I thought you removed the dependency. If that changes how this works, I wonder what could be depending on iteration order... Is it still getting the same ArrayIndexOutOfBoundException in SquidLib? Unless you were using the brand-new code (SparseLayers) that uses an unordered map (but with a consistent hashing algorithm, so even that shouldn't have issues unless insertion/removal happens during iteration), all of the normal, frequently-used classes like SquidLayers draw their contents in a very stable, reliable order. Maybe because OrderedMap in libGDX and in SquidLib both do not define foreach(), iterating over them is OK regarding the pooling, but some detail of how pools work in LibGDX may not work well with foreach in Scala.

Hmm... Are you using a glyphMap or similar data structure in the SquidLib code that's having the issue? If you are, can you switch it to use the libGDX com.badlogic.gdx.utils.OrderedMap, and also maybe try using the squidpony.squidmath.OrderedMap class as a substitute, to see if there is a difference?

coldwarrl commented 6 years ago

No, I've used squidpony.squidmath.OrderedMap.

Although to my embarrassment , after done additional tests, I've to announce the error showed up again. (without SquidPony). So, I close the issue...SquidPony is still very helpful for non-UI stuff for me, for the UI itself I probably swap libgdx by JavaFx.

tommyettinger commented 6 years ago

Fair enough. I wonder what the issue is though... If your game/application gets open sourced at some point, at least the libgdx version that had the issues, maybe I could go through it and post an issue on libGDX, since this seems like a genuine bug in their code regarding pools and bitmap fonts.

coldwarrl commented 6 years ago

yes, I'll do...I'm occupied with a side project currently, but will check if it is feasible to create an example for the issue...thx for your offer

tommyettinger commented 6 years ago

So I should have posted on this issue when I found out about a possible cause, but WickedShell from the libgdx IRC channel on Freenode had a remarkably similar bug using Clojure, where he had issues with graphics code for fonts sometimes not running on the main thread. He solved it by cloning libGDX, adding debug information all over the possible problem spots, and using the debug info to find where a Texture might have been used before a graphics context was available. In his case, the issue was a Label defined at the top-level (in Java, this would be like public static final Label notGood = new Label("This is really not good...") inside a class, not in a method). His changes were pretty wide-ranging (you can see them at https://github.com/WickedShell/libgdx/commit/611c9d4358574f283fb5834a1bbb48d114dcbe9b or just download the code from https://github.com/WickedShell/libgdx/tree/wickedshell/error-checking-hack and compile it), but they may be important if threads need to be debugged in a case where the application isn't behaving predictably.