charles-lunarg / vk-bootstrap

Vulkan Bootstrapping Iibrary
MIT License
821 stars 82 forks source link

Feature Request: Improve extension selection functions #224

Closed DexterHaxxor closed 1 year ago

DexterHaxxor commented 1 year ago

Add vkb::InstanceBuilder ::enable_extensions just like vkb::PhysicalDeviceSelector:: add_required_extensions. Add overloads to both that take (const char* const *extensions, size_t count) and (const std::vector<const char*> &extensions), additionally something with a std::span if C++20 is enabled.

This should avoid using for-loops or pointless allocations when using things like SDL_Vulkan_GetInstanceExtensions.

charles-lunarg commented 1 year ago

Thought this was a much bigger request than it actually ended up being...

Apologies for taking so long to get back to you.

Would absolutely appreciate review & critique. Simple PR, but I'm sure I could have messed something up.

monomere commented 8 months ago

Not sure if I should make a new issue, but the pr didn't seem to add a std::span version of the method. Afaik, you can pass containers into std::span directly even, so the vector overload would be unnecessary. I'll try making a pr for this today.

Edit: because the c++20 version is not required, the vector overload should still exist.

charles-lunarg commented 8 months ago

The library is currently C++17. Not sure there is an easy way to add a C++20 feature that would be 'backwards compatible', though I am open to PR's.

And yes, you should create a separate issue to better track it (cause this issue is closed and I will forget)

monomere commented 8 months ago

ill make the issue later today, but the easiest way to check for the language version is to do something like this:

#if __cplusplus >= 202002L
#include <span>
#endif
charles-lunarg commented 8 months ago

Where does 202002L come from? Who defines it? Cause I have not seen that used to detect language version, and any solution given MUST work in a cross platform manner.

monomere commented 8 months ago

it's in the standard (the number itself comes from the year and month i think) cppreference

charles-lunarg commented 8 months ago

Have I been living under a rock??? Apparently because I either forgot about that or never knew it in the first place. Looking forward to that PR.