Added len and group attributes to wgl.xml #569

NogginBops · 2023-03-24T00:41:40Z

This PR aims to add some of the metadata present in gl.xml into wgl.xml.

One problem I've found is that if needing to reference enums or enum groups from gl.xml, and I do think the namespaces should be kept separate. Maybe the GL enum values could be added under some tag in the xml? Or prefix the tags with something like GL::TextureTarget for example. None of the solutions stand out as the "correct" one.

NogginBops · 2023-03-24T00:42:29Z

@SunSerega @Perksey might be interested to comment on this PR.

SunSerega · 2023-03-24T09:18:40Z

ColorBuffer and PixelType are already found in gl.xml and have different purposes from groups of the same name you added.

NogginBops · 2023-03-24T14:00:39Z

ColorBuffer and PixelType are already found in gl.xml and have different purposes from groups of the same name you added.

Yeah, when making this PR I was envisioning GL, WGL and GLX as different enum group namespaces. I personally think that they should be considered different namespaces for groups, but there could be arguments for making them the same namespace.

This is not a C problem as these groups are not used in the C headers, so that is at least one things that we don't have to worry about.

xml/wgl.xml

Perksey · 2023-03-24T14:10:28Z

xml/wgl.xml

@@ -72,44 +72,44 @@ Registry at https://github.com/KhronosGroup/OpenGL-Registry
         sometimes reused for other purposes -->



