KhronosGroup / OpenGL-Registry

OpenGL, OpenGL ES, and OpenGL ES-SC API and Extension Registry
678 stars 274 forks source link

Added len and group attributes to wgl.xml #569

Open NogginBops opened 1 year ago

NogginBops commented 1 year ago

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 commented 1 year ago

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

SunSerega commented 1 year ago

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

NogginBops commented 1 year ago

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.

SunSerega commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

SunSerega commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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.

NogginBops commented 1 year ago

@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.

SunSerega commented 1 year ago

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
NogginBops commented 6 months ago

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