[*] Work in progress but broken

Author: IAmNotHanni

include/inexor/vulkan-renderer/wrapper/pipelines/graphics_pipeline.hpp

@@ -34,6 +34,47 @@ namespace inexor::vulkan_renderer::wrapper::pipelines {
 
 // TODO: Implement RAII wrapper for ComputePipeline
 
+/// When creating a graphics pipeline, the lifetime of certain data which is used to create the pipeline must be
+/// ensured. In particular, the VkGraphicsPipelineCreateInfo struct must not be stored, however, the memory to which the
+/// pointers inside of VkGraphicsPipelineCreateInfo point to must be stored. For example, VkGraphicsPipelineCreateInfo
+/// has a member VkPipelineViewportStateCreateInfo, which itself has a pointer that point to VkViewport data, for
+/// example. This means we must make sure the lifetime of all data that the pointers point to must be preserved.
+/// Initially, we collected all the data to create the graphics pipeline in GraphicsPipelineBuilder, and reset all the
+/// data of the builder after the build() method has been called. However, this is wrong, because the lifetime of the
+/// data ends with calling reset(). This causes some bugs which are hard to find.
+struct GraphicsPipelineSetupData {
+    // @TODO Add whatever we need here!
+    // This is the underlying data for the create info structures
+    std::vector<VkPipelineShaderStageCreateInfo> m_shader_stages;
+    std::vector<VkVertexInputBindingDescription> m_vertex_input_binding_descriptions;
+    std::vector<VkVertexInputAttributeDescription> m_vertex_input_attribute_descriptions;
+    std::vector<VkPipelineColorBlendAttachmentState> m_color_blend_attachment_states;
+    std::vector<VkViewport> m_viewports;
+    std::vector<VkRect2D> m_scissors;
+    std::vector<VkPushConstantRange> m_push_constant_ranges;
+    std::vector<VkDescriptorSetLayout> m_descriptor_set_layouts;
+    VkRenderPass m_render_pass;
+    VkFormat m_depth_attachment_format;
+    VkFormat m_stencil_attachment_format;
+    std::vector<VkFormat> m_color_attachments;
+    std::vector<VkDynamicState> m_dynamic_states;
+    VkPipelineLayout m_pipeline_layout;
+
+    // These are the create info structures required to fill the VkGraphicsPipelineCreateInfo
+    VkPipelineVertexInputStateCreateInfo m_vertex_input_sci;
+    VkPipelineInputAssemblyStateCreateInfo m_input_assembly_sci;
+    VkPipelineTessellationStateCreateInfo m_tesselation_sci;
+    VkPipelineViewportStateCreateInfo m_viewport_sci;
+    VkPipelineRasterizationStateCreateInfo m_rasterization_sci;
+    VkPipelineDepthStencilStateCreateInfo m_depth_stencil_sci;
+    VkPipelineRenderingCreateInfo m_pipeline_rendering_ci;
+    VkPipelineMultisampleStateCreateInfo m_multisample_sci;
+    VkPipelineColorBlendStateCreateInfo m_color_blend_sci;
+    VkPipelineDynamicStateCreateInfo m_dynamic_states_sci;
+
+    // @TODO: Implement move constructor for GraphicsPipelineSetupData?
+};
+
 /// RAII wrapper for graphics pipelines
 class GraphicsPipeline {
     friend class commands::CommandBuffer;
@@ -42,6 +83,7 @@ class GraphicsPipeline {
 private:
     const Device &m_device;
     std::string m_name;
+    GraphicsPipelineSetupData m_pipeline_setup_data;
 
     VkPipeline m_pipeline;
     std::unique_ptr<PipelineLayout> m_pipeline_layout;
@@ -52,12 +94,12 @@ class GraphicsPipeline {
     /// @param pipeline_cache The Vulkan pipeline cache
     /// @param descriptor_set_layouts The descriptor set layouts in the pipeline layout
     /// @param push_constant_ranges The push constant ranges in the pipeline layout
-    /// @param pipeline_ci The pipeline create info
+    /// @param pipeline_setup_data The graphics pipeline setup data
     /// @param name The internal debug name of the graphics pipeline
     GraphicsPipeline(const Device &device, const PipelineCache &pipeline_cache,
                      std::span<const VkDescriptorSetLayout> descriptor_set_layouts,
                      std::span<const VkPushConstantRange> push_constant_ranges,
-                     VkGraphicsPipelineCreateInfo pipeline_ci, std::string name);
+                     GraphicsPipelineSetupData pipeline_setup_data, std::string name);
 
     GraphicsPipeline(const GraphicsPipeline &) = delete;
     GraphicsPipeline(GraphicsPipeline &&) noexcept;

include/inexor/vulkan-renderer/wrapper/pipelines/graphics_pipeline_builder.hpp

@@ -40,78 +40,14 @@ class PipelineCache;
 /// to implement checks in case required fields are not set in this builder.
 class GraphicsPipelineBuilder {
 private:
-    /// The device wrapper reference
+    /// The device wrapper
     const Device &m_device;
-
     /// The Vulkan pipeline cache
     const PipelineCache &m_pipeline_cache;
-
-    // We are not using member initializers here:
-    // Note that all members are initialized in the reset() method
-    // This method is also called after the graphics pipeline has been created,
-    // allowing one instance of GraphicsPipelineBuilder to be reused
-
-    // With the builder we can either call add_shader or set_shaders
-    std::vector<VkPipelineShaderStageCreateInfo> m_shader_stages;
-
-    std::vector<VkVertexInputBindingDescription> m_vertex_input_binding_descriptions;
-    std::vector<VkVertexInputAttributeDescription> m_vertex_input_attribute_descriptions;
-    // With the builder we can fill vertex binding descriptions and vertex attribute descriptions in here
-    VkPipelineVertexInputStateCreateInfo m_vertex_input_sci;
-
-    // With the builder we can set topology in here
-    VkPipelineInputAssemblyStateCreateInfo m_input_assembly_sci;
-
-    // With the builder we can set the patch control point count in here
-    VkPipelineTessellationStateCreateInfo m_tesselation_sci;
-
-    std::vector<VkViewport> m_viewports;
-    std::vector<VkRect2D> m_scissors;
-    // With the builder we can set viewport(s) and scissor(s) in here
-    VkPipelineViewportStateCreateInfo m_viewport_sci;
-
-    // With the builder we can set polygon mode, cull mode, front face, and line width
-    // TODO: Implement methods to enable depth bias and for setting the depth bias parameters
-    VkPipelineRasterizationStateCreateInfo m_rasterization_sci;
-
-    // With the builder we can't set individial fields of this struct,
-    // since it's easier to specify an entire VkPipelineDepthStencilStateCreateInfo struct to the builder instead
-    VkPipelineDepthStencilStateCreateInfo m_depth_stencil_sci;
-
-    VkRenderPass m_render_pass;
-
-    /// This is used for dynamic rendering
-    VkFormat m_depth_attachment_format;
-    VkFormat m_stencil_attachment_format;
-    std::vector<VkFormat> m_color_attachments;
-
-    VkPipelineRenderingCreateInfo m_pipeline_rendering_ci;
-
-    // With the builder we can set rasterization samples and min sample shading
-    // TODO: Expose more multisampling parameters if desired
-    VkPipelineMultisampleStateCreateInfo m_multisample_sci;
-
-    // With the builder we can't set individial fields of this struct,
-    // since it's easier to specify an entire VkPipelineColorBlendStateCreateInfo struct to the builder instead
-    VkPipelineColorBlendStateCreateInfo m_color_blend_sci;
-
-    std::vector<VkDynamicState> m_dynamic_states;
-    // This will be filled in the build() method
-    VkPipelineDynamicStateCreateInfo m_dynamic_states_sci;
-
-    /// The layout of the graphics pipeline
-    VkPipelineLayout m_pipeline_layout;
-
-    // With the builder we can either call add_color_blend_attachment or set_color_blend_attachments
-    std::vector<VkPipelineColorBlendAttachmentState> m_color_blend_attachment_states;
-
-    /// The push constant ranges of the graphics pass
-    std::vector<VkPushConstantRange> m_push_constant_ranges;
-
-    std::vector<VkDescriptorSetLayout> m_descriptor_set_layouts;
+    /// The graphics pipeline setup data which will be std::moved into GraphicsPipeline wrapper
+    GraphicsPipelineSetupData m_graphics_pipeline_setup_data;
 
     /// Reset all data in this class so the builder can be re-used
-    /// @note This is called by the constructor
     void reset();
 
 public:

src/vulkan-renderer/wrapper/pipelines/graphics_pipeline.cpp

@@ -14,17 +14,16 @@ namespace inexor::vulkan_renderer::wrapper::pipelines {
 GraphicsPipeline::GraphicsPipeline(const Device &device, const PipelineCache &pipeline_cache,
                                    std::span<const VkDescriptorSetLayout> descriptor_set_layouts,
                                    std::span<const VkPushConstantRange> push_constant_ranges,
-                                   VkGraphicsPipelineCreateInfo pipeline_ci, std::string name)
-    : m_device(device), m_name(std::move(name)) {
+                                   GraphicsPipelineSetupData pipeline_setup_data, std::string name)
+    : m_device(device), m_pipeline_setup_data(std::move(pipeline_setup_data)), m_name(std::move(name)) {
 
-    // @TODO Should the pipeline layout really be part of graphics pipeline wrapper?
-
-    // Create the graphics pipeline layout
+    // @TODO Expose the pipeline layout as parameter
     m_pipeline_layout = std::make_unique<PipelineLayout>(m_device, m_name, std::move(descriptor_set_layouts),
                                                          std::move(push_constant_ranges));
 
-    // Set the pipeline layout in the pipeline create info struct
-    pipeline_ci.layout = m_pipeline_layout->m_pipeline_layout;
+    const auto pipeline_ci = make_info<VkGraphicsPipelineCreateInfo>({
+        .layout = m_pipeline_layout->pipeline_layout(),
+    });
 
     if (const auto result = vkCreateGraphicsPipelines(m_device.device(), pipeline_cache.m_pipeline_cache, 1,
                                                       &pipeline_ci, nullptr, &m_pipeline);

src/vulkan-renderer/wrapper/pipelines/graphics_pipeline_builder.cpp

@@ -17,28 +17,8 @@ GraphicsPipelineBuilder::GraphicsPipelineBuilder(const Device &device, const Pip
 
 GraphicsPipelineBuilder::GraphicsPipelineBuilder(GraphicsPipelineBuilder &&other) noexcept
     : m_device(other.m_device), m_pipeline_cache(other.m_pipeline_cache) {
-    // TODO: Check me!
-    m_pipeline_rendering_ci = std::move(other.m_pipeline_rendering_ci);
-    m_color_attachments = std::move(other.m_color_attachments);
-    m_depth_attachment_format = other.m_depth_attachment_format;
-    m_stencil_attachment_format = other.m_stencil_attachment_format;
-    m_shader_stages = std::move(other.m_shader_stages);
-    m_vertex_input_binding_descriptions = std::move(other.m_vertex_input_binding_descriptions);
-    m_vertex_input_attribute_descriptions = std::move(other.m_vertex_input_attribute_descriptions);
-    m_vertex_input_sci = std::move(other.m_vertex_input_sci);
-    m_input_assembly_sci = std::move(other.m_input_assembly_sci);
-    m_tesselation_sci = std::move(other.m_tesselation_sci);
-    m_viewport_sci = std::move(other.m_viewport_sci);
-    m_viewports = std::move(other.m_viewports);
-    m_scissors = std::move(other.m_scissors);
-    m_rasterization_sci = std::move(m_rasterization_sci);
-    m_multisample_sci = std::move(other.m_multisample_sci);
-    m_depth_stencil_sci = std::move(other.m_depth_stencil_sci);
-    m_color_blend_sci = std::move(other.m_color_blend_sci);
-    m_dynamic_states = std::move(other.m_dynamic_states);
-    m_dynamic_states_sci = std::move(other.m_dynamic_states_sci);
-    m_pipeline_layout = std::exchange(other.m_pipeline_layout, VK_NULL_HANDLE);
-    m_color_blend_attachment_states = std::move(other.m_color_blend_attachment_states);
+    // @TODO: Implement move constructor for GraphicsPipelineSetupData?
+    m_graphics_pipeline_setup_data = other.m_graphics_pipeline_setup_data;
 }
 
 std::shared_ptr<GraphicsPipeline> GraphicsPipelineBuilder::build(std::string name, bool use_dynamic_rendering) {