-
Notifications
You must be signed in to change notification settings - Fork 439
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bootstrapping Vulkan support #234
Merged
Conversation
This file contains 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
mosra
force-pushed
the
vulkan
branch
3 times, most recently
from
March 2, 2018 23:01
a8d67a5
to
88cbdf6
Compare
mosra
force-pushed
the
split
branch
2 times, most recently
from
March 6, 2018 23:12
c792bc9
to
e425f37
Compare
mosra
force-pushed
the
split
branch
2 times, most recently
from
March 28, 2018 19:55
3917fe9
to
3c3105b
Compare
mosra
force-pushed
the
split
branch
13 times, most recently
from
April 21, 2018 20:10
acbbe8b
to
adef5d9
Compare
mosra
force-pushed
the
split
branch
5 times, most recently
from
April 28, 2018 22:43
be06426
to
aaf39a5
Compare
This avoids allocating a potentially large array in case just the first device is needed. Originally I did this in a hope to avoid a stall + CPU power management issues due to some bad shit in the AMD driver, but it seems that enumerating even just one device still makes it stall. Sigh.
Today I spent six hours wrongly convincing myself that it's a driver bug when vkGetPhysicalDeviceProperties2() is null on a 1.1 instance for a 1.0 physical device. It's not a bug, it's me not reading specs carefully. This commit thus basically moves all Instance-level extension-dependent state to DeviceProperties, because it's actually device-dependent. Which makes the DeviceProperties class quite heavy and thus it's good it was readied to be transferred all the way to a Device instance a few commits back -- I don't really want to do all the dispatch, string processing, sorting and other mess more times than strictly necessary. In addition, DeviceProperties::apiVersion() got renamed to version() and a new isVersionSupported() API got added, mirroring what's on Device itself; plus thanks to the chicken-and-egg problem of having to call vkGetPhysicalDeviceProperties() twice, the device version and other things can now be retrieved in a slightly more efficient way.
For some reason it wants me to allocate 16 bytes more. Why can't that be stored somewhere else, I wonder? Hm, and for this I implemented VK_KHR_driver_properties only to discover that the info is not queryable if we run the tests with KHR_get_physical_device_properties2 disabled. Sigh.
Will be used for mapping and in the future possibly for memory slices / views.
More convenient to use since one doesn't have to explicitly store a DeviceProperties instance to call pickQueueFamily() on it and then move it to DeviceCreateInfo to keep it efficient. This required a slight redesign of how DeviceProperties are stored in DeviceCreateInfo, now we need the instance to be always valid (but it can get never used). The wrap() API isn't doing any extra work so this won't add any inefficiency.
Nope, it's 2020 and there's still no way to link to &-qualified functions. Feature that is in C++ since 2011.
Makes it possible to write Vk::Instance instance{{argc, argv}} which is a good tradeoff between passing no arguments at all and doing the fully verbose thing.
Basically the same as with Image, but easier.
Again basically the same as with Image.
Again mirrors what's done in Image.
This was easy ... except for the fun with taking ownership of the binary. And it led to me inventing CORRADE_INTERNAL_ASSERT_EXPRESSION().
Thus it has no place in the general overview docs of Vk::Instance and Vk::Device. Better to put it into the VUlkan wrapping docs, where it also makes sense to have both the device and instance side together.
Realized usefulness of such an utility while writing a documentation snippet.
It's a new extension in Vulkan 1.2.
In Vulkan there's no mess like with GL/GLES/WebGL. Well, yet.
Now that the initial design got pretty stable, the overhead from branch switching and rebasing overweighted the advantages of being able to experiment, so I'm merging. Future development will happen directly on the master branch. |
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 currently depends on #233, so I'm setting base branch to that instead of#233 is merged now.master
so the changes are visible clearer. I'm also working on both at the same time.Things to do:
WITH_VK
)Require CMake 3.7not neededRequire C++14 for dependencies of this librather not, unless really needed by some featureTARGET_VK
variable@fn_vk
etc Doxygen aliases, update Coding Style for themvk.xml
file is not sorted in type dependency order, submit patch upstream (or hotfix in flextGL?)flextVkInitInstance()
/flextVkInitDevice()
function (done just because there was no Instance/Device wrapper yet)Integrate SPIRV-Tools linker for putting multiple stages together in a single file (heh, i thought glslang/shaderc did that, but no)postponedothers? glsl-optimizer, shaderc, glsl-validator(?), DirectXShaderCompiler)postponedGLSL minifier (whitespace and comment removal, at least), this could be used in Magnum itself to help trimming down the size of the Shaders librarypostponedIntrospection for SPIR-V (querying attribute, uniform, ... names and bindings of an imported shader bytecode)Specialization constants, specialization constant folding (because apparently even for a disabled branch all uniforms and inputs need to exist, sigh)Ability to disable even device extensions that don't need enabling (such aspostponedVK_KHR_driver_properties
so we can the codepath for drivers that don't have it)--magnum-device
Enabling validation layers etc usingpostponed--magnum-gpu-validation
Command buffer submit, resetpostponedFlushingpostponedDedicated strong type for "memory type bits" as it's otherwise too easy to mix up with size when allocatingpostponedSupportpostponedKHR_dedicated_allocation
since it makes sense for large data and render targets (and we don't have anything else now anyway)Generic API for hooking up non-trivial memory allocators like the AMD onepostponed, needs a whole new abstraction on top(implicitly convertible) image view wrapper, ideally as simple as GL texture / framebuffer wrappers for the common casepostponed"Preset" tags for linear mappable / framebuffer images?postponedBuffer view wrapperpostponedPipeline + shader setuppostponedVulkan-enabled alternative of the GLpostponedMeshTools::compile()
KHR_surface for Linux, Windows, Android and macOSpostponedmagnum-vk-info
utilityVulkan-specific projection matrix creation. Would need to be moved out frompostponedMath::Matrix[34]::*Projection()
to some API-specific header.BUILD_VK_TESTS
optionSetting up debug layers? Leak detection?later maybeSetting up queues?later maybeMake it working on Windows as well -- currently it fails withVK_ERROR_INITIALIZATION_FAILED
(does it require some stuff being added to registry? ugh)libMoltenVk
instead oflibvulkan
)-DVulkan_LIBRARY=${MINGW_PREFIX}/lib/libvulkan.dll.a
.. maybe it just needs to explicitly listlibvulkan
in addition tovulkan
?Gradually rewrite it using the new, less error-prone and less verbose wrapperspostponedCc: @Squareys