-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Android openGL ES 3 support #1147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Resync fork
Update fork
Added instancing support for android which is core feature in GLES3.0 Added fix for shadows in android
…l GLSL ES versions
Added proper checkings for new OpenGL ES versions Fixed some issues with shadow rendering in GLES 3.0 or better (strict type checking, OpenGL ES specific extensions and percision definition) Added GLSL320 and GLSL300 to Unshaded and Lighting materials
…GLES 3.2 as core Modified test materials to use GLSL310
Mods on Shadows.glsllib to try to fix android shadow rendering Fixed ssaoBlur.frag that was using a reserved word Fixed precision type missmatch from vertex to fragment shader
Framebuffer MRT support
Enabled multisampling
…ecision missmatch error but created rendering issues
…on GLES2.0 devices Added texture compare mode for GLES3.0 and better to be able to use sampler2DShadow Modified base materials (Lighting and Unshaded) to use only GLSL100 for post shadow pass
… to support ShaderNodes in GLES SL30 and included it's usage in DesktopAssetManager. Nowadays just copied and overwrote the version string. Needs a full review Added image format RGB16F to be used as texture format only in GLES30 (cannot be used as FB format) Added GLSL300 and/or GLSL310 to different materials
Fixed shadows glsl lib removing use of GL_EXT_gpu_shaders5
…s unsupported in GLES2.0 Modified PBRLighting frag to properly compile on GLES3
Set highp as default precision to fix glitched shadows caused because not enough precision
…ly unsupported in GLES
… LightScattering material. ** Not fully tested
…Previous file moved to _dtx1.dds
…ly mipmapped images and PBR rendering
Removed also Pond and rock png files previously used in TestTextureArray
If this is a dependency, gradle can just load it from the sdk folder. I will open an issue for that, but for now i just want to verify the integrity of the android.jar provided in this pr. This is the sha256 hash of android.jar from the last android studio release and this is the hash of the jar provided in this pr Can you point me to the release from which this jar comes from? |
|
Sure, it's the android.jar from api level 28. It should be located at your android sdk installation path. In my case /opt/android-sdk/ : sha256sum /opt/android-sdk/platforms/android-28/android.jar |
|
Ok, i was testing the wrong file. |
riccardobl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is jme3-effects/src/main/resources/Common/MatDefs/Water/Textures/water_normalmap_dxt1.dds used anywhere?
| stringBuf.insert(idx + 1, "precision highp sampler3D;\n"); | ||
| stringBuf.insert(idx + 1, "precision highp sampler2D;\n"); | ||
| } | ||
| stringBuf.insert(idx + 1, "precision highp float;\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a good idea to force this on the developer?
I know the code already forced precision mediump float; , but this looks like bad practice to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So you mean removing all these inserts and making the game/shader developer? This will lead to plenty of current jme3 shaders not to compile on gles at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could it be implemented in GLSLCompat? So that devs can leave it out for custom shaders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, precision directive must be after last #extension line. All shaders include GLSLCompat, then other requirements (that could have extension directives) and then the shader code, so having precision defined in GLSLCompat will make compilation fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the shader loader supposed to move the #extension directives to the top?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right and should work. I'll change and test it and get back to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried changed it and some shaders included by default in jme3 like Common/MatDefs/Terrain/TerrainLighting.frag don't include GLSLCompat.glsllib. We could either leave it as it's now or review all shaders to include GLSLCompat as you think is a better option 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, not a problem, we can add the GLSLCompat to the remaining shaders with a future PR,
jme3-core/src/main/java/com/jme3/renderer/opengl/GLRenderer.java
Outdated
Show resolved
Hide resolved
Not at all, I didn't realize that file when removing the others 🤦♂️ just removed and comited |
riccardobl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If possible i'd like to see the precision instructions moved to the GLSLCompat as we discussed.
That aside, i think this is ready to be merged/
|
I'd like to include this in the 3.3.0-alpha3 release. How soon can it be finished? |
|
From my side it's finished. @riccardobl changed it to Testing but I don't know who and when should test it before merging |
|
Have the |
|
No because plenty of shaders would stop working as they don't include GLSLCompat. So if I understood Riccardo correctly, we'll leave this for the future |
|
I meant to add the precision to GLSLCompat now and include GLSLCompat on all the shaders in a future pr, but if you think we should delay that and leave the pr as it is, then it's fine either way. Since i'm not an android developer i'm waiting for the approvation of someone else who can validate and maybe test this pr on an existing application to see if we missed something. |
|
Testing on existing application using https://github.com/joliver82/jmonkeyengine and android-gl3 branch public class MainActivity extends Activity { } public class Main extends SimpleApplication { public class TestPBR extends BaseAppState { } |
|
Problem appears while trying to use running heightmap on 3.3.0-alpha2 |
|
@kaloyanDEV About the heightmap issue, did it work previously in main jme3? Did you run a sample from jme3-examples or any personal code? Could you send me a simple testcase if the later case? |
if you can not reproduce it I will send you the assets also |
|
What is the status for this? |
|
The issue that @kaloyanDEV mentioned seems related to terrain stuff, not sure what it has to do with this PR 🙄 |
|
yes Terrain.j3md is not part of changes but might be new post in hub if someone try using it. I run all tests from JME3 Tests Android apk successfully from this joliver repo. |
|
Hi, yes, there's nothing explicitly related to terraing in this PR. From my point of view, if @kaloyanDEV tested every other tests and worked, we could merge and leave this new issue for a future PR. I'll get into it also once I have some more spare time |
|
Okay, I am letting this to be for 2 more days, going to merge it after 2 days if no objection is raised until then. |
Added gles30 and newer functionality for android. All the information at the forum thread: https://hub.jmonkeyengine.org/t/opengl-es-3-0-and-newer-on-android/41913
Chage list:
Known issues/pending work: