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

Make vulkaninfo removal of promoted extension structures optional #963

Closed
charles-lunarg opened this issue Mar 7, 2024 · 8 comments · Fixed by #999
Closed

Make vulkaninfo removal of promoted extension structures optional #963

charles-lunarg opened this issue Mar 7, 2024 · 8 comments · Fixed by #999
Labels
enhancement New feature or request vulkaninfo

Comments

@charles-lunarg
Copy link
Contributor

charles-lunarg commented Mar 7, 2024

PR #948 removed extension structures from vulkaninfo output that were promoted to core Vulkan, eg into the VkPhysicalDeviceVulkan(11|12|13)(Features|Properties) structs. It was done for Vulkan profile generation, but it users who are looking for the extension structures explicitly as they no longer appear.

PR 948 was a necessary contribution to allow correct Vulkan-Profile output, but should be a command line option, eg --remove-promoted-extensions --strip_duplicate_struct.

@charles-lunarg charles-lunarg added enhancement New feature or request vulkaninfo labels Mar 7, 2024
@christophe-lunarg
Copy link

christophe-lunarg commented Mar 7, 2024

In the Vulkan Profiles merge script, the argument is called --strip_duplicate_struct if we want to be consistant. If you prefer a different name, I'll happy to use that name.

(It's not promoted extensions, but the structures, even promoted structures, that are removed)

@charles-lunarg
Copy link
Contributor Author

--strip_duplicate_struct is a much better name, because 'strip' better describes whats happening.

@rumblehhh
Copy link

Jumping on this to add that we were recently bitten by this. We use vulkaninfo's json option for checking implementation features/properties to avoid running unsupported tests/content.
After upgrading to a version of vulkaninfo after #948 a number of our tests were being unexpectedly skipped. For now we've had to downgrade to workaround the missing structures.

@charles-lunarg charles-lunarg linked a pull request Jun 17, 2024 that will close this issue
@charles-lunarg
Copy link
Contributor Author

@rumblehhh I pushed up a new option to vulkaninfo show-all-structs. Could you try out the branch? And also very open to bikeshedding the name of the option. In my head, this option 'shows all structs' that would previously be hidden. But I'm open to suggestions.

@rumblehhh
Copy link

@charles-lunarg thanks for the changes! I've checked out the branch and I believe this should work fine for us.
Should this, or another option if the name changes, actually show all structures including alias structures?

Taking the dynamicRendering feature as an example

  • main shows VkPhysicalDeviceVulkan13Features
  • --show-all-structs shows VkPhysicalDeviceVulkan13Features and VkPhysicalDeviceDynamicRenderingFeatures

--show-all-structs implies vulkaninfo should be showing all structures which you may think should additionally include VkPhysicalDeviceDynamicRenderingFeaturesKHR.

As for other names:
--hide-promoted-structs
--show-promoted-structs
and depending on the above you may also want show-alias-structs or hide-alias-structs

@JamesRumble-IMG
Copy link

@charles-lunarg are there any updates on this?

@charles-lunarg
Copy link
Contributor Author

@rumblehhh I took your advice and changed it to `--show-promoted-structs. "all" implies things that are above and beyond what are actually being done.

@JamesRumble-IMG Thank you for the reminder - this PR was low priority and thus fell off my todo list. It was a simple rebase and name change to get this ready to ship. (I also found a bug in my code, so that was good).

@JamesRumble-IMG
Copy link

Thanks very much for adding the --show-promoted-structs feature!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request vulkaninfo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants