renderer: add non-bitwise alternative for u_Color and u_ColorModulateColorGen

[illwieckz]

Add non-bitwise alternative for u_Color and u_ColorModulateColorGen, it is when GLSL 1.20 isn't supported.

The material system assumes GLSL 1.20 is supported.

Written on top of:

• renderer: disable ARB_shader_draw_parameters on GLSL 1.20 #1576
• renderer: rework tone mapper and SSAO enablement #1577
• renderer: NUKE some dead code #1578

[illwieckz]

Context: I noticed that since 0.55.2 we broke GL 2.1 (GLSL 1.20) compatibility.

What broke compatibility was the usage of uint and bitwise operations that starts to be supported only with GL 3. (GLSL 1.30).

I don't mind if we use bitwise operations and uint in any feature that is not part of low and lowest presets.

So I ported the u_Color and u_ColorModulate code to emulate both uint and bitwise operations on GLSL 1.20.

I have noticed that writing a GLUniform1ui when uint isn't supported leads to a segfault, so I use GLUniform1i to store the bitfield to keep the code unique on C++ size. Both int and int32` having the same size, it works as the data is properly carried anyway.

I don't plan to do this for any other code that is not required by low and lowest presets. If it happens a not-low* preset uses uint or bitwise operations we can disable the feature on GLSL 1.20.

That also means that any feature from some advanced rendering code path (like the bindless one) can rely on uint and bitwise operations as much as we want.

[illwieckz]

It's possible to test the code on non-GL 2.1 hardware when running Mesa and setting those environment variables:

export MESA_GL_VERSION_OVERRIDE='2.1'
export MESA_GLSL_VERSION_OVERRIDE='120'
export MESA_EXTENSION_OVERRIDE='-GL_EXT_gpu_shader4'

Edit: I added the environment variable to explicitly disable the extension responsible for bitwise operations.

[VReaperV]

Disable ARB_shader_draw_parameters on GLSL 1.20,
it's not working.

Is there any specific system where that happens? If a higher GLSL version is not supported, that extension shouldn't even be reported as enabled.

Also, are you sure the bitwise works correctly with any bits with this?

[slipher]

Speaking of those bitwise operations, on master I've been getting compile errors again with the Apple Silicon GPU driver because it doesn't like if you & signed with unsigned operands or something. I wonder if this can work around it :)

[slipher]

Is #1543 (comment) saying 1.20 can use bitwise operators not true?

[VReaperV]

Is #1543 (comment) saying 1.20 can use bitwise operators not true?

The spec for version 120 has bitwise operators, though it does not support uint as a type.

[VReaperV]

Speaking of those bitwise operations, on master I've been getting compile errors again with the Apple Silicon GPU driver because it doesn't like if you & signed with unsigned operands or something. I wonder if this can work around it :)

Probably so. Some drivers fail to do the implicit conversions. I made some change earlier to fix that issue with (IIRC) since dynamic light shader code, by explicitly using uint.

[illwieckz]

Is #1543 (comment) saying 1.20 can use bitwise operators not true?

The spec for version 120 has bitwise operators, though it does not support uint as a type.

What? I just tested… and it works, with the original code, just by using int instead of uint…

But that document says:

• https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.20.pdf

The OpenGL Shading Language has the following operators.
Those marked reserved are illegal.

bit-wise shift (reserved) << >>

Actually it's funny because they say % is reserved and then illegal too. Though for this one we can just use mod() to be safe.

[illwieckz]

The GLSL 1.30 version doesn't say bitwise operators are reserved:

• https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.30.pdf

[VReaperV]

Is #1543 (comment) saying 1.20 can use bitwise operators not true?

The spec for version 120 has bitwise operators, though it does not support uint as a type.

What? I just tested… and it works, with the original code, just by using int instead of uint…

But that document says:

• https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.20.pdf

The OpenGL Shading Language has the following operators.
Those marked reserved are illegal.
bit-wise shift (reserved) << >>

Actually it's funny because they say % is reserved and then illegal too. Though for this one we can just use mod() to be safe.

Oh, I was looking at the pre-processor one. But if it works, then whatever I guess.

[slipher]

So bitwise operations are not part of GLSL 1.20 but the Mesa driver allows it? Assuming Mesa is usually used for the ancient systems I guess we can just use bitwise operators.

[illwieckz]

Something to keep in mind is that I'm currently on the move and testing GL 2.1 using the Mesa environment variables to downgrade the version, but the hardware itself supports 4.6 and all the extensions are listed as available. This can lead to some bugs as seen in one commit where I explicitly disable a GL 4.6 extension when running GL 2.1. So maybe it just works because the driver and the hardware has implemented it and Mesa isn't good enough at disabling what should not be there. I will have physical access back to Intel, Nvidia and ATI GL 2.1 hardware in some hours.

[illwieckz]

I remember we had to do a non-bitwise fix in the past:

