This is a new version of the tutorial using RAII and SLang #61
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.
en/00_Introduction.adoc
Outdated
| can click them to learn more. | ||
| Vulkan is a very new API, so there may be some shortcomings in the | ||
| specification itself. | ||
| You are encouraged to submit feedback to https://github |
There was a problem hiding this comment.
It looks like the line width changed for all adoc files. This makes it kinda hard to compare and in some cases also breaks links:
Is there a way to revert the line width/break changes?
There was a problem hiding this comment.
There's not a good way to do that sadly :( The line width in the adoc pages didn't seem to work so I just grabbed the original markdown settings and used them.
|
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? |
|
|
||
| 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.
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.
Should be updated to use the vulkan.hpp enums instead (e.g. vk::DebugUtilsMessageSeverityFlagBitsEXT::eWarning)
There was a problem hiding this comment.
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.
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.
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
|
|
||
| layout(location = 0) out vec4 outColor; | ||
| Sampler2D texture; |
There was a problem hiding this comment.
Even though it may not be required, maybe explicitly add the binding slot, so people know it's something that might be required by Slang and Vulkan.
There was a problem hiding this comment.
Let's come back to this after the merge. I'd like to do it across all of the tutorial instead of this one instance so we commit to one method as the "best practice" for teaching it.
attachments/09_shader_modules.cpp
Outdated
| } | ||
| [[nodiscard]] vk::raii::ShaderModule createShaderModule(const std::vector<char>& code) const { | ||
| vk::ShaderModuleCreateInfo createInfo({}, code.size(), reinterpret_cast<const uint32_t*>(code.data()) ); | ||
| vk::raii::ShaderModule shaderModule(*device, createInfo); |
There was a problem hiding this comment.
Maybe just
return device->createShaderModule( createInfo );
|
|
||
| std::vector<vk::ExtensionProperties> props = context.enumerateInstanceExtensionProperties(); | ||
| if (const auto propsIterator = std::ranges::find_if(props, []( vk::ExtensionProperties const & ep ) { return strcmp( ep.extensionName, vk::EXTDebugUtilsExtensionName ) == 0; } ); propsIterator == props.end() ) |
There was a problem hiding this comment.
Maybe perform this check only if enableValidationLayers is true?
| for ( auto image : swapChainImages ) | ||
| { | ||
| imageViewCreateInfo.image = image; | ||
| swapChainImageViews.emplace_back( *device, imageViewCreateInfo ); | ||
| } | ||
| } | ||
|
|
||
| void createGraphicsPipeline() { | ||
|
|
||
| } |
There was a problem hiding this comment.
Is it intended to have this empty function to create the graphics pipeline?
In the tutorial named 08_graphics_pipeline?
| vkDestroyShaderModule(device, vertShaderModule, nullptr); | ||
| vk::PipelineShaderStageCreateInfo vertShaderStageInfo({}, vk::ShaderStageFlagBits::eVertex, shaderModule, "vertMain"); | ||
| vk::PipelineShaderStageCreateInfo fragShaderStageInfo({}, vk::ShaderStageFlagBits::eFragment, shaderModule, "fragMain"); | ||
| vk::PipelineShaderStageCreateInfo shaderStages[] = {vertShaderStageInfo, fragShaderStageInfo}; |
There was a problem hiding this comment.
createGraphicsPipeline just creates a ShaderModule?
|
|
||
| if (result == VK_ERROR_OUT_OF_DATE_KHR) { | ||
| if (result == vk::Result::eErrorOutOfDateKHR) { |
There was a problem hiding this comment.
As long as you don't have VULKAN_HPP_NO_EXCEPTIONS defined, vk::Result::eErrorOutOfDateKHR would be thrown as an exception.
That is, you would need to try acquireNextImage and either catch the specific vk::OutOfDateKHRError, or the general vk::SystemError.
| if (result == VK_ERROR_OUT_OF_DATE_KHR || result == VK_SUBOPTIMAL_KHR || framebufferResized) { | ||
| const vk::PresentInfoKHR presentInfoKHR( **renderFinishedSemaphore[currentFrame], **swapChain, imageIndex ); | ||
| result = presentQueue->presentKHR( presentInfoKHR ); | ||
| if (result == vk::Result::eErrorOutOfDateKHR || result == vk::Result::eSuboptimalKHR || framebufferResized) { |
There was a problem hiding this comment.
As above: you need to try/catch on vk::Result::eErrorOutOfDataKHR.
| } | ||
|
|
||
| vkBindBufferMemory(device, buffer, bufferMemory, 0); | ||
| void createBuffer(vk::DeviceSize size, vk::BufferUsageFlags usage, vk::MemoryPropertyFlags properties, vk::raii::Buffer& buffer, vk::raii::DeviceMemory& bufferMemory) { |
There was a problem hiding this comment.
Maybe a slightly different signature:
std::pair<vk::raii::Buffer, vk::raii::DeviceMemory> createBuffer( vk::DeviceSize size, vk::BufferUsageFlags usage, vk::MemoryPropertyFlags properties )
Would be called then like
vk::raii::Buffer stagingBuffer(nullptr);
vk::raii::DeviceMemory stagingBufferMemory(nullptr);
std::tie( stagingBuffer, stagingBufferMemory ) =
createBuffer( bufferSize, vk::BufferUsageFlagBits::eTransferSrc, vk::MemoryPropertyFlagBits::eHostVisible | vk::MemoryPropertyFlagBits::eHostCoherent );
or
auto[ stagingBuffer, stagingBufferMemory ] =
createBuffer( bufferSize, vk::BufferUsageFlagBits::eTransferSrc, vk::MemoryPropertyFlagBits::eHostVisible | vk::MemoryPropertyFlagBits::eHostCoherent );
| memcpy(data, vertices.data(), (size_t) bufferSize); | ||
| vkUnmapMemory(device, stagingBufferMemory); | ||
| vk::DeviceSize bufferSize = sizeof(vertices[0]) * vertices.size(); | ||
| vk::raii::Buffer stagingBuffer({}); |
There was a problem hiding this comment.
To make sure, a vk::raii::Object is generated empty, pass a nullptr in:
vk::raii::Buffer stagingBuffer(nullptr);
attachments/21_index_buffer.cpp
Outdated
| commandBuffers[currentFrame]->setViewport(0, vk::Viewport(0.0f, 0.0f, static_cast<float>(swapChainExtent.width), static_cast<float>(swapChainExtent.height), 0.0f, 1.0f)); | ||
| commandBuffers[currentFrame]->setScissor( 0, vk::Rect2D( vk::Offset2D( 0, 0 ), swapChainExtent ) ); | ||
| commandBuffers[currentFrame]->bindVertexBuffers(0, **vertexBuffer, {0}); | ||
| commandBuffers[currentFrame]->bindIndexBuffer( *indexBuffer, 0, vk::IndexType::eUint16 ); |
There was a problem hiding this comment.
Maybe, instead of vk::IndexType::eUint16:
vk::IndexTypeValue<decltype(indices)::value_type>::value
It's substantially harder to read, but it makes clear where that value comes from (indices), and it stay correct in case the indices are changed to uint32_t, or so.
attachments/22_descriptor_layout.cpp
Outdated
There was a problem hiding this comment.
Shouldn't this file be named 22_descriptor_set_layout.cpp?
There was a problem hiding this comment.
Let's do that after the merge. The changes are already getting a bit too large for one PR and a move in git commit seems like a quick way to make it worse.
attachments/24_texture_image.cpp
Outdated
| createImage(texWidth, texHeight, VK_FORMAT_R8G8B8A8_SRGB, VK_IMAGE_TILING_OPTIMAL, VK_IMAGE_USAGE_TRANSFER_DST_BIT | VK_IMAGE_USAGE_SAMPLED_BIT, VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT, textureImage, textureImageMemory); | ||
| vk::raii::Image textureImageTemp({}); | ||
| vk::raii::DeviceMemory textureImageMemoryTemp({}); | ||
| createImage(texWidth, texHeight, vk::Format::eR8G8B8A8Srgb, vk::ImageTiling::eOptimal, vk::ImageUsageFlagBits::eTransferDst | vk::ImageUsageFlagBits::eSampled, vk::MemoryPropertyFlagBits::eDeviceLocal, textureImageTemp, textureImageMemoryTemp); |
There was a problem hiding this comment.
createImage takes uint32_t for width and height, but texWidth and texHeight are int. Maybe change them to uint32_t?
There was a problem hiding this comment.
stbi_load takes them as ints. It might be worthwhile to make the type specifics clear but maybe after the merge this is worth doing?
attachments/25_sampler.cpp
Outdated
| vk::DeviceQueueCreateInfo deviceQueueCreateInfo( {}, graphicsIndex, 1, &queuePriority ); | ||
| vk::DeviceCreateInfo deviceCreateInfo( {}, deviceQueueCreateInfo ); | ||
| vk::PhysicalDeviceFeatures deviceFeatures; | ||
| deviceFeatures.samplerAnisotropy = vk::True; |
There was a problem hiding this comment.
Is it checked somewhere if samplerAnisotropy is supported?
The previous version checked it in pickPhysicalDevice via isDeviceSuitable
| @@ -711,354 +421,203 @@ class HelloTriangleApplication { | |||
| throw std::runtime_error("failed to load texture image!"); | |||
There was a problem hiding this comment.
Three lines above: VkDeviceSize -> vk::DeviceSize
attachments/27_depth_buffering.cpp
Outdated
| for (VkFormat format : candidates) { | ||
| VkFormatProperties props; | ||
| vkGetPhysicalDeviceFormatProperties(physicalDevice, format, &props); | ||
| vk::Format findSupportedFormat(const std::vector<vk::Format>& candidates, vk::ImageTiling tiling, vk::FormatFeatureFlags features) { |
There was a problem hiding this comment.
Maybe
auto formatIt = std::ranges::find_if(candidates, [&physicalDevice, &tiling, &features](auto const format){
vk::FormatProperties props = physicalDevice->getFormatProperties(format);
return (((tiling == vk::ImageTiling::eLinear) && ((props.linearTilingFeatures & features) == features)) ||
((tiling == vk::ImageTiling::eOptimal) && ((props.optimalTilingFeatures & features) == features)));
});
if ( formatIt == candidates.end())
{
throw std::runtime_error("failed to find supported format!");
}
return *formatIt;
attachments/27_depth_buffering.cpp
Outdated
| {VK_FORMAT_D32_SFLOAT, VK_FORMAT_D32_SFLOAT_S8_UINT, VK_FORMAT_D24_UNORM_S8_UINT}, | ||
| VK_IMAGE_TILING_OPTIMAL, | ||
| VK_FORMAT_FEATURE_DEPTH_STENCIL_ATTACHMENT_BIT | ||
| {vk::Format::eD32Sfloat, vk::Format::eD32SfloatS8Uint, vk::Format::eD24UnormS8Uint}, |
There was a problem hiding this comment.
What about all the other depth formats (like eD16Unorm, eX8D24UnormPack32, eD16UnormS8Uint,...)?
There was a problem hiding this comment.
Let's add that after the merge.
attachments/27_depth_buffering.cpp
Outdated
| bool hasStencilComponent(VkFormat format) { | ||
| return format == VK_FORMAT_D32_SFLOAT_S8_UINT || format == VK_FORMAT_D24_UNORM_S8_UINT; | ||
| bool hasStencilComponent(vk::Format format) { | ||
| return format == vk::Format::eD32SfloatS8Uint || format == vk::Format::eD24UnormS8Uint; |
There was a problem hiding this comment.
What about the other stencil formats (like eS8Uint, eD16UnormS8Uint, eS8Uint,...)
There was a problem hiding this comment.
same here, I want to test all the code in as many places as possible and this PR already has too much scope.
attachments/28_model_loading.cpp
Outdated
| if (vkCreateImageView(device, &viewInfo, nullptr, &imageView) != VK_SUCCESS) { | ||
| throw std::runtime_error("failed to create image view!"); | ||
| } | ||
| [[nodiscard]] std::unique_ptr<vk::raii::ImageView> createImageView(const vk::raii::Image& image, vk::Format format, vk::ImageAspectFlags aspectFlags) const { |
There was a problem hiding this comment.
Maybe the consts should be applied to the samples before as well.
There was a problem hiding this comment.
agreed, but this is already getting to be a lot of changes, I'll do another PR to cleanup things like this and ensure it's the same in all the files after this one is merged.
attachments/28_model_loading.cpp
Outdated
| @@ -1012,7 +601,7 @@ class HelloTriangleApplication { | |||
|
|
|||
| vertex.color = {1.0f, 1.0f, 1.0f}; | |||
|
|
|||
| if (uniqueVertices.count(vertex) == 0) { | |||
| if (!uniqueVertices.contains(vertex)) { | |||
There was a problem hiding this comment.
Maybe instead:
auto [vertexIt, inserted] = uniqueVertices.insert({vertex, static_cast<uint32_t>(vertices.size())});
if (inserted)
{
vertices.push_back(vertex);
}
indices.push_back(vertexIt->second);
attachments/30_multisampling.cpp
Outdated
| vk::DeviceQueueCreateInfo deviceQueueCreateInfo( {}, graphicsIndex, 1, &queuePriority ); | ||
| vk::PhysicalDeviceFeatures deviceFeatures; | ||
| vk::DeviceCreateInfo deviceCreateInfo( {}, deviceQueueCreateInfo, {}, deviceExtensions, &deviceFeatures ); | ||
| deviceFeatures.samplerAnisotropy = vk::True; |
There was a problem hiding this comment.
Is it checked anywhere that samplerAnisotropy is supported?
attachments/30_multisampling.cpp
Outdated
| deviceFeatures.samplerAnisotropy = vk::True; | ||
| deviceCreateInfo.pEnabledFeatures = &deviceFeatures; | ||
| deviceCreateInfo.enabledExtensionCount = deviceExtensions.size(); | ||
| deviceCreateInfo.ppEnabledExtensionNames = deviceExtensions.data(); |
There was a problem hiding this comment.
Is it checked anywhere that the deviceExtensions are supported?
attachments/30_multisampling.cpp
Outdated
| barrier.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; | ||
| barrier.subresourceRange.aspectMask = VK_IMAGE_ASPECT_COLOR_BIT; | ||
| barrier.srcQueueFamilyIndex = vk::QueueFamilyIgnored; | ||
| barrier.dstQueueFamilyIndex = vk::QueueFamilyIgnored; |
There was a problem hiding this comment.
Those three members are already set in the constructor right above.
attachments/31_compute_shader.cpp
Outdated
| queueFamilyProperties.end(), | ||
| []( vk::QueueFamilyProperties const & qfp ) { return (qfp.queueFlags & vk::QueueFlagBits::eGraphics && qfp.queueFlags & vk::QueueFlagBits::eCompute); } ); | ||
|
|
||
| graphicsAndComputeIndex = static_cast<uint32_t>( std::distance( queueFamilyProperties.begin(), graphicsAndComputeQueueFamilyProperty ) ); |
There was a problem hiding this comment.
What if there's no queue family supporting both, graphics and compute?
attachments/31_compute_shader.cpp
Outdated
| device = std::make_unique<vk::raii::Device>( *physicalDevice, deviceCreateInfo ); | ||
| graphicsQueue = std::make_unique<vk::raii::Queue>( *device, graphicsAndComputeIndex, 0 ); | ||
| computeQueue = std::make_unique<vk::raii::Queue>( *device, graphicsAndComputeIndex, 0 ); | ||
| presentQueue = std::make_unique<vk::raii::Queue>( *device, presentIndex, 0 ); |
There was a problem hiding this comment.
While presentQueue might be different from the graphicsQueue, computeQueue and graphicsQueue are the same. Why do you use them as separate entities?
Maybe you should at least use different queue indices for those two?
attachments/31_compute_shader.cpp
Outdated
|
|
||
| vk::AttachmentReference colorAttachmentRef(0, vk::ImageLayout::eColorAttachmentOptimal); | ||
| vk::SubpassDescription subpass({}, vk::PipelineBindPoint::eGraphics, {}, {colorAttachmentRef}); | ||
| vk::SubpassDependency dependency(VK_SUBPASS_EXTERNAL, {}, |
There was a problem hiding this comment.
VK_SUBPASS_EXTERNAL -> vk::SubpassExternal?
attachments/31_compute_shader.cpp
Outdated
|
|
||
| if (vkAllocateCommandBuffers(device, &allocInfo, computeCommandBuffers.data()) != VK_SUCCESS) { | ||
| throw std::runtime_error("failed to allocate compute command buffers!"); | ||
| vk::CommandBufferAllocateInfo allocInfo(*commandPool, vk::CommandBufferLevel::ePrimary, computeCommandBuffers.size()); |
There was a problem hiding this comment.
Not a big difference to createCommandBuffers. Maybe replace them by a function handling both cases?
There should only be an "attachment" folder. That's what Antora requires, all files from code need to be moved there. |
images/extra_square.svg
Outdated
There was a problem hiding this comment.
Why have you changed the background of this and all the following images?
There was a problem hiding this comment.
I didn't even know that I made that change. I didn't touch the images.
… attachments directory and only there. * switch to using VULKAN_HPP_NO_STRUCT_CONSTRUCTORS so we get slightly more verbosity in our example code. * use Vulkan 1.4 modern techniques ** show removal of framebuffer and renderpasses ** show timeline semaphores in the compute example. ** show synch2 practices.
Simplified examples by updating to RAII patterns for Vulkan-Hpp bindings. Added dynamic rendering to replace render pass and framebuffer usage, aligning with Vulkan 1.3+. Improved code readability with structured initialization and fixed typos in documentation.
I'm going to come back to this request after it's merged. It's definitely a good idea. Don't know if it's better/worse than submodules, but it's a good idea. |
Updated code and documentation to align with modern Vulkan best practices: - Adopted RAII wrappers for better resource management and type safety. - Introduced structured initialization for clarity and consistency. - Improved tutorial content with grammatical fixes, terminology updates, and modernization of examples. - Rephrased sections for better readability and alignment with Vulkan 1.3+ features.
|
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.
Maybe:
return (qfp.queueFlags & vk::QueueFlagBits::eGraphics);
There was a problem hiding this comment.
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.
| std::vector<VkDynamicState> dynamicStates = { | ||
| VK_DYNAMIC_STATE_VIEWPORT, | ||
| VK_DYNAMIC_STATE_SCISSOR | ||
| std::vector dynamicStates = { |
There was a problem hiding this comment.
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.
Afaikr because the tutorial now uses dynamic rendering, so no more need for render passes.
There was a problem hiding this comment.
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
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.
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.