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

Use vulkan_core.h optionally instead of vulkan.h in ICD #719

Open
oddhack opened this issue Oct 14, 2021 · 1 comment
Open

Use vulkan_core.h optionally instead of vulkan.h in ICD #719

oddhack opened this issue Oct 14, 2021 · 1 comment
Assignees
Labels
build enhancement

Comments

@oddhack
Copy link

@oddhack oddhack commented Oct 14, 2021

This came up in internal Vulkan issue 2857. @cubanismo noted some concerns with using vulkan.h in their driver builds and suggested replacing the Wayland header include in vulkan.h with incomplete types for the few types it needs. There is a similar issue KhronosGroup/Vulkan-Headers#234 regarding the Windows platform with concerns about compilation time / complexity consequences of including windows.h from vulkan.h.

In the first case, they are unable to migrate to vulkan_core.h due to some sort of conflict with the ICD interface headers. So we were wondering as to the feasibility of updating the ICD headers to optionally allow use of vulkan_core.h, putting responsibility on the code including the ICD headers to define all needed platform types.

Making changes to vulkan.h itself poses the risk of breakage to any developers that are assuming including it gets them all the platform headers they need (and which is how it is literally specified as functioning). So updating middleware like the ICD (for Wayland case) or Vulkan-Hpp (for the Windows case) seems like a decent fix if it's possible.

@charles-lunarg
Copy link
Collaborator

@charles-lunarg charles-lunarg commented Oct 15, 2021

Just did a quick hack test where I modified vk_icd.h to include vulkan_core.h instead of vulkan.h and the loader compiled & ran the tests successfully because the code includes vulkan.h before vk_icd.h. This change wouldn't affect the loader currently (eg, the loader relies on vulkan.h to make the necessary WSI types available).

I am not opposed to using vulkan_core.h in vk_icd.h.

@charles-lunarg charles-lunarg added build enhancement labels Feb 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build enhancement
Projects
None yet
Development

No branches or pull requests

2 participants