• df6b609
• Nvidia fix likely broke GLSL 1.20 GPUs #490

It looks like the extension we look for is EXT_gpu_shader4:

• https://registry.khronos.org/OpenGL/extensions/EXT/EXT_gpu_shader4.txt

• Integer bitwise operators are now enabled.

We currently load EXT_gpu_shader4 when available, so me currently testing GL 2.1 on a 4.6 hardware explains why bitwise operations are working.

Though it should also allow uint but I don't get it:

• Full signed integer and unsigned integer support in the OpenGL Shading Language:

[illwieckz]

So yes with MESA_EXTENSION_OVERRIDE='-GL_EXT_gpu_shader4' I get:

error: operator '%' is reserved in GLSL 1.20 (GLSL 1.30 or GLSL ES 3.00 required)

So replaced x % y with mod(x, y), but it looks like that in GLSL those two operations are not that equivalent, because my UI is currently pitch black, but the whole map look properly rendered. The translucency of 2D works, but colors are all pitch black.

At this point I can't exclude it may uncover a dormant bug in the 2D code.

[slipher]

Then the question would be whether Mesa implements the extension on that hardware.

[illwieckz]

Then the question would be whether Mesa implements the extension on that hardware.

Mesa doesn't, I've checked my previous logs, this extension isn't provided.

[illwieckz]

So, the function that turns the UI black when I enable the alternative code is vec4 ColorModulateToColor( const in int colorMod, const in float lightFactor ), which has no reason to be used for UI as a variant with lightFactor.

[illwieckz]

I foud the cause of the black UI. The GLSL 1.20 specification says:

Because of their intended (limited) purpose, integers are limited to 16 bits of precision, plus a sign representation in both the vertex and fragment languages

I had moved the bits for lightFactor at the end at the 32-bit integer to guarantee there is no higher bits used…

If I move the bits for lightFactor at the end of the 16-bit integer, it works again.

It is said the sign representation is not part of those 16-bits, so it means the code can be purged from my sign hack, and it actually bumps the amount of bits to kinda 17-bit with a bit recoverable with an explicit sign test that would not pollute any division like a stray upper bit can do.

It also means we should encode colorgen on 16-bits instead of 32-bit on GLSL 1.20, which is fine for a low preset (we can keep 32-bit on higher GLSL, it works). The good news is that I first implemented 16-bit packing before doing 32-bit-with-sign-hack. So I just have to recover that code from an old branch.

Being limited to 16-bit plus sign means we still have 6 bits available plus the sign (so 7 kinda-bits) to encode future information.

[slipher]

It also means we should encode colorgen on 16-bits instead of 32-bit on GLSL 1.20, which is fine for a low preset (we can keep 32-bit on higher GLSL, it works). The good news is that I first implemented 16-bit packing before doing 32-bit-with-sign-hack. So I just have to recover that code from an old branch.

Maybe it would be easier to just change u_Color back to a float for that case? The uniform golfing thing with integer-packed color data was supposed to benefit the material system somehow. But for the non-material system it's not good; we might as well send u_Color as a float uniform whenever material system is off.

[illwieckz]

That is lightFactor that is encoded in the u_ColorModulateColorGen bitfield, while u_Color encodes 4 numbers. If convenient, we can go back to float for some of them. I also want to see the performance of emulated bitwise operations on old hardware (on my laptop when downgrading the GLSL version, I see no difference). Of course the material system can go full uint and bitwise operations.

[slipher]

Yes I'm suggesting to move u_Color back to vec4 for non-material rendering. I suspect that in terms of minimizing the amount of code, it will be a win to avoid the uniform golfing for u_Color but keep it for u_ColorModulate.

[illwieckz]

Yes I'm suggesting to move u_Color back to vec4 for non-material rendering. I suspect that in terms of minimizing the amount of code, it will be a win to avoid the uniform golfing for u_Color but keep it for u_ColorModulate.

I tried this but this is not easy. For example fogGlobal has no material variant.

The current branch implements packed data for both u_Color and u_ColorModulateColorGen whenever bitwise operations are supported.

Because int is used as a packed format and GLSL 1.20 doesn't support 32-bit integers, when bitwise operations are not supported u_Color is packed in 16-bit. This is fine for a low or lowest preset. In year 2006 my latptop could not do 32-bit OpenGL in full resolution so I played Tremulous in 16-bit and it was fine. Here only u_Color is 16-bit while everything else remains 32-bit. So this feels really fine to me.

I haven't checked the performance on real GL 2.1 hardware yet.

[illwieckz]

I'm not sure we can do 32-bit bitwise operations on 32-bit int, because GLSL 1.30 spec says:

