Skip to content

Commit 9da133e

Browse files
Fix add_pNext() dropping pNext chain elements in passed in structures
When a user provides a pNext chain to DeviceBuilder::add_pNext(), vk-bootstrap did not look at the pNext chain of the passed in struct, leaving to missing elements. While the library could require that each struct must be passed in explicitly with DeviceBuilder::add_pNext(), it isn't difficult to iterate the passed in pNext chain and grab the pointers to the structs to build the final pNext chain with. This commit also adds deduplication to building of the final pNext chain so that the same struct (as in, same memory address, it doesn't take sTypes into consideration) isn't in the chain more than once.
1 parent 4551e98 commit 9da133e

4 files changed

Lines changed: 55 additions & 11 deletions

File tree

src/VkBootstrap.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,16 @@ std::vector<vkb::detail::FeaturesChain::StructInfo>::const_iterator FeaturesChai
138138
});
139139
}
140140

141+
void add_all_pnext_structures(std::vector<void*>& pNext_chain, void* structure_to_add) {
142+
auto pNext_iter = structure_to_add;
143+
while (pNext_iter != nullptr) {
144+
pNext_chain.push_back(pNext_iter);
145+
VkBaseOutStructure out_structure{};
146+
memcpy(&out_structure, pNext_iter, sizeof(VkBaseOutStructure));
147+
pNext_iter = out_structure.pNext;
148+
}
149+
}
150+
141151

