Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

performance: unnecessary window.makeCurrent() in LWJGL3 backend #7360

Open
3 of 6 tasks
McMurat opened this issue Mar 8, 2024 · 4 comments
Open
3 of 6 tasks

performance: unnecessary window.makeCurrent() in LWJGL3 backend #7360

McMurat opened this issue Mar 8, 2024 · 4 comments

Comments

@McMurat
Copy link

McMurat commented Mar 8, 2024

Please ensure you have given all the following requested information in your report.

Issue details

In the main loop of a Lwjgl3Application there is a loop which iterates over all Lwjgl3Windows and calls window.makeCurrent() which triggers an OpenGL context switch using glfwMakeContextCurrent(). This is unnecessary to call repeatedly when there is only one single window. Optimizing this will cause my application to run around 10% faster.

I suggest a simple change:

                if (currentWindow != window) {
                    window.makeCurrent();
                    currentWindow = window;
                }

This is sufficient because currentWindow is null initially. So glfwMakeContextCurrent() will be called at least once.

Please select the affected platforms

  • Android
  • iOS
  • HTML/GWT
  • Windows
  • Linux
  • macOS
@crykn
Copy link
Member

crykn commented Mar 8, 2024

Good catch! I think currentWindow would need to be set here as well, so that the right context is active in the first render pass after a second window is created. Do you want to submit a PR for this?

@McMurat
Copy link
Author

McMurat commented Mar 8, 2024

Good catch! I think currentWindow would need to be set here as well, so that the right context is active in the first render pass after a second window is created. Do you want to submit a PR for this?

Yes, setting currentWindow to the created window and also calling makeCurrent() on the it here is also necessary for multi-window support. The additional call to makeCurrent() is only needed to update Gdx.graphics, Gdx.input, Gdx.gl, ... for the new active window (currently this is not happening ant therefore a bug in the current implementation).

Edit: Alternatively, the context switch in createGlfwWindow could be reverted to the previous context at the end of the createWindow method which calls glClearColor, glClear and glfwSwapBuffers for the created window. In this case we would not need to update the currentWindow and also do not need to invoke makeCurrent(). I think this is a design question. Getting the current context is a simple call to glfwGetCurrentContext.

I don't have a fork. As these are just a few lines it would probably be easier if someone else pushes/PRs this.

@ryuukk
Copy link

ryuukk commented Mar 8, 2024

you can edit file and submit a PR automatically using the github interface, go on the file, then click edit, it'll do the work for you

@McMurat
Copy link
Author

McMurat commented Mar 10, 2024

you can edit file and submit a PR automatically using the github interface, go on the file, then click edit, it'll do the work for you

I opened a PR: #7362

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants