KhronosGroup / Vulkan-Samples

One stop solution for all Vulkan samples
Apache License 2.0
4.36k stars 650 forks source link

Use Vulkan API 1.0 support funcs for Vulkan API 1.0 sample host_image_copy #1174

Closed SRSaunders closed 1 month ago

SRSaunders commented 2 months ago

Description

This PR replaces Vulkan 1.1 API support function vkGetPhysicalDeviceFormatProperties2() with Vulkan 1.0 equivalent vkGetPhysicalDeviceFormatProperties2KHR() for Vulkan API 1.0 sample _host_imagecopy. This prevents runtime failures (undefined functions) on Vulkan drivers that enforce the specified API version.

I did not up-version the sample to Vulkan API 1.1 since it appears that Vulkan API 1.0 was the intended API version, with extensions enabled by VK_KHR_get_physical_device_properties2.

Tested on macOS Ventura and iOS 17 using both the Vulkan loader and MoltenVK standalone.

General Checklist:

Please ensure the following points are checked:

Sample Checklist

If your PR contains a new or modified sample, these further checks must be carried out in addition to the General Checklist:

SaschaWillems commented 2 months ago

Shouldn't we also use VkFormatProperties2KHR instead of VkFormatProperties2 then?

SRSaunders commented 2 months ago

Shouldn't we also use VkFormatProperties2KHR instead of VkFormatProperties2 then?

That's a natural question, but the Vulkan Spec seems to say otherwise. See https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/vkGetPhysicalDeviceFormatProperties2.html. Both versions of the function (API 1.1 and 1.0) reference VkFormatProperties2, and this seems to be borne out by the header files.

However, for readability and consistency, I could make the change since the following is also true (see https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VkFormatProperties2.html):

 // Provided by VK_KHR_get_physical_device_properties2
typedef VkFormatProperties2 VkFormatProperties2KHR;

The current change works as is, but please let me know what you would prefer.

SaschaWillems commented 2 months ago

If we use the KHR suffix in one place, we should use it in the other places too to make it consistent.