diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index cb399d9f23d1b..3bc11ba252784 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -4,6 +4,8 @@ #include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "fml/status.h" +#include "impeller/core/allocator.h" +#include "impeller/core/device_buffer.h" #include "impeller/core/shader_types.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" @@ -23,15 +25,19 @@ namespace impeller { // manually changed. static constexpr size_t kMagicSubpassInputBinding = 64; -static bool BindImages(const Bindings& bindings, - Allocator& allocator, - const std::shared_ptr& encoder, - vk::DescriptorSet& vk_desc_set, - std::vector& images, - std::vector& writes) { +static bool BindImages( + const Bindings& bindings, + Allocator& allocator, + const std::shared_ptr& encoder, + vk::DescriptorSet& vk_desc_set, + std::array& image_workspace, + size_t& image_offset, + std::array& + write_workspace, + size_t& write_offset) { for (const TextureAndSampler& data : bindings.sampled_images) { - auto texture = data.texture.resource; - const auto& texture_vk = TextureVK::Cast(*texture); + const std::shared_ptr& texture = data.texture.resource; + const TextureVK& texture_vk = TextureVK::Cast(*texture); const SamplerVK& sampler = SamplerVK::Cast(*data.sampler); if (!encoder->Track(texture) || @@ -45,36 +51,35 @@ static bool BindImages(const Bindings& bindings, image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; image_info.sampler = sampler.GetSampler(); image_info.imageView = texture_vk.GetImageView(); - images.push_back(image_info); + image_workspace[image_offset++] = image_info; vk::WriteDescriptorSet write_set; write_set.dstSet = vk_desc_set; write_set.dstBinding = slot.binding; write_set.descriptorCount = 1u; write_set.descriptorType = vk::DescriptorType::eCombinedImageSampler; - write_set.pImageInfo = &images.back(); + write_set.pImageInfo = &image_workspace[image_offset - 1]; - writes.push_back(write_set); + write_workspace[write_offset++] = write_set; } return true; }; -static bool BindBuffers(const Bindings& bindings, - Allocator& allocator, - const std::shared_ptr& encoder, - vk::DescriptorSet& vk_desc_set, - const std::vector& desc_set, - std::vector& buffers, - std::vector& writes) { +static bool BindBuffers( + const Bindings& bindings, + Allocator& allocator, + const std::shared_ptr& encoder, + vk::DescriptorSet& vk_desc_set, + const std::vector& desc_set, + std::array& buffer_workspace, + size_t& buffer_offset, + std::array& + write_workspace, + size_t& write_offset) { for (const BufferAndUniformSlot& data : bindings.buffers) { - const auto& buffer_view = data.view.resource.buffer; - - auto device_buffer = buffer_view; - if (!device_buffer) { - VALIDATION_LOG << "Failed to get device buffer for vertex binding"; - return false; - } + const std::shared_ptr& device_buffer = + data.view.resource.buffer; auto buffer = DeviceBufferVK::Cast(*device_buffer).GetBuffer(); if (!buffer) { @@ -91,7 +96,7 @@ static bool BindBuffers(const Bindings& bindings, buffer_info.buffer = buffer; buffer_info.offset = offset; buffer_info.range = data.view.resource.range.length; - buffers.push_back(buffer_info); + buffer_workspace[buffer_offset++] = buffer_info; // TODO(jonahwilliams): remove this part by storing more data in // ShaderUniformSlot. @@ -113,156 +118,111 @@ static bool BindBuffers(const Bindings& bindings, write_set.dstBinding = uniform.binding; write_set.descriptorCount = 1u; write_set.descriptorType = ToVKDescriptorType(layout.descriptor_type); - write_set.pBufferInfo = &buffers.back(); + write_set.pBufferInfo = &buffer_workspace[buffer_offset - 1]; - writes.push_back(write_set); + write_workspace[write_offset++] = write_set; } return true; } -fml::StatusOr> AllocateAndBindDescriptorSets( +fml::StatusOr AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands, - const TextureVK& input_attachment) { - if (commands.empty()) { - return std::vector{}; - } - - // Step 1: Determine the total number of buffer and sampler descriptor - // sets required. Collect this information along with the layout information - // to allocate a correctly sized descriptor pool. - size_t buffer_count = 0; - size_t samplers_count = 0; - size_t subpass_count = 0; - std::vector layouts; - layouts.reserve(commands.size()); - - for (const auto& command : commands) { - buffer_count += command.vertex_bindings.buffers.size(); - buffer_count += command.fragment_bindings.buffers.size(); - samplers_count += command.fragment_bindings.sampled_images.size(); - subpass_count += - command.pipeline->GetDescriptor().UsesSubpassInput() ? 1 : 0; - - layouts.emplace_back( - PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); - } + Allocator& allocator, + const Command& command, + const TextureVK& input_attachment, + std::array& image_workspace, + std::array& buffer_workspace, + std::array& + write_workspace) { auto descriptor_result = encoder->AllocateDescriptorSets( - buffer_count, samplers_count, subpass_count, layouts); + PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout(), context); if (!descriptor_result.ok()) { return descriptor_result.status(); } - auto descriptor_sets = descriptor_result.value(); - if (descriptor_sets.empty()) { - return fml::Status(); + vk::DescriptorSet descriptor_set = descriptor_result.value(); + + size_t buffer_offset = 0u; + size_t image_offset = 0u; + size_t write_offset = 0u; + + auto& pipeline_descriptor = command.pipeline->GetDescriptor(); + auto& desc_set = + pipeline_descriptor.GetVertexDescriptor()->GetDescriptorSetLayouts(); + + if (!BindBuffers(command.vertex_bindings, allocator, encoder, descriptor_set, + desc_set, buffer_workspace, buffer_offset, write_workspace, + write_offset) || + !BindBuffers(command.fragment_bindings, allocator, encoder, + descriptor_set, desc_set, buffer_workspace, buffer_offset, + write_workspace, write_offset) || + !BindImages(command.fragment_bindings, allocator, encoder, descriptor_set, + image_workspace, image_offset, write_workspace, + write_offset)) { + return fml::Status(fml::StatusCode::kUnknown, + "Failed to bind texture or buffer."); } - // Step 2: Update the descriptors for all image and buffer descriptors used - // in the render pass. - std::vector images; - std::vector buffers; - std::vector writes; - images.reserve(samplers_count + subpass_count); - buffers.reserve(buffer_count); - writes.reserve(samplers_count + buffer_count + subpass_count); - - auto& allocator = *context.GetResourceAllocator(); - auto desc_index = 0u; - for (const auto& command : commands) { - auto desc_set = command.pipeline->GetDescriptor() - .GetVertexDescriptor() - ->GetDescriptorSetLayouts(); - if (!BindBuffers(command.vertex_bindings, allocator, encoder, - descriptor_sets[desc_index], desc_set, buffers, writes) || - !BindBuffers(command.fragment_bindings, allocator, encoder, - descriptor_sets[desc_index], desc_set, buffers, writes) || - !BindImages(command.fragment_bindings, allocator, encoder, - descriptor_sets[desc_index], images, writes)) { - return fml::Status(fml::StatusCode::kUnknown, - "Failed to bind texture or buffer."); - } + if (pipeline_descriptor.UsesSubpassInput()) { + vk::DescriptorImageInfo image_info; + image_info.imageLayout = vk::ImageLayout::eGeneral; + image_info.sampler = VK_NULL_HANDLE; + image_info.imageView = input_attachment.GetImageView(); + image_workspace[image_offset++] = image_info; - if (command.pipeline->GetDescriptor().UsesSubpassInput()) { - vk::DescriptorImageInfo image_info; - image_info.imageLayout = vk::ImageLayout::eGeneral; - image_info.sampler = VK_NULL_HANDLE; - image_info.imageView = input_attachment.GetImageView(); - images.push_back(image_info); - - vk::WriteDescriptorSet write_set; - write_set.dstSet = descriptor_sets[desc_index]; - write_set.dstBinding = kMagicSubpassInputBinding; - write_set.descriptorCount = 1u; - write_set.descriptorType = vk::DescriptorType::eInputAttachment; - write_set.pImageInfo = &images.back(); - - writes.push_back(write_set); - } - desc_index += 1; + vk::WriteDescriptorSet write_set; + write_set.dstSet = descriptor_set; + write_set.dstBinding = kMagicSubpassInputBinding; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eInputAttachment; + write_set.pImageInfo = &image_workspace[image_offset - 1]; + + write_workspace[write_offset++] = write_set; } - context.GetDevice().updateDescriptorSets(writes, {}); - return descriptor_sets; + context.GetDevice().updateDescriptorSets(write_offset, write_workspace.data(), + 0u, {}); + + return descriptor_set; } -fml::StatusOr> AllocateAndBindDescriptorSets( +fml::StatusOr AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands) { - if (commands.empty()) { - return std::vector{}; - } - // Step 1: Determine the total number of buffer and sampler descriptor - // sets required. Collect this information along with the layout information - // to allocate a correctly sized descriptor pool. - size_t buffer_count = 0; - size_t samplers_count = 0; - std::vector layouts; - layouts.reserve(commands.size()); - - for (const auto& command : commands) { - buffer_count += command.bindings.buffers.size(); - samplers_count += command.bindings.sampled_images.size(); - - layouts.emplace_back( - ComputePipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout()); - } - auto descriptor_result = - encoder->AllocateDescriptorSets(buffer_count, samplers_count, 0, layouts); + Allocator& allocator, + const ComputeCommand& command, + std::array& image_workspace, + std::array& buffer_workspace, + std::array& + write_workspace) { + auto descriptor_result = encoder->AllocateDescriptorSets( + ComputePipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout(), + context); if (!descriptor_result.ok()) { return descriptor_result.status(); } - auto descriptor_sets = descriptor_result.value(); - if (descriptor_sets.empty()) { - return fml::Status(); - } - // Step 2: Update the descriptors for all image and buffer descriptors used - // in the render pass. - std::vector images; - std::vector buffers; - std::vector writes; - images.reserve(samplers_count); - buffers.reserve(buffer_count); - writes.reserve(samplers_count + buffer_count); - - auto& allocator = *context.GetResourceAllocator(); - auto desc_index = 0u; - for (const auto& command : commands) { - auto desc_set = command.pipeline->GetDescriptor().GetDescriptorSetLayouts(); - - if (!BindBuffers(command.bindings, allocator, encoder, - descriptor_sets[desc_index], desc_set, buffers, writes) || - !BindImages(command.bindings, allocator, encoder, - descriptor_sets[desc_index], images, writes)) { - return fml::Status(fml::StatusCode::kUnknown, - "Failed to bind texture or buffer."); - } - desc_index += 1; + auto descriptor_set = descriptor_result.value(); + + size_t buffer_offset = 0u; + size_t image_offset = 0u; + size_t write_offset = 0u; + + auto& pipeline_descriptor = command.pipeline->GetDescriptor(); + auto& desc_set = pipeline_descriptor.GetDescriptorSetLayouts(); + + if (!BindBuffers(command.bindings, allocator, encoder, descriptor_set, + desc_set, buffer_workspace, buffer_offset, write_workspace, + write_offset) || + !BindImages(command.bindings, allocator, encoder, descriptor_set, + image_workspace, image_offset, write_workspace, + write_offset)) { + return fml::Status(fml::StatusCode::kUnknown, + "Failed to bind texture or buffer."); } + context.GetDevice().updateDescriptorSets(write_offset, write_workspace.data(), + 0u, {}); - context.GetDevice().updateDescriptorSets(writes, {}); - return descriptor_sets; + return descriptor_set; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.h b/impeller/renderer/backend/vulkan/binding_helpers_vk.h index b232d360cf4d4..1dc7f32d2a4b6 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.h +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BINDING_HELPERS_VK_H_ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BINDING_HELPERS_VK_H_ -#include - #include "fml/status_or.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" @@ -15,16 +13,30 @@ namespace impeller { -fml::StatusOr> AllocateAndBindDescriptorSets( +// Limit on the total number of buffer and image bindings that allow the Vulkan +// backend to avoid dynamic heap allocations. +static constexpr size_t kMaxBindings = 32; + +fml::StatusOr AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands, - const TextureVK& input_attachment); - -fml::StatusOr> AllocateAndBindDescriptorSets( + Allocator& allocator, + const Command& command, + const TextureVK& input_attachment, + std::array& image_workspace, + std::array& buffer_workspace, + std::array& + write_workspace); + +fml::StatusOr AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, - const std::vector& commands); + Allocator& allocator, + const ComputeCommand& command, + std::array& image_workspace, + std::array& buffer_workspace, + std::array& + write_workspace); } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.cc b/impeller/renderer/backend/vulkan/command_encoder_vk.cc index 4cd2284dc5cfc..603d3d97b35f4 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.cc +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.cc @@ -294,18 +294,15 @@ bool CommandEncoderVK::IsTracking( return tracked_objects_->IsTracking(source); } -fml::StatusOr> -CommandEncoderVK::AllocateDescriptorSets( - uint32_t buffer_count, - uint32_t sampler_count, - uint32_t subpass_count, - const std::vector& layouts) { +fml::StatusOr CommandEncoderVK::AllocateDescriptorSets( + const vk::DescriptorSetLayout& layout, + const ContextVK& context) { if (!IsValid()) { return fml::Status(fml::StatusCode::kUnknown, "command encoder invalid"); } - return tracked_objects_->GetDescriptorPool().AllocateDescriptorSets( - buffer_count, sampler_count, subpass_count, layouts); + return tracked_objects_->GetDescriptorPool().AllocateDescriptorSets(layout, + context); } void CommandEncoderVK::PushDebugGroup(const char* label) const { diff --git a/impeller/renderer/backend/vulkan/command_encoder_vk.h b/impeller/renderer/backend/vulkan/command_encoder_vk.h index 3622447a3041b..740c1123eae0b 100644 --- a/impeller/renderer/backend/vulkan/command_encoder_vk.h +++ b/impeller/renderer/backend/vulkan/command_encoder_vk.h @@ -82,11 +82,9 @@ class CommandEncoderVK { void InsertDebugMarker(const char* label) const; - fml::StatusOr> AllocateDescriptorSets( - uint32_t buffer_count, - uint32_t sampler_count, - uint32_t subpass_count, - const std::vector& layouts); + fml::StatusOr AllocateDescriptorSets( + const vk::DescriptorSetLayout& layout, + const ContextVK& context); private: friend class ContextVK; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index fffc615e5d04b..4788ec1a42451 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -101,26 +101,28 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, VALIDATION_LOG << "Could not update binding layouts for compute pass."; return false; } - auto desc_sets_result = - AllocateAndBindDescriptorSets(vk_context, encoder, commands_); - if (!desc_sets_result.ok()) { - return false; - } - auto desc_sets = desc_sets_result.value(); + auto& allocator = *context.GetResourceAllocator(); TRACE_EVENT0("impeller", "EncodeComputePassCommands"); - size_t desc_index = 0; for (const auto& command : commands_) { + auto desc_set_result = AllocateAndBindDescriptorSets( + vk_context, encoder, allocator, command, image_workspace_, + buffer_workspace_, write_workspace_); + if (!desc_set_result.ok()) { + return false; + } + auto desc_set = desc_set_result.value(); + const auto& pipeline_vk = ComputePipelineVK::Cast(*command.pipeline); cmd_buffer.bindPipeline(vk::PipelineBindPoint::eCompute, pipeline_vk.GetPipeline()); cmd_buffer.bindDescriptorSets( - vk::PipelineBindPoint::eCompute, // bind point - pipeline_vk.GetPipelineLayout(), // layout - 0, // first set - {vk::DescriptorSet{desc_sets[desc_index]}}, // sets - nullptr // offsets + vk::PipelineBindPoint::eCompute, // bind point + pipeline_vk.GetPipelineLayout(), // layout + 0, // first set + {vk::DescriptorSet{desc_set}}, // sets + nullptr // offsets ); // TOOD(dnfield): This should be moved to caps. But for now keeping this @@ -148,7 +150,6 @@ bool ComputePassVK::OnEncodeCommands(const Context& context, } cmd_buffer.dispatch(width, height, 1); } - desc_index += 1; } return true; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index caf88577bc5dd..2e2259c8121dc 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -6,6 +6,7 @@ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_COMPUTE_PASS_VK_H_ #include "flutter/fml/macros.h" +#include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/compute_pass.h" @@ -24,6 +25,10 @@ class ComputePassVK final : public ComputePass { std::weak_ptr command_buffer_; std::string label_; bool is_valid_ = false; + mutable std::array image_workspace_; + mutable std::array buffer_workspace_; + mutable std::array + write_workspace_; ComputePassVK(std::weak_ptr context, std::weak_ptr command_buffer); diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc index 9d9461453c11f..c7545ea9a156d 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.cc @@ -3,10 +3,9 @@ // found in the LICENSE file. #include "impeller/renderer/backend/vulkan/descriptor_pool_vk.h" + #include -#include "flutter/fml/trace_event.h" -#include "impeller/base/allocation.h" #include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/resource_manager_vk.h" #include "vulkan/vulkan_enums.hpp" @@ -14,6 +13,22 @@ namespace impeller { +struct DescriptorPoolSize { + size_t buffer_bindings; + size_t texture_bindings; + size_t storage_bindings; + size_t subpass_bindings; +}; + +/// Descriptor pools are always allocated with the following sizes. +static const constexpr DescriptorPoolSize kDefaultBindingSize = + DescriptorPoolSize{ + .buffer_bindings = 512u, // Buffer Bindings + .texture_bindings = 256u, // Texture Bindings + .storage_bindings = 32, + .subpass_bindings = 4u // Subpass Bindings + }; + // Holds the command pool in a background thread, recyling it when not in use. class BackgroundDescriptorPoolVK final { public: @@ -21,11 +36,8 @@ class BackgroundDescriptorPoolVK final { explicit BackgroundDescriptorPoolVK( vk::UniqueDescriptorPool&& pool, - uint32_t allocated_capacity, std::weak_ptr recycler) - : pool_(std::move(pool)), - allocated_capacity_(allocated_capacity), - recycler_(std::move(recycler)) {} + : pool_(std::move(pool)), recycler_(std::move(recycler)) {} ~BackgroundDescriptorPoolVK() { auto const recycler = recycler_.lock(); @@ -38,7 +50,7 @@ class BackgroundDescriptorPoolVK final { return; } - recycler->Reclaim(std::move(pool_), allocated_capacity_); + recycler->Reclaim(std::move(pool_)); } private: @@ -52,14 +64,11 @@ class BackgroundDescriptorPoolVK final { std::weak_ptr recycler_; }; -DescriptorPoolVK::DescriptorPoolVK( - const std::weak_ptr& context) - : context_(context) { - FML_DCHECK(context.lock()); -} +DescriptorPoolVK::DescriptorPoolVK(std::weak_ptr context) + : context_(std::move(context)) {} DescriptorPoolVK::~DescriptorPoolVK() { - if (!pool_) { + if (pools_.empty()) { return; } @@ -72,50 +81,56 @@ DescriptorPoolVK::~DescriptorPoolVK() { return; } - auto reset_pool_when_dropped = BackgroundDescriptorPoolVK( - std::move(pool_), allocated_capacity_, recycler); + for (auto i = 0u; i < pools_.size(); i++) { + auto reset_pool_when_dropped = + BackgroundDescriptorPoolVK(std::move(pools_[i]), recycler); - UniqueResourceVKT pool( - context->GetResourceManager(), std::move(reset_pool_when_dropped)); + UniqueResourceVKT pool( + context->GetResourceManager(), std::move(reset_pool_when_dropped)); + } + pools_.clear(); } -fml::StatusOr> -DescriptorPoolVK::AllocateDescriptorSets( - uint32_t buffer_count, - uint32_t sampler_count, - uint32_t subpass_count, - const std::vector& layouts) { - std::shared_ptr strong_context = context_.lock(); - if (!strong_context) { - return fml::Status(fml::StatusCode::kUnknown, "No device"); +fml::StatusOr DescriptorPoolVK::AllocateDescriptorSets( + const vk::DescriptorSetLayout& layout, + const ContextVK& context_vk) { + if (pools_.empty()) { + CreateNewPool(context_vk); } - auto minimum_capacity = - std::max(std::max(sampler_count, buffer_count), subpass_count); - auto [new_pool, capacity] = - strong_context->GetDescriptorPoolRecycler()->Get(minimum_capacity); - if (!new_pool) { - return fml::Status(fml::StatusCode::kUnknown, - "Failed to create descriptor pool"); - } - pool_ = std::move(new_pool); - allocated_capacity_ = capacity; vk::DescriptorSetAllocateInfo set_info; - set_info.setDescriptorPool(pool_.get()); - set_info.setSetLayouts(layouts); + set_info.setDescriptorPool(pools_.back().get()); + set_info.setPSetLayouts(&layout); + set_info.setDescriptorSetCount(1); + + vk::DescriptorSet set; + auto result = context_vk.GetDevice().allocateDescriptorSets(&set_info, &set); + if (result == vk::Result::eErrorOutOfPoolMemory) { + // If the pool ran out of memory, we need to create a new pool. + CreateNewPool(context_vk); + set_info.setDescriptorPool(pools_.back().get()); + result = context_vk.GetDevice().allocateDescriptorSets(&set_info, &set); + } - auto [result, sets] = - strong_context->GetDevice().allocateDescriptorSets(set_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Could not allocate descriptor sets: " << vk::to_string(result); return fml::Status(fml::StatusCode::kUnknown, ""); } - return sets; + return set; +} + +fml::Status DescriptorPoolVK::CreateNewPool(const ContextVK& context_vk) { + auto new_pool = context_vk.GetDescriptorPoolRecycler()->Get(); + if (!new_pool) { + return fml::Status(fml::StatusCode::kUnknown, + "Failed to create descriptor pool"); + } + pools_.emplace_back(std::move(new_pool)); + return fml::Status(); } -void DescriptorPoolRecyclerVK::Reclaim(vk::UniqueDescriptorPool&& pool, - uint32_t allocated_capacity) { +void DescriptorPoolRecyclerVK::Reclaim(vk::UniqueDescriptorPool&& pool) { // Reset the pool on a background thread. auto strong_context = context_.lock(); if (!strong_context) { @@ -128,50 +143,21 @@ void DescriptorPoolRecyclerVK::Reclaim(vk::UniqueDescriptorPool&& pool, Lock recycled_lock(recycled_mutex_); if (recycled_.size() < kMaxRecycledPools) { - recycled_.push_back(std::make_pair(std::move(pool), allocated_capacity)); + recycled_.push_back(std::move(pool)); return; } - - // If recycled has exceeded the max size of 32, then we need to remove a pool - // from the list. If we were to drop this pool, then there is a risk that - // the list of recycled descriptor pools could fill up with descriptors that - // are too small to reuse. This would lead to all subsequent descriptor - // allocations no longer being recycled. Instead, we pick the first - // descriptor pool with a smaller capacity than the reseting pool to drop. - // This may result in us dropping the current pool instead. - std::optional selected_index = std::nullopt; - for (auto i = 0u; i < recycled_.size(); i++) { - const auto& [_, capacity] = recycled_[i]; - if (capacity < allocated_capacity) { - selected_index = i; - break; - } - } - if (selected_index.has_value()) { - recycled_[selected_index.value()] = - std::make_pair(std::move(pool), allocated_capacity); - } - // If selected index has no value, then no pools had a smaller capacity than - // this one and we drop it instead. } -DescriptorPoolAndSize DescriptorPoolRecyclerVK::Get(uint32_t minimum_capacity) { - // See note on DescriptorPoolRecyclerVK doc comment. - auto rounded_capacity = - std::max(Allocation::NextPowerOfTwoSize(minimum_capacity), 64u); - +vk::UniqueDescriptorPool DescriptorPoolRecyclerVK::Get() { // Recycle a pool with a matching minumum capcity if it is available. - auto recycled_pool = Reuse(rounded_capacity); + auto recycled_pool = Reuse(); if (recycled_pool.has_value()) { return std::move(recycled_pool.value()); } - return Create(rounded_capacity); + return Create(); } -DescriptorPoolAndSize DescriptorPoolRecyclerVK::Create( - uint32_t minimum_capacity) { - FML_DCHECK(Allocation::NextPowerOfTwoSize(minimum_capacity) == - minimum_capacity); +vk::UniqueDescriptorPool DescriptorPoolRecyclerVK::Create() { auto strong_context = context_.lock(); if (!strong_context) { VALIDATION_LOG << "Unable to create a descriptor pool"; @@ -180,44 +166,36 @@ DescriptorPoolAndSize DescriptorPoolRecyclerVK::Create( std::vector pools = { vk::DescriptorPoolSize{vk::DescriptorType::eCombinedImageSampler, - minimum_capacity}, + kDefaultBindingSize.texture_bindings}, vk::DescriptorPoolSize{vk::DescriptorType::eUniformBuffer, - minimum_capacity}, + kDefaultBindingSize.buffer_bindings}, vk::DescriptorPoolSize{vk::DescriptorType::eStorageBuffer, - minimum_capacity}, + kDefaultBindingSize.storage_bindings}, vk::DescriptorPoolSize{vk::DescriptorType::eInputAttachment, - minimum_capacity}}; + kDefaultBindingSize.subpass_bindings}}; vk::DescriptorPoolCreateInfo pool_info; - pool_info.setMaxSets(minimum_capacity + minimum_capacity); + pool_info.setMaxSets(kDefaultBindingSize.texture_bindings + + kDefaultBindingSize.buffer_bindings + + kDefaultBindingSize.storage_bindings + + kDefaultBindingSize.subpass_bindings); pool_info.setPoolSizes(pools); auto [result, pool] = strong_context->GetDevice().createDescriptorPoolUnique(pool_info); if (result != vk::Result::eSuccess) { VALIDATION_LOG << "Unable to create a descriptor pool"; } - return std::make_pair(std::move(pool), minimum_capacity); + return std::move(pool); } -std::optional DescriptorPoolRecyclerVK::Reuse( - uint32_t minimum_capacity) { - FML_DCHECK(Allocation::NextPowerOfTwoSize(minimum_capacity) == - minimum_capacity); +std::optional DescriptorPoolRecyclerVK::Reuse() { Lock lock(recycled_mutex_); - - std::optional found_index = std::nullopt; - for (auto i = 0u; i < recycled_.size(); i++) { - const auto& [_, capacity] = recycled_[i]; - if (capacity >= minimum_capacity) { - found_index = i; - break; - } - } - if (!found_index.has_value()) { + if (recycled_.empty()) { return std::nullopt; } - auto pair = std::move(recycled_[found_index.value()]); - recycled_.erase(recycled_.begin() + found_index.value()); - return pair; + + auto recycled = std::move(recycled_[recycled_.size() - 1]); + recycled_.pop_back(); + return recycled; } } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h index f4965ec224f6a..53f35a94ab890 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk.h +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk.h @@ -14,7 +14,7 @@ namespace impeller { //------------------------------------------------------------------------------ -/// @brief A short-lived fixed-sized descriptor pool. Descriptors +/// @brief A per-frame descriptor pool. Descriptors /// from this pool don't need to be freed individually. Instead, the /// pool must be collected after all the descriptors allocated from /// it are done being used. @@ -26,44 +26,28 @@ namespace impeller { /// threading and lifecycle restrictions. class DescriptorPoolVK { public: - explicit DescriptorPoolVK(const std::weak_ptr& context); + explicit DescriptorPoolVK(std::weak_ptr context); ~DescriptorPoolVK(); - fml::StatusOr> AllocateDescriptorSets( - uint32_t buffer_count, - uint32_t sampler_count, - uint32_t subpass_count, - const std::vector& layouts); + fml::StatusOr AllocateDescriptorSets( + const vk::DescriptorSetLayout& layout, + const ContextVK& context_vk); private: std::weak_ptr context_; - vk::UniqueDescriptorPool pool_ = {}; - uint32_t allocated_capacity_ = 0; + std::vector pools_; + + fml::Status CreateNewPool(const ContextVK& context_vk); DescriptorPoolVK(const DescriptorPoolVK&) = delete; DescriptorPoolVK& operator=(const DescriptorPoolVK&) = delete; }; -// A descriptor pool and its allocated buffer/sampler size. -using DescriptorPoolAndSize = std::pair; - //------------------------------------------------------------------------------ /// @brief Creates and manages the lifecycle of |vk::DescriptorPoolVK| /// objects. -/// -/// To make descriptor pool recycling more effective, the number of requusted -/// descriptor slots is rounded up the nearest power of two. This also makes -/// determining whether a recycled pool has sufficient slots easier as only a -/// single number comparison is required. -/// -/// We round up to a minimum of 64 as the smallest power of two to reduce the -/// range of potential allocations to approximately: 64, 128, 256, 512, 1024, -/// 2048, 4096. Beyond this size applications will have far too many drawing -/// commands to render correctly. We also limit the number of cached descriptor -/// pools to 32, which is somewhat arbitrarily chosen, but given 2-ish frames in -/// flight is about 16 descriptors pools per frame which is extremely generous. class DescriptorPoolRecyclerVK final : public std::enable_shared_from_this { public: @@ -78,41 +62,34 @@ class DescriptorPoolRecyclerVK final explicit DescriptorPoolRecyclerVK(std::weak_ptr context) : context_(std::move(context)) {} - /// @brief Gets a descriptor pool with at least [minimum_capacity] - /// sampler and slots. + /// @brief Gets a descriptor pool. /// /// This may create a new descriptor pool if no existing pools had /// the necessary capacity. - DescriptorPoolAndSize Get(uint32_t minimum_capacity); + vk::UniqueDescriptorPool Get(); /// @brief Returns the descriptor pool to be reset on a background /// thread. /// /// @param[in] pool The pool to recycler. - void Reclaim(vk::UniqueDescriptorPool&& pool, uint32_t allocated_capacity); + void Reclaim(vk::UniqueDescriptorPool&& pool); private: std::weak_ptr context_; Mutex recycled_mutex_; - std::vector recycled_ IPLR_GUARDED_BY(recycled_mutex_); + std::vector recycled_ IPLR_GUARDED_BY( + recycled_mutex_); /// @brief Creates a new |vk::CommandPool|. /// - /// The descriptor pool will have at least [minimum_capacity] - /// buffer and texture slots. - /// /// @returns Returns a |std::nullopt| if a pool could not be created. - DescriptorPoolAndSize Create(uint32_t minimum_capacity); + vk::UniqueDescriptorPool Create(); /// @brief Reuses a recycled |vk::CommandPool|, if available. /// - /// The descriptor pool will have at least [minimum_capacity] - /// buffer and texture slots. [minimum_capacity] should be rounded - /// up to the next power of two for more efficient cache reuse. - /// /// @returns Returns a |std::nullopt| if a pool was not available. - std::optional Reuse(uint32_t minimum_capacity); + std::optional Reuse(); DescriptorPoolRecyclerVK(const DescriptorPoolRecyclerVK&) = delete; diff --git a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc index 8749f52af4daa..34bbcdd91bdfe 100644 --- a/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc +++ b/impeller/renderer/backend/vulkan/descriptor_pool_vk_unittests.cc @@ -14,8 +14,8 @@ namespace testing { TEST(DescriptorPoolRecyclerVKTest, GetDescriptorPoolRecyclerCreatesNewPools) { auto const context = MockVulkanContextBuilder().Build(); - auto const [pool1, _] = context->GetDescriptorPoolRecycler()->Get(1024); - auto const [pool2, __] = context->GetDescriptorPoolRecycler()->Get(1024); + auto const pool1 = context->GetDescriptorPoolRecycler()->Get(); + auto const pool2 = context->GetDescriptorPoolRecycler()->Get(); // The two descriptor pools should be different. EXPECT_NE(pool1.get(), pool2.get()); @@ -23,22 +23,6 @@ TEST(DescriptorPoolRecyclerVKTest, GetDescriptorPoolRecyclerCreatesNewPools) { context->Shutdown(); } -TEST(DescriptorPoolRecyclerVKTest, DescriptorPoolCapacityIsRoundedUp) { - auto const context = MockVulkanContextBuilder().Build(); - auto const [pool1, capacity] = context->GetDescriptorPoolRecycler()->Get(1); - - // Rounds up to a minimum of 64. - EXPECT_EQ(capacity, 64u); - - auto const [pool2, capacity_2] = - context->GetDescriptorPoolRecycler()->Get(1023); - - // Rounds up to the next power of two. - EXPECT_EQ(capacity_2, 1024u); - - context->Shutdown(); -} - namespace { // Invokes the provided callback when the destructor is called. @@ -66,7 +50,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimMakesDescriptorPoolAvailable) { { // Fetch a pool (which will be created). auto pool = DescriptorPoolVK(context); - pool.AllocateDescriptorSets(1024, 1024, 1024, {}); + pool.AllocateDescriptorSets({}, *context); } // There is a chance that the first death rattle item below is destroyed in @@ -88,7 +72,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimMakesDescriptorPoolAvailable) { waiter.Wait(); } - auto const [pool, _] = context->GetDescriptorPoolRecycler()->Get(1024); + auto const pool = context->GetDescriptorPoolRecycler()->Get(); // Now check that we only ever created one pool. auto const called = GetMockVulkanFunctions(context->GetDevice()); @@ -106,7 +90,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimDropsDescriptorPoolIfSizeIsExceeded) { std::vector> pools; for (auto i = 0u; i < 33; i++) { auto pool = std::make_unique(context); - pool->AllocateDescriptorSets(1024, 1024, 1024, {}); + pool->AllocateDescriptorSets({}, *context); pools.push_back(std::move(pool)); } } @@ -135,7 +119,7 @@ TEST(DescriptorPoolRecyclerVKTest, ReclaimDropsDescriptorPoolIfSizeIsExceeded) { std::vector> pools; for (auto i = 0u; i < 33; i++) { auto pool = std::make_unique(context); - pool->AllocateDescriptorSets(1024, 1024, 1024, {}); + pool->AllocateDescriptorSets({}, *context); pools.push_back(std::move(pool)); } } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 42254364a0ee3..ef2fbff69ecfc 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -10,6 +10,7 @@ #include "flutter/fml/trace_event.h" #include "impeller/base/validation.h" +#include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" #include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" @@ -369,7 +370,8 @@ static bool EncodeCommand(const Context& context, CommandEncoderVK& encoder, PassBindingsCache& command_buffer_cache, const ISize& target_size, - const vk::DescriptorSet vk_desc_set) { + const vk::DescriptorSet vk_desc_set, + const vk::CommandBuffer& cmd_buffer) { #ifdef IMPELLER_DEBUG fml::ScopedCleanupClosure pop_marker( [&encoder]() { encoder.PopDebugGroup(); }); @@ -380,15 +382,15 @@ static bool EncodeCommand(const Context& context, } #endif // IMPELLER_DEBUG - const auto& cmd_buffer = encoder.GetCommandBuffer(); - const auto& pipeline_vk = PipelineVK::Cast(*command.pipeline); + const PipelineVK& pipeline_vk = PipelineVK::Cast(*command.pipeline); - encoder.GetCommandBuffer().bindDescriptorSets( - vk::PipelineBindPoint::eGraphics, // bind point - pipeline_vk.GetPipelineLayout(), // layout - 0, // first set - {vk::DescriptorSet{vk_desc_set}}, // sets - nullptr // offsets + cmd_buffer.bindDescriptorSets(vk::PipelineBindPoint::eGraphics, // bind point + pipeline_vk.GetPipelineLayout(), // layout + 0, // first set + 1, // set count + &vk_desc_set, // sets + 0, // offset count + nullptr // offsets ); command_buffer_cache.BindPipeline( @@ -403,39 +405,37 @@ static bool EncodeCommand(const Context& context, command.stencil_reference); // Configure vertex and index and buffers for binding. - auto& vertex_buffer_view = command.vertex_buffer.vertex_buffer; - - if (!vertex_buffer_view) { - return false; - } - - auto& allocator = *context.GetResourceAllocator(); - auto vertex_buffer = vertex_buffer_view.buffer; - - if (!vertex_buffer) { + if (!command.vertex_buffer.vertex_buffer) { VALIDATION_LOG << "Failed to acquire device buffer" << " for vertex buffer view"; return false; } + auto& allocator = *context.GetResourceAllocator(); + const std::shared_ptr& vertex_buffer = + command.vertex_buffer.vertex_buffer.buffer; + if (!encoder.Track(vertex_buffer)) { return false; } // Bind the vertex buffer. - auto vertex_buffer_handle = DeviceBufferVK::Cast(*vertex_buffer).GetBuffer(); + vk::Buffer vertex_buffer_handle = + DeviceBufferVK::Cast(*vertex_buffer).GetBuffer(); vk::Buffer vertex_buffers[] = {vertex_buffer_handle}; - vk::DeviceSize vertex_buffer_offsets[] = {vertex_buffer_view.range.offset}; + vk::DeviceSize vertex_buffer_offsets[] = { + command.vertex_buffer.vertex_buffer.range.offset}; cmd_buffer.bindVertexBuffers(0u, 1u, vertex_buffers, vertex_buffer_offsets); if (command.vertex_buffer.index_type != IndexType::kNone) { // Bind the index buffer. - auto index_buffer_view = command.vertex_buffer.index_buffer; + const BufferView& index_buffer_view = command.vertex_buffer.index_buffer; if (!index_buffer_view) { return false; } - auto index_buffer = index_buffer_view.buffer; + const std::shared_ptr& index_buffer = + index_buffer_view.buffer; if (!index_buffer) { VALIDATION_LOG << "Failed to acquire device buffer" << " for index buffer view"; @@ -446,7 +446,8 @@ static bool EncodeCommand(const Context& context, return false; } - auto index_buffer_handle = DeviceBufferVK::Cast(*index_buffer).GetBuffer(); + vk::Buffer index_buffer_handle = + DeviceBufferVK::Cast(*index_buffer).GetBuffer(); cmd_buffer.bindIndexBuffer(index_buffer_handle, index_buffer_view.range.offset, ToVKIndexType(command.vertex_buffer.index_type)); @@ -476,16 +477,18 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& vk_context = ContextVK::Cast(context); - auto command_buffer = command_buffer_.lock(); + std::shared_ptr command_buffer = command_buffer_.lock(); if (!command_buffer) { VALIDATION_LOG << "Command buffer died before commands could be encoded."; return false; } - auto encoder = command_buffer->GetEncoder(); + const std::shared_ptr& encoder = + command_buffer->GetEncoder(); if (!encoder) { return false; } +#ifdef IMPELLER_DEBUG fml::ScopedCleanupClosure pop_marker( [&encoder]() { encoder->PopDebugGroup(); }); if (!debug_label_.empty()) { @@ -493,8 +496,9 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { } else { pop_marker.Release(); } +#endif // IMPELLER_DEBUG - auto cmd_buffer = encoder->GetCommandBuffer(); + vk::CommandBuffer cmd_buffer = encoder->GetCommandBuffer(); if (!UpdateBindingLayouts(commands_, cmd_buffer)) { return false; @@ -509,7 +513,7 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { const auto& target_size = render_target_.GetRenderTargetSize(); - auto render_pass = CreateVKRenderPass( + SharedHandleVK render_pass = CreateVKRenderPass( vk_context, command_buffer, vk_context.GetCapabilities()->SupportsFramebufferFetch()); if (!render_pass) { @@ -537,14 +541,9 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { static_cast(target_size.height); pass_info.setClearValues(clear_values); - const auto& color_image_vk = TextureVK::Cast( + const TextureVK& color_image_vk = TextureVK::Cast( *render_target_.GetColorAttachments().find(0u)->second.texture); - auto desc_sets_result = AllocateAndBindDescriptorSets( - vk_context, encoder, commands_, color_image_vk); - if (!desc_sets_result.ok()) { - return false; - } - auto desc_sets = desc_sets_result.value(); + Allocator& allocator = *context.GetResourceAllocator(); { TRACE_EVENT0("impeller", "EncodeRenderPassCommands"); @@ -553,13 +552,18 @@ bool RenderPassVK::OnEncodeCommands(const Context& context) const { fml::ScopedCleanupClosure end_render_pass( [cmd_buffer]() { cmd_buffer.endRenderPass(); }); - auto desc_index = 0u; for (const auto& command : commands_) { + fml::StatusOr desc_set_result = + AllocateAndBindDescriptorSets(vk_context, encoder, allocator, command, + color_image_vk, image_workspace_, + buffer_workspace_, write_workspace_); + if (!desc_set_result.ok()) { + return false; + } if (!EncodeCommand(context, command, *encoder, pass_bindings_cache_, - target_size, desc_sets[desc_index])) { + target_size, desc_set_result.value(), cmd_buffer)) { return false; } - desc_index += 1; } } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index a173aafe35736..2d31abd4019f6 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -6,6 +6,7 @@ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_RENDER_PASS_VK_H_ #include "flutter/fml/macros.h" +#include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" #include "impeller/renderer/backend/vulkan/pass_bindings_cache.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" @@ -27,6 +28,12 @@ class RenderPassVK final : public RenderPass { std::weak_ptr command_buffer_; std::string debug_label_; bool is_valid_ = false; + + mutable std::array image_workspace_; + mutable std::array buffer_workspace_; + mutable std::array + write_workspace_; + mutable PassBindingsCache pass_bindings_cache_; RenderPassVK(const std::shared_ptr& context, diff --git a/impeller/renderer/backend/vulkan/surface_context_vk.h b/impeller/renderer/backend/vulkan/surface_context_vk.h index 0394d45c9650f..5c8a327685dba 100644 --- a/impeller/renderer/backend/vulkan/surface_context_vk.h +++ b/impeller/renderer/backend/vulkan/surface_context_vk.h @@ -8,7 +8,6 @@ #include #include "impeller/base/backend_cast.h" -#include "impeller/core/host_buffer.h" #include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/context.h"