[PATCH 1/2] winevulkan: Adequate vkEnumerateInstanceLayerProperties to the Vulkan spec
The section for vkEnumerateInstanceLayerProperties states [1]: On success, this command returns * VK_SUCCESS * VK_INCOMPLETE On failure, this command returns * VK_ERROR_OUT_OF_HOST_MEMORY * VK_ERROR_OUT_OF_DEVICE_MEMORY Always setting the layer count to zero and returning VK_SUCCESS is valid, as even if the function is given an array in pProperties, it's filled with zero layers [1]: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkEnum... Signed-off-by: Victor Hermann Chiletto <v(a)hnn.net.br> --- dlls/winevulkan/vulkan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index d24fcb2f64b..971394eb9dd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -1065,13 +1065,8 @@ VkResult WINAPI wine_vkEnumerateInstanceLayerProperties(uint32_t *count, VkLayer { TRACE("%p, %p\n", count, properties); - if (!properties) - { - *count = 0; - return VK_SUCCESS; - } - - return VK_ERROR_LAYER_NOT_PRESENT; + *count = 0; + return VK_SUCCESS; } VkResult WINAPI wine_vkEnumerateInstanceVersion(uint32_t *version) -- 2.30.1
This is generally implemented by the system's Vulkan ICD, and passing it through results in applications (namely, Unreal Engine) attempting to query for layer extensions in vkEnumerateDeviceExtensionProperties. Signed-off-by: Victor Hermann Chiletto <v(a)hnn.net.br> --- dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 1243f211b5e..228b4eae878 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -153,6 +153,7 @@ FUNCTION_OVERRIDES = { "vkCreateDevice" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkDestroyInstance" : {"dispatch" : False, "driver" : True, "thunk" : False }, "vkEnumerateDeviceExtensionProperties" : {"dispatch" : True, "driver" : False, "thunk" : False}, + "vkEnumerateDeviceLayerProperties": {"dispatch": True, "driver": False, "thunk": False}, "vkEnumeratePhysicalDeviceGroups" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkEnumeratePhysicalDevices" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkGetPhysicalDeviceExternalBufferProperties" : {"dispatch" : False, "driver" : False, "thunk" : False}, diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..23087c9df2e 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -1061,6 +1061,14 @@ VkResult WINAPI wine_vkEnumerateInstanceExtensionProperties(const char *layer_na return *count < num_properties ? VK_INCOMPLETE : VK_SUCCESS; } +VkResult WINAPI wine_vkEnumerateDeviceLayerProperties(VkPhysicalDevice phys_dev, uint32_t *count, VkLayerProperties *properties) +{ + TRACE("%p, %p, %p\n", phys_dev, count, properties); + + *count = 0; + return VK_SUCCESS; +} + VkResult WINAPI wine_vkEnumerateInstanceLayerProperties(uint32_t *count, VkLayerProperties *properties) { TRACE("%p, %p\n", count, properties); -- 2.30.1
Signed-off-by: Joshua Ashton <joshua(a)froggi.es>
On 3/6/21 12:46 PM, Victor Hermann Chiletto wrote:
This is generally implemented by the system's Vulkan ICD, and passing it through results in applications (namely, Unreal Engine) attempting to
It would be nice to name a specific UE4 application you're seeing this with for posterity.
query for layer extensions in vkEnumerateDeviceExtensionProperties.
I think the wording is a bit off here. Did you mean to cite vkEnumerateDeviceLayerProperties() rather than vkEnumerateDeviceExtensionProperties() here? Rather than saying "layer extensions" I think it would make sense to say "device layers". Thanks, Liam Middlebrook
Signed-off-by: Victor Hermann Chiletto <v(a)hnn.net.br> --- dlls/winevulkan/make_vulkan | 1 + dlls/winevulkan/vulkan.c | 8 ++++++++ 2 files changed, 9 insertions(+)
diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 1243f211b5e..228b4eae878 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -153,6 +153,7 @@ FUNCTION_OVERRIDES = { "vkCreateDevice" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkDestroyInstance" : {"dispatch" : False, "driver" : True, "thunk" : False }, "vkEnumerateDeviceExtensionProperties" : {"dispatch" : True, "driver" : False, "thunk" : False}, + "vkEnumerateDeviceLayerProperties": {"dispatch": True, "driver": False, "thunk": False}, "vkEnumeratePhysicalDeviceGroups" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkEnumeratePhysicalDevices" : {"dispatch" : True, "driver" : False, "thunk" : False}, "vkGetPhysicalDeviceExternalBufferProperties" : {"dispatch" : False, "driver" : False, "thunk" : False}, diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index 971394eb9dd..23087c9df2e 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -1061,6 +1061,14 @@ VkResult WINAPI wine_vkEnumerateInstanceExtensionProperties(const char *layer_na return *count < num_properties ? VK_INCOMPLETE : VK_SUCCESS; }
+VkResult WINAPI wine_vkEnumerateDeviceLayerProperties(VkPhysicalDevice phys_dev, uint32_t *count, VkLayerProperties *properties) +{ + TRACE("%p, %p, %p\n", phys_dev, count, properties); + + *count = 0; + return VK_SUCCESS; +} + VkResult WINAPI wine_vkEnumerateInstanceLayerProperties(uint32_t *count, VkLayerProperties *properties) { TRACE("%p, %p\n", count, properties);
On 16/03/2021 19:59, Liam Middlebrook wrote:
On 3/6/21 12:46 PM, Victor Hermann Chiletto wrote:
This is generally implemented by the system's Vulkan ICD, and passing it through results in applications (namely, Unreal Engine) attempting to
It would be nice to name a specific UE4 application you're seeing this with for posterity.
Project Wingman (Steam appid 895870), but inspection of the UE4 source code [1] shows that this issue will happen in any UE4 application that uses the Vulkan RHI. [1] https://github.com/EpicGames/UnrealEngine/blob/5df54b7ef1714f28fb5da319c3e83...
query for layer extensions in vkEnumerateDeviceExtensionProperties.
I think the wording is a bit off here. Did you mean to cite vkEnumerateDeviceLayerProperties() rather than vkEnumerateDeviceExtensionProperties() here? Rather than saying "layer extensions" I think it would make sense to say "device layers".
Currently, this function is passed through winevulkan to the system's Vulkan loader, which causes the loader to write the system's device layers properties in pProperties. UE4 then calls vkEnumerateDeviceExtensionProperties, using VkLayerProperties::layerName as the pLayerName. winevulkan's implementation of vkEnumerateDeviceExtensionProperties then returns VK_ERROR_LAYER_NOT_PRESENT, crashing UE4. If this is an okay description I can resubmit the patch.
On 3/17/21 10:44 PM, Victor Chiletto wrote:
On 16/03/2021 19:59, Liam Middlebrook wrote:
On 3/6/21 12:46 PM, Victor Hermann Chiletto wrote:
This is generally implemented by the system's Vulkan ICD, and passing it through results in applications (namely, Unreal Engine) attempting to
It would be nice to name a specific UE4 application you're seeing this with for posterity.
Project Wingman (Steam appid 895870), but inspection of the UE4 source code [1] shows that this issue will happen in any UE4 application that uses the Vulkan RHI.
[1] https://github.com/EpicGames/UnrealEngine/blob/5df54b7ef1714f28fb5da319c3e83...
Great, I'd just note that in the description as well (perhaps linking to the source would be most practical).
query for layer extensions in vkEnumerateDeviceExtensionProperties.
I think the wording is a bit off here. Did you mean to cite vkEnumerateDeviceLayerProperties() rather than vkEnumerateDeviceExtensionProperties() here? Rather than saying "layer extensions" I think it would make sense to say "device layers".
Currently, this function is passed through winevulkan to the system's Vulkan loader, which causes the loader to write the system's device layers properties in pProperties.
UE4 then calls vkEnumerateDeviceExtensionProperties, using VkLayerProperties::layerName as the pLayerName.
winevulkan's implementation of vkEnumerateDeviceExtensionProperties then returns VK_ERROR_LAYER_NOT_PRESENT, crashing UE4.
If this is an okay description I can resubmit the patch.
Yeah that looks good, it clears up the flow of events leading up to the crash, and why this change is not only more correct, but also necessary. Thanks, Liam Middlebrook
On 3/18/21 10:54 AM, Liam Middlebrook wrote:
On 3/17/21 10:44 PM, Victor Chiletto wrote:
On 16/03/2021 19:59, Liam Middlebrook wrote:
On 3/6/21 12:46 PM, Victor Hermann Chiletto wrote:
This is generally implemented by the system's Vulkan ICD, and passing it through results in applications (namely, Unreal Engine) attempting to
It would be nice to name a specific UE4 application you're seeing this with for posterity.
Project Wingman (Steam appid 895870), but inspection of the UE4 source code [1] shows that this issue will happen in any UE4 application that uses the Vulkan RHI.
[1] https://github.com/EpicGames/UnrealEngine/blob/5df54b7ef1714f28fb5da319c3e83...
Great, I'd just note that in the description as well (perhaps linking to the source would be most practical).
Actually I was logged into GitHub so I didn't notice that this is a private repository. I think just citing the game and noting this should affect all UE4 titles is more sensible than linking to a private repository on GitHub. Thanks, Liam Middlebrook
query for layer extensions in vkEnumerateDeviceExtensionProperties.
I think the wording is a bit off here. Did you mean to cite vkEnumerateDeviceLayerProperties() rather than vkEnumerateDeviceExtensionProperties() here? Rather than saying "layer extensions" I think it would make sense to say "device layers".
Currently, this function is passed through winevulkan to the system's Vulkan loader, which causes the loader to write the system's device layers properties in pProperties.
UE4 then calls vkEnumerateDeviceExtensionProperties, using VkLayerProperties::layerName as the pLayerName.
winevulkan's implementation of vkEnumerateDeviceExtensionProperties then returns VK_ERROR_LAYER_NOT_PRESENT, crashing UE4.
If this is an okay description I can resubmit the patch.
Yeah that looks good, it clears up the flow of events leading up to the crash, and why this change is not only more correct, but also necessary.
Thanks,
Liam Middlebrook
This matches my reading of the Vulkan specification, thanks for finding this. Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> On 3/6/21 12:46 PM, Victor Hermann Chiletto wrote:
The section for vkEnumerateInstanceLayerProperties states [1]:
On success, this command returns * VK_SUCCESS * VK_INCOMPLETE
On failure, this command returns * VK_ERROR_OUT_OF_HOST_MEMORY * VK_ERROR_OUT_OF_DEVICE_MEMORY
Always setting the layer count to zero and returning VK_SUCCESS is valid, as even if the function is given an array in pProperties, it's filled with zero layers
[1]: https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkEnum...
Signed-off-by: Victor Hermann Chiletto <v(a)hnn.net.br> --- dlls/winevulkan/vulkan.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c index d24fcb2f64b..971394eb9dd 100644 --- a/dlls/winevulkan/vulkan.c +++ b/dlls/winevulkan/vulkan.c @@ -1065,13 +1065,8 @@ VkResult WINAPI wine_vkEnumerateInstanceLayerProperties(uint32_t *count, VkLayer { TRACE("%p, %p\n", count, properties);
- if (!properties) - { - *count = 0; - return VK_SUCCESS; - } - - return VK_ERROR_LAYER_NOT_PRESENT; + *count = 0; + return VK_SUCCESS; }
VkResult WINAPI wine_vkEnumerateInstanceVersion(uint32_t *version)
participants (4)
-
Joshua Ashton -
Liam Middlebrook -
Victor Chiletto -
Victor Hermann Chiletto