Skip to content

Updated JOGL canvas classes to fix shutdown bug. Also added frame rate support.#174

Merged
3 commits merged into
jMonkeyEngine:masterfrom
saloisio:master
Sep 23, 2014
Merged

Updated JOGL canvas classes to fix shutdown bug. Also added frame rate support.#174
3 commits merged into
jMonkeyEngine:masterfrom
saloisio:master

Conversation

@saloisio
Copy link
Copy Markdown
Contributor

Changes to JOGL canvas classes to fix stalled shutdown, as discussed on the jme forum: http://hub.jmonkeyengine.org/forum/topic/testcanvas-doesnt-shutdown-with-jogl-renderer/

@shadowislord
Copy link
Copy Markdown
Member

@gouessej: Is it fine to merge this?

@ghost
Copy link
Copy Markdown

ghost commented Sep 19, 2014

@shadowislord Thank you for bumping me about this change. Please wait for a few minutes, I have to digest the salad, the tartiflette and the milk chocolate warmed lava cake.

@ghost
Copy link
Copy Markdown

ghost commented Sep 19, 2014

Personally, I would call animator.isAnimating() before calling animator.stop() and keep in mind that the requested frame rate can be different from the chosen frame rate.

@saloisio
Copy link
Copy Markdown
Contributor Author

From what I can see, the animator is started in startGLCanvas() and then it is never stopped. In what case will animator.isAnimating() return false? The frame rate support I added is basic but it does work, and I thought it would be better to have some frame rate support rather than none.

@ghost
Copy link
Copy Markdown

ghost commented Sep 20, 2014

It's difficult to imagine how the developers use the public APIs. animator.isAnimating() returns false when the animator hasn't been started, is paused or stopped.

I understand your position but I just remind you that FPSAnimator doesn't have a consistent cross-platform behaviour. The vertical synchronization is more reliable. Yes, some support is better than nothing.

@saloisio
Copy link
Copy Markdown
Contributor Author

The animator is not accessible through any public API. This was the reason I needed to modify the source in the first place. If this is an incorrect assumption please explain how I could gain access to the animator without extending any of the canvas classes.

It would be great to have vsync support as well. I did not see any easy way to do this so settled for frame rate setting, which is accurate enough (+- 1 fps) on a Windows 7 OS.

@ghost
Copy link
Copy Markdown

ghost commented Sep 21, 2014

The animator can be stopped in JoglCanvas.display() for example. The vertical synchronisation is already handled here: https://github.com/saloisio/jmonkeyengine/blob/master/jme3-jogl/src/main/java/com/jme3/system/jogl/JoglAbstractDisplay.java#L118

@saloisio
Copy link
Copy Markdown
Contributor Author

Is this ok now?

@saloisio
Copy link
Copy Markdown
Contributor Author

@gouessej have you tested vsync on multiple jme contexts with the JOGL canvas? It doesn't work very well. Only one context is vsynced properly, the other has a very low fps. In my case on a 75 Hz screen I have context one refreshing at ~75 fps and context two refreshing at ~18 fps. When I re-run the test using setFramerate(60) both contexts refresh at ~60 fps. Seems my updates to support setting frame rate is more reliable at the moment.

@ghost
Copy link
Copy Markdown

ghost commented Sep 22, 2014

Your change is ok for me. It's valuable and useful, I don't deny it. As I said, FPSAnimator doesn't have a consistent behaviour across platforms and your update doesn't do anything about that. You need to use a swap group (NV_swap_group) to use v-sync on several canvases / windows:
https://www.opengl.org/registry/specs/NV/glx_swap_group.txt
http://jogamp.org/deployment/jogamp-next/javadoc/jogl/javadoc/javax/media/opengl/GLContext.html#queryMaxSwapGroups(int[],%20int,%20int[],%20int)

You can look at this unit test based on JOGL and NEWT:
http://jogamp.org/git/?p=jogl.git;a=blob;f=src/test/com/jogamp/opengl/test/junit/jogl/acore/TestNVSwapGroupNEWT.java;h=54f99433cc8bbaa77d0675bf3d3ed47c7f4e753e;hb=HEAD#l75

@saloisio
Copy link
Copy Markdown
Contributor Author

Ok good. So can you merge this?

ghost pushed a commit that referenced this pull request Sep 23, 2014
Updated JOGL canvas classes to fix shutdown bug. Also added frame rate support.
@ghost ghost merged commit e448e46 into jMonkeyEngine:master Sep 23, 2014
This pull request was closed.
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

Successfully merging this pull request may close these issues.

2 participants