GLSLCompat.glsllib doesn't compile on Android

[nickidebruyn]

While test jme3.7.0 with my android projects I found that the jme3-core/src/main/resources/Common/ShaderLib/GLSLCompat.glsllib
code contains ';' semicolons at the end of the first few lines and android is not happy with that.

Error I am experiencing:

ERROR: 0:11: ';' : Syntax error:  syntax error
INTERNAL ERROR: no main() function!
ERROR: 1 compilation errors.  No code generated.                                                                                                    
                                                                                                    	at com.jme3.renderer.opengl.GLRenderer.updateShaderSourceData(GLRenderer.java:1658)
                                                                                                    	at com.jme3.renderer.opengl.GLRenderer.updateShaderData(GLRenderer.java:1685)
                                                                                                    	at com.jme3.renderer.opengl.GLRenderer.setShader(GLRenderer.java:1750)

After some more investigation I found that the first part is the problem:

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    #if __VERSION__ >= 310
      precision highp sampler2DMS;
    #endif
  #endif
#endif

[stephengold]

Thanks for documenting the issue, @nickidebruyn!

[stephengold]

According to "git blame", the failing code was last modified by PR #2217, which was integrated between v3.7.0-alpha2 and v3.7.0-beta1.

@nickidebruyn Do your Android projects work with JME v3.7.0-alpha2 ?

[nickidebruyn]

I tested a few scenarios for this. I can confirm that none of the new version (v3.7.0-alpha2, v3.7.0-beta1, v3.7.0-stable) works.
I then tested again with v3.6.1 and it worked again.
After that I changed back to v3.7.0-stable and created my own local GLSLCompat.glsllib and changed the frag shader to point to that and it worked again. This is the code I removed from my local: GLSLCompat.glsllib

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    #if __VERSION__ >= 310
      precision highp sampler2DMS;
    #endif
  #endif
#endif

[stephengold]

Thank you for the investigation.
Based on your findings, I'm convinced PR #2061 caused the regression.
At the time (August 2023), Riccardo indicated 2061 would need to be tested on Android.
Apparently it never got such testing, and neither did JME 3.7!
I think this calls for a 3.7.1 release...

[nickidebruyn]

Agree, that is probably a good decision.
Most shaders uses that lib.

[pavly-gerges]

What's the status of this issue? Are there any working PRs to fix this?

[yaRnMcDonuts]

I've never looked into the code in GLSLCompat.glsllib and unfortunately I don't have any android experience with jme, so I can't test things to feel certain enough to confidently provide a solution.

But I'd agree with @sgold that this PR likely caused the issue: riccardobl@4e851bf

I'm not particularly knowledgeable when it comes to knowing which exact fragment/vertex shader versions and features to use in each situation especially on android, but it sounds like the "precision highp" is not fully supported on android.

And, from my understanding, this code is simply trying to set the precision for each shader varType which I'm rading mixed things about whether that's even necessary.

But I think some of this info I'm reading is coming from AI, and the rest from random stack overflow posts... so I am skeptical until I can confirm this information from another source, or preferably from our own testing.

Nonetheless, what I'm reading is saying that:

setting precision for types other than float and int (like sampler2DArray, samplerCube, etc.) is unnecessary in most cases.

Only float and int have multiple precision levels in GLSL.

highp, mediump, and lowp only apply to floating-point and integer types.
Example: highp float, mediump int, etc.
Samplers (sampler2D, samplerCube, etc.) are not affected by precision.

Samplers are not numerical types—they are handles to textures.
The precision of texture sampling is controlled by hardware, not by GLSL precision qualifiers.
This means precision highp sampler2D; does nothing.

So, unless I'm mistaken, the only potentially useful code changed in GLSLCompat in that PR is these 2 lines:

precision highp float; precision highp int;

