sys: add workaround.* cvars to disable workarounds

[illwieckz]

So, I'm thinking about it for years now, probably since the current implementation of the nvidia 340 workaround:

• https://github.com/DaemonEngine/Daemon/pull/370

I even discussed it there:

• https://github.com/DaemonEngine/Daemon/issues/360

The idea is to selectively disable a workaround we implement, to be able to test the code without it.

This is especially useful when bisecting a bug in some underlying tech like a driver, to make sure we actually test the feature in the way we expect it to fail if the third-party bug is still there.

For now I go with a dot-based hierarchy (there is no legacy cvar to mimic), and I currently use the workaround.<whatIsDone>.<whenItIsRequired> naming scheme.

For example workaround_noTexturGather_nvidia340 disables ARB_texture_gather on Nvidia driver version 340, while firstProvokingVertex_intel makes the engine use first provoking vertex on Intel hardware supporting ARB_provoking_vertex.

The implementation is trivial, I'm mostly looking for comments about the idea itself, the naming scheme…

[illwieckz]

I added a code to disable bindless texture on AMD OGLP on Linux, and the related workaround cvar.

[illwieckz]

I implemented a fix for:

• https://github.com/DaemonEngine/Daemon/issues/343

With workaround.noHyperZ.mesaRv600 off:

With workaround.noHyperZ.mesaRv600 on:

The fix requires to restart SDL after having read GL_RENDERER, so a mechanism to restart SDL on some conditions was implemented.

It was tested on Radeon HD 3650 (RV635) on Mesa 24.0.9.

[illwieckz]

Outside of the naming topic, I've implemented some new workarounds, they work as expected on my end.

[illwieckz]

Those being developer cvars, they are not expected to be used by users and it's fine to can rename them later if we find a better naming later. I don't like to have those cvars in r_ namespace with annoying name, and the dotted hierarchy actually look better for them. I prefer workaround.noBindlessTexture.mesa241 to r_workaroundNoBindlessTextureMesa241…

The code looks fine to me and I don't want to postpone the feature because of naming concerns.

[slipher]

Maybe some 3rd person can give an opinion on whether to stick with r_ prefix or not?

[illwieckz]

I guess I can go forward and merge it?

[DolceTriade]

If renderer.* is the future, then I'm fine with that. However, if we have no intention of doing a full migration to r_ -> renderer, then I don't think we should use the renderer prefix.

Not a strong opinion either way.

[illwieckz]

@DolceTriade here is a thread on the topic:

• https://github.com/DaemonEngine/Daemon/pull/1224#discussion_r1696175164

Basically my own opinion is:

• I don't want to add more r_ cvars and especially not pollute them with this ones.
• I do think renderer.* is the future, I prefer such hierarchical scheme (like we have with common.framerate.max or vm.cgame.type) than the old messy x_* in general, even if more verbose. The only reason I'm still creating r_* renderer cvars is for consistency (we better to convert them both at once if we do, and would need an alias mechanism for backward compatibility of scripts).
• I do believe having a workaround. prefix is a good idea, like we have.
• I don't see the need to write in the name some renderer string, even if maybe all those workarounds will be for the renderer, I'm not excluding the fact we may have to implement a workaround for audio or something else.
• Those cvars are meant to be used by developers (either engine developers, either driver developers, either operating system developers) or people doing bug reporting and testing while being in touch with a developer, not by users.

Actually I do prefer a generic workaround.* and I don't see the need to add any renderer string somewhere, the name part noBindless or gl21 is enough for a developer. Adding a renderer component in the name just looks verbose for no reason.

Even if we go the renderer. way for r_ cvars one day, I don't see the need for those workaround cvars to be part of that.

[illwieckz]

In fact we actually do have non-renderer workarounds I want to implement disablement cvars for, related to the nacl loader.

So I believe the better thng is to do workaround.topic.name. And the topic for the ones in this PR may even not be the renderer. We do not workaround the renderer, we workaround opengl software or hardware.

So it appears to me better names would be like that:

• workaround.opengl.*
• workaround.nacl.*

and possibly if needed:

• workaround.glew.*
• workaround.sdl2.*
• workaround.openal.*

etc.

[VReaperV]

If we're being pedantic, it's workaround.driver.* since the issue is not within the API spec, but with the specific implementation.

[illwieckz]

I did that:

workaround.glDriver.amd.oglp.disableBindlessTexture
workaround.glDriver.mesa.ati.rv300.disableRgba16Blend
workaround.glDriver.mesa.ati.rv600.disableHyperZ
workaround.glDriver.mesa.forceS3tc
workaround.glDriver.mesa.intel.gma3.forceFragmentShader
workaround.glDriver.mesa.intel.gma3.stubOcclusionQuery
workaround.glDriver.mesa.v241.disableBindlessTexture
workaround.glDriver.nvidia.v340.disableTextureGather
workaround.glExtension.missingArbFbo.useExtFbo
workaround.glHardware.intel.useFirstProvokinVertex

I have another glHardware workaround to implement but it can be done later and we don't need to wait for it. This is just to say even the branches with currently only one leaf may get more leaves in the future.

[DolceTriade]

ok sgtm

[slipher]

1. Should namespaces be hierarchical? AFAIK no one really opposes that. We can make hierarchies equally well with dot or underscore separators, for example g_bot_. Though not all the workaround cvars given above actually seem hierarchical; sometimes it's just a generic word/phrase separator:

• workaround.glExtension.missingArbFbo.useExtFbo: missingArbFbo doesn't make sense as a hierarchical element.
• workaround.glDriver.mesa.v241.disableBindlessTexture: version 24.1 shouldn't be a hierarchical element because the work around is not specific to exactly 24.1, but rather a version range starting from there. I think some of the ones with hardware version numbers have this property too.

2. Should namespaces be like r_, cl_, or renderer., client.? I prefer the former:

• The convention of naming cvar C++ objects the same as the cvar name is nice, so that all references can be searched for with a single string.
• The dotted ones are too verbose. It's annoying every type I don't have my maxfps alias and have to type common.framerate.max.
• Status quo bias - Everyone's already used to the current thing; we don't need to migrate scripts (or create multiple names of one cvar which would also be unfortunate); same as other engines, etc.

3. Should workarounds have a top-level namespace? Heretofore all cvar namespaces were based on subsystems of the program: g_ for sgame, cg_ for cgame, r_ for renderer, sv_ for server... Only com_/common. is a bit unclear to me; I think it's supposed to be a part of the engine that's included in both client and server builds (but some FPS cvars which only affect the client must be there by mistake). I'm not sure if illwieckz considers that prefixing cvars by subsystem is a bad idea in general and wants to reorganize top-level namespaces around different principles? Or that "workarounds" have some very special reason to be an exception?

[illwieckz]

The convention of naming cvar C++ objects the same as the cvar name is nice, so that all references can be searched for with a single string.

There may be a bias from my vim-centered mind but a regex searching for workaround.glDriver.amd.oglp.disableBindlessTexture also matches workaround_glDriver_amd_oglp_disableBindlessTexture.

For renaming the cvars I just did %s/one\([_.\]\)two.three.four/un\1deux\1trois\1quatre/.

Of course an universal _ separator would be easier on that side. I don't really know why the “new cvar naming style” used dots, on that part I'm just sticking to what is already done (like vm.cgame.type etc.). I'm not disliking the dot that much though.

Should workarounds have a top-level namespace? […] Or that "workarounds" have some very special reason to be an exception?

I consider workaround to be an exception. The purpose of them is to be used in test scripts, CI, bug reports giving instructions to third-party software developers (like drivers) to test a specific code path, giving some copy-pastable test commands to a tester, etc. I expect that most usages of such cvars is to be copy pasted commands. The location of them in the source code doesn't matter to me, they targets specific hardware, libraries or operating system components.

For example I would not exclude the possibility of some workaround.alDriver.alsa. or workaround.alHardware. (audio) or a workaround.fs.vfat. (filesystem) cvars. I doubt they will exist in engine are we may not be low-level enough on those topics, but for having written some radio-broadcast-purposed software in my life, I know that even some sound cards may require similar workarounds as the ones we implemented for graphics card, for example some sound cards may claim to support 48KHz sampling but only produce garbage if used, and an audio software would have to detect such card and force 44KHz and ignore the advertised capability if the default sampling of the software is 48KHz. This is very similar to the Nvidia workaround we have with a driver for specific cards falsely claiming to support ARB_texture_gather and failing if attempting to make use of it. Even for filesystems, people doing portable installations of the game on USB sticks may require some workarounds. For example: what if we attempt to create the engine IPC pipe on FAT32? We may also have a workaround. cvar to disable the recently implemented case-independent search if we want to test direct access to the file system for whatever debugging purpose.

Also having a workaround namespace makes it very easy to get a list of those workaround this way:

/listCvars workaround

This makes me think it would be nice if listCvars would print things in alphabetical order…

[illwieckz]

Also I want to say such cvars may be renamed in the future. For example if the bindless texture bug in OGLP is fixed one day, we may rename the related workaround cvar to mention the version. I don't consider those names to be written in stone. All we need is to have something to copy-paste for testing, and I prefer such cvar names to be verbose. For workaround cvars I consider verbosity a feature.

[illwieckz]

• workaround.glExtension.missingArbFbo.useExtFbo: missingArbFbo doesn't make sense as a hierarchical element.

Well, I mainly named the cvars this way: workaround.topic.whenTheWorkaroundIsApplicable.enableWorkaround.

We can continue the discussion later and rename the cvars later, especially since they're not meant to be used by users and to be set in presets. I prefer to not wait more for merging this to not delay the implementation of other patches above this code. It looks like the logic itself is ready and has no comment to receive anymore.

So let's do it.

[slipher]

Of course an universal _ separator would be easier on that side. I don't really know why the “new cvar naming style” used dots, on that part I'm just sticking to what is already done (like vm.cgame.type etc.). I'm not disliking the dot that much though.

I guess we can split that up into question (2a) r vs. renderer and (2b) _ vs. ..

I consider workaround to be an exception. The purpose of them is to be used in test scripts, CI, bug reports giving instructions to third-party software developers (like drivers) to test a specific code path, giving some copy-pastable test commands to a tester, etc. I expect that most usages of such cvars is to be copy pasted commands.

What does any of that have to do with whether workaround is a top-level namespace?

they targets specific hardware, libraries or operating system components.

I can see this argument somewhat. Wanting to organize by an external subsystem rather than one of the game's subsystems.

Also having a workaround namespace makes it very easy to get a list of those workaround

If "workaround" is somewhere in the middle of the names, you can do listCvars *workaround*.