Skip to content

fix: patch for the main thread (awt | lwjgl3) - #2935

Open
JNightRider wants to merge 3 commits into
jMonkeyEngine:masterfrom
JNightRider:fix/synchronization-awt
Open

JNightRider wants to merge 3 commits into
jMonkeyEngine:masterfrom
JNightRider:fix/synchronization-awt

Conversation

@JNightRider

Copy link
Copy Markdown
Contributor

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.

@riccardobl
riccardobl requested review from jaime-jmebot and removed request for jaime-jmebot September 16, 2026 17:07
Comment on lines 532 to 533
settings.setResolution(framebufferWidth, framebufferHeight);
listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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:

Suggested change
settings.setResolution(framebufferWidth, framebufferHeight);
listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight);
settings.setResolution(framebufferWidth, framebufferHeight);
listener.reshape(framebufferWidth, framebufferHeight, framebufferWidth, framebufferHeight);
listener.reshape(framebufferWidth, framebufferHeight);

@JNightRider JNightRider Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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

Comment thread jme3-lwjgl3/src/main/java/com/jme3/system/lwjgl/LwjglCanvas.java
Comment on lines +741 to +742
data.majorVersion = 3;
data.minorVersion = 2;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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 jaime-jmebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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, and deinitInThread() is still reached when needClose is 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, AwtPanelsContext and LwjglContext are unchanged, so the stated macOS-only scope otherwise holds.

@jaime-jmebot jaime-jmebot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

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