Note this comment (was originally in gl.xml but must've got removed):



This was added in #280 and #291. While thanks to community efforts this situation has got a lot better, I do think it is worth adding a comment in both files that the groups at the very least are exclusively community-supported, and that the OpenGL Working Group has no intention of keeping the groupings up-to-date themselves due to them not being relevant for Khronos uses. It's annoying we missed this comment being removed from gl.xml.

We can add this comment to these files too.

Perksey · 2023-03-24T14:12:40Z

xml/wgl.xml

-        <enum value="0x21A6"        name="WGL_GPU_NUM_SIMD_AMD"/>
-        <enum value="0x21A7"        name="WGL_GPU_NUM_RB_AMD"/>
-        <enum value="0x21A8"        name="WGL_GPU_NUM_SPI_AMD"/>
+        <enum value="0x21A6"        name="WGL_GPU_NUM_SIMD_AMD" group="GPUProperty"/>


What's our ruling on vendor-specific enum groups? In gl.xml this would've likely been GPUPropertyAMD. Related: #481

I'm fine with making this enum vendor specific, but I think most enum groups in gl.xml do not have a vendor postfix, even if it's mostly an extension concept. For this group, if someone else made an extension it would likely not fit into the same enum groups so it might reduce future confusion (but also, gl, wgl, and glx aren't getting that many new changes to them and probably won't).

Then there is also the fact that almost all API in wgl comes in the form of extensions, so basically all groups would need a postfix. But maybe that is what we want.

SunSerega · 2023-03-25T09:00:10Z

Yeah, when making this PR I was envisioning GL, WGL and GLX as different enum group namespaces

Hm... Is it possible for, for instance, a WGL function to accept/return an enum from gl.xml?
If it ever becomes needed in the future, how could it be denoted after separating groups into namespaces based on XML files?

NogginBops · 2023-03-25T13:59:33Z

Yeah, when making this PR I was envisioning GL, WGL and GLX as different enum group namespaces

Hm... Is it possible for, for instance, a WGL function to accept/return an enum from gl.xml? If it ever becomes needed in the future, how could it be denoted after separating groups into namespaces based on XML files?

Yes, this happens and is marked with a few FIXMEs comments in the PR. I guess this can be solved by making them the same namespace and add a WGL prefix to all groups defined in wgl.xml.

To me it seemed more cleaner to have them be different namespaces and figure out a way to cross reference, rather than make them the same namespace but add a prefix to the names in wgl and glx. I initially thought of it like this as it fit better into the current structure of our generator, but it's fine to change that.

There are some downsides to sharing the namespace between files, group changes in gl.xml would then have to be checked against wgl.xml and glx.xml which could very easily be forgotten, vice versa for changes in wgl.xml and glx.xml.

I think there is also another problem where some WGL functions only accept a set of enums that do not have groups defined in gl.xml, so these would have to be added to gl.xml and look like an unused group, but in fact it's used by wgl.xml and potentially glx.xml.

EDIT:
Some ideas for cross referencing enums could be something like group="gl.xml::EnumGroup", group="GL::EnumGroup", or whatever prefix we want to denote a group from another file. I think we would just have to decide on a prefix and it doesn't need to be the perfect prefix that we are all 100% happy with, just a prefix that doesn't conflict with anything and isn't obviously bad would do.

pdaniell-nv · 2023-03-28T20:00:14Z

@NogginBops is this PR still a work in progress? If so, can you "convert to draft". Thanks.

SunSerega · 2023-05-19T14:50:06Z

Not for this PR but...

        <command>
            <proto><ptype>__GLXextFuncPtr</ptype> <name>glXGetProcAddressARB</name></proto>
            <param>const <ptype>GLubyte</ptype> *<name>procName</name></param>
        </command>
        <command>
            <proto><ptype>__GLXextFuncPtr</ptype> <name>glXGetProcAddress</name></proto>
            <param>const <ptype>GLubyte</ptype> *<name>procName</name></param>
        </command>

I guess think would also need to be kind="gl::String"?
Feels awkward to dup kind definitions between the XML files...

NogginBops · 2023-07-14T13:15:33Z

Not for this PR but...

        <command>
            <proto><ptype>__GLXextFuncPtr</ptype> <name>glXGetProcAddressARB</name></proto>
            <param>const <ptype>GLubyte</ptype> *<name>procName</name></param>
        </command>
        <command>
            <proto><ptype>__GLXextFuncPtr</ptype> <name>glXGetProcAddress</name></proto>
            <param>const <ptype>GLubyte</ptype> *<name>procName</name></param>
        </command>

I guess think would also need to be kind="gl::String"? Feels awkward to dup kind definitions between the XML files...

I agree these should be marked with the kind attribute.
And I'm guessing you are ok with using a prefix (gl::, wgl::, and glx::) to reference between the xml files?
I could put that into the xml spec for the group and kind tags.

SunSerega · 2023-07-14T20:01:48Z

Yes, I've since rewritten my data scrappers. Had too much legacy code, your idea with group namespaces was the final push.

Right now I'm using XML files from the branch in my own fork, which has the changes from this PR plus a bunch of my own, with the namespace prefixes supported for all sorts of items, including the kinds too.

NogginBops · 2023-07-17T11:50:16Z

Ok great then I will move forward on this assuming we are going to go with namespaces.
Will document how the namespaces work in the xml specification before marking this as ready for review.

NogginBops · 2023-07-23T17:03:01Z

@SunSerega how do you feel about doing something like this in gl.xml:

<enum value="0x0DE1" name="GL_TEXTURE_2D" group="...,wgl::ObjectType"/>
<enum value="0x0DE1" name="GL_TEXTURE_3D" group="...,wgl::ObjectType"/>

to be able to create enum groups for wgl.xml? or should I create a group called WGLObjectType (or something like WGLObjectTypeDX)

This relates to this to the DX interop extension but will be applicable to other extensions as well
https://registry.khronos.org/OpenGL/extensions/NV/WGL_NV_DX_interop.txt

SunSerega · 2023-07-23T17:10:28Z

Yeah, this also works with how I imagined it: If no namespace is specified - it defaults to the same namespace as the file name. Otherwise, any namespace can be specified.

…pdate-wgl

NogginBops · 2023-07-23T21:17:41Z

@SunSerega what do you think about my newest commit? Is it in the right direction?
It's quite small, I'm just trying to figure out how we want to structure this.

xml/wgl.xml

SunSerega · 2023-08-10T11:53:39Z

Since we are going for file-based namespaces, besides WGLObjectTypeDX, there are a few more groups that start with redundant WGL:

wgl::WGLColorBufferMask
wgl::WGLContextFlagsMask
wgl::WGLContextProfileMask
wgl::WGLDXInteropMask
wgl::WGLImageBufferMask
wgl::WGLLayerPlaneMask

…ed BOM.

…pdate-wgl

NogginBops · 2024-03-16T17:20:22Z

@SunSerega is this PR getting ready or is there something else needed before I can un-draft it?

SunSerega

Checked everything again, LGTM.

NogginBops · 2024-03-20T12:17:45Z

xml/wgl.xml

@@ -766,6 +766,7 @@ Registry at https://github.com/KhronosGroup/OpenGL-Registry
        <command>
            <proto><ptype>UINT</ptype> <name>wglEnumerateVideoCaptureDevicesNV</name></proto>
            <param><ptype>HDC</ptype> <name>hDc</name></param>
+            <!-- FIXME: Some way for len="" to refer to the return value. Maybe len="RETURN()"? -->


@SunSerega what do you think about this?
Should we introduce RETURN() or should we just leave this unhandled?

I wouldn't mind either way, but also while this is the only case - it doesn't matter much, since this case would need to be handled manually anyway, and IMO len attribute values are not yet consistent enough to use as notification for consumers, like "some new, yet unexpected value appeared, need to figure out how to handle this". Mainly because more than a 1/4 of them are just COMPSIZE.

Fair enough, I could remove this comment or leave it in.

I'll leave it to @oddhack to say whether this comments gets to stay or gets removed.

Added len and group attributes to wgl.xml

3dbe6d3

Fix typo

6509718

Perksey reviewed Mar 24, 2023

View reviewed changes

Changed some enum groups to use existing names

7048d3d

NogginBops marked this pull request as draft March 28, 2023 22:26

NogginBops changed the title ~~[WIP] Added len and group attributes to wgl.xml~~ Added len and group attributes to wgl.xml Mar 28, 2023

NogginBops added 2 commits July 23, 2023 19:31

Merge branch 'main' of github.com:KhronosGroup/OpenGL-Registry into u…

589cbc7

…pdate-wgl

Started adding gl:: references in wgl.xml

93296f3

SunSerega reviewed Jul 24, 2023

View reviewed changes

xml/wgl.xml Outdated Show resolved Hide resolved

NogginBops mentioned this pull request Aug 26, 2023

Support enum group namespaces in generator opentk/opentk#1626

Merged

NogginBops added 5 commits October 16, 2023 13:57

Fixed ScalarType enum group and referenced that for wglGetGPUInfoAMD

0ba18ca

Removed WGL prefix from multiple wgl.xml enum groups

31a018c

Fixed gl::WGLObjectTypeDX -> ObjectTypeDX in wgl.xml

98e0253

Add PixelFormatAttribute to wglGetPixelFormatAttrib* functions. Remov…

29651a8

…ed BOM.

Merge branch 'main' of github.com:KhronosGroup/OpenGL-Registry into u…

70a776f

…pdate-wgl

SunSerega approved these changes Mar 18, 2024

View reviewed changes

NogginBops commented Mar 20, 2024

View reviewed changes

NogginBops marked this pull request as ready for review March 20, 2024 13:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added len and group attributes to wgl.xml #569

Added len and group attributes to wgl.xml #569

NogginBops commented Mar 24, 2023

NogginBops commented Mar 24, 2023

SunSerega commented Mar 24, 2023 •

edited

NogginBops commented Mar 24, 2023 •

edited

Perksey Mar 24, 2023

NogginBops Mar 25, 2023

Perksey Mar 24, 2023

NogginBops Mar 25, 2023

SunSerega commented Mar 25, 2023 •

edited

NogginBops commented Mar 25, 2023 •

edited

pdaniell-nv commented Mar 28, 2023 •

edited

SunSerega commented May 19, 2023

NogginBops commented Jul 14, 2023

SunSerega commented Jul 14, 2023

NogginBops commented Jul 17, 2023

NogginBops commented Jul 23, 2023

SunSerega commented Jul 23, 2023

NogginBops commented Jul 23, 2023

SunSerega commented Aug 10, 2023

NogginBops commented Mar 16, 2024

SunSerega left a comment

NogginBops Mar 20, 2024

SunSerega Mar 20, 2024 •

edited

NogginBops Mar 20, 2024

		@@ -72,44 +72,44 @@ Registry at https://github.com/KhronosGroup/OpenGL-Registry
		sometimes reused for other purposes -->

Added len and group attributes to wgl.xml #569

Are you sure you want to change the base?

Added len and group attributes to wgl.xml #569

Conversation

NogginBops commented Mar 24, 2023

NogginBops commented Mar 24, 2023

SunSerega commented Mar 24, 2023 • edited

NogginBops commented Mar 24, 2023 • edited

Perksey Mar 24, 2023

Choose a reason for hiding this comment

NogginBops Mar 25, 2023

Choose a reason for hiding this comment

Perksey Mar 24, 2023

Choose a reason for hiding this comment

NogginBops Mar 25, 2023

Choose a reason for hiding this comment

SunSerega commented Mar 25, 2023 • edited

NogginBops commented Mar 25, 2023 • edited

pdaniell-nv commented Mar 28, 2023 • edited

SunSerega commented May 19, 2023

NogginBops commented Jul 14, 2023

SunSerega commented Jul 14, 2023

NogginBops commented Jul 17, 2023

NogginBops commented Jul 23, 2023

SunSerega commented Jul 23, 2023

NogginBops commented Jul 23, 2023

SunSerega commented Aug 10, 2023

NogginBops commented Mar 16, 2024

SunSerega left a comment

Choose a reason for hiding this comment

NogginBops Mar 20, 2024

Choose a reason for hiding this comment

SunSerega Mar 20, 2024 • edited

Choose a reason for hiding this comment

NogginBops Mar 20, 2024

Choose a reason for hiding this comment

SunSerega commented Mar 24, 2023 •

edited

NogginBops commented Mar 24, 2023 •

edited

SunSerega commented Mar 25, 2023 •

edited

NogginBops commented Mar 25, 2023 •

edited

pdaniell-nv commented Mar 28, 2023 •

edited

SunSerega Mar 20, 2024 •

edited