And it sounds like both of these highp int/float precision qualifiers should also be okay on android. And if not, then something like a (#if VERSION >= 300) check around that code should prevent it from running on android devices that don't support highp int or float. But we'll have to try and find out I guess.

So to conclude, (from my limited understanding), the highp precision on non-numerical varTypes is unsupported on android, and it is unnecessary and redundant on desktop since desktop will already default to using highp for everything if supported. But android defaults to mediump or lowp when nothing is set, so I think it is still good to declare int and float as highp if the android version supports it.

Here's what I think is the best solution from my understanding thus far, feedback is greatly appreciated though: #2381

[pavly-gerges]

I tested a few scenarios for this. I can confirm that none of the new version (v3.7.0-alpha2, v3.7.0-beta1, v3.7.0-stable) works.
I then tested again with v3.6.1 and it worked again.
After that I changed back to v3.7.0-stable and created my own local GLSLCompat.glsllib and changed the frag shader to point to that and it worked again. This is the code I removed from my local: GLSLCompat.glsllib

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    #if __VERSION__ >= 310
      precision highp sampler2DMS;
    #endif
  #endif
#endif

So far, it seems that the code added recently requires a 'main()' function to be compiled as a GLSL executable. That's why we get this error:

INTERNAL ERROR: no main() function!
ERROR: 1 compilation errors.  No code generated

[yaRnMcDonuts]

So far, it seems that the code added recently requires a 'main()' function to be compiled as a GLSL executable. That's why we get this error:

This is just a case of bad error reporting from the shader. The code setting precision qualifiers is supposed to be called before the main() function (not within it). And essentially every .fragment shader has a main() function, I'd be extremely surprised if there were any changes that completely removed the main() function from a fragment shader that imports GLSLCompat.glsllib, as this would have already caused severe issues on every device, and not just android.

But because the compiler is failing while compiling the code that is setting precision before the main() function, the compiler stops before it manages to compile the rest of the shader (including the main() function) and then the gpu tries running a half-compiled version of the shader with no main() function, hence the incorrect error reporting that it is missing even though it is there. All the remaining un-compiled code will not get compiled because the shader compiler fails in a particularly fatal way first.

[pavly-gerges]

Alright, @nickidebruyn Could you please guide me to reproduce this issue?

[riccardobl]

can anyone with an android setup try to remove the version check from here and see if it reproduces?

 #ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    #if __VERSION__ >= 310
      precision highp sampler2DMS;
    #endif
  #endif
#endif

to

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    precision highp sampler2DMS;
  #endif
#endif

it might have something to do with VERSION being unset for some combination of shaders + platform

[yaRnMcDonuts]

I have doubts that this error is related to the use of VERSION (based on this comment from @nickidebruyn where the error went away after removing this whole code block)

I changed back to v3.7.0-stable and created my own local GLSLCompat.glsllib and changed the frag shader to point to that and it worked again. This is the code I removed from my local: GLSLCompat.glsllib

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
    precision highp float;
    precision highp int;
    precision highp sampler2DArray;
    precision highp sampler2DShadow;
    precision highp samplerCube;
    precision highp sampler3D;
    precision highp sampler2D;
    #if __VERSION__ >= 310
      precision highp sampler2DMS;
    #endif
  #endif
#endif

There are still other VERSION checks in GLSLCompat after this code block (here is one place: https://github.com/jMonkeyEngine/jmonkeyengine/blob/master/jme3-core/src/main/resources/Common/ShaderLib/GLSLCompat.glsllib#L77), so if VERSION were the issue, the crash would not have been resolved from removing only 1 instance of a version check while other version checks remain.

After more research from yesterday, I am even more confident in my speculation that setting precision for some sampler var types is not supported on certain android devices. But of course we will have to have android users test things to find out for sure, as the khronos and glsl documentation for precision qualifiers in regards to different devices/versions is exceptionally poor (in my opinion).

Alright, @nickidebruyn Could you please guide me to reproduce this issue?

It should be reproducible by using any shader that imports GLSLCompat.glsllib (this includes PBRLIghting.j3md and Lighting.j3md). But there is also no guarantee that the error will be reproducible on your specific android device, because different devices/gpus will have different support for these types of things. You may have a newer device that can bypass errors or support things that the OP's device cannot.

But it would still be useful to know specifically which shader and device @nickidebruyn was using when encountering this error.

[riccardobl]

I don't think it is correct to say that the precision qualifiers are not needed for samplers, the opengl es docs says

The fragment language has no default precision qualifier for floating point types. Hence for float, floating
point vector and matrix variable declarations, either the declaration must include a precision qualifier or
the default float precision must have been previously declared. Similarly, there is no default precision
qualifier for the following sampler types in either the vertex or fragment language:
sampler3D;
samplerCubeShadow;
sampler2DShadow;
sampler2DArray;
sampler2DArrayShadow;
isampler2D;
isampler3D;
isamplerCube;
isampler2DArray;
usampler2D;
usampler3D;
usamplerCube;
usampler2DArray

I have 3 theories of what might cause the issue:

1. by considering only the error log: this could be a precompiler issues that causes it to fails to replace __VERSION__ in that specific condition, for whatever reason.
2. by considering the docs: sampler2DMS is not included in the list, maybe the precision qualifier doesn't have a widespread support for this sampler.
3. sampler2DShadow is another possible candidate since it is another special sampler

imo 2 is very likely, since it is a special sampler, is closest to the reported line and fitting with the current findings.
But might need some trial and error by someone who can reproduce it.

[yaRnMcDonuts]

there is no default precision
qualifier for the following sampler types in either the vertex or fragment language:

This is the type of wording that I found confusing when I read that doc.

I wasn't sure if this wording is meant to imply that we aren't supposed to be setting default values for those samplers, or if it means that the shader internally will not have default values if they are unset.

l'd assume that if we don't set a desired precision, that the shader or device (or something) would have to default to some precision values. Otherwise would it just have null or non existent precision? I felt like that sort of makes no sense; but again I have very little experience with this prior to reading that doc and some other random posts I found that are likely not reliable sources.

imo 2 is very likely, since it is a special sampler, is closest to the reported line and fitting with the current findings.
But might need some trial and error by someone who can reproduce it.

I agree on this. I feel like I read a lot of conflicting and possibly incorrect thins from random posts around the web. But the one consistent thing I read is that some android devicse don't like when certain sampler types have their precision set.

[nickidebruyn]

I have only noticed the comments on this issue now. Sorry for not coming back earlier.
So I already have a local copy of GLSLCompat.glsllib which I can modify in my current workflow.

I also have access to a few different android devices, old and new.
There for I am will to test what ever solution or code for GLSLCompat.glsllib you want me to test.

Once we have identified the exact problem, we can merge that into the jme code.

What do you think?

[yaRnMcDonuts]

I also have access to a few different android devices, old and new.
There for I am will to test what ever solution or code for GLSLCompat.glsllib you want me to test.

For the most thorough testing, you could test each individual line of code in this block by itself by commenting out all but one line at a time:

#ifdef GL_ES
  #ifdef FRAGMENT_SHADER
//    precision highp float;
//    precision highp int;
//    precision highp sampler2DArray;
//    precision highp sampler2DShadow;
//    precision highp samplerCube;
//    precision highp sampler3D;
//    precision highp sampler2D;
    precision highp sampler2DMS;
  #endif
#endif

And then let us know which ones cause a crash, and which are safe to use on android

So far, it is likely that precision highp sampler2DMS; will certainly cause the crash, and I suspect it could crash when trying to set highp on a few other sampler types.

I've also read things that say some older android devices do not like having float or int set to high precision either, so you could confirm (or disprove) this theory by testing precision highp float; and precision highp int; alone while using your oldest and most outdated android device, and also on a newer android device.