[RFE] LWJGL 3.0

[spasi]

The latest nightly (#26) has fixed GLFWcursorposfun and GLFWscrollfun. It also has the glfwSetCallback overloads in the Callbacks class.

in any event we should try make the LWJGL side as consistent as we can before hitting alpha.

Could you elaborate on what you mean exactly? Should we use the same rules across all APIs? For example:

CLEventCallback
GLDebugMessageCallback (was DEBUGPROC)
GLFWcursorposCallback (was GLFWcursorposfun)

[kappa]

The latest nightly (#26) has fixed GLFWcursorposfun and GLFWscrollfun. It also has the glfwSetCallback overloads in the Callbacks class.

Getting native crash instantly on startup on latest nightly build dump here.

[spasi]

You must be using old native binaries, jni_CallVoidMethodA shouldn't be there.

[kappa]

You must be using old native binaries, jni_CallVoidMethodA shouldn't be there.

Sorry my bad, you are correct, didn't update the natives, works correctly now. Thx for the fix.

The latest nightly (#26) has fixed GLFWcursorposfun and GLFWscrollfun. It also has the glfwSetCallback overloads in the Callbacks class.

in any event we should try make the LWJGL side as consistent as we can before hitting alpha.

Could you elaborate on what you mean exactly? Should we use the same rules across all APIs? For example:

CLEventCallback
GLDebugMessageCallback (was DEBUGPROC)
GLFWcursorposCallback (was GLFWcursorposfun)

Yup, i'd say we should use the same rules across the API for callbacks. i.e. they should all end with the word Callback.

Although it would be ideal to keep the native api names, but Callbacks are a special case, having this consistency will allow users to immediately know that its a Callback and they have to deal with it as such (i.e. need to release it when finished with it, etc).

In future there may be bindings to other native api's which might have their own naming scheme but would allow consistency to remain on the lwjgl/java side.

[spasi]

I think I'll also use camel case for GLFW callbacks, i.e. GLFWCursorPosCallback instead of GLFWcursorposCallback.

[kappa]

I think I'll also use camel case for GLFW callbacks, i.e. GLFWCursorPosCallback instead of GLFWcursorposCallback.

Yeh Camel case is probably better, can see why you'd not want to use it though (due to the GLFW acronym prefix being in caps).

Another point that comes to mind is that Callback style functionality in the Java API has traditionally ended with the word 'Listener' but guess in this case its better to keep using 'Callback' since there is also the manual clean up users need to keep in mind. Proper Java style Listeners can easily be added by users themselves on top of Callbacks.

[spasi]

OK, lets make a final decision. Need votes on:

CLEventCallback
GLDebugMessageCallback (currently DEBUGPROC)
GLFWCursorPosCallback (currently GLFWcursorposfun)

vs

EventCallback (currently CLEventCallback)
DebugMessageCallback (currently DEBUGPROC)
CursorPosCallback (currently GLFWcursorposfun)

Also, there's a special case for clEnqueueNativeKernel, the callback is called CLNativeKernel instead of CLNativeKernelCallback. I'd like to keep it that way, because callbacks are notifications that something (else) has happened, whereas native kernels are what's actually happening. It's a subtle difference, but I think adding a Callback postfix there would be weird.

[xsupermetroidx]

I prefer the first style, GLFWCursorPosCallback etc. Prefix and camel case.

I haven't been able to register for the forums using an outlook email account for days-- finally had to use a gmail account. In any case, I'm here to make a bug report.

Exception in thread "main" org.lwjgl.system.libffi.ClosureError: Callback failed because the closure instance has been garbage collected.
   at org.lwjgl.glfw.GLFW.glfwPollEvents(Native Method)
        ...

This error occurs at random (after some time has passed) when I press keys after using glfwSetKeyCallback. It seems that something is getting garbage collected that shouldn't. Perhaps this is related to the new ClosureRegistry that attempts to do automatic GC of closures?

It's possible that this affects other callbacks that are GCed at improper times as well, but I haven't tested them.

Also, it doesn't seem to be possible to use the system property "org.lwjgl.system.libffi.ClosureRegistry" to disable the automatic GC. I'm not sure if you intended that to be possible, but for it to work I'd have to set the entry to null, which System.setProperty doesn't allow.

Update: Switched to the latest nightly build and this error still occurs.

[shivshank]

This error occurs at random (after some time has passed) when I press keys after using glfwSetKeyCallback. It seems that something is getting garbage collected that shouldn't. Perhaps this is related to the new ClosureRegistry that attempts to do automatic GC of closures?

GLFWkeyCallback keyCallback = new GLFWkeyCallback() {
    // ...
};

Did you store a local reference, as above (keyCallback)? I haven't stared at the ClosureRegistry source (which also doesn't look like it's in use right now), but from the other GLFW/Callbacks source, it looks like callbacks are only freed by nglfwSetWindowPosCallback, which wouldn't store a reference to the Java Callback instance. Hence, the JVM has no way to know that you meant to keep it around, I think.

GLDebugMessageCallback (currently DEBUGPROC)

Camel case with prefix sounds good. Diverging from native APIs is okay, I think, as long as the naming is more helpful, more Java-like, and has the same meaning.

[xsupermetroidx]

Alright, I've tried your suggestion and it does indeed seem to solve the problem. However it's rather counterintuitive, since there is no situation in Java where one would typically need to store a reference to a listener/callback object to prevent it from being GCed. I see no reason why glfwSetKeyCallback couldn't store a reference to the callback internally, but the fact that it's a machine-generated file might complicate things. In any case, the documentation should state clearly for each callback function that keeping a reference around is necessary.

[kappa]

@spasi - although second style looks simpler i'd vote for the first style because:

1) Its immediately obvious which library the call belongs to.
2) Future library bindings with style one could end up with callbacks that should have the same name, style two's library name prefix avoids this problem.

[Cornix]

I vote for #1 as well.

[spasi]

However it's rather counterintuitive, since there is no situation in Java where one would typically need to store a reference to a listener/callback object to prevent it from being GCed. I see no reason why glfwSetKeyCallback couldn't store a reference to the callback internally, but the fact that it's a machine-generated file might complicate things. In any case, the documentation should state clearly for each callback function that keeping a reference around is necessary.

LWJGL callbacks are different from typical Java listeners in two ways:

a) They are registered with "native code". This means you're passing a function pointer to a native library, you do not pass a Java object reference. There is no target Java object that will store a reference to the LWJGL callback object.

b) There are "native resources" associated with them (specifically, libffi closures), that must be destroyed when the callback is no longer needed. Otherwise you'd be leaking memory.

There are a few solutions to the above problems, but all come with drawbacks: risk of JVM crashes when not used correctly, worse performance and/or GC implications, complicated implementation details the user needs to be aware of. In the end, we decided that the user should be responsible for storing references and doing manual cleanup. It's the simplest solution, easy to understand, and we also have protection against crashes with the ClosureError that lets users know they have done something wrong.

[spasi]

Also, it doesn't seem to be possible to use the system property "org.lwjgl.system.libffi.ClosureRegistry" to disable the automatic GC. I'm not sure if you intended that to be possible, but for it to work I'd have to set the entry to null, which System.setProperty doesn't allow.

The ClosureRegistry is experimental and not used by default. It's only active when the org.lwjgl.system.libffi.ClosureRegistry system property is set. Don't set the property and it will be ignored.

[advanced_set]

#1 Prefix + CamelCase