From a3c9543cb17f21697bb24cc41efbc6f43af74775 Mon Sep 17 00:00:00 2001 From: Johannes Schneider Date: Tue, 16 Jul 2024 19:41:49 +0200 Subject: [PATCH] [WIP] Work in progress --- .../vulkan-renderer/render-graph/buffer.hpp | 3 +- .../wrapper/commands/command_buffer.hpp | 44 +++++---------- .../vulkan-renderer/wrapper/swapchain.hpp | 4 +- src/vulkan-renderer/application.cpp | 20 ++++--- .../render-graph/render_graph.cpp | 42 ++++++++++----- src/vulkan-renderer/renderers/imgui.cpp | 54 +++++++++---------- .../wrapper/commands/command_buffer.cpp | 15 +++--- .../wrapper/commands/command_pool.cpp | 4 +- src/vulkan-renderer/wrapper/swapchain.cpp | 4 +- 9 files changed, 93 insertions(+), 97 deletions(-) diff --git a/include/inexor/vulkan-renderer/render-graph/buffer.hpp b/include/inexor/vulkan-renderer/render-graph/buffer.hpp index 60398d9d2..4d21a895c 100644 --- a/include/inexor/vulkan-renderer/render-graph/buffer.hpp +++ b/include/inexor/vulkan-renderer/render-graph/buffer.hpp @@ -37,6 +37,7 @@ class GraphicsPass; using inexor::vulkan_renderer::wrapper::Device; using inexor::vulkan_renderer::wrapper::commands::CommandBuffer; +using inexor::vulkan_renderer::wrapper::descriptors::DescriptorSetUpdateBuilder; // TODO: Store const reference to rendergraph and retrieve the swapchain image index for automatic buffer tripling @@ -47,7 +48,7 @@ class Buffer { friend class RenderGraph; friend class GraphicsPass; friend class CommandBuffer; - friend class inexor::vulkan_renderer::wrapper::descriptors::DescriptorSetUpdateBuilder; + friend class DescriptorSetUpdateBuilder; /// The device wrapper const Device &m_device; diff --git a/include/inexor/vulkan-renderer/wrapper/commands/command_buffer.hpp b/include/inexor/vulkan-renderer/wrapper/commands/command_buffer.hpp index 22f7b955f..c803e7053 100644 --- a/include/inexor/vulkan-renderer/wrapper/commands/command_buffer.hpp +++ b/include/inexor/vulkan-renderer/wrapper/commands/command_buffer.hpp @@ -20,8 +20,9 @@ class GraphicsPipeline; } // namespace inexor::vulkan_renderer::wrapper::pipelines namespace inexor::vulkan_renderer::render_graph { -// Forward declaration +// Forward declarations class Buffer; +class RenderGraph; } // namespace inexor::vulkan_renderer::render_graph namespace inexor::vulkan_renderer::wrapper::commands { @@ -29,23 +30,16 @@ namespace inexor::vulkan_renderer::wrapper::commands { /// RAII wrapper class for VkCommandBuffer /// @todo Make trivially copyable (this class doesn't really "own" the command buffer, more just an OOP wrapper). class CommandBuffer { + friend class Device; + friend class render_graph::RenderGraph; + friend class CommandPool; + +private: VkCommandBuffer m_cmd_buf{VK_NULL_HANDLE}; const Device &m_device; std::string m_name; std::unique_ptr m_wait_fence; - // The Device wrapper must be able to call begin_command_buffer and end_command_buffer - friend class Device; - - /// The staging buffers which are maybe used in the command buffer - /// This vector of staging buffers will be cleared every time ``begin_command_buffer`` is called - /// @note We are not recycling staging buffers. Once they are used and the command buffer handle has reached the end - /// of its lifetime, the staging bufers will be cleared. We trust Vulkan Memory Allocator (VMA) in managing the - /// memory for staging buffers. - mutable std::vector m_staging_bufs; - - friend class CommandPool; - /// Call vkBeginCommandBuffer /// @param flags The command buffer usage flags, ``VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT`` by default const CommandBuffer & // NOLINT @@ -154,7 +148,7 @@ class CommandBuffer { VkPipelineStageFlags dst_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT) const; /// Call vkCmdPipelineBarrier - /// @param image The image + /// @param img The image /// @param old_layout The old layout of the image /// @param new_layout The new layout of the image /// @param mip_level_count The number of mip levels (The parameter in ``VkImageSubresourceRange``) @@ -165,7 +159,7 @@ class CommandBuffer { /// @param dst_mask The destination pipeline stage flags (``VK_PIPELINE_STAGE_ALL_COMMANDS_BIT`` by default) /// @return A const reference to the dereferenced ``this`` pointer (allowing for method calls to be chained) const CommandBuffer & // NOLINT - change_image_layout(VkImage image, + change_image_layout(VkImage imgs, VkImageLayout old_layout, VkImageLayout new_layout, std::uint32_t mip_level_count = 1, @@ -419,24 +413,14 @@ class CommandBuffer { return push_constants(pipeline.lock()->m_pipeline_layout, stage, sizeof(data), &data, offset); } - // Graphics commands - // TODO(): Switch to taking in OOP wrappers when we have them (e.g. bind_vertex_buffers takes in a VertexBuffer) - - [[nodiscard]] VkCommandBuffer get() const { - return m_cmd_buf; - } - - [[nodiscard]] const synchronization::Fence &get_wait_fence() const { - return *m_wait_fence; - } - - [[nodiscard]] const VkCommandBuffer *ptr() const { - return &m_cmd_buf; - } - /// Call the reset method of the Fence member const CommandBuffer &reset_fence() const; + /// Set the name of a command buffer during recording of a specific command in the current command buffer + /// @param name The name of the suboperation + /// @return A const reference to the this pointer (allowing method calls to be chained) + const CommandBuffer &set_suboperation_debug_name(std::string name) const; + /// Call vkQueueSubmit /// @param submit_infos The submit infos const CommandBuffer &submit(std::span submit_infos) const; // NOLINT diff --git a/include/inexor/vulkan-renderer/wrapper/swapchain.hpp b/include/inexor/vulkan-renderer/wrapper/swapchain.hpp index 319e43427..f337a0a65 100644 --- a/include/inexor/vulkan-renderer/wrapper/swapchain.hpp +++ b/include/inexor/vulkan-renderer/wrapper/swapchain.hpp @@ -66,9 +66,7 @@ class Swapchain { /// Call vkAcquireNextImageKHR /// @param timeout (``std::numeric_limits::max()`` by default) /// @exception VulkanException vkAcquireNextImageKHR call failed - /// @return The index of the next image - [[nodiscard]] std::uint32_t - acquire_next_image_index(std::uint64_t timeout = std::numeric_limits::max()); + void acquire_next_image_index(std::uint64_t timeout = std::numeric_limits::max()); /// Choose the composite alpha /// @param request_composite_alpha requested compositing flag diff --git a/src/vulkan-renderer/application.cpp b/src/vulkan-renderer/application.cpp index a58c3923b..165cc4d50 100644 --- a/src/vulkan-renderer/application.cpp +++ b/src/vulkan-renderer/application.cpp @@ -513,25 +513,23 @@ void Application::setup_render_graph() { return m_octree_pipeline; }); - auto on_record_cmd_buffer = [&](const wrapper::commands::CommandBuffer &cmd_buf) { - cmd_buf.bind_pipeline(m_octree_pipeline) - .bind_descriptor_set(m_descriptor_set, m_octree_pipeline) - .bind_vertex_buffer(m_vertex_buffer) - .bind_index_buffer(m_index_buffer) - .draw_indexed(static_cast(m_octree_indices.size())); - }; - m_render_graph->add_graphics_pass([&](render_graph::GraphicsPassBuilder &builder) { m_octree_pass = builder.add_color_attachment(m_back_buffer, VkClearValue{1.0f, 0.0f, 0.0f, 1.0f}) .add_depth_attachment(m_depth_buffer) - .set_on_record(std::move(on_record_cmd_buffer)) + .set_on_record([&](const wrapper::commands::CommandBuffer &cmd_buf) { + cmd_buf.bind_pipeline(m_octree_pipeline) + .bind_descriptor_set(m_descriptor_set, m_octree_pipeline) + .bind_vertex_buffer(m_vertex_buffer) + .bind_index_buffer(m_index_buffer) + .draw_indexed(static_cast(m_octree_indices.size())); + }) .build("Octree"); return m_octree_pass; }); // TODO: We don't need to recreate the imgui overlay when swapchain is recreated, use a .recreate() method instead? - m_imgui_overlay = std::make_unique(*m_device, *m_swapchain, *m_render_graph.get(), - m_back_buffer, [&]() { update_imgui_overlay(); }); + // m_imgui_overlay = std::make_unique(*m_device, *m_swapchain, *m_render_graph.get(), + // m_back_buffer, [&]() { update_imgui_overlay(); }); m_render_graph->compile(); } diff --git a/src/vulkan-renderer/render-graph/render_graph.cpp b/src/vulkan-renderer/render-graph/render_graph.cpp index 1a7c142b2..c4b4ac5c7 100644 --- a/src/vulkan-renderer/render-graph/render_graph.cpp +++ b/src/vulkan-renderer/render-graph/render_graph.cpp @@ -75,9 +75,11 @@ void RenderGraph::compile() { } void RenderGraph::create_buffers() { - m_device.execute("[RenderGraph::create_buffers]", [&](const CommandBuffer &cmd_buf) { + m_device.execute("[RenderGraph::create_buffers|", [&](const CommandBuffer &cmd_buf) { for (const auto &buffer : m_buffers) { buffer->m_on_update(); + // Rename the command buffer before creating every buffer for fine-grained debugging + cmd_buf.set_suboperation_debug_name("Buffer:" + buffer->m_name + "]"); buffer->create(cmd_buf); } }); @@ -180,19 +182,25 @@ void RenderGraph::create_rendering_infos() { } }; + bool msaa_enabled = (attach_ptr->m_msaa_img != nullptr); + // This decides if MSAA is enabled on a per-texture basis return wrapper::make_info({ - .imageView = attach_ptr->m_img->m_img_view, + .imageView = msaa_enabled ? attach_ptr->m_msaa_img->m_img_view : attach_ptr->m_img->m_img_view, + // The image layout is the same for m_img and m_msaa_img .imageLayout = img_layout, - .resolveMode = (attach_ptr->m_sample_count > VK_SAMPLE_COUNT_1_BIT) - // The resolve mode must be chosen on whether it's an integer format - ? (!is_integer_format(attach_ptr->m_format) ? VK_RESOLVE_MODE_AVERAGE_BIT - : VK_RESOLVE_MODE_MIN_BIT) - : VK_RESOLVE_MODE_NONE, - .resolveImageView = attach_ptr->m_msaa_img ? attach_ptr->m_msaa_img->m_img_view : VK_NULL_HANDLE, - .resolveImageLayout = attach_ptr ? img_layout : VK_IMAGE_LAYOUT_UNDEFINED, + // The resolve mode must be chosen on whether it's an integer format + .resolveMode = msaa_enabled ? (!is_integer_format(attach_ptr->m_format) ? VK_RESOLVE_MODE_AVERAGE_BIT + : VK_RESOLVE_MODE_MIN_BIT) + : VK_RESOLVE_MODE_NONE, // No resolve if MSAA is disabled + .resolveImageView = msaa_enabled ? attach_ptr->m_img->m_img_view : VK_NULL_HANDLE, + // TODO: Is this correct layout? + .resolveImageLayout = msaa_enabled ? img_layout : VK_IMAGE_LAYOUT_UNDEFINED, + // Does this pass require clearing this attachment? .loadOp = attachment.second ? VK_ATTACHMENT_LOAD_OP_CLEAR : VK_ATTACHMENT_LOAD_OP_LOAD, + // The result will always be stored .storeOp = VK_ATTACHMENT_STORE_OP_STORE, + // If clearing is enabled, specify the clear value .clearValue = attachment.second ? attachment.second.value() : VkClearValue{}, }); }; @@ -233,14 +241,18 @@ void RenderGraph::create_rendering_infos() { } void RenderGraph::create_textures() { - m_device.execute("[RenderGraph::create_textures]", [&](const CommandBuffer &cmd_buf) { + m_device.execute("[RenderGraph::create_textures|", [&](const CommandBuffer &cmd_buf) { for (const auto &texture : m_textures) { // TODO: Check if this initializes all textures (internal ones from rendergraph and external like ImGui?) if (texture->m_on_init) { std::invoke(texture->m_on_init.value()); } + // Rename the command buffer before creating every texture for fine-grained debugging + cmd_buf.set_suboperation_debug_name("Texture-Create:" + texture->m_name + "]"); texture->create(); if (texture->m_usage == TextureUsage::NORMAL) { + // Rename the command buffer before creating every texture for fine-grained debugging + cmd_buf.set_suboperation_debug_name("Texture-Update:" + texture->m_name + "]"); // Only external textures are updated, not back or depth buffers used internally in rendergraph texture->update(cmd_buf); } @@ -254,6 +266,9 @@ void RenderGraph::determine_pass_order() { } void RenderGraph::record_command_buffer_for_pass(const CommandBuffer &cmd_buf, const GraphicsPass &pass) { + // Rename the command buffer before creating every texture for fine-grained debugging + cmd_buf.set_suboperation_debug_name("Pass:" + pass.m_name + "]"); + // TODO: Define default color values for debug labels! // Start a new debug label for this graphics pass (visible in graphics debuggers like RenderDoc) cmd_buf.begin_debug_label_region(pass.m_name, {1.0f, 0.0f, 0.0f, 1.0f}); @@ -286,7 +301,10 @@ void RenderGraph::record_command_buffers(const CommandBuffer &cmd_buf) { } void RenderGraph::render() { - const auto &cmd_buf = m_device.request_command_buffer("[RenderGraph::render]"); + // Acquire the next swapchain image index + m_swapchain.acquire_next_image_index(); + + const auto &cmd_buf = m_device.request_command_buffer("[RenderGraph::render|"); // TODO: Record command buffers for passes in parallel! record_command_buffers(cmd_buf); @@ -297,7 +315,7 @@ void RenderGraph::render() { .pWaitSemaphores = m_swapchain.image_available_semaphore(), .pWaitDstStageMask = stage_mask.data(), .commandBufferCount = 1, - .pCommandBuffers = cmd_buf.ptr(), + .pCommandBuffers = &cmd_buf.m_cmd_buf, })); m_swapchain.present(); } diff --git a/src/vulkan-renderer/renderers/imgui.cpp b/src/vulkan-renderer/renderers/imgui.cpp index e46f45bea..03cc36d38 100644 --- a/src/vulkan-renderer/renderers/imgui.cpp +++ b/src/vulkan-renderer/renderers/imgui.cpp @@ -120,37 +120,35 @@ ImGuiRenderer::ImGuiRenderer(const Device &device, return m_imgui_pipeline; }); - auto on_record_cmd_buffer = [&](const wrapper::commands::CommandBuffer &cmd_buf) { - const ImGuiIO &io = ImGui::GetIO(); - m_push_const_block.scale = glm::vec2(2.0f / io.DisplaySize.x, 2.0f / io.DisplaySize.y); - - cmd_buf.bind_pipeline(m_imgui_pipeline) - .bind_vertex_buffer(m_vertex_buffer) - .bind_index_buffer(m_index_buffer) - .bind_descriptor_set(m_descriptor_set, m_imgui_pipeline) - .push_constant(m_imgui_pipeline, m_push_const_block, VK_SHADER_STAGE_VERTEX_BIT); - - ImDrawData *draw_data = ImGui::GetDrawData(); - if (draw_data == nullptr) { - return; - } - std::uint32_t index_offset = 0; - std::int32_t vertex_offset = 0; - for (std::size_t i = 0; i < draw_data->CmdListsCount; i++) { - const ImDrawList *cmd_list = draw_data->CmdLists[i]; - for (std::int32_t j = 0; j < cmd_list->CmdBuffer.Size; j++) { - const ImDrawCmd &draw_cmd = cmd_list->CmdBuffer[j]; - cmd_buf.draw_indexed(draw_cmd.ElemCount, 1, index_offset, vertex_offset); - index_offset += draw_cmd.ElemCount; - } - vertex_offset += cmd_list->VtxBuffer.Size; - } - }; - render_graph.add_graphics_pass([&](render_graph::GraphicsPassBuilder &builder) { // TODO: builder.reads_from(octree_pass)..! m_imgui_pass = builder.add_color_attachment(m_color_attachment) - .set_on_record(std::move(on_record_cmd_buffer)) + .set_on_record([&](const wrapper::commands::CommandBuffer &cmd_buf) { + const ImGuiIO &io = ImGui::GetIO(); + m_push_const_block.scale = glm::vec2(2.0f / io.DisplaySize.x, 2.0f / io.DisplaySize.y); + + cmd_buf.bind_pipeline(m_imgui_pipeline) + .bind_vertex_buffer(m_vertex_buffer) + .bind_index_buffer(m_index_buffer) + .bind_descriptor_set(m_descriptor_set, m_imgui_pipeline) + .push_constant(m_imgui_pipeline, m_push_const_block, VK_SHADER_STAGE_VERTEX_BIT); + + ImDrawData *draw_data = ImGui::GetDrawData(); + if (draw_data == nullptr) { + return; + } + std::uint32_t index_offset = 0; + std::int32_t vertex_offset = 0; + for (std::size_t i = 0; i < draw_data->CmdListsCount; i++) { + const ImDrawList *cmd_list = draw_data->CmdLists[i]; + for (std::int32_t j = 0; j < cmd_list->CmdBuffer.Size; j++) { + const ImDrawCmd &draw_cmd = cmd_list->CmdBuffer[j]; + cmd_buf.draw_indexed(draw_cmd.ElemCount, 1, index_offset, vertex_offset); + index_offset += draw_cmd.ElemCount; + } + vertex_offset += cmd_list->VtxBuffer.Size; + } + }) .build("ImGui"); return m_imgui_pass; }); diff --git a/src/vulkan-renderer/wrapper/commands/command_buffer.cpp b/src/vulkan-renderer/wrapper/commands/command_buffer.cpp index 037d03595..53533c830 100644 --- a/src/vulkan-renderer/wrapper/commands/command_buffer.cpp +++ b/src/vulkan-renderer/wrapper/commands/command_buffer.cpp @@ -36,7 +36,6 @@ CommandBuffer::CommandBuffer(CommandBuffer &&other) noexcept : m_device(other.m_ m_cmd_buf = std::exchange(other.m_cmd_buf, VK_NULL_HANDLE); m_name = std::move(other.m_name); m_wait_fence = std::exchange(other.m_wait_fence, nullptr); - m_staging_bufs = std::move(other.m_staging_bufs); } const CommandBuffer &CommandBuffer::begin_command_buffer(const VkCommandBufferUsageFlags flags) const { @@ -44,9 +43,6 @@ const CommandBuffer &CommandBuffer::begin_command_buffer(const VkCommandBufferUs .flags = flags, }); vkBeginCommandBuffer(m_cmd_buf, &begin_info); - - // We must clear the staging buffers which could be left over from previous use of this command buffer - m_staging_bufs.clear(); return *this; } @@ -181,7 +177,7 @@ const CommandBuffer &CommandBuffer::change_image_layout(const VkImage image, return pipeline_image_memory_barrier(src_mask, dst_mask, barrier); } -const CommandBuffer &CommandBuffer::change_image_layout(const VkImage image, +const CommandBuffer &CommandBuffer::change_image_layout(const VkImage img, const VkImageLayout old_layout, const VkImageLayout new_layout, const std::uint32_t mip_level_count, @@ -190,8 +186,8 @@ const CommandBuffer &CommandBuffer::change_image_layout(const VkImage image, const std::uint32_t base_array_layer, const VkPipelineStageFlags src_mask, const VkPipelineStageFlags dst_mask) const { - assert(image); - return change_image_layout(image, old_layout, new_layout, + assert(img); + return change_image_layout(img, old_layout, new_layout, { .aspectMask = VK_IMAGE_ASPECT_COLOR_BIT, .baseMipLevel = base_mip_level, @@ -450,6 +446,11 @@ const CommandBuffer &CommandBuffer::reset_fence() const { return *this; } +const CommandBuffer &CommandBuffer::set_suboperation_debug_name(std::string name) const { + m_device.set_debug_name(m_cmd_buf, m_name + name); + return *this; +} + const CommandBuffer &CommandBuffer::submit(const std::span submit_infos) const { assert(!submit_infos.empty()); end_command_buffer(); diff --git a/src/vulkan-renderer/wrapper/commands/command_pool.cpp b/src/vulkan-renderer/wrapper/commands/command_pool.cpp index 181000973..057f84d01 100644 --- a/src/vulkan-renderer/wrapper/commands/command_pool.cpp +++ b/src/vulkan-renderer/wrapper/commands/command_pool.cpp @@ -44,7 +44,7 @@ const commands::CommandBuffer &CommandPool::request_command_buffer(const std::st // Reset the command buffer's fence to make it usable again cmd_buf->reset_fence(); cmd_buf->begin_command_buffer(); - m_device.set_debug_name(cmd_buf->get(), name); + m_device.set_debug_name(cmd_buf->m_cmd_buf, name); return *cmd_buf; } } @@ -57,7 +57,7 @@ const commands::CommandBuffer &CommandPool::request_command_buffer(const std::st auto &new_cmd_buf = *m_cmd_bufs.back(); new_cmd_buf.begin_command_buffer(); - m_device.set_debug_name(new_cmd_buf.get(), name); + m_device.set_debug_name(new_cmd_buf.m_cmd_buf, name); return new_cmd_buf; } diff --git a/src/vulkan-renderer/wrapper/swapchain.cpp b/src/vulkan-renderer/wrapper/swapchain.cpp index 34e2039bb..d9d592ea2 100644 --- a/src/vulkan-renderer/wrapper/swapchain.cpp +++ b/src/vulkan-renderer/wrapper/swapchain.cpp @@ -38,7 +38,7 @@ Swapchain::Swapchain(Swapchain &&other) noexcept : m_device(other.m_device) { m_img_index = other.m_img_index; } -std::uint32_t Swapchain::acquire_next_image_index(const std::uint64_t timeout) { +void Swapchain::acquire_next_image_index(const std::uint64_t timeout) { if (const auto result = vkAcquireNextImageKHR(m_device.device(), m_swapchain, timeout, *m_img_available->semaphore(), VK_NULL_HANDLE, &m_img_index); result != VK_SUCCESS) { @@ -51,7 +51,6 @@ std::uint32_t Swapchain::acquire_next_image_index(const std::uint64_t timeout) { } m_current_img_view = m_img_views[m_img_index]; m_current_img = m_imgs[m_img_index]; - return m_img_index; } std::optional @@ -292,7 +291,6 @@ void Swapchain::setup(const std::uint32_t width, const std::uint32_t height, con throw VulkanException("Error: vkCreateImageView failed for swapchain image view!", result); } } - acquire_next_image_index(); } Swapchain::~Swapchain() {