New iOS SDL3 + GLES 3.0 Angle Metal backend#2775
Conversation
|
🖼️ 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 migrates the iOS backend to the libJGLIOS framework, replacing legacy native code with generated bindings for GLES and OpenAL and significantly improving input handling for touch, mouse, and gamepads. The changes also include a simplified spherical harmonics baking process and the addition of an iOS test chooser. Reviewers identified several critical issues, including a shader compilation error in IBLSphH.frag caused by the missing Hammersley function and a type mismatch. Additionally, the PR introduces a GPU resource leak in IBLGLEnvBakerLight, performance bottlenecks in JmeIosGLES due to excessive heap allocations, and a potential rendering bug in the IGLESContext sRGB blit logic. Clarification is also requested regarding the intentional disabling of hardware shadow comparison on iOS.
|
|
||
| const uint SAMPLE_COUNT = 4096u; | ||
| for(uint sampleIndex = 0u; sampleIndex < SAMPLE_COUNT; sampleIndex++) { | ||
| vec4 xi = Hammersley(sampleIndex, SAMPLE_COUNT); |
There was a problem hiding this comment.
The function Hammersley is used here but it is neither defined in this shader nor imported via #import. This will cause a shader compilation error. Furthermore, the standard Hammersley implementation in jMonkeyEngine's shader library returns a vec2, which would cause a type mismatch error when assigned to a vec4.
| public static void glUniform1fv(int location, int count, FloatBuffer v) { GLES.glUniform1fv(location, count, floats(v, count)); } | ||
| public static void glUniform1fv2(int location, int count, float[] v, int offset) { GLES.glUniform1fv(location, count, slice(v, offset, count)); } | ||
| public static void glUniform1i(int location, int x) { GLES.glUniform1i(location, x); } | ||
| public static void glUniform1iv(int location, int count, IntBuffer v) { GLES.glUniform1iv(location, count, ints(v, count)); } | ||
| public static void glUniform1iv2(int location, int count, int[] v, int offset) { GLES.glUniform1iv(location, count, slice(v, offset, count)); } | ||
| public static void glUniform2f(int location, float x, float y) { GLES.glUniform2f(location, x, y); } | ||
| public static void glUniform2fv(int location, int count, FloatBuffer v) { GLES.glUniform2fv(location, count, floats(v, count * 2)); } | ||
| public static void glUniform2fv2(int location, int count, float[] v, int offset) { GLES.glUniform2fv(location, count, slice(v, offset, count * 2)); } | ||
| public static void glUniform3f(int location, float x, float y, float z) { GLES.glUniform3f(location, x, y, z); } | ||
| public static void glUniform3fv(int location, int count, FloatBuffer v) { GLES.glUniform3fv(location, count, floats(v, count * 3)); } | ||
| public static void glUniform3fv2(int location, int count, float[] v, int offset) { GLES.glUniform3fv(location, count, slice(v, offset, count * 3)); } | ||
| public static void glUniform4f(int location, float x, float y, float z, float w) { GLES.glUniform4f(location, x, y, z, w); } | ||
| public static void glUniform4fv(int location, int count, FloatBuffer v) { GLES.glUniform4fv(location, count, floats(v, count * 4)); } | ||
| public static void glUniform4fv2(int location, int count, float[] v, int offset) { GLES.glUniform4fv(location, count, slice(v, offset, count * 4)); } | ||
| public static void glUniformMatrix3fv(int location, int count, boolean transpose, FloatBuffer value) { GLES.glUniformMatrix3fv(location, count, bool(transpose), floats(value, count * 9)); } | ||
| public static void glUniformMatrix3fv2(int location, int count, boolean transpose, float[] value, int offset) { GLES.glUniformMatrix3fv(location, count, bool(transpose), slice(value, offset, count * 9)); } | ||
| public static void glUniformMatrix4fv(int location, int count, boolean transpose, FloatBuffer value) { GLES.glUniformMatrix4fv(location, count, bool(transpose), floats(value, count * 16)); } | ||
| public static void glUniformMatrix4fv2(int location, int count, boolean transpose, float[] value, int offset) { GLES.glUniformMatrix4fv(location, count, bool(transpose), slice(value, offset, count * 16)); } |
There was a problem hiding this comment.
The glUniform*fv2 and glUniformMatrix*fv2 methods perform heap allocations on every call by using floats() or slice(). This generates significant GC pressure and negatively impacts performance. Consider using a reusable thread-local buffer. Additionally, ensure you check if the source data is null before processing to prevent unnecessary operations.
References
- When updating a buffer, check if the source data is null to prevent potential issues and unnecessary operations.
| try { | ||
| renderer.setFrameBuffer(null); | ||
| blitGeometry.updateGeometricState(); | ||
| application.getRenderManager().renderGeometry(blitGeometry); |
There was a problem hiding this comment.
Rendering the blitGeometry (a Picture with a 1x1 size) using renderManager.renderGeometry() will use the last active camera in the RenderManager. Unless that camera is an ortho camera specifically configured to cover the 0..1 range, the blit will likely fail to fill the screen correctly. It is safer to use a dedicated ViewPort or manually set a temporary ortho camera matching the screen dimensions before this call.
| } | ||
|
|
||
| Texture2D shCoefTx[] = { new Texture2D(NUM_SH_COEFFICIENT, 1, 1, format), new Texture2D(NUM_SH_COEFFICIENT, 1, 1, format) }; | ||
| Texture2D shCoefTx = new Texture2D(NUM_SH_COEFFICIENT, 1, 1, format); |
There was a problem hiding this comment.
The shCoefTx texture is created here but is never disposed. Since this baking process can be triggered multiple times, this will lead to a leak of GPU resources. You should call shCoefTx.getImage().dispose() at the end of the method.
|
|
||
| renderer = new GLRenderer(gl, gl, gl); | ||
| renderer.initialize(); | ||
| renderer.getCaps().remove(Caps.TextureShadowCompare); |
There was a problem hiding this comment.
Caps.TextureShadowCompare is explicitly removed from the renderer's capabilities immediately after initialization. This contradicts the logic added in GLRenderer which enables this cap for GLES 3.0. If hardware shadow comparison is intentionally disabled for the iOS backend (e.g., due to ANGLE or Metal driver issues), please add a comment explaining the reason.
riccardobl
changed the title
riccardobl
changed the title
1 participant
New iOS backend built around a set of native bindings for sdl3, angle and openal.
Contrary to the current iOS backend, this one does not rely on deprecated platform native gles or unofficial JREs, instead it builds as a native application with graalvm 21 and binds to google angle on Metal.
This should make iOS development and capabilities on par with the new desktop Angle backend and android backend, with support for modern java.
Currently WIP.
To test run on macos (make sure to have xcode installed)
if a real device is connected to xcode, the example chooser will be installed there, otherwise it will start a simulator.
Known issues so far: