Unclear way to safely aggregate features in `xrCreateVulkanDeviceKHR`

[Cyphall]

xrCreateVulkanDeviceKHR from XR_KHR_vulkan_enable2 requires that the implementation aggregate its own extension/feature requirements with the app's requirements passed in XrVulkanDeviceCreateInfoKHR::vulkanCreateInfo.

Say, an implementation requires timeline semaphores.
In the case where this feature is not already enabled by the app but the structure enabling it is in the pNext chain (e.g. VkPhysicalDeviceTimelineSemaphoreFeatures or VkPhysicalDeviceVulkan12Features is present but timelineSemaphore is false), since the vulkanCreateInfo field is const, the only way to safely enable timeline semaphores is to deep-copy the whole structure chain.

The issue is that the structure chain may contain structures that the implementation is not aware of, and cannot deep-copy.

What is the expected way to resolve this issue?

[rpavlik-bot]

An issue (number 2628) has been filed to correspond to this issue in the internal Khronos GitLab (Khronos members only: KHR:openxr/openxr#2628 ), to facilitate working group processes.

This GitHub issue will continue to be the main site of discussion.

[nv-jakob]

If you are lucky you can work around it, but there are cases were you are not allowed to force enable the timeline semaphores. The annoying part is that for Windows timeline semaphores is part of native DX12, and for Linux most drivers will internally use timeline semaphores anyways. So it's mostly just to make the validation layer happy.

Yes, this a problem with Vulkan that you are not allowed to have multiple of the same struct in the next chain. But you are allowed to copy and modify the VkDeviceCreateInfo structure so you can always insert extra structs. So if the struct isn't already there then you can add it, or if you are lucky you might by-pass the "bad" structs.

You can see the layer I made a long time ago to enable force enable timeline semaphores. https://gitlab.freedesktop.org/monado/utilities/vulkan-layers/-/blob/main/src/layer.cpp?ref_type=heads#L218

I'm unsure what the driver does if you pass in both VkPhysicalDeviceVulkan12Features and VkPhysicalDeviceTimelineSemaphoreFeatures to vkCreateDevice and they differ in enable state?

[Cyphall]

But you are allowed to copy and modify the VkDeviceCreateInfo structure

The VkDeviceCreateInfo struct is passed as const, which means const_casting away the const to modify the chain is actually UB (including potential access violation) in the case where the passed-in VkDeviceCreateInfo started its lifespan as a const object (which is perfectly legal for a user to do).

This is why we currently need to deep-copy the whole chain if we ever need to modify anything in it, which may not be possible as it may contain structs unknown to the runtime.

The only way I see to safely be able to modify the chain would be to change XrVulkanDeviceCreateInfoKHR::vulkanCreateInfo from const VkDeviceCreateInfo* to VkDeviceCreateInfo*.

[nv-jakob]

Yes if you want to be able to modify any part of the chain that's correct.

Tho there are some cases were you don't need to modify any part of the chain.

1. The app didn't pass in VkPhysicalDeviceVulkan12Features or VkPhysicalDeviceTimelineSemaphoreFeatures in the next chain.
2. The app passed in VkPhysicalDeviceVulkan12Features in the next chain, but put it first / early.

For one 1 what you do then is:

VkDeviceCreateInfo my_info = info->vulkanCreateInfo;

VkPhysicalDeviceTimelineSemaphoreFeatures timeline_semaphore = {
    semaphore.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_TIMELINE_SEMAPHORE_FEATURES;
    semaphore.timelineSemaphore = true;
    semaphore.pNext = (void *)my_info.pNext; // Have to cast here.
};

// Safe to modify it's a copy.
my_info->next = &timeline_semaphore;

vkCreateDevice(..., &my_info); // Pass in modified struct.

Number 2 is a little bit involved, but you get the point. But yes worst case you either have to deep copy the entire chain, or give up. Which is what that layer does in some cases.

Something I was informed of is that it's not allowed to pass in a VkPhysicalDeviceTimelineSemaphoreFeatures struct to vkCreateDevice if you also pass in a VkPhysicalDeviceVulkan12Features struct in the next chain.

So in short: there is no way to always ensure that timeline semaphores are enabled in XR_KHR_vulkan_enable2, unless you do deep copies of the next chain. There are ways to increase the likelihood that you can do it by implementing limited deep copy. Say only VkPhysicalDeviceVulkan12Features and VkPhysicalDeviceTimelineSemaphoreFeatures and hope that the application put them first in the chain.

[Cyphall]

I expect more complex apps/engines to dynamically build the pNext chain based on user-enabled features (this is what our engine do), in which case neither case 1 nor case 2 would reliably apply.

The only way I see to safely be able to modify the chain would be to change XrVulkanDeviceCreateInfoKHR::vulkanCreateInfo from const VkDeviceCreateInfo* to VkDeviceCreateInfo*.

Would such a fix be implementable in the extension spec?
This would officially allow the runtime to modify the user chain and change the "aggregate user and runtime chains" behavior into "integrate runtime requirements into the user chain".

If such modification is too breaking to implement, could the spec text be expanded to explicitly forbid passing actually const memory? This would allow safely const_casting and modifying the chain.
I would expect most if not all users to already pass non-const structs.