-
Notifications
You must be signed in to change notification settings - Fork 11
This is a new version of the tutorial using RAII and SLang #61
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
base: main
Are you sure you want to change the base?
Conversation
Streamline formatting, clarify key setup steps, and consolidate redundant information in Vulkan development guide. Focus on improving readability while maintaining cross-platform support for Vulkan SDK, dependencies, and build tools like CMake and GLFW.
Refactor code to replace raw Vulkan handles with RAII wrappers from Vulkan-Hpp for better resource management and safety. Key changes include RAII usage for buffers, images, and device memory, along with modernized function calls and parameter handling. Code readability and alignment with updated Vulkan practices were also improved.
Migrated codebase to use `vk::` RAII wrappers, enhanced type safety, and updated Vulkan API conventions. Made textual corrections in documentation for grammatical consistency and clarity. Adjusted resource creation and rendering process to comply with Vulkan's multi-sampling requirements.
Corrected punctuation, grammar, and terminology for clarity and accuracy. Improved consistency in formatting, adopted RAII style for Vulkan handles, and refined examples by updating to modern Vulkan C++ bindings for better maintainability.
Fix typos and improve clarity in compute shader documentation Corrected grammatical mistakes and improved phrasing for better readability in compute shader documentation. Adjusted links to use `.adoc` extension and reformatted lines for consistency and clarity. No functional changes were made to the content. ```
Updated tutorial text to use inclusive language ("we" instead of "I") and improved readability. Modernized Vulkan instance creation using RAII and cleaner initialization code in compliance with up-to-date Vulkan best practices.
We might need to check if Antora supports highlighting for slang. Afair it does glsl out of the box, but for hlsl I had to manually add in a syntax highlighter. |
Slang isn't in the list for our antora site. Here's the list of languages we have a highlight script for. We should probably add SLang at the very least. var hljs = require('highlight.js/lib/highlight') hljs.registerLanguage('asciidoc', require('highlight.js/lib/languages/asciidoc')) |
We might get away by duplicating the hlsl one, and adding in a few slang keywords ;) |
Yep, that's what I'm thinking. I'll make a tracking issue on the vulkan-site repo and begin a pass at making a highlight.js. It might be worth it to investigate how to contribute that back upstream to help promote use of Slang? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of questions...
|
||
return availableFormats[0]; | ||
static vk::Format chooseSwapSurfaceFormat(const std::vector<vk::SurfaceFormatKHR>& availableFormats) { | ||
return ( availableFormats[0].format == vk::Format::eUndefined ) ? vk::Format::eB8G8R8A8Unorm : availableFormats[0].format; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very different from the original.
Remarks on the CMake build setup: First, kudos for providing a proper CMakeLists.txt. That has always been one of my main issues with the current tutorial 👍🏻 On Windows though it does not yet work out of the box, as you have to manually specify folders for e.g. glfw and that does not seem to work with the glfw package you can download from their site. For my own projects I moved to using CMake's FetchContent for such third party libraries, as that takes care of download, compiling and even installing these dependencies. Would it be possible to use FetchContent for the tutorial too? That would simplify project setup even further 😄 |
* `VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT`: Informational message like the creation of a resource | ||
* `VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT`: Message about behavior that is not necessarily an error, but very likely a bug in your application | ||
* `VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT`: Message about behavior that is invalid and may cause crashes | ||
* `VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT`: Informational message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be updated to use the vulkan.hpp enums instead (e.g. vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left it with the C version in the tutorial text on purpose with the idea that we can link directly to the spec such that the reader will know to look for the spec's version instead of the C++ version. That might be confusing though, maybe we should change it to C++ everywhere?
} | ||
---- | ||
|
||
The next section will introduce the first requirements that we'll check for in the `isDeviceSuitable` function. | ||
As we'll start using more Vulkan features in the later chapters we will also extend this function to include more checks. | ||
|
||
== Base device suitability checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider rewriting or removing this chapter all along and instead replace it with explicit device selection. This "device suitability check" has caused lots of confusion judging from community feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be good for after the merge to revisit. I agree, this needs to be revisited.
en/03_Drawing_a_triangle/00_Setup/04_Logical_device_and_queues.adoc
Outdated
Show resolved
Hide resolved
If possible, everyone take another look. There's several comments here that I have left open and haven't yet addressed. However, this PR is getting pretty complicated to work with and I'd like to get it merged so we can address any short comings in their own PR. So unless you see anything drastically as a bug, let's go ahead and merge this and then circle back for improvements. |
One thing that I might've missed initially: Base requirement is now Vulkan1.4. Not sure if that's what we want. Android is still far behind, and I fear we would leave too many people behind? |
Yep, that's actually new from adding in dynamic rendering etc. My thoughts
were to add a chapter about mobile and working with older versions in
general after the merge. So we get core principals and then how to handle
with getting more systems and use profiles etc.
…On Fri, Jun 20, 2025, 9:20 AM Sascha Willems ***@***.***> wrote:
*SaschaWillems* left a comment (KhronosGroup/Vulkan-Tutorial#61)
<#61 (comment)>
One thing that I might've missed initially: Base requirement is now
Vulkan1.4.
Not sure if that's what we want. Android is still far behind, and I fear
we would leave too many people behind?
—
Reply to this email directly, view it on GitHub
<#61 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA5IAY2EQJDMANKD2J3FDYT3EQYDZAVCNFSM6AAAAABZJMJGTCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSOJSGE4DENJZGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Introduce `install_dependencies_windows.bat` and `install_dependencies_linux.sh` to simplify setup for Vulkan projects. Updated documentation with instructions for using the scripts and clarified Vulkan SDK installation steps.
I went ahead and made two install scripts one for Linux and one for Windows. The Windows one is using vcpkg. While I might not use vcpkg anywhere, enough people have asked for it, and it does make writing the windows install script easier. |
Awesome 👍🏻 That simplifies things a lot. CMake is now almost working out of the box. I'm still seeing these errors: CMake Error at CMakeLists.txt:123 (file): (multiple similar errors) and CMake Error at V:/github/vcpkg/scripts/buildsystems/vcpkg.cmake:598 (_add_executable):
Call Stack (most recent call first): (miltuple simial errors) The first one can be fixed by adjusting the path for the resources (they're now located right besides the CMake file) The second one seems to be caused by files still being included that are no longer present (due to using dynamic rendering). Here is a patch file for that (not sure if it works outside of windows): |
A few chapters won't compile for me (Windows 11, Visual Studio 2022) due to syntax errors: 06_swap_chain_creation.cpp(263,29): error C2039: 'contains': is not a member of 'std::ranges'e formal list?) 05_window_surface.cpp(176,49): error C2187: syntax error: ')' was unexpected here 07_image_views.cpp(234,14): error C2039: 'enabledExtensionCount': is not a member of 'vk::DeviceQueueCreateInfo' 27_depth_buffering.cpp(1002,16): error C1075: '(': no matching token found It also looks like MSVC is having some serious issues with the Vulkan module, most samples look like this: They do compile, but code highlighting is completely broken. Matches my former experience with modules and MSVC :( |
So I got at least some of the chapters working. Did test e.g. the compute shader (since I wrote that one), but it didn't work as expected. First, it looks very different: Top is what it should look like, bottom is what it now looks like. Should be easy to fix. But the sample now also crashes when trying to resize the window: vk::Queue::presentKHR: ErrorOutOfDateKHR |
…ctory didn't fully make it over. This should compile everything again, I'm still working on compute_shader.
Updated swap chain format to use `vk::SurfaceFormatKHR` for better type clarity and consistency. Refined format and color space references throughout the code. Improved `chooseSwapSurfaceFormat` logic for desirable format selection and adjusted pipeline blending constants for correctness. Removed unused shader modules in the graphics pipeline setup.
I'll have to look at the color blending again when there's more time. The cause of that change is escaping me but I'm not getting a crash when I resize. I'll have to wait until after the move when I can setup my windows machine again to test if there's something I'm missing there that might cause the resize issue. Thanks for testing @SaschaWillems |
Thanks for fixing. I do get a compilation error in the base code project, but that looks totally bogus and I think it's a bug with MSVC and not the actual code. All other chapters now compile fine 👍🏻 |
Did a build of the docs site with this PR and read through all chapters. I'm happy with the changes, and I only noticed a few minotr things like some chapters still linking to the GLSL shaders or some broken links. But those can be easily fixed after the merge. Maybe we can discuss this PR on the next docs call and decide on how to move forward. |
[]( vk::QueueFamilyProperties const & qfp ) { return qfp.queueFlags & vk::QueueFlagBits::eGraphics; } ); | ||
// get the first index into queueFamilyProperties which supports graphics | ||
auto graphicsQueueFamilyProperty = std::ranges::find_if( queueFamilyProperties, []( auto const & qfp ) | ||
{ return (qfp.queueFlags & vk::QueueFlagBits::eGraphics) != static_cast<vk::QueueFlags>(0); } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe:
return (qfp.queueFlags & vk::QueueFlagBits::eGraphics);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiler error complaining that the type is not a bool. I could cast it, but I bet the compiler would give the same asm output from casting as simply comparing against 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some more comments...
std::vector<VkDynamicState> dynamicStates = { | ||
VK_DYNAMIC_STATE_VIEWPORT, | ||
VK_DYNAMIC_STATE_SCISSOR | ||
std::vector dynamicStates = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What a std::vector
is this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clang lint has started suggesting when the compiler can figure out what the types are automatically. This is an instance of trying that suggestion out. If you feel this loses clarity to the tutorial, we can certainly be more verbose.
attachments/11_render_passes.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this file deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaikr because the tutorial now uses dynamic rendering, so no more need for render passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, same for the framebuffers. Those go away. Once we merge, I'll renumber the chapters. I also am trying to hold back a new chapter on Mobile and using Vulkan prior to 1.4 and determining when that's needed. But we're already pretty complicated PR as it stands so I don't want to add until it can be an atomic add.
attachments/13_framebuffers.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this file deleted?
Refactor device extension handling and physical device selection logic Unified device extension references to `requiredDeviceExtension` for clarity and consistency. Improved physical device selection by incorporating checks for Vulkan 1.3 features and necessary dynamic rendering capabilities. Enhanced code readability with structured initialization and modern Vulkan C++ practices.
Improved device extension handling and shader input/output structures. Enhanced readability with structured initialization. Updated device selection to validate Vulkan 1.3 features and required extensions. Refined pipeline blending and color attachment setup for clarity and correctness.
…ng up unused code
@SaschaWillems I'm still not getting a crash on resize for 31_compute_shader. However, the bug was in the slang shader code. It took me a bit to figure out how to correctly replace gl_PointCoord with the slang equivalent. That should work now. I'm going to see if I can test on a few more platforms and get a reproduce on the resize crashbug. |
Interesting. I still get that with the latest change, and even more interestingly, it does NOT happen in the swap chain code of the sample. If I add a try..catch, I do run into the catch block: try {
result = presentQueue.presentKHR(presentInfo);
if (result == vk::Result::eErrorOutOfDateKHR || result == vk::Result::eSuboptimalKHR || framebufferResized) {
framebufferResized = false;
recreateSwapChain();
}
else if (result != vk::Result::eSuccess) {
throw std::runtime_error("failed to present swap chain image!");
}
}
catch (const std::exception& e) {
throw std::runtime_error("failed to present swap chain image!");
} So maybe the raii variant of vulkan-hpp defaults to exception raising for me? Visually the sample now looks as expected 👍🏻 But anyway, it's something we can safely fix after this PR is merged ;) |
Also need to find out how to make this work with MSVC's intellisense. It does not find anything inside the vulkan module for me, and it seems that MSVC needs to build a special file for intellisense to work with modules: https://developercommunity.visualstudio.com/t/Additional-intermediate-build-files-for-/10642332 |
This is a rewrite of the tutorial to use SLang and RAII. While it is complete and everything is working, this is a work in progress; PR is here to enable discussion and thought.