[xml] Fixes for defined but unused enum groups

[SunSerega]

fix #360 I mainly stalled in fixing it because of groups like TextureMinFilter: It can be passed to, for instance, glTextureParameteri, but there is currently no way to denote that in XML - because the same parameter can also take TextureMagFilter, TextureWrapMode, etc., depending on the <pname>. But since that issue, I updated my data scrappers and generators, so it became easier to separate these cases from actual misdefined groups.

I thought of making a bunch of small PRs, but I really don't like how unreadable that looks on git commit graphs. So instead, I separated fixes by one or a few related groups. But I can still break this into many PRs if that would be easier to review.

[NogginBops]

Not entirely sure why MaterialFace got preferred over something like FaceCullMode. glCullFace taking a MaterialFace instead of FaceCullMode doesn't make much sense even though the group contains the same enums

[SunSerega]

why MaterialFace got preferred

Because it was used in almost all cases, where these enums are accepted. This way it's a minimal change. Well, If that's not a good reason, how about renaming it to something like TriangleFace?

[NogginBops]

Sounds like a good idea to me.

[SunSerega]

I've only completely removed 5 groups. Look at => X in commits and at descriptions of them:

GetPixelMap => X

Same as PixelMap, but never used

That is because I've looked at all the enums of these groups and found where they are used, mainly adding the group attribute to <param> or merging them with other groups. And then there is also the TextureMinFilter case I described in the very beginning - these kinds of groups were left untouched, even though they are still not used anywhere in XML. So, at least theoretically, no correct information should be lost in these changes. But I would be glad if anyone could give a fresh look at what exactly is changed.

[Perksey]

@null77 @joakim-arm Can you look over these changes before approval? It'd be good to use known group users by Khronos members/projects as a litmus test for these sorts of breaking group changes in lieu of #481

[pdaniell-nv]

Are we still waiting for approval from @null77 and @joakim-arm?

[Perksey]

Ideally yes. We can continue without their approval, but ultimately this will likely impact them as group users and given they represent Khronos members/projects, I feel like we should solicit their opinion first!

If we haven't heard back in a little bit (say 2 weeks) then I think we should merge.

[joakim-arm]

Looks good, but just one comment:

Removing the group TraceMaskMESA from the bitmask values makes things a bit inconsistent. All other bitmask values are assigned to a group.

@SunSerega, did you add this group back?

[SunSerega]

No, it has an unresolved conversation here. I encourage other reviewers to also look at unresolved conversations. They are what other people found non-trivial and what I left unresolved because I would appreciate more feedback.

---

I personally think these enums should be removed from XML because there is no documentation/functions in this repo, corresponding to them. Seeing how smoothly #523 went, I guess this kind of change isn't seen as breaking, so I'll go ahead and comment these out.

[Perksey]

I personally think these enums should be removed from XML because there is no documentation/functions in this repo, corresponding to them. Seeing how smoothly https://github.com/KhronosGroup/OpenGL-Registry/pull/523 went, I guess this kind of change isn't seen as breaking, so I'll go ahead and comment these out.

I don't agree with this, mainly because:

Possibly fun historical note: early in GL history, when it was still all workstation manufacturers, one of the CAD vendors came up with a "secret" extension. They convinced multiple GL implementers to support it in their drivers, but not to expose it in the GL_EXTENSIONS string. We speculated the purpose was to give them some sort of competitive edge over other CAD vendors, while keeping the driver suppliers from realizing what was going on.

• oddhack

We shouldn't actively remove information from the registry just because it doesn't look to be used. We have 30 years of history to cover here, can you really say that this information wasn't relevant for the entirety of those 30 years? Moreover, it's unknowable what dependency someone will have taken on any part of the registry, so I don't think it's really great to make these sorts of breaks lightly. If a Khronos member approves the removal of this, given groups aren't WG-controlled we can remove it but only if there is reasonable cause & consensus.

[joakim-arm]

Looks good, but just one comment:

I just wanted to clarify that this change doesn't break anything for our internal tool. However, I am afraid I don't have enough background and resources to comment on other possible implications that this change could introduce.

What Perksey says about covering history makes sense to me.

[NogginBops]

I'm fine with these changes too. They are going to cause a lot of breaking changes in bindings, but as we are preparing for a new major version, I personally have no objection.

[oddhack]

I agree with @Perksey - completely removing enums from the registry affects the Historical Record. If an enum isn't being used by an extension but was still reserved that's useful to document, and nobody needs to be actually processing it if it isn't tagged as part of an extension or core API.

[SunSerega]

Ok, then how about defining a special group/attribute for such enums, to show they only exist as a legacy?

Note enums in question aren't even reserved in this case, because they are bitmasks.

[oddhack]

Ok, then how about defining a special group/attribute for such enums, to show they only exist as a legacy?

Note enums in question aren't even reserved in this case, because they are bitmasks.

We don't have any historical preservation need to add new information about these enums. I'm not sure why it even comes up for anyone. If there's no dependency path from an extension requiring these values, they should be ignored by whatever is consuming the XML. If the people concerned with the groups collectively agree there's some value to adding it, though, feel free.

[Perksey]

I don't agree that adding an attribute for these is worth it. It's more maintenance, more work to figure out how to represent such information, and even then there will be exactly one person who will be using such info as it currently stands. Moreover, drawing a "legacy" line is a hard thing to do. As I said in my previous comment, all of this information was relevant at one point, so there's no real reason to do anything other than keep it in the registry. If downstream consumers don't want to care about these, they can filter them out at their own accord.

[NogginBops]

I would also be careful with removing enums, and I don't think there is a need for a special group to mark them. Though I would be open to hear why such a group might be useful if you (@SunSerega) want to argue for it.

[SunSerega]

I think this has grown a bit out of the scope of this pull, so I'll remove changes to TraceMaskMESA and make a separate issue after sorting my thoughts a bit more.

[pdaniell-nv]

As far as I can tell, this PR is complete and awaiting sign-off from the OpenGL/ES working group. We'll approve this tomorrow. Please let me know if anyone has objections to this getting merged, otherwise it will likely be approved for merge tomorrow.

[NogginBops]

I have no complaints for this PR 🙂

[Perksey]

Approved for groups.

[pdaniell-nv]

@oddhack this is approved to merge.