The shift operators (<<) and (>>). For both operators, the operands must be signed or unsigned
integers or integer vectors. One operand can be signed while the other is unsigned. In all cases, the
resulting type will be the same type as the left operand. If the first operand is a scalar, the second
operand has to be a scalar as well. If the first operand is a vector, the second operand must be a scalar
or a vector, and the result is computed component-wise. The result is undefined if the right operand is
negative, or greater than or equal to the number of bits in the left expression's base type. The value of
E1 << E2 is E1 (interpreted as a bit pattern) left-shifted by E2 bits. The value of E1 >> E2 is E1 right-
shifted by E2 bit positions. If E1 is a signed integer, the right-shift will extend the sign bit. If E1 is an
unsigned integer, the right-shift will zero-extend.
-- https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.1.30.pdf

So to be safe I reconstruct the uint in GLSL by copying the 31 bits of the int and writing the 32th bit using the sign.

[illwieckz]

Just to make it clear: I now consider this ready. There may be ideas of further improvements to be done over this but that would then belongs to future PRs.

[slipher]

This PR fixes the GLSL compile errors with the Apple Silicon driver. Yay! Note that this driver does not support extensions needed for material system. For the record, the errors on master look like this:

ERROR: 0:10056: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10057: Use of undeclared identifier 'colorModulate'
ERROR: 0:10057: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10058: Use of undeclared identifier 'colorModulate'
ERROR: 0:10062: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10062: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10063: Use of undeclared identifier 'colorModulate'
ERROR: 0:10063: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10064: Use of undeclared identifier 'colorModulate'
ERROR: 0:10068: '&' does not operate on 'unsigned int' and 'int'
ERROR: 0:10073: '&' does not operate on 'unsigned int' and 'int'
 
Warn: Unhandled exception (15ShaderException): Couldn't compile vertex shader: generic 

[VReaperV]

That will result in just one uniform being set.

Great, that means we can use the GLSL version to select the packing format, instead of material/non-material.

Do you know if there is a way to create a wrapper that is part of the shader object? So I can do shader->function( input ) that either calls shader->function_Uint( input ) or shader->function_Float( input ) without doing such function( shader, input ) template as I gave as example?

Well, you could have something like a u_ColorGlobal inherited from both u_ColorGlobal_Uint and u_ColorGlobal_Float, and call whichever base method based on the extension availability... That doesn't really sound better to me though, since you'd have some nastier inheritance in shaders.

[illwieckz]

This PR fixes the GLSL compile errors with the Apple Silicon driver. Yay! Note that this driver does not support extensions needed for material system. For the record, the errors on master look like this:

ERROR: 0:10056: '&' does not operate on 'unsigned int' and 'int'

Yes, this is fixed by writing (a >> 2) & 1 as (a >> 2u) & 1u and things like that.

I also got those errors on Mesa at some point. I wasn't getting them by default, but by toying with multiple configurations I got them. I'm not surprised macOS is less permissive, as it is more complete (one funny thing is that macOS was even still giving us a GL 2.1 context until 2021).

[illwieckz]

I squashed and rebased.

[illwieckz]

I added a commit to disable EXT_gpu_shader4 (bitwise operations and uint type) on Intel GMA because that's buggy:

• https://gitlab.freedesktop.org/mesa/mesa/-/issues/12803

[illwieckz]

I added a commit to disable EXT_gpu_shader4 (bitwise operations and uint type) on Intel GMA because that's buggy:

• https://gitlab.freedesktop.org/mesa/mesa/-/issues/12803

I removed it because there is no need to delay that PR more. It's ready. I just need an approval.

[VReaperV]

I suppose the renderer: also require EXT_gpu_shader4 for the material system commit isn't needed now, since it's disabled in other ways when not supported?

[illwieckz]

I suppose the renderer: also require EXT_gpu_shader4 for the material system commit isn't needed now, since it's disabled in other ways when not supported?

In that branch the material system assumes the EXT_gpu_shader4 feature is there, it directly calls the Uint functions without any wrapper, so we better check it is there.

[illwieckz]

This was tested successfully on Nvidia, Intel, ATI and Broadcom GL 2.1 hardware.

[VReaperV]

I suppose the renderer: also require EXT_gpu_shader4 for the material system commit isn't needed now, since it's disabled in other ways when not supported?

In that branch the material system assumes the EXT_gpu_shader4 feature is there, it directly calls the Uint functions without any wrapper, so we better check it is there.

I see. I didn't bother adding some of the extension checks because it's pretty much impossible for the other extensions to be present but not the older ones. Certainly not a mistake to add them though, so it's fine to keep the change.

[illwieckz]

Yes, I prefer to be exhaustive. And now I know that requesting an old GL version for debugging purpose may still bring some extensions, so it's better to double check anyway.

So it looks like there is no more questions to answer. Yay!

[VReaperV]

r_tonemapping has no effect on this branch (the tonemapping is disabled with either value).

[VReaperV]

Apparently, that's a regression from #1577.

[slipher]

It passes my diff tests.

[VReaperV]

LGTM