Re: [PATCH] winevulkan: Avoid returning 0 for swapchain maxImageCount.

[Liam Middlebrook]

On 7/29/20 10:45 AM, Georg Lehmann wrote:
> Many Windows games do not expect that maxImageCount can be set to 0.
> A value of 0 means that there is no limit on the number of images.
> Nvidia reports 8 on Windows, AMD 16.
>
> Based on a patch by Józef Kucia.
> Signed-off-by: Georg Lehmann
>
> dlls/winevulkan/make_vulkan     |  4 ++--
>   dlls/winevulkan/vulkan.c        | 41 +++++++++++++++++++++++++++++++++
>   dlls/winevulkan/vulkan_thunks.c |  8 ++-----
>   dlls/winevulkan/vulkan_thunks.h |  4 ++++
>   4 files changed, 49 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan
> index fc0e2182dd..6a039bd268 100755
> --- a/dlls/winevulkan/make_vulkan
> +++ b/dlls/winevulkan/make_vulkan
> @@ -183,12 +183,12 @@ FUNCTION_OVERRIDES = {
>       # VK_KHR_surface
>       "vkDestroySurfaceKHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
>       "vkGetPhysicalDeviceSurfaceSupportKHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
> - "vkGetPhysicalDeviceSurfaceCapabilitiesKHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
> -
>
> "vkGetPhysicalDeviceSurfaceCapabilitiesKHR" : {"dispatch" : True, "driver" : True, "thunk" : False, "private_thunk" : True},
>  "vkGetPhysicalDeviceSurfaceFormatsKHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
>  "vkGetPhysicalDeviceSurfacePresentModesKHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
>
> # VK_KHR_get_surface_capabilities2
> - "vkGetPhysicalDeviceSurfaceCapabilities2KHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
> -
>
> "vkGetPhysicalDeviceSurfaceCapabilities2KHR" : {"dispatch" : True, "driver" : True, "thunk" : False, "private_thunk" : True},
>  "vkGetPhysicalDeviceSurfaceFormats2KHR" : {"dispatch" : True, "driver" : True, "thunk" : True},
>
> # VK_KHR_win32_surface
>
> diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
> index be614f1cef..1e83defa18 100644
> --- a/dlls/winevulkan/vulkan.c
> +++ b/dlls/winevulkan/vulkan.c
> @@ -1555,6 +1555,47 @@ void WINAPI wine_vkGetPrivateDataEXT(VkDevice device, VkObjectType object_type,
>       device->funcs.p_vkGetPrivateDataEXT(device->device, object_type, object_handle, private_data_slot, data);
>   }
>
> +static inline void adjust_max_image_count(VkSurfaceCapabilitiesKHR* capabilities)
> +{
> - /* Many Windows games do not expect that maxImageCount can be set to 0.
> -
> * A value of 0 means that there is no limit on the number of images.
> -
> * Nvidia reports 8 on Windows, AMD 16.*/

I think it would be nice to list out some of these games and maybe link 
to some reports showing off the maxImageCount values you're describing.

Should this be behind a new "quirk" 
WINEVULKAN_QUIRK_ADJUST_MAX_IMAGE_COUNT ? I think it would be fine to 
have this quirk enabled by default (it seems harmless if my comment on 
following the spec below is addressed).

Giving it a quirk bit should make it easy for someone to add a mechanism 
to disable this in the future if it becomes an issue (or they want a 
quirkless implementation for whatever reason).
> - if (!capabilities->maxImageCount)
> - {
> -
>    capabilities->maxImageCount = 16;

Would it be better if this was max(minImageCount, 16) ? That way we're 
ensuring that this override doesn't go against the Vulkan specification.

Thanks,

Liam Middlebrook
> - }
>
> +}
> -
>
> +VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice phys_dev,
> -
>    VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR *capabilities)
>
> +{
> - VkResult res;
> -
> - TRACE("%p, 0x%s, %p\n", phys_dev, wine_dbgstr_longlong(surface), capabilities);
> -
> - res = thunk_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev, surface, capabilities);
> -
> - if (res == VK_SUCCESS)
> -
>    adjust_max_image_count(capabilities);
> -
> - return res;
>
> +}
> -
>
> +VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice phys_dev,
> -
>    const VkPhysicalDeviceSurfaceInfo2KHR *surface_info, VkSurfaceCapabilities2KHR *capabilities)
>
> +{
> - VkResult res;
> -
> - TRACE("%p, %p, %p\n", phys_dev, surface_info, capabilities);
> -
> - res = thunk_vkGetPhysicalDeviceSurfaceCapabilities2KHR(phys_dev, surface_info, capabilities);
> -
> - if (res == VK_SUCCESS)
> -
>    adjust_max_image_count(&capabilities->surfaceCapabilities);
> -
> - return res;
>
> +}
> - BOOL WINAPI DllMain(HINSTANCE hinst, DWORD reason, void *reserved)
> {
>     TRACE("%p, %u, %p\n", hinst, reason, reserved);
>
> diff --git a/dlls/winevulkan/vulkan_thunks.c b/dlls/winevulkan/vulkan_thunks.c
> index fcefa6a767..4f75a4ec7c 100644
> --- a/dlls/winevulkan/vulkan_thunks.c
> +++ b/dlls/winevulkan/vulkan_thunks.c
> @@ -4998,26 +4998,22 @@ static VkResult WINAPI wine_vkGetPhysicalDeviceSupportedFramebufferMixedSamplesC
>       return physicalDevice->instance->funcs.p_vkGetPhysicalDeviceSupportedFramebufferMixedSamplesCombinationsNV(physicalDevice->phys_dev, pCombinationCount, pCombinations);
>   }
>
> -VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceSurfaceInfo2KHR *pSurfaceInfo, VkSurfaceCapabilities2KHR *pSurfaceCapabilities)
> +VkResult thunk_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceSurfaceInfo2KHR *pSurfaceInfo, VkSurfaceCapabilities2KHR *pSurfaceCapabilities)
>   {
>   #if defined(USE_STRUCT_CONVERSION)
>       VkResult result;
>       VkPhysicalDeviceSurfaceInfo2KHR_host pSurfaceInfo_host;
> -
>
> TRACE("%p, %p, %p\n", physicalDevice, pSurfaceInfo, pSurfaceCapabilities);
> -
> convert_VkPhysicalDeviceSurfaceInfo2KHR_win_to_host(pSurfaceInfo, &pSurfaceInfo_host);
> result = physicalDevice->instance->funcs.p_vkGetPhysicalDeviceSurfaceCapabilities2KHR(physicalDevice->phys_dev, &pSurfaceInfo_host, pSurfaceCapabilities);
>
> return result;
>
> #else
> -
>
> TRACE("%p, %p, %p\n", physicalDevice, pSurfaceInfo, pSurfaceCapabilities);
>  return physicalDevice->instance->funcs.p_vkGetPhysicalDeviceSurfaceCapabilities2KHR(physicalDevice->phys_dev, pSurfaceInfo, pSurfaceCapabilities);
> #endif
> }
>
> -VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR *pSurfaceCapabilities)
> +VkResult thunk_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR *pSurfaceCapabilities)
>   {
> - TRACE("%p, 0x%s, %p\n", physicalDevice, wine_dbgstr_longlong(surface), pSurfaceCapabilities);
>  return physicalDevice->instance->funcs.p_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(physicalDevice->phys_dev, surface, pSurfaceCapabilities);
> }
>
> diff --git a/dlls/winevulkan/vulkan_thunks.h b/dlls/winevulkan/vulkan_thunks.h
> index f2d650decf..d35e8c4269 100644
> --- a/dlls/winevulkan/vulkan_thunks.h
> +++ b/dlls/winevulkan/vulkan_thunks.h
> @@ -42,6 +42,8 @@ VkResult WINAPI wine_vkGetPhysicalDeviceImageFormatProperties2(VkPhysicalDevice
>   VkResult WINAPI wine_vkGetPhysicalDeviceImageFormatProperties2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceImageFormatInfo2 *pImageFormatInfo, VkImageFormatProperties2 *pImageFormatProperties) DECLSPEC_HIDDEN;
>   void WINAPI wine_vkGetPhysicalDeviceProperties2(VkPhysicalDevice physicalDevice, VkPhysicalDeviceProperties2 *pProperties);
>   void WINAPI wine_vkGetPhysicalDeviceProperties2KHR(VkPhysicalDevice physicalDevice, VkPhysicalDeviceProperties2 *pProperties) DECLSPEC_HIDDEN;
> +VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceSurfaceInfo2KHR *pSurfaceInfo, VkSurfaceCapabilities2KHR *pSurfaceCapabilities);
> +VkResult WINAPI wine_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR *pSurfaceCapabilities);
>   void WINAPI wine_vkGetPrivateDataEXT(VkDevice device, VkObjectType objectType, uint64_t objectHandle, VkPrivateDataSlotEXT privateDataSlot, uint64_t *pData) DECLSPEC_HIDDEN;
>   VkResult WINAPI wine_vkQueueSubmit(VkQueue queue, uint32_t submitCount, const VkSubmitInfo *pSubmits, VkFence fence);
>   VkResult WINAPI wine_vkSetPrivateDataEXT(VkDevice device, VkObjectType objectType, uint64_t objectHandle, VkPrivateDataSlotEXT privateDataSlot, uint64_t data) DECLSPEC_HIDDEN;
> @@ -51,6 +53,8 @@ VkResult thunk_vkGetPhysicalDeviceImageFormatProperties2(VkPhysicalDevice physic
>   VkResult thunk_vkGetPhysicalDeviceImageFormatProperties2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceImageFormatInfo2 *pImageFormatInfo, VkImageFormatProperties2 *pImageFormatProperties) DECLSPEC_HIDDEN;
>   void thunk_vkGetPhysicalDeviceProperties2(VkPhysicalDevice physicalDevice, VkPhysicalDeviceProperties2 *pProperties) DECLSPEC_HIDDEN;
>   void thunk_vkGetPhysicalDeviceProperties2KHR(VkPhysicalDevice physicalDevice, VkPhysicalDeviceProperties2 *pProperties) DECLSPEC_HIDDEN;
> +VkResult thunk_vkGetPhysicalDeviceSurfaceCapabilities2KHR(VkPhysicalDevice physicalDevice, const VkPhysicalDeviceSurfaceInfo2KHR *pSurfaceInfo, VkSurfaceCapabilities2KHR *pSurfaceCapabilities) DECLSPEC_HIDDEN;
> +VkResult thunk_vkGetPhysicalDeviceSurfaceCapabilitiesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, VkSurfaceCapabilitiesKHR *pSurfaceCapabilities) DECLSPEC_HIDDEN;
>
> typedef struct VkAcquireNextImageInfoKHR_host
>   {