LwjglCanvas backend improvement (LWJGLX + AWT)#2669
LwjglCanvas backend improvement (LWJGLX + AWT)#2669JNightRider wants to merge 14 commits intojMonkeyEngine:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the LwjglCanvas implementation to improve OpenGL context management and lifecycle handling within AWT/Swing environments. Key changes include a new main loop in the run() method, the introduction of explicit lock/unlock and context creation methods, and logic to handle Wayland/X11 platform detection. Several methods and fields in LwjglWindow were changed to protected to facilitate this. The review feedback identifies several critical issues: the renderable flag is never reset after context recreation, which would stop the display; blocking the AWT Event Dispatch Thread (EDT) with lock.wait() poses a high risk of deadlock; and skipping the superclass call in createContext() bypasses glfwInit(), potentially breaking GLFW-dependent functionality. Other feedback points out potential NullPointerExceptions, redundant flag assignments, missing GLFW event polling, and the use of System.out.println instead of a logger.
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Outdated
Show resolved
Hide resolved
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Outdated
Show resolved
Hide resolved
|
Thanks! Feel free to go through gemini's suggestions or to close them if they are wrong. |
|
This pull request should be ready by now... I've reviewed the AI assistant's reviews and taken some of your feedback into account. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the LWJGLX + AWT (LwjglCanvas) backend to improve canvas remove/re-add behavior and runtime performance, including changes for Wayland/XWayland (GLX) handling and OpenCL sharing setup.
Changes:
- Refactors
LwjglCanvasto manage context creation/destruction and swapping more explicitly, including reinit/invalidation when the canvas is removed and re-added. - Extends OpenCL initialization to support custom GL sharing properties (needed for LWJGLX contexts).
- Adds Wayland session detection and X11 display access helpers; loosens some
LwjglWindowmembers for subclass customization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
jme3-lwjgl3/src/main/java/com/jme3/system/lwjglx/X11GLPlatform.java |
Adjusts Linux (X11/GLX) platform handling, adds X11 display access and explicit GLX context destruction. |
jme3-lwjgl3/src/main/java/com/jme3/system/lwjglx/LwjglxDefaultGLPlatform.java |
Adds Wayland detection helper and exposes X11 Display* retrieval for LWJGLX canvases; enables FREEBSD mapping to X11 platform. |
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglWindow.java |
Makes select members/methods protected for LwjglCanvas overrides; updates OpenCL init call signature. |
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglContext.java |
Adds a sharing callback to OpenCL context creation so non-GLFW contexts can provide interop properties. |
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java |
Major restructuring of the AWT rendering loop and context lifecycle; adds Wayland/XWayland GLX forcing and OpenCL interop setup for LWJGLX contexts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Outdated
Show resolved
Hide resolved
|
I believe the relevant corrections have been incorporated... |
I have recently been working on the problem that LwjglCanvas is currently presenting:
These errors have now been corrected; the problem when deleting the canvas was that the renderer was not invalidated when creating the new one.
The performance problem was due to the thread remaining idle for 10 milliseconds and glwf creating a context, which meant that two contexts were running.
Screenshots
Some screenshots, whether running natively or with AWT, show that JME3 looks exactly the same (the window on the left is from the native version, the one on the right is from the AWT version).
Linux (Wayland + XWayland)
Windows
While removing and re-adding the canvas works perfectly, using a LightProbe generated by
FastLightProbeFactory.makeProbe(...)can cause program crashes, and the new shadow filterSdsmDirectionalLightShadowFiltermight not work (when removing and re-adding the canvas). These filters must be removed before deleting the canvas from its main components and then re-adding it when inserting it.It is not currently compatible withOpenCL(), as the lwjgl3 module is tightly linked to glwf.