Skip to content

Conversation

@tonihele
Copy link
Contributor

Fixes the input issue mentioned in issue #1013 . It of course happens on LWJGL3 universally. Not a Nifty thing.

The inputs are very much tied to the window context.

@tonihele
Copy link
Contributor Author

Anything needed for this? Clarifications, better coding..? :)

@stephengold
Copy link
Member

As far as I'm concerned, all this needs is a reviewer, someone (other than you) who understands LWJGL.

@stephengold stephengold added this to the LWJGLv3 Issues milestone Feb 13, 2020
Copy link
Member

@MeFisto94 MeFisto94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically looks good, a few small things to adjust here and there, let's see.

Hint for all: Use the "Split View" and not the unified view, that way it's better to see where the code moved from and to.

}
initCallbacks();

initialized = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't resetted when resetting the context.
Do you know about the special purpose of it?
Like I wonder if we should reset this value, like when initCallback fails and thus initialized is false after a restart.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use the flag to see if I need to re-create the callbacks. I'm unsure what the flag otherwise symbolizes, it was there already.

Comment on lines +181 to +187
if (listener != null) {
sendFirstMouseEvent();
}

setCursorVisible(cursorVisible);
logger.fine("Mouse created.");
initialized = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed when restarting?
I mean setCursorVisible probably isn't, as for that you don't have to restart the context.

What about sendFirstMouseEvent?
The initialized question from above also applies here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this all just mimics LWJGL 2. It doesn't seem to send either after restart.

Comment on lines +301 to +305

// Create OpenCL
if (settings.isOpenCLSupport()) {
initOpenCL(window);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason why this should now only happen when restarting and not as part of the general init process? Wouldn't this prevent openCL init when regularly starting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is part of the general init process. This is called in createContext which in turn is used by the restart and the initial context.


destroyContext();
super.internalDestroy();
glfwTerminate();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just wonder here if we can't/shouldn't arrange it that the restart could call deinitInThread() and initInThread instead of manually re-creating the context.

I think all that would be required is moving the glfwTerminate post deinitInThread, right?
I don't know if that works by the API.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I tried it just now. Seems to actually fix #1013 . But also causes some side effects. Cursor is somehow gone as well as some other elements and all inputs... Need to investigate this a bit more...

@stephengold stephengold merged commit b93ea18 into jMonkeyEngine:master Feb 26, 2020
@tonihele tonihele deleted the lwjgl3-restart-input-handle branch March 20, 2020 09:37
pspeed42 pushed a commit that referenced this pull request Mar 24, 2020
* Reinit inputs on context restart

* Added test issue from issue #1013

* Verify that the inputs are already initialized
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants