KhronosGroup / Vulkan-Utility-Libraries

Utility libraries for Vulkan developers
Other
56 stars 25 forks source link

safestruct: Remove Feature structs #216

Closed spencer-lunarg closed 4 months ago

spencer-lunarg commented 4 months ago

For https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8110

The feature structs are only used at vkCreateDevice time to get the features and there is no reason they need to be safe structs as things like VVL will just save a "what is enabled" after and never actually need to a safe struct version later

... also 16k lines gone!

spencer-lunarg commented 4 months ago

Ok, so I think I figured out the issue is Validation just can't use safe_VkDeviceCreateInfo and then I don't actually need the VUL change

charles-lunarg commented 4 months ago

Looking at the original issue, it seems like changing safe structs to not recursively destroy objects would be a solution, if a bit annoying to implement.

spencer-lunarg commented 4 months ago

I think the simpler solution is just not using safe structs for Features in VVL (via safe_VkDeviceCreateInfo)... like there is no need regardless... also not sure how to actually remove the recursion here, I tried looking into it and some spots need it

charles-lunarg commented 4 months ago

There was a cppcon keynote by herb sutter where he described this exact problem, he talk was about how c++ can handle memory stuff (it was not long after c++11). While it might be fun to crack out the c++ meta programming I agree with your direction of how to fix this.

jeremyg-lunarg commented 4 months ago

I know this is already closed, but IMO using safe versions of the feature structs is important. A use case that is becoming more common is 'take the user's VkDeviceCreateInfo and change which features are enabled'. An example can be found here. I have a bigger example in a repo that is currently private. If there's a better way to handle this I'd love to know about it.

spencer-lunarg commented 4 months ago

so your example will blowup of stack calls if you run something with enough feature structs chained (dEQP-VK.api.version_check.version will do this for you)

my solution was to go

 auto* chain_end = vku::FindLastStructInPNextChain((void*)modified_create_info.pNext);

 vkCreateDevice();

 if (chain_end) chain_end->pNext = nullptr;

and have the person adding features to cleanup there added structs