bugfix: decrease minimum required GL version for most shaders

[Skylion007]

Motivation and Context

• Some of our more rarely used shaders use GL versions that some GPUs don't fully support despite the fact we aren't using any features that necessitate these new versions. Downgrade the GL version where appropriate to improve shader compatibility. Fixes error: GLSL 4.10 is not supported. Supported versions are: 1.10, 1.20, 1.30, 1.40, 1.50, 3.30, 1.00 ES, and 3.00 ES #2089

How Has This Been Tested

• Locally with pytest test suite

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• My code follows the code style of this project.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• All new and existing tests passed.

[jturner65]

Can we not do this just for mac?

[Skylion007]

Can we not do this just for mac?

We could if we wanted to, the current shaders work fine on my Mac, so it's not a problem with all Macs apparently. Also it's not an issue since these shaders don't use any 4+ GL features (except maybe the PTex Shader).

[jturner65]

Also it's not an issue since these shaders don't use any 4+ GL features (except maybe the PTex Shader).

Actually, I'm getting a cast error in my PBR work if I use 3.30 instead of 4.1

[Skylion007]

Also it's not an issue since these shaders don't use any 4+ GL features (except maybe the PTex Shader).

Actually, I'm getting a cast error in my PBR work if I use 3.30 instead of 4.1

Ah, interesting... Could you post the full cast error?

[jturner65]

implicit cast uint to int.

Easy enough/painless to address, I'm only concerned about how we may be hindering ourselves in the future just because Apple is trying to force folks to Metal.

I have insufficient expertise to say with impunity that 330 will always be sufficient. For example, does this mean that cubemap array textures only became available in 4.0? Will we wish to use those? I don't know.

[jturner65]

This is a @mosra question :)

[jturner65]

It does seem like folks are able to use 4.1 on some macs. Could this be a configuration/setup issue for the user?

[mosra]

Going with GLSL 3.30 is a good thing because it gets you closer to the syntax WebGL 2 expects, so this is a good change. Anything from newer versions can be opted in via extensions (you can look at code of Magnum's builtin shaders if you're interested, they're also mostly just on GLSL 3.30 or even earlier).

So if you fix that one casting issue that popped up, I think this PR is good to go. It might be good to have the shader code validated for WebGL 2 as well. I'll see if I can provide you the tools to do that without having to compile & run the whole Emscripten build. But even without that, the PR already fixes something, so it could get merged even without the shaders being valid for WebGL 2.

It does seem like folks are able to use 4.1 on some macs.

Some older Macs have only GL 3.3 IIRC, and those Macs don't even have Metal, just GL. Another possible case is older GPUs, GL 4.0 requires certain features such as 64-bit integers and floats in shaders that older architectures don't have any support for.

[jturner65]

Cool, ok! Has my vote then :).

[jturner65]

LGTM!

[jturner65]

@Skylion007 Probably want to add PTex shader too, no?

[mosra]

PTex shader

I don't think it's worth modifying that one. Its test coverage is very low (if any at all), no current dataset uses PTex, and so if you accidentally break it you may not even know. Leave it at 4.1.

This still needs to fix that cast error though you mentioned, so no approval yet -- John, can you say what to change and where?

[jturner65]

I don't think it's worth modifying that one. Its test coverage is very low (if any at all), no current dataset uses PTex, and so if you accidentally break it you may not even know. Leave it at 4.1.

Should we remove that support then? There's so much custom ptex stuff, if we're not going to support it anymore then we should consider removing it, I think.

This still needs to fix that cast error though you mentioned, so no approval yet -- John, can you say what to change and where?

I made these changes, and fixed the issues that resulted, in the most recent commit in my PBR/IBL WIP Pr .

I'll make the changes that I found and add them here.

[Skylion007]

@jturner65 Whoops, accidentally removed your approval.

[jturner65]

One thing that needs to be tested that I did not get to is the various texture queries for ID - not sure if that will cause the cast complaint. I'll check it tomorrow if nobody else has by then :).

[mosra]

texture() returns a vector type matching the sampler type (docs, the g stands for a "generic type prefix"). So in this case, sampler2D will return a vec4, usampler2D a uvec4. No cast needed.

[jturner65]

oof i knew that. sorry, forgot.

LGTM