search

kondrak / vkQuake2

id Software's Quake 2 v3.21 with mission packs and Vulkan support (Windows, Linux, macOS, FreeBSD, Raspberry Pi 4)
GNU General Public License v2.0
896 stars 91 forks source link

Invalid usage of vkGetPhysicalDeviceSurfaceFormatsKHR #118

Closed Nodli closed 3 years ago

Nodli commented 3 years ago

Hi,

https://github.com/kondrak/vkQuake2/blob/ef49294ae860a4892d9caf82fef8b494b0f9e1a0/ref_vk/vk_device.c#L82 https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/vkGetPhysicalDeviceSurfaceFormatsKHR.html

vkGetPhysicalDeviceSurfaceFormatsKHR is used to determine if the device can be used with the surface but the spec says in "Valid Usage" that the surface parameter "must be supported by physicalDevice, as reported by vkGetPhysicalDeviceSurfaceSupportKHR or an equivalent platform-specific mechanism". Moreover "The number of format pairs supported must be greater than or equal to 1" which means that the formatCount == 0 check is unnecessary.

From what i read in the spec, it seems that the right thing to do would be to remove the call to vkGetPhysicalDeviceSurfaceFormatsKHR or put it after vkGetPhysicalDeviceSurfaceSupportKHR .

Nodli commented 3 years ago

https://github.com/kondrak/vkQuake2/blob/ef49294ae860a4892d9caf82fef8b494b0f9e1a0/ref_vk/vk_device.c#L83 https://www.khronos.org/registry/vulkan/specs/1.2-extensions/man/html/VkPresentModeKHR.html

The call to vkGetPhysicalDeviceSurfacePresentModesKHR could be removed too. The call is not invalid but VK_PRESENT_MODE_FIFO_KHR is required to be supported by the spec (see Description above). As long as it is checked that the device suports presentation with vkGetPhysicalDeviceSurfaceSupportKHR, then we are guaranteed to have VK_PRESENT_MODE_FIFO_KHR when calling vkGetPhysicalDeviceSurfacePresentModesKHR and the presentModesCount == 0 check is unnecessary.

kondrak commented 3 years ago

Thanks, this is a valid point and the calls are in fact redundant in this case.