Fix waitFor not being implemented in lwjgl3 - #1048

Not sure why the compare is not working correctly, only a couple lines changed...

Probably you checked the file in with a different line ending. You may want to fix it as it's harder to see what actually changed.

Ah, I did not think about that. I'll fix it right now

FYI: https://github.blog/2018-05-01-ignore-white-space-in-code-review/
As pointed out below:
https://github.com/jMonkeyEngine/jmonkeyengine/pull/1048/files?w=1 also works

Note though that this still means we don't want "unclean" commits, as git tools might not ignore whitespaces and it's not clear which line ending/white space is the correct one.

Actually we should have linters/auto formatters as bots which comment on the PRs :)

Or I sort of remember that it's possible to set line ending style on the repo or something... but that may make a whole bunch of source change because I don't believe it's consistent right now.

OK I have line endings fixed.

I would also like to note, that with LWJGL3, the headless context does return from start, which makes inconsistent behavior within the library itself.

OK, I just made a new commit. If waitFor is false, we run it on the main thread, otherwise, create a new thread. How about this solution?

I do not have an apple computer, perhaps someone can test this?

Just for documentation purpose, it was issue #433 that has the os x workaround info in it.

I have tested this proposed fix on my apps in windows, and I think it is ready but still needs tested in OS X. Any thoughts?

Thanks,
Trevor

Perhaps someone at the Forum would be willing to help us.

What exactly needs to be tested here? Is there some sort of test case to run?

Just needs to be tested in a working application. Behavior should not change on OS X and that needs to be confirmed.

Sorry for not reacting on this so long, I've just re-encountered this without the PR:
Currently, start() blocks until the application is closed, where it should:

• Return instantly in the case of start(false)
• Return as soon as the context is initialized in start(true).

I think your PR addresses the first point, but not the second one? [Which would at least be one improvement]

Edit: Forgot to say: the current behavior is required on Mac OS X regardless of the waitFor, since start() may NOT return, since we have to run on that thread.

Edit2: Here's a read up on why things are like they are: LWJGL/lwjgl3#311
I'll have to give it a read too, but it seems to be related to GlfwWindow, so maybe we can rework lwjgl3 so it just creates the render thread after the window is created (which would be the true waitFor=true behavior anyway)

Edit3: It is a problem. Most GLFW functions are required to be called from the main thread, which also has to be the first thread on macOS (see -XstartOnFirstThread). The javadoc of each function mentions if it's callable from any thread or only from main. This is neither an LWJGL nor a GLFW limitation; it's the only way to write a usable cross-platform windowing system. Your application may work on Windows, but will certainly fail on macOS. Note that this applies to GLFW functions only; OpenGL contexts can be made current in other threads and those threads can be used for rendering.

So a separate rendering thread is possible, but then all calls like resizing the window will fail as the main thread aka. first thread has already exited...

Edit 4: https://www.glfw.org/docs/latest/group__input.html#ga1caf18159767e761185e49a3be019f8d
I think we can't merge this PR as is, as in the current form, not even the keyboard input can be run off-thread, so it would need to happen before creating the render thread etc.

Okay, so after Review in #1248, I think this PR cannot work like this, it needs a more complicated approach, which I didn't manage to do.

The thing is: the GLFW docs say you must not access these functions (most of initInThread) from outside the main thread. They say it's because some platforms require that. I don't know if that will lead to exceptions or work on all those other platforms (i.e. not working on mac os soley), but it's discouraged by the docs.

I think we have to accept that behavior, because everything else gives additional problems, starting with context restarts and a proper app shutdown.

For now I'd recommend discussing on #1248 about our options.

@MeFisto94 thank you for looking into this. I am not sure why the documentation is so unclear on LWJGL's side on this, but I have been using a different thread for quite some time now in my applications be using this approached:

                Thread t = new Thread() {
                    @Override
                    public void run() {
                        jme.start();
                    }
                };
                t.start();

This was taken right from a working application. I have only tested it in windows though. Using JME3 LWJGL3.

Perhaps this only works in Windows...?

why the documentation is so unclear on LWJGL's side on this

Because it's a limitation of glfw and their Docs state this clearly.

Perhaps this only works in Windows...?

And Linux, but it seems to just crash on Mac OS. So while it's forbidden you can get away with that on most platforms.
But as you said in your comment: having start() blocking isn't that bad.

I'm confused about the status of this PR. Does it need code changes? Testing? Or just integration?

From what I've gathered, there isn't a good way to maintain the previous functionality with lwjgl3 on all platforms. Creating a new thread will crash on OSX. So this isn't really a simple fix, because we can't match the lwjgl2 functionality.

What Jeddic says, we're violating the docs/contract when creating a new thread. I tried a different way but failed.

It's a problem: Currently one cannot use start(false) and thus the thread calling into app.start() is locked forever, which is bad for Editors/SDKs. With this PR, that would work on Windows and Linux but crash the JVM on Mac OS X.

I don't know if some (if Mac OS waitFor == true) would work, but we might need to find a solution to the glfw problems anyway

That is what this PR does. It keeps Mac os on main thread, and forks for everything else.

EDIT: @MeFisto94 I did not look at the username to realize I was replying to you. I know you know what this PR is. Sorry about that.