search

KhronosGroup / OpenGL-Registry

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

Added structs for passing in values to "glDispatchComputeIndirect", "glDrawArraysIndirect", "glDrawElementsIndirect", "glMultiDrawElementsIndirect" and "glMultiDrawElementsIndirect" commands #594

Closed Rytis-Stan closed 10 months ago

Rytis-Stan commented 10 months ago

Added the structs because they get mentioned in documentation but are nowhere to be found in gl.xml

Perksey commented 10 months ago

I don't think this is the intended use of the registry given that these structs are not required by any function exposed (this would also be against OpenGL's design generally). If these is a specific documentation page referencing this then that page should include these definitions to provide clarity.

oddhack commented 10 months ago

These are not part of the API, and we would not put them into gl.xml. They are part of pseudocode for how an implementation behaves.

pdaniell-nv commented 10 months ago

Closing as this isn't something we want in the XML and API headers.

Rytis-Stan commented 9 months ago

@Perksey @oddhack @SunSerega @pdaniell-nv - Greetings. Sorry for the late reply.

The specification PDF files explicitly specify the names and contents of structs which are used for glDrawArraysIndirect and glDrawElementsIndirect commands, and then glMultiDrawArraysIndirect and glMultiDrawElementsIndirect simply reuse those same structs. For glDispatchComputeIndirect command it looks a bit different, with the specification containing the names of all the struct's fields, but not the name of the struct itself. But that seems to be more of an inconsistency in the spec, as even the online documentation explicitly specifies a full struct.

In addition, the structs differ slightly in OpenGL 4.0 and 4.3 specifications, where the same field is called either reservedMustBeZero or baseInstance, so strictly speaking there should be 2 different versions of each struct, depending on the OpenGL version. However, a simpler approach would probably be to just use the more "appropriate" name for the field, which is baseInstace.

Related links to online documentation pages: https://registry.khronos.org/OpenGL-Refpages/gl4/html/glDispatchComputeIndirect.xhtml https://registry.khronos.org/OpenGL-Refpages/gl4/html/glDrawArraysIndirect.xhtml https://registry.khronos.org/OpenGL-Refpages/gl4/html/glDrawElementsIndirect.xhtml https://registry.khronos.org/OpenGL-Refpages/gl4/html/glMultiDrawArraysIndirect.xhtml https://registry.khronos.org/OpenGL-Refpages/gl4/html/glMultiDrawElementsIndirect.xhtml

And below you will find some screenshots from specification PDF files, where these structs are being mentioned.

glDrawArraysIndirect:

glspec40 core pdf_glDrawArraysIndirect_page_34

glspec43 core pdf_glDrawArraysIndirect_page_309

glDrawElementsIndirect:

glDrawElementsIndirect:

glMultiDrawElementsIndirect:

glDispatchComputeIndirect:

SunSerega commented 9 months ago

For me, it seems the spec means that relevant parameters take in arbitrary arrays, and structures are only defined as an example, expecting the user to have their own structure (or use raw uint-s). But then @oddhack answered before I could, and I guess they remember the original intent behind the spec, so I've scrapped what I've written and liked that reply instead.

That said, while C bindings definitely don't need this, these definitions, in combination with IntelliSense, can telegraph field order and sign-ness, so I can see some merit in adding this info to my bindings, as an alternative overload. But that touches on another point: https://github.com/KhronosGroup/OpenGL-Registry/blob/0ef89b84d3bb5880a6553231d9cc64b2abd525a7/xml/wgl.xml#L49C78-L49C78 To me, it's not just indentation that's painful about this... I guess it was added with minimal friction because gl.xml has no structures and it would be weird to add more complexity just for WGL. But if you want to add this to gl.xml - it's better to rethink how structures are defined in XML. I think the best would be something similar to functions:

        <type generate="true">struct <name>GPU_DEVICE</name>
            <field><ptype>DWORD</ptype>  <name>cb</name></field>
            <field><ptype>CHAR</ptype>   <name>DeviceName</name>[<len>32</len>]</field>
            <field><ptype>CHAR</ptype>   <name>DeviceString</name>[<len>128</len>]</field>
            <field><ptype>DWORD</ptype>  <name>Flags</name></field>
            <field><ptype>RECT</ptype>   <name>rcVirtualScreen</name></field>
        </type>

But that would require extra work in how official C bindings are generated.

Rytis-Stan commented 9 months ago

For me, it seems the spec means that relevant parameters take in arbitrary arrays, and structures are only defined as an example, expecting the user to have their own structure (or use raw uint-s). But then @oddhack answered before I could, and I guess they remember the original intent behind the spec, so I've scrapped what I've written and liked that reply instead.

@SunSerega @oddhack - I still don't understand why the structures would be considered not part of the API. For example, in the online documentation of the glDrawArraysIndirect command behaves similarly to glDrawArraysInstancedBaseInstance command: image So, if we compare the structure mentioned in the documentation of glDrawArraysIndirect... image ... and the signature of glDrawArraysInstancedBaseInstance... image ... it can be seen that the ...Indirect... method version simply just has it's parameters gathered together in a struct which simply gets passed in an indirect manner. Why would the structure suddenly become pseudo-code or not part of the API? What's so special about having direct parameters passed in instead of via a struct? Why would the user need anything else but the struct provided?

That said, while C bindings definitely don't need this, these definitions, in combination with IntelliSense, can telegraph field order and sign-ness, so I can see some merit in adding this info to my bindings, as an alternative overload. But that touches on another point: https://github.com/KhronosGroup/OpenGL-Registry/blob/0ef89b84d3bb5880a6553231d9cc64b2abd525a7/xml/wgl.xml#L49C78-L49C78 To me, it's not just indentation that's painful about this... I guess it was added with minimal friction because gl.xml has no structures and it would be weird to add more complexity just for WGL. But if you want to add this to gl.xml - it's better to rethink how structures are defined in XML. I think the best would be something similar to functions:

        <type generate="true">struct <name>GPU_DEVICE</name>
            <field><ptype>DWORD</ptype>  <name>cb</name></field>
            <field><ptype>CHAR</ptype>   <name>DeviceName</name>[<len>32</len>]</field>
            <field><ptype>CHAR</ptype>   <name>DeviceString</name>[<len>128</len>]</field>
            <field><ptype>DWORD</ptype>  <name>Flags</name></field>
            <field><ptype>RECT</ptype>   <name>rcVirtualScreen</name></field>
        </type>

But that would require extra work in how official C bindings are generated.

I also don't like the way the structs are currently defined in the file, but it's also the same with things like function type definitions and command proto tags. It's just C code pasted between a few XML tags, instead of the XML being fully used to define the whole structure of the code. So using a more structured XML approach would be desirable, though some modifications should be made.

@SunSerega - And what would be the purpose of the generate attribute?

SunSerega commented 9 months ago

And what would be the purpose of the generate attribute?

To not generate the structs you are talking about, but generate WGL structs, for Khronos C bindings.

NogginBops commented 9 months ago

I still don't understand why the structures would be considered not part of the API.

The specification tells us that these functions behave identical to some code, in that code the struct itself is defined. If it where part of the API you wouldn't need to typedef the struct.

A practical reason for this is that these structs are most likely going to be written to in compute shaders that generate a list of drawcalls to make (at least nowadays, not sure if this is what was expected when the API was originally constructed). Not a reason for the structs to not be part of the specification but at least some context for why they are not part of it.

I know it might not be a super satisfying answer but it at least answers why the struct defined in the standard isn't part of the API.