Introduced unified vk::DispatchLoader and removed dispatcher templating#2497
Draft
M2-TE wants to merge 13 commits intoKhronosGroup:mainfrom
Draft
Introduced unified vk::DispatchLoader and removed dispatcher templating#2497M2-TE wants to merge 13 commits intoKhronosGroup:mainfrom
M2-TE wants to merge 13 commits intoKhronosGroup:mainfrom
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR aims to massively reduce the templating required for most functions and reduce dispatcher complexity by offering only a single unified dispatcher interface.
Blocked by #2495, #2496 and #2502.Resolves #2490.Summary of changes:
vk::DispatchLoader, which is provided invulkan_dispatch_loader.hppand is able to handle both dynamic dispatching (ProcAddr), as well as static dispatching (vulkan-1). The former works just as the previous dynamic dispatcher, while the latter simply default-assigns to the Vulkan C functions. This is done for core and WSI functions only (see vulkan-1 symbols).vk::DispatchLoaderis in the normalvknamespace. Only thevk::detail::DynamicLoadershould be an implementation detail.vulkan.hpp. The init functions usingvk::Instanceandvk::Deviceare simply forward declared and defined later (they are very small as the bulk init usesVkInstanceandVkDevice).vulkan_structs.hpp, but it's a start.getVkHeaderVersion()from the dispatcher, as the version comparison check is handled by the header itself. Besides, using a dispatcher from a slightly outdated header should be allowed (e.g. when it is placed in a dynamic libary).There are some downsides that might warrant discussion:
vk::DispatchLoader. You will have storage for all the function pointers, but can implement your owninit()functions to only load what you need. Can also override the static dispatcher portion (though that likely won't ever be needed).vulkan-1list of symbols. To be honest though, dynamic dispatching should probably be the standard as that is what most people use.Current issue:
Thevk::UniqueHandlesare broken. This is not due to my PR specifically, but because the templating is gone from some of the functions, resulting in the compiler actually checking them for validity. This is further explained in #2496. Because of this, I temporarily disabled tests using the smart handles, as well as samples in general.Another similar issue blocks progress, as described in #2502.
ETC:
Another one of my goals was to try and merge the normal dispatching with raii dispatching, but that is a bit more challenging. The raii dispatchers are three separate ones; one for
context, one forinstanceand another fordevicefunctions.However, this raii setup transparently handles dynamic dispatching in a very clean way, even automatically handling per-device functions perfectly. It might be worth discussing a VULKAN_HPP_* flag that allows normal
vk::Instanceandvk::Devicehandles to each store their own dispatcher. Even with careless C<->C++ handle casting, this should be fine and actually make things much easier for users, especially beginners! No more explicit dynamic dispatch storage or anything.