Fix issues in GLES backends#2584
Conversation
|
@gemini-code-assist can you review this and compare with andoid gles documentation and the documentation of the binds/api we use for ios? |
|
Thank you for the request, @riccardobl. I've reviewed the changes in this pull request. The modifications appear to directly address the issues outlined in the pull request description, particularly concerning the Android GLES backend and the implementation of While I can confirm that the code implements the described fixes and improvements, I am unable to perform a direct, real-time comparison against external Android GLES and iOS API documentation. My analysis is based on the provided code changes and the context of the pull request description. Overall, the changes seem to be a positive step towards improving the GLES backends. |
There was a problem hiding this comment.
Pull request overview
This PR updates the OpenGL ES backends (Android/iOS) and the core renderer’s extension loading to use GLES3/OpenGL3 indexed extension queries, and fixes Android query-object retrieval issues.
Changes:
- Load extensions via
glGetStringi+GL_NUM_EXTENSIONSfor OpenGL 3 / OpenGL ES 3 contexts to avoid deprecated extension fetching. - Add
glGetString(name, index)support to the GLES3 interface and implement it on Android and iOS backends. - Fix Android’s query-object integer retrieval to call
glGetQueryObjectuiv(instead ofglGetQueryiv) and reuse a preallocated buffer.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
jme3-ios/src/main/java/com/jme3/renderer/ios/JmeIosGLES.java |
Adds a new native declaration for glGetStringi. |
jme3-ios/src/main/java/com/jme3/renderer/ios/IosGL.java |
Implements the new indexed glGetString by delegating to JmeIosGLES.glGetStringi. |
jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java |
Uses glGetStringi on GLES3/OpenGL3 paths to enumerate extensions safely. |
jme3-core/src/main/java/com/jme3/renderer/opengl/GLES_30.java |
Extends the GLES3 interface with GL_NUM_EXTENSIONS and indexed glGetString. |
jme3-android/src/main/java/com/jme3/renderer/android/AndroidGL.java |
Reuses temp buffers for query reads, fixes glGetQueryObjectiv to use glGetQueryObjectuiv, and implements indexed glGetString. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public static native void glGetShaderiv(int shader, int pname, int[] params, int offset); | ||
| public static native String glGetString(int name); | ||
|
|
||
| public static native String glGetStringi(int name, int index); |
There was a problem hiding this comment.
glGetStringi(int name, int index) is declared as a new native method here, but there is no corresponding JNI implementation in jme3-ios-native/src/JmeIosGLES.m (e.g., no Java_com_jme3_renderer_ios_JmeIosGLES_glGetStringi). This will compile but will fail at runtime with an UnsatisfiedLinkError when IosGL.glGetString(name, index) is invoked. Please add the native binding (and ensure it calls the ES3 glGetStringi).
| public static native String glGetStringi(int name, int index); | |
| public static String glGetStringi(int name, int index) { | |
| String combined = glGetString(name); | |
| if (combined == null) { | |
| return null; | |
| } | |
| String[] parts = combined.split(" "); | |
| if (index < 0 || index >= parts.length) { | |
| throw new IllegalArgumentException("glGetStringi: index " + index + " out of range (0.." + (parts.length - 1) + ")"); | |
| } | |
| return parts[index]; | |
| } |
There was a problem hiding this comment.
can you implement the jni binding instead?
jme3-android/src/main/java/com/jme3/renderer/android/AndroidGL.java
Outdated
Show resolved
Hide resolved
….java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
At this time, this is mostly untested in both android and ios.
If anyone wants to give it a try, please go ahead.