Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a namespace for all GL extension test cvars #1236

Open
slipher opened this issue Aug 10, 2024 · 3 comments
Open

Use a namespace for all GL extension test cvars #1236

slipher opened this issue Aug 10, 2024 · 3 comments

Comments

@slipher
Copy link
Member

slipher commented Aug 10, 2024

I propose moving all of the cvars that cause the renderer to assume failure when testing for a certain optional OpenGL feature under the r_ext_ prefix. Currently they are divided between r_ext_ and r_arb_. As a user I don't care about which OpenGL committee process the feature was proposed under, or whatever the stupid prefix means. There is even an extension that is supposedly available under three different names: ARB_framebuffer_object, EXT_framebuffer_object, and OES_framebuffer_object. So the ARB or EXT prefix doesn't seem like a reliable or informative way of grouping these; the feature test cvars are all the same sort of thing. Let's make ext mean extension test rather than a part of the verbatim extension identifier.

We should also consider removing the subsequent underscores, to make the names respect the new convention of underscore as namespace.

@illwieckz
Copy link
Member

cvar, extension and feature mapping

I propose moving all of the cvars that cause the renderer to assume failure when testing for a certain optional OpenGL feature under the r_ext_ prefix.

I disagree as I prefer the cvar to exactly match an extension, no less and no more, and not be about a vague feature that can be achieved with different combinations of various extensions.

EXT or ARB is part of the extension name and cannot be ignored

As a user I don't care about which OpenGL committee process the feature was proposed under, or whatever the stupid prefix means.

As a first thing to say, r_ext_ is not a prefix, the prefix is r_, ext_ is part of the extension name, the underscore after ext is not a separator (I elaborate further below).

I also disagree about using r_ext_ prefix instead of r_arb_ because this is wrong: ARB_framebuffer_object, and EXT_framebuffer_object are two different extensions without the same amount of features.

Basically EXT_framebuffer_object is an older subset of ARB_framebuffer_object and then naming ARB_framebuffer_object as ext_framebuffer_object would be wrong and confusing.

For details, ARB_framebuffer_object also includes EXT_framebuffer_blit but EXT_framebuffer_object doesn't.

Common prefix for GL extension cvars

I don't disagree with the idea of having a common prefix for GL extension cvars, but ext_ and arb_ are not namespaces, they are part of the full name of the extension.

Namespaces and conventions

We should also consider removing the subsequent underscores, to make the names respect the new convention of underscore as namespace.

There is no new conventions of underscore as a name space.

There are two conventions:

Legacy convention, a 2-level tree:

  • <single-letter-namespace>_<name> like r_dynamicLights

The r_ext_framebuffer_object cvar is read as (r) → (ext_framebuffer_object) 1 level tree, not as (r) → (ext) → (framebuffer) → (object) 4-level tree.

New convention, a N-level tree:

  • <namespace>.<sub-namespace>[.<sub-namespaces…].<name> like common.framerate.max

It happens that a mistake has been done once in game with some bots cvar being N-level trees with _ as a separator, but it would be really good to not continue on that road, to not introduce more cvars using that, and if possible to rename at some point the few having done the mistake. It's better to not add more and more conventions, being de-facto or not.

Some example of cvar naming scheme

We can imagine something like renderer.glExt.ARB_framebuffer_object and renderer.glExt.EXT_framebuffer_object for the two different extensions, it would be both new-style dot-separated tree cvar and provide a common prefix for GL extension cvars.

@illwieckz
Copy link
Member

illwieckz commented Aug 13, 2024

On a related topic, I'm in favor of the convention that every LOAD_EXTENSION that doesn't load a required extension use LOAD_EXTENSION_WITH_TEST and have a related cvar as test, to make it easier to test the alternative code path, and to make such testing more portable (relying less on driver features to disable extensions).

@slipher
Copy link
Member Author

slipher commented Aug 13, 2024

What is new is that underscore treatment as namespace is officially codified in the engine in some way, namely with tab completion.

The "new convention" with dots doesn't seem like an improvement; we should probably abandon that... common.framerate.max for example is a shit name. It is far more verbose than, let's say, cl_maxfps. I don't want to type something that long. Also the "common" part doesn't make any sense, what is that even supposed to mean? The cvar is used in clients but not the dedicated server, so cl/client seems like a logical grouping. I guess common was only used because it was called com_maxfps before.

If we want a single convention, moving the ones with dots back to underscores would be easier. Migrating all cvars to dots would be a tremendous waste of time that just makes the cvar names needlessly verbose. It seems the only advantage of dots is that you could stick these extension names that happen to contain underscores in them.

The point about wanting to retain the EXT_ or ARB_ prefixes is fair. Taking that into account I would suggest names like r_ext_arbFramebufferObject and r_ext_extFramebufferObject. Simply removing the underscores shouldn't be too onerous for someone looking up an extension.

@slipher slipher changed the title Use r_ext namespace for all GL extension test cvars Use a namespace for all GL extension test cvars Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants