RendererException in PBRTerrain.frag

[stephengold]

5th generation Mac Mini (Apple Silicon)
OpenJDK 64-Bit Server VM Temurin-17.0.2+8 (build 17.0.2+8, mixed mode)
JME v3.5 branch

The "HelloMav" application crashes with the message is "ERROR: '' 0:1306: : syntax error: preprocessor command must not be preceded by any other statement in that line". Googling that message led me to https://stackoverflow.com/questions/9574919/webgl-differs-from-opengl-preprocessor-on-same-graphics-stack

My conclusion: "PBRTerrain.frag" uses the ## operator in 20+ places, but that operator isn't supported in some versions of GLSL.

GitHub won't allow me to post the full console output. Here's a redacted version:

Mar 18, 2022 3:11:41 PM com.jme3.renderer.opengl.GLRenderer updateShaderSourceData
WARNING: Bad compile of:
1       #version 110
2       #define SRGB 1
3       #define FRAGMENT_SHADER 1
4       #define TRI_PLANAR_MAPPING 1
5       #define ALBEDOMAP_0 1
6       #define ALBEDOMAP_1 1
7       #define ALBEDOMAP_2 1
8       #define ALBEDOMAP_3 1
9       #define ALBEDOMAP_4 1
10      #define ALBEDOMAP_5 1
11      #define ALBEDOMAP_6 1
12      #define ALPHAMAP 1
13      #define ALPHAMAP_1 1
14      #define ALBEDOMAP_0_SCALE 0.1
15      #define ALBEDOMAP_1_SCALE 0.05
16      #define ALBEDOMAP_2_SCALE 0.1
17      #define ALBEDOMAP_3_SCALE 0.1
18      #define ALBEDOMAP_4_SCALE 0.3
19      #define ALBEDOMAP_5_SCALE 0.1
20      #define ALBEDOMAP_6_SCALE 0.5
21      #define DEBUG_VALUES_MODE 0
22      #define SINGLE_PASS_LIGHTING 1
23      #define NB_LIGHTS 3
24      #define NB_PROBES 1
25      #define USE_AMBIENT_LIGHT 1
26      #extension GL_ARB_shader_texture_lod : enable
27      // -- begin import Common/ShaderLib/GLSLCompat.glsllib --
28      #if defined GL_ES

...

1301        #ifdef ALBEDOMAP_0   
1302                        //NOTE! the old (phong) terrain shaders do not have an "_0" for the first diffuse map, it is just "DiffuseMap"
1303            #ifdef NORMALMAP_0
1304                BLEND_NORMAL(_0,  alphaBlend.r)
1305            #else
1306                BLEND(_0,  alphaBlend.r)
1307            #endif
1308            
1309        #endif
1310        #ifdef ALBEDOMAP_1
1311            #ifdef NORMALMAP_1
1312                BLEND_NORMAL(_1,  alphaBlend.g)
1313            #else
1314                BLEND(_1,  alphaBlend.g)
1315            #endif

...

1934            
1935        #endif
1936        
1937        gl_FragColor.a = albedo.a;
1938    
1939    }

Mar 18, 2022 3:11:41 PM com.jme3.app.LegacyApplication handleError
SEVERE: Uncaught exception thrown in Thread[jME3 Main,5,main]
com.jme3.renderer.RendererException: compile error in: ShaderSource[name=Common/MatDefs/Terrain/PBRTerrain.frag, defines, type=Fragment, language=GLSL100]
ERROR: 0:1306: '' : syntax error: preprocessor command must not be preceded by any other statement in that line
ERROR: 0:1306: 'premature EOF' : syntax error syntax error

        at com.jme3.renderer.opengl.GLRenderer.updateShaderSourceData(GLRenderer.java:1539)
        at com.jme3.renderer.opengl.GLRenderer.updateShaderData(GLRenderer.java:1566)
        at com.jme3.renderer.opengl.GLRenderer.setShader(GLRenderer.java:1631)
        at com.jme3.material.logic.SinglePassAndImageBasedLightingLogic.render(SinglePassAndImageBasedLightingLogic.java:276)
        at com.jme3.material.Technique.render(Technique.java:167)
        at com.jme3.material.Material.render(Material.java:1052)
        at com.jme3.renderer.RenderManager.renderGeometry(RenderManager.java:651)
        at com.jme3.renderer.queue.RenderQueue.renderGeometryList(RenderQueue.java:273)
        at com.jme3.renderer.queue.RenderQueue.renderQueue(RenderQueue.java:312)
        at com.jme3.renderer.RenderManager.renderViewPortQueues(RenderManager.java:928)
        at com.jme3.renderer.RenderManager.flushQueue(RenderManager.java:823)
        at com.jme3.renderer.RenderManager.renderViewPort(RenderManager.java:1184)
        at com.jme3.renderer.RenderManager.render(RenderManager.java:1248)
        at com.jme3.app.SimpleApplication.update(SimpleApplication.java:278)
        at com.jme3.system.lwjgl.LwjglWindow.runLoop(LwjglWindow.java:580)
        at com.jme3.system.lwjgl.LwjglWindow.run(LwjglWindow.java:669)
        at com.jme3.system.lwjgl.LwjglWindow.create(LwjglWindow.java:493)
        at com.jme3.app.LegacyApplication.start(LegacyApplication.java:490)
        at com.jme3.app.LegacyApplication.start(LegacyApplication.java:442)
        at com.jme3.app.SimpleApplication.start(SimpleApplication.java:126)
        at com.jayfella.jme.vehicle.simpledemo.HelloMav.main(HelloMav.java:104)

[JME ERROR] Uncaught exception thrown in Thread[jME3 Main,5,main]
RendererException: compile error in: ShaderSource[name=Common/MatDefs/Terrain/PBRTerrain.frag, defines, type=Fragment, language=GLSL100]
ERROR: 0:1306: '' : syntax error: preprocessor command must not be preceded by any other statement in that line
ERROR: 0:1306: 'premature EOF' : syntax error syntax error

[yaRnMcDonuts]

I unfortunately don't have a mac to test anything with this on my own, although I can still do my best to help solve the issue, especially since I plan for my own project to be runnable on mac eventually as well.

I used the ## operator as a way to add ##index at the end of the textures and roughness/metallic values for each of the 12 textures slots in a few #define methods, rather than putting that some code copy/pasted 12 times. So it is possible to rewrite the shader in a less-efficient way but that would be adding another ~800 lines of copy/pasted code from the #define methods that currently use ## into all 12 if blocks for the terrain (in 4 places since the terrain has a #define method for terrains with normal maps, without normal maps, tri planar with normal maps, and tri-planar without normal maps), and then you'd need to change the ##index value for each code block to match texture slots 0-12.

And that would be even more for the advanced version of the shader since it has more code for metallic/roughness/ao map. Plus both shaders would become a lot less managable, requiring any changes that currently would be put into 1 of 4 (now invalid on mac) methods to instead be copied to ~24 places in each shader.

So refactoring to remove ## not necessarily something I'd like to do unless we've exhausted all other possible options for fixing it. Is it certain that there's no way for mac to support the use of ##?

If ## is completely off the table for mac, and the code definitely needs changed, then I think the only other realistic option would be to use the newly support for loops that @riccardobl recently added :
#1758

However I am not sure at all if that would be supported by mac or not, since it seems really strange already that mac disallows ## with no apparent excpetions.

[riccardobl]

However I am not sure at all if that would be supported by mac or not, since it seems really strange already that mac disallows ## with no apparent excpetions.

The for loop thing will be supported, since it is resolved by jme, not by the glsl preprocessor (it is a fake preprocessor directive).
Either that or we can add a custom preprocessor in front of shaders compilation, it is possible, but we have to figure out if there is a good C preprocessor in java or that can be ported to java, since implementing it from scratch would take quite some time.

[stephengold]

I found this: https://github.com/shevek/jcpp

[riccardobl]

I found this: https://github.com/shevek/jcpp

Yeah, me too, but it doesn't seem very easy to adapt to our problem

If ## is completely off the table for mac, and the code definitely needs changed, then I think the only other realistic option would be to use the newly support for loops that @riccardobl recently added :
#1758

I think this is the way to solve the issue, it will also make the code more readable.

I unfortunately don't have a mac to test anything with this on my own, although I can still do my best to help solve the issue, especially since I plan for my own project to be runnable on mac eventually as well.

@yaRnMcDonuts if you are working on this, i can send you the credentials to access a remote macos instance for testing

[yaRnMcDonuts]

Yeah I'm not actively working on this right now but plan to soon. First I will refactor the shaders to use the for loops and get familiar with using that, then once its all working good on my windows device I'll do testing on the remote mac instance to make sure its working.

It looks like your PR to add the shader loop support is merged to master but not the latest 3.5 release (if I'm seeing things correctly), so it could also be a good idea to include that in the next (3.6?) release, so then I could plan to have the updated terrain shaders ready for that release too.

[riccardobl]

Asking @stephengold since he is managing jme3.x releases

[stephengold]

I agree that if there's ever a JMonkeyEngine 3.6 release, then PR #1758 and the fix for this issue should be included.

As long as the fix for this issue is integrated into "master" branch before the 3.6 code freeze, I don't foresee any difficulty.

Currently there's no 3.6 release scheduled, so I don't know when code freeze will occur. My best guess would be summer of 2022.

[stephengold]

@yaRnMcDonuts Are you still planning to refactor the shaders to use for loops?

[yaRnMcDonuts]

Yes it's still on my to do list, but I've gotten distracted by some high priority things relating to my main JME project the past few months and haven't had time to focus on this issue.

I should be able to get started on this sometime this month though. Changing the code should be pretty fast and easy, but since this shader for-loop functionality is new and not used anywhere else in JME yet, I've been wanting to make sure I work on this issue when I'm able to spend a solid amount of time testing it to ensure it doesn't cause any unexpected behavior or lower the framerate when compared to the current version.

[zzuegg]

May i ask why all the terrain shaders don't use texture arrays and arrays as parameters? Is there any limitation i am not aware of?
Or is it just for compability reasons?

[Ali-RS]

Doesn't a texture array require the textures to be the same size?

[zzuegg]

Ah yeah. That is the case.
I will wait until this issue is fixed before converting it to deferred. this shader is a nightmare 🗡️

[yaRnMcDonuts]

There is another version of the pbr terrain shader that uses texture arrays (AdvancedPbrTerrain.j3md), but that one is also somewhat of a jumbled mess trying to support tri-planar with optional normalMaps and metallicRoughnessAoMaps, so I think that would also be a struggle to convert to deferred until its using for-loops. I will try to get to work on this issue to add for-loops to both shaders as soon as I can.

[yaRnMcDonuts]

I submitted a PR updating PBRTerrain.frag to use the new for-loops, so the shader no longer use any pre-processor functions with ## and should not cause any issues on mac anymore, although I did not get to test it on a mac to confirm for 100% certain.

#1901

@zzuegg This should also hopefully make it easier to make a deferred version of the terrain now. The code was reduced by nearly half and I also tried to organize and clean up a lot of the remaining code so it isn't as jumbled.
However there are still some hacky things in the terrain shader that are for features exclusive to my game that may seem strange or could cause more confusion (specifically the affliction splatting and using the red channel of the vertex color buffer as an AO map for just the directionalLight so sunlight can be dimmed or entirely blocked indoors without requiring a shadow filter), so let me know if you have any trouble and I will be glad to help however I can!

[zzuegg]

I will give it a look as soon i worked down the queue for my own project a bit. The light dimming will not work in a default deferred pipeline since it requires additional information at the lighting stage. The pipeline alteady supports such changes if one want to add custom features by not using the shipped materials and buffer layouts.

Might take a week or two until i have time for this. thanks