mgsx-dev / gdx-gltf

GLTF 2.0 3D format support and PBR shader implementation for LibGDX
Apache License 2.0
214 stars 50 forks source link

handling of CSM uniforms made compatible with WebGL #127

Closed MonstrousSoftware closed 4 months ago

MonstrousSoftware commented 4 months ago

This is linked to issue #126 to fix cascaded shadow maps for the web platforms (HTML/GWT and TeaVM).

Changing the way array uniforms are handled (i.e. without integer arithmetic) helps in supporting WebGL. Uniform array of sampler2d was replaced by separate uniforms.

mgsx-dev commented 4 months ago

Nice catch ! I followed the discussion on Discord and made my own investigations. I may have a better fix, see #128 Could you please, test it on TeaVM backends (and also GWT) ? I only tested it on GWT and it's good to have more tests for those GPU driver dependant issues.

MonstrousSoftware commented 4 months ago

I have tested your fix following the #128 modification but it doesn't work for Chrome with the default ANGLE backend.

It does work if you switch the Chrome ANGLE backend from default (=D3D11) to OpenGL. (Use chrome:://flags and search for ANGLE).

The problem is the array of samplers in the shader code (the arrays of Transforms and PCFClip work fine). I have even tried fetching the location of each individual array element (u_csmSamplers[0], u_csmSamplers[1], etc.) and setting the elements individually, but every time you get exactly the 'weird result' as shown in the discord thread. The console reports 'Internal D3D11 error: Error compiling dynamic pixel executable.'

A fix which resolves this both for html and teavm and regardless of the ANGLE backend is to have individual sampler uniforms ( uniform sampler2D u_csmSamplers0; uniform sampler2D u_csmSamplers1; etc. and setting their value individually.

MonstrousSoftware commented 4 months ago

Please also note that I have canRender(Renderable) return false if the number of cascades change in order to force a recompile of the shader.

mgsx-dev commented 4 months ago

@MonstrousSoftware Thank you very much for your tests and investigations. I'm OK having individual samplers instead of array. This is not really a fix, but a ANGLE bug workaround (apparently on Windows). We lose a small optimization here but not a big deal IMHO.

About the canRender fix, i saw that and i checked, i'm OK to have that too, i can see some use cases where you want to switch from differents CSM config during the game without having to recompile shaders.

Could you please cleanup your changes ? to be ready to be merged.

MonstrousSoftware commented 4 months ago

I cleaned up the code, but I will not be able to run any tests until tomorrow.

MonstrousSoftware commented 4 months ago

Okay, I have tested this latest update and it works on lwjgl3, html/GWT, teavm with the different Chrome ANGLE backends (OpenGL, DX11, DX9, DX11on12). You can also change numCSM on the fly and it will recompile the shader automatically.

I published a CSM test application here (change gdxGltfVersion in gradle.properties as needed) : https://github.com/MonstrousSoftware/CascadedShadows

mgsx-dev commented 4 months ago

@MonstrousSoftware Thank you for your contribution ! I fixed few minor things.