LWJGL 3 - API Freeze (request for feedback)

Started by spasi, September 29, 2015, 18:50:47

Previous topic - Next topic

spasi

The last planned API refactoring for LWJGL 3.0 has just been completed. It's a major overhaul of structs support and you can try it out with nightly build 3.0.0b #39. I will describe the changes in detail in a following post.

This topic is a request for feedback on the general LWJGL 3 API. The API will officially freeze with the 3.0 release, but I would like the upcoming beta release to be very very close to the final API. I plan to more actively promote LWJGL 3 ahead of the official release and that can only happen if any code/tutorials/videos posted don't become obsolete until then. So, if you have anything to contribute, please speak now:

- Is there anything in the current API hard to use?
- Is there anything in the current API hard to understand?
- Is there anything in the current API ugly or unconventional?
- Is there an implementation detail that you want exposed?
- Is there something complex that you want simplified?
- Do you need more options/configuration?
- Do you frequently get bugs or crashes when using a particular binding or API?

I would prefer any requests or suggestions to be accompanied by example code. Harsh (but constructive) criticism is welcome, you won't hurt my feelings! :)

If this topic stays idle for more than a few days, with a minimum of a week, I will release the 3.0 beta.

Kai

There was once a thread about refactoring the GLContext/GLCapabilities classes. Back then I was fine with things turned out to be, but now I somehow am uncomfortable with the way to make the GL context current in different threads.
Provided, there is a way to make the native GL context current in a calling thread, be it glfwMakeContextCurrent or SWT GLCanvas.setCurrent, or manually invoking wglMakeCurrent..., there does not seem to be a proper solution (or I am missing it) to tell LWJGL that the context owner thread has changed.
A call to GL.createCapabilities() seems not well named for that, and it also crashes with me when I make the context current in thread 'main' (which works) which happens-before switching to another spawned thread 'B' (which crashes). There, the call fails with a NPE in org.lwjgl.system.Checks.checkPointer(Checks.java:80).
So, am I probably missing a simple GLContext.makeCurrent() ?

spasi

The latest nightly build includes the following changes to structs:

- Methods in bindings that previously used ByteBuffers as pointers to struct values, now use the corresponding struct class. This improves usability and type-safety.
- Methods in bindings that previously used ByteBuffers as pointers to struct arrays, now use a typed subclass of the new org.lwjgl.system.StructBuffer class. For example glfwGetVideoModes now returns an instance of GLFWvidmode.Buffer, where Buffer extends StructBuffer. The StructBuffer API is similar to NIO buffers and PointerBuffer, with the difference that each element is a struct instead of a primitive value. This improves usability and type-safety.
- The struct classes themselves wrap a memory address directly, instead of wrapping a ByteBuffer instance. This reduces overhead. A struct class can still share memory with a ByteBuffer, but it is optional.
- The static API still exists, but now better matches the LWJGL bindings. There are methods that can be used to get/set struct values on a ByteBuffer and there are unsafe versions (with the 'n' prefix) that work with pointer addresses. This offers maximum flexibility, while keeping auto-complete lists clean.
- There are 3 ways to allocate a struct instance or buffer: via malloc, calloc or BufferUtils. You make the choice with static factory methods. If malloc/calloc are used, then you're responsible for freeing the corresponding allocation. This improves performance and allows allocation without zeroing out the struct.
- There is a clear() method that explicitly zeroes out a struct. This is often useful.

Note that a StructBuffer currently wraps a ByteBuffer, which has some overhead: a second instance required and some division operations in methods. This is an implementation detail and can change in the future.

spasi

Quote from: Kai on September 29, 2015, 19:02:00There was once a thread about refactoring the GLContext/GLCapabilities classes. Back then I was fine with things turned out to be, but now I somehow am uncomfortable with the way to make the GL context current in different threads.
Provided, there is a way to make the native GL context current in a calling thread, be it glfwMakeContextCurrent or SWT GLCanvas.setCurrent, or manually invoking wglMakeCurrent..., there does not seem to be a proper solution (or I am missing it) to tell LWJGL that the context owner thread has changed.
A call to GL.createCapabilities() seems not well named for that, and it also crashes with me when I make the context current in thread 'main' (which works) which happens-before switching to another spawned thread 'B' (which crashes). There, the call fails with a NPE in org.lwjgl.system.Checks.checkPointer(Checks.java:80).
So, am I probably missing a simple GLContext.makeCurrent() ?

To anyone else reading: this is exactly what I'm looking for in this topic. Would have been perfect with some example code that reproduces the crash(es).

To Kai: You're probably missing a call to GL.setCapabilities(GLCapabilities). But I would like to see some code to make sure. If this works for you, let me know if you have any suggestion on how to make this more discoverable. A method on GLCapabilities perhaps?

Kai

Hm, it might be that I am doing something totally wrong, but I just changed the "Multithreaded" demo in lwjgl3-demos repository to this:

In "main" thread right at the beginning in winProcLoop()
glfwMakeContextCurrent(window);
		caps = GL.createCapabilities();
		debugProc = GLUtil.setupDebugMessageCallback();
		glClearColor(0.3f, 0.5f, 0.7f, 0.0f);


This above happens-before the things now in a spawned "render" thread, right at the beginning of renderLoop():
glfwMakeContextCurrent(window);
		GL.setCapabilities(caps);
		glClearColor(0.3f, 0.5f, 0.7f, 0.0f);


Now, with this there is no more NPE or any other exception, but the render is complete black, whereas before it was blueish with a white spinning quad.

EDIT:

I committed my changes with: https://github.com/LWJGL/lwjgl3-demos/commit/a7fc7eeeb510f0741b8f8bc422e492dbc0fff617

Before the commit, everything worked. After the commit: black window with me.

spasi

I see now. Indeed the previous code was working fine and I'm now getting a black screen too. The problem is that you're making the context current in the secondary thread, while it is still current in the main thread. You can fix the current code by calling:

glfwMakeContextCurrent(NULL);

in the main thread, right before spawning the secondary thread.

edit: as a summary, to "transfer" a context to another thread:

- Make context current in thread A
- caps = GL.createCapabilities() in thread A
- Clear current context in thread A
- Make context current in thread B
- GL.setCapabilities(caps) in thread B

Note that calling GL.createCapabilities() again in thread B is legal, but setCapabilities() is more efficient (creating the GLCapabilities instance can take a few hundred milliseconds).

Kai

Ohh.. thanks! That's it. Did not know that one actually has to manually release the context from one thread before making it current in another one.
I think the solution with GL.createCapabilities(), storing it, and making it current in LWJGL's thread local with GL.setCapabilities() is the best thing I can think of right now, concerning clean/convenient API, so no request for change. :)

spasi

Nightly build 3.0.0b #40 includes two more changes:

- Structs that are used in an output-only/read-only manner now come without setter methods. This results in a cleaner API, a lot of methods that were never going to be used are now gone. Let me know if I missed a read-only struct or if I marked a read/write struct as read-only.

- The StructBuffer implementations now come with getters/setters that work on the struct value at the current buffer position. This means you can use struct buffers as flyweights. For example, the two loops below are equivalent:

// WARNING: OBSOLETE CODE
IntBuffer count = memAllocInt(1);
GLFWvidmode.Buffer modes = glfwGetVideoModes(glfwGetPrimaryMonitor(), count);

// element access
for ( int i = 0; i < count.get(0); i++ ) {
	GLFWvidmode mode = modes.get(i);
	System.out.println(mode.getWidth() + " x " + mode.getHeight());
}

// flyweight access
for ( int i = 0; i < count.get(0); i++ ) {
	modes.position(i);
	System.out.println(modes.getWidth() + " x " + modes.getHeight());
}

memFree(count);


edit: Build 3.0.0b #41 contains a fix that hides parameters such count in glfwGetVideoModes, so the above code becomes:

GLFWvidmode.Buffer modes = glfwGetVideoModes(glfwGetPrimaryMonitor());

for ( int i = 0; i < modes.capacity(); i++ ) {
	GLFWvidmode mode = modes.get(i);
	System.out.println(mode.getWidth() + " x " + mode.getHeight());
}

for ( int i = 0; i < modes.capacity(); i++ ) {
	modes.position(i);
	System.out.println(modes.getWidth() + " x " + modes.getHeight());
}

spasi

The GLFW structs somehow ended up with their original names all this time. Should I change them to the Java naming convention? That is:

GLFWimage -> GLFWImage
GLFWvidmode -> GLFWVidmode (or GLFWVidMode?)
GLFWgammaramp -> GLFWGammaramp (or GLFWGammaRamp?)

SHC

I suggest not to rename them, for the same reason we chose to not remove the suffixes of OpenGL functions.

spasi

Quote from: SHC on September 30, 2015, 13:40:33I suggest not to rename them, for the same reason we chose to not remove the suffixes of OpenGL functions.

The same could be said about callbacks though. Callbacks were renamed last December, to be consistent across APIs. Is anyone in favor of reverting their names to match the original C API? Examples:

GLFWErrorCallback -> GLFWerrorfun
GLFWWindowSizeCallback -> GLFWwindowsizefun
GLDebugMessageCallback -> DEBUGPROC

Cornix

I would go with the java convention since it is easier to read in my opinion.
With todays IDE's a simple lowercase/uppercase mistake should take only the press of a button to fix.

Kai

I too think that going with the Java naming conventions is a little bit easier to read:
- GLFWGammaRamp compared to GLFWgammaramp
- GLFWVidMode compared to GLFWvidmode
And it would also allow for the "initials typing scheme" in Eclipse when searching types by name, such as GLFWGR or GLFWVM if that's worth anything for anyone. :)

kappa

Quote from: spasi on September 30, 2015, 14:10:09
Is anyone in favor of reverting their names to match the original C API? Examples:

GLFWErrorCallback -> GLFWerrorfun
GLFWWindowSizeCallback -> GLFWwindowsizefun
GLDebugMessageCallback -> DEBUGPROC
I'd stay with what we have now in this case too, the original C API versions are really ugly (at least from a Java programmer point of view), plus the consistency across different bindings is nice.

To stay consistent we should therefore also change the GLFW structs to the Java naming conventions.

spasi

Quote from: Kai on September 30, 2015, 14:40:35And it would also allow for the "initials typing scheme" in Eclipse when searching types by name, such as GLFWGR or GLFWVM if that's worth anything for anyone. :)

That's a very good point, IntelliJ has this too and I use it all the time.