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

MVKDevice: Clamp max per-set descriptor limit to minimum 1024. #2073

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

cdavis5e
Copy link
Collaborator

As required by the Vulkan spec.

Fixes the CTS tests
dEQP-VK.api.info.vulkan1p2_limits_validation.khr_maintenance_3 and
dEQP-VK.api.maintenance3_check.maintenance3_properties.

Copy link
Contributor

@billhollings billhollings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels arbitrary. Unless arg buffers are in play, this minimum total is never going to be met.

Walk me through this. By inflating this value, what are we telling an app that might be dangerous?

@cdavis5e
Copy link
Collaborator Author

This feels arbitrary. Unless arg buffers are in play, this minimum total is never going to be met.

Walk me through this. By inflating this value, what are we telling an app that might be dangerous?

This is the maximum number of total descriptors of all types across all stages that may be contained in a descriptor set at once. It's meant to be a constraint on the size of a descriptor set itself. Even if we were to raise it, an app would still be constrained by the maximum per-type descriptor counts.

This is the minimum maximum that is mandated by the Vulkan spec. The CTS actually tests that the limit is at least 1024.

@cdavis5e cdavis5e force-pushed the min-max-descriptor-count branch from 3f88fbb to eb558bb Compare November 23, 2023 03:04
As required by the Vulkan spec.

Fixes the CTS tests
`dEQP-VK.api.info.vulkan1p2_limits_validation.khr_maintenance_3` and
`dEQP-VK.api.maintenance3_check.maintenance3_properties`.
@cdavis5e cdavis5e force-pushed the min-max-descriptor-count branch from eb558bb to 44b3613 Compare November 23, 2023 03:06
@billhollings billhollings merged commit 645aaa4 into KhronosGroup:main Nov 23, 2023
5 checks passed
@cdavis5e cdavis5e deleted the min-max-descriptor-count branch November 23, 2023 21:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants