From 86b44f1202664807f70386a7fe9e18cf6703f64f Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Wed, 22 Feb 2023 14:33:47 +0100 Subject: [PATCH] Vulkan: Refactor shader interface. Descriptor set locations are now part of the vulkan shader interface. --- source/blender/gpu/tests/gpu_shader_test.cc | 2 +- .../blender/gpu/vulkan/vk_descriptor_set.cc | 4 ++ .../blender/gpu/vulkan/vk_descriptor_set.hh | 13 ++-- source/blender/gpu/vulkan/vk_index_buffer.cc | 4 +- source/blender/gpu/vulkan/vk_shader.cc | 31 +++------- .../blender/gpu/vulkan/vk_shader_interface.cc | 61 +++++++++++++++---- .../blender/gpu/vulkan/vk_shader_interface.hh | 16 +++++ .../blender/gpu/vulkan/vk_storage_buffer.cc | 4 +- source/blender/gpu/vulkan/vk_texture.cc | 5 +- source/blender/gpu/vulkan/vk_vertex_buffer.cc | 4 +- 10 files changed, 97 insertions(+), 47 deletions(-) diff --git a/source/blender/gpu/tests/gpu_shader_test.cc b/source/blender/gpu/tests/gpu_shader_test.cc index cbbee0192a1..f5d3981d97f 100644 --- a/source/blender/gpu/tests/gpu_shader_test.cc +++ b/source/blender/gpu/tests/gpu_shader_test.cc @@ -225,7 +225,7 @@ static void test_gpu_shader_compute_ssbo() EXPECT_NE(shader, nullptr); GPU_shader_bind(shader); - /* Construct IBO. */ + /* Construct SSBO. */ GPUStorageBuf *ssbo = GPU_storagebuf_create_ex( SIZE * sizeof(uint32_t), nullptr, GPU_USAGE_DEVICE_ONLY, __func__); GPU_storagebuf_bind(ssbo, GPU_shader_get_ssbo_binding(shader, "data_out")); diff --git a/source/blender/gpu/vulkan/vk_descriptor_set.cc b/source/blender/gpu/vulkan/vk_descriptor_set.cc index e41c21dda54..579852088ca 100644 --- a/source/blender/gpu/vulkan/vk_descriptor_set.cc +++ b/source/blender/gpu/vulkan/vk_descriptor_set.cc @@ -119,6 +119,10 @@ void VKDescriptorSet::update(VkDevice vk_device) descriptor_writes.append(write_descriptor); } + BLI_assert_msg(image_infos.size() + buffer_infos.size() == descriptor_writes.size(), + "Not all changes have been converted to a write descriptor. Check " + "`Binding::is_buffer` and `Binding::is_image`."); + vkUpdateDescriptorSets( vk_device, descriptor_writes.size(), descriptor_writes.data(), 0, nullptr); diff --git a/source/blender/gpu/vulkan/vk_descriptor_set.hh b/source/blender/gpu/vulkan/vk_descriptor_set.hh index f16f939c98c..9e2fae1d703 100644 --- a/source/blender/gpu/vulkan/vk_descriptor_set.hh +++ b/source/blender/gpu/vulkan/vk_descriptor_set.hh @@ -15,10 +15,11 @@ #include "vk_common.hh" namespace blender::gpu { -class VKStorageBuffer; -class VKVertexBuffer; class VKIndexBuffer; +class VKShaderInterface; +class VKStorageBuffer; class VKTexture; +class VKVertexBuffer; /** * In vulkan shader resources (images and buffers) are grouped in descriptor sets. @@ -26,7 +27,7 @@ class VKTexture; * The resources inside a descriptor set can be updated and bound per set. * * Currently Blender only supports a single descriptor set per shader, but it is planned to be able - * to use 2 descriptor sets per shader. Only for each #blender::gpu::shader::Frequency. + * to use 2 descriptor sets per shader. One for each #blender::gpu::shader::Frequency. */ class VKDescriptorSet : NonCopyable { struct Binding; @@ -50,9 +51,12 @@ class VKDescriptorSet : NonCopyable { */ uint32_t binding; - Location() = default; + Location(uint32_t binding) : binding(binding) + { + } public: + Location() = default; Location(const ShaderInput *shader_input) : binding(shader_input->location) { } @@ -68,6 +72,7 @@ class VKDescriptorSet : NonCopyable { } friend struct Binding; + friend class VKShaderInterface; }; private: diff --git a/source/blender/gpu/vulkan/vk_index_buffer.cc b/source/blender/gpu/vulkan/vk_index_buffer.cc index 1d6e620a395..32b5659b8db 100644 --- a/source/blender/gpu/vulkan/vk_index_buffer.cc +++ b/source/blender/gpu/vulkan/vk_index_buffer.cc @@ -24,9 +24,9 @@ void VKIndexBuffer::bind_as_ssbo(uint binding) VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); - const ShaderInput *shader_input = shader_interface.shader_input_get( + const VKDescriptorSet::Location location = shader_interface.descriptor_set_location( shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, binding); - shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, shader_input); + shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, location); } void VKIndexBuffer::read(uint32_t *data) const diff --git a/source/blender/gpu/vulkan/vk_shader.cc b/source/blender/gpu/vulkan/vk_shader.cc index 5a721bf764f..977123efdda 100644 --- a/source/blender/gpu/vulkan/vk_shader.cc +++ b/source/blender/gpu/vulkan/vk_shader.cc @@ -326,10 +326,10 @@ static std::ostream &print_qualifier(std::ostream &os, const Qualifier &qualifie } static void print_resource(std::ostream &os, - const ShaderInput &shader_input, + const VKDescriptorSet::Location location, const ShaderCreateInfo::Resource &res) { - os << "layout(binding = " << shader_input.location; + os << "layout(binding = " << static_cast(location); if (res.bind_type == ShaderCreateInfo::Resource::BindType::IMAGE) { os << ", " << to_string(res.image.format); } @@ -379,12 +379,8 @@ static void print_resource(std::ostream &os, const VKShaderInterface &shader_interface, const ShaderCreateInfo::Resource &res) { - const ShaderInput *shader_input = shader_interface.shader_input_get(res); - if (shader_input == nullptr) { - BLI_assert_msg(shader_input, "Cannot find shader input for resource"); - return; - } - print_resource(os, *shader_input, res); + const VKDescriptorSet::Location location = shader_interface.descriptor_set_location(res); + print_resource(os, location, res); } static void print_resource_alias(std::ostream &os, const ShaderCreateInfo::Resource &res) @@ -860,10 +856,10 @@ static VkDescriptorType descriptor_type(const shader::ShaderCreateInfo::Resource } static VkDescriptorSetLayoutBinding create_descriptor_set_layout_binding( - const ShaderInput &shader_input, const shader::ShaderCreateInfo::Resource &resource) + const VKDescriptorSet::Location location, const shader::ShaderCreateInfo::Resource &resource) { VkDescriptorSetLayoutBinding binding = {}; - binding.binding = shader_input.location; + binding.binding = location; binding.descriptorType = descriptor_type(resource); binding.descriptorCount = 1; binding.stageFlags = VK_SHADER_STAGE_ALL; @@ -878,13 +874,8 @@ static void add_descriptor_set_layout_bindings( Vector &r_bindings) { for (const shader::ShaderCreateInfo::Resource &resource : resources) { - const ShaderInput *shader_input = interface.shader_input_get(resource); - if (shader_input == nullptr) { - BLI_assert_msg(shader_input, "Cannot find shader input for resource."); - continue; - } - - r_bindings.append(create_descriptor_set_layout_binding(*shader_input, resource)); + const VKDescriptorSet::Location location = interface.descriptor_set_location(resource); + r_bindings.append(create_descriptor_set_layout_binding(location, resource)); } } @@ -1033,12 +1024,6 @@ std::string VKShader::vertex_interface_declare(const shader::ShaderCreateInfo &i ss << "layout(location = " << attr.index << ") "; ss << "in " << to_string(attr.type) << " " << attr.name << ";\n"; } - /* NOTE(D4490): Fix a bug where shader without any vertex attributes do not behave correctly. - */ - if (GPU_type_matches_ex(GPU_DEVICE_APPLE, GPU_OS_MAC, GPU_DRIVER_ANY, GPU_BACKEND_OPENGL) && - info.vertex_inputs_.is_empty()) { - ss << "in float gpu_dummy_workaround;\n"; - } ss << "\n/* Interfaces. */\n"; int location = 0; for (const StageInterfaceInfo *iface : info.vertex_out_interfaces_) { diff --git a/source/blender/gpu/vulkan/vk_shader_interface.cc b/source/blender/gpu/vulkan/vk_shader_interface.cc index cec6213bc5b..50f6401103f 100644 --- a/source/blender/gpu/vulkan/vk_shader_interface.cc +++ b/source/blender/gpu/vulkan/vk_shader_interface.cc @@ -41,7 +41,7 @@ void VKShaderInterface::init(const shader::ShaderCreateInfo &info) } } /* Make sure that the image slots don't overlap with the sampler slots.*/ - image_offset_ += 1; + image_offset_++; int32_t input_tot_len = ubo_len_ + uniform_len_ + ssbo_len_; inputs_ = static_cast( @@ -51,14 +51,11 @@ void VKShaderInterface::init(const shader::ShaderCreateInfo &info) name_buffer_ = (char *)MEM_mallocN(info.interface_names_size_, "name_buffer"); uint32_t name_buffer_offset = 0; - int location = 0; - /* Uniform blocks */ for (const ShaderCreateInfo::Resource &res : all_resources) { if (res.bind_type == ShaderCreateInfo::Resource::BindType::UNIFORM_BUFFER) { copy_input_name(input, res.image.name, name_buffer_, name_buffer_offset); - input->location = location++; - input->binding = res.slot; + input->location = input->binding = res.slot; input++; } } @@ -67,14 +64,12 @@ void VKShaderInterface::init(const shader::ShaderCreateInfo &info) for (const ShaderCreateInfo::Resource &res : all_resources) { if (res.bind_type == ShaderCreateInfo::Resource::BindType::SAMPLER) { copy_input_name(input, res.sampler.name, name_buffer_, name_buffer_offset); - input->location = location++; - input->binding = res.slot; + input->location = input->binding = res.slot; input++; } else if (res.bind_type == ShaderCreateInfo::Resource::BindType::IMAGE) { copy_input_name(input, res.image.name, name_buffer_, name_buffer_offset); - input->location = location++; - input->binding = res.slot + image_offset_; + input->location = input->binding = res.slot + image_offset_; input++; } } @@ -83,13 +78,57 @@ void VKShaderInterface::init(const shader::ShaderCreateInfo &info) for (const ShaderCreateInfo::Resource &res : all_resources) { if (res.bind_type == ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER) { copy_input_name(input, res.storagebuf.name, name_buffer_, name_buffer_offset); - input->location = location++; - input->binding = res.slot; + input->location = input->binding = res.slot; input++; } } sort_inputs(); + + /* Determine the descriptor set locations after the inputs have been sorted.*/ + descriptor_set_locations_ = Array(input_tot_len); + uint32_t descriptor_set_location = 0; + for (ShaderCreateInfo::Resource &res : all_resources) { + const ShaderInput *input = shader_input_get(res); + descriptor_set_location_update(input, descriptor_set_location++); + } +} + +static int32_t shader_input_index(const ShaderInput *shader_inputs, + const ShaderInput *shader_input) +{ + int32_t index = (shader_input - shader_inputs); + return index; +} + +void VKShaderInterface::descriptor_set_location_update(const ShaderInput *shader_input, + const VKDescriptorSet::Location location) +{ + int32_t index = shader_input_index(inputs_, shader_input); + descriptor_set_locations_[index] = location; +} + +const VKDescriptorSet::Location VKShaderInterface::descriptor_set_location( + const ShaderInput *shader_input) const +{ + int32_t index = shader_input_index(inputs_, shader_input); + return descriptor_set_locations_[index]; +} + +const VKDescriptorSet::Location VKShaderInterface::descriptor_set_location( + const shader::ShaderCreateInfo::Resource &resource) const +{ + const ShaderInput *shader_input = shader_input_get(resource); + BLI_assert(shader_input); + return descriptor_set_location(shader_input); +} + +const VKDescriptorSet::Location VKShaderInterface::descriptor_set_location( + const shader::ShaderCreateInfo::Resource::BindType &bind_type, int binding) const +{ + const ShaderInput *shader_input = shader_input_get(bind_type, binding); + BLI_assert(shader_input); + return descriptor_set_location(shader_input); } const ShaderInput *VKShaderInterface::shader_input_get( diff --git a/source/blender/gpu/vulkan/vk_shader_interface.hh b/source/blender/gpu/vulkan/vk_shader_interface.hh index 4e099feb680..b931890d903 100644 --- a/source/blender/gpu/vulkan/vk_shader_interface.hh +++ b/source/blender/gpu/vulkan/vk_shader_interface.hh @@ -7,9 +7,13 @@ #pragma once +#include "BLI_array.hh" + #include "gpu_shader_create_info.hh" #include "gpu_shader_interface.hh" +#include "vk_descriptor_set.hh" + namespace blender::gpu { class VKShaderInterface : public ShaderInterface { private: @@ -21,11 +25,19 @@ class VKShaderInterface : public ShaderInterface { * overlapping. */ uint32_t image_offset_ = 0; + Array descriptor_set_locations_; public: VKShaderInterface() = default; void init(const shader::ShaderCreateInfo &info); + + const VKDescriptorSet::Location descriptor_set_location( + const shader::ShaderCreateInfo::Resource &resource) const; + const VKDescriptorSet::Location descriptor_set_location( + const shader::ShaderCreateInfo::Resource::BindType &bind_type, int binding) const; + + private: /** * Retrieve the shader input for the given resource. * @@ -35,5 +47,9 @@ class VKShaderInterface : public ShaderInterface { const ShaderInput *shader_input_get(const shader::ShaderCreateInfo::Resource &resource) const; const ShaderInput *shader_input_get( const shader::ShaderCreateInfo::Resource::BindType &bind_type, int binding) const; + const VKDescriptorSet::Location descriptor_set_location(const ShaderInput *shader_input) const; + void descriptor_set_location_update(const ShaderInput *shader_input, + const VKDescriptorSet::Location location); }; + } // namespace blender::gpu diff --git a/source/blender/gpu/vulkan/vk_storage_buffer.cc b/source/blender/gpu/vulkan/vk_storage_buffer.cc index 5c6d8a10244..8b5545f223f 100644 --- a/source/blender/gpu/vulkan/vk_storage_buffer.cc +++ b/source/blender/gpu/vulkan/vk_storage_buffer.cc @@ -34,9 +34,9 @@ void VKStorageBuffer::bind(int slot) } VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); - const ShaderInput *shader_input = shader_interface.shader_input_get( + const VKDescriptorSet::Location location = shader_interface.descriptor_set_location( shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, slot); - shader->pipeline_get().descriptor_set_get().bind(*this, shader_input); + shader->pipeline_get().descriptor_set_get().bind(*this, location); } void VKStorageBuffer::unbind() diff --git a/source/blender/gpu/vulkan/vk_texture.cc b/source/blender/gpu/vulkan/vk_texture.cc index bb587d477aa..a52c99cc061 100644 --- a/source/blender/gpu/vulkan/vk_texture.cc +++ b/source/blender/gpu/vulkan/vk_texture.cc @@ -227,8 +227,9 @@ void VKTexture::image_bind(int binding) } VKContext &context = *VKContext::get(); VKShader *shader = static_cast(context.shader); - VKDescriptorSet::Location location(shader->interface_get().shader_input_get( - shader::ShaderCreateInfo::Resource::BindType::IMAGE, binding)); + const VKShaderInterface &shader_interface = shader->interface_get(); + const VKDescriptorSet::Location location = shader_interface.descriptor_set_location( + shader::ShaderCreateInfo::Resource::BindType::IMAGE, binding); shader->pipeline_get().descriptor_set_get().image_bind(*this, location); } diff --git a/source/blender/gpu/vulkan/vk_vertex_buffer.cc b/source/blender/gpu/vulkan/vk_vertex_buffer.cc index 5d44b2e2c4f..b28822ab705 100644 --- a/source/blender/gpu/vulkan/vk_vertex_buffer.cc +++ b/source/blender/gpu/vulkan/vk_vertex_buffer.cc @@ -27,9 +27,9 @@ void VKVertexBuffer::bind_as_ssbo(uint binding) VKShader *shader = static_cast(context.shader); const VKShaderInterface &shader_interface = shader->interface_get(); - const ShaderInput *shader_input = shader_interface.shader_input_get( + const VKDescriptorSet::Location location = shader_interface.descriptor_set_location( shader::ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER, binding); - shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, shader_input); + shader->pipeline_get().descriptor_set_get().bind_as_ssbo(*this, location); } void VKVertexBuffer::bind_as_texture(uint /*binding*/) -- 2.30.2