From 9a84921e4eef62d6cc485072776fa47bc9296124 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Jan 2024 13:28:47 -0800 Subject: [PATCH 01/13] merge desc set changes. --- .../backend/vulkan/binding_helpers_vk.cc | 262 ++++++++---------- .../backend/vulkan/binding_helpers_vk.h | 28 +- .../backend/vulkan/command_encoder_vk.cc | 13 +- .../backend/vulkan/command_encoder_vk.h | 8 +- .../backend/vulkan/compute_pass_vk.cc | 27 +- .../renderer/backend/vulkan/compute_pass_vk.h | 5 + .../backend/vulkan/descriptor_pool_vk.cc | 178 ++++++------ .../backend/vulkan/descriptor_pool_vk.h | 53 +--- .../vulkan/descriptor_pool_vk_unittests.cc | 28 +- .../renderer/backend/vulkan/render_pass_vk.cc | 80 +++--- .../renderer/backend/vulkan/render_pass_vk.h | 7 + .../backend/vulkan/surface_context_vk.h | 1 - 12 files changed, 306 insertions(+), 384 deletions(-) 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" From 6285909d2a842b6735e0dd6b090390eb4f063ac7 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Jan 2024 16:07:42 -0800 Subject: [PATCH 02/13] [Impeller] encode commands to Vulkan command buffer. --- impeller/entity/inline_pass_context.cc | 3 +- impeller/entity/inline_pass_context.h | 1 - .../backend/vulkan/command_buffer_vk.cc | 9 +- .../backend/vulkan/command_buffer_vk.h | 1 - .../renderer/backend/vulkan/context_vk.cc | 2 +- .../renderer/backend/vulkan/gpu_tracer_vk.cc | 4 +- .../renderer/backend/vulkan/render_pass_vk.cc | 570 ++++++++++-------- .../renderer/backend/vulkan/render_pass_vk.h | 77 ++- impeller/renderer/render_pass.h | 52 +- impeller/renderer/renderer_unittests.cc | 15 - 10 files changed, 432 insertions(+), 302 deletions(-) diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index b8295c2cb444a..88d353d60f549 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -22,7 +22,6 @@ InlinePassContext::InlinePassContext( std::optional collapsed_parent_pass) : context_(std::move(context)), pass_target_(pass_target), - entity_count_(entity_count), is_collapsed_(collapsed_parent_pass.has_value()) { if (collapsed_parent_pass.has_value()) { pass_ = collapsed_parent_pass.value().pass; @@ -154,7 +153,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( // Commands are fairly large (500B) objects, so re-allocation of the command // buffer while encoding can add a surprising amount of overhead. We make a // conservative npot estimate to avoid this case. - pass_->ReserveCommands(Allocation::NextPowerOfTwoSize(entity_count_)); + // pass_->ReserveCommands(Allocation::NextPowerOfTwoSize(entity_count_)); pass_->SetLabel( "EntityPass Render Pass: Depth=" + std::to_string(pass_depth) + " Count=" + std::to_string(pass_count_)); diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index 595d291b5a1c8..c852394f53cdd 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -50,7 +50,6 @@ class InlinePassContext { std::shared_ptr command_buffer_; std::shared_ptr pass_; uint32_t pass_count_ = 0; - uint32_t entity_count_ = 0; // Whether this context is collapsed into a parent entity pass. bool is_collapsed_ = false; diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 78eb547847082..6b7b14f243fdb 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -7,13 +7,10 @@ #include #include -#include "flutter/fml/logging.h" -#include "impeller/base/validation.h" #include "impeller/renderer/backend/vulkan/blit_pass_vk.h" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/compute_pass_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" -#include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/render_pass_vk.h" #include "impeller/renderer/command_buffer.h" #include "impeller/renderer/render_target.h" @@ -73,9 +70,9 @@ std::shared_ptr CommandBufferVK::OnCreateRenderPass( return nullptr; } auto pass = - std::shared_ptr(new RenderPassVK(context, // - target, // - weak_from_this() // + std::shared_ptr(new RenderPassVK(context, // + target, // + shared_from_this() // )); if (!pass->IsValid()) { return nullptr; diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h index b1ede05b724f0..d85ce290082a8 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.h @@ -5,7 +5,6 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_COMMAND_BUFFER_VK_H_ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_COMMAND_BUFFER_VK_H_ -#include "flutter/fml/macros.h" #include "impeller/base/backend_cast.h" #include "impeller/renderer/backend/vulkan/vk.h" #include "impeller/renderer/command_buffer.h" diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 6c1804dd74022..ee4df3534b180 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -156,7 +156,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = settings.enable_validation; + auto enable_validation = false; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 921677408697c..964af7b2a167d 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -28,9 +28,7 @@ GPUTracerVK::GPUTracerVK(const std::shared_ptr& device_holder) return; } // Disable tracing in release mode. -#ifdef IMPELLER_DEBUG - enabled_ = true; -#endif + enabled_ = false; } bool GPUTracerVK::IsEnabled() const { diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index ef2fbff69ecfc..7025aca256529 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -8,7 +8,7 @@ #include #include -#include "flutter/fml/trace_event.h" +#include "fml/status.h" #include "impeller/base/validation.h" #include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" @@ -20,15 +20,58 @@ #include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/pipeline_vk.h" +#include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "impeller/renderer/command.h" + #include "vulkan/vulkan_enums.hpp" #include "vulkan/vulkan_handles.hpp" #include "vulkan/vulkan_to_string.hpp" namespace impeller { +static vk::ClearColorValue VKClearValueFromColor(Color color) { + vk::ClearColorValue value; + value.setFloat32( + std::array{color.red, color.green, color.blue, color.alpha}); + return value; +} + +static vk::ClearDepthStencilValue VKClearValueFromDepthStencil(uint32_t stencil, + Scalar depth) { + vk::ClearDepthStencilValue value; + value.depth = depth; + value.stencil = stencil; + return value; +} + +static std::vector GetVKClearValues( + const RenderTarget& target) { + std::vector clears; + + for (const auto& [_, color] : target.GetColorAttachments()) { + clears.emplace_back(VKClearValueFromColor(color.clear_color)); + if (color.resolve_texture) { + clears.emplace_back(VKClearValueFromColor(color.clear_color)); + } + } + + const auto& depth = target.GetDepthAttachment(); + const auto& stencil = target.GetStencilAttachment(); + + if (depth.has_value()) { + clears.emplace_back(VKClearValueFromDepthStencil( + stencil ? stencil->clear_stencil : 0u, depth->clear_depth)); + } + + if (stencil.has_value()) { + clears.emplace_back(VKClearValueFromDepthStencil( + stencil->clear_stencil, depth ? depth->clear_depth : 0.0f)); + } + + return clears; +} + static vk::AttachmentDescription CreateAttachmentDescription( const Attachment& attachment, const std::shared_ptr Attachment::*texture_ptr, @@ -199,61 +242,85 @@ SharedHandleVK RenderPassVK::CreateVKRenderPass( RenderPassVK::RenderPassVK(const std::shared_ptr& context, const RenderTarget& target, - std::weak_ptr command_buffer) + std::shared_ptr command_buffer) : RenderPass(context, target), command_buffer_(std::move(command_buffer)) { - is_valid_ = true; -} + const auto& vk_context = ContextVK::Cast(*context); + const std::shared_ptr& encoder = + command_buffer_->GetEncoder(); + command_buffer_vk_ = encoder->GetCommandBuffer(); + render_target_.IterateAllAttachments( + [&encoder](const auto& attachment) -> bool { + encoder->Track(attachment.texture); + encoder->Track(attachment.resolve_texture); + return true; + }); -RenderPassVK::~RenderPassVK() = default; + const auto& target_size = render_target_.GetRenderTargetSize(); -bool RenderPassVK::IsValid() const { - return is_valid_; -} + SharedHandleVK render_pass = CreateVKRenderPass( + vk_context, command_buffer_, + vk_context.GetCapabilities()->SupportsFramebufferFetch()); + if (!render_pass) { + VALIDATION_LOG << "Could not create renderpass."; + is_valid_ = false; + return; + } -void RenderPassVK::OnSetLabel(std::string label) { - debug_label_ = std::move(label); -} + auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass); + if (!framebuffer) { + VALIDATION_LOG << "Could not create framebuffer."; + is_valid_ = false; + return; + } -static vk::ClearColorValue VKClearValueFromColor(Color color) { - vk::ClearColorValue value; - value.setFloat32( - std::array{color.red, color.green, color.blue, color.alpha}); - return value; -} + if (!encoder->Track(framebuffer) || !encoder->Track(render_pass)) { + is_valid_ = false; + return; + } -static vk::ClearDepthStencilValue VKClearValueFromDepthStencil(uint32_t stencil, - Scalar depth) { - vk::ClearDepthStencilValue value; - value.depth = depth; - value.stencil = stencil; - return value; -} + auto clear_values = GetVKClearValues(render_target_); -static std::vector GetVKClearValues( - const RenderTarget& target) { - std::vector clears; + vk::RenderPassBeginInfo pass_info; + pass_info.renderPass = *render_pass; + pass_info.framebuffer = *framebuffer; + pass_info.renderArea.extent.width = static_cast(target_size.width); + pass_info.renderArea.extent.height = + static_cast(target_size.height); + pass_info.setClearValues(clear_values); - for (const auto& [_, color] : target.GetColorAttachments()) { - clears.emplace_back(VKClearValueFromColor(color.clear_color)); - if (color.resolve_texture) { - clears.emplace_back(VKClearValueFromColor(color.clear_color)); - } - } + command_buffer_vk_.beginRenderPass(pass_info, vk::SubpassContents::eInline); - const auto& depth = target.GetDepthAttachment(); - const auto& stencil = target.GetStencilAttachment(); + // Set the initial viewport and scissors. + const auto vp = Viewport{.rect = Rect::MakeSize(target_size)}; + vk::Viewport viewport = vk::Viewport() + .setWidth(vp.rect.GetWidth()) + .setHeight(-vp.rect.GetHeight()) + .setY(vp.rect.GetHeight()) + .setMinDepth(0.0f) + .setMaxDepth(1.0f); + command_buffer_vk_.setViewport(0, 1, &viewport); - if (depth.has_value()) { - clears.emplace_back(VKClearValueFromDepthStencil( - stencil ? stencil->clear_stencil : 0u, depth->clear_depth)); - } + // Set the initial scissor rect. + const auto sc = IRect::MakeSize(target_size); + vk::Rect2D scissor = + vk::Rect2D() + .setOffset(vk::Offset2D(sc.GetX(), sc.GetY())) + .setExtent(vk::Extent2D(sc.GetWidth(), sc.GetHeight())); + command_buffer_vk_.setScissor(0, 1, &scissor); - if (stencil.has_value()) { - clears.emplace_back(VKClearValueFromDepthStencil( - stencil->clear_stencil, depth ? depth->clear_depth : 0.0f)); - } + color_image_vk_ = + render_target_.GetColorAttachments().find(0u)->second.texture; + is_valid_ = true; +} - return clears; +RenderPassVK::~RenderPassVK() = default; + +bool RenderPassVK::IsValid() const { + return is_valid_; +} + +void RenderPassVK::OnSetLabel(std::string label) { + debug_label_ = std::move(label); } SharedHandleVK RenderPassVK::CreateVKFramebuffer( @@ -302,134 +369,115 @@ SharedHandleVK RenderPassVK::CreateVKFramebuffer( return MakeSharedVK(std::move(framebuffer)); } -static bool UpdateBindingLayouts(const Bindings& bindings, - const vk::CommandBuffer& buffer) { - // All previous writes via a render or blit pass must be done before another - // shader attempts to read the resource. - BarrierVK barrier; - barrier.cmd_buffer = buffer; - barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | - vk::AccessFlagBits::eTransferWrite; - barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | - vk::PipelineStageFlagBits::eTransfer; - barrier.dst_access = vk::AccessFlagBits::eShaderRead; - barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; - - barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; +// |RenderPass| +void RenderPassVK::SetPipeline( + const std::shared_ptr>& pipeline) { + PipelineVK& pipeline_vk = PipelineVK::Cast(*pipeline); - for (const TextureAndSampler& data : bindings.sampled_images) { - if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { - return false; - } + auto descriptor_result = + command_buffer_->GetEncoder()->AllocateDescriptorSets( + pipeline_vk.GetDescriptorSetLayout(), + ContextVK::Cast(*context_.lock())); + if (!descriptor_result.ok()) { + return; } - return true; -} - -static bool UpdateBindingLayouts(const Command& command, - const vk::CommandBuffer& buffer) { - return UpdateBindingLayouts(command.vertex_bindings, buffer) && - UpdateBindingLayouts(command.fragment_bindings, buffer); -} + pipeline_valid_ = true; + descriptor_set_ = descriptor_result.value(); + pipeline_layout_ = pipeline_vk.GetPipelineLayout(); + command_buffer_vk_.bindPipeline(vk::PipelineBindPoint::eGraphics, + pipeline_vk.GetPipeline()); -static bool UpdateBindingLayouts(const std::vector& commands, - const vk::CommandBuffer& buffer) { - for (const Command& command : commands) { - if (!UpdateBindingLayouts(command, buffer)) { - return false; + if (pipeline->GetDescriptor().UsesSubpassInput()) { + if (bound_image_offset_ >= kMaxBindings) { + pipeline_valid_ = false; + return; } - } - return true; -} -static void SetViewportAndScissor(const Command& command, - const vk::CommandBuffer& cmd_buffer, - PassBindingsCache& cmd_buffer_cache, - const ISize& target_size) { - // Set the viewport. - const auto& vp = command.viewport.value_or( - {.rect = Rect::MakeSize(target_size)}); - vk::Viewport viewport = vk::Viewport() - .setWidth(vp.rect.GetWidth()) - .setHeight(-vp.rect.GetHeight()) - .setY(vp.rect.GetHeight()) - .setMinDepth(0.0f) - .setMaxDepth(1.0f); - cmd_buffer_cache.SetViewport(cmd_buffer, 0, 1, &viewport); + vk::DescriptorImageInfo image_info; + image_info.imageLayout = vk::ImageLayout::eGeneral; + image_info.sampler = VK_NULL_HANDLE; + image_info.imageView = TextureVK::Cast(*color_image_vk_).GetImageView(); + image_workspace_[bound_image_offset_++] = image_info; - // Set the scissor rect. - const auto& sc = command.scissor.value_or(IRect::MakeSize(target_size)); - vk::Rect2D scissor = - vk::Rect2D() - .setOffset(vk::Offset2D(sc.GetX(), sc.GetY())) - .setExtent(vk::Extent2D(sc.GetWidth(), sc.GetHeight())); - cmd_buffer_cache.SetScissor(cmd_buffer, 0, 1, &scissor); + vk::WriteDescriptorSet write_set; + write_set.dstBinding = 64; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eInputAttachment; + write_set.pImageInfo = &image_workspace_[bound_image_offset_ - 1]; + + write_workspace_[descriptor_write_offset_++] = write_set; + } } -static bool EncodeCommand(const Context& context, - const Command& command, - CommandEncoderVK& encoder, - PassBindingsCache& command_buffer_cache, - const ISize& target_size, - const vk::DescriptorSet vk_desc_set, - const vk::CommandBuffer& cmd_buffer) { +// |RenderPass| +void RenderPassVK::SetCommandLabel(std::string_view label) { #ifdef IMPELLER_DEBUG - fml::ScopedCleanupClosure pop_marker( - [&encoder]() { encoder.PopDebugGroup(); }); - if (!command.label.empty()) { - encoder.PushDebugGroup(command.label.c_str()); - } else { - pop_marker.Release(); - } + std::string label_copy(label); + command_buffer_->GetEncoder()->PushDebugGroup(label_copy.c_str()); + has_label_ = true; #endif // IMPELLER_DEBUG +} - const PipelineVK& pipeline_vk = PipelineVK::Cast(*command.pipeline); +// |RenderPass| +void RenderPassVK::SetStencilReference(uint32_t value) { + command_buffer_vk_.setStencilReference( + vk::StencilFaceFlagBits::eVkStencilFrontAndBack, value); +} - 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 - ); +// |RenderPass| +void RenderPassVK::SetBaseVertex(uint64_t value) { + base_vertex_ = value; +} - command_buffer_cache.BindPipeline( - cmd_buffer, vk::PipelineBindPoint::eGraphics, pipeline_vk.GetPipeline()); +// |RenderPass| +void RenderPassVK::SetViewport(Viewport viewport) { + vk::Viewport viewport_vk = vk::Viewport() + .setWidth(viewport.rect.GetWidth()) + .setHeight(-viewport.rect.GetHeight()) + .setY(viewport.rect.GetHeight()) + .setMinDepth(0.0f) + .setMaxDepth(1.0f); + command_buffer_vk_.setViewport(0, 1, &viewport_vk); +} - // Set the viewport and scissors. - SetViewportAndScissor(command, cmd_buffer, command_buffer_cache, target_size); +// |RenderPass| +void RenderPassVK::SetScissor(IRect scissor) { + vk::Rect2D scissor_vk = + vk::Rect2D() + .setOffset(vk::Offset2D(scissor.GetX(), scissor.GetY())) + .setExtent(vk::Extent2D(scissor.GetWidth(), scissor.GetHeight())); + command_buffer_vk_.setScissor(0, 1, &scissor_vk); +} - // Set the stencil reference. - command_buffer_cache.SetStencilReference( - cmd_buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, - command.stencil_reference); +// |RenderPass| +void RenderPassVK::SetInstanceCount(size_t count) { + instance_count_ = count; +} - // Configure vertex and index and buffers for binding. - if (!command.vertex_buffer.vertex_buffer) { - VALIDATION_LOG << "Failed to acquire device buffer" - << " for vertex buffer view"; +// |RenderPass| +bool RenderPassVK::SetVertexBuffer(VertexBuffer buffer) { + vertex_count_ = buffer.vertex_count; + if (buffer.index_type == IndexType::kUnknown) { return false; } - auto& allocator = *context.GetResourceAllocator(); - const std::shared_ptr& vertex_buffer = - command.vertex_buffer.vertex_buffer.buffer; - - if (!encoder.Track(vertex_buffer)) { + if (!command_buffer_->GetEncoder()->Track(buffer.vertex_buffer.buffer)) { return false; } // Bind the vertex buffer. vk::Buffer vertex_buffer_handle = - DeviceBufferVK::Cast(*vertex_buffer).GetBuffer(); + DeviceBufferVK::Cast(*buffer.vertex_buffer.buffer).GetBuffer(); vk::Buffer vertex_buffers[] = {vertex_buffer_handle}; - vk::DeviceSize vertex_buffer_offsets[] = { - command.vertex_buffer.vertex_buffer.range.offset}; - cmd_buffer.bindVertexBuffers(0u, 1u, vertex_buffers, vertex_buffer_offsets); + vk::DeviceSize vertex_buffer_offsets[] = {buffer.vertex_buffer.range.offset}; - if (command.vertex_buffer.index_type != IndexType::kNone) { - // Bind the index buffer. - const BufferView& index_buffer_view = command.vertex_buffer.index_buffer; + command_buffer_vk_.bindVertexBuffers(0u, 1u, vertex_buffers, + vertex_buffer_offsets); + + // Bind the index buffer. + if (buffer.index_type != IndexType::kNone) { + has_index_buffer_ = true; + const BufferView& index_buffer_view = buffer.index_buffer; if (!index_buffer_view) { return false; } @@ -442,131 +490,183 @@ static bool EncodeCommand(const Context& context, return false; } - if (!encoder.Track(index_buffer)) { + if (!command_buffer_->GetEncoder()->Track(index_buffer)) { return false; } 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)); - - // Engage! - cmd_buffer.drawIndexed(command.vertex_buffer.vertex_count, // index count - command.instance_count, // instance count - 0u, // first index - command.base_vertex, // vertex offset - 0u // first instance - ); + command_buffer_vk_.bindIndexBuffer(index_buffer_handle, + index_buffer_view.range.offset, + ToVKIndexType(buffer.index_type)); } else { - cmd_buffer.draw(command.vertex_buffer.vertex_count, // vertex count - command.instance_count, // instance count - command.base_vertex, // vertex offset - 0u // first instance - ); + has_index_buffer_ = false; } return true; } -bool RenderPassVK::OnEncodeCommands(const Context& context) const { - TRACE_EVENT0("impeller", "RenderPassVK::OnEncodeCommands"); - if (!IsValid()) { - return false; +// |RenderPass| +fml::Status RenderPassVK::Draw() { + if (!pipeline_valid_) { + return fml::Status(fml::StatusCode::kCancelled, + "No valid pipeline is bound to the RenderPass."); } - const auto& vk_context = ContextVK::Cast(context); - - std::shared_ptr command_buffer = command_buffer_.lock(); - if (!command_buffer) { - VALIDATION_LOG << "Command buffer died before commands could be encoded."; - return false; + const ContextVK& context_vk = ContextVK::Cast(*context_.lock()); + for (auto i = 0u; i < descriptor_write_offset_; i++) { + write_workspace_[i].dstSet = descriptor_set_; } - const std::shared_ptr& encoder = - command_buffer->GetEncoder(); - if (!encoder) { - return false; + + context_vk.GetDevice().updateDescriptorSets(descriptor_write_offset_, + write_workspace_.data(), 0u, {}); + + command_buffer_vk_.bindDescriptorSets( + vk::PipelineBindPoint::eGraphics, // bind point + pipeline_layout_, // layout + 0, // first set + 1, // set count + &descriptor_set_, // sets + 0, // offset count + nullptr // offsets + ); + + if (has_index_buffer_) { + command_buffer_vk_.drawIndexed(vertex_count_, // index count + instance_count_, // instance count + 0u, // first index + base_vertex_, // vertex offset + 0u // first instance + ); + } else { + command_buffer_vk_.draw(vertex_count_, // vertex count + instance_count_, // instance count + base_vertex_, // vertex offset + 0u // first instance + ); } #ifdef IMPELLER_DEBUG - fml::ScopedCleanupClosure pop_marker( - [&encoder]() { encoder->PopDebugGroup(); }); - if (!debug_label_.empty()) { - encoder->PushDebugGroup(debug_label_.c_str()); - } else { - pop_marker.Release(); + if (has_label_) { + command_buffer_->GetEncoder()->PopDebugGroup(); } #endif // IMPELLER_DEBUG + has_label_ = false; + has_index_buffer_ = false; + bound_image_offset_ = 0u; + bound_buffer_offset_ = 0u; + descriptor_write_offset_ = 0u; + instance_count_ = 1u; + base_vertex_ = 0u; + vertex_count_ = 0u; + pipeline_valid_ = false; + return fml::Status(); +} + +// Vulkan Binding only cares about binding and set values. +// ShaderMetadata is unused and ignored. +bool RenderPassVK::BindResource(ShaderStage stage, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) { + return BindResource(slot.binding, std::move(view)); +} - vk::CommandBuffer cmd_buffer = encoder->GetCommandBuffer(); +bool RenderPassVK::BindResource( + ShaderStage stage, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view) { + return BindResource(slot.binding, std::move(view)); +} - if (!UpdateBindingLayouts(commands_, cmd_buffer)) { +bool RenderPassVK::BindResource(size_t binding, BufferView view) { + if (bound_buffer_offset_ >= kMaxBindings) { return false; } - render_target_.IterateAllAttachments( - [&encoder](const auto& attachment) -> bool { - encoder->Track(attachment.texture); - encoder->Track(attachment.resolve_texture); - return true; - }); + const std::shared_ptr& device_buffer = view.buffer; + auto buffer = DeviceBufferVK::Cast(*device_buffer).GetBuffer(); + if (!buffer) { + return false; + } - const auto& target_size = render_target_.GetRenderTargetSize(); + if (!command_buffer_->GetEncoder()->Track(device_buffer)) { + return false; + } - SharedHandleVK render_pass = CreateVKRenderPass( - vk_context, command_buffer, - vk_context.GetCapabilities()->SupportsFramebufferFetch()); - if (!render_pass) { - VALIDATION_LOG << "Could not create renderpass."; + uint32_t offset = view.range.offset; + + vk::DescriptorBufferInfo buffer_info; + buffer_info.buffer = buffer; + buffer_info.offset = offset; + buffer_info.range = view.range.length; + buffer_workspace_[bound_buffer_offset_++] = buffer_info; + + // TODO: currently this does not work for storage buffers. + // Add more binding metdata that can be used to record this. + + vk::WriteDescriptorSet write_set; + // write_set.dstSet = vk_desc_set; // The desc set will be filled in layer. + write_set.dstBinding = binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = ToVKDescriptorType(DescriptorType::kUniformBuffer); + write_set.pBufferInfo = &buffer_workspace_[bound_buffer_offset_ - 1]; + + write_workspace_[descriptor_write_offset_++] = write_set; + return true; +} + +bool RenderPassVK::BindResource(ShaderStage stage, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) { + if (bound_buffer_offset_ >= kMaxBindings) { return false; } + const TextureVK& texture_vk = TextureVK::Cast(*texture); + const SamplerVK& sampler_vk = SamplerVK::Cast(*sampler); - auto framebuffer = CreateVKFramebuffer(vk_context, *render_pass); - if (!framebuffer) { - VALIDATION_LOG << "Could not create framebuffer."; + // All previous writes via a render or blit pass must be done before another + // shader attempts to read the resource. + BarrierVK barrier; + barrier.cmd_buffer = command_buffer_vk_; + barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + barrier.dst_access = vk::AccessFlagBits::eShaderRead; + barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + + barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; + + if (!texture_vk.SetLayout(barrier)) { return false; } - if (!encoder->Track(framebuffer) || !encoder->Track(render_pass)) { + if (!command_buffer_->GetEncoder()->Track(texture) || + !command_buffer_->GetEncoder()->Track(sampler_vk.GetSharedSampler())) { return false; } - auto clear_values = GetVKClearValues(render_target_); + vk::DescriptorImageInfo image_info; + image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; + image_info.sampler = sampler_vk.GetSampler(); + image_info.imageView = texture_vk.GetImageView(); + image_workspace_[bound_image_offset_++] = image_info; - vk::RenderPassBeginInfo pass_info; - pass_info.renderPass = *render_pass; - pass_info.framebuffer = *framebuffer; - pass_info.renderArea.extent.width = static_cast(target_size.width); - pass_info.renderArea.extent.height = - static_cast(target_size.height); - pass_info.setClearValues(clear_values); + vk::WriteDescriptorSet write_set; + write_set.dstBinding = slot.binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = vk::DescriptorType::eCombinedImageSampler; + write_set.pImageInfo = &image_workspace_[bound_image_offset_ - 1]; - const TextureVK& color_image_vk = TextureVK::Cast( - *render_target_.GetColorAttachments().find(0u)->second.texture); - Allocator& allocator = *context.GetResourceAllocator(); - - { - TRACE_EVENT0("impeller", "EncodeRenderPassCommands"); - cmd_buffer.beginRenderPass(pass_info, vk::SubpassContents::eInline); - - fml::ScopedCleanupClosure end_render_pass( - [cmd_buffer]() { cmd_buffer.endRenderPass(); }); - - 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_set_result.value(), cmd_buffer)) { - return false; - } - } - } + write_workspace_[descriptor_write_offset_++] = write_set; + return true; +} +bool RenderPassVK::OnEncodeCommands(const Context& context) const { + command_buffer_->GetEncoder()->GetCommandBuffer().endRenderPass(); return true; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 2d31abd4019f6..6911716bf85f0 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -5,10 +5,9 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_RENDER_PASS_VK_H_ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_RENDER_PASS_VK_H_ -#include "flutter/fml/macros.h" +#include "impeller/core/buffer_view.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" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" @@ -25,20 +24,80 @@ class RenderPassVK final : public RenderPass { private: friend class CommandBufferVK; - std::weak_ptr command_buffer_; + std::shared_ptr command_buffer_; std::string debug_label_; bool is_valid_ = false; - mutable std::array image_workspace_; - mutable std::array buffer_workspace_; - mutable std::array + vk::CommandBuffer command_buffer_vk_; + std::array image_workspace_; + std::array buffer_workspace_; + std::array write_workspace_; - - mutable PassBindingsCache pass_bindings_cache_; + size_t bound_image_offset_ = 0u; + size_t bound_buffer_offset_ = 0u; + size_t descriptor_write_offset_ = 0u; + size_t instance_count_ = 1u; + size_t base_vertex_ = 0u; + size_t vertex_count_ = 0u; + std::shared_ptr color_image_vk_; + bool has_index_buffer_ = false; + bool has_label_ = false; + bool pipeline_valid_ = false; + vk::DescriptorSet descriptor_set_ = {}; + vk::PipelineLayout pipeline_layout_ = {}; RenderPassVK(const std::shared_ptr& context, const RenderTarget& target, - std::weak_ptr command_buffer); + std::shared_ptr command_buffer); + + // |RenderPass| + void SetPipeline( + const std::shared_ptr>& pipeline) override; + + // |RenderPass| + void SetCommandLabel(std::string_view label) override; + + // |RenderPass| + void SetStencilReference(uint32_t value) override; + + // |RenderPass| + void SetBaseVertex(uint64_t value) override; + + // |RenderPass| + void SetViewport(Viewport viewport) override; + + // |RenderPass| + void SetScissor(IRect scissor) override; + + // |RenderPass| + void SetInstanceCount(size_t count) override; + + // |RenderPass| + bool SetVertexBuffer(VertexBuffer buffer) override; + + // |RenderPass| + fml::Status Draw() override; + + // |ResourceBinder| + bool BindResource(ShaderStage stage, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) override; + + // |RenderPass| + bool BindResource(ShaderStage stage, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view) override; + + // |ResourceBinder| + bool BindResource(ShaderStage stage, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) override; + + bool BindResource(size_t binding, BufferView view); // |RenderPass| bool IsValid() const override; diff --git a/impeller/renderer/render_pass.h b/impeller/renderer/render_pass.h index c2bd755c58f9b..ba5b51bc98dab 100644 --- a/impeller/renderer/render_pass.h +++ b/impeller/renderer/render_pass.h @@ -46,21 +46,14 @@ class RenderPass : public ResourceBinder { void SetLabel(std::string label); - /// @brief Reserve [command_count] commands in the HAL command buffer. - /// - /// Note: this is not the native command buffer. - void ReserveCommands(size_t command_count) { - commands_.reserve(command_count); - } - //---------------------------------------------------------------------------- /// The pipeline to use for this command. - void SetPipeline( + virtual void SetPipeline( const std::shared_ptr>& pipeline); //---------------------------------------------------------------------------- /// The debugging label to use for the command. - void SetCommandLabel(std::string_view label); + virtual void SetCommandLabel(std::string_view label); //---------------------------------------------------------------------------- /// The reference value to use in stenciling operations. Stencil configuration @@ -69,9 +62,9 @@ class RenderPass : public ResourceBinder { /// @see `Pipeline` /// @see `PipelineDescriptor` /// - void SetStencilReference(uint32_t value); + virtual void SetStencilReference(uint32_t value); - void SetBaseVertex(uint64_t value); + virtual void SetBaseVertex(uint64_t value); //---------------------------------------------------------------------------- /// The viewport coordinates that the rasterizer linearly maps normalized @@ -79,14 +72,14 @@ class RenderPass : public ResourceBinder { /// If unset, the viewport is the size of the render target with a zero /// origin, znear=0, and zfar=1. /// - void SetViewport(Viewport viewport); + virtual void SetViewport(Viewport viewport); //---------------------------------------------------------------------------- /// The scissor rect to use for clipping writes to the render target. The /// scissor rect must lie entirely within the render target. /// If unset, no scissor is applied. /// - void SetScissor(IRect scissor); + virtual void SetScissor(IRect scissor); //---------------------------------------------------------------------------- /// The number of instances of the given set of vertices to render. Not all @@ -95,7 +88,7 @@ class RenderPass : public ResourceBinder { /// @warning Setting this to more than one will limit the availability of /// backends to use with this command. /// - void SetInstanceCount(size_t count); + virtual void SetInstanceCount(size_t count); //---------------------------------------------------------------------------- /// @brief Specify the vertex and index buffer to use for this command. @@ -105,28 +98,29 @@ class RenderPass : public ResourceBinder { /// /// @return returns if the binding was updated. /// - bool SetVertexBuffer(VertexBuffer buffer); + virtual bool SetVertexBuffer(VertexBuffer buffer); /// Record the currently pending command. - fml::Status Draw(); + virtual fml::Status Draw(); // |ResourceBinder| - bool BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, - const ShaderMetadata& metadata, - BufferView view) override; + virtual bool BindResource(ShaderStage stage, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) override; - bool BindResource(ShaderStage stage, - const ShaderUniformSlot& slot, - const std::shared_ptr& metadata, - BufferView view); + virtual bool BindResource( + ShaderStage stage, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view); // |ResourceBinder| - bool BindResource(ShaderStage stage, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - std::shared_ptr texture, - std::shared_ptr sampler) override; + virtual bool BindResource(ShaderStage stage, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) override; //---------------------------------------------------------------------------- /// @brief Encode the recorded commands to the underlying command buffer. diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index e36f6345c3adf..32dcb99db469e 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -1255,21 +1255,6 @@ TEST_P(RendererTest, StencilMask) { OpenPlaygroundHere(callback); } -TEST_P(RendererTest, CanPreAllocateCommands) { - auto context = GetContext(); - auto cmd_buffer = context->CreateCommandBuffer(); - auto render_target_cache = std::make_shared( - GetContext()->GetResourceAllocator()); - - auto render_target = - RenderTarget::CreateOffscreen(*context, *render_target_cache, {100, 100}); - auto render_pass = cmd_buffer->CreateRenderPass(render_target); - - render_pass->ReserveCommands(100u); - - EXPECT_EQ(render_pass->GetCommands().capacity(), 100u); -} - TEST_P(RendererTest, CanLookupRenderTargetProperties) { auto context = GetContext(); auto cmd_buffer = context->CreateCommandBuffer(); From 58b35b9249908310a3a239490a5403912870d511 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Jan 2024 19:05:31 -0800 Subject: [PATCH 03/13] Set layout on producer side, codegen descriptor type. --- impeller/compiler/code_gen_template.h | 2 +- impeller/compiler/reflector.cc | 6 ++ impeller/compiler/reflector.h | 1 + impeller/core/resource_binder.h | 2 + .../contents/runtime_effect_contents.cc | 12 ++-- impeller/entity/inline_pass_context.cc | 3 +- impeller/entity/inline_pass_context.h | 1 + .../backend/vulkan/binding_helpers_vk.cc | 66 ----------------- .../backend/vulkan/binding_helpers_vk.h | 11 --- .../renderer/backend/vulkan/context_vk.cc | 2 +- .../renderer/backend/vulkan/render_pass_vk.cc | 71 +++++++++++-------- .../renderer/backend/vulkan/render_pass_vk.h | 13 +++- .../renderer/backend/vulkan/texture_vk.cc | 18 +++++ impeller/renderer/command.cc | 3 + impeller/renderer/command.h | 3 + impeller/renderer/compute_command.cc | 3 + impeller/renderer/compute_command.h | 2 + impeller/renderer/render_pass.cc | 9 ++- impeller/renderer/render_pass.h | 7 ++ lib/gpu/render_pass.cc | 29 ++++---- 20 files changed, 134 insertions(+), 130 deletions(-) diff --git a/impeller/compiler/code_gen_template.h b/impeller/compiler/code_gen_template.h index 7927c71d2952c..dbef3017c0e41 100644 --- a/impeller/compiler/code_gen_template.h +++ b/impeller/compiler/code_gen_template.h @@ -161,7 +161,7 @@ struct {{camel_case(shader_name)}}{{camel_case(shader_stage)}}Shader { {% endfor %}) { return {{ proto.args.0.argument_name }}.BindResource({% for arg in proto.args %} {% if loop.is_first %} -{{to_shader_stage(shader_stage)}}, kResource{{ proto.name }}, kMetadata{{ proto.name }}, {% else %} +{{to_shader_stage(shader_stage)}}, {{ proto.descriptor_type }}, kResource{{ proto.name }}, kMetadata{{ proto.name }}, {% else %} std::move({{ arg.argument_name }}){% if not loop.is_last %}, {% endif %} {% endif %} {% endfor %}); diff --git a/impeller/compiler/reflector.cc b/impeller/compiler/reflector.cc index e01755bccc38f..cafa894256f4d 100644 --- a/impeller/compiler/reflector.cc +++ b/impeller/compiler/reflector.cc @@ -1308,6 +1308,7 @@ std::vector Reflector::ReflectBindPrototypes( auto& proto = prototypes.emplace_back(BindPrototype{}); proto.return_type = "bool"; proto.name = ToCamelCase(uniform_buffer.name); + proto.descriptor_type = "DescriptorType::kUniformBuffer"; { std::stringstream stream; stream << "Bind uniform buffer for resource named " << uniform_buffer.name @@ -1327,6 +1328,7 @@ std::vector Reflector::ReflectBindPrototypes( auto& proto = prototypes.emplace_back(BindPrototype{}); proto.return_type = "bool"; proto.name = ToCamelCase(storage_buffer.name); + proto.descriptor_type = "DescriptorType::kStorageBuffer"; { std::stringstream stream; stream << "Bind storage buffer for resource named " << storage_buffer.name @@ -1346,6 +1348,7 @@ std::vector Reflector::ReflectBindPrototypes( auto& proto = prototypes.emplace_back(BindPrototype{}); proto.return_type = "bool"; proto.name = ToCamelCase(sampled_image.name); + proto.descriptor_type = "DescriptorType::kSampledImage"; { std::stringstream stream; stream << "Bind combined image sampler for resource named " @@ -1369,6 +1372,7 @@ std::vector Reflector::ReflectBindPrototypes( auto& proto = prototypes.emplace_back(BindPrototype{}); proto.return_type = "bool"; proto.name = ToCamelCase(separate_image.name); + proto.descriptor_type = "DescriptorType::kImage"; { std::stringstream stream; stream << "Bind separate image for resource named " << separate_image.name @@ -1388,6 +1392,7 @@ std::vector Reflector::ReflectBindPrototypes( auto& proto = prototypes.emplace_back(BindPrototype{}); proto.return_type = "bool"; proto.name = ToCamelCase(separate_sampler.name); + proto.descriptor_type = "DescriptorType::kSampler"; { std::stringstream stream; stream << "Bind separate sampler for resource named " @@ -1416,6 +1421,7 @@ nlohmann::json::array_t Reflector::EmitBindPrototypes( item["return_type"] = res.return_type; item["name"] = res.name; item["docstring"] = res.docstring; + item["descriptor_type"] = res.descriptor_type; auto& args = item["args"] = nlohmann::json::array_t{}; for (const auto& arg : res.args) { auto& json_arg = args.emplace_back(nlohmann::json::object_t{}); diff --git a/impeller/compiler/reflector.h b/impeller/compiler/reflector.h index 3de086c00a553..1448e412d4738 100644 --- a/impeller/compiler/reflector.h +++ b/impeller/compiler/reflector.h @@ -187,6 +187,7 @@ class Reflector { std::string name; std::string return_type; std::string docstring; + std::string descriptor_type = ""; std::vector args; }; diff --git a/impeller/core/resource_binder.h b/impeller/core/resource_binder.h index 6a6172710d700..bd2dfea50508e 100644 --- a/impeller/core/resource_binder.h +++ b/impeller/core/resource_binder.h @@ -24,11 +24,13 @@ struct ResourceBinder { virtual ~ResourceBinder() = default; virtual bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) = 0; virtual bool BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/impeller/entity/contents/runtime_effect_contents.cc b/impeller/entity/contents/runtime_effect_contents.cc index 454ff2a5f25fc..0a0ca655aa86f 100644 --- a/impeller/entity/contents/runtime_effect_contents.cc +++ b/impeller/entity/contents/runtime_effect_contents.cc @@ -191,8 +191,9 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, ShaderUniformSlot uniform_slot; uniform_slot.name = uniform.name.c_str(); uniform_slot.ext_res_0 = uniform.location; - pass.BindResource(ShaderStage::kFragment, uniform_slot, metadata, - buffer_view); + pass.BindResource(ShaderStage::kFragment, + DescriptorType::kUniformBuffer, uniform_slot, + metadata, buffer_view); buffer_index++; buffer_offset += uniform.GetSize(); break; @@ -229,7 +230,8 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, auto buffer_view = renderer.GetTransientsBuffer().Emplace( reinterpret_cast(uniform_buffer.data()), sizeof(float) * uniform_buffer.size(), alignment); - pass.BindResource(ShaderStage::kFragment, uniform_slot, + pass.BindResource(ShaderStage::kFragment, + DescriptorType::kUniformBuffer, uniform_slot, ShaderMetadata{}, buffer_view); } } @@ -263,8 +265,8 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer, image_slot.binding = sampler_binding_location; image_slot.texture_index = uniform.location - minimum_sampler_index; - pass.BindResource(ShaderStage::kFragment, image_slot, *metadata, - input.texture, sampler); + pass.BindResource(ShaderStage::kFragment, DescriptorType::kSampledImage, + image_slot, *metadata, input.texture, sampler); sampler_index++; break; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 3201a014b6ea4..1a4589c24d32f 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -24,6 +24,7 @@ InlinePassContext::InlinePassContext( std::optional collapsed_parent_pass) : context_(std::move(context)), pass_target_(pass_target), + entity_count_(entity_count), is_collapsed_(collapsed_parent_pass.has_value()) { if (collapsed_parent_pass.has_value()) { pass_ = collapsed_parent_pass.value().pass; @@ -164,7 +165,7 @@ InlinePassContext::RenderPassResult InlinePassContext::GetRenderPass( // Commands are fairly large (500B) objects, so re-allocation of the command // buffer while encoding can add a surprising amount of overhead. We make a // conservative npot estimate to avoid this case. - // pass_->ReserveCommands(Allocation::NextPowerOfTwoSize(entity_count_)); + pass_->ReserveCommands(Allocation::NextPowerOfTwoSize(entity_count_)); pass_->SetLabel( "EntityPass Render Pass: Depth=" + std::to_string(pass_depth) + " Count=" + std::to_string(pass_count_)); diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index c852394f53cdd..595d291b5a1c8 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -50,6 +50,7 @@ class InlinePassContext { std::shared_ptr command_buffer_; std::shared_ptr pass_; uint32_t pass_count_ = 0; + uint32_t entity_count_ = 0; // Whether this context is collapsed into a parent entity pass. bool is_collapsed_ = false; diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc index 3bc11ba252784..92b30313f5f3a 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc @@ -20,11 +20,6 @@ namespace impeller { -// Warning: if any of the constant values or layouts are changed in the -// framebuffer fetch shader, then this input binding may need to be -// manually changed. -static constexpr size_t kMagicSubpassInputBinding = 64; - static bool BindImages( const Bindings& bindings, Allocator& allocator, @@ -125,67 +120,6 @@ static bool BindBuffers( return true; } -fml::StatusOr AllocateAndBindDescriptorSets( - const ContextVK& context, - const std::shared_ptr& encoder, - 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( - PipelineVK::Cast(*command.pipeline).GetDescriptorSetLayout(), context); - if (!descriptor_result.ok()) { - return descriptor_result.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."); - } - - 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; - - 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(write_offset, write_workspace.data(), - 0u, {}); - - return descriptor_set; -} - fml::StatusOr AllocateAndBindDescriptorSets( const ContextVK& context, const std::shared_ptr& encoder, diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.h b/impeller/renderer/backend/vulkan/binding_helpers_vk.h index 1dc7f32d2a4b6..be35567a77e3b 100644 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.h +++ b/impeller/renderer/backend/vulkan/binding_helpers_vk.h @@ -17,17 +17,6 @@ namespace impeller { // backend to avoid dynamic heap allocations. static constexpr size_t kMaxBindings = 32; -fml::StatusOr AllocateAndBindDescriptorSets( - const ContextVK& context, - const std::shared_ptr& encoder, - 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, diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index ee4df3534b180..6c1804dd74022 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -156,7 +156,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = false; + auto enable_validation = settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index a22d71bba4700..91934eba34729 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -12,6 +12,7 @@ #include "impeller/base/validation.h" #include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" +#include "impeller/core/texture.h" #include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" @@ -30,6 +31,11 @@ namespace impeller { +// Warning: if any of the constant values or layouts are changed in the +// framebuffer fetch shader, then this input binding may need to be +// manually changed. +static constexpr size_t kMagicSubpassInputBinding = 64; + static vk::ClearColorValue VKClearValueFromColor(Color color) { vk::ClearColorValue value; value.setFloat32( @@ -310,6 +316,8 @@ RenderPassVK::RenderPassVK(const std::shared_ptr& context, color_image_vk_ = render_target_.GetColorAttachments().find(0u)->second.texture; + resolve_image_vk_ = + render_target_.GetColorAttachments().find(0u)->second.resolve_texture; is_valid_ = true; } @@ -400,7 +408,7 @@ void RenderPassVK::SetPipeline( image_workspace_[bound_image_offset_++] = image_info; vk::WriteDescriptorSet write_set; - write_set.dstBinding = 64; + write_set.dstBinding = kMagicSubpassInputBinding; write_set.descriptorCount = 1u; write_set.descriptorType = vk::DescriptorType::eInputAttachment; write_set.pImageInfo = &image_workspace_[bound_image_offset_ - 1]; @@ -562,24 +570,28 @@ fml::Status RenderPassVK::Draw() { return fml::Status(); } -// Vulkan Binding only cares about binding and set values. -// ShaderMetadata is unused and ignored. +// The RenderPassVK binding methods only need the binding, set, and buffer type +// information. bool RenderPassVK::BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { - return BindResource(slot.binding, std::move(view)); + return BindResource(slot.binding, type, std::move(view)); } bool RenderPassVK::BindResource( ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) { - return BindResource(slot.binding, std::move(view)); + return BindResource(slot.binding, type, std::move(view)); } -bool RenderPassVK::BindResource(size_t binding, BufferView view) { +bool RenderPassVK::BindResource(size_t binding, + DescriptorType type, + BufferView view) { if (bound_buffer_offset_ >= kMaxBindings) { return false; } @@ -602,14 +614,10 @@ bool RenderPassVK::BindResource(size_t binding, BufferView view) { buffer_info.range = view.range.length; buffer_workspace_[bound_buffer_offset_++] = buffer_info; - // TODO(jonahwilliams): currently this does not work for storage buffers. - // Add more binding metdata that can be used to record this. - vk::WriteDescriptorSet write_set; - // write_set.dstSet = vk_desc_set; // The desc set will be filled in layer. write_set.dstBinding = binding; write_set.descriptorCount = 1u; - write_set.descriptorType = ToVKDescriptorType(DescriptorType::kUniformBuffer); + write_set.descriptorType = ToVKDescriptorType(type); write_set.pBufferInfo = &buffer_workspace_[bound_buffer_offset_ - 1]; write_workspace_[descriptor_write_offset_++] = write_set; @@ -617,6 +625,7 @@ bool RenderPassVK::BindResource(size_t binding, BufferView view) { } bool RenderPassVK::BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, @@ -627,23 +636,6 @@ bool RenderPassVK::BindResource(ShaderStage stage, const TextureVK& texture_vk = TextureVK::Cast(*texture); const SamplerVK& sampler_vk = SamplerVK::Cast(*sampler); - // All previous writes via a render or blit pass must be done before another - // shader attempts to read the resource. - BarrierVK barrier; - barrier.cmd_buffer = command_buffer_vk_; - barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | - vk::AccessFlagBits::eTransferWrite; - barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | - vk::PipelineStageFlagBits::eTransfer; - barrier.dst_access = vk::AccessFlagBits::eShaderRead; - barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; - - barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; - - if (!texture_vk.SetLayout(barrier)) { - return false; - } - if (!command_buffer_->GetEncoder()->Track(texture) || !command_buffer_->GetEncoder()->Track(sampler_vk.GetSharedSampler())) { return false; @@ -667,6 +659,29 @@ bool RenderPassVK::BindResource(ShaderStage stage, bool RenderPassVK::OnEncodeCommands(const Context& context) const { command_buffer_->GetEncoder()->GetCommandBuffer().endRenderPass(); + + // If this render target will be consumed by a subsequent render pass, + // perform a layout transition to a shader read state. + const std::shared_ptr& result_texture = + resolve_image_vk_ ? resolve_image_vk_ : color_image_vk_; + if (result_texture->GetTextureDescriptor().usage & + static_cast(TextureUsage::kShaderRead)) { + BarrierVK barrier; + barrier.cmd_buffer = command_buffer_vk_; + barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + barrier.dst_access = vk::AccessFlagBits::eShaderRead; + barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + + barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; + + if (!TextureVK::Cast(*result_texture).SetLayout(barrier)) { + return false; + } + } + return true; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 6911716bf85f0..f6ff36dea727f 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -29,6 +29,10 @@ class RenderPassVK final : public RenderPass { bool is_valid_ = false; vk::CommandBuffer command_buffer_vk_; + std::shared_ptr color_image_vk_; + std::shared_ptr resolve_image_vk_; + + // Per-command state. std::array image_workspace_; std::array buffer_workspace_; std::array @@ -39,7 +43,6 @@ class RenderPassVK final : public RenderPass { size_t instance_count_ = 1u; size_t base_vertex_ = 0u; size_t vertex_count_ = 0u; - std::shared_ptr color_image_vk_; bool has_index_buffer_ = false; bool has_label_ = false; bool pipeline_valid_ = false; @@ -78,26 +81,32 @@ class RenderPassVK final : public RenderPass { // |RenderPass| fml::Status Draw() override; + // |RenderPass| + void ReserveCommands(size_t command_count) override {} + // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) override; // |RenderPass| bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) override; // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) override; - bool BindResource(size_t binding, BufferView view); + bool BindResource(size_t binding, DescriptorType type, BufferView view); // |RenderPass| bool IsValid() const override; diff --git a/impeller/renderer/backend/vulkan/texture_vk.cc b/impeller/renderer/backend/vulkan/texture_vk.cc index cbe525400c303..36f5d3878fe91 100644 --- a/impeller/renderer/backend/vulkan/texture_vk.cc +++ b/impeller/renderer/backend/vulkan/texture_vk.cc @@ -107,6 +107,24 @@ bool TextureVK::OnSetContents(const uint8_t* contents, © // regions ); + // Transition to shader-read. + { + BarrierVK barrier; + barrier.cmd_buffer = vk_cmd_buffer; + barrier.src_access = vk::AccessFlagBits::eColorAttachmentWrite | + vk::AccessFlagBits::eTransferWrite; + barrier.src_stage = vk::PipelineStageFlagBits::eColorAttachmentOutput | + vk::PipelineStageFlagBits::eTransfer; + barrier.dst_access = vk::AccessFlagBits::eShaderRead; + barrier.dst_stage = vk::PipelineStageFlagBits::eFragmentShader; + + barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; + + if (!SetLayout(barrier)) { + return false; + } + } + return cmd_buffer->SubmitCommands(); } diff --git a/impeller/renderer/command.cc b/impeller/renderer/command.cc index 40dfd8c09d518..6507c81d565c0 100644 --- a/impeller/renderer/command.cc +++ b/impeller/renderer/command.cc @@ -23,6 +23,7 @@ bool Command::BindVertices(VertexBuffer buffer) { } bool Command::BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { @@ -31,6 +32,7 @@ bool Command::BindResource(ShaderStage stage, bool Command::BindResource( ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) { @@ -66,6 +68,7 @@ bool Command::DoBindResource(ShaderStage stage, } bool Command::BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/impeller/renderer/command.h b/impeller/renderer/command.h index 0e27d74750a71..0df30895b7959 100644 --- a/impeller/renderer/command.h +++ b/impeller/renderer/command.h @@ -162,17 +162,20 @@ struct Command : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) override; bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view); // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc index fbbfe3b58666f..e1a06015fa611 100644 --- a/impeller/renderer/compute_command.cc +++ b/impeller/renderer/compute_command.cc @@ -8,10 +8,12 @@ #include "impeller/base/validation.h" #include "impeller/core/formats.h" +#include "impeller/core/shader_types.h" namespace impeller { bool ComputeCommand::BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { @@ -29,6 +31,7 @@ bool ComputeCommand::BindResource(ShaderStage stage, } bool ComputeCommand::BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/impeller/renderer/compute_command.h b/impeller/renderer/compute_command.h index fb2639df664e8..c0c56e9f698f6 100644 --- a/impeller/renderer/compute_command.h +++ b/impeller/renderer/compute_command.h @@ -53,12 +53,14 @@ struct ComputeCommand : public ResourceBinder { // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) override; // |ResourceBinder| bool BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/impeller/renderer/render_pass.cc b/impeller/renderer/render_pass.cc index 10c5a16556117..8c2880aaf6b02 100644 --- a/impeller/renderer/render_pass.cc +++ b/impeller/renderer/render_pass.cc @@ -136,27 +136,30 @@ fml::Status RenderPass::Draw() { // |ResourceBinder| bool RenderPass::BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { - return pending_.BindResource(stage, slot, metadata, view); + return pending_.BindResource(stage, type, slot, metadata, view); } bool RenderPass::BindResource( ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) { - return pending_.BindResource(stage, slot, metadata, std::move(view)); + return pending_.BindResource(stage, type, slot, metadata, std::move(view)); } // |ResourceBinder| bool RenderPass::BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, std::shared_ptr sampler) { - return pending_.BindResource(stage, slot, metadata, std::move(texture), + return pending_.BindResource(stage, type, slot, metadata, std::move(texture), std::move(sampler)); } diff --git a/impeller/renderer/render_pass.h b/impeller/renderer/render_pass.h index ba5b51bc98dab..52294163cddc5 100644 --- a/impeller/renderer/render_pass.h +++ b/impeller/renderer/render_pass.h @@ -46,6 +46,10 @@ class RenderPass : public ResourceBinder { void SetLabel(std::string label); + virtual void ReserveCommands(size_t command_count) { + commands_.reserve(command_count); + } + //---------------------------------------------------------------------------- /// The pipeline to use for this command. virtual void SetPipeline( @@ -105,18 +109,21 @@ class RenderPass : public ResourceBinder { // |ResourceBinder| virtual bool BindResource(ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) override; virtual bool BindResource( ShaderStage stage, + DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view); // |ResourceBinder| virtual bool BindResource(ShaderStage stage, + DescriptorType type, const SampledImageSlot& slot, const ShaderMetadata& metadata, std::shared_ptr texture, diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 35a43793fb637..70e75e210ada6 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -151,23 +151,27 @@ bool RenderPass::Draw() { } render_pass_->SetVertexBuffer(result.vertex_buffer); for (const auto& buffer : result.vertex_bindings.buffers) { - render_pass_->BindResource(impeller::ShaderStage::kVertex, buffer.slot, - *buffer.view.GetMetadata(), + render_pass_->BindResource(impeller::ShaderStage::kVertex, + impeller::DescriptorType::kUniformBuffer, + buffer.slot, *buffer.view.GetMetadata(), buffer.view.resource); } for (const auto& texture : result.vertex_bindings.sampled_images) { - render_pass_->BindResource(impeller::ShaderStage::kVertex, texture.slot, - *texture.texture.GetMetadata(), + render_pass_->BindResource(impeller::ShaderStage::kVertex, + impeller::DescriptorType::kUniformBuffer, + texture.slot, *texture.texture.GetMetadata(), texture.texture.resource, texture.sampler); } for (const auto& buffer : result.fragment_bindings.buffers) { - render_pass_->BindResource(impeller::ShaderStage::kFragment, buffer.slot, - *buffer.view.GetMetadata(), + render_pass_->BindResource(impeller::ShaderStage::kFragment, + impeller::DescriptorType::kSampledImage, + buffer.slot, *buffer.view.GetMetadata(), buffer.view.resource); } for (const auto& texture : result.fragment_bindings.sampled_images) { - render_pass_->BindResource(impeller::ShaderStage::kFragment, texture.slot, - *texture.texture.GetMetadata(), + render_pass_->BindResource(impeller::ShaderStage::kFragment, + impeller::DescriptorType::kSampledImage, + texture.slot, *texture.texture.GetMetadata(), texture.texture.resource, texture.sampler); } return render_pass_->Draw().ok(); @@ -377,7 +381,8 @@ static bool BindUniform(flutter::gpu::RenderPass* wrapper, } return command.BindResource( - shader->GetShaderStage(), uniform_struct->slot, uniform_struct->metadata, + shader->GetShaderStage(), impeller::DescriptorType::kUniformBuffer, + uniform_struct->slot, uniform_struct->metadata, impeller::BufferView{ .buffer = buffer, .range = impeller::Range(offset_in_bytes, length_in_bytes), @@ -446,9 +451,9 @@ bool InternalFlutterGpu_RenderPass_BindTexture( auto sampler = wrapper->GetContext().lock()->GetSamplerLibrary()->GetSampler( sampler_desc); - return command.BindResource(shader->GetShaderStage(), *image_slot, - impeller::ShaderMetadata{}, texture->GetTexture(), - sampler); + return command.BindResource( + shader->GetShaderStage(), impeller::DescriptorType::kSampledImage, + *image_slot, impeller::ShaderMetadata{}, texture->GetTexture(), sampler); } void InternalFlutterGpu_RenderPass_ClearBindings( From c554219f9917c1a52c0ab77e8f1f9b9c2278331d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Jan 2024 20:41:11 -0800 Subject: [PATCH 04/13] fix gpu tracer maybe --- impeller/renderer/backend/vulkan/gpu_tracer_vk.cc | 15 +++++---------- .../renderer/backend/vulkan/render_pass_vk.cc | 6 +++--- impeller/renderer/backend/vulkan/render_pass_vk.h | 4 +++- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 964af7b2a167d..45c57ae5e7562 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -8,6 +8,7 @@ #include #include #include + #include "fml/logging.h" #include "fml/trace_event.h" #include "impeller/base/validation.h" @@ -28,7 +29,7 @@ GPUTracerVK::GPUTracerVK(const std::shared_ptr& device_holder) return; } // Disable tracing in release mode. - enabled_ = false; + enabled_ = true; } bool GPUTracerVK::IsEnabled() const { @@ -103,16 +104,14 @@ void GPUTracerVK::RecordCmdBufferStart(const vk::CommandBuffer& buffer, trace_states_[current_state_].query_pool.get(), state.current_index); state.current_index += 1; - if (!probe.index_.has_value()) { - state.pending_buffers += 1; - probe.index_ = current_state_; - } + probe.index_ = current_state_; + state.pending_buffers += 1; } void GPUTracerVK::RecordCmdBufferEnd(const vk::CommandBuffer& buffer, GPUProbe& probe) { if (!enabled_ || std::this_thread::get_id() != raster_thread_id_ || - !in_frame_) { + !in_frame_ || !probe.index_.has_value()) { return; } Lock lock(trace_state_mutex_); @@ -126,10 +125,6 @@ void GPUTracerVK::RecordCmdBufferEnd(const vk::CommandBuffer& buffer, state.query_pool.get(), state.current_index); state.current_index += 1; - if (!probe.index_.has_value()) { - state.pending_buffers += 1; - probe.index_ = current_state_; - } } void GPUTracerVK::OnFenceComplete(size_t frame_index) { diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 91934eba34729..c53b65adbf0e9 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -577,7 +577,7 @@ bool RenderPassVK::BindResource(ShaderStage stage, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { - return BindResource(slot.binding, type, std::move(view)); + return BindResource(slot.binding, type, view); } bool RenderPassVK::BindResource( @@ -586,12 +586,12 @@ bool RenderPassVK::BindResource( const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) { - return BindResource(slot.binding, type, std::move(view)); + return BindResource(slot.binding, type, view); } bool RenderPassVK::BindResource(size_t binding, DescriptorType type, - BufferView view) { + const BufferView& view) { if (bound_buffer_offset_ >= kMaxBindings) { return false; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index f6ff36dea727f..370518f845107 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -106,7 +106,9 @@ class RenderPassVK final : public RenderPass { std::shared_ptr texture, std::shared_ptr sampler) override; - bool BindResource(size_t binding, DescriptorType type, BufferView view); + bool BindResource(size_t binding, + DescriptorType type, + const BufferView& view); // |RenderPass| bool IsValid() const override; From 3bb9e526ac2d380a66c38486ad5182aa768763a6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 12 Jan 2024 20:41:11 -0800 Subject: [PATCH 05/13] fix gpu tracer maybe --- .../tiled_texture_contents_unittests.cc | 3 +++ impeller/renderer/backend/vulkan/context_vk.cc | 2 +- .../renderer/backend/vulkan/gpu_tracer_vk.cc | 17 +++++++---------- .../renderer/backend/vulkan/render_pass_vk.cc | 6 +++--- .../renderer/backend/vulkan/render_pass_vk.h | 4 +++- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 4715eaad79133..0306705dcf57a 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -17,6 +17,9 @@ namespace testing { using EntityTest = EntityPlayground; TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { + if (GetParam() == PlaygroundBackend::kVulkan) { + GTEST_SKIP_("Vulkan does not support querying recorded commands."); + } TextureDescriptor texture_desc; texture_desc.size = {100, 100}; texture_desc.type = TextureType::kTexture2D; diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 6c1804dd74022..c097b232de3a1 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -156,7 +156,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = settings.enable_validation; + auto enable_validation = false; // settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; diff --git a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc index 964af7b2a167d..4867c9626c941 100644 --- a/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc +++ b/impeller/renderer/backend/vulkan/gpu_tracer_vk.cc @@ -8,6 +8,7 @@ #include #include #include + #include "fml/logging.h" #include "fml/trace_event.h" #include "impeller/base/validation.h" @@ -28,7 +29,9 @@ GPUTracerVK::GPUTracerVK(const std::shared_ptr& device_holder) return; } // Disable tracing in release mode. - enabled_ = false; +#ifdef IMPELLER_DEBUG + enabled_ = true; +#endif // IMPELLER_DEBUG } bool GPUTracerVK::IsEnabled() const { @@ -103,16 +106,14 @@ void GPUTracerVK::RecordCmdBufferStart(const vk::CommandBuffer& buffer, trace_states_[current_state_].query_pool.get(), state.current_index); state.current_index += 1; - if (!probe.index_.has_value()) { - state.pending_buffers += 1; - probe.index_ = current_state_; - } + probe.index_ = current_state_; + state.pending_buffers += 1; } void GPUTracerVK::RecordCmdBufferEnd(const vk::CommandBuffer& buffer, GPUProbe& probe) { if (!enabled_ || std::this_thread::get_id() != raster_thread_id_ || - !in_frame_) { + !in_frame_ || !probe.index_.has_value()) { return; } Lock lock(trace_state_mutex_); @@ -126,10 +127,6 @@ void GPUTracerVK::RecordCmdBufferEnd(const vk::CommandBuffer& buffer, state.query_pool.get(), state.current_index); state.current_index += 1; - if (!probe.index_.has_value()) { - state.pending_buffers += 1; - probe.index_ = current_state_; - } } void GPUTracerVK::OnFenceComplete(size_t frame_index) { diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index 91934eba34729..c53b65adbf0e9 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -577,7 +577,7 @@ bool RenderPassVK::BindResource(ShaderStage stage, const ShaderUniformSlot& slot, const ShaderMetadata& metadata, BufferView view) { - return BindResource(slot.binding, type, std::move(view)); + return BindResource(slot.binding, type, view); } bool RenderPassVK::BindResource( @@ -586,12 +586,12 @@ bool RenderPassVK::BindResource( const ShaderUniformSlot& slot, const std::shared_ptr& metadata, BufferView view) { - return BindResource(slot.binding, type, std::move(view)); + return BindResource(slot.binding, type, view); } bool RenderPassVK::BindResource(size_t binding, DescriptorType type, - BufferView view) { + const BufferView& view) { if (bound_buffer_offset_ >= kMaxBindings) { return false; } diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index f6ff36dea727f..370518f845107 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -106,7 +106,9 @@ class RenderPassVK final : public RenderPass { std::shared_ptr texture, std::shared_ptr sampler) override; - bool BindResource(size_t binding, DescriptorType type, BufferView view); + bool BindResource(size_t binding, + DescriptorType type, + const BufferView& view); // |RenderPass| bool IsValid() const override; From 2a677115370953461f7d0e2ed1d58a7ec1a73c38 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jan 2024 18:56:37 -0800 Subject: [PATCH 06/13] capture drawing commands in debug mode. --- impeller/aiks/BUILD.gn | 2 + impeller/aiks/testing/context_spy.cc | 21 ++-- impeller/aiks/testing/context_spy.h | 4 +- .../aiks/testing/recording_render_pass.cc | 113 ++++++++++++++++++ impeller/aiks/testing/recording_render_pass.h | 87 ++++++++++++++ .../renderer/backend/vulkan/render_pass_vk.h | 2 + impeller/renderer/render_pass.h | 2 +- 7 files changed, 221 insertions(+), 10 deletions(-) create mode 100644 impeller/aiks/testing/recording_render_pass.cc create mode 100644 impeller/aiks/testing/recording_render_pass.h diff --git a/impeller/aiks/BUILD.gn b/impeller/aiks/BUILD.gn index f1d7d8ef681ac..d80b286e61342 100644 --- a/impeller/aiks/BUILD.gn +++ b/impeller/aiks/BUILD.gn @@ -74,6 +74,8 @@ impeller_component("context_spy") { "testing/context_mock.h", "testing/context_spy.cc", "testing/context_spy.h", + "testing/recording_render_pass.cc", + "testing/recording_render_pass.h", ] deps = [ "//flutter/impeller/renderer", diff --git a/impeller/aiks/testing/context_spy.cc b/impeller/aiks/testing/context_spy.cc index e831719cd3008..e6da5fa1a9055 100644 --- a/impeller/aiks/testing/context_spy.cc +++ b/impeller/aiks/testing/context_spy.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include "impeller/aiks/testing/context_spy.h" +#include +#include "impeller/aiks/testing/recording_render_pass.h" namespace impeller { namespace testing { @@ -64,14 +66,17 @@ std::shared_ptr ContextSpy::MakeContext( }); ON_CALL(*spy, OnCreateRenderPass) - .WillByDefault( - [real_buffer, shared_this](const RenderTarget& render_target) { - std::shared_ptr result = - CommandBufferMock::ForwardOnCreateRenderPass( - real_buffer.get(), render_target); - shared_this->render_passes_.push_back(result); - return result; - }); + .WillByDefault([real_buffer, shared_this, + real_context](const RenderTarget& render_target) { + std::shared_ptr result = + CommandBufferMock::ForwardOnCreateRenderPass( + real_buffer.get(), render_target); + std::shared_ptr recorder = + std::make_shared(result, real_context, + render_target); + shared_this->render_passes_.push_back(recorder); + return recorder; + }); ON_CALL(*spy, OnCreateBlitPass).WillByDefault([real_buffer]() { return CommandBufferMock::ForwardOnCreateBlitPass(real_buffer.get()); diff --git a/impeller/aiks/testing/context_spy.h b/impeller/aiks/testing/context_spy.h index 28a5ec3907ae4..91314f77c32fe 100644 --- a/impeller/aiks/testing/context_spy.h +++ b/impeller/aiks/testing/context_spy.h @@ -6,7 +6,9 @@ #define FLUTTER_IMPELLER_AIKS_TESTING_CONTEXT_SPY_H_ #include + #include "impeller/aiks/testing/context_mock.h" +#include "impeller/aiks/testing/recording_render_pass.h" namespace impeller { namespace testing { @@ -20,7 +22,7 @@ class ContextSpy : public std::enable_shared_from_this { std::shared_ptr MakeContext( const std::shared_ptr& real_context); - std::vector> render_passes_; + std::vector> render_passes_; private: ContextSpy() = default; diff --git a/impeller/aiks/testing/recording_render_pass.cc b/impeller/aiks/testing/recording_render_pass.cc new file mode 100644 index 0000000000000..0b1960091584c --- /dev/null +++ b/impeller/aiks/testing/recording_render_pass.cc @@ -0,0 +1,113 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "impeller/aiks/testing/recording_render_pass.h" + +#include + +namespace impeller { + +RecordingRenderPass::RecordingRenderPass(std::shared_ptr delegate, + const std::weak_ptr& context, + const RenderTarget& render_target) + : RenderPass(context, render_target), delegate_(std::move(delegate)) {} + +// |RenderPass| +void RecordingRenderPass::SetPipeline( + const std::shared_ptr>& pipeline) { + pending_.pipeline = pipeline; + delegate_->SetPipeline(pipeline); +} + +void RecordingRenderPass::SetCommandLabel(std::string_view label) { + pending_.label = std::string(label); + delegate_->SetCommandLabel(label); +} + +// |RenderPass| +void RecordingRenderPass::SetStencilReference(uint32_t value) { + pending_.stencil_reference = value; + delegate_->SetStencilReference(value); +} + +// |RenderPass| +void RecordingRenderPass::SetBaseVertex(uint64_t value) { + pending_.base_vertex = value; + delegate_->SetBaseVertex(value); +} + +// |RenderPass| +void RecordingRenderPass::SetViewport(Viewport viewport) { + pending_.viewport = viewport; + delegate_->SetViewport(viewport); +} + +// |RenderPass| +void RecordingRenderPass::SetScissor(IRect scissor) { + pending_.scissor = scissor; + delegate_->SetScissor(scissor); +} + +// |RenderPass| +void RecordingRenderPass::SetInstanceCount(size_t count) { + pending_.instance_count = count; + delegate_->SetInstanceCount(count); +} + +// |RenderPass| +bool RecordingRenderPass::SetVertexBuffer(VertexBuffer buffer) { + pending_.vertex_buffer = buffer; + return delegate_->SetVertexBuffer(buffer); +} + +// |RenderPass| +fml::Status RecordingRenderPass::Draw() { + commands_.emplace_back(std::move(pending_)); + pending_ = {}; + return delegate_->Draw(); +} + +// |RenderPass| +void RecordingRenderPass::OnSetLabel(std::string label) { + return; +} + +// |RenderPass| +bool RecordingRenderPass::OnEncodeCommands(const Context& context) const { + return true; +} + +// |RenderPass| +bool RecordingRenderPass::BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) { + pending_.BindResource(stage, type, slot, metadata, view); + return delegate_->BindResource(stage, type, slot, metadata, view); +} + +// |RenderPass| +bool RecordingRenderPass::BindResource( + ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view) { + pending_.BindResource(stage, type, slot, metadata, view); + return delegate_->BindResource(stage, type, slot, metadata, view); +} + +// |RenderPass| +bool RecordingRenderPass::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) { + pending_.BindResource(stage, type, slot, metadata, texture, sampler); + return delegate_->BindResource(stage, type, slot, metadata, texture, sampler); +} + +} // namespace impeller \ No newline at end of file diff --git a/impeller/aiks/testing/recording_render_pass.h b/impeller/aiks/testing/recording_render_pass.h new file mode 100644 index 0000000000000..3fa25b37c495e --- /dev/null +++ b/impeller/aiks/testing/recording_render_pass.h @@ -0,0 +1,87 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ +#define FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ + +#include "impeller/renderer/render_pass.h" + +namespace impeller { + +class RecordingRenderPass : public RenderPass { + public: + explicit RecordingRenderPass(std::shared_ptr delegate, + const std::weak_ptr& context, + const RenderTarget& render_target); + + ~RecordingRenderPass() = default; + + const std::vector& GetCommands() const override { return commands_; } + + // |RenderPass| + void SetPipeline( + const std::shared_ptr>& pipeline) override; + + void SetCommandLabel(std::string_view label) override; + + // |RenderPass| + void SetStencilReference(uint32_t value) override; + + // |RenderPass| + void SetBaseVertex(uint64_t value) override; + + // |RenderPass| + void SetViewport(Viewport viewport) override; + + // |RenderPass| + void SetScissor(IRect scissor) override; + + // |RenderPass| + void SetInstanceCount(size_t count) override; + + // |RenderPass| + bool SetVertexBuffer(VertexBuffer buffer) override; + + // |RenderPass| + fml::Status Draw(); + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) override; + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view); + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) override; + + // |RenderPass| + void OnSetLabel(std::string label) override; + + // |RenderPass| + bool OnEncodeCommands(const Context& context) const override; + + bool IsValid() const override { return true; } + + private: + Command pending_; + std::shared_ptr delegate_; + std::vector commands_; +}; + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index 370518f845107..26c635c41c6fe 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -8,6 +8,7 @@ #include "impeller/core/buffer_view.h" #include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" +#include "impeller/renderer/backend/vulkan/pipeline_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" @@ -46,6 +47,7 @@ class RenderPassVK final : public RenderPass { bool has_index_buffer_ = false; bool has_label_ = false; bool pipeline_valid_ = false; + vk::Pipeline last_pipeline_; vk::DescriptorSet descriptor_set_ = {}; vk::PipelineLayout pipeline_layout_ = {}; diff --git a/impeller/renderer/render_pass.h b/impeller/renderer/render_pass.h index 52294163cddc5..8bcc5f58255a9 100644 --- a/impeller/renderer/render_pass.h +++ b/impeller/renderer/render_pass.h @@ -142,7 +142,7 @@ class RenderPass : public ResourceBinder { /// /// @details Visible for testing. /// - const std::vector& GetCommands() const { return commands_; } + virtual const std::vector& GetCommands() const { return commands_; } //---------------------------------------------------------------------------- /// @brief The sample count of the attached render target. From 9964586b3277dbd15e8609483568c51ecf0f9739 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jan 2024 20:54:02 -0800 Subject: [PATCH 07/13] use more recording pass. --- impeller/aiks/testing/recording_render_pass.cc | 2 ++ .../entity/contents/checkerboard_contents_unittests.cc | 10 +++++++--- .../contents/tiled_texture_contents_unittests.cc | 10 +++++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/impeller/aiks/testing/recording_render_pass.cc b/impeller/aiks/testing/recording_render_pass.cc index 0b1960091584c..02f9360fab5ff 100644 --- a/impeller/aiks/testing/recording_render_pass.cc +++ b/impeller/aiks/testing/recording_render_pass.cc @@ -21,7 +21,9 @@ void RecordingRenderPass::SetPipeline( } void RecordingRenderPass::SetCommandLabel(std::string_view label) { +#ifdef IMPELLER_DEBUG pending_.label = std::string(label); +#endif // IMPELLER_DEBUG delegate_->SetCommandLabel(label); } diff --git a/impeller/entity/contents/checkerboard_contents_unittests.cc b/impeller/entity/contents/checkerboard_contents_unittests.cc index 63a2cbdd3aa07..ed21255e759ed 100644 --- a/impeller/entity/contents/checkerboard_contents_unittests.cc +++ b/impeller/entity/contents/checkerboard_contents_unittests.cc @@ -7,6 +7,7 @@ #include "gtest/gtest.h" +#include "impeller/aiks/testing/recording_render_pass.h" #include "impeller/entity/contents/checkerboard_contents.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/entity.h" @@ -38,11 +39,14 @@ TEST_P(EntityTest, RendersWithoutError) { *GetContentContext()->GetRenderTargetCache(), {100, 100}, /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); + auto recording_pass = std::make_shared( + render_pass, GetContext(), render_target); + Entity entity; - ASSERT_TRUE(render_pass->GetCommands().empty()); - ASSERT_TRUE(contents->Render(*content_context, entity, *render_pass)); - ASSERT_FALSE(render_pass->GetCommands().empty()); + ASSERT_TRUE(recording_pass->GetCommands().empty()); + ASSERT_TRUE(contents->Render(*content_context, entity, *recording_pass)); + ASSERT_FALSE(recording_pass->GetCommands().empty()); } #endif // IMPELLER_DEBUG diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index 0306705dcf57a..b42dd0eaff77e 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -4,6 +4,7 @@ #include +#include "impeller/aiks/testing/recording_render_pass.h" #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" #include "impeller/entity/contents/tiled_texture_contents.h" @@ -17,9 +18,6 @@ namespace testing { using EntityTest = EntityPlayground; TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { - if (GetParam() == PlaygroundBackend::kVulkan) { - GTEST_SKIP_("Vulkan does not support querying recorded commands."); - } TextureDescriptor texture_desc; texture_desc.size = {100, 100}; texture_desc.type = TextureType::kTexture2D; @@ -39,9 +37,11 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { *GetContentContext()->GetRenderTargetCache(), {100, 100}, /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); + auto recording_pass = std::make_shared( + render_pass, GetContext(), render_target); - ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *render_pass)); - const std::vector& commands = render_pass->GetCommands(); + ASSERT_TRUE(contents.Render(*GetContentContext(), {}, *recording_pass)); + const std::vector& commands = recording_pass->GetCommands(); ASSERT_EQ(commands.size(), 1u); ASSERT_STREQ(commands[0].pipeline->GetDescriptor().GetLabel().c_str(), From e7ff38ae00b6fbd6a8f1b5f5063e267b76c7298d Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jan 2024 21:13:52 -0800 Subject: [PATCH 08/13] move recording render pass to contents. --- impeller/aiks/BUILD.gn | 3 +-- impeller/aiks/testing/context_spy.cc | 4 ++-- impeller/aiks/testing/context_spy.h | 2 +- impeller/entity/BUILD.gn | 2 ++ .../entity/contents/checkerboard_contents_unittests.cc | 2 +- .../contents/test}/recording_render_pass.cc | 2 +- .../contents/test}/recording_render_pass.h | 10 +++++----- .../contents/tiled_texture_contents_unittests.cc | 2 +- 8 files changed, 14 insertions(+), 13 deletions(-) rename impeller/{aiks/testing => entity/contents/test}/recording_render_pass.cc (98%) rename impeller/{aiks/testing => entity/contents/test}/recording_render_pass.h (88%) diff --git a/impeller/aiks/BUILD.gn b/impeller/aiks/BUILD.gn index d80b286e61342..06d643c7b9993 100644 --- a/impeller/aiks/BUILD.gn +++ b/impeller/aiks/BUILD.gn @@ -74,10 +74,9 @@ impeller_component("context_spy") { "testing/context_mock.h", "testing/context_spy.cc", "testing/context_spy.h", - "testing/recording_render_pass.cc", - "testing/recording_render_pass.h", ] deps = [ + "//flutter/impeller/entity:entity_test_helpers", "//flutter/impeller/renderer", "//flutter/testing:testing_lib", ] diff --git a/impeller/aiks/testing/context_spy.cc b/impeller/aiks/testing/context_spy.cc index e6da5fa1a9055..460013cc2808a 100644 --- a/impeller/aiks/testing/context_spy.cc +++ b/impeller/aiks/testing/context_spy.cc @@ -2,9 +2,9 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "impeller/aiks/testing/context_spy.h" #include -#include "impeller/aiks/testing/recording_render_pass.h" + +#include "impeller/aiks/testing/context_spy.h" namespace impeller { namespace testing { diff --git a/impeller/aiks/testing/context_spy.h b/impeller/aiks/testing/context_spy.h index 91314f77c32fe..fe251b89e5999 100644 --- a/impeller/aiks/testing/context_spy.h +++ b/impeller/aiks/testing/context_spy.h @@ -8,7 +8,7 @@ #include #include "impeller/aiks/testing/context_mock.h" -#include "impeller/aiks/testing/recording_render_pass.h" +#include "impeller/entity/contents/test/recording_render_pass.h" namespace impeller { namespace testing { diff --git a/impeller/entity/BUILD.gn b/impeller/entity/BUILD.gn index c3fc2a8dca355..79ac4a76a5ee7 100644 --- a/impeller/entity/BUILD.gn +++ b/impeller/entity/BUILD.gn @@ -256,6 +256,8 @@ impeller_component("entity_test_helpers") { sources = [ "contents/test/contents_test_helpers.cc", "contents/test/contents_test_helpers.h", + "contents/test/recording_render_pass.cc", + "contents/test/recording_render_pass.h", ] deps = [ ":entity" ] diff --git a/impeller/entity/contents/checkerboard_contents_unittests.cc b/impeller/entity/contents/checkerboard_contents_unittests.cc index ed21255e759ed..97e1dae4276d5 100644 --- a/impeller/entity/contents/checkerboard_contents_unittests.cc +++ b/impeller/entity/contents/checkerboard_contents_unittests.cc @@ -7,9 +7,9 @@ #include "gtest/gtest.h" -#include "impeller/aiks/testing/recording_render_pass.h" #include "impeller/entity/contents/checkerboard_contents.h" #include "impeller/entity/contents/contents.h" +#include "impeller/entity/contents/test/recording_render_pass.h" #include "impeller/entity/entity.h" #include "impeller/entity/entity_playground.h" #include "impeller/renderer/render_target.h" diff --git a/impeller/aiks/testing/recording_render_pass.cc b/impeller/entity/contents/test/recording_render_pass.cc similarity index 98% rename from impeller/aiks/testing/recording_render_pass.cc rename to impeller/entity/contents/test/recording_render_pass.cc index 02f9360fab5ff..74f88a269173d 100644 --- a/impeller/aiks/testing/recording_render_pass.cc +++ b/impeller/entity/contents/test/recording_render_pass.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "impeller/aiks/testing/recording_render_pass.h" +#include "impeller/entity/contents/test/recording_render_pass.h" #include diff --git a/impeller/aiks/testing/recording_render_pass.h b/impeller/entity/contents/test/recording_render_pass.h similarity index 88% rename from impeller/aiks/testing/recording_render_pass.h rename to impeller/entity/contents/test/recording_render_pass.h index 3fa25b37c495e..86c5df059ef65 100644 --- a/impeller/aiks/testing/recording_render_pass.h +++ b/impeller/entity/contents/test/recording_render_pass.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ -#define FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ +#ifndef FLUTTER_IMPELLER_ENTITY_CONTENTS_TEST_RECORDING_RENDER_PASS_H_ +#define FLUTTER_IMPELLER_ENTITY_CONTENTS_TEST_RECORDING_RENDER_PASS_H_ #include "impeller/renderer/render_pass.h" @@ -44,7 +44,7 @@ class RecordingRenderPass : public RenderPass { bool SetVertexBuffer(VertexBuffer buffer) override; // |RenderPass| - fml::Status Draw(); + fml::Status Draw() override; // |RenderPass| bool BindResource(ShaderStage stage, @@ -58,7 +58,7 @@ class RecordingRenderPass : public RenderPass { DescriptorType type, const ShaderUniformSlot& slot, const std::shared_ptr& metadata, - BufferView view); + BufferView view) override; // |RenderPass| bool BindResource(ShaderStage stage, @@ -84,4 +84,4 @@ class RecordingRenderPass : public RenderPass { } // namespace impeller -#endif // FLUTTER_IMPELLER_AIKS_TESTING_RECORDING_RENDER_PASS_H_ +#endif // FLUTTER_IMPELLER_ENTITY_CONTENTS_TEST_RECORDING_RENDER_PASS_H_ diff --git a/impeller/entity/contents/tiled_texture_contents_unittests.cc b/impeller/entity/contents/tiled_texture_contents_unittests.cc index b42dd0eaff77e..ddaedc3efc1f1 100644 --- a/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -4,9 +4,9 @@ #include -#include "impeller/aiks/testing/recording_render_pass.h" #include "impeller/core/formats.h" #include "impeller/core/texture_descriptor.h" +#include "impeller/entity/contents/test/recording_render_pass.h" #include "impeller/entity/contents/tiled_texture_contents.h" #include "impeller/entity/entity_playground.h" #include "impeller/playground/playground_test.h" From e0a962ee69cbdae4741ac8d8eb0953b085c319a1 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 15 Jan 2024 21:59:10 -0800 Subject: [PATCH 09/13] Update context_vk.cc --- impeller/entity/contents/vertices_contents_unittests.cc | 9 ++++++--- impeller/renderer/backend/vulkan/context_vk.cc | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc index 17fac72d722dc..687b981cd7454 100644 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ b/impeller/entity/contents/vertices_contents_unittests.cc @@ -7,9 +7,11 @@ #include "gtest/gtest.h" +#include "impeller/entity/contents/test/recording_render_pass.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/contents/test/contents_test_helpers.h" +#include "impeller/entity/contents/test/recording_render_pass.h" #include "impeller/entity/contents/vertices_contents.h" #include "impeller/entity/entity.h" #include "impeller/entity/entity_playground.h" @@ -64,12 +66,13 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { *GetContentContext()->GetRenderTargetCache(), {100, 100}, /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); + auto recording_pass = std::make_shared(render_pass, GetContext(), render_target); Entity entity; - ASSERT_TRUE(render_pass->GetCommands().empty()); - ASSERT_TRUE(contents->Render(*content_context, entity, *render_pass)); + ASSERT_TRUE(recording_pass->GetCommands().empty()); + ASSERT_TRUE(contents->Render(*content_context, entity, *recording_pass)); - const auto& cmd = render_pass->GetCommands()[0]; + const auto& cmd = recording_pass->GetCommands()[0]; auto* frag_uniforms = GetFragInfo(cmd); ASSERT_TRUE(frag_uniforms); diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index c097b232de3a1..6c1804dd74022 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -156,7 +156,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = false; // settings.enable_validation; + auto enable_validation = settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; From dbaf4e478d8de6840be048243dcf602e4f995a76 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jan 2024 22:18:15 -0800 Subject: [PATCH 10/13] fix last test. --- impeller/entity/contents/vertices_contents_unittests.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc index 687b981cd7454..52eecf90e8a43 100644 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ b/impeller/entity/contents/vertices_contents_unittests.cc @@ -7,7 +7,6 @@ #include "gtest/gtest.h" -#include "impeller/entity/contents/test/recording_render_pass.h" #include "impeller/entity/contents/contents.h" #include "impeller/entity/contents/solid_color_contents.h" #include "impeller/entity/contents/test/contents_test_helpers.h" @@ -66,7 +65,8 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { *GetContentContext()->GetRenderTargetCache(), {100, 100}, /*mip_count=*/1); auto render_pass = buffer->CreateRenderPass(render_target); - auto recording_pass = std::make_shared(render_pass, GetContext(), render_target); + auto recording_pass = std::make_shared( + render_pass, GetContext(), render_target); Entity entity; ASSERT_TRUE(recording_pass->GetCommands().empty()); From de107205f3e6988cd77d76f933d915d60d81e9ab Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 15 Jan 2024 22:55:57 -0800 Subject: [PATCH 11/13] remove extra trace events. --- impeller/renderer/command_buffer.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/impeller/renderer/command_buffer.cc b/impeller/renderer/command_buffer.cc index 98b23204604b1..b25ab3af000ff 100644 --- a/impeller/renderer/command_buffer.cc +++ b/impeller/renderer/command_buffer.cc @@ -38,7 +38,6 @@ void CommandBuffer::WaitUntilScheduled() { bool CommandBuffer::EncodeAndSubmit( const std::shared_ptr& render_pass) { - TRACE_EVENT0("impeller", "CommandBuffer::EncodeAndSubmit"); if (!render_pass->IsValid() || !IsValid()) { return false; } @@ -52,7 +51,6 @@ bool CommandBuffer::EncodeAndSubmit( bool CommandBuffer::EncodeAndSubmit( const std::shared_ptr& blit_pass, const std::shared_ptr& allocator) { - TRACE_EVENT0("impeller", "CommandBuffer::EncodeAndSubmit"); if (!blit_pass->IsValid() || !IsValid()) { return false; } From b220e9115762d19384341cf2ad571f5b93c808e5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 16 Jan 2024 10:03:43 -0800 Subject: [PATCH 12/13] strong ptr. --- .../entity/contents/test/recording_render_pass.cc | 7 ++++--- impeller/entity/contents/test/recording_render_pass.h | 2 +- impeller/playground/imgui/imgui_impl_impeller.cc | 2 +- impeller/renderer/backend/gles/command_buffer_gles.cc | 6 +++++- impeller/renderer/backend/gles/render_pass_gles.cc | 2 +- impeller/renderer/backend/gles/render_pass_gles.h | 2 +- impeller/renderer/backend/metal/command_buffer_mtl.mm | 6 +++++- impeller/renderer/backend/metal/render_pass_mtl.h | 2 +- impeller/renderer/backend/metal/render_pass_mtl.mm | 2 +- impeller/renderer/backend/vulkan/render_pass_vk.cc | 5 ++--- impeller/renderer/render_pass.cc | 11 +++-------- impeller/renderer/render_pass.h | 7 ++++--- impeller/renderer/testing/mocks.h | 2 +- lib/gpu/render_pass.cc | 8 ++++---- lib/gpu/render_pass.h | 2 +- 15 files changed, 35 insertions(+), 31 deletions(-) diff --git a/impeller/entity/contents/test/recording_render_pass.cc b/impeller/entity/contents/test/recording_render_pass.cc index 74f88a269173d..baac7ca7b4317 100644 --- a/impeller/entity/contents/test/recording_render_pass.cc +++ b/impeller/entity/contents/test/recording_render_pass.cc @@ -8,9 +8,10 @@ namespace impeller { -RecordingRenderPass::RecordingRenderPass(std::shared_ptr delegate, - const std::weak_ptr& context, - const RenderTarget& render_target) +RecordingRenderPass::RecordingRenderPass( + std::shared_ptr delegate, + const std::shared_ptr& context, + const RenderTarget& render_target) : RenderPass(context, render_target), delegate_(std::move(delegate)) {} // |RenderPass| diff --git a/impeller/entity/contents/test/recording_render_pass.h b/impeller/entity/contents/test/recording_render_pass.h index 86c5df059ef65..eb585c27767eb 100644 --- a/impeller/entity/contents/test/recording_render_pass.h +++ b/impeller/entity/contents/test/recording_render_pass.h @@ -12,7 +12,7 @@ namespace impeller { class RecordingRenderPass : public RenderPass { public: explicit RecordingRenderPass(std::shared_ptr delegate, - const std::weak_ptr& context, + const std::shared_ptr& context, const RenderTarget& render_target); ~RecordingRenderPass() = default; diff --git a/impeller/playground/imgui/imgui_impl_impeller.cc b/impeller/playground/imgui/imgui_impl_impeller.cc index ada717b742ea8..279c94e51189c 100644 --- a/impeller/playground/imgui/imgui_impl_impeller.cc +++ b/impeller/playground/imgui/imgui_impl_impeller.cc @@ -126,7 +126,7 @@ void ImGui_ImplImpeller_RenderDrawData(ImDrawData* draw_data, return; // Nothing to render. } auto host_buffer = impeller::HostBuffer::Create( - render_pass.GetContext().lock()->GetResourceAllocator()); + render_pass.GetContext()->GetResourceAllocator()); using VS = impeller::ImguiRasterVertexShader; using FS = impeller::ImguiRasterFragmentShader; diff --git a/impeller/renderer/backend/gles/command_buffer_gles.cc b/impeller/renderer/backend/gles/command_buffer_gles.cc index 737984d3ae1a8..ea6a651d0189a 100644 --- a/impeller/renderer/backend/gles/command_buffer_gles.cc +++ b/impeller/renderer/backend/gles/command_buffer_gles.cc @@ -49,8 +49,12 @@ std::shared_ptr CommandBufferGLES::OnCreateRenderPass( if (!IsValid()) { return nullptr; } + auto context = context_.lock(); + if (!context) { + return nullptr; + } auto pass = std::shared_ptr( - new RenderPassGLES(context_, target, reactor_)); + new RenderPassGLES(context, target, reactor_)); if (!pass->IsValid()) { return nullptr; } diff --git a/impeller/renderer/backend/gles/render_pass_gles.cc b/impeller/renderer/backend/gles/render_pass_gles.cc index 20b6fd4bea71b..6cff519a41270 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.cc +++ b/impeller/renderer/backend/gles/render_pass_gles.cc @@ -20,7 +20,7 @@ namespace impeller { -RenderPassGLES::RenderPassGLES(std::weak_ptr context, +RenderPassGLES::RenderPassGLES(std::shared_ptr context, const RenderTarget& target, ReactorGLES::Ref reactor) : RenderPass(std::move(context), target), diff --git a/impeller/renderer/backend/gles/render_pass_gles.h b/impeller/renderer/backend/gles/render_pass_gles.h index e7e408306336c..9d4f986108d57 100644 --- a/impeller/renderer/backend/gles/render_pass_gles.h +++ b/impeller/renderer/backend/gles/render_pass_gles.h @@ -26,7 +26,7 @@ class RenderPassGLES final std::string label_; bool is_valid_ = false; - RenderPassGLES(std::weak_ptr context, + RenderPassGLES(std::shared_ptr context, const RenderTarget& target, ReactorGLES::Ref reactor); diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm index 7adb44a189225..c7899d48982f3 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -275,8 +275,12 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { return nullptr; } + auto context = context_.lock(); + if (!context) { + return nullptr; + } auto pass = std::shared_ptr( - new RenderPassMTL(context_, target, buffer_)); + new RenderPassMTL(context, target, buffer_)); if (!pass->IsValid()) { return nullptr; } diff --git a/impeller/renderer/backend/metal/render_pass_mtl.h b/impeller/renderer/backend/metal/render_pass_mtl.h index 69c78573e8add..00240d45249d3 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.h +++ b/impeller/renderer/backend/metal/render_pass_mtl.h @@ -26,7 +26,7 @@ class RenderPassMTL final : public RenderPass { std::string label_; bool is_valid_ = false; - RenderPassMTL(std::weak_ptr context, + RenderPassMTL(std::shared_ptr context, const RenderTarget& target, id buffer); diff --git a/impeller/renderer/backend/metal/render_pass_mtl.mm b/impeller/renderer/backend/metal/render_pass_mtl.mm index 83918f71fce84..396127d598733 100644 --- a/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -134,7 +134,7 @@ static bool ConfigureStencilAttachment( return result; } -RenderPassMTL::RenderPassMTL(std::weak_ptr context, +RenderPassMTL::RenderPassMTL(std::shared_ptr context, const RenderTarget& target, id buffer) : RenderPass(std::move(context), target), diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index c53b65adbf0e9..dc40fc6b70b67 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -384,8 +384,7 @@ void RenderPassVK::SetPipeline( auto descriptor_result = command_buffer_->GetEncoder()->AllocateDescriptorSets( - pipeline_vk.GetDescriptorSetLayout(), - ContextVK::Cast(*context_.lock())); + pipeline_vk.GetDescriptorSetLayout(), ContextVK::Cast(*context_)); if (!descriptor_result.ok()) { return; } @@ -520,7 +519,7 @@ fml::Status RenderPassVK::Draw() { "No valid pipeline is bound to the RenderPass."); } - const ContextVK& context_vk = ContextVK::Cast(*context_.lock()); + const ContextVK& context_vk = ContextVK::Cast(*context_); for (auto i = 0u; i < descriptor_write_offset_; i++) { write_workspace_[i].dstSet = descriptor_set_; } diff --git a/impeller/renderer/render_pass.cc b/impeller/renderer/render_pass.cc index 8c2880aaf6b02..8c3998d8720af 100644 --- a/impeller/renderer/render_pass.cc +++ b/impeller/renderer/render_pass.cc @@ -7,7 +7,7 @@ namespace impeller { -RenderPass::RenderPass(std::weak_ptr context, +RenderPass::RenderPass(std::shared_ptr context, const RenderTarget& target) : context_(std::move(context)), sample_count_(target.GetSampleCount()), @@ -77,15 +77,10 @@ bool RenderPass::AddCommand(Command&& command) { } bool RenderPass::EncodeCommands() const { - auto context = context_.lock(); - // The context could have been collected in the meantime. - if (!context) { - return false; - } - return OnEncodeCommands(*context); + return OnEncodeCommands(*context_); } -const std::weak_ptr& RenderPass::GetContext() const { +const std::shared_ptr& RenderPass::GetContext() const { return context_; } diff --git a/impeller/renderer/render_pass.h b/impeller/renderer/render_pass.h index 8bcc5f58255a9..cae0fe3046f7d 100644 --- a/impeller/renderer/render_pass.h +++ b/impeller/renderer/render_pass.h @@ -34,7 +34,7 @@ class RenderPass : public ResourceBinder { public: virtual ~RenderPass(); - const std::weak_ptr& GetContext() const; + const std::shared_ptr& GetContext() const; const RenderTarget& GetRenderTarget() const; @@ -157,7 +157,7 @@ class RenderPass : public ResourceBinder { bool HasStencilAttachment() const; protected: - const std::weak_ptr context_; + const std::shared_ptr context_; // The following properties: sample_count, pixel_format, // has_stencil_attachment, and render_target_size are cached on the // RenderTarget to speed up numerous lookups during rendering. This is safe as @@ -182,7 +182,8 @@ class RenderPass : public ResourceBinder { /// bool AddCommand(Command&& command); - RenderPass(std::weak_ptr context, const RenderTarget& target); + RenderPass(std::shared_ptr context, + const RenderTarget& target); virtual void OnSetLabel(std::string label) = 0; diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index 4960a57ef78fd..2c3b096bda6f7 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -92,7 +92,7 @@ class MockBlitPass : public BlitPass { class MockRenderPass : public RenderPass { public: - MockRenderPass(std::weak_ptr context, + MockRenderPass(std::shared_ptr context, const RenderTarget& target) : RenderPass(std::move(context), target) {} MOCK_METHOD(bool, IsValid, (), (const, override)); diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 70e75e210ada6..880fb8c4a7d75 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -28,7 +28,7 @@ RenderPass::RenderPass() RenderPass::~RenderPass() = default; -const std::weak_ptr& RenderPass::GetContext() const { +const std::shared_ptr& RenderPass::GetContext() const { return render_pass_->GetContext(); } @@ -115,7 +115,7 @@ RenderPass::GetOrCreatePipeline() { } } - auto& context = *GetContext().lock(); + auto& context = *GetContext(); render_pipeline_->BindToPipelineDescriptor(*context.GetShaderLibrary(), pipeline_desc); @@ -448,8 +448,8 @@ bool InternalFlutterGpu_RenderPass_BindTexture( flutter::gpu::ToImpellerSamplerAddressMode(width_address_mode); sampler_desc.height_address_mode = flutter::gpu::ToImpellerSamplerAddressMode(height_address_mode); - auto sampler = wrapper->GetContext().lock()->GetSamplerLibrary()->GetSampler( - sampler_desc); + auto sampler = + wrapper->GetContext()->GetSamplerLibrary()->GetSampler(sampler_desc); return command.BindResource( shader->GetShaderStage(), impeller::DescriptorType::kSampledImage, diff --git a/lib/gpu/render_pass.h b/lib/gpu/render_pass.h index 1581ac1597ead..94b7d5faebcca 100644 --- a/lib/gpu/render_pass.h +++ b/lib/gpu/render_pass.h @@ -33,7 +33,7 @@ class RenderPass : public RefCountedDartWrappable { ~RenderPass() override; - const std::weak_ptr& GetContext() const; + const std::shared_ptr& GetContext() const; impeller::Command& GetCommand(); const impeller::Command& GetCommand() const; From 34137439f5b20d811fe9dc32935ef2392d870282 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 16 Jan 2024 12:58:01 -0800 Subject: [PATCH 13/13] ++ --- impeller/entity/contents/test/recording_render_pass.cc | 2 +- impeller/entity/contents/vertices_contents_unittests.cc | 1 + lib/gpu/render_pass.cc | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/impeller/entity/contents/test/recording_render_pass.cc b/impeller/entity/contents/test/recording_render_pass.cc index baac7ca7b4317..ef22d86d69ed6 100644 --- a/impeller/entity/contents/test/recording_render_pass.cc +++ b/impeller/entity/contents/test/recording_render_pass.cc @@ -113,4 +113,4 @@ bool RecordingRenderPass::BindResource(ShaderStage stage, return delegate_->BindResource(stage, type, slot, metadata, texture, sampler); } -} // namespace impeller \ No newline at end of file +} // namespace impeller diff --git a/impeller/entity/contents/vertices_contents_unittests.cc b/impeller/entity/contents/vertices_contents_unittests.cc index 52eecf90e8a43..4f1e778986070 100644 --- a/impeller/entity/contents/vertices_contents_unittests.cc +++ b/impeller/entity/contents/vertices_contents_unittests.cc @@ -72,6 +72,7 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { ASSERT_TRUE(recording_pass->GetCommands().empty()); ASSERT_TRUE(contents->Render(*content_context, entity, *recording_pass)); + ASSERT_TRUE(recording_pass->GetCommands().size() > 0); const auto& cmd = recording_pass->GetCommands()[0]; auto* frag_uniforms = GetFragInfo(cmd); diff --git a/lib/gpu/render_pass.cc b/lib/gpu/render_pass.cc index 880fb8c4a7d75..206da54bde4ac 100644 --- a/lib/gpu/render_pass.cc +++ b/lib/gpu/render_pass.cc @@ -158,13 +158,13 @@ bool RenderPass::Draw() { } for (const auto& texture : result.vertex_bindings.sampled_images) { render_pass_->BindResource(impeller::ShaderStage::kVertex, - impeller::DescriptorType::kUniformBuffer, + impeller::DescriptorType::kSampledImage, texture.slot, *texture.texture.GetMetadata(), texture.texture.resource, texture.sampler); } for (const auto& buffer : result.fragment_bindings.buffers) { render_pass_->BindResource(impeller::ShaderStage::kFragment, - impeller::DescriptorType::kSampledImage, + impeller::DescriptorType::kUniformBuffer, buffer.slot, *buffer.view.GetMetadata(), buffer.view.resource); }