fix: patch for the main thread (awt | lwjgl3) - #2935
JNightRider wants to merge 3 commits into
Conversation
| settings.setResolution(framebufferWidth, framebufferHeight); | ||
| listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight); |
There was a problem hiding this comment.
This removes the call to SystemListener.reshape(int, int). That overload is deprecated but explicitly "kept only for backward compatibility", and LwjglWindow.updateSizes() still invokes both overloads. After this change an AWT-canvas app whose SystemListener/Application overrides only the 2-arg method silently stops receiving resize notifications when the canvas is resized, while the same app on the Display path still gets them. This is also outside the scope of the main-thread fix. Keep both calls unless the removal is deliberate and documented:
| settings.setResolution(framebufferWidth, framebufferHeight); | |
| listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight); | |
| settings.setResolution(framebufferWidth, framebufferHeight); | |
| listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight); | |
| listener.reshape(framebufferWidth, framebufferHeight); |
There was a problem hiding this comment.
When I started working on this PR, the new reshape() method called the old method (causing a duplicate call) but it seems this has been removed (in this PR #2881).
Was there really any difference between having the new method call the old one and making the call explicitly whenever necessary?
working on the changes
| data.majorVersion = 3; | ||
| data.minorVersion = 2; |
There was a problem hiding this comment.
The fallback renderer configuration changed from GL 2.0 to GL 3.2 here. It is unrelated to the main-thread fix (it only triggers for renderer strings missing from RENDER_CONFIGS) and it changes which context is requested on every platform. Bundling it makes the diff harder to reason about and to bisect; please split it into its own PR or state why the canvas path needs 3.2 as the fallback.
There was a problem hiding this comment.
Even though it is a minor change that doesn't actually affect anything (since the minimum supported version is already set to that value) should I create a separate PR for it?
jaime-jmebot
left a comment
There was a problem hiding this comment.
Reviewed jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java against base c40043a (the only source file that differs from the base).
- The core change is coherent: on macOS the loop is executed via
SwingUtilities.invokeLater()(parallel == false) so it runs on the AWT/main thread,removeNotify()no longer blocks the EDT waiting for the render thread, anddeinitInThread()is still reached whenneedCloseis set. - Three items need attention: a removed deprecated
reshape(int,int)callback,Toolkit.getDefaultToolkit().sync()dropped for the non-macOS (parallel) path, and an unrelated GL fallback version change — see inline comments. LwjglWindow,AwtPanelsContextandLwjglContextare unchanged, so the stated macOS-only scope otherwise holds.
jaime-jmebot
left a comment
There was a problem hiding this comment.
No new state since the previous review on this revision: the review request in the timeline (2026-09-17T16:52Z) precedes my submitted review, the head content of LwjglCanvas.java is unchanged (the 2-arg listener.reshape(...) is still missing in run() while LwjglWindow.updateSizes() still calls both overloads, Toolkit.getDefaultToolkit().sync() still sits in the !parallel branch outside the swap block, and the fallback render config still requests 3.2), and none of my three inline threads has an author reply. Nothing new to add; re-posting the same findings would be duplicate noise.
This PR fixes execution on macOS on the main thread; the
invokeLater()method is used to run the loop. This applies only to macOS, as it uses the main thread for rendering; other platforms are not affected.