From 12d9f9817c4500a9106fddcf810d1c306714b843 Mon Sep 17 00:00:00 2001 From: Johannes Schneider Date: Sun, 21 Jul 2024 08:34:49 +0200 Subject: [PATCH] [WIP] Work in progress --- .../render-graph/render_graph.hpp | 22 +++--- .../vulkan-renderer/render-graph/texture.hpp | 10 +-- .../vulkan-renderer/renderers/imgui.hpp | 1 + src/vulkan-renderer/application.cpp | 5 +- src/vulkan-renderer/render-graph/buffer.cpp | 3 +- .../render-graph/render_graph.cpp | 71 ++++++------------- src/vulkan-renderer/render-graph/texture.cpp | 12 ++-- src/vulkan-renderer/renderers/imgui.cpp | 15 ++-- 8 files changed, 52 insertions(+), 87 deletions(-) diff --git a/include/inexor/vulkan-renderer/render-graph/render_graph.hpp b/include/inexor/vulkan-renderer/render-graph/render_graph.hpp index a449fddc8..6b555edb1 100644 --- a/include/inexor/vulkan-renderer/render-graph/render_graph.hpp +++ b/include/inexor/vulkan-renderer/render-graph/render_graph.hpp @@ -258,9 +258,6 @@ class RenderGraph { /// @param pass The graphics pass void fill_graphics_pass_rendering_info(GraphicsPass &pass); - /// Call m_on_initialize for every texture - void initialize_textures(); - /// Record the command buffer of a pass. After a lot of discussions about the API design of rendergraph, we came to /// the conclusion that it's the full responsibility of the programmer to manually bind pipelines, descriptors sets, /// and buffers inside of the on_record function instead of attempting to abstract all of this in rendergraph. This @@ -340,17 +337,16 @@ class RenderGraph { /// @param usage The usage of the texture inside of rendergraph /// @param format The format of the texture /// @param sample_count The sample count of the texture - /// @param on_init The texture initialization function - /// @param on_update The texture update function + /// @param m_on_check_for_updates The texture update function (an empty lambda by default) /// @return A weak pointer to the texture that was created - [[nodiscard]] std::weak_ptr add_texture(std::string texture_name, - TextureUsage usage, - VkFormat format, - std::uint32_t width, - std::uint32_t height, - VkSampleCountFlagBits sample_count = VK_SAMPLE_COUNT_1_BIT, - std::optional> on_init = std::nullopt, - std::optional> on_update = std::nullopt); + [[nodiscard]] std::weak_ptr add_texture( + std::string texture_name, + TextureUsage usage, + VkFormat format, + std::uint32_t width, + std::uint32_t height, + VkSampleCountFlagBits sample_count = VK_SAMPLE_COUNT_1_BIT, + std::function m_on_check_for_updates = []() {}); /// Compile the rendergraph void compile(); diff --git a/include/inexor/vulkan-renderer/render-graph/texture.hpp b/include/inexor/vulkan-renderer/render-graph/texture.hpp index 93f9e9543..9530e9a5e 100644 --- a/include/inexor/vulkan-renderer/render-graph/texture.hpp +++ b/include/inexor/vulkan-renderer/render-graph/texture.hpp @@ -81,10 +81,8 @@ class Texture { void *m_src_texture_data{nullptr}; std::size_t m_src_texture_data_size{0}; - /// This is for initialization of textures which are of TextureUsage::NORMAL - std::optional> m_on_init{std::nullopt}; /// By definition, if this is not std::nullopt, this is a dynamic texture - std::optional> m_on_check_for_updates{std::nullopt}; + std::function m_on_check_for_updates; // The staging buffer for updating the texture data VkBuffer m_staging_buffer{VK_NULL_HANDLE}; @@ -117,10 +115,7 @@ class Texture { /// @param width The width of the texture /// @param height The height of the texture /// @param sample_count The sample count of the texture - /// @param on_init The initialization function of the texture - /// @note If ``on_init`` is ``std::nullopt``, it means the texture will be initialized internally in rendergraph. /// @param on_check_for_updates The update function of the texture - /// @note If ``on_check_for_updates`` is ``std::nullopt``, the texture will not be updated at all! Texture(const Device &device, std::string name, TextureUsage usage, @@ -128,8 +123,7 @@ class Texture { std::uint32_t width, std::uint32_t height, VkSampleCountFlagBits sample_count, // TODO: Channel count as well? - std::optional> on_init = std::nullopt, - std::optional> on_check_for_updates = std::nullopt); + std::function on_check_for_updates); Texture(const Texture &) = delete; Texture(Texture &&other) noexcept; diff --git a/include/inexor/vulkan-renderer/renderers/imgui.hpp b/include/inexor/vulkan-renderer/renderers/imgui.hpp index 98ca13648..6a778a70e 100644 --- a/include/inexor/vulkan-renderer/renderers/imgui.hpp +++ b/include/inexor/vulkan-renderer/renderers/imgui.hpp @@ -68,6 +68,7 @@ class ImGuiRenderer { int m_font_texture_width{0}; int m_font_texture_height{0}; int m_font_texture_data_size{0}; + bool m_font_texture_initialized{false}; // Neither scale nor translation change struct PushConstBlock { diff --git a/src/vulkan-renderer/application.cpp b/src/vulkan-renderer/application.cpp index 3c1b66013..61cdfa3b3 100644 --- a/src/vulkan-renderer/application.cpp +++ b/src/vulkan-renderer/application.cpp @@ -553,9 +553,8 @@ void Application::setup_render_graph() { // TODO: We don't need to recreate the imgui overlay when swapchain is recreated, use a .recreate() method instead? // TODO: Decouple ImGuiRenderer form ImGuiLoader - // m_imgui_overlay = std::make_unique(*m_device, m_render_graph, m_octree_pass, - // m_swapchain, - // [&]() { update_imgui_overlay(); }); + m_imgui_overlay = std::make_unique(*m_device, m_render_graph, m_octree_pass, m_swapchain, + [&]() { update_imgui_overlay(); }); m_render_graph->compile(); } diff --git a/src/vulkan-renderer/render-graph/buffer.cpp b/src/vulkan-renderer/render-graph/buffer.cpp index f730373f0..7d937948a 100644 --- a/src/vulkan-renderer/render-graph/buffer.cpp +++ b/src/vulkan-renderer/render-graph/buffer.cpp @@ -141,12 +141,14 @@ void Buffer::create(const CommandBuffer &cmd_buf) { .copy_buffer(m_staging_buffer, m_buffer, m_src_data_size) .pipeline_buffer_memory_barrier_after_copy_buffer(m_buffer); } + // Update the descriptor buffer info m_descriptor_buffer_info = VkDescriptorBufferInfo{ .buffer = m_buffer, .offset = 0, .range = m_alloc_info.size, }; + // The update is finished m_update_requested = false; m_src_data = nullptr; @@ -154,7 +156,6 @@ void Buffer::create(const CommandBuffer &cmd_buf) { // NOTE: The staging buffer needs to stay valid until command buffer finished executing! // It will be destroyed either in the destructor or the next time create is called. - // NOTE: Another option would have been to wrap each call to create() into its own single time command buffer, which // would increase the total number of command buffer submissions though. } diff --git a/src/vulkan-renderer/render-graph/render_graph.cpp b/src/vulkan-renderer/render-graph/render_graph.cpp index 4c0d0afbf..92a9cf1e4 100644 --- a/src/vulkan-renderer/render-graph/render_graph.cpp +++ b/src/vulkan-renderer/render-graph/render_graph.cpp @@ -49,10 +49,9 @@ std::weak_ptr RenderGraph::add_texture(std::string texture_name, const std::uint32_t width, const std::uint32_t height, const VkSampleCountFlagBits sample_count, - std::optional> on_init, - std::optional> on_update) { + std::function m_on_check_for_updates) { m_textures.emplace_back(std::make_shared(m_device, std::move(texture_name), usage, format, width, height, - sample_count, std::move(on_init), std::move(on_update))); + sample_count, std::move(m_on_check_for_updates))); return m_textures.back(); } @@ -106,7 +105,6 @@ void RenderGraph::compile() { validate_render_graph(); determine_pass_order(); update_buffers(); - initialize_textures(); update_textures(); create_descriptor_set_layouts(); allocate_descriptor_sets(); @@ -149,8 +147,8 @@ void RenderGraph::determine_pass_order() { void RenderGraph::fill_graphics_pass_rendering_info(GraphicsPass &pass) { pass.reset_rendering_info(); - /// - /// + /// Fill the VkRenderingattachmentInfo for a color, depth, or stencil attachment + /// @param write_attachment The attachment this graphics pass writes to /// auto fill_rendering_info_for_attachment = [&](const std::weak_ptr &write_attachment, const std::optional &clear_value) { @@ -172,6 +170,7 @@ void RenderGraph::fill_graphics_pass_rendering_info(GraphicsPass &pass) { // TODO: Support MSAA again! return wrapper::make_info({ + // TODO: Implement m_current_img_view when double/triple buffering and do this on init, not per-frame? .imageView = attachment->m_img->m_img_view, .imageLayout = get_image_layout(), .resolveMode = VK_RESOLVE_MODE_NONE, @@ -209,13 +208,14 @@ void RenderGraph::fill_graphics_pass_rendering_info(GraphicsPass &pass) { } } - /// - /// - /// + /// Fill the VkRenderingAttachmentInfo for a swapchain + /// @param write_swapchain The swapchain to which this graphics pass writes to + /// @param clear_value The optional clear value for the swapchain image auto fill_write_info_for_swapchain = [&](const std::weak_ptr &write_swapchain, const std::optional &clear_value) { // TODO: Support MSAA again! return wrapper::make_info({ + // TODO: Does this mean we can do this on init now? Not on a per-frame basis? .imageView = write_swapchain.lock()->m_current_swapchain_img_view, .imageLayout = VK_IMAGE_LAYOUT_COLOR_ATTACHMENT_OPTIMAL, .resolveMode = VK_RESOLVE_MODE_NONE, @@ -250,37 +250,6 @@ void RenderGraph::fill_graphics_pass_rendering_info(GraphicsPass &pass) { }); } -// TODO: Consistent naming: create, initialize, update=create for static textures...? - -void RenderGraph::initialize_textures() { - // Check if there is any update required - bool any_update_required = false; - for (const auto &texture : m_textures) { - // NOTE: For textures which are created internally by rendergraph, m_on_init is std::nullopt - if (texture->m_on_init) { - std::invoke(texture->m_on_init.value()); - if (texture->m_update_requested) { - any_update_required = true; - } - } - } - // Only start recording and submitting a command buffer on transfer queue if any update is required - if (any_update_required) { - m_device.execute("[RenderGraph::initialize_textures]", VK_QUEUE_TRANSFER_BIT, DebugLabelColor::LIME, - [&](const CommandBuffer &cmd_buf) { - for (const auto &texture : m_textures) { - if (texture->m_update_requested) { - cmd_buf.set_suboperation_debug_name("[Texture|Create:" + texture->m_name + "]"); - texture->create(); - texture->update(cmd_buf); - } - } - }); - } - // NOTE: For the "else" case: We can't insert a debug label here telling us that there are no buffer updates - // required because that command itself would require a command buffer to be in recording state -} - void RenderGraph::record_command_buffer_for_pass(const CommandBuffer &cmd_buf, GraphicsPass &pass) { cmd_buf.set_suboperation_debug_name("[Pass:" + pass.m_name + "]"); // Start a new debug label for this graphics pass (visible in graphics debuggers like RenderDoc) @@ -337,6 +306,7 @@ void RenderGraph::record_command_buffer_for_pass(const CommandBuffer &cmd_buf, G void RenderGraph::render() { update_buffers(); update_textures(); + // TODO: Optimize this: Only call if any data changed and try to accumulate write descriptor sets update_write_descriptor_sets(); m_device.execute( @@ -365,7 +335,7 @@ void RenderGraph::update_buffers() { } // Only start recording and submitting a command buffer on transfer queue if any update is required if (any_update_required) { - m_device.execute("[RenderGraph::update_buffers]", VK_QUEUE_TRANSFER_BIT, DebugLabelColor::MAGENTA, + m_device.execute("[RenderGraph::update_buffers]", VK_QUEUE_GRAPHICS_BIT, DebugLabelColor::MAGENTA, [&](const CommandBuffer &cmd_buf) { for (const auto &buffer : m_buffers) { if (buffer->m_update_requested) { @@ -385,26 +355,27 @@ void RenderGraph::update_textures() { // Check if there is any update required bool any_update_required = false; for (const auto &texture : m_textures) { - // Only for dynamic textures m_on_lambda which is not std::nullopt - if (texture->m_on_check_for_updates) { - std::invoke(texture->m_on_check_for_updates.value()); - if (texture->m_update_requested) { - any_update_required = true; - } + // Check if this texture needs an update + if (texture->m_usage == TextureUsage::NORMAL) { + texture->m_on_check_for_updates(); + } else { + texture->m_update_requested = true; + } + if (texture->m_update_requested) { + any_update_required = true; } } // Only start recording and submitting a command buffer on transfer queue if any update is required if (any_update_required) { - m_device.execute("[RenderGraph::update_textures]", VK_QUEUE_TRANSFER_BIT, DebugLabelColor::LIME, + m_device.execute("[RenderGraph::update_textures]", VK_QUEUE_GRAPHICS_BIT, DebugLabelColor::LIME, [&](const CommandBuffer &cmd_buf) { for (const auto &texture : m_textures) { if (texture->m_update_requested) { cmd_buf.set_suboperation_debug_name("[Texture|Destroy:" + texture->m_name + "]"); texture->destroy(); - cmd_buf.set_suboperation_debug_name("[Texture|Update:" + texture->m_name + "]"); - std::invoke(texture->m_on_check_for_updates.value()); cmd_buf.set_suboperation_debug_name("[Texture|Create:" + texture->m_name + "]"); texture->create(); + texture->update(cmd_buf); } } }); diff --git a/src/vulkan-renderer/render-graph/texture.cpp b/src/vulkan-renderer/render-graph/texture.cpp index 775543d17..fe0cbc0d0 100644 --- a/src/vulkan-renderer/render-graph/texture.cpp +++ b/src/vulkan-renderer/render-graph/texture.cpp @@ -18,11 +18,9 @@ Texture::Texture(const Device &device, const std::uint32_t width, const std::uint32_t height, const VkSampleCountFlagBits sample_count, - std::optional> on_init, - std::optional> on_check_for_updates) + std::function on_check_for_updates) : m_device(device), m_name(std::move(name)), m_usage(usage), m_format(format), m_width(width), m_height(height), - m_sample_count(sample_count), m_on_init(std::move(on_init)), - m_on_check_for_updates(std::move(on_check_for_updates)) { + m_sample_count(sample_count), m_on_check_for_updates(std::move(on_check_for_updates)) { if (m_name.empty()) { throw std::invalid_argument("[Texture::Texture] Error: Parameter 'name' is empty!"); } @@ -75,7 +73,7 @@ void Texture::create() { }); const auto img_view_ci = wrapper::make_info({ - // NOTE: .image will be filled by the Texture wrapper + // NOTE: .image will be filled by the Image wrapper .viewType = VK_IMAGE_VIEW_TYPE_2D, .format = m_format, .subresourceRange = @@ -159,6 +157,8 @@ void Texture::update(const CommandBuffer &cmd_buf) { cmd_buf.insert_debug_label("[Texture::update|" + m_name + "]", wrapper::get_debug_label_color(wrapper::DebugLabelColor::ORANGE)); + + // TODO: Check on which queue the udpate is carried out and adjust the stages in the pipeline barrier accordingly cmd_buf.pipeline_image_memory_barrier_before_copy_buffer_to_image(m_img->m_img) .copy_buffer_to_image(m_staging_buffer, m_img->m_img, { @@ -169,6 +169,7 @@ void Texture::update(const CommandBuffer &cmd_buf) { .pipeline_image_memory_barrier_after_copy_buffer_to_image(m_img->m_img); // Update the descriptor image info + // TODO: Does this mean we can this in create() function, not on a per-frame basis? m_descriptor_img_info = VkDescriptorImageInfo{ .sampler = m_img->m_sampler->m_sampler, .imageView = m_img->m_img_view, @@ -182,7 +183,6 @@ void Texture::update(const CommandBuffer &cmd_buf) { // NOTE: The staging buffer needs to stay valid until command buffer finished executing! // It will be destroyed either in the destructor or the next time execute_update is called. - // NOTE: Another option would have been to wrap each call to execute_update() into its own single time // command buffer, which would increase the total number of command buffer submissions though. } diff --git a/src/vulkan-renderer/renderers/imgui.cpp b/src/vulkan-renderer/renderers/imgui.cpp index fa6f28f8c..dc75cef10 100644 --- a/src/vulkan-renderer/renderers/imgui.cpp +++ b/src/vulkan-renderer/renderers/imgui.cpp @@ -78,12 +78,15 @@ ImGuiRenderer::ImGuiRenderer(const Device &device, m_fragment_shader = std::make_shared(device, "ImGui", VK_SHADER_STAGE_FRAGMENT_BIT, "shaders/ui.frag.spv"); - m_imgui_texture = - graph->add_texture("ImGui-Font", render_graph::TextureUsage::NORMAL, VK_FORMAT_R8G8B8A8_UNORM, - m_font_texture_width, m_font_texture_width, VK_SAMPLE_COUNT_1_BIT, [&]() { - // Initialize the ImGui font texture - m_imgui_texture.lock()->request_update(m_font_texture_data, m_font_texture_data_size); - }); + m_imgui_texture = graph->add_texture("ImGui-Font", render_graph::TextureUsage::NORMAL, VK_FORMAT_R8G8B8A8_UNORM, + m_font_texture_width, m_font_texture_width, VK_SAMPLE_COUNT_1_BIT, [&]() { + // Initialize the ImGui font texture only once in the update function + if (!m_font_texture_initialized) { + m_imgui_texture.lock()->request_update(m_font_texture_data, + m_font_texture_data_size); + m_font_texture_initialized = true; + } + }); graph->add_resource_descriptor( [&](wrapper::descriptors::DescriptorSetLayoutBuilder &builder) {