Re: [PATCH] winevulkan: Implement vk_icdGetPhysicalDeviceProcAddr

[Liam Middlebrook]

Signed-off-by: Liam Middlebrook

One non-review comment written inline.

On 3/22/21 6:02 PM, Joshua Ashton wrote:
> Needed for Red Dead Redemption 2 under vkd3d-proton.
>
> RDR2 requires us to use the native Vulkan loader, and vkd3d-proton recently enabled VK_KHR_fragment_shading_rate which exposes physical device procs unknown to the loader.
>
> Previously, when we queried vkGetPhysicalDeviceFragmentShadingRatesKHR via. vkGetInstanceProcAddr, the native loader tried to call vk_icdGetPhysicalDeviceProcAddr from the WineVulkan ICD to resolve this, which didn't exist causing the loader to nullptr itself and crash.
>
> This commit implements the vk_icdGetPhysicalDeviceProcAddr export, required for ICD version 4, via. forwarding to wine_vkGetInstanceProcAddr and fixes that problem.
> Signed-off-by: Joshua Ashton
>
> dlls/winevulkan/make_vulkan |  1 +
>   dlls/winevulkan/vulkan.c    | 15 ++++++++++++---
>   2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan
> index 3ad210e4701..f32ba9b90f1 100755
> --- a/dlls/winevulkan/make_vulkan
> +++ b/dlls/winevulkan/make_vulkan
> @@ -2647,6 +2647,7 @@ class VkGenerator(object):
>       def generate_vulkan_spec(self, f):
>           self._generate_copyright(f, spec_file=True)
>           f.write("@ stdcall -private vk_icdGetInstanceProcAddr(ptr str) wine_vk_icdGetInstanceProcAddr\n")
> -
>    f.write("@ stdcall -private vk_icdGetPhysicalDeviceProcAddr(ptr str) wine_vk_icdGetPhysicalDeviceProcAddr\n")
>     f.write("@ stdcall -private vk_icdNegotiateLoaderICDInterfaceVersion(ptr) wine_vk_icdNegotiateLoaderICDInterfaceVersion\n")
>     f.write("@ cdecl -norelay native_vkGetInstanceProcAddrWINE(ptr str)\n")
>
> diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
> index 971394eb9dd..2e02714a9fd 100644
> --- a/dlls/winevulkan/vulkan.c
> +++ b/dlls/winevulkan/vulkan.c
> @@ -37,9 +37,7 @@ DEFINE_DEVPROPKEY(DEVPROPKEY_GPU_LUID, 0x60b193cb, 0x5276, 0x4d0f, 0x96, 0xfc, 0
>   DEFINE_DEVPROPKEY(WINE_DEVPROPKEY_GPU_VULKAN_UUID, 0x233a9ef3, 0xafc4, 0x4abd, 0xb5, 0x64, 0xc3, 0x2f, 0x21, 0xf1, 0x53, 0x5c, 2);
>
> /* For now default to 4 as it felt like a reasonable version feature wise to support.
> -
> - Don't support the optional vk_icdGetPhysicalDeviceProcAddr introduced in this version
> -
> - as it is unlikely we will implement physical device extensions, which the loader is not
> -
> - aware of. Version 5 adds more extensive version checks. Something to tackle later.
> -
> - Version 5 adds more extensive version checks. Something to tackle later.
> */
> #define WINE_VULKAN_ICD_VERSION 4
>
> @@ -1239,6 +1237,17 @@ void * WINAPI wine_vk_icdGetInstanceProcAddr(VkInstance instance, const char *na
>       return wine_vkGetInstanceProcAddr(instance, name);
>   }
>
> +void * WINAPI wine_vk_icdGetPhysicalDeviceProcAddr(VkInstance instance, const char *name)
> +{
> - TRACE("%p, %s\n", instance, debugstr_a(name));
> -
> - /* This is exported directly by the ICD, despite the loader documentation
> -
> * stating that this is queried via vk_icdGetInstanceProcAddr
> -
> */

Could you file an issue / send a pull-request to the loader repository 
that addresses this hole in documentation?

Thanks,

Liam Middlebrook
> -
> - return wine_vkGetInstanceProcAddr(instance, name);
>
> +}
> - VkResult WINAPI wine_vk_icdNegotiateLoaderICDInterfaceVersion(uint32_t *supported_version)
> {
>     uint32_t req_version;