I have been using jME for quite a while now and noticed many issues.
I would like to get some comments regarding some of those before I add them to the issue tracker.
StandardGame
Cannot be used for an AWTCanvas or Applet. I suggest a unified jME application framework which can manage many of the requirments of 3D applications with multithreading, loading, gamestates and support for different GL contexts.
There's no way to access the gameThread handle outside of StandardGame.
Lack of support for render passes
GameStateManager and GameTaskQueueManager do not support multiple contexts because they are singleton classes.
Image
Lacks many OpenGL image formats. I have to use my alphamaps as RGBA8888 (4 times more memory) instead of ALPHA8 because jME doesn’t support it. Since my alphamaps are quite large, this will cause a performance impact.
Last, I suggest to improve the documentation by adding a warning when the method makes calls to the graphics hardware, to make multithreading easier. Here’s a screenshot of a good example:
If the application is multi threaded, the user is very likely to use the method deleteAll from a non-GL thread, causing a crash. Here are some methods which make GL calls:
Probably all methods with the word ‘draw’ or ‘render’ in the name
Renderer.setBackgroundColor
Skybox.preloadTextures
Renderer.applyStates
RenderState.apply
TextureState.load*
TextureState.delete*
JMEDesktop.setup and all it’s constructors (It’s generally bad to have GL code in constructors, because they are usually created in game loading threads)
MotionBlurRenderPass.reloadShader and it’s constructor
WaterRenderPass.reloadShader and it’s constructor
SimpleWaterRenderPass.constructor
Spatial and friends.lockMeshes (this is also problematic because locks are made in game loading threads)
Spatial and friends.unlockMeshes
TextureManager.doTextureCleanup
TextureManager.preloadCache
There’s probably more problems that I forgot to mention, I might update this thread later.
Wow, this is the first time I've noticed this thread. I must have glazed right over it when it was posted.
StandardGame
- The game continues loading even if cancel is pressed at the Setting dialog
Fixed in CVS
- Cannot be used for an AWTCanvas or Applet. I suggest a unified jME application framework which can manage many of the requirments of 3D applications with multithreading, loading, gamestates and support for different GL contexts.
Can someone submit a patch to help fix this? I don't use AWTCanvas or Applets so I haven't looked at what would need to be changed to support this
- There's no way to access the gameThread handle outside of StandardGame.
This was on purpose. Can you give a valid reason why it would ever be good practice for this? The intent was to keep people from doing bad things to the thread.
- There's no name assigned to the gameThread handle, making it random like "Thread-5" which makes it hard to find the GL thread during profiling or debugging.[/quote
Just added this feature. It is now named "OpenGL".
- Lack of support for render passes
This is another area I'm going to need some help with. I haven't delved into passes yet, so someone that has needs to give me a hand here.
[quote - GameStates are renderered in the order they were attached, which can cause problems when a certain order is desired. I suggest a priority system for controlling the order of rendering (sort of like ZOrder for ortho spatials)
That is a feature request for GameStates, not StandardGame. If you're using ZOrder though then things render in their proper order anyway regardless of what order update is called in.
- GameStateManager and GameTaskQueueManager do not support multiple contexts because they are singleton classes.
There's actually a lot of work there that needs to be done to better support multiple contexts in the same VM.
- GameStateManager is initialized at StandardGame.initGame() while GameTaskQueueManager initializes at the first call to getManager()
Why does that matter? The premise of StandardGame is the utilization of GameStateManager, while GameTaskQueueManager may or may not ever be used in the application so it would make sense to only instantiate it if it's needed.
- cleanUp() is not called on GameStates that were detached from the GameStateManager, which could cause memory leaks.
I think the logic here was that you should be calling cleanup() yourself rather than depending on the GameStateManager to do so. Especially since just because you are detaching a GameState doesn't mean you necessarily want it to clean up, you may just be removing it temporarily and then adding it back.
Can you give a valid reason why it would ever be good practice for this? The intent was to keep people from doing bad things to the thread.
The first reason was to change its name, but that's fixed now.
This is another area I'm going to need some help with. I haven't delved into passes yet, so someone that has needs to give me a hand here.
If you're using ZOrder though then things render in their proper order anyway regardless of what order update is called in.
Unfotunatly, the setZOrder method only works for spatials rendered in the ortho queue; other queues sort by distance to camera.
I don't really know what is the best way to integrate passes into GameStates, because they are somewhat the same thing, the problem is that the official API uses passes for stuff like shadowing and other effects, which takes GameStates and StandardGame out of the loop.
Why does that matter? The premise of StandardGame is the utilization of GameStateManager, while GameTaskQueueManager may or may not ever be used in the application so it would make sense to only instantiate it if it's needed.
True but the task queue is automatically initialized because the update and render methods in StandardGame use them. Anyway it doesn't really matter, just an oddity I noticed.
I think the logic here was that you should be calling cleanup() yourself rather than depending on the GameStateManager to do so. Especially since just because you are detaching a GameState doesn't mean you necessarily want it to clean up, you may just be removing it temporarily and then adding it back.
Yeah you're right. What I wanted to say is that a shutdown-hook like feature should be added to StandardGame; i.e some code that needs to be called when the game ends to cleanup user allocated resources.
Yeah you're right. What I wanted to say is that a shutdown-hook like feature should be added to StandardGame; i.e some code that needs to be called when the game ends to cleanup user allocated resources.
What would you think about StandardGame's shutdown method iterating over the GameStates still attached to GameStateManager and removing and calling cleanup() on them? Would that suffice?
What would you think about StandardGame's shutdown method iterating over the GameStates still attached to GameStateManager and removing and calling cleanup() on them? Would that suffice?
That is what I am using right now, but it might not work in some cases. For example, my jnvperf API has a NVPerfGameState which requires some native resources to be cleaned when exiting, if the user detaches this GameState at some point, the resources will never get deallocated.
There are other uses for it as well, I am currently opening an editor window in my application and when I close the display, the editor doesn't close and the game continues running. As I said, this is just a suggestion, not really an issue.
Well, technically the "user" would never be removing a GameState, it's always your code that does it. ;) To that end, you can always call cleanup() on the state if you're finished with it. The only thing I think would be relevant for StandardGame to handle would be calling cleanup on the states that haven't been dealt with at shutdown.
Node.setModelBound()
Geometry.setModelBound()
- Throw NullPointerException when null is used as BoundingVolume. For model editors, removing bounding volumes from spatials is an expected feature. (i.e if you accidently set it and want to remove it)
Agreed. Fixed locally.
Momoko_Fan said:
Vector2f.getAngle()
- Returns an incorrect angle (see Math.atan2 spec why, range is -pi to pi) the correct return is FastMath.PI - FastMath.atan2(y,x)
There is no such method in Vector2f? Perhaps you mean angleBetween(Vector2f)? If so, the method returns the smallest angle difference between two unit vectors. Following that definition, it returns the correct value. I think you are right though that it would be helpful to allow for more information to be gleaned. I'll add both as methods.
I'll also add a getAngle method in case that is what you are talking about... I believe simply -FastMath.atan2(y,x) would be the correct result however.
Momoko_Fan said:
LWJGLDisplaySystem.createCanvas()
LWJGLDisplaySystem.createWindow()
- createWindow assignes the GLContext and creates a LWJGLRenderer, while createCanvas requires it to be done manually. For the same reason I don't really understand the JMECanvas system, why is SimpleCanvasImpl (a non lwjgl class) creates a LWJGLRenderer.
It's a hack. doSetup() in SimpleCanvasImpl is inside the GLThread whereas createCanvas will by definition not be (as the gl thread is owned by swing's repaint) We could add something to DisplaySystem that the canvas could call from doSetup to do the rest of the init though. I'll check that out.
Momoko_Fan said:
Image
- Lacks many OpenGL image formats. I have to use my alphamaps as RGBA8888 (4 times more memory) instead of ALPHA8 because jME doesn't support it. Since my alphamaps are quite large, this will cause a performance impact.
I suggest that you at least support the GL 1.1 formats listed here: http://http.download.nvidia.com/developer/OpenGL_Texture_Formats/nv_ogl_texture_formats.pdf
Here's another page listing the texture formats: http://www.bluevoid.com/opengl/sig00/advanced00/notes/node52.html
Yes, I agree. I'll look into it.
Momoko_Fan said:
TextureKey.setLocationOverride()
- In my opinion this is the worst place for an important method like this. Many people probably don't even know about this class. I suggest that you add a copy of this method to Texture and TextureManager.
This one should go away (or at least be unnecessary for a user to access) once some planned changes for 1.0 have been put in place.
I also agree about the multithreading documentation. But I'll leave this to another dev as I've probably picked up enough for my plate in this thread already. :P
After looking at Image… I'm thinking this would be a perfect place to start using enums in jME (about time ). Anyone with thoughts here? It will be a little bit of time before I can refactor this area.
ShadowedRenderPass doesn't work unless it is used together with another RenderPass in a PassManager (thanks to a user in the forum for reminding me of that)
The API uses Renderer.defaultStateList as a fallback states in case none have been set. This creates a requirement that all contexts within one VM must be from the same implementor otherwise the 2nd implementor will use the 1st's states and cause problems.
I am currently re-creating the jME displaysystem framework to more easily support multiple contexts and so this is one problem I encountered because I still would like to use LWJGLRenderer.
LWJGLDisplaySystem.createCanvas()
LWJGLDisplaySystem.createWindow()
- createWindow assignes the GLContext and creates a LWJGLRenderer, while createCanvas requires it to be done manually. For the same reason I don't really understand the JMECanvas system, why is SimpleCanvasImpl (a non lwjgl class) creates a LWJGLRenderer.
With regard to creating the LWJGLRenderer manually, can you point me towards an example of how to do this? I tried and I got this:
java.lang.NullPointerException
at com.jme.scene.state.lwjgl.LWJGLTextureState.<init>(Unknown Source)
at com.jme.renderer.lwjgl.LWJGLRenderer.createTextureState(Unknown Source)
at com.jme.renderer.lwjgl.LWJGLRenderer.<init>(Unknown Source)
With regard to creating the LWJGLRenderer manually, can you point me towards an example of how to do this?
The canvas must be attached and displaying before you can create a renderer, that's because LWJGLRenderer uses the GL context in its constructor. You need to create the renderer within your JMECanvasImplementor class.
I am happy I don't have to deal with any of this as I am using a custom jme context system...
Here's another:
FragmentProgramState.isSupported()
VertexProgramState.isSupported()
GLSLShaderObjectsState.isSupported()
- Those require me to create the state before I can check if the card supports the extension. The renderer should cache this info in a static field when its initializing; just like TextureState.getNumberOfUnits***
It's a hack. doSetup() in SimpleCanvasImpl is inside the GLThread whereas createCanvas will by definition not be (as the gl thread is owned by swing's repaint) We could add something to DisplaySystem that the canvas could call from doSetup to do the rest of the init though. I'll check that out.
How about creating the renderer from initGL in the LWJGLCanvas?
There is no such method in Vector2f?
I just remembered I modified my jME copy 6 months ago and added this method, sorry about that. :oops:
I'll also add a getAngle method in case that is what you are talking about... I believe simply -FastMath.atan2(y,x) would be the correct result however.
Well I prefer the angle to be returned between 0 and 2pi. Using your method would return the correct value but between -pi and pi which is not normalized.
The canvas must be attached and displaying before you can create a renderer, that's because LWJGLRenderer uses the GL context in its constructor. You need to create the renderer within your JMECanvasImplementor class.
I am happy I don't have to deal with any of this as I am using a custom jme context system...
Thanks for the rapid response.
I'm a total jME noob and you are way over my head here. When you say 'within your JMECanvasImplementor' do you mean I need to create an implementation of this class?
And I am lost with the custom context system. Has no meaning to me. Sorry.
Here's another:
FragmentProgramState.isSupported()
VertexProgramState.isSupported()
GLSLShaderObjectsState.isSupported()
- Those require me to create the state before I can check if the card supports the extension. The renderer should cache this info in a static field when its initializing; just like TextureState.getNumberOfUnits***
I think this is not too hard to support mainly because the rendering system creates one of each states to use as the default state (Renderer.defaultStateList), and it does so in the GL thread. So when a state is created you check if the variables are not set, and if so, GLContext is queried for support information.
