Rename group attribute for non GLenum parameters not corresponding to any enum group

[NogginBops]

Fixes #517

I've used the word kind here to indicate that the label indicates the kind if value the parameter is (eg ClampedFloat32 or CoordF). I was thinking about other words to use such as "attrib", "category", "variety", "set" or "label", but personally it seemed like "kind" was the best, most descriptive name.

@Perksey @SunSerega comments?

[SunSerega]

The changes are definitely better than keeping it as is. But I have my nitpicks, mostly minor improvements that are better done now before the fate of kind starts to feel sealed:

(no more)

---

Long term (probably other PR's)

- [ ] Some of `CheckedFloat32` seems to be more of a "checked and clamped", rather than "checked with errors", as it sounds to me. But it's better to leave them for the next PR because that change will be significant on its own. - [ ] `kind`'s mentioned in the "Extension" table from #428, like `Path`, `FenceNV`, etc. should be classes. Well, I was planning to make classes for them myself... I can always make another pull after this one. - [ ] What was `PathElement` describes a more general pattern: The type of values in the array is determined by another parameter. This can also be for instance found in `glColorPointer`. Similar case with the `pname` parameter. For instance, the `glTex[,ture]Parameteri` can take many different groups depending on `pname`. Same but simpler with `SpriteModeSGIX` from #546. - [ ] Kinds for `*Uniformui64v*` and `*UniformHandleui64v*`?[^[.]](https://github.com/KhronosGroup/OpenGL-Registry/pull/534#issuecomment-1337264311) ---

Completed

- [x] I don't think there is a point in having both `class` and `kind` attributes on the same parameter, like in the case of `texture`, `framebuffer`, etc. The point of `kind` I can see (besides removing the wrong `group` uses) is to indicate some additional info, which is not necessary in any case but is sometimes good for improving bindings. When I was adding `class=texture` I looked at all `group=texture`, but didn't delete them, because I didn't want to touch them yet. - [x] `kind=handleARB` doesn't add any info, because it only exists on parameters of the type `GLhandleARB`. There are a few cases like the return value of `glGetImageHandleARB`: `GLuint64` is used instead of `GLhandleARB`. If you consider them, it makes sense to use common `kind` or even `class` to include them. On the other hand, it seems to me that everything with `GLhandleARB` is outdated. So, I'm not sure if there is a point in adding/keeping `kind`/`class` attributes. - [x] `Half16NV` is also a copy of the parameter type. But unlike `handleARB` - I don't see the reason to keep it. Same for `kind=Boolean`. And that one case with `BooleanPointer` (which, if anything, is a `BooleanPointerPointer`). - [x] There are multiple of `kind=Clamped...`, but they all mean the same. I think combining them into something like `Clamped[0; 1]` is better. - [x] Same for `kind=Color...` and `kind=Coord...`. Type is already defined on the parameter, so it's better to combine it into something like `kind=Color` or `kind=ColorValue`. (and a separate one for `Coord...`) - [x] `DrawBufferName` is not a name, but rather an index (0..15) - [x] `BufferOffset` and `BufferOffsetARB`, as well as `BufferSize` and `BufferSizeARB`, are the same. - [x] `PathElement` describes something more general: The type of values in the array is determined by another parameter. This pattern can be found in many other places, including core spec. And I think I saw a few other kinds with the same meaning, but can't find them now... Regardless, it is not very useful to mark this relation using the `kind` attribute, because there is no info on what parameter describes type. So I think this `kind` should be removed, and maybe at some point replaced with a different attribute. - [x] `ProgramCharacterNV`, as far as I can see is not a program-specific thing, but rather a regular `char`, disguised as `GLubyte`, just like in `glGetString`, so it should also be `kind=string`. Or maybe change both to `kind=char`, for consistency with other kinds? - [x] `kind=LineStipple` exists only once and doesn't do anything special. Its parameter can accept any 16-bit number. - [x] Same for `SampleMaskNV`. - [x] `ReplacementCodeSUN` actually is a group. In #520 I've merged it with `TriangleListSUN`. - [x] I'm not sure if `cl_context`, `cl_event`, and `vdpauSurfaceNV` should be `kind`'s, `class`'s, or neither... They definitely have their own namespace. But also maybe they shouldn't be mixed with other classes, which are defined only in and for OpenGL. - [x] It seems `PathCommand` should instead be a group `PathCoordType`. Though I don't have a thorough understanding of [this](https://registry.khronos.org/OpenGL/extensions/NV/NV_path_rendering.txt), so maybe it's better if someone from NV confirms. @pdaniell-nv? - [x] `MaskedColorIndexValue`, `MaskedStencilValue`, and `StencilValue`. - [x] Floating-point color values (like in `glBlendColor`) are also clamped. `kind="Clamped01,Color"`? Tbh, I expected there to be many more cases where the parameter has multiple kinds at the same time... - [x] How about `` somewhere at the top, to make them more well-defined? Unlike classes, which are solely defined by their uses and being a namespace, each unique `kind` represents a new parameter usage pattern. And that pattern can't always be inferred from uses. For instance, all the `Clamped...`, as far as I can see, are clamped between 0 and 1. (P.S. Wrong, some are [-1; +1]) And the process of making descriptions will probably uncover more things to improve. ---

Non-kind_attribute changes

(this is all good, noting here just to keep track) - Added all the metadata to the functions of the extension `NV_path_rendering`. - And in the process, group `InstancedPathCoverMode` was split off from `PathCoverMode`. `InstancedPathCoverMode` only differs by additionally having a `BOUNDING_BOX_OF_BOUNDING_BOXES` enum. - Removed groups: - Dup of info of parameter type: - `handleARB`: Dup of `GLhandleARB`. - `Half16NV`: Dup of `GLhalfNV`. - `Boolean` (and `BooleanPointer`): Dup of `GLboolean`. - `cl_context`: Dup of `struct _cl_context`. - `cl_event`: Dup of `struct _cl_event`. - `vdpauSurfaceNV`: Dup of `GLvdpauSurfaceNV`. - Dup of info of `class` attribute: - `Texture`: Dup of `texture`. - `List`: Dup of `display list`. - `sync`: Dup of `sync`. - `Framebuffer`: Dup of `framebuffer`. - `Renderbuffer`: Dup of `renderbuffer` - `PathElement`: Not useful as it was, to be reworked. - `LineStipple` and `SampleMaskNV`: Didn't mean anything. - `glClearNamedBufferSubData`: Added `len="COMPSIZE(format,type)"`. - `PathCommand` renamed to `PathCoordType`. ---

[oddhack]

Also needs to be added to xml/readme.tex, the schema description document.

[pdaniell-nv]

It appears this isn't ready yet for approval. Let me know if you think otherwise.

[NogginBops]

Some thoughts I had while working on fixing these. @SunSerega (will fix the missed color stuff!)

I'm not sure if cl_context, cl_event, and vdpauSurfaceNV should be kind's, class's, or neither... They definitely have their own namespace. But also maybe they shouldn't be mixed with other classes, which are defined only in- and for OpenGL.

The cl_context and cl_event info is available as struct _cl_context and struct _cl_event so they should not be kinds imo. The same is true for vdpauSurfaceNV that is available as GLvdpauSurfaceNV.

ProgramCharacterNV, as far as I can see is not a program-specific thing, but rather a regular char, disguised as GLubyte, just like in glGetString, so it should also be kind=string. Or maybe change both to kind=char, for consistency with other kinds?

Why is char more consistent than String?

Same for MaskedColorIndexValue..., MaskedStencilValue, and StencilValue.

I'm not sure what you are referring to in regards to MaskedStencilValue, and StencilValue.

How about <kind name="..." description="..."/> somewhere at the top, to make them more well-defined? Unlike classes, which are solely defined by their uses and being a namespace, each unique kind defines a new parameter usage pattern. And that pattern can't always be inferred from uses. For instance all the Clamped..., as far as I can see, are clamped between 0 and 1. And the process of making descriptions will probably uncover more things to improve.

I like this idea, though it seems like quite a lot of work.

[SunSerega]

The cl_context and cl_event info is available as

Agree. But, just a reminder, so far you removed vdpauSurfaceNV and kept the other 2.

Why is char more consistent than String?

I was sure kind is most often relative to type under the pointer. Like here:

            <param kind="CheckedInt32" len="COMPSIZE(pname)">const <ptype>GLint</ptype> *<name>params</name></param>

It is every referenced value being checked, not the pointer itself.

Though now that I look at it, there are also lots of cases of:

            <param kind="Color" len="3">const <ptype>GLbyte</ptype> *<name>v</name></param>

The reason I proposed ColorValue instead of Color is because parameters of that kind come in groups, each specifying one of RGB/RGBA values. Similar logic for Coord. (I don't mind if you keep it as kind=Color, but it would be great if you'd also comment on this)

But in this example, you can say it's not ColorValue, it's the whole parameter being a color.

---

On the other hand:

            <param kind="CertainKind" len="3"><ptype>ItemType</ptype> ** <name>v</name></param>

Note: Double pointer. Is CertainKind a kind of ItemType? Of the array of ItemType? Of the jagged array of ItemType? Well, maybe it would make sense to specify it in the kind's description. That is also fine since every kind is its own pattern.

---

And on the... Third hand - it is important to return from abstractions to actual use. And I think there (even in the case of the Color kind) if the binding is defining a mechanism to handle the kind, that mechanism will probably be applied per value. At least I struggle to find counterexamples.

I'm not sure what you are referring to in regards to MaskedStencilValue, and StencilValue.

I remember, from a quick (not thorough) look at spec - I found that MaskedStencilValue and StencilValue mean the same, i.e. the uses of StencilValue are "masked" in all the same way. But these kinds sure are pretty convoluted and confusing.

---

like this idea, though it seems like quite a lot of work.

How about this: You add the description to simple kinds, like String, Color, etc. And, as for this pull, kinds that are not documented (like MaskedStencilValue) are thought of as unsupported (and expected to be heavily changed). Then unsupported kinds can be incrementally improved. I will also add a warning to the log of my data scrappers for these kinds, to not forget.

---

P.S. You should probably merge the official main branch into this one because there are conflicts.

[SunSerega]

Also finished updating my initial review and previous comment, so it's easier to see what's left.

[NogginBops]

Same for MaskedColorIndexValue, MaskedStencilValue, and StencilValue.

MaskedStencilValue seems to be used with GLuint while StencilValue seems to be used for GLint types. There probably isn't that much more going on there, I can make them the same.

But I'm unsure how MaskedColorIndexValue relates to this? It is used in glIndexMask and glClearIndex. For glIndexMask I think the usage is similar, but I don't understand what is going on with glClearIndex. In glIndexMask it's a proper mask, but in glClearIndex it is an index?? (and it's also a GLfloat?)