142152
class VulkanFunctions {
143153
private:
@@ -455,6 +465,17 @@ template <typename T> void setup_pNext_chain(T& structure, std::vector<void*> co
455465
structure.pNext = nullptr;
456466
if (structs.empty()) return;
457467
for (size_t i = 0; i < structs.size() - 1; i++) {
468+
// Make sure we aren't adding the same struct more than once. We do not care about duplicate sTypes here.
469+
bool is_duplicate = false;
470+
for (size_t j = 0; j != i; j++) {
471+
if (structs.at(i) == structs.at(j)) {
472+
is_duplicate = true;
473+
break;
474+
}
475+
}
476+
if (is_duplicate) {
477+
continue;
478+
}
458479
VkBaseOutStructure out_structure{};
459480
memcpy(&out_structure, structs.at(i), sizeof(VkBaseOutStructure));
460481
#if !defined(NDEBUG)
@@ -1713,8 +1734,8 @@ Result<Device> DeviceBuilder::build() const {
17131734
}
17141735
}
17151736

1716-
for (auto& pnext : info.pNext_chain) {
1717-
final_pnext_chain.push_back(pnext);
1737+
for (auto& pNext : info.pNext_chain) {
1738+
final_pnext_chain.push_back(pNext);
17181739
}
17191740

17201741
detail::setup_pNext_chain(device_create_info, final_pnext_chain);
@@ -1766,6 +1787,10 @@ DeviceBuilder& DeviceBuilder::custom_queue_setup(std::span<const CustomQueueDesc
17661787
return *this;
17671788
}
17681789
#endif
1790+
DeviceBuilder& DeviceBuilder::add_pNext(void* structure_to_add) {
1791+
detail::add_all_pnext_structures(info.pNext_chain, structure_to_add);
1792+
return *this;
1793+
}
17691794

17701795
// ---- Swapchain ---- //
17711796

@@ -2168,6 +2193,10 @@ SwapchainBuilder& SwapchainBuilder::set_allocation_callbacks(VkAllocationCallbac
21682193
info.allocation_callbacks = callbacks;
21692194
return *this;
21702195
}
2196+
SwapchainBuilder& SwapchainBuilder::add_pNext(void* structure_to_add) {
2197+
detail::add_all_pnext_structures(info.pNext_chain, structure_to_add);
2198+
return *this;
2199+
}
21712200
SwapchainBuilder& SwapchainBuilder::set_image_usage_flags(VkImageUsageFlags usage_flags) {
21722201
info.image_usage_flags = usage_flags;
21732202
return *this;

src/VkBootstrap.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -820,12 +820,11 @@ class DeviceBuilder {
820820
DeviceBuilder& custom_queue_setup(std::span<const CustomQueueDescription> queue_descriptions);
821821
#endif
822822

823-
// Add a structure to the pNext chain of VkDeviceCreateInfo.
823+
// Add a pNext chain structure to the pNext chain of VkDeviceCreateInfo.
824824
// The structure must be valid when DeviceBuilder::build() is called.
825-
template <typename T> DeviceBuilder& add_pNext(T* structure) {
826-
info.pNext_chain.push_back(structure);
827-
return *this;
828-
}
825+
// The structure must be a type that is valid to add to the pNext chain of VkDeviceCreateInfo
826+
// Any structures chained through the pNext pointer will also be added
827+
DeviceBuilder& add_pNext(void* structure_to_add);
829828

830829
// Provide custom allocation callbacks.
831830
DeviceBuilder& set_allocation_callbacks(VkAllocationCallbacks* callbacks);
@@ -978,10 +977,9 @@ class SwapchainBuilder {
978977

979978
// Add a structure to the pNext chain of VkSwapchainCreateInfoKHR.
980979
// The structure must be valid when SwapchainBuilder::build() is called.
981-
template <typename T> SwapchainBuilder& add_pNext(T* structure) {
982-
info.pNext_chain.push_back(structure);
983-
return *this;
984-
}
980+
// The structure must be a type that is valid to add to the pNext chain of VkSwapchainCreateInfoKHR
981+
// Any structures chained through the pNext pointer will also be added
982+
SwapchainBuilder& add_pNext(void* structure_to_add);
985983

986984
// Provide custom allocation callbacks.
987985
SwapchainBuilder& set_allocation_callbacks(VkAllocationCallbacks* callbacks);

tests/bootstrap_tests.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,12 +195,25 @@ TEST_CASE("Device Configuration", "[VkBootstrap.bootstrap]") {
195195
}
196196

197197
SECTION("VkPhysicalDeviceFeatures2 in pNext Chain") {
198+
VkPhysicalDeviceMultiviewFeatures multiview_features{};
199+
multiview_features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MULTIVIEW_FEATURES;
200+
198201
VkPhysicalDeviceShaderDrawParameterFeatures shader_draw_features{};
199202
shader_draw_features.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_DRAW_PARAMETER_FEATURES;
203+
shader_draw_features.pNext = &multiview_features;
200204

201205
vkb::DeviceBuilder device_builder(phys_device_ret.value());
202206
auto device_ret = device_builder.add_pNext(&shader_draw_features).build();
203207
REQUIRE(device_ret.has_value());
208+
209+
// Check that the pNext chain was passed through successfully
210+
auto& features_pNextChain = mock.physical_devices_details.at(0).created_device_details.at(0).features_pNextChain;
211+
auto* s1 = reinterpret_cast<VkPhysicalDeviceShaderDrawParameterFeatures*>(features_pNextChain.at(0).data());
212+
auto* s2 = reinterpret_cast<VkPhysicalDeviceMultiviewFeatures*>(features_pNextChain.at(1).data());
213+
214+
REQUIRE(s1->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_DRAW_PARAMETER_FEATURES);
215+
REQUIRE(s2->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MULTIVIEW_FEATURES);
216+
204217
vkb::destroy_device(device_ret.value());
205218
}
206219

tests/vulkan_mock.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@ inline SerializedStruct create_serialized_struct_from_features2_struct(const voi
3636
return create_serialized_struct_from_object(*static_cast<const VkPhysicalDeviceVulkan12Features*>(input_data));
3737
case (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SUBGROUP_SIZE_CONTROL_FEATURES):
3838
return create_serialized_struct_from_object(*static_cast<const VkPhysicalDeviceSubgroupSizeControlFeatures*>(input_data));
39+
case (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_MULTIVIEW_FEATURES):
40+
return create_serialized_struct_from_object(*static_cast<const VkPhysicalDeviceMultiviewFeatures*>(input_data));
41+
case (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_SHADER_DRAW_PARAMETER_FEATURES):
42+
return create_serialized_struct_from_object(*static_cast<const VkPhysicalDeviceShaderDrawParameterFeatures*>(input_data));
3943
default:
4044
return SerializedStruct{};
4145
}

0 commit comments

Comments
 (0)