From 2a352b5a25b3afc71f8c633e6475926262c9706b Mon Sep 17 00:00:00 2001 From: Charles Giessen <46324611+charles-lunarg@users.noreply.github.com> Date: Sun, 18 Apr 2021 14:09:12 -0600 Subject: [PATCH] Revert "Simplified structures and plugged into existing logic." This reverts commit 6ee81abdd3e9451d66b3d018695a652a45222dbc. --- src/VkBootstrap.cpp | 90 ++++++++++++++++----------------------- src/VkBootstrap.h | 83 ++++++++++++++++++++++++------------ tests/bootstrap_tests.cpp | 12 +++--- 3 files changed, 98 insertions(+), 87 deletions(-) diff --git a/src/VkBootstrap.cpp b/src/VkBootstrap.cpp index f366207..7fe74e9 100644 --- a/src/VkBootstrap.cpp +++ b/src/VkBootstrap.cpp @@ -690,9 +690,6 @@ detail::Result InstanceBuilder::build() const { VkInstanceCreateInfo instance_create_info = {}; instance_create_info.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO; detail::setup_pNext_chain(instance_create_info, pNext_chain); - for(auto& node : pNext_chain) { - assert(node->sType != VK_STRUCTURE_TYPE_APPLICATION_INFO); - } instance_create_info.flags = info.flags; instance_create_info.pApplicationInfo = &app_info; instance_create_info.enabledExtensionCount = static_cast(extensions.size()); @@ -849,8 +846,8 @@ std::vector check_device_extension_support( // clang-format off bool supports_features(VkPhysicalDeviceFeatures supported, VkPhysicalDeviceFeatures requested, - std::vector const& extension_supported, - std::vector const& extension_requested) { + std::vector const& extension_supported, + std::vector const& extension_requested) { if (requested.robustBufferAccess && !supported.robustBufferAccess) return false; if (requested.fullDrawIndexUint32 && !supported.fullDrawIndexUint32) return false; if (requested.imageCubeArray && !supported.imageCubeArray) return false; @@ -908,8 +905,7 @@ bool supports_features(VkPhysicalDeviceFeatures supported, if (requested.inheritedQueries && !supported.inheritedQueries) return false; for(auto i = 0; i < extension_requested.size(); ++i) { - //auto res = extension_requested[i].match(extension_supported[i]); - auto res = GenericFeaturesPNextNode::match(extension_requested[i], extension_supported[i]); + auto res = extension_requested[i].match(extension_supported[i]); if(!res) return false; } @@ -996,7 +992,7 @@ uint32_t get_present_queue_index(VkPhysicalDevice const phys_device, PhysicalDeviceSelector::PhysicalDeviceDesc PhysicalDeviceSelector::populate_device_details( uint32_t instance_version, VkPhysicalDevice phys_device, - std::vector const& src_extended_features_chain) const { + std::vector extension_features_as_template) const { PhysicalDeviceSelector::PhysicalDeviceDesc desc{}; desc.phys_device = phys_device; auto queue_families = detail::get_vector_noerror( @@ -1008,31 +1004,23 @@ PhysicalDeviceSelector::PhysicalDeviceDesc PhysicalDeviceSelector::populate_devi detail::vulkan_functions().fp_vkGetPhysicalDeviceMemoryProperties(phys_device, &desc.mem_properties); #if defined(VK_API_VERSION_1_1) - desc.device_features2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; - - auto fill_chain = src_extended_features_chain; - - if(!fill_chain.empty() && instance_version >= VK_API_VERSION_1_1) { - - detail::GenericFeaturesPNextNode* prev = nullptr; - for (auto& extension : fill_chain) { - if (prev != nullptr) { - prev->pNext = &extension; + desc.device_features2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; + desc.extension_features = extension_features_as_template; + if (instance_version >= VK_API_VERSION_1_1) { + detail::FeaturesContainer* prev = nullptr; + for(auto& extension : desc.extension_features) { + if(prev != nullptr) { + prev->header->pNext = extension.header; } prev = &extension; } - - VkPhysicalDeviceFeatures2 local_features{}; - local_features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; - local_features.pNext = &fill_chain.front(); - - detail::vulkan_functions().fp_vkGetPhysicalDeviceFeatures2(phys_device, &local_features); - + if(desc.extension_features.size() > 0) { + desc.device_features2.pNext = desc.extension_features.front().header; + } + detail::vulkan_functions().fp_vkGetPhysicalDeviceFeatures2(phys_device, &desc.device_features2); } - - desc.extended_features_chain = fill_chain; #endif - return desc; + return desc; } PhysicalDeviceSelector::Suitable PhysicalDeviceSelector::is_device_suitable(PhysicalDeviceDesc pd) const { @@ -1101,7 +1089,7 @@ PhysicalDeviceSelector::Suitable PhysicalDeviceSelector::is_device_suitable(Phys } bool required_features_supported = detail::supports_features(pd.device_features, criteria.required_features, - pd.extended_features_chain, criteria.extended_features_chain); + pd.extension_features, criteria.extension_features); if (!required_features_supported) return Suitable::no; bool has_required_memory = false; @@ -1154,7 +1142,7 @@ detail::Result PhysicalDeviceSelector::select() const { for (auto& phys_device : physical_devices) { phys_device_descriptions.push_back(populate_device_details(instance_info.version, phys_device, - criteria.extended_features_chain)); + criteria.extension_features)); } PhysicalDeviceDesc selected_device{}; @@ -1180,7 +1168,7 @@ detail::Result PhysicalDeviceSelector::select() const { out_device.physical_device = selected_device.phys_device; out_device.surface = instance_info.surface; out_device.features = criteria.required_features; - out_device.extended_features_chain = criteria.extended_features_chain; + out_device.extension_features = criteria.extension_features; out_device.properties = selected_device.device_properties; out_device.memory_properties = selected_device.mem_properties; out_device.queue_families = selected_device.queue_families; @@ -1411,18 +1399,25 @@ detail::Result DeviceBuilder::build() const { extensions.push_back({ VK_KHR_SWAPCHAIN_EXTENSION_NAME }); bool has_phys_dev_features_2 = false; - std::vector final_pnext_chain; #if defined(VK_API_VERSION_1_1) + // Setup the pNexts of all the extension features + std::vector match = physical_device.extension_features; VkPhysicalDeviceFeatures2 local_features2{}; - local_features2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; - auto physical_device_extension_features_copy = physical_device.extended_features_chain; - if (physical_device.instance_version >= VK_MAKE_VERSION(1, 1, 0)) { + if (physical_device.instance_version >= VK_MAKE_VERSION(1, 1, 0) && + match.size() > 0) { + detail::FeaturesContainer* prev = nullptr; + for(auto& extension : match) { + if(prev != nullptr) { + prev->header->pNext = extension.header; + } + prev = &extension; + } + local_features2.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2; + local_features2.features = physical_device.features; + local_features2.pNext = match.front().header; has_phys_dev_features_2 = true; - for(auto& features_node : physical_device_extension_features_copy) { - final_pnext_chain.push_back(reinterpret_cast(&features_node)); - } - } + } #endif VkDeviceCreateInfo device_create_info = {}; @@ -1433,23 +1428,15 @@ detail::Result DeviceBuilder::build() const { device_create_info.enabledExtensionCount = static_cast(extensions.size()); device_create_info.ppEnabledExtensionNames = extensions.data(); - for(auto& pnext : info.pNext_chain) { - final_pnext_chain.push_back(pnext); - } - - detail::setup_pNext_chain(device_create_info, final_pnext_chain); - for(auto& node : final_pnext_chain) { - assert(node->sType != VK_STRUCTURE_TYPE_APPLICATION_INFO); - } + detail::setup_pNext_chain(device_create_info, info.pNext_chain); #if defined(VK_API_VERSION_1_1) // VUID-VkDeviceCreateInfo-pNext-00373 - don't add pEnabledFeatures if the phys_dev_features_2 is present if (has_phys_dev_features_2) { device_create_info.pNext = &local_features2; - if(!final_pnext_chain.empty()) { - local_features2.pNext = final_pnext_chain.front(); + if(info.pNext_chain.size() > 0) { + match.back().header->pNext = info.pNext_chain.front(); } - device_create_info.pEnabledFeatures = nullptr; } else { device_create_info.pEnabledFeatures = &physical_device.features; } @@ -1691,9 +1678,6 @@ detail::Result SwapchainBuilder::build() const { VkSwapchainCreateInfoKHR swapchain_create_info = {}; swapchain_create_info.sType = VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR; detail::setup_pNext_chain(swapchain_create_info, info.pNext_chain); - for(auto& node : info.pNext_chain) { - assert(node->sType != VK_STRUCTURE_TYPE_APPLICATION_INFO); - } swapchain_create_info.flags = info.create_flags; swapchain_create_info.surface = info.surface; swapchain_create_info.minImageCount = image_count; diff --git a/src/VkBootstrap.h b/src/VkBootstrap.h index c9a2ec6..a0cc6b2 100644 --- a/src/VkBootstrap.h +++ b/src/VkBootstrap.h @@ -115,31 +115,60 @@ template class Result { bool m_init; }; -struct GenericFeaturesPNextNode { +struct FeaturesContainer { - GenericFeaturesPNextNode() : sType(static_cast(0)), - pNext(nullptr) { - memset(fields, 0, sizeof(fields)); - } + FeaturesContainer() = default; - VkStructureType sType = static_cast(0); - void* pNext = nullptr; - VkBool32 fields[256]; + FeaturesContainer(FeaturesContainer const& other) { copy(other); } - template - void set(T const& features) { - GenericFeaturesPNextNode node; - *reinterpret_cast(this) = features; - } + FeaturesContainer(FeaturesContainer&& other) { copy(other); } - static bool match(GenericFeaturesPNextNode const& requested, GenericFeaturesPNextNode const& supported) { - assert(requested.sType == supported.sType && - "Non-matching sTypes in features nodes!"); - for (uint32_t i = 0; i < (sizeof(fields) / sizeof(VkBool32)); i++) { - if (requested.fields[i] && !supported.fields[i]) return false; - } - return true; - } + FeaturesContainer& operator=(FeaturesContainer const& other) { copy(other); return *this; } + + FeaturesContainer& operator=(FeaturesContainer&& other) { copy(other); return *this; } + + template + static FeaturesContainer make(T src) { + FeaturesContainer extension_features; + extension_features.set(src); + return extension_features; + + } + + template + void set(T const& features) { + data.resize(sizeof(T)); + *reinterpret_cast(data.data()) = features; + count = (sizeof(T) - sizeof(VkBaseOutStructure)) / sizeof(VkBool32); + header = reinterpret_cast(data.data()); + fields = reinterpret_cast(data.data() + sizeof(VkBaseOutStructure)); + } + + bool match(FeaturesContainer const& other) const { + if(!header || !other.header || header->sType != other.header->sType) { return false; } + for(auto i = 0; i < count; ++i) { + if(fields[i] == VK_TRUE && other.fields[i] == VK_FALSE) { + return false; + } + } + return true; + } + + VkBaseOutStructure* header = nullptr; +private: + + // Just to avoid having it copied in 4 places + void copy(FeaturesContainer const& other) { + data = other.data; + count = other.count; + header = reinterpret_cast(data.data()); + fields = reinterpret_cast(data.data() + sizeof(VkBaseOutStructure)); + } + + std::vector data; + VkBool32* fields = nullptr; + void* extend = nullptr; // Future proofing + int count = 0; }; @@ -383,7 +412,7 @@ struct PhysicalDevice { uint32_t instance_version = VK_MAKE_VERSION(1, 0, 0); std::vector extensions_to_enable; std::vector queue_families; - std::vector extended_features_chain; + std::vector extension_features; bool defer_surface_initialization = false; friend class PhysicalDeviceSelector; friend class DeviceBuilder; @@ -450,9 +479,7 @@ class PhysicalDeviceSelector { PhysicalDeviceSelector& add_required_extension_features(T const& features) { assert(features.sType != 0 && "Features struct sType must be filled with the struct's corresponding VkStructureType enum"); - detail::GenericFeaturesPNextNode node; - node.set(features); - criteria.extended_features_chain.push_back(node); + criteria.extension_features.push_back(detail::FeaturesContainer::make(features)); return *this; } #endif @@ -494,7 +521,7 @@ class PhysicalDeviceSelector { VkPhysicalDeviceMemoryProperties mem_properties{}; #if defined(VK_API_VERSION_1_1) VkPhysicalDeviceFeatures2 device_features2{}; - std::vector extended_features_chain; + std::vector extension_features; #endif }; @@ -503,7 +530,7 @@ class PhysicalDeviceSelector { PhysicalDeviceDesc populate_device_details(uint32_t instance_version, VkPhysicalDevice phys_device, - std::vector const& src_extended_features_chain) const; + std::vector extension_features_as_template) const; struct SelectionCriteria { PreferredDeviceType preferred_type = PreferredDeviceType::discrete; @@ -525,7 +552,7 @@ class PhysicalDeviceSelector { VkPhysicalDeviceFeatures required_features{}; #if defined(VK_API_VERSION_1_1) VkPhysicalDeviceFeatures2 required_features2{}; - std::vector extended_features_chain; + std::vector extension_features; #endif bool defer_surface_initialization = false; bool use_first_gpu_unconditionally = false; diff --git a/tests/bootstrap_tests.cpp b/tests/bootstrap_tests.cpp index 3420800..6d3afe7 100644 --- a/tests/bootstrap_tests.cpp +++ b/tests/bootstrap_tests.cpp @@ -106,7 +106,7 @@ TEST_CASE("instance configuration", "[VkBootstrap.bootstrap]") { TEST_CASE("Headless Vulkan", "[VkBootstrap.bootstrap]") { vkb::InstanceBuilder builder; - auto instance_ret = builder.request_validation_layers().set_headless().use_default_debug_messenger().build(); + auto instance_ret = builder.request_validation_layers().set_headless().build(); REQUIRE(instance_ret.has_value()); vkb::PhysicalDeviceSelector phys_device_selector(instance_ret.value()); @@ -127,7 +127,7 @@ TEST_CASE("Device Configuration", "[VkBootstrap.bootstrap]") { auto window = create_window_glfw("Device Configuration"); vkb::InstanceBuilder builder; - auto instance_ret = builder.request_validation_layers().require_api_version(1, 1).use_default_debug_messenger().build(); + auto instance_ret = builder.request_validation_layers().require_api_version(1, 1).build(); REQUIRE(instance_ret.has_value()); auto surface = create_surface_glfw(instance_ret.value().instance, window); @@ -192,7 +192,7 @@ TEST_CASE("Swapchain", "[VkBootstrap.bootstrap]") { auto window = create_window_glfw("Swapchain"); vkb::InstanceBuilder builder; - auto instance_ret = builder.request_validation_layers().use_default_debug_messenger().build(); + auto instance_ret = builder.request_validation_layers().build(); REQUIRE(instance_ret.has_value()); auto surface = create_surface_glfw(instance_ret.value().instance, window); @@ -320,7 +320,7 @@ TEST_CASE("Allocation Callbacks", "[VkBootstrap.bootstrap]") { vkb::InstanceBuilder builder; auto instance_ret = - builder.request_validation_layers().set_allocation_callbacks(&allocation_callbacks).use_default_debug_messenger().build(); + builder.request_validation_layers().set_allocation_callbacks(&allocation_callbacks).build(); REQUIRE(instance_ret.has_value()); auto surface = create_surface_glfw(instance_ret.value().instance, window); @@ -417,7 +417,7 @@ TEST_CASE("Querying Required Extension Features", "[VkBootstrap.version]") { vkb::InstanceBuilder builder; auto instance_ret = - builder.request_validation_layers().require_api_version(1, 2).set_headless().use_default_debug_messenger().build(); + builder.request_validation_layers().require_api_version(1, 2).set_headless().build(); REQUIRE(instance_ret.has_value()); // Requires a device that supports runtime descriptor arrays via descriptor indexing extension. { @@ -449,7 +449,7 @@ TEST_CASE("Querying Vulkan 1.1 and 1.2 features", "[VkBootstrap.version]") { vkb::InstanceBuilder builder; auto instance_ret = - builder.request_validation_layers().require_api_version(1, 2).set_headless().use_default_debug_messenger().build(); + builder.request_validation_layers().require_api_version(1, 2).set_headless().build(); REQUIRE(instance_ret.has_value()); // Requires a device that supports multiview and bufferDeviceAddress {