[SunSerega]

but in glClearIndex it is an index?? (and it's also a GLfloat?)

Yeah, something crazy is happening there. I remember reading about GLfloat being converted to fixed, then int, and then also getting masked. At least at the moment, the part where it's masked seemed very similar to StencilValue, IIRC because a few lowest bits are taken. But I didn't look into it thoroughly.

[NogginBops]

@SunSerega I've made an initial <kinds> tag that lists and describes some kinds. The don't need to be final names, and I haven't changed all of the places where they are used. It's more to give a general idea of how the tags could look.

[SunSerega]

Looks mostly good so far, updated the review again.

[SunSerega]

Trying to apply this to my bindings. In the process found that SampleMaskNV also isn't a group. Though, I'm not sure if it should be a kind, because it accepts any bit combination.

And there doesn't seem to be much of missing descriptions, all cases left are already covered by other points, like Path (which I'll make into a class later) Checked... (which IMO needs separate PR, because there is a lot to check) and ClampedFloat32. Here is the full list of kinds without descriptions, if interested:

• ClampedFloat32
• ClampedFloat64
• ClampedStencilValue
• CheckedFloat32
• CheckedInt32
• Path
• FenceNV

So yes, only a few Clamped... kinds left for this PR. But also don't forget registry.rnc and readme.tex.

