LWJGL 3 Capabilities (request for feedback)

Started by spasi, July 29, 2015, 22:51:55

Previous topic - Next topic

spasi

Hey all,

I'm currently working on the EGL & OpenGL ES bindings for LWJGL 3. It was the largest amount of work left for the 3.0 beta and it's almost done now. But I'm looking for some feedback on some API issues that have been bothering me. I want to get them fixed before the beta, if possible.

It has to do with the *Capabilities classes. In LWJGL 2 we had:

- OpenAL: n/a
- OpenCL: CLCapabilities, CLDeviceCapabilities, CLPlatformCapabilities
- OpenGL: ContextCapabilities
- OpenGL ES: ContextCapabilities

In LWJGL 3 we currently have:

- OpenAL: ALCapabilities, ALCCapabilities
- OpenCL: CLCapabilities
- OpenGL: ContextCapabilities
- OpenGL ES: ?

OpenAL in 3 supports several AL/ALC extensions, so we had to add new capability classes. OpenCL in 3 is not backwards compatible, so it was a chance to reduce complexity; we now have a single class.

My suggestion has two parts:

a) Rename the OpenGL ContextCapabilities to GLCapabilities. OpenGL ES would then have GLESCapabilities and everything will be symmetric with no aliasing. The downside will be that it will break (even more) existing LWJGL 2 code.

b) Replace GLContext GLContext.createFromCurrent() with GLCapabilities GL.createFromCurrent(). LWJGL would then hold the GLCapabilities instance in thread-local state, instead of the GLContext instance. I'd like to do this to decouple the capability instance (which contains the context function pointers, the stuff LWJGL needs to work) from whatever is managing the OpenGL context. The current situation is confusing, because GLFW has context management functions (e.g. glfwMakeContextCurrent(window)) and the GLContext instance has the same functions. It's not clear which ones must be called.

To move a context to another thread, now you'd do:

long window = glfwCreateWindow(...);
...
glfwMakeContextCurrent(window);
GLContext context = GLContext.createFromCurrent();
...
// in another thread
glfwMakeContextCurrent(window);
GL.setCurrent(context); // what does this do? do I have to call context.makeCurrent()?
if ( context.getCapabilities().OpenGL40 ) { ... }


Used concepts: GLFW window/context, GLContext, ContextCapabilities, GL

with the proposed change:

long window = glfwCreateWindow(...);
...
glfwMakeContextCurrent(window);
GLCapabilities caps = GL.createFromCurrent();
...
// in another thread
glfwMakeContextCurrent(window);
GL.setCurrent(caps); // better: we already made the context current, now let LWJGL know
if ( caps.OpenGL40 ) { ... }


Used concepts: GLFW window/context, GLCapabilities, GL. So we have to do deal with one less concept. The GLContext class would then only be useful if someone did custom context management without GLFW, via WGL/GLX/CGL. We could also just remove it altogether.

The proposed change is not only about usability and aesthetics. You can actually create and manage OpenGL contexts using EGL. Right now the GL thread-local state is coupled with the GLContext class. In order to create a "GLContextEGL" implementation, the two packages (org.lwjgl.egl and org.lwjgl.opengl) will be coupled as well. This is the kind of bad design we had to deal with in LWJGL 2 and I'd like to avoid that. If the GL only knows about GLCapabilities, that GLCapabilities instance can be created externally, without infecting anything.

I'd love to hear any thoughts on the above.

Cornix

Am I understanding correctly that options A and B are not mutually exclusive? As far as I can see they are both independent, right?

I like the sound of both of them. A solid and intuitive API is more important then backwards compatibility, especially since we already broke backwards compatibility so who cares.

spasi

Correct, they're independent, but parts of the same proposal.

The difficulty lies in that part B will break all existing LWJGL 3 code, in addition to requiring more work when porting LWJGL 2 code. It's going to be annoying for sure. Every current post, guide and code sample about LWJGL 3 will be wrong. The 3.0.0b nightly builds will be incompatible with 3.0.0a, etc.

Part A is just about having consistent naming and less ambiguity (GLCapabilities is more specific than ContextCapabilities). If part B is implemented, then A is "free" in terms of damage control.

Cornix

Well if that is so it is not really a question for us to answer since it would be you who has all the work to do with the refactoring. It is obviously a desireable output, but do the benefits outweight the work?

ra4king

The refactoring will not be difficult, the more difficult question to answer is what spasi proposed.

@spasi
Is it expected for there to be breaking changes from alpha to beta? I believe that in alpha it should be fine to make breaking changes. Beta is when everything should be locked and only bugs are fixed. Your proposals are most definitely much cleaner than the workaround, so I support these.
-Roi

spasi

It is expected, yes. But I should have worked on GLContext earlier, to avoid the pain. The problem only became apparent while working on EGL/GLES.

kappa

As for renaming ContextCapabilities to GLCapabilities, its a good change and I'd go for it. I'd not worry too much about breaking LWJGL2 code, since the current ContextCapabilities class in LWJGL3 has a slightly different API and has already broken it :)

abcdef

The changes don't look that bad so for the sake of making things cleaner and consistent I say go for it.

BrickFarmer

+1 go for it! it's beta after all and not that big a change for the benefit of a cleaner implementation.  Looking forward to trying ES!
Oculus Rift CV1, MBP 2016 - 2.9 i7 - Radeon Pro 460  OSX 10.12.4,  Win7 - i5 4670K - GTX1070.
Oculus Rift VR Experiments: https://github.com/WhiteHexagon

quew8

I also agree, make the changes. Clarity is paramount in an API if you ask me. Lots of people still haven't moved to LWJGL3 so it won't even affect them. And it is a beta, you can do stuff like that.

SilverTiger

I'm supporting both, option A and option B. Better do the changes now than doing it later when LWJGL3 moves out of its beta state.

spasi

The next nightly (3.0.0b #10) will have the above changes. New init code:

glfwMakeContextCurrent(window);
GL.createCapabilities(); // replaces GLContext.createFromCurrent()
debugProc = GLUtil.setupDebugMessageCallback(); // New class, similar to org.lwjgl.Util in LWJGL 2


The old GLContext class and platform implementations have been removed.

BrickFarmer

is the sonotype build pulling in stable build 9 or nightly build 14?  I was going to update my code for the above, but its not quite clear which build I'm using, seems not to have the changes.  The JNI_LIBRARY_NAME only tells me it's lwjgl-3.0.0b
Oculus Rift CV1, MBP 2016 - 2.9 i7 - Radeon Pro 460  OSX 10.12.4,  Win7 - i5 4670K - GTX1070.
Oculus Rift VR Experiments: https://github.com/WhiteHexagon

spasi


BrickFarmer

Thanks.  I just deleted the local cached repo version, and it has pulled in latest now.
Oculus Rift CV1, MBP 2016 - 2.9 i7 - Radeon Pro 460  OSX 10.12.4,  Win7 - i5 4670K - GTX1070.
Oculus Rift VR Experiments: https://github.com/WhiteHexagon