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

Add VK_KHR_portability_enumeration extension and flag #525

Merged
merged 1 commit into from Feb 24, 2024

Conversation

Helco
Copy link
Contributor

@Helco Helco commented Feb 22, 2024

Currently using Vulkan on MacOS using MoltenVK causes a validation error related to the VK_KHR_portability_enumeration extension. The MoltenVK documentation states:

Because MoltenVK supports the VK_KHR_portability_subset extension, when using the Vulkan Loader from the Vulkan SDK to run MoltenVK on macOS, the Vulkan Loader will only include MoltenVK VkPhysicalDevices in the list returned by vkEnumeratePhysicalDevices() if the VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR flag is enabled in vkCreateInstance(). See the description of the VK_KHR_portability_enumeration extension in the Vulkan specification for more information about the use of the VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR flag.

This is reproducible in NeoDemo by just switching to the Vulkan backend.

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

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

I'm a bit concerned that using this flag can cause unexpected behaviour - either the app behaving badly because it's a non-conformant implementation, or it somehow preferring a portability implementation over the native one (I don't know if this is possible, I've only done a rudimentary reading of the documentation).

VulkanDeviceOptions already contains an InstanceExtensions member, so how about allowing the callsite to request this extension themselves? That way, a consumer such as ourselves could request it only if the target platform is macOS?

src/Veldrid/Vk/VkGraphicsDevice.cs Show resolved Hide resolved
@Helco
Copy link
Contributor Author

Helco commented Feb 22, 2024

I share your sentiment as I am also not experienced enough in Vulkan to definitively say this. However:

Just setting the extension won't work as one still has to set the flag, which is not possible with VulkanDeviceOptions

As far as I have read Veldrid already opens itself up to portability implementations by activating the VK_KHR_portability_subset extension:

Fully-conformant Vulkan implementations provide all the required capabilities, and so will not provide this extension. Therefore, the existence of this extension can be used to determine that an implementation is likely not fully conformant with the Vulkan spec.

Based on the VK_KHR_portability_enumeration documentation it should only be present if there are portability devices so unless there are both portability and non-portability devices there should be no problem choosing.
If there are multiple devices, Veldrid currently already chooses the first one without regard which would be the "best" one. I don't think portability is an additional problem to not being able to enumerate or choose devices in the first place.

I also contemplated adding the extension and flag only for MacOS because as it stands Veldrid does not work at all with MoltenVK and there are no reports of portability problems with Veldrid on other platforms.

(maybe another argument is the list of potentially affected devices: a short list of quite old GPUs and Apple GPUs with a total of 2.6% share based on vulkan.gpuinfo.org)

@smoogipoo smoogipoo merged commit 8ef576a into veldrid:master Feb 24, 2024
1 check passed
@Helco Helco deleted the fix-portability-moltenvk branch February 24, 2024 16:07
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.

None yet

2 participants