[SunSerega]

You added Clamp01 instead of Clamped01... And if you decided to fix floating-point kind=Color without Clamped01 - there are a few more.

[SunSerega]

Also, it's great you also look at unrelated groups, but I think you should fix them in separate, smaller PR. This one is huge on its own. And cases where you removed ColorPointerType should probably still have a group, just a different one.

[NogginBops]

@SunSerega I've done a, from what I can tell, complete marking of color parameters with kind=Color. During that I found glClearAccum that clamps values to the range [-1, 1], I gave it a temporary name but I would like to come up with a better one if possible.

[SunSerega]

I would like to come up with a better one if possible

In the first place, there is no restriction on characters, except , (coma). So how about Clamped 0..1 and Clamped -1..+1? And maybe then change the separator to ; (Clamped 0..1; Color), for readability.

[Perksey]

For list-like things we tend to use commas instead of semicolons. But it’s worth noting this is already too complicated to be maintainable by vendors or Khronos, so given this consider how much augmentation you want to put here vs how much you want to put in your own libraries.

[NogginBops]

We don't expect Khronos nor vendors will use or update these. As these where groups initially we expect the same policy as with groups, "as long as it doesn't affect the gl.h header and there is a consensus".

And I think these could enable some interesting generation options once we have them complete. And as the specification doesn't change much these days a lot of the work put in now won't need to be monitored and maintained for changes. They will pretty much remain as they are.

[NogginBops]

In the first place, there is no restriction on characters, except , (coma). So how about Clamped 0..1 and Clamped -1..+1?

Looks funky but I guess we don't need to limit ourselves. An issue with separating numbers with .. is that parsing floats is harder that way. But putting in floating point number into the clamped kind will likely not happen, and is likely going overboard.

I'm fine with Clamped -1..1 and Clamped 0..1, but it would also be worth to consider Clamped[0 1] and Clamped[-1 1] as well. What do you think @SunSerega ?

[SunSerega]

I don't think float parsing is a concern. It would only make sense if there were many kinds of Clamped kind. However something like Clamped[-1/+1] or Clamped[-1:+1] would be more readable than a..b. And also looks great with commas as separators: Clamped[-1:+1], Color.

[Perksey]

We don't expect Khronos nor vendors will use or update these.

Ok fair enough, figured I'd bring it up given a lot of the design behind the current group representation was to make it as easy as possible for vendors to keep up to date (we've already seen vendors such as ARM and Qualcomm using/updating the groups now it's easy to do so).

[pdaniell-nv]

What's the status of this PR? It's not clear to me if there are more changes pending or it's ready for final review and approval.

[NogginBops]

There are more changes pending. Will try finish this PR as soon as I have time.

[pdaniell-nv]

There are more changes pending. Will try finish this PR as soon as I have time.

Sure, no worries. I've marked the PR as draft, which you can reset once everything is in. Thanks.

[SunSerega]

@NogginBops comma is one symbol that really shouldn't be used in a kind name because it's a name separator. I would still root for : (and without space after it) because it better shows the connection between min and max numbers.

[NogginBops]

I think it's worth to consider Clamp[0; 1] as that would retain a comma, while keeping it a separate character. For some reason I don't want to go for : as the separator but if need be we can go for it.

[SunSerega]

Fix the last remaining Clamped

Still a few Clamp01 (without "ed") from earlier.

[SunSerega]

Also, it's a bad idea to add the SpriteModeSGIX group to glSpriteParameterfSGIX and glSpriteParameterfvSGIX (but fine in the other 2 cases you touched).

