From 2a1b4bf2196eaab3d29eff73a705d26039001331 Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Tue, 23 Apr 2024 14:49:41 +0200 Subject: [PATCH] Vulkan: Move push constants to VKShader VKPipeline class is deprecated and will be phased out in the near future. This PR moves the push constants to VKShader as it was wrongly placed in the pipeline. Pull Request: https://projects.blender.org/blender/blender/pulls/120980 --- source/blender/gpu/vulkan/vk_context.cc | 4 ++-- source/blender/gpu/vulkan/vk_pipeline.cc | 28 +++++------------------- source/blender/gpu/vulkan/vk_pipeline.hh | 16 +++----------- source/blender/gpu/vulkan/vk_shader.cc | 9 ++++---- source/blender/gpu/vulkan/vk_shader.hh | 2 ++ 5 files changed, 17 insertions(+), 42 deletions(-) diff --git a/source/blender/gpu/vulkan/vk_context.cc b/source/blender/gpu/vulkan/vk_context.cc index 0daf5a581bf..97f624ba8f9 100644 --- a/source/blender/gpu/vulkan/vk_context.cc +++ b/source/blender/gpu/vulkan/vk_context.cc @@ -207,7 +207,7 @@ void VKContext::bind_compute_pipeline() BLI_assert(shader); VKPipeline &pipeline = shader->pipeline_get(); pipeline.bind(*this, VK_PIPELINE_BIND_POINT_COMPUTE); - pipeline.update_push_constants(*this); + shader->push_constants.update(*this); if (shader->has_descriptor_set()) { descriptor_set_.bind(*this, shader->vk_pipeline_layout_get(), VK_PIPELINE_BIND_POINT_COMPUTE); } @@ -234,7 +234,7 @@ void VKContext::bind_graphics_pipeline(const GPUPrimType prim_type, VKPipeline &pipeline = shader->pipeline_get(); pipeline.bind(*this, VK_PIPELINE_BIND_POINT_GRAPHICS); - pipeline.update_push_constants(*this); + shader->push_constants.update(*this); if (shader->has_descriptor_set()) { descriptor_set_.bind(*this, shader->vk_pipeline_layout_get(), VK_PIPELINE_BIND_POINT_GRAPHICS); } diff --git a/source/blender/gpu/vulkan/vk_pipeline.cc b/source/blender/gpu/vulkan/vk_pipeline.cc index efe566a94d3..1653b5dd10d 100644 --- a/source/blender/gpu/vulkan/vk_pipeline.cc +++ b/source/blender/gpu/vulkan/vk_pipeline.cc @@ -17,13 +17,7 @@ namespace blender::gpu { -VKPipeline::VKPipeline(VKPushConstants &&push_constants) - : active_vk_pipeline_(VK_NULL_HANDLE), push_constants_(std::move(push_constants)) -{ -} - -VKPipeline::VKPipeline(VkPipeline vk_pipeline, VKPushConstants &&push_constants) - : active_vk_pipeline_(vk_pipeline), push_constants_(std::move(push_constants)) +VKPipeline::VKPipeline(VkPipeline vk_pipeline) : active_vk_pipeline_(vk_pipeline) { vk_pipelines_.append(vk_pipeline); } @@ -37,10 +31,8 @@ VKPipeline::~VKPipeline() } } -VKPipeline VKPipeline::create_compute_pipeline( - VkShaderModule compute_module, - VkPipelineLayout &pipeline_layout, - const VKPushConstants::Layout &push_constants_layout) +VKPipeline VKPipeline::create_compute_pipeline(VkShaderModule compute_module, + VkPipelineLayout &pipeline_layout) { VK_ALLOCATION_CALLBACKS const VKDevice &device = VKBackend::get().device_get(); @@ -66,15 +58,12 @@ VKPipeline VKPipeline::create_compute_pipeline( return VKPipeline(); } - VKPushConstants push_constants(&push_constants_layout); - return VKPipeline(vk_pipeline, std::move(push_constants)); + return VKPipeline(vk_pipeline); } -VKPipeline VKPipeline::create_graphics_pipeline( - const VKPushConstants::Layout &push_constants_layout) +VKPipeline VKPipeline::create_graphics_pipeline() { - VKPushConstants push_constants(&push_constants_layout); - return VKPipeline(std::move(push_constants)); + return VKPipeline(); } VkPipeline VKPipeline::vk_handle() const @@ -200,9 +189,4 @@ void VKPipeline::bind(VKContext &context, VkPipelineBindPoint vk_pipeline_bind_p command_buffers.bind(*this, vk_pipeline_bind_point); } -void VKPipeline::update_push_constants(VKContext &context) -{ - push_constants_.update(context); -} - } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_pipeline.hh b/source/blender/gpu/vulkan/vk_pipeline.hh index 83e52bb6859..06e6cd9befe 100644 --- a/source/blender/gpu/vulkan/vk_pipeline.hh +++ b/source/blender/gpu/vulkan/vk_pipeline.hh @@ -38,34 +38,25 @@ class VKPipeline : NonCopyable { VkPipeline active_vk_pipeline_ = VK_NULL_HANDLE; /** Keep track of all pipelines as they can still be in flight. */ Vector vk_pipelines_; - VKPushConstants push_constants_; VKPipelineStateManager state_manager_; public: VKPipeline() = default; virtual ~VKPipeline(); - VKPipeline(VKPushConstants &&push_constants); - VKPipeline(VkPipeline vk_pipeline, VKPushConstants &&push_constants); + VKPipeline(VkPipeline vk_pipeline); VKPipeline &operator=(VKPipeline &&other) { active_vk_pipeline_ = other.active_vk_pipeline_; other.active_vk_pipeline_ = VK_NULL_HANDLE; - push_constants_ = std::move(other.push_constants_); vk_pipelines_ = std::move(other.vk_pipelines_); other.vk_pipelines_.clear(); return *this; } static VKPipeline create_compute_pipeline(VkShaderModule compute_module, - VkPipelineLayout &pipeline_layouts, - const VKPushConstants::Layout &push_constants_layout); - static VKPipeline create_graphics_pipeline(const VKPushConstants::Layout &push_constants_layout); - - VKPushConstants &push_constants_get() - { - return push_constants_; - } + VkPipelineLayout &pipeline_layouts); + static VKPipeline create_graphics_pipeline(); VKPipelineStateManager &state_manager_get() { @@ -84,7 +75,6 @@ class VKPipeline : NonCopyable { const VKVertexAttributeObject &vertex_attribute_object); void bind(VKContext &context, VkPipelineBindPoint vk_pipeline_bind_point); - void update_push_constants(VKContext &context); }; } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_shader.cc b/source/blender/gpu/vulkan/vk_shader.cc index 0d5df2553f1..2b4adc24754 100644 --- a/source/blender/gpu/vulkan/vk_shader.cc +++ b/source/blender/gpu/vulkan/vk_shader.cc @@ -662,6 +662,8 @@ bool VKShader::finalize(const shader::ShaderCreateInfo *info) return false; } + push_constants = VKPushConstants(&vk_interface->push_constants_layout_get()); + /* TODO we might need to move the actual pipeline construction to a later stage as the graphics * pipeline requires more data before it can be constructed. */ bool result; @@ -669,7 +671,7 @@ bool VKShader::finalize(const shader::ShaderCreateInfo *info) BLI_assert((fragment_module_ != VK_NULL_HANDLE && info->tf_type_ == GPU_SHADER_TFB_NONE) || (fragment_module_ == VK_NULL_HANDLE && info->tf_type_ != GPU_SHADER_TFB_NONE)); BLI_assert(compute_module_ == VK_NULL_HANDLE); - pipeline_ = VKPipeline::create_graphics_pipeline(vk_interface->push_constants_layout_get()); + pipeline_ = VKPipeline::create_graphics_pipeline(); result = true; } else { @@ -677,8 +679,7 @@ bool VKShader::finalize(const shader::ShaderCreateInfo *info) BLI_assert(geometry_module_ == VK_NULL_HANDLE); BLI_assert(fragment_module_ == VK_NULL_HANDLE); BLI_assert(compute_module_ != VK_NULL_HANDLE); - pipeline_ = VKPipeline::create_compute_pipeline( - compute_module_, vk_pipeline_layout_, vk_interface->push_constants_layout_get()); + pipeline_ = VKPipeline::create_compute_pipeline(compute_module_, vk_pipeline_layout_); result = pipeline_.is_valid(); } @@ -784,13 +785,11 @@ void VKShader::unbind() {} void VKShader::uniform_float(int location, int comp_len, int array_size, const float *data) { - VKPushConstants &push_constants = pipeline_get().push_constants_get(); push_constants.push_constant_set(location, comp_len, array_size, data); } void VKShader::uniform_int(int location, int comp_len, int array_size, const int *data) { - VKPushConstants &push_constants = pipeline_get().push_constants_get(); push_constants.push_constant_set(location, comp_len, array_size, data); } diff --git a/source/blender/gpu/vulkan/vk_shader.hh b/source/blender/gpu/vulkan/vk_shader.hh index 3f6d6cdd1da..f2da07d24e5 100644 --- a/source/blender/gpu/vulkan/vk_shader.hh +++ b/source/blender/gpu/vulkan/vk_shader.hh @@ -37,6 +37,8 @@ class VKShader : public Shader { VKPipeline pipeline_; public: + VKPushConstants push_constants; + VKShader(const char *name); virtual ~VKShader();