[RFE] LWJGL 3.0

Started by spasi, November 08, 2012, 13:23:54

Previous topic - Next topic

Kai

My personal opinion on memory management is:
Have the user take care of it.

Let LWJGL just be an enabling technology that provides simple translation schemes to convert between concepts in the Java and the native world (functions/methods, enums/constants).

Now we have a scheme for callback functions and I think, what Ioannis suggested, sounds quite good.

Of course we have a leaky abstraction here, because memory management of native structures is exposed to the user.
Therefore, we need to make it very clear in the documentation.

How people then go about memory management could be different: Some users may want to integrate memory management of LWJGL objects with other things in their app by interfacing with GC and using WeakReferences and ReferenceQueues. Some other users may want to do it entirely different, again. We never know.

shivshank

I'm a novice programmer, so I don't fully grasp all of this, so sorry if I am not understanding. LWJGL's success is probably related to its ease of use, and opening up sometimes complicated memory management for the user seems like a bad idea.

Instead of a mostly-unified callback system, is there a way it could be *clearly* specified between each API? Like in org.lwjgl there would be a "OpenGLCallback" and "GLFWCallback" and so on?

QuoteCould a release() be automatically triggered internally on any previous handle if a null or new callback is passed to the method?
I'm not sure that is necessary. It'd be most important that things are consistent. All methods call .retain, I take it? But some can't call .release because every API has its own nuanced release system, right? Being mildly convenient in 10% of places just seems like more of an annoyance, and more of an unnecessary burden on maintenance.

As a secondary suggestion on layout, I think a display package is justifiable because most games don't have highly complex demands on input and windows (AFAIK). System packages seem like a good way to say "hey you, this is a little more advanced," so leaving GLFW there seems okay (assuming a display package is added).

If a display package is deemed out of scope, then none of that matters, though. The WindowCallback class seems like it'd suffice, if its not a heavy burden to maintain.

spasi

Quote from: shivshank on November 30, 2014, 04:30:23Instead of a mostly-unified callback system, is there a way it could be *clearly* specified between each API? Like in org.lwjgl there would be a "OpenGLCallback" and "GLFWCallback" and so on?

The implementation is unified, i.e. all callback types go through the same code (basically one C and one Java file). The only thing that changes is the function signature definition and the public interface the user sees in the API. So each callback type is a separate interface (e.g. DEBUGPROC, GLFWcursorposfun, CLNativeKernel), which is generated automatically.

Quote from: shivshank on November 30, 2014, 04:30:23As a secondary suggestion on layout, I think a display package is justifiable because most games don't have highly complex demands on input and windows (AFAIK). System packages seem like a good way to say "hey you, this is a little more advanced," so leaving GLFW there seems okay (assuming a display package is added).

Yes, GLFW shouldn't be in the system package. The original intent was to have a "display" package with a pure Java API for windows, input, etc. Then GLFW would simply be the back-end for that API. Back-ends would be pluggable and power-users could also use the back-end directly, in the "advanced/unsafe" system package.

The plan has changed now. Everyone seems happy with GLFW and you could say it's a noob-friendly API, there's no reason to "hide" it. I'm already planning to move it out of the system package and leave only unsafe and non-cross-platform APIs in there. The question I'd like feedback with is whether to put it at the top level (org.lwjgl.glfw) or under a "display" package (org.lwjgl.display.glfw, org.lwjgl.window.glfw, org.lwjgl.gui.glfw, etc).

kappa

Quote from: spasi on November 30, 2014, 10:42:56
The plan has changed now. Everyone seems happy with GLFW and you could say it's a noob-friendly API, there's no reason to "hide" it. I'm already planning to move it out of the system package and leave only unsafe and non-cross-platform APIs in there. The question I'd like feedback with is whether to put it at the top level (org.lwjgl.glfw) or under a "display" package (org.lwjgl.display.glfw, org.lwjgl.window.glfw, org.lwjgl.gui.glfw, etc).
Agreed, go for top level package "org.lwjgl.glfw".

Mainly because, it provides a lot more than just windows, a high resolution timer and access to the clipboard (previously in org.lwjgl.Sys) and also handles input of Keyboard, Mouse, Controller (previously in org.lwjgl.input.*) also it will highlight that its now the primary windowing system

The 'gui' package name would not be good as GLFW itself provides very little user interface (button, scroll pane, message box, etc) and best saved for an opengl gui library (unlikely to happen :) ) or maybe for a FreeType2 binding. The 'window' package name is confusing with the windows platform name used in other places.

spasi

Progress update: After several iterations, the new callback system is ready. I didn't want to rush it, since this will break everyone's code and I wanted to avoid having to break it more than once. This will happen in the next nightly build. For the same reason, the next build will also move GLFW to a top-level package. It should be ready tomorrow.

These are the changes since last time (I will use GLFWwindowposfun as an example again):

a) Before we had a GLFWwindowposfun interface and GLFWwindowposfun.Handle (abstract) class. This was ugly and had complications related to cleanup. Now we have a GLFWwindowposfun (abstract) class and a GLFWwindowposfun.SAM interface. You never, ever, have to deal with the SAM interface directly, it's only used for Java 8 lambdas (automatic SAM conversions, hence the name).

b) The bindings generator now associates a specific callback type with a specific class (GLFWwindowposfun with GLFW, DEBUGPROCARB with ARBDebugOutput, etc). The class now includes additional methods that construct the callback type, by delegating to the corresponding .SAM interface. This is useful for Java 8 and works in a very clean way using static imports. I got the idea from Kotlin constructors, which look like plain methods, without a "new" keyword". This will make more sense in the examples below.

c) IMPORTANT: The JNI global reference created for each callback, is now a weak reference. The intend is that users that forget to store the callback in their Java code, will very soon get an error (a ClosureError will be thrown with an appropriate message). This way we solve two problems: 1) LWJGL causes no memory leaks by default 2) We guarantee that the user is aware of the problem, even if they skip reading the documentation.

The weak dereference + null check has a minor performance impact (about 10%), but callbacks are almost never performance sensitive and are already fast enough. For example, sending a single character with DebugMessageInsert to an empty DEBUGPROC costs around 270 nanoseconds. That's pretty good for a Java -> JNI -> OpenGL driver -> libffi -> JNI -> Java chain imho.

d) There's a new, optional system property "org.lwjgl.system.libffi.ClosureRegistry". If set, any callback allocated will be automatically registered to a ClosureRegistry instance, retrieved from that system property using reflection. This can be used to do semi-automatic cleanup. This is still experimental, so won't say anything more. An implementation will be available at org.lwjgl.demo.util.ClosureGC.

Examples:

GLFWwindowposfun windowPosFun; // strong reference

// Java 6/7
glfwSetWindowPosCallback(window, windowPosFun = new GLFWwindowposfun() {
	@Override
	public void invoke(long window, int xpos, int ypos) {
		// ...
	}
});
// Java 8, lambda
glfwSetWindowPosCallback(window, windowPosFun = GLFWwindowposfun((win, xpos, ypos) -> {
	// ..
}));
// Java 8, method reference
glfwSetWindowPosCallback(window, windowPosFun = GLFWwindowposfun(this::windowPosEvent));

// ...after callback has been unregistered, or window has been destroyed:
windowPosFun.release();

Note that GLFWwindowposfun(...) in the Java 8 examples, is a simple static method, not a constructor. It is available automatically by doing an import static org.lwjgl.system.glfw.GLFW.*;

kappa

Sounds awesome, some really good changes there.

Also been busy with the compatibility layer, ran into some issues getting LibGDX running. LibGDX seems to run LWJGL in another thread, LWJGL3 doesn't seem to work well on OS X yet when running on a secondary thread, firstly the application was freezing, using -XstartOnFirstThread it gets further however now get an odd looking LWJGL window without a title bar, again suspect its due to LWJGL3 running on a different thread.

Anyway decided to give Minecraft a go and it seems to work well with minimal changes, should be useful for the people who have been requesting better unicode language/chat support:


A few issues:

Quarter screen size was expected due to HiDPI support being switched on by default in GLFW, don't think there is a runtime switch to turn it off yet without recompiling GLFW with a compiler switch (LWJGL2 didn't use HiDPI mode by default but did have a switch to turn it on).

Did run into one odd issue though, was getting this null pointer exception:

java.lang.NullPointerException: Initializing game
	at org.lwjgl.opengl.GL30.nglGenFramebuffers(GL30.java:2364)
	at org.lwjgl.opengl.ARBFramebufferObject.glGenFramebuffers(ARBFramebufferObject.java:668)
	at net.minecraft.client.renderer.OpenGlHelper.func_153165_e(OpenGlHelper.java:577)
	at net.minecraft.client.shader.Framebuffer.createFramebuffer(Framebuffer.java:100)
	at net.minecraft.client.shader.Framebuffer.createBindFramebuffer(Framebuffer.java:53)
	at net.minecraft.client.shader.Framebuffer.<init>(Framebuffer.java:34)
	at net.minecraft.client.Minecraft.startGame(Minecraft.java:538)
	at net.minecraft.client.Minecraft.run(Minecraft.java:949)
	at net.minecraft.client.main.Main.main(Main.java:161)
	at Start.main(Start.java:11)

This is odd because not sure why ARBFramebufferObject.glGenFramebuffers would be calling GL30.nglGenFramebuffers. GL3 doesn't work on OS X unless a forward context is requested hence the crash, shouldn't ARBFramebufferObject be using its own native implementation of this method?

One other thing LWJGL2's GL20.glShaderSource(int shader, ByteBuffer string) is no longer in LWJGL3, to wrap it would it be correct to use the following to obtain the same result?:
public static void glShaderSource(int shader, java.nio.ByteBuffer string) {
	org.lwjgl.opengl.GL20.glShaderSource(shader, 1, string, null);
}

ra4king

In regards to the ARBFramebufferObject.glGenFramebuffers calling the GL30 function, LWJGL 2 does the same thing if it detects that the function exists.
-Roi

spasi

Quote from: kappa on December 03, 2014, 01:22:59Also been busy with the compatibility layer, ran into some issues getting LibGDX running. LibGDX seems to run LWJGL in another thread, LWJGL3 doesn't seem to work well on OS X yet when running on a secondary thread, firstly the application was freezing, using -XstartOnFirstThread it gets further however now get an odd looking LWJGL window without a title bar, again suspect its due to LWJGL3 running on a different thread.

I'm about to give up on OS X + GLFW on any thread other than the first. It's just the way it is on OS X and I don't think there's anything we can do.

We could provide a different solution for OS X + AWT/JavaFX/etc. Something that isn't GLFW that is. Something hacky like what we had in LWJGL 2.

Quote from: kappa on December 03, 2014, 01:22:59Quarter screen size was expected due to HiDPI support being switched on by default in GLFW, don't think there is a runtime switch to turn it off yet without recompiling GLFW with a compiler switch (LWJGL2 didn't use HiDPI mode by default but did have a switch to turn it on).

Yes, it's a compile time flag and the nightly builds have it on. This is what elmindreda has to say on the matter (render to smaller FBO and upscale) and I think I agree.

Quote from: kappa on December 03, 2014, 01:22:59Did run into one odd issue though, was getting this null pointer exception:

java.lang.NullPointerException: Initializing game
	at org.lwjgl.opengl.GL30.nglGenFramebuffers(GL30.java:2364)
	at org.lwjgl.opengl.ARBFramebufferObject.glGenFramebuffers(ARBFramebufferObject.java:668)
	at net.minecraft.client.renderer.OpenGlHelper.func_153165_e(OpenGlHelper.java:577)
	at net.minecraft.client.shader.Framebuffer.createFramebuffer(Framebuffer.java:100)
	at net.minecraft.client.shader.Framebuffer.createBindFramebuffer(Framebuffer.java:53)
	at net.minecraft.client.shader.Framebuffer.<init>(Framebuffer.java:34)
	at net.minecraft.client.Minecraft.startGame(Minecraft.java:538)
	at net.minecraft.client.Minecraft.run(Minecraft.java:949)
	at net.minecraft.client.main.Main.main(Main.java:161)
	at Start.main(Start.java:11)

This is odd because not sure why ARBFramebufferObject.glGenFramebuffers would be calling GL30.nglGenFramebuffers. GL3 doesn't work on OS X unless a forward context is requested hence the crash, shouldn't ARBFramebufferObject be using its own native implementation of this method?

Thanks, this was a generator bug that has been fixed in the current nightly.

Quote from: kappa on December 03, 2014, 01:22:59One other thing LWJGL2's GL20.glShaderSource(int shader, ByteBuffer string) is no longer in LWJGL3, to wrap it would it be correct to use the following to obtain the same result?:
public static void glShaderSource(int shader, java.nio.ByteBuffer string) {
	org.lwjgl.opengl.GL20.glShaderSource(shader, 1, string, null);
}

I'm afraid not. ShaderSource expects an array of strings, i.e. an array of pointers to strings. You'll have to use a PointerBuffer or a ByteBuffer where your write the memAddress of your string buffer. I didn't add generator support for a single ByteBuffer or an array of ByteBuffers, because I thought it was confusing. Full example:

String source = "#version x.xx";
ByteBuffer string = memEncodeUTF8(source);

PointerBuffer strings = BufferUtils.createPointerBuffer(1);
IntBuffer lengths = BufferUtils.createIntBuffer(1);

strings.put(0, string);
lengths.put(0, source.length());
glShaderSource(shader, strings, lengths);

spasi

The latest nightly (#23) has the new callbacks and GLFW moved to org.lwjgl.glfw. I won't update the stable build yet.

spasi

Build #23 had a bug that caused crashes instead of throwing ClosureError. This has been fixed now, please download #24.

spasi

The latest build, #25, adds Java-friendly struct wrappers:

GLFWvidmode vm = new GLFWvidmode(glfwGetVideoMode(glfwGetPrimaryMonitor()));

int w = vm.getWidth();
int h = vm.getHeight();
int r = vm.getRefreshRate();

System.out.println(w + " x " + h + " @ " + r + "Hz");

Currently a ByteBuffer is wrapped, I will probably add support for direct pointer values (long), to avoid the extra allocation.

kappa

nice additions, the java-friendly structs makes code a lot cleaner.

Also the new Callback API are nice, here are some thoughts/feedback:

1) I think something might have broken in recent LWJGL night builds with the GLFWcursorposfun callback as the values returned are not correct (on OS X, using latest nightly build), getting constantly the value: "6.9532603094713E-310" on each mouse move for both xpos and ypos.

2) In terms of API naming and to make it a tiny bit more clearer/consistent, IMO renaming the GLFW*fun to GLFW*Callback might help readability a bit more since they are going to be provided to the glfwSet*Callback methods. Also when looking at the overall lwjgl glfw api and all the classes in it, IMO it would make it clearer to pick up that its a callback function/class, which needs to be provided to one of GLFW's callback methods.
e.g.
current
GLFWkeyfun keyfun = new GLFWkeyfun() {
	public void invoke(long window, int key, int scancode, int action, int mods) {
		// code
	}
};

glfwSetKeyCallback(windowHandle, keyfun);

vs
renamed
GLFWkeyCallback keyCallback = new GLFWkeyCallback() {
	public void invoke(long window, int key, int scancode, int action, int mods) {
		// code
	}
};

glfwSetKeyCallback(windowHandle, keyCallback);


3) In the API we also now have 1 method (glfwSet*Callback) and 1 classes/fun (GLFW*fun) for each glfw callback type, one has to first look up the method needed and then look up the corresponding class type, although they are logically named, I had to refer to the javadoc API almost 18 times to implement 9 different callback types. We could possibly eliminate the need for one look up for each type by just having a single method named glfwSetCallback(), that accepts all the different variations of GLFW*fun.

e.g.
glfwSetCallback(handle, new GLFWkeyfun());
glfwSetCallback(handle, new GLFWcharfun());
glfwSetCallback(handle, new GLFWwindowsizefun());
etc


Thus reducing the API and different names to remember/lookup.

Cornix

I fully agree with what kappa says.

spasi