For high-level languages, it basically says "this should be replaced with a special type", but also it cannot be the main type created for SpriteModeSGIX, because it holds values represented in memory as GLint. So things passed to glSpriteParameterfvSGIX would need to be stored in another type for SpriteModeSGIX, which represents them as floats.

This unnecessarily complicates binding generators. But it doesn't have to, because there is no real reason to pass enum values converted to floats when there are functions doing same thing but taking ints.

[NogginBops]

Also, it's a bad idea to add the SpriteModeSGIX group to glSpriteParameterfSGIX and glSpriteParameterfvSGIX (but fine in the other 2 cases you touched).

Why is this a bad idea, the extension document says those are the only accepted values to these functions. I'm fine with removing them though, if the argument is that another extension might add functionality to the function.

[SunSerega]

the extension document says those are the only accepted values to these functions

No, the values from SpriteModeSGIX are integers. Because that's the default for enum values. What glSpriteParameterfvSGIX accepts are those values converted to floats. After conversion, they have different representations in memory.

Again, there is no critical problem with that, but there is no point in keeping that information in XML, because while the extension says that only integers can be passed to the last parameter of all 4 (relevant here) functions - only the functions accepting integers are meant to be used. And the other 2 are reserved for the future. Even though they technically already work.

---

Well, to be frank, just not adding group information also doesn't feel like the best solution. But it's a similar case to what PathElement was. (look in the "Completed" section of my review) It's a part of XML that doesn't do any good. Or in the case of SpriteModeSGIX on floats - does more harm by polluting with irrelevant info, which is then really awkward to handle in binding generators. And like PathElement - this is a more general case, this time with the pname parameter influencing the group of another parameter. If handled properly, it would be much different.

For an example far better than glSpriteParameter[f,i][,v]SGIX - look at glTex[,ture]Parameteri: Depending on the pname parameter, the param can take, besides just a naked integer, the groups:

• TextureMinFilter
• TextureMagFilter
• TextureWrapMode
• DepthStencilTextureMode
• TextureSwizzle
• TextureCompareMode
• MapTextureFormatINTEL
• FoveationConfigBitQCOM

