Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML Type Issues #717

Closed
Keith-Cancel opened this issue May 15, 2018 · 6 comments
Closed

XML Type Issues #717

Keith-Cancel opened this issue May 15, 2018 · 6 comments
Assignees
Labels
Feature Request Feature request (enhancement) Registry

Comments

@Keith-Cancel
Copy link

Why are these marked as a define?

        <type category="define">struct <name>ANativeWindow</name>;</type>
        <type category="define">struct <name>AHardwareBuffer</name>;</type>

Although, reading over the schema doc I don't see how you would describe a forward declared struct? Does the struct/union type have any thing to mark them as a forward declaration? Otherwise a separate category.

Additionally, why are these constants specified as enums if they are not an enum type? Moreover, some of these values are compile time dependent is that intended? For instance (~0U) may have a different bit width on 32 bit system vs a 64 bit system.

<enums name="API Constants" comment="Vulkan hardcoded constants - not an enumerated type, part of the header boilerplate">
        <enum value="256"   name="VK_MAX_PHYSICAL_DEVICE_NAME_SIZE"/>
        <enum value="16"    name="VK_UUID_SIZE"/>
        <enum value="8"     name="VK_LUID_SIZE"/>
        <enum               name="VK_LUID_SIZE_KHR" alias="VK_LUID_SIZE"/>
        <enum value="256"   name="VK_MAX_EXTENSION_NAME_SIZE"/>
        <enum value="256"   name="VK_MAX_DESCRIPTION_SIZE"/>
        <enum value="32"    name="VK_MAX_MEMORY_TYPES"/>
        <enum value="16"    name="VK_MAX_MEMORY_HEAPS" comment="The maximum number of unique memory heaps, each of which supporting 1 or more memory types"/>
        <enum value="1000.0f" name="VK_LOD_CLAMP_NONE"/>
        <enum value="(~0U)" name="VK_REMAINING_MIP_LEVELS"/>
        <enum value="(~0U)" name="VK_REMAINING_ARRAY_LAYERS"/>
        <enum value="(~0ULL)" name="VK_WHOLE_SIZE"/>
        <enum value="(~0U)" name="VK_ATTACHMENT_UNUSED"/>
        <enum value="1"     name="VK_TRUE"/>
        <enum value="0"     name="VK_FALSE"/>
        <enum value="(~0U)" name="VK_QUEUE_FAMILY_IGNORED"/>
        <enum value="(~0U-1)" name="VK_QUEUE_FAMILY_EXTERNAL"/>
        <enum               name="VK_QUEUE_FAMILY_EXTERNAL_KHR" alias="VK_QUEUE_FAMILY_EXTERNAL"/>
        <enum value="(~0U-2)" name="VK_QUEUE_FAMILY_FOREIGN_EXT"/>
        <enum value="(~0U)" name="VK_SUBPASS_EXTERNAL"/>
        <enum value="32"    name="VK_MAX_DEVICE_GROUP_SIZE"/>
        <enum               name="VK_MAX_DEVICE_GROUP_SIZE_KHR" alias="VK_MAX_DEVICE_GROUP_SIZE"/>
    </enums>

@Keith-Cancel
Copy link
Author

Woops my first question seem to be a duplicate of: #679

@krOoze
Copy link
Contributor

krOoze commented May 15, 2018

#662 and #664 relevant to your second question.

@Keith-Cancel
Copy link
Author

Keith-Cancel commented May 15, 2018

Okay, those are relevant to the later half of the question. It just does not seem great to rely on values that are generated at compile time. Yea definitely makes it harder for other languages since they have to generate sane values for what the compiler/system would.

However, still why are these constants marked with enum tags? While an enum can be similar to a constant it's also akin to a class. It's odd that this is different from other enums, and there is no tag marking it as such. I have read through this. https://www.khronos.org/registry/vulkan/specs/1.1/registry.html#tag-enums

I don't see anything allowing such construction. Moreover the comment for this enum section even says "not an enumerated type". It's just seems like the schema could be better organized so when parsing the XML one does not have to know the enums section named "API Constants" is not really a enum.

I just found the use of the type marked as a define, and this enums block inconsistent from what the schema documentation explained.

@oddhack oddhack self-assigned this May 21, 2018
@oddhack oddhack added Feature Request Feature request (enhancement) Registry labels May 21, 2018
@oddhack
Copy link
Contributor

oddhack commented May 21, 2018

They're marked as <enum> types because that's a legacy of the OpenGL XML schema this started with, where every GLenum is actually a compile-time constant, not an enumerant. This could stand to be changed, but changes to the schema are very disruptive to all the downstream SDK and external consumers that depend on stability of the XML, and we're cautious about doing that due to painful internal experience.

We'll take this under advisement along with other feedback, but we're unlikely to do anything about it in the near future.

@oddhack
Copy link
Contributor

oddhack commented May 21, 2018

N.b. the way the dependency-driven approach to generating interfaces works, the only time an "enums" block is required is when the corresponding enumerated type is being defined, and at that point it spits out all the contents of that block as the corresponding type. For a block like this one, the individual "enum" tags are required, and in that case a #define is generated. The description in registry.html could also stand to be improved as you note.

@oddhack
Copy link
Contributor

oddhack commented Apr 19, 2021

Closing as non-actionable. The constant values are now all explicitly typed as uint32_t, uint64_t, or float which should help with FFI. Schema changes of larger scope, beyond additions such as this explicit tagging, are unlikely to happen due to disruption to us and everyone downstream from us consuming the XML.

@oddhack oddhack closed this as completed Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Feature request (enhancement) Registry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants