The Problem is that DebugGamestate creates an StatisticsGamestate in its Constructor.
In the StatisticsGamestates Constructor, a LineGraph is created which throws a NPE if it happens outside the OpenGL thread.
This fix creates the StatisticGamestates in the update() Method instead inside the constructor.
The GameStates update() and render() Methods are usually called inside the OpenGl thread. (GameStateManager).
edit: new patch
Index: src/com/jmex/game/state/DebugGameState.java
===================================================================
--- src/com/jmex/game/state/DebugGameState.java (revision 4029)
+++ src/com/jmex/game/state/DebugGameState.java (working copy)
@@ -71,6 +71,7 @@
protected boolean showBounds = false;
protected boolean showDepth = false;
protected boolean showNormals = false;
+ private boolean statisticsCreated = false;
public DebugGameState() {
this(true);
@@ -84,9 +85,6 @@
private void init(boolean handleInput) {
rootNode = new Node("RootNode");
- // create a statistics game state
- GameStateManager.getInstance().attachChild(new StatisticsGameState("stats", 1f, 0.25f, 0.75f, true));
-
// Create a wirestate to toggle on and off. Starts disabled with default
// width of 1 pixel.
wireState = DisplaySystem.getDisplaySystem().getRenderer()
@@ -211,6 +209,12 @@
/** If toggle_stats is a valid command (via key F4), change depth. */
if (KeyBindingManager.getKeyBindingManager().isValidCommand(
"toggle_stats", false)) {
+ if (statisticsCreated == false) {
+ // create a statistics game state
+ GameStateManager.getInstance().attachChild(
+ new StatisticsGameState("stats", 1f, 0.25f, 0.75f, true));
+ statisticsCreated = true;
+ }
GameStateManager.getInstance().getChild("stats").setActive(
!GameStateManager.getInstance().getChild("stats").isActive());
}
see also:
http://www.jmonkeyengine.com/jmeforum/index.php?topic=9124.0
Issue #18
Even better would be to make TextureRenderer's Constructor and TextureRenderer.setupTexture() thread save, but thats a bit more work, and i'm not sure how to do it.
edit:
changed with jjmontes suggestions
I already stated (in the other thread) I don't agree with this but in any case, isSetUp should be set to true within the setupGraph() method, which should be public.
But how should StatisticsGamestate then be used, would the user call setUpGraph() wrapped in a GameTask?
I think in its current state StatisticsGamestate can be more seen as an example.
There are many more things which are not very nice.
For example the width / height and position of the graph and text should be easier to set etc.
If you keep lazy initialization, user could chose to pre-initialize or to let it auto initialize on the first update.
Even if you see StatisticsGameState as an example the point for me is that we should try to keep consistency about how GameStates are designed and used across JME… if we do this here then we should provide lazy initialization in every state that needs to be created in the OpenGL task, and I don't quite like that: using a particular thread should be up to the user.
Also, StatisticsGameState could be improved but that doesn't mean that it is not the current stable version :). I don't see it as an example, but maybe as an early version. However the general contract should be maintained not only here, but in all GameStates.
I am sorry but moving your initialization code into a GameTask to run in the OpenGL thread shouldn't be that problematic. We'd rather correct the code relying on it to run on the OpenGL thread.
Mmm… Now reading this I sound a bit strong. So I just want to add that I'm open to any other point of view, I don't know the absolute truth. My main concern is about consistency. If we go this way then we should expect all GameStates to do the same (that would be part of the GameState contract).
we'd rather correct the code relying on it to run on the OpenGL thread
That is of course the best idea, but that would mean changing TextureRenderer, which i tried but failed miserably :)
Mmm... Now reading this I sound a bit strong
no worries, we're here to discuss these things :)
I am sorry but moving your initialization code into a GameTask to run in the OpenGL thread shouldn't be that problematic
Do you mean simply wrapping setupGraph() in a GameTask? That would work too of course and we wouldn't touch the update method.
Index: src/com/jmex/game/state/StatisticsGameState.java
===================================================================
--- src/com/jmex/game/state/StatisticsGameState.java (revision 4029)
+++ src/com/jmex/game/state/StatisticsGameState.java (working copy)
@@ -31,6 +31,7 @@
*/
package com.jmex.game.state;
+import java.util.concurrent.Callable;
import java.util.logging.Logger;
import com.jme.renderer.ColorRGBA;
@@ -40,6 +41,7 @@
import com.jme.scene.shape.Quad;
import com.jme.system.DisplaySystem;
import com.jme.util.Debug;
+import com.jme.util.GameTaskQueueManager;
import com.jme.util.stat.StatCollector;
import com.jme.util.stat.StatType;
import com.jme.util.stat.graph.GraphFactory;
@@ -95,7 +97,14 @@
this.heightFactor = heightFactor;
this.alpha = alpha;
this.doLineGraph = doLineGraph;
- setupGraph();
+ GameTaskQueueManager.getManager().update(new Callable<Object>() {
+ @Override
+ public Object call() throws Exception {
+ setupGraph();
+ return null;
+ }
+ });
+
}
private void setupGraph() {
I just don't really like to tell users to use GameTasks to use the DebugGameState or StatisticsGameState, those things should work out of the box.
Mmm…l now we have a GameState that we cannot preload, or if we preload it will be initialized twice…
The GameState itself shouldn't be aware of this. I reckon it should be created elsewhere inside the OpenGL thread, and that that's up to user code. For example, DebugGameState could "create and attach" the StatisticsGameState inside a GameTask. Then, I see no problem calling setupGraph() from StatisticsGameState's constructor as it is common practice to perform initialization and loading tasks in GameState constructors in any case.
I have checked TextureRenderer and I can't see where StatisticsGameState or DebugGameState are used.
jjmontes said:
I have checked TextureRenderer and I can't see where StatisticsGameState or DebugGameState are used.
no no, TextureRenderer is the root of the problem :)
Thats the dependency:
DebugGameState
It will add a GameTask at the beginning of my loop.
Seriously I really think initialization is a matter of the user. DebugGameState should create that GameTask. And it should do it inside a similar doSetup() method, not by default.
Now, I don't like this as I am supposed to do the same with all my GameStates that require to be initializated in the OpenGL thread? What if we have dozens of gamestates that loads dozens of textures? I don't want all of them to be initialized like that :(.
To me, this modification has very serious implications. I repeat: gamestates should not target any particular thread.
I honestly don't see the reason to do this. Why not leaving as it is and initializing the DebugGameState in a GameTask?
jjmontes said:
It will add a GameTask at the beginning of my loop.
hmm but i don't see why this is a problem, can you explain what issues that brings up?
Why not leaving as it is and initializing the DebugGameState in a GameTask?
Because before we added StatisticsGamestate to DebugGameState, the DebugGamestate was doing fine without a GameTask.
Now if a User switches from jme 1 to 2 it suddelny throws a NPE at him.
I think DebugGamestate / StatisticsGamestate should be as easy as possible to use.
ok, updated first post with a new patch after chatting with jjmontes in irc 
We leave StatisticsGamestate as it is but fix DebugGameState.
will check in tomorrow
checked in, also enabled statistic gathering in ModelLoader.
Thanks Core!
(BTW: what does "enabled statistic gathering in ModelLoader" mean?)
it is needed to set a property to make the StatisticsGamestate do its job, see javadoc of StatisticsGamestate
Yeah I know about the property. Where is it being set?
I thought that property wasn't set by default for performance reasons, and that was up to the user to set that up.
I don't see why ModelLoader should be setting that by default.
And in any case, that's a change that wasn't proposed in the first place to this thread. If we were serious you should revert that change and open a new thread for that.
:( Sorry. I just can't help saying what I think. This new development model scares me.
If you had checked the source you would have seen its just a minor thing.
ModelLoader has a convenience main Method to load and display models.
It also has a DebugGamestate to move around the model.
Enabling Statistics gathering makes the StatisticsGamestate actually display anything at all, which is quite useful in the ModelLoader anyway.
So this i really just a minor thing.
But i get your point, believe me, i am very cautious if i check in anything.
You just have to think about this like a bill being voted on…before it can make it in it must have a lot of extra pork added first. 
Core-Dump, I don't see anything wrong with what you added…just wanted to throw out the comment.
I am ok with the changes too.
I am trying to make some noise against the system.
I think I also made my point.
And sorry Core-Dump for taking you as an example (it was just because I actually didn't agree with the initial patch for StatisticsGameState). I very much respect your opinion and contributions to this forum. I reckon everybody knows who is who and that's why I thought that I could throw this at you without anyone taking this personally.
AND… It didn't work. I updated my SVN and tried it again with nothing new happening. Maybe I uppdated my SVN to early…?
what didn't work ?
i verified that ModelLoader and the TestStandardGame example from the wiki works.