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

Inconsistant limittype specification #2461

Closed
qbojj opened this issue Nov 7, 2024 · 4 comments
Closed

Inconsistant limittype specification #2461

qbojj opened this issue Nov 7, 2024 · 4 comments

Comments

@qbojj
Copy link
Contributor

qbojj commented Nov 7, 2024

Let us assume that if the limittype contains min for limit alpha then: device->alpha >= limit->alpha (similar holds for max)

As for:

  • minMemoryMapAlignment is min,pot then device->minMemoryMapAlignment >= limit
  • minStorageBufferOffsetAlignment is also min,pot so device->minStorageBufferOffsetAlignment >= limit

But as minMemoryMapAlignment restricts the device (the higher the limit the better), but minStorageBufferOffsetAlignment restricts the user (the lower the better) maybe (even if it sounds unintuitive) the limits that are restricting the user should have reversed limittypes.

Then we would have minMemoryMapAlignment: min,pot, but minStorageBufferOffsetAlignment would be max,pot, but that would align with the device/user restriction reasoning.

Originally posted by @qbojj in KhronosGroup/Vulkan-Profiles#703 (comment)

@r-potter r-potter self-assigned this Nov 13, 2024
@r-potter
Copy link
Contributor

r-potter commented Dec 9, 2024

These do seem inconsistent. The Required Limits table in the spec defines minMemoryMapAlignment as >= 64, but minStorageBufferOffsetAlignment <= 256.

@qbojj
Copy link
Contributor Author

qbojj commented Dec 10, 2024

Maybe I should have sent this issue to the XML repo, as minMemoryMapAlignment >= 64, but minStorageBufferOffsetAlignment <= 256 is what I would expect (as stated in the introduction), but the xml sets the limittype to the same value (min,pot)

            <member limittype="bits"><type>uint32_t</type>              <name>viewportSubPixelBits</name><comment>number bits of subpixel precision for viewport</comment></member>
            <member limittype="min,pot"><type>size_t</type>             <name>minMemoryMapAlignment</name><comment>min required alignment of pointers returned by MapMemory (bytes)</comment></member>
            <member limittype="min,pot"><type>VkDeviceSize</type>       <name>minTexelBufferOffsetAlignment</name><comment>min required alignment for texel buffer offsets (bytes) </comment></member>
            <member limittype="min,pot"><type>VkDeviceSize</type>       <name>minUniformBufferOffsetAlignment</name><comment>min required alignment for uniform buffer sizes and offsets (bytes)</comment></member>
            <member limittype="min,pot"><type>VkDeviceSize</type>       <name>minStorageBufferOffsetAlignment</name><comment>min required alignment for storage buffer offsets (bytes)</comment></member>
            <member limittype="min"><type>int32_t</type>                <name>minTexelOffset</name><comment>min texel offset for OpTextureSampleOffset</comment></member>
            <member limittype="max"><type>uint32_t</type>               <name>maxTexelOffset</name><comment>max texel offset for OpTextureSampleOffset</comment></member>

@oddhack
Copy link
Contributor

oddhack commented Dec 11, 2024

Maybe I should have sent this issue to the XML repo

Vulkan-Docs holds the canonical version of vk.xml, if that's what you mean by "XML repo".

@oddhack
Copy link
Contributor

oddhack commented Dec 20, 2024

This should be fixed in the 1.4.304 spec update.

@oddhack oddhack closed this as completed Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants