From 96657394ec6725effe18a2af0516e7f9941811b1 Mon Sep 17 00:00:00 2001 From: Iason Paraskevopoulos Date: Mon, 8 Nov 2021 13:32:12 +0000 Subject: [PATCH] Store enabled instance and device extensions Stores the enabled instance and device extensions during vkCreateInstance and vkCreateDevice respectively. This fixes an issue where function pointers for functions of disabled extensions were returned during vkGetInstanceProcAddr/vkGetDeviceProcAddr. Adds functionality for adding extensions that belong to a subset of other extensions in util::extension_list. Adds function for checking if the proper surface extension has been enabled in each WSI backend. Signed-off-by: Iason Paraskevopoulos Change-Id: If5e23e0d07c9f09006be18c410c4e1d4c3a1e676 --- layer/layer.cpp | 87 ++++++++++++++++++++++++++----------- layer/private_data.cpp | 43 +++++++++++++++--- layer/private_data.hpp | 49 +++++++++++++++++++++ util/extension_list.cpp | 27 ++++++++++++ util/extension_list.hpp | 15 +++++++ wsi/headless/surface_properties.cpp | 5 +++ wsi/headless/surface_properties.hpp | 2 + wsi/surface_properties.hpp | 6 +++ wsi/wayland/surface_properties.cpp | 5 +++ wsi/wayland/surface_properties.hpp | 2 + wsi/wsi_factory.cpp | 4 +- wsi/wsi_factory.hpp | 3 +- 12 files changed, 215 insertions(+), 33 deletions(-) diff --git a/layer/layer.cpp b/layer/layer.cpp index 9282662..e2b4c8d 100644 --- a/layer/layer.cpp +++ b/layer/layer.cpp @@ -152,7 +152,6 @@ VKAPI_ATTR VkResult create_instance(const VkInstanceCreateInfo *pCreateInfo, con /* Find all the platforms that the layer can handle based on pCreateInfo->ppEnabledExtensionNames. */ auto layer_platforms_to_enable = wsi::find_enabled_layer_platforms(pCreateInfo); - /* Following the spec: use the callbacks provided to vkCreateInstance() if not nullptr, * otherwise use the default callbacks. */ @@ -165,9 +164,26 @@ VKAPI_ATTR VkResult create_instance(const VkInstanceCreateInfo *pCreateInfo, con { table.DestroyInstance(*pInstance, pAllocator); } + return result; } - return result; + /* + * Store the enabled instance extensions in order to return nullptr in + * vkGetInstanceProcAddr for functions of disabled extensions. + */ + result = + instance_private_data::get(*pInstance) + .set_instance_enabled_extensions(pCreateInfo->ppEnabledExtensionNames, pCreateInfo->enabledExtensionCount); + if (result != VK_SUCCESS) + { + if (table.DestroyInstance != nullptr) + { + table.DestroyInstance(*pInstance, pAllocator); + } + return result; + } + + return VK_SUCCESS; } VKAPI_ATTR VkResult create_device(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo, @@ -240,7 +256,6 @@ VKAPI_ATTR VkResult create_device(VkPhysicalDevice physicalDevice, const VkDevic { table.DestroyDevice(*pDevice, pAllocator); } - return result; } @@ -250,16 +265,31 @@ VKAPI_ATTR VkResult create_device(VkPhysicalDevice physicalDevice, const VkDevic util::allocator device_allocator{ inst_data.get_allocator(), VK_SYSTEM_ALLOCATION_SCOPE_DEVICE, pAllocator }; result = device_private_data::associate(*pDevice, inst_data, physicalDevice, table, loader_callback, device_allocator); + if (result != VK_SUCCESS) + { + if (table.DestroyDevice != nullptr) + { + table.DestroyDevice(*pDevice, pAllocator); + } + return result; + } + /* + * Store the enabled device extensions in order to return nullptr in + * vkGetDeviceProcAddr for functions of disabled extensions. + */ + result = layer::device_private_data::get(*pDevice).set_device_enabled_extensions( + modified_info.ppEnabledExtensionNames, modified_info.enabledExtensionCount); if (result != VK_SUCCESS) { if (table.DestroyDevice != nullptr) { table.DestroyDevice(*pDevice, pAllocator); } + return result; } - return result; + return VK_SUCCESS; } } /* namespace layer */ @@ -350,16 +380,19 @@ VK_LAYER_EXPORT wsi_layer_vkNegotiateLoaderLayerInterfaceVersion(VkNegotiateLaye VWL_VKAPI_CALL(PFN_vkVoidFunction) wsi_layer_vkGetDeviceProcAddr(VkDevice device, const char *funcName) VWL_API_POST { - GET_PROC_ADDR(vkCreateSwapchainKHR); - GET_PROC_ADDR(vkDestroySwapchainKHR); - GET_PROC_ADDR(vkGetSwapchainImagesKHR); - GET_PROC_ADDR(vkAcquireNextImageKHR); - GET_PROC_ADDR(vkQueuePresentKHR); + if (layer::device_private_data::get(device).is_device_extension_enabled(VK_KHR_SWAPCHAIN_EXTENSION_NAME)) + { + GET_PROC_ADDR(vkCreateSwapchainKHR); + GET_PROC_ADDR(vkDestroySwapchainKHR); + GET_PROC_ADDR(vkGetSwapchainImagesKHR); + GET_PROC_ADDR(vkAcquireNextImageKHR); + GET_PROC_ADDR(vkQueuePresentKHR); + GET_PROC_ADDR(vkAcquireNextImage2KHR); + GET_PROC_ADDR(vkGetDeviceGroupPresentCapabilitiesKHR); + GET_PROC_ADDR(vkGetDeviceGroupSurfacePresentModesKHR); + } GET_PROC_ADDR(vkDestroyDevice); - GET_PROC_ADDR(vkGetDeviceGroupSurfacePresentModesKHR); - GET_PROC_ADDR(vkGetDeviceGroupPresentCapabilitiesKHR); - GET_PROC_ADDR(vkAcquireNextImage2KHR); GET_PROC_ADDR(vkCreateImage); GET_PROC_ADDR(vkBindImageMemory2); @@ -369,24 +402,28 @@ wsi_layer_vkGetDeviceProcAddr(VkDevice device, const char *funcName) VWL_API_POS VWL_VKAPI_CALL(PFN_vkVoidFunction) wsi_layer_vkGetInstanceProcAddr(VkInstance instance, const char *funcName) VWL_API_POST { - PFN_vkVoidFunction wsi_func = wsi::get_proc_addr(funcName); - if (wsi_func) - { - return wsi_func; - } - GET_PROC_ADDR(vkGetDeviceProcAddr); GET_PROC_ADDR(vkGetInstanceProcAddr); GET_PROC_ADDR(vkCreateInstance); GET_PROC_ADDR(vkDestroyInstance); GET_PROC_ADDR(vkCreateDevice); - GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceSupportKHR); - GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceCapabilitiesKHR); - GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceFormatsKHR); - GET_PROC_ADDR(vkGetPhysicalDeviceSurfacePresentModesKHR); - GET_PROC_ADDR(vkDestroySurfaceKHR); - GET_PROC_ADDR(vkGetPhysicalDevicePresentRectanglesKHR); - return layer::instance_private_data::get(instance).disp.GetInstanceProcAddr(instance, funcName); + auto &instance_data = layer::instance_private_data::get(instance); + if (instance_data.is_instance_extension_enabled(VK_KHR_SURFACE_EXTENSION_NAME)) + { + PFN_vkVoidFunction wsi_func = wsi::get_proc_addr(funcName, instance_data); + if (wsi_func) + { + return wsi_func; + } + + GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceSupportKHR); + GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceCapabilitiesKHR); + GET_PROC_ADDR(vkGetPhysicalDeviceSurfaceFormatsKHR); + GET_PROC_ADDR(vkGetPhysicalDeviceSurfacePresentModesKHR); + GET_PROC_ADDR(vkDestroySurfaceKHR); + } + + return instance_data.disp.GetInstanceProcAddr(instance, funcName); } diff --git a/layer/private_data.cpp b/layer/private_data.cpp index 6fa3d38..4e377a2 100644 --- a/layer/private_data.cpp +++ b/layer/private_data.cpp @@ -40,6 +40,14 @@ static std::mutex g_data_lock; static util::unordered_map g_instance_data{ util::allocator::get_generic() }; static util::unordered_map g_device_data{ util::allocator::get_generic() }; +static const std::array supported_instance_extensions = { + VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME, VK_EXT_HEADLESS_SURFACE_EXTENSION_NAME, +}; + +static const std::array supported_device_extensions = { + VK_KHR_SWAPCHAIN_EXTENSION_NAME, +}; + template static PFN_vkVoidFunction get_proc_helper(object_type obj, get_proc_type get_proc, const char* proc_name, bool required, bool &ok) @@ -78,11 +86,12 @@ instance_private_data::instance_private_data(const instance_dispatch_table &tabl PFN_vkSetInstanceLoaderData set_loader_data, util::wsi_platform_set enabled_layer_platforms, const util::allocator &alloc) - : disp(table) - , SetInstanceLoaderData(set_loader_data) - , enabled_layer_platforms(enabled_layer_platforms) - , allocator(alloc) - , surfaces(alloc) + : disp{ table } + , SetInstanceLoaderData{ set_loader_data } + , enabled_layer_platforms{ enabled_layer_platforms } + , allocator{ alloc } + , surfaces{ alloc } + , enabled_extensions{ allocator } { } @@ -261,6 +270,18 @@ bool instance_private_data::should_layer_handle_surface(VkPhysicalDevice phys_de return ret; } +VkResult instance_private_data::set_instance_enabled_extensions(const char *const *extension_names, + size_t extension_count) +{ + return enabled_extensions.add(extension_names, extension_count, supported_instance_extensions.data(), + supported_instance_extensions.size()); +} + +bool instance_private_data::is_instance_extension_enabled(const char *extension_name) const +{ + return enabled_extensions.contains(extension_name); +} + device_private_data::device_private_data(instance_private_data &inst_data, VkPhysicalDevice phys_dev, VkDevice dev, const device_dispatch_table &table, PFN_vkSetDeviceLoaderData set_loader_data, const util::allocator &alloc) @@ -271,6 +292,7 @@ device_private_data::device_private_data(instance_private_data &inst_data, VkPhy , device{ dev } , allocator{ alloc } , swapchains{ allocator } + , enabled_extensions{ allocator } { } @@ -382,6 +404,17 @@ bool device_private_data::can_icds_create_swapchain(VkSurfaceKHR vk_surface) return disp.CreateSwapchainKHR != nullptr; } +VkResult device_private_data::set_device_enabled_extensions(const char *const *extension_names, size_t extension_count) +{ + return enabled_extensions.add(extension_names, extension_count, supported_device_extensions.data(), + supported_device_extensions.size()); +} + +bool device_private_data::is_device_extension_enabled(const char *extension_name) const +{ + return enabled_extensions.contains(extension_name); +} + void device_private_data::destroy(device_private_data *device_data) { assert(device_data); diff --git a/layer/private_data.hpp b/layer/private_data.hpp index 6092500..11fa5ce 100644 --- a/layer/private_data.hpp +++ b/layer/private_data.hpp @@ -28,6 +28,7 @@ #include "util/custom_allocator.hpp" #include "util/unordered_set.hpp" #include "util/unordered_map.hpp" +#include "util/extension_list.hpp" #include #include @@ -278,6 +279,25 @@ public: return allocator; } + /** + * @brief Store the enabled instance extensions. + * + * @param extension_names Names of the enabled instance extensions. + * @param extension_count Size of the enabled instance extensions. + * + * @return VK_SUCCESS if successful, otherwise an error. + */ + VkResult set_instance_enabled_extensions(const char *const *extension_names, size_t extension_count); + + /** + * @brief Check whether an instance extension is enabled. + * + * param extension_name Extension's name. + * + * @return true if is enabled, false otherwise. + */ + bool is_instance_extension_enabled(const char *extension_name) const; + const instance_dispatch_table disp; private: @@ -324,6 +344,11 @@ private: * @brief Lock for thread safe access to @ref surfaces */ std::mutex surfaces_lock; + + /** + * @brief List with the names of the enabled instance extensions. + */ + util::extension_list enabled_extensions; }; /** @@ -400,6 +425,25 @@ public: return allocator; } + /** + * @brief Store the enabled device extensions. + * + * @param extension_names Names of the enabled device extensions. + * @param extension_count Size of the enabled device extensions. + * + * @return VK_SUCCESS if successful, otherwise an error. + */ + VkResult set_device_enabled_extensions(const char *const *extension_names, size_t extension_count); + + /** + * @brief Check whether a device extension is enabled. + * + * param extension_name Extension's name. + * + * @return true if is enabled, false otherwise. + */ + bool is_device_extension_enabled(const char *extension_name) const; + const device_dispatch_table disp; instance_private_data &instance_data; const PFN_vkSetDeviceLoaderData SetDeviceLoaderData; @@ -435,6 +479,11 @@ private: const util::allocator allocator; util::unordered_set swapchains; mutable std::mutex swapchains_lock; + + /** + * @brief List with the names of the enabled device extensions. + */ + util::extension_list enabled_extensions; }; } /* namespace layer */ diff --git a/util/extension_list.cpp b/util/extension_list.cpp index 47dfa06..ec2184d 100644 --- a/util/extension_list.cpp +++ b/util/extension_list.cpp @@ -57,6 +57,33 @@ VkResult extension_list::add(const char *const *extensions, uint32_t count) return VK_SUCCESS; } +VkResult extension_list::add(const char *const *extensions, uint32_t count, const char *const *extensions_subset, + uint32_t subset_count) +{ + util::vector extensions_to_add(m_alloc); + for (uint32_t ext_index = 0; ext_index < count; ++ext_index) + { + for (uint32_t subset_index = 0; subset_index < subset_count; ++subset_index) + { + if (!strcmp(extensions[ext_index], extensions_subset[subset_index])) + { + if (!extensions_to_add.try_push_back(extensions[ext_index])) + { + return VK_ERROR_OUT_OF_HOST_MEMORY; + } + } + } + } + + VkResult result = add(extensions_to_add.data(), extensions_to_add.size()); + if (result != VK_SUCCESS) + { + return result; + } + + return VK_SUCCESS; +} + VkResult extension_list::add(VkExtensionProperties ext_prop) { if (!contains(ext_prop.extensionName)) diff --git a/util/extension_list.hpp b/util/extension_list.hpp index a71888e..0268efa 100644 --- a/util/extension_list.hpp +++ b/util/extension_list.hpp @@ -85,6 +85,21 @@ public: VkResult add(const extension_list &ext_list); VkResult add(const char *const *extensions, uint32_t count); + /** + * @brief Perform intersection between extensions and add them to the list. + * + * Adds the extensions from @p extensions that are also part of @p extensions_subset. + * + * @param extensions A list with the names of the extensions to be added. + * @param count Length of the extensions list. + * @param extensions_subset A list with the names of the target extensions. + * @param subset_count Length of the target list. + * + * @return VK_SUCCESS on success, otherwise an error. + */ + VkResult add(const char *const *extensions, uint32_t count, const char *const *extensions_subset, + uint32_t subset_count); + private: util::allocator m_alloc; diff --git a/wsi/headless/surface_properties.cpp b/wsi/headless/surface_properties.cpp index 9f944bf..a24e0d6 100644 --- a/wsi/headless/surface_properties.cpp +++ b/wsi/headless/surface_properties.cpp @@ -202,5 +202,10 @@ PFN_vkVoidFunction surface_properties::get_proc_addr(const char *name) return nullptr; } +bool surface_properties::is_surface_extension_enabled(const layer::instance_private_data &instance_data) +{ + return instance_data.is_instance_extension_enabled(VK_EXT_HEADLESS_SURFACE_EXTENSION_NAME); +} + } /* namespace headless */ } /* namespace wsi */ diff --git a/wsi/headless/surface_properties.hpp b/wsi/headless/surface_properties.hpp index 539b234..b2d1ff9 100644 --- a/wsi/headless/surface_properties.hpp +++ b/wsi/headless/surface_properties.hpp @@ -47,6 +47,8 @@ public: PFN_vkVoidFunction get_proc_addr(const char *name) override; + bool is_surface_extension_enabled(const layer::instance_private_data &instance_data) override; + static surface_properties &get_instance(); }; diff --git a/wsi/surface_properties.hpp b/wsi/surface_properties.hpp index ca97fbb..23d49f9 100644 --- a/wsi/surface_properties.hpp +++ b/wsi/surface_properties.hpp @@ -32,6 +32,7 @@ #include #include "util/extension_list.hpp" +#include "layer/private_data.hpp" namespace wsi { @@ -76,6 +77,11 @@ public: */ virtual PFN_vkVoidFunction get_proc_addr(const char *name) = 0; + /** + * @brief Check if the proper surface extension has been enabled for the specific VkSurface type. + */ + virtual bool is_surface_extension_enabled(const layer::instance_private_data &instance_data) = 0; + /* There is no maximum theoretically speaking however we choose 3 for practicality */ static constexpr uint32_t MAX_SWAPCHAIN_IMAGE_COUNT = 3; }; diff --git a/wsi/wayland/surface_properties.cpp b/wsi/wayland/surface_properties.cpp index 094696a..5d2eb13 100644 --- a/wsi/wayland/surface_properties.cpp +++ b/wsi/wayland/surface_properties.cpp @@ -274,5 +274,10 @@ PFN_vkVoidFunction surface_properties::get_proc_addr(const char *name) return nullptr; } +bool surface_properties::is_surface_extension_enabled(const layer::instance_private_data &instance_data) +{ + return instance_data.is_instance_extension_enabled(VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME); +} + } // namespace wayland } // namespace wsi diff --git a/wsi/wayland/surface_properties.hpp b/wsi/wayland/surface_properties.hpp index 75e4491..1ea5730 100644 --- a/wsi/wayland/surface_properties.hpp +++ b/wsi/wayland/surface_properties.hpp @@ -62,6 +62,8 @@ public: PFN_vkVoidFunction get_proc_addr(const char *name) override; + bool is_surface_extension_enabled(const layer::instance_private_data &instance_data) override; + private: surface_properties(); diff --git a/wsi/wsi_factory.cpp b/wsi/wsi_factory.cpp index a8286e8..6d7013a 100644 --- a/wsi/wsi_factory.cpp +++ b/wsi/wsi_factory.cpp @@ -201,7 +201,7 @@ void destroy_surface_swapchain(swapchain_base *swapchain, layer::device_private_ alloc.destroy(1, swapchain); } -PFN_vkVoidFunction get_proc_addr(const char *name) +PFN_vkVoidFunction get_proc_addr(const char *name, const layer::instance_private_data &instance_data) { /* * Note that we here assume that there are no two get_proc_addr implementations @@ -216,7 +216,7 @@ PFN_vkVoidFunction get_proc_addr(const char *name) } PFN_vkVoidFunction func = props->get_proc_addr(name); - if (func) + if (props->is_surface_extension_enabled(instance_data) && func) { return func; } diff --git a/wsi/wsi_factory.hpp b/wsi/wsi_factory.hpp index b913489..298bf24 100644 --- a/wsi/wsi_factory.hpp +++ b/wsi/wsi_factory.hpp @@ -104,10 +104,11 @@ VkResult add_extensions_required_by_layer(VkPhysicalDevice phys_dev, const util: * implementation of the @p name function. * * @param name The name of the target function + * @param instance_data The instance specific data. * * @return A pointer to the implementation of the @p name function or null pointer in case this * function isn't implemented for any platform. */ -PFN_vkVoidFunction get_proc_addr(const char *name); +PFN_vkVoidFunction get_proc_addr(const char *name, const layer::instance_private_data &instance_data); } // namespace wsi -- 2.7.4