GraalVM support#2740
Conversation
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
|
🖼️ Screenshot tests have failed. The purpose of these tests is to ensure that changes introduced in this PR don't break visual features. They are visual unit tests. 📄 Where to find the report:
✅ If you did mean to change things: ✨ If you are creating entirely new tests: Note; it is very important that the committed reference images are created on the build pipeline, locally created images are not reliable. Similarly tests will fail locally but you can look at the report to check they are "visually similar". See https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-screenshot-tests/README.md for more information Contact @richardTingle (aka richtea) for guidance if required |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant updates to the Android support, including a migration to AndroidX Fragments, improved GraalVM native-image support, and enhancements to the renderer's handling of sRGB and mipmap generation. I have reviewed the changes and provided feedback regarding the abrupt process termination in the error dialog and a potential logic issue in the buffer update loop.
| .setNegativeButton("Kill", (d, w) -> { | ||
| android.os.Process.killProcess(android.os.Process.myPid()); | ||
| }) |
There was a problem hiding this comment.
Using android.os.Process.killProcess() can be quite abrupt and might prevent cleanup logic from running. While this is an error dialog, it might be better to offer a less drastic 'Kill' option. For instance, you could just call requireActivity().finishAffinity() to close all activities in the task. However, for a debug build, providing a hard kill switch can be useful for developers to stop a completely hung application.
| while ((reg = it.next()) != null) { | ||
| gl3.glBindBuffer(type, bufferId); | ||
| gl.glBindBuffer(type, bufferId); | ||
| if (reg.isFullBufferRegion()) { | ||
| ByteBuffer bbf = bo.getData(); | ||
| if (logger.isLoggable(java.util.logging.Level.FINER)) { | ||
| logger.log(java.util.logging.Level.FINER, "Update full buffer {0} with {1} bytes", new Object[] { bo, bbf.remaining() }); | ||
| } | ||
| gl.glBufferData(type, bbf, usage); | ||
| gl3.glBindBuffer(type, 0); | ||
| gl.glBindBuffer(type, 0); | ||
| reg.clearDirty(); | ||
| break; | ||
| } else { | ||
| if (logger.isLoggable(java.util.logging.Level.FINER)) { | ||
| logger.log(java.util.logging.Level.FINER, "Update region {0} of {1}", new Object[] { reg, bo }); | ||
| } | ||
| gl.glBufferSubData(type, reg.getStart(), reg.getData()); | ||
| gl3.glBindBuffer(type, 0); | ||
| gl.glBindBuffer(type, 0); | ||
| reg.clearDirty(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic in this while loop seems to have a potential issue. It binds the buffer inside the loop, but if reg.isFullBufferRegion() is true, it breaks out of the loop after processing one region. If there are other dirty regions, they will be missed. If isFullBufferRegion() implies it's the only dirty region, this is fine, but it's worth double-checking if it.next() could yield more regions after a full buffer update.
References
- When analyzing loops, carefully verify the position of increment/decrement operations relative to break and continue statements, as the actual control flow may differ from common patterns.
1 participant
While JME is already technically compatible with GraalVM, most of the setup required to make it work has historically been left to application developers.
This PR adds GraalVM as a detectable platform and automatically generates JME reachability metadata, reducing the manual configuration needed to build and run JME applications with GraalVM.
WIP, needs to be rebased