Group tags that don't correspond to actual enum groups

[NogginBops]

There is a lot of different parameters that are marked with group attributes that are not actually enum groups, but some other kind of metadata.
Some examples includes:

<!--from glGetListParameterfvSGIX-->
<param group="CheckedFloat32" len="COMPSIZE(pname)"><ptype>GLfloat</ptype> *<name>params</name></param>

<!--from glGetMultiTexImageEXT-->
<param group="CheckedInt32"><ptype>GLint</ptype> <name>level</name></param>

<!--from glGetTextureImageEXT-->
<param class="texture" group="Texture"><ptype>GLuint</ptype> <name>texture</name></param>

These do not denote enum groups like the majority of groups, but instead indicate some other out-of-band information about these parameters.

In total I'm pretty sure that atm there are 1076 functions, with a total of 1849 parameters that are tagged with an group but doesn't have the GLenum type.

Unfortunately some of these parameters are actually enums, but their type doesn't reflect this. See #516 for example.
There are 20 functions that fit this description: (written as <type> <parameter name>: <group>)

glGetDebugMessageLogAMD:
  GLuint* severities: DebugSeverity
glGetFramebufferPixelLocalStorageSizeEXT:
  GLuint target: FramebufferTarget
glMultiTexImage1DEXT:
  GLint internalformat: InternalFormat
glMultiTexImage2DEXT:
  GLint internalformat: InternalFormat
glMultiTexImage3DEXT:
  GLint internalformat: InternalFormat
glQueryObjectParameteruiAMD:
  GLuint param: OcclusionQueryEventMaskAMD
glTexImage1D:
  GLint internalformat: InternalFormat
glTexImage2D:
  GLint internalformat: InternalFormat
glTexImage2DMultisampleCoverageNV:
  GLint internalFormat: InternalFormat
glTexImage3D:
  GLint internalformat: InternalFormat
glTexImage3DMultisampleCoverageNV:
  GLint internalFormat: InternalFormat
glTexStorageAttribs2DEXT:
  GLint* attrib_list: TexStorageAttribs
glTexStorageAttribs3DEXT:
  GLint* attrib_list: TexStorageAttribs
glTextureImage1DEXT:
  GLint internalformat: InternalFormat
glTextureImage2DEXT:
  GLint internalformat: InternalFormat
glTextureImage2DMultisampleCoverageNV:
  GLint internalFormat: InternalFormat
glTextureImage2DMultisampleNV:
  GLint internalFormat: InternalFormat
glTextureImage3DEXT:
  GLint internalformat: InternalFormat
glTextureImage3DMultisampleCoverageNV:
  GLint internalFormat: InternalFormat
glTextureImage3DMultisampleNV:
  GLint internalFormat: InternalFormat

There are also a lot of places where parameters of type GLboolean are marked with the group Boolean, this is technically a valid group as there are entries inside of it, but it would make no sense to change GLboolean to GLenum (and it would be breaking too).

It would be nice if something could be done about this.

I would like to propose the following course of action.

1. Fix the 20 functions so that they take GLenum instead of their current GLint/GLuint (uint might be a problem here).
2. Change the attribute name on the rest of the parameters from group to something like kind.
3. Now all group attributes indicate valid enums, and this way we can work towards complete correctness for enum groups.

This is working towards having robust enum groups that were discussed in #481, this can be used as somewhat of a discussion forum for figuring out how these changes should be done.

Here I'm attaching a file containing a complete breakdown of the functions that either are valid enums but do not sure GLenum or have a group attribute that is not a valid enum (with the Boolean group filtered out).

misplaced_groups.txt

[pdaniell-nv]

@Perksey do you have thoughts on this? I think you're probably the main expert on all things enum group related.

[Perksey]

Thanks for filing this, I'll have a look through the attached file tomorrow and figure out how many groups we're talking, I agree on the premise but want to make sure we can get all the information that the groups convey elsewhere in the spec!

[Perksey]

Valid enums but invalid type (20 functions)

Disagree on changes here. While yes these parameters take enums but do not have the type defined as such, changes of the types here would propagate to the header files and I don't think changing parameter types in the headers for the sake of group correctness is worth it.

Invalid groups (840 functions)

  GLuint texture: Texture
  GLuint* fences: FenceNV

Instances of these are superseded by the kind attribute, happy for these group attributes to be removed.

  GLint drawbuffer: DrawBufferName

This should be a class attribute (class="draw buffer") to be honest if we wanted to keep this information, but in any case happy for these group attributes to be removed.

  GLubyte* return value: String

Support removal. Technically there's no way for a downstream consumer to know this is a string (see #363) but in practice I think XML users have all the info they need.

For everything else, there's nothing here that jumps out at me an inherently dangerous/wrong to remove so I'm fine with this. It's a big change, but I think where we've landed in #481 is that big changes are fine considering we don't have a stable baseline today.

TL;DR: happy with everything in the "Invalid groups", but would rather no changes to "Valid enums but invalid types".

[SunSerega]

Valid enums but invalid type

Currently, there is no good way to automatically detect them when parsing XML, because of other uses of the group attribute, like group=Texture.
If the resolution to this issue is to deprecate and then remove all uses of the group attribute, other than enum groupings, that's also fine.
But at the same time, a fix for lack of information is more robust if done in both directions, as it can then be tested by each binding in their own way when re-parsing XML (for instance I output this kind of info to the .log file, so I see it in git changes after pulling this repo).

And while I understand how everyone values compatibility here, it's not right how old typo's aren't just unfixable in old bindings, but also propagate to new ones unless fixed in each binding manually.

I've thought of this a few times since #363, and, if official bindings cannot be touched, then maybe these typos should be fixed in an XML-only way? (note replace=)

        <command>
            <proto group="String">const <ptype replace="GLchar">GLubyte</ptype> *<name>glGetString</name></proto>
            <param group="StringName"><ptype>GLenum</ptype> <name>name</name></param>
            <glx type="single" opcode="129"/>
        </command>

Instances of these are superseded by the kind attribute

The 2 examples you've shown should only have the class attribute instead.

All group=Texture already have it - since #428. All that is left is to remove redundant (and probably unusable since enum groupings) group=Texture.
Things like group=FenceNV were described in the header of #428, which everyone (including me) forgot about.

[NogginBops]

Changing argument types from GLint to GLenum should not be binary or source breaking as they resolve to the same type. Though it seems like there are some places where GLint unfortunately is the "correct" type, see #283 (comment).

This is unfortunate as that means we can't change signature from GLint to GLenum.
Unless you think it's ok to change the type to GLenum as it still allows users to pass in numbers as the arguments. @oddhack
Guessing there is some type of verification thing that would be affected by the change? as it wouldn't be a binary nor source breaking change.

But as @SunSerega said, we could add some information to the xml which allows users to get this information. Not entirely convinced that a replace="GLchar" attribute is the solution to go for. For the case of glTexImage2D we would potentially need to have it conditioned on the GL version.

[Perksey]

If the resolution to this issue is to deprecate and then remove all uses of the group attribute, other than enum groupings, that's also fine.

I believe this is the way forward yes.

an XML-only way? (note replace=)

I don't think this adds value and instead just clutters the specification. Don't get me wrong, it'd be useful, but for #363 specifically that function is the exception not the rule. Likewise, modifying the function signature after it has been specified, approved, and shipped should once again be the exception not the rule.

---

Flipping the question a bit here, why would we make it illegal for enum values to be passed to non-enum parameters? At the end of the day, all of these enums are just macros for specific numbers and all of these parameters accept numbers. There are also some cases where some parameters accept both enumerated values and non-enumerated values, or cases where the enumerated value is actually just a well-known constant. I don't think imposing restrictions makes sense.

And in any case, a header change like that will likely have to go the Working Group, who will almost certainly deem it unnecessary. Moreover, I wouldn't want to make any static analyzers throw any new warnings when downstream users have done nothing but update their headers (e.g. think about casts to the parameter type).

I think the impact of saying "the group attribute means that parameter, whatever type it may be, accepts enums from this group" is far less than saying "the group attribute means that parameter must be a GLenum parameter and accepts enums from this group".

Finally, I don't think augmentation of the XML specification for downstream consumers of it should impact the headers in any way. Ultimately, all of this is irrelevant to Khronos' needs and if we do want to augment the specification with extra data, it should be done in a way that does not impact Khronos.

[NogginBops]

the group attribute means that parameter, whatever type it may be, accepts enums from this group

I'm fine with this.

So remove if I've got the sentiment right:

• Remove/rename the group tags that are not referring to enums.
• Allow group tags on parameters that are not GLenum, which would mean, "this parameter takes all the values from the enum group, but also possibly some other values".

[Perksey]

Sounds good to me!