From 370eb9e6a1e54da52047ba843e25c36be9789209 Mon Sep 17 00:00:00 2001 From: Mikko Rasa Date: Wed, 16 Mar 2022 10:44:06 +0200 Subject: [PATCH] Rewrite descriptor set management Descriptor sets have been moved from PipelineCache to a dedicated class. PipelineState refers to descriptor sets using slot indices, allowing the descriptor pool to be recreated if more space is needed. Dynamic uniform buffers are no longer used. Instead multiple copies of the descriptor set are created if it contains dynamic uniform blocks. --- source/backends/vulkan/commands_backend.cpp | 2 + source/backends/vulkan/destroyqueue.cpp | 5 ++ source/backends/vulkan/destroyqueue.h | 1 + source/backends/vulkan/device_backend.cpp | 3 +- source/backends/vulkan/device_backend.h | 3 ++ source/backends/vulkan/pipelinecache.cpp | 52 +------------------ source/backends/vulkan/pipelinecache.h | 3 -- .../backends/vulkan/pipelinestate_backend.cpp | 46 +++++++++------- .../backends/vulkan/pipelinestate_backend.h | 6 ++- source/backends/vulkan/program_backend.cpp | 2 +- 10 files changed, 46 insertions(+), 77 deletions(-) diff --git a/source/backends/vulkan/commands_backend.cpp b/source/backends/vulkan/commands_backend.cpp index 8f32e0b0..16926044 100644 --- a/source/backends/vulkan/commands_backend.cpp +++ b/source/backends/vulkan/commands_backend.cpp @@ -191,6 +191,8 @@ void VulkanCommands::begin_frame(unsigned index) current_pool->primary.next_buffer = 0; current_pool->secondary.next_buffer = 0; } + + device.get_descriptor_pool().begin_frame(); } void VulkanCommands::submit_frame(Semaphore *wait_sem, Semaphore *signal_sem) diff --git a/source/backends/vulkan/destroyqueue.cpp b/source/backends/vulkan/destroyqueue.cpp index 32ec5cff..929f89c6 100644 --- a/source/backends/vulkan/destroyqueue.cpp +++ b/source/backends/vulkan/destroyqueue.cpp @@ -20,6 +20,11 @@ void DestroyQueue::destroy(VkBuffer handle, unsigned mem_id) destroy(handle, mem_id); } +void DestroyQueue::destroy(VkDescriptorPool handle) +{ + destroy(handle); +} + void DestroyQueue::destroy(VkFence handle) { destroy(handle); diff --git a/source/backends/vulkan/destroyqueue.h b/source/backends/vulkan/destroyqueue.h index 43ceeeaa..e8da7a0c 100644 --- a/source/backends/vulkan/destroyqueue.h +++ b/source/backends/vulkan/destroyqueue.h @@ -30,6 +30,7 @@ public: ~DestroyQueue(); void destroy(VkBuffer, unsigned); + void destroy(VkDescriptorPool); void destroy(VkFence); void destroy(VkFramebuffer); void destroy(VkImage, unsigned); diff --git a/source/backends/vulkan/device_backend.cpp b/source/backends/vulkan/device_backend.cpp index 3be29c63..f1f2e327 100644 --- a/source/backends/vulkan/device_backend.cpp +++ b/source/backends/vulkan/device_backend.cpp @@ -18,7 +18,8 @@ VulkanDevice::VulkanDevice(Graphics::Window &wnd, const Graphics::VulkanOptions destroy_queue(*static_cast(this)), synchronizer(*static_cast(this)), transfer_queue(*static_cast(this)), - pipeline_cache(*static_cast(this)) + pipeline_cache(*static_cast(this)), + descriptor_pool(*static_cast(this)) { } // Cause the destructor of RefPtr to be emitted here diff --git a/source/backends/vulkan/device_backend.h b/source/backends/vulkan/device_backend.h index b4b53717..d490c6f9 100644 --- a/source/backends/vulkan/device_backend.h +++ b/source/backends/vulkan/device_backend.h @@ -3,6 +3,7 @@ #include #include +#include "descriptorpool.h" #include "destroyqueue.h" #include "handles.h" #include "memoryallocator.h" @@ -29,6 +30,7 @@ protected: Synchronizer synchronizer; TransferQueue transfer_queue; PipelineCache pipeline_cache; + DescriptorPool descriptor_pool; unsigned n_frames_in_flight = 3; VulkanDevice(Graphics::Window &, const Graphics::VulkanOptions &); @@ -47,6 +49,7 @@ public: Synchronizer &get_synchronizer() { return synchronizer; } TransferQueue &get_transfer_queue() { return transfer_queue; } PipelineCache &get_pipeline_cache() { return pipeline_cache; } + DescriptorPool &get_descriptor_pool() { return descriptor_pool; } unsigned get_n_frames_in_flight() const { return n_frames_in_flight; } }; diff --git a/source/backends/vulkan/pipelinecache.cpp b/source/backends/vulkan/pipelinecache.cpp index 1568e643..052ce37f 100644 --- a/source/backends/vulkan/pipelinecache.cpp +++ b/source/backends/vulkan/pipelinecache.cpp @@ -15,23 +15,7 @@ namespace GL { PipelineCache::PipelineCache(Device &d): device(d) -{ - const VulkanFunctions &vk = device.get_functions(); - - VkDescriptorPoolSize pool_sizes[2] = { }; - pool_sizes[0].type = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; - pool_sizes[0].descriptorCount = 10000; - pool_sizes[1].type = VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER; - pool_sizes[1].descriptorCount = 10000; - - VkDescriptorPoolCreateInfo pool_info = { }; - pool_info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO; - pool_info.maxSets = 10000; - pool_info.poolSizeCount = 2; - pool_info.pPoolSizes = pool_sizes; - - vk.CreateDescriptorPool(pool_info, descriptor_pool); -} +{ } PipelineCache::~PipelineCache() { @@ -41,7 +25,6 @@ PipelineCache::~PipelineCache() vk.DestroyRenderPass(kvp.second); for(const auto &kvp: pipelines) vk.DestroyPipeline(kvp.second); - vk.DestroyDescriptorPool(descriptor_pool); } VkRenderPass PipelineCache::get_render_pass(const FrameFormat &format, bool clear, bool discard, bool to_present) @@ -150,38 +133,5 @@ VkPipeline PipelineCache::get_pipeline(const PipelineState &ps) return pipeline; } -VkDescriptorSet PipelineCache::get_descriptor_set(const PipelineState &ps, unsigned index) -{ - const VulkanFunctions &vk = device.get_functions(); - - uint64_t key = ps.compute_descriptor_set_hash(index); - auto i = descriptor_sets.find(key); - if(i!=descriptor_sets.end()) - return i->second; - - VkDescriptorSetLayout layout = ps.get_descriptor_set_layout(index); - - VkDescriptorSetAllocateInfo alloc_info = { }; - alloc_info.sType = VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO; - alloc_info.descriptorPool = handle_cast<::VkDescriptorPool>(descriptor_pool); - alloc_info.descriptorSetCount = 1; - alloc_info.pSetLayouts = handle_cast<::VkDescriptorSetLayout *>(&layout); - - VkDescriptorSet desc_set; - vk.AllocateDescriptorSets(alloc_info, &desc_set); - - vector buffer; - unsigned n_writes = ps.fill_descriptor_writes(index, buffer); - VkWriteDescriptorSet *writes = reinterpret_cast(buffer.data()); - for(unsigned j=0; j(desc_set); - - vk.UpdateDescriptorSets(n_writes, writes, 0, 0); - - descriptor_sets.insert(make_pair(key, desc_set)); - - return desc_set; -} - } // namespace GL } // namespace Msp diff --git a/source/backends/vulkan/pipelinecache.h b/source/backends/vulkan/pipelinecache.h index 1017a93f..5023b40c 100644 --- a/source/backends/vulkan/pipelinecache.h +++ b/source/backends/vulkan/pipelinecache.h @@ -16,10 +16,8 @@ class PipelineCache { private: Device &device; - VkDescriptorPool descriptor_pool; std::map render_passes; std::map pipelines; - std::map descriptor_sets; public: PipelineCache(Device &); @@ -28,7 +26,6 @@ public: VkRenderPass get_render_pass(const FrameFormat &, bool, bool, bool); VkPipeline get_pipeline(const PipelineState &); - VkDescriptorSet get_descriptor_set(const PipelineState &, unsigned); }; } // namespace GL diff --git a/source/backends/vulkan/pipelinestate_backend.cpp b/source/backends/vulkan/pipelinestate_backend.cpp index 31488bad..a6a548af 100644 --- a/source/backends/vulkan/pipelinestate_backend.cpp +++ b/source/backends/vulkan/pipelinestate_backend.cpp @@ -76,10 +76,10 @@ void VulkanPipelineState::update() const if(changed_sets) { - descriptor_set_handles.resize(self.shprog->get_n_descriptor_sets()); - for(unsigned i=0; iget_n_descriptor_sets()); + for(unsigned i=0; i(this); + + auto i = lower_bound_member(self.uniform_blocks, static_cast(index)<<20, &PipelineState::BoundUniformBlock::binding); + for(; (i!=self.uniform_blocks.end() && static_cast(i->binding)>>20==index); ++i) + if(i->used && i->buffer && i->buffer->get_usage()==STREAMING) + return true; + + return false; +} + VkDescriptorSetLayout VulkanPipelineState::get_descriptor_set_layout(unsigned index) const { return static_cast(this)->shprog->desc_set_layout_handles[index]; } -unsigned VulkanPipelineState::fill_descriptor_writes(unsigned index, vector &buffer) const +unsigned VulkanPipelineState::fill_descriptor_writes(unsigned index, unsigned frame, vector &buffer) const { const PipelineState &self = *static_cast(this); @@ -320,12 +332,14 @@ unsigned VulkanPipelineState::fill_descriptor_writes(unsigned index, vectorbuffer = handle_cast<::VkBuffer>(i->buffer->handle); buffer_ptr->offset = i->block->get_offset(); + if(i->buffer->get_usage()==STREAMING) + buffer_ptr->offset += frame*i->buffer->get_size(); buffer_ptr->range = i->block->get_data_size(); write_ptr->sType = VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET; write_ptr->dstBinding = i->binding&0xFFFFF; write_ptr->descriptorCount = 1; - write_ptr->descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; + write_ptr->descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; write_ptr->pBufferInfo = buffer_ptr; ++buffer_ptr; @@ -369,8 +383,8 @@ void VulkanPipelineState::apply(VkCommandBuffer command_buffer, const VulkanPipe unapplied |= PipelineState::SHPROG; if(self.vertex_setup!=last_ps.vertex_setup) unapplied |= PipelineState::VERTEX_SETUP; - for(unsigned i=0; (idescriptor_set_handles.size()); ++i) - if(descriptor_set_handles[i]!=last->descriptor_set_handles[i]) + for(unsigned i=0; (idescriptor_set_slots.size()); ++i) + if(descriptor_set_slots[i]!=last->descriptor_set_slots[i]) { unapplied |= PipelineState::UNIFORMS; break; @@ -403,21 +417,15 @@ void VulkanPipelineState::apply(VkCommandBuffer command_buffer, const VulkanPipe } } - if((unapplied&PipelineState::UNIFORMS) && !descriptor_set_handles.empty()) + if((unapplied&PipelineState::UNIFORMS) && !descriptor_set_slots.empty()) { - vector dynamic_offsets; - dynamic_offsets.reserve(self.uniform_blocks.size()); - for(const PipelineState::BoundUniformBlock &u: self.uniform_blocks) - if(u.used && u.binding>=0) - { - if(u.buffer->get_usage()==STREAMING) - dynamic_offsets.push_back(frame*u.buffer->get_size()); - else - dynamic_offsets.push_back(0); - } + vector descriptor_set_handles; + descriptor_set_handles.reserve(self.descriptor_set_slots.size()); + for(unsigned i=0; ilayout_handle, - 0, descriptor_set_handles.size(), descriptor_set_handles.data(), dynamic_offsets.size(), dynamic_offsets.data()); + 0, descriptor_set_handles.size(), descriptor_set_handles.data(), 0, 0); } if(unapplied&(PipelineState::VIEWPORT|PipelineState::SCISSOR)) diff --git a/source/backends/vulkan/pipelinestate_backend.h b/source/backends/vulkan/pipelinestate_backend.h index d482081b..085309e6 100644 --- a/source/backends/vulkan/pipelinestate_backend.h +++ b/source/backends/vulkan/pipelinestate_backend.h @@ -11,6 +11,7 @@ class Device; class VulkanPipelineState: public NonCopyable { + friend class DescriptorPool; friend class PipelineCache; friend class VulkanCommands; @@ -19,7 +20,7 @@ protected: mutable unsigned changes = 0; mutable unsigned unapplied = 0; mutable VkPipeline handle; - mutable std::vector descriptor_set_handles; + mutable std::vector descriptor_set_slots; VulkanPipelineState(); VulkanPipelineState(VulkanPipelineState &&); @@ -31,8 +32,9 @@ protected: std::uint64_t compute_hash() const; void fill_creation_info(std::vector &) const; std::uint64_t compute_descriptor_set_hash(unsigned) const; + bool is_descriptor_set_dynamic(unsigned) const; VkDescriptorSetLayout get_descriptor_set_layout(unsigned) const; - unsigned fill_descriptor_writes(unsigned, std::vector &) const; + unsigned fill_descriptor_writes(unsigned, unsigned, std::vector &) const; void apply(VkCommandBuffer, const VulkanPipelineState *, unsigned, bool) const; }; diff --git a/source/backends/vulkan/program_backend.cpp b/source/backends/vulkan/program_backend.cpp index 5af7d67a..0c30b99e 100644 --- a/source/backends/vulkan/program_backend.cpp +++ b/source/backends/vulkan/program_backend.cpp @@ -126,7 +126,7 @@ void VulkanProgram::finalize_uniforms() bindings.emplace_back(); VkDescriptorSetLayoutBinding &binding = bindings.back(); binding.binding = b.bind_point; - binding.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC; + binding.descriptorType = VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER; binding.descriptorCount = 1; binding.stageFlags = VK_SHADER_STAGE_ALL; binding.pImmutableSamplers = 0; -- 2.43.0