From 2c25cd6fc0f76315f175e75f35881bffaf7222cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 24 Jan 2021 14:38:10 +0100 Subject: [PATCH] Vk: an ability to check if a particular extension revision is supported. Qt's QVulkanWindow has that and it seems useful, especially for weird extensions like VK_KHR_memory_model where their structure layout changes over time. --- src/Magnum/Vk/DeviceFeatures.h | 3 +- src/Magnum/Vk/ExtensionProperties.cpp | 22 +++++++++----- src/Magnum/Vk/ExtensionProperties.h | 30 +++++++++++-------- src/Magnum/Vk/Test/DevicePropertiesVkTest.cpp | 22 ++++++++++++++ .../Vk/Test/ExtensionPropertiesVkTest.cpp | 21 +++++++++++++ 5 files changed, 76 insertions(+), 22 deletions(-) diff --git a/src/Magnum/Vk/DeviceFeatures.h b/src/Magnum/Vk/DeviceFeatures.h index ae71805a1..4fe443858 100644 --- a/src/Magnum/Vk/DeviceFeatures.h +++ b/src/Magnum/Vk/DeviceFeatures.h @@ -1173,7 +1173,8 @@ enum class DeviceFeature: UnsignedShort { * chains with more than one element. * @see @ref DeviceFeature::VulkanMemoryModel, * @ref DeviceFeature::VulkanMemoryModelDeviceScope - * @requires_vk12 Extension @vk_extension{KHR,vulkan_memory_model} + * @requires_vk12 Extension @vk_extension{KHR,vulkan_memory_model} since + * revision 3 */ VulkanMemoryModelAvailabilityVisibilityChains, diff --git a/src/Magnum/Vk/ExtensionProperties.cpp b/src/Magnum/Vk/ExtensionProperties.cpp index f75951be2..a9d0bf8e9 100644 --- a/src/Magnum/Vk/ExtensionProperties.cpp +++ b/src/Magnum/Vk/ExtensionProperties.cpp @@ -113,12 +113,18 @@ Containers::ArrayView ExtensionProperties::names() return _names; } -bool ExtensionProperties::isSupported(const Containers::StringView extension) const { - return std::binary_search(_names.begin(), _names.end(), extension); +bool ExtensionProperties::isSupported(const Containers::StringView extension, const UnsignedInt revision) const { + /* Thanks, C++, for forcing me to have a larger bug surface instead of + providing a library helper to find the damn thing. */ + auto found = std::lower_bound(_names.begin(), _names.end(), extension); + + /* The view target is contents of the VkExtensionProperties structure, + the revision is stored nearby */ + return found != _names.end() && *found == extension && reinterpret_cast(found->data() - offsetof(VkExtensionProperties, extensionName))->specVersion >= revision; } -bool ExtensionProperties::isSupported(const Extension& extension) const { - return isSupported(extension.string()); +bool ExtensionProperties::isSupported(const Extension& extension, const UnsignedInt revision) const { + return isSupported(extension.string(), revision); } Containers::StringView ExtensionProperties::name(const UnsignedInt id) const { @@ -164,12 +170,12 @@ InstanceExtensionProperties::~InstanceExtensionProperties() = default; InstanceExtensionProperties& InstanceExtensionProperties::operator=(InstanceExtensionProperties&&) noexcept = default; -bool InstanceExtensionProperties::isSupported(Containers::StringView extension) const { - return ExtensionProperties::isSupported(extension); +bool InstanceExtensionProperties::isSupported(Containers::StringView extension, const UnsignedInt revision) const { + return ExtensionProperties::isSupported(extension, revision); } -bool InstanceExtensionProperties::isSupported(const InstanceExtension& extension) const { - return isSupported(extension.string()); +bool InstanceExtensionProperties::isSupported(const InstanceExtension& extension, const UnsignedInt revision) const { + return isSupported(extension.string(), revision); } UnsignedInt InstanceExtensionProperties::revision(Containers::StringView extension) const { diff --git a/src/Magnum/Vk/ExtensionProperties.h b/src/Magnum/Vk/ExtensionProperties.h index 9bf18b96a..303de7ef1 100644 --- a/src/Magnum/Vk/ExtensionProperties.h +++ b/src/Magnum/Vk/ExtensionProperties.h @@ -99,10 +99,14 @@ class MAGNUM_VK_EXPORT ExtensionProperties { /** * @brief Whether given extension is supported + * @param extension Extension string + * @param revision Minimal required revision. If the extension is + * present but in an older revision, the function returns + * @cpp false @ce. * - * Accepts extensions from the @ref Extension namespace as a template - * parameter. Use the other overloads to query support of a runtime - * extension or a plain extension string. + * Since extension strings are easy to mistype, you're encouraged to + * use the other overloads such as @ref isSupported(UnsignedInt) const + * together with extensions from the @ref Extensions namespace. * * Search complexity is @f$ \mathcal{O}(\log n) @f$ in the total * extension count; in contrast extension queries on a created instance @@ -110,15 +114,15 @@ class MAGNUM_VK_EXPORT ExtensionProperties { * @see @ref revision(), @ref Instance::isExtensionEnabled(), * @ref Device::isExtensionEnabled() */ - bool isSupported(Containers::StringView extension) const; + bool isSupported(Containers::StringView extension, UnsignedInt revision = 1) const; /** @overload */ - bool isSupported(const Extension& extension) const; + bool isSupported(const Extension& extension, UnsignedInt revision = 1) const; /** @overload */ - template bool isSupported() const { - static_assert(Implementation::IsExtension::value, "expected a Vulkan deviceension"); - return isSupported(E::string()); + template bool isSupported(UnsignedInt revision = 1) const { + static_assert(Implementation::IsExtension::value, "expected a Vulkan device extension"); + return isSupported(E::string(), revision); } /** @@ -219,16 +223,16 @@ class MAGNUM_VK_EXPORT InstanceExtensionProperties: public ExtensionProperties { /** @brief Move assignment */ InstanceExtensionProperties& operator=(InstanceExtensionProperties&&) noexcept; - /** @copydoc ExtensionProperties::isSupported(Containers::StringView) const */ - bool isSupported(Containers::StringView extension) const; + /** @copydoc ExtensionProperties::isSupported(Containers::StringView, UnsignedInt) const */ + bool isSupported(Containers::StringView extension, UnsignedInt revision = 1) const; /** @overload */ - bool isSupported(const InstanceExtension& extension) const; + bool isSupported(const InstanceExtension& extension, UnsignedInt revision = 1) const; /** @overload */ - template bool isSupported() const { + template bool isSupported(UnsignedInt revision = 1) const { static_assert(Implementation::IsInstanceExtension::value, "expected a Vulkan instance extension"); - return isSupported(E::string()); + return isSupported(E::string(), revision); } /** @copydoc ExtensionProperties::revision(UnsignedInt) const */ diff --git a/src/Magnum/Vk/Test/DevicePropertiesVkTest.cpp b/src/Magnum/Vk/Test/DevicePropertiesVkTest.cpp index 404310dd4..02dddc373 100644 --- a/src/Magnum/Vk/Test/DevicePropertiesVkTest.cpp +++ b/src/Magnum/Vk/Test/DevicePropertiesVkTest.cpp @@ -64,6 +64,7 @@ struct DevicePropertiesVkTest: VulkanTester { void extensionConstructMove(); void extensionIsSupported(); + void extensionIsSupportedRevision(); void extensionNamedRevision(); void driverProperties(); @@ -118,6 +119,7 @@ DevicePropertiesVkTest::DevicePropertiesVkTest(): VulkanTester{NoCreate} { &DevicePropertiesVkTest::extensionConstructMove, &DevicePropertiesVkTest::extensionIsSupported, + &DevicePropertiesVkTest::extensionIsSupportedRevision, &DevicePropertiesVkTest::extensionNamedRevision, &DevicePropertiesVkTest::driverProperties, @@ -398,6 +400,26 @@ void DevicePropertiesVkTest::extensionIsSupported() { CORRADE_VERIFY(properties.isSupported(Extensions::KHR::maintenance1{})); } +void DevicePropertiesVkTest::extensionIsSupportedRevision() { + Containers::Array devices = enumerateDevices(instance()); + CORRADE_VERIFY(!devices.empty()); + + ExtensionProperties properties = devices[0].enumerateExtensionProperties(); + + /* This extension should be available almost always */ + if(!properties.isSupported("VK_KHR_maintenance1")) + CORRADE_SKIP("VK_KHR_maintenance1 not supported, can't fully test"); + + UnsignedInt revision = properties.revision("VK_KHR_maintenance1"); + + CORRADE_VERIFY(properties.isSupported("VK_KHR_maintenance1", revision)); + CORRADE_VERIFY(properties.isSupported(revision)); + CORRADE_VERIFY(properties.isSupported(Extensions::KHR::maintenance1{}, revision)); + CORRADE_VERIFY(!properties.isSupported("VK_KHR_maintenance1", revision + 1)); + CORRADE_VERIFY(!properties.isSupported(revision + 1)); + CORRADE_VERIFY(!properties.isSupported(Extensions::KHR::maintenance1{}, revision + 1)); +} + void DevicePropertiesVkTest::extensionNamedRevision() { Containers::Array devices = enumerateDevices(instance()); CORRADE_VERIFY(!devices.empty()); diff --git a/src/Magnum/Vk/Test/ExtensionPropertiesVkTest.cpp b/src/Magnum/Vk/Test/ExtensionPropertiesVkTest.cpp index d57deaf11..dbcb556e2 100644 --- a/src/Magnum/Vk/Test/ExtensionPropertiesVkTest.cpp +++ b/src/Magnum/Vk/Test/ExtensionPropertiesVkTest.cpp @@ -47,6 +47,7 @@ struct ExtensionPropertiesVkTest: TestSuite::Tester { void enumerateInstanceWithKhronosValidationLayer(); void enumerateInstanceNonexistentLayer(); void instanceExtensionIsSupported(); + void instanceExtensionIsSupportedRevision(); /* Device extensions tested in DevicePropertiesVkTest */ void outOfRange(); @@ -60,6 +61,7 @@ ExtensionPropertiesVkTest::ExtensionPropertiesVkTest() { &ExtensionPropertiesVkTest::enumerateInstanceWithKhronosValidationLayer, &ExtensionPropertiesVkTest::enumerateInstanceNonexistentLayer, &ExtensionPropertiesVkTest::instanceExtensionIsSupported, + &ExtensionPropertiesVkTest::instanceExtensionIsSupportedRevision, &ExtensionPropertiesVkTest::outOfRange, &ExtensionPropertiesVkTest::namedRevision}); @@ -161,6 +163,9 @@ void ExtensionPropertiesVkTest::instanceExtensionIsSupported() { } CORRADE_VERIFY(!properties.isSupported("VK_this_doesnt_exist")); + /* Verify that we don't dereference garbage when std::lower_bound() returns + `last` */ + CORRADE_VERIFY(!properties.isSupported("ZZZZZ")); /* Verify that we're not just comparing a prefix */ const std::string extension = std::string(properties.name(0)) + "_hello"; @@ -175,6 +180,22 @@ void ExtensionPropertiesVkTest::instanceExtensionIsSupported() { CORRADE_VERIFY(properties.isSupported(Extensions::KHR::get_physical_device_properties2{})); } +void ExtensionPropertiesVkTest::instanceExtensionIsSupportedRevision() { + InstanceExtensionProperties properties = enumerateInstanceExtensionProperties(); + /** @todo use Extensions::KHR::surface once the extension is recognized */ + if(!properties.isSupported("VK_KHR_get_physical_device_properties2")) + CORRADE_SKIP("VK_KHR_get_physical_device_properties2 not supported, can't test"); + + UnsignedInt revision = properties.revision("VK_KHR_get_physical_device_properties2"); + + CORRADE_VERIFY(properties.isSupported("VK_KHR_get_physical_device_properties2", revision)); + CORRADE_VERIFY(properties.isSupported(revision)); + CORRADE_VERIFY(properties.isSupported(Extensions::KHR::get_physical_device_properties2{}, revision)); + CORRADE_VERIFY(!properties.isSupported("VK_KHR_get_physical_device_properties2", revision + 1)); + CORRADE_VERIFY(!properties.isSupported(revision + 1)); + CORRADE_VERIFY(!properties.isSupported(Extensions::KHR::get_physical_device_properties2{}, revision + 1)); +} + void ExtensionPropertiesVkTest::outOfRange() { #ifdef CORRADE_NO_ASSERT CORRADE_SKIP("CORRADE_NO_ASSERT defined, can't test assertions");