feat: lwjglx-awt library update (lwjgl3) - v0.2.5 - #2980
JNightRider wants to merge 6 commits into
Conversation
jaime-jmebot
left a comment
There was a problem hiding this comment.
Nice work — the platform selection now mirrors the library logic (Wayland + no X11 preference → WaylandGLPlatform, which extends the EGL canvas from the library), and the video-driver string is nicely delegated to the platform classes.
- The GLX/EGL locking in the render loop (
canvas.lock()arounddeleteContext()/dispose()) looks correct and needed for the EGL path. - Two small notes in the
TestCanvasexample about the defaultplatformX11value and the now-stale comment. - FYI: on the EGL path
destroy()is a no-op, so the EGL display is only released viadeleteContext()/dispose()on re-init. That's harmless at app shutdown, just something to keep in mind if an app restarts canvases repeatedly.
jaime-jmebot
left a comment
There was a problem hiding this comment.
Both notes are addressed — the example now starts on the default (EGL/XWayland) path and the stale comment is reworded. Nothing blocking left from me.
- Platform selection and driver-string delegation look good.
- GLX/EGL locking in the render loop is correct for the new path.
- Heads-up for the final PR description: it's still in draft, and it would help to note that on Wayland sessions X11 is still used for window management only, so folks don't expect X11 to disappear.
jaime-jmebot
left a comment
There was a problem hiding this comment.
Thanks for the fixes — both notes are addressed and the platform/driver-string delegation reads nicely.
- One last housekeeping item:
TestCanvas.javagot converted from CRLF to LF, which turns the whole file into a diff. Re-saving with its original line endings will keep the change readable. - Otherwise the code looks good to me. Not blocking.
23e0fc1 to
39161d2
Compare
jaime-jmebot
left a comment
There was a problem hiding this comment.
Checked the CRLF re-save — the full-file rewrite is gone and the diff is down to your actual changes.
- Only remaining nit: the added lines are still LF while the rest of the file is CRLF, so the file has mixed endings (the whitespace-only line before
setX11PlatformPreferredstill shows up as changed). Re-saving with CRLF throughout clears it up. Purely cosmetic, not blocking. - Everything else looks good to me: the platform/driver-string delegation and the GLX/EGL locking in the render loop.
jaime-jmebot
left a comment
There was a problem hiding this comment.
Re-checked after the latest commit — the diff is down to the real changes and everything I raised earlier is addressed.
- Platform selection (
WaylandGLPlatform→ EGL canvas when Wayland + no X11 preference) and the video-driver delegation to the platform classes read nicely. - The GLX/EGL locking around
makeCurrent()and in the render loop is correct for the new EGL path. - One cosmetic leftover in
TestCanvas.java: a single added line still has an LF ending while the rest of the block is CRLF (thread above has the details).
This PR updates the lwjglx-awt library, a version published by jme3 is currently being used (this occurred due to issues with releasing a new version from the main repository); however, since this is no longer an issue, the library is being updated to the latest version from the main repository.
This brings new changes: there is now support for EGL contexts in XWayland (this does not mean X11 is not used; it remains present for window management).
Additionally, this PR would resolve issue #2602 (although the problem has already been partially resolved by PR #2645, it remains open; therefore, I suggest closing it upon merging this PR.)
NOTE: A small test was added to the TestCanvas class to verify the preferred platform change on Linux.