Quote from: kappa on December 06, 2014, 08:09:201) I think something might have broken in recent LWJGL night builds with the GLFWcursorposfun callback as the values returned are not correct (on OS X, using latest nightly build), getting constantly the value: "6.9532603094713E-310" on each mouse move for both xpos and ypos.

Will look into it.

Quote from: kappa on December 06, 2014, 08:09:202) In terms of API naming and to make it a tiny bit more clearer/consistent, IMO renaming the GLFW*fun to GLFW*Callback might help readability a bit more since they are going to be provided to the glfwSet*Callback methods.

This is a fair request, but there is a counterargument.

On OpenCL, the callback function types are anonymous. For example:

cl_int clBuildProgram (
    cl_program program,
    cl_uint num_devices,
    const cl_device_id *device_list,
    const char *options,
    void (CL_CALLBACK *pfn_notify)(cl_program program, void *user_data),
    void *user_data
)

All callback arguments are called "pfn_notify" and that's it basically. So, with OpenCL, I chose arbitrary names (CLProgramCallback, CLEventCallback, CLNativeKernel, etc). They could be better, but I preferred in this case to be compatible with LWJGL 2 (not 100%, but mostly).

The situation is different with OpenGL and GLFW. In both APIs the callback types are explicitly named in the documentation/headers. For example:

// OpenGL
typedef void (APIENTRY *DEBUGPROC)(
    GLenum source,
    GLenum type,
    GLuint id,
    GLenum severity,
    GLsizei length,
    const GLchar* message,
    const void* userParam
);
void DebugMessageCallback(DEBUGPROC callback, const void* userParam);
// GLFW
typedef void (* GLFWkeyfun)(GLFWwindow*,int,int,int,int);
GLFWAPI GLFWkeyfun glfwSetKeyCallback(GLFWwindow* window, GLFWkeyfun cbfun);

Choosing a different name for the callback types would go against the general theme in LWJGL 3, that of perfectly matching the native signatures.

With that said, the generator already supports using different names (than the native type) for the Java classes, so I can make the change very easily. I personally don't like the "fun" postfix either.

Quote from: kappa on December 06, 2014, 08:09:203) In the API we also now have 1 method (glfwSet*Callback) and 1 classes/fun (GLFW*fun) for each glfw callback type, one has to first look up the method needed and then look up the corresponding class type, although they are logically named, I had to refer to the javadoc API almost 18 times to implement 9 different callback types. We could possibly eliminate the need for one look up for each type by just having a single method named glfwSetCallback(), that accepts all the different variations of GLFW*fun.

e.g.
glfwSetCallback(handle, new GLFWkeyfun());
glfwSetCallback(handle, new GLFWcharfun());
glfwSetCallback(handle, new GLFWwindowsizefun());
etc


Thus reducing the API and different names to remember/lookup.

This is a good idea. Is it OK if these methods are added to the org.lwjgl.glfw.Callbacks class? I'd like to avoid org.lwjgl.glfw.GLFW because it's auto-generated and this is a very special case that won't be applicable to any other binding.

kappa

Quote from: spasi on December 06, 2014, 09:27:06

This is a fair request, but there is a counterargument.

On OpenCL, the callback function types are anonymous.
...
All callback arguments are called "pfn_notify" and that's it basically. So, with OpenCL, I chose arbitrary names (CLProgramCallback, CLEventCallback, CLNativeKernel, etc). They could be better, but I preferred in this case to be compatible with LWJGL 2 (not 100%, but mostly).

The situation is different with OpenGL and GLFW. In both APIs the callback types are explicitly named in the documentation/headers.
...
With that said, the generator already supports using different names (than the native type) for the Java classes, so I can make the change very easily. I personally don't like the "fun" postfix either.
At this stage the api can be changed without much problems as people porting LWJGL2 apps will have to make big changes anyway, in any event we should try make the LWJGL side as consistent as we can before hitting alpha.

Quote from: spasi on December 06, 2014, 09:27:06
This is a good idea. Is it OK if these methods are added to the org.lwjgl.glfw.Callbacks class? I'd like to avoid org.lwjgl.glfw.GLFW because it's auto-generated and this is a very special case that won't be applicable to any other binding.
Yeh, that sounds fine.