Most of these groups are defined but unused in XML (they weren't deleted in #520, but caused trouble making that PR). I'm still in the stage of just forming ideas on how to nicely handle it in XML.

---

And, a bit of step back and look... I said already, and I'll say again: I like that you found and made SpriteModeSGIX and also added to glSpriteParameteri[,v]SGIX functions. But it would be much better as a separate PR. If you don't want to flood this repo with many small PRs - you can make 1 branch for multiple small unrelated fixes that you find along something big like this. But, at least for me, it feels terrible to take discussion space of the kind attribute and put here something unrelated. Because the kind attribute is big on itself, there are already many changes and things to talk about. That may also be the reason it was hard to understand my initial explanation (why SpriteModeSGIX on floats bad) - because I tried to make it short. And I still thought but not added some parts/POVs of this new explanation, which might not be as important.

[NogginBops]

I can definitely move the SpriteModeSGIX enum change out of this PR, I just added it as I was verifying kind= attributes. Are there any other such changes that you feel would be better suited for another PR? @SunSerega Thinking I'm going to make a kind of "Enum fixes found while making #534" PR.

[SunSerega]

Yeah, also note this. Every other change not related to the kind attribute is trivial, so it's fine just to leave them here.

[SunSerega]

Changed Clamped[0, 1] to Clamped[0; 1]

You forgot to rename Clamped[-1, 1] into Clamped[-1; 1].

[NogginBops]

@SunSerega I removed the ColorPointerType changes and made reverted the SpriteModeSGIX changes and put them into #546 .

[SunSerega]

Finished checking the changes of the whole PR again. Added some info to review and made my bindings generate this file, listing cases where the parameter should probably be marked as clamped because it's a floating-point color value. Though I didn't make it check all the floating point types. Only types which were marked as clamped elsewhere: GLfixed GLfloat GLdouble GLclampf GLclampd

[NogginBops]

I've gone through the list your have provided. Only one of the functions (glMinSampleShading) should have the clamping. I have provided links to the relevant parts of the spec in the following file.

[SunSerega]

Hmm, it's weird. #549

[NogginBops]

@SunSerega

SampleMaskNV also isn't a group. Though it probably shouldn't be a kind, because it accepts any bit combination.

I think the type GLbitmask describes this situation exactly, no need for kind or group tags.

[NogginBops]

I have tried updating the registry.rnc and readme.tex files, but I don't really have any way to verify if I've done so correctly, so would be good if someone could take a look and see if there is anything that needs to be changed.

[SunSerega]

I like the idea of Matrix kinds: It adds value to XML because it groups 2 kinds of functions, glProgramUniformMatrix*v and glUniformMatrix*v, describing how their input can be represented as a common struct. I think the same idea would be as useful for vectors. Here is a config I have in my bindings:

# glUniform[%c:1,2,3,4%][%t:f,d,i,ui%]v
# glUniform[%c:1,2,3,4%][%t:      ui%]vEXT
# glUniform[%c:1,2,3,4%][%t:f,  i   %]vARB
!possible_par_types
 *  | * | +var Vec{%c%}{%t%}    |

(This isn't comprehensive: At the very least, I'm missing all the glProgramUniform*v's)

---

Also, I searched for UniformMatrix and there seem to be a few you missed:

• glProgramUniformMatrix2dv
• glProgramUniformMatrix2dvEXT
• glProgramUniformMatrix3x2fvEXT
• glUniformMatrix2dv

[NogginBops]

I've added kind=VectorN now, it's added for all of the uniform functions but I didn't go through any other functions to add the tag there, but that is also something that could be done.

[SunSerega]

Well, there are also glGetUniform*, which don't fit (they can be used with any size of vectors) And there are a few exotic ones: *Uniformui64v* and *UniformHandleui64v*, which, I think, only accept individual values, not vectors.

But anyway, I think it's better to now finish up Clamp01 andSampleMaskNV, and un-draft this pull. IMO there is nothing left that needed to be done before the first version of the kind attribute. The rest can be done in separate incremental pulls.

[NogginBops]

I've fixed the Clamp01 and SampleMaskNV things.

The functions glUniformui64vNV, glProgramUniformui64vNV, glUniformHandleui64vARB, glUniformHandleui64vIMG, glUniformHandleui64vNV, glProgramUniformHandleui64vARB, glProgramUniformHandleui64vIMG, and glProgramUniformHandleui64vNV

all take special values such as buffer addresses or bindless texture handles. I feel like kind=Vector1 would be the wrong kind to put here. I would suggest we add something like kind=BindlessTextureHandle (or some other name) in a future PR.

[NogginBops]

Is there anything more to do or should I make this ready for review?

[pdaniell-nv]

@Perksey would you like to give this another review?

[NogginBops]

I just realized I need to remove the Clamped[0; 1] on glClearColor, will do that soon.

[NogginBops]

Should the kind="Color" attribute be changed to kinds="Color" to indicate that it is a list of kinds instead of a single kind. I initially thought about kind="" as a single value, and it is kind of stuck in my mind, and I'm wondering if changing the plurality of the word used would be more clear?

[Perksey]

I think it's fine as is, as to me it reads as "the parameter is of a kind that is a color and is clamped to a specific range" i.e. they're describing the properties of the kind, they are not kinds themselves.

[pdaniell-nv]

@NogginBops could you resolve the merge conflicts? I assume otherwise this is ready to go in? Thanks.

[pdaniell-nv]

Aside from the note on restricting kind annotations to not contain the ',' separator, I'll need to run this through the schema validator and ensure the generated headers aren't affected before merging, but it seems fine.

@oddhack did you get a chance to run this through the schema validator?

[pdaniell-nv]

@oddhack, this is approved to merge as soon as the merge conflicts are resolved and you're happy with it.