From 845e8f6aab2ada8eb02576f9d24daa504fc9364e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 20 Jan 2024 14:08:28 -0800 Subject: [PATCH 01/12] [Impeller] dont emulate command buffers for compute on Metal/Vulkan. --- .../entity/geometry/point_field_geometry.cc | 31 +- impeller/renderer/BUILD.gn | 2 - .../backend/metal/command_buffer_mtl.mm | 6 +- .../renderer/backend/metal/compute_pass_mtl.h | 125 +++++++- .../backend/metal/compute_pass_mtl.mm | 271 +++++------------- impeller/renderer/backend/vulkan/BUILD.gn | 5 - .../backend/vulkan/binding_helpers_vk.cc | 162 ----------- .../backend/vulkan/binding_helpers_vk.h | 32 --- .../backend/vulkan/command_buffer_vk.cc | 4 +- .../backend/vulkan/compute_pass_vk.cc | 261 ++++++++++------- .../renderer/backend/vulkan/compute_pass_vk.h | 58 +++- .../backend/vulkan/pass_bindings_cache.cc | 72 ----- .../backend/vulkan/pass_bindings_cache.h | 50 ---- .../vulkan/pass_bindings_cache_unittests.cc | 82 ------ .../renderer/backend/vulkan/pipeline_vk.h | 4 + .../renderer/backend/vulkan/render_pass_vk.cc | 1 - .../renderer/backend/vulkan/render_pass_vk.h | 1 - impeller/renderer/compute_command.cc | 59 ---- impeller/renderer/compute_command.h | 76 ----- impeller/renderer/compute_pass.cc | 39 +-- impeller/renderer/compute_pass.h | 43 +-- .../renderer/compute_subgroup_unittests.cc | 1 - impeller/renderer/compute_tessellator.cc | 40 ++- impeller/renderer/compute_unittests.cc | 112 +++----- 24 files changed, 493 insertions(+), 1044 deletions(-) delete mode 100644 impeller/renderer/backend/vulkan/binding_helpers_vk.cc delete mode 100644 impeller/renderer/backend/vulkan/binding_helpers_vk.h delete mode 100644 impeller/renderer/backend/vulkan/pass_bindings_cache.cc delete mode 100644 impeller/renderer/backend/vulkan/pass_bindings_cache.h delete mode 100644 impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc delete mode 100644 impeller/renderer/compute_command.cc delete mode 100644 impeller/renderer/compute_command.h diff --git a/impeller/entity/geometry/point_field_geometry.cc b/impeller/entity/geometry/point_field_geometry.cc index 27369da784e1c..0e7bf69476ce8 100644 --- a/impeller/entity/geometry/point_field_geometry.cc +++ b/impeller/entity/geometry/point_field_geometry.cc @@ -5,7 +5,6 @@ #include "impeller/entity/geometry/point_field_geometry.h" #include "impeller/renderer/command_buffer.h" -#include "impeller/renderer/compute_command.h" namespace impeller { @@ -168,9 +167,9 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( BufferView output; { using PS = PointsComputeShader; - ComputeCommand cmd; - DEBUG_COMMAND_INFO(cmd, "Points Geometry"); - cmd.pipeline = renderer.GetPointComputePipeline(); + + compute_pass->SetPipeline(renderer.GetPointComputePipeline()); + compute_pass->SetCommandLabel("Points Geometry"); PS::FrameInfo frame_info; frame_info.count = points_.size(); @@ -180,11 +179,11 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( frame_info.points_per_circle = points_per_circle; frame_info.divisions_per_circle = vertices_per_geom; - PS::BindFrameInfo(cmd, host_buffer.EmplaceUniform(frame_info)); - PS::BindGeometryData(cmd, geometry_buffer); - PS::BindPointData(cmd, points_data); + PS::BindFrameInfo(*compute_pass, host_buffer.EmplaceUniform(frame_info)); + PS::BindGeometryData(*compute_pass, geometry_buffer); + PS::BindPointData(*compute_pass, points_data); - if (!compute_pass->AddCommand(std::move(cmd))) { + if (!compute_pass->Compute(ISize(total, 1)).ok()) { return {}; } output = geometry_buffer; @@ -201,9 +200,8 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( using UV = UvComputeShader; - ComputeCommand cmd; - DEBUG_COMMAND_INFO(cmd, "UV Geometry"); - cmd.pipeline = renderer.GetUvComputePipeline(); + compute_pass->SetCommandLabel("UV Geometry"); + compute_pass->SetPipeline(renderer.GetUvComputePipeline()); UV::FrameInfo frame_info; frame_info.count = total; @@ -211,19 +209,16 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( frame_info.texture_origin = {0, 0}; frame_info.texture_size = Vector2(texture_coverage.value().GetSize()); - UV::BindFrameInfo(cmd, host_buffer.EmplaceUniform(frame_info)); - UV::BindGeometryData(cmd, geometry_buffer); - UV::BindGeometryUVData(cmd, geometry_uv_buffer); + UV::BindFrameInfo(*compute_pass, host_buffer.EmplaceUniform(frame_info)); + UV::BindGeometryData(*compute_pass, geometry_buffer); + UV::BindGeometryUVData(*compute_pass, geometry_uv_buffer); - if (!compute_pass->AddCommand(std::move(cmd))) { + if (!compute_pass->Compute(ISize(total, 1)).ok()) { return {}; } output = geometry_uv_buffer; } - compute_pass->SetGridSize(ISize(total, 1)); - compute_pass->SetThreadGroupSize(ISize(total, 1)); - if (!compute_pass->EncodeCommands() || !cmd_buffer->SubmitCommands()) { return {}; } diff --git a/impeller/renderer/BUILD.gn b/impeller/renderer/BUILD.gn index 39e98fd6a4058..1456b65cb3ac2 100644 --- a/impeller/renderer/BUILD.gn +++ b/impeller/renderer/BUILD.gn @@ -55,8 +55,6 @@ impeller_component("renderer") { "command.h", "command_buffer.cc", "command_buffer.h", - "compute_command.cc", - "compute_command.h", "compute_pass.cc", "compute_pass.h", "compute_pipeline_builder.cc", diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm index d9c43d1b4e836..5909426cbb544 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -220,9 +220,13 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { if (!buffer_) { return nullptr; } + auto context = context_.lock(); + if (!context) { + return nullptr; + } auto pass = - std::shared_ptr(new ComputePassMTL(context_, buffer_)); + std::shared_ptr(new ComputePassMTL(context, buffer_)); if (!pass->IsValid()) { return nullptr; } diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.h b/impeller/renderer/backend/metal/compute_pass_mtl.h index 812749639def4..757e2f1127c61 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.h +++ b/impeller/renderer/backend/metal/compute_pass_mtl.h @@ -9,9 +9,96 @@ #include "flutter/fml/macros.h" #include "impeller/renderer/compute_pass.h" +#include "impeller/renderer/pipeline_descriptor.h" namespace impeller { +//----------------------------------------------------------------------------- +/// @brief Ensures that bindings on the pass are not redundantly set or +/// updated. Avoids making the driver do additional checks and makes +/// the frame insights during profiling and instrumentation not +/// complain about the same. +/// +/// There should be no change to rendering if this caching was +/// absent. +/// +struct ComputePassBindingsCache { + explicit ComputePassBindingsCache() {} + + ComputePassBindingsCache(const ComputePassBindingsCache&) = delete; + + ComputePassBindingsCache(ComputePassBindingsCache&&) = delete; + + void SetComputePipelineState(id pipeline) { + if (pipeline == pipeline_) { + return; + } + pipeline_ = pipeline; + [encoder_ setComputePipelineState:pipeline_]; + } + + id GetPipeline() const { return pipeline_; } + + void SetEncoder(id encoder) { encoder_ = encoder; } + + void SetBuffer(uint64_t index, uint64_t offset, id buffer) { + auto found = buffers_.find(index); + if (found != buffers_.end() && found->second.buffer == buffer) { + // The right buffer is bound. Check if its offset needs to be updated. + if (found->second.offset == offset) { + // Buffer and its offset is identical. Nothing to do. + return; + } + + // Only the offset needs to be updated. + found->second.offset = offset; + + [encoder_ setBufferOffset:offset atIndex:index]; + return; + } + + buffers_[index] = {buffer, static_cast(offset)}; + [encoder_ setBuffer:buffer offset:offset atIndex:index]; + } + + void SetTexture(uint64_t index, id texture) { + auto found = textures_.find(index); + if (found != textures_.end() && found->second == texture) { + // Already bound. + return; + } + textures_[index] = texture; + [encoder_ setTexture:texture atIndex:index]; + return; + } + + void SetSampler(uint64_t index, id sampler) { + auto found = samplers_.find(index); + if (found != samplers_.end() && found->second == sampler) { + // Already bound. + return; + } + samplers_[index] = sampler; + [encoder_ setSamplerState:sampler atIndex:index]; + return; + } + + private: + struct BufferOffsetPair { + id buffer = nullptr; + size_t offset = 0u; + }; + using BufferMap = std::map; + using TextureMap = std::map>; + using SamplerMap = std::map>; + + id encoder_; + id pipeline_ = nullptr; + BufferMap buffers_; + TextureMap textures_; + SamplerMap samplers_; +}; + class ComputePassMTL final : public ComputePass { public: // |RenderPass| @@ -21,27 +108,47 @@ class ComputePassMTL final : public ComputePass { friend class CommandBufferMTL; id buffer_ = nil; - std::string label_; + id encoder_ = nil; + ComputePassBindingsCache pass_bindings_cache_ = ComputePassBindingsCache(); bool is_valid_ = false; + bool has_label_ = false; - ComputePassMTL(std::weak_ptr context, + ComputePassMTL(std::shared_ptr context, id buffer); // |ComputePass| bool IsValid() const override; + // |ComputePass| + fml::Status Compute(const ISize& grid_size) override; + + // |ComputePass| + void SetCommandLabel(std::string_view label) override; + // |ComputePass| void OnSetLabel(const std::string& label) override; // |ComputePass| - bool OnEncodeCommands(const Context& context, - const ISize& grid_size, - const ISize& thread_group_size) const override; + void SetPipeline(const std::shared_ptr>& + pipeline) override; + + // |ComputePass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) override; - bool EncodeCommands(const std::shared_ptr& allocator, - id pass, - const ISize& grid_size, - const ISize& thread_group_size) const; + // |ComputePass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) override; + + // |ComputePass| + bool EncodeCommands() const override; ComputePassMTL(const ComputePassMTL&) = delete; diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index 5abab34389a31..6b3bf3a63341d 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -6,13 +6,14 @@ #include #include -#include #include "flutter/fml/backtrace.h" #include "flutter/fml/closure.h" #include "flutter/fml/logging.h" #include "flutter/fml/trace_event.h" +#include "fml/status.h" #include "impeller/base/backend_cast.h" +#include "impeller/core/device_buffer.h" #include "impeller/core/formats.h" #include "impeller/core/host_buffer.h" #include "impeller/core/shader_types.h" @@ -22,16 +23,20 @@ #include "impeller/renderer/backend/metal/sampler_mtl.h" #include "impeller/renderer/backend/metal/texture_mtl.h" #include "impeller/renderer/command.h" -#include "impeller/renderer/compute_command.h" namespace impeller { -ComputePassMTL::ComputePassMTL(std::weak_ptr context, +ComputePassMTL::ComputePassMTL(std::shared_ptr context, id buffer) : ComputePass(std::move(context)), buffer_(buffer) { if (!buffer_) { return; } + encoder_ = [buffer_ computeCommandEncoder]; + if (!encoder_) { + return; + } + pass_bindings_cache_.SetEncoder(encoder_); is_valid_ = true; } @@ -42,231 +47,113 @@ } void ComputePassMTL::OnSetLabel(const std::string& label) { +#ifdef IMPELLER_DEBUG if (label.empty()) { return; } - label_ = label; + [encoder_ setLabel:@(label.c_str())]; +#endif // IMPELLER_DEBUG } -bool ComputePassMTL::OnEncodeCommands(const Context& context, - const ISize& grid_size, - const ISize& thread_group_size) const { - TRACE_EVENT0("impeller", "ComputePassMTL::EncodeCommands"); - if (!IsValid()) { - return false; - } - - FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty()); - - // TODO(dnfield): Support non-serial dispatch type on higher iOS versions. - auto compute_command_encoder = [buffer_ computeCommandEncoder]; - - if (!compute_command_encoder) { - return false; - } - - if (!label_.empty()) { - [compute_command_encoder setLabel:@(label_.c_str())]; - } - - // Success or failure, the pass must end. The buffer can only process one pass - // at a time. - fml::ScopedCleanupClosure auto_end( - [compute_command_encoder]() { [compute_command_encoder endEncoding]; }); - - return EncodeCommands(context.GetResourceAllocator(), compute_command_encoder, - grid_size, thread_group_size); +void ComputePassMTL::SetCommandLabel(std::string_view label) { + has_label_ = true; + [encoder_ pushDebugGroup:@(label.data())]; } -//----------------------------------------------------------------------------- -/// @brief Ensures that bindings on the pass are not redundantly set or -/// updated. Avoids making the driver do additional checks and makes -/// the frame insights during profiling and instrumentation not -/// complain about the same. -/// -/// There should be no change to rendering if this caching was -/// absent. -/// -struct ComputePassBindingsCache { - explicit ComputePassBindingsCache(id encoder) - : encoder_(encoder) {} - - ComputePassBindingsCache(const ComputePassBindingsCache&) = delete; - - ComputePassBindingsCache(ComputePassBindingsCache&&) = delete; - - void SetComputePipelineState(id pipeline) { - if (pipeline == pipeline_) { - return; - } - pipeline_ = pipeline; - [encoder_ setComputePipelineState:pipeline_]; - } - - id GetPipeline() const { return pipeline_; } - - void SetBuffer(uint64_t index, uint64_t offset, id buffer) { - auto found = buffers_.find(index); - if (found != buffers_.end() && found->second.buffer == buffer) { - // The right buffer is bound. Check if its offset needs to be updated. - if (found->second.offset == offset) { - // Buffer and its offset is identical. Nothing to do. - return; - } - - // Only the offset needs to be updated. - found->second.offset = offset; - - [encoder_ setBufferOffset:offset atIndex:index]; - return; - } - - buffers_[index] = {buffer, static_cast(offset)}; - [encoder_ setBuffer:buffer offset:offset atIndex:index]; - } - - void SetTexture(uint64_t index, id texture) { - auto found = textures_.find(index); - if (found != textures_.end() && found->second == texture) { - // Already bound. - return; - } - textures_[index] = texture; - [encoder_ setTexture:texture atIndex:index]; - return; - } - - void SetSampler(uint64_t index, id sampler) { - auto found = samplers_.find(index); - if (found != samplers_.end() && found->second == sampler) { - // Already bound. - return; - } - samplers_[index] = sampler; - [encoder_ setSamplerState:sampler atIndex:index]; - return; - } - - private: - struct BufferOffsetPair { - id buffer = nullptr; - size_t offset = 0u; - }; - using BufferMap = std::map; - using TextureMap = std::map>; - using SamplerMap = std::map>; - - const id encoder_; - id pipeline_ = nullptr; - BufferMap buffers_; - TextureMap textures_; - SamplerMap samplers_; -}; +// |ComputePass| +void ComputePassMTL::SetPipeline( + const std::shared_ptr>& pipeline) { + pass_bindings_cache_.SetComputePipelineState( + ComputePipelineMTL::Cast(*pipeline).GetMTLComputePipelineState()); +} -static bool Bind(ComputePassBindingsCache& pass, - Allocator& allocator, - size_t bind_index, - const BufferView& view) { +// |ComputePass| +bool ComputePassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) { if (!view.buffer) { return false; } - auto device_buffer = view.buffer; + const std::shared_ptr& device_buffer = view.buffer; if (!device_buffer) { return false; } - auto buffer = DeviceBufferMTL::Cast(*device_buffer).GetMTLBuffer(); + id buffer = DeviceBufferMTL::Cast(*device_buffer).GetMTLBuffer(); // The Metal call is a void return and we don't want to make it on nil. if (!buffer) { return false; } - pass.SetBuffer(bind_index, view.range.offset, buffer); + pass_bindings_cache_.SetBuffer(slot.ext_res_0, view.range.offset, buffer); return true; } -static bool Bind(ComputePassBindingsCache& pass, - size_t bind_index, - const Sampler& sampler, - const Texture& texture) { - if (!sampler.IsValid() || !texture.IsValid()) { +// |ComputePass| +bool ComputePassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) { + if (!sampler->IsValid() || !texture->IsValid()) { return false; } - pass.SetTexture(bind_index, TextureMTL::Cast(texture).GetMTLTexture()); - pass.SetSampler(bind_index, SamplerMTL::Cast(sampler).GetMTLSamplerState()); + pass_bindings_cache_.SetTexture(slot.texture_index, + TextureMTL::Cast(*texture).GetMTLTexture()); + pass_bindings_cache_.SetSampler( + slot.texture_index, SamplerMTL::Cast(*sampler).GetMTLSamplerState()); return true; } -bool ComputePassMTL::EncodeCommands(const std::shared_ptr& allocator, - id encoder, - const ISize& grid_size, - const ISize& thread_group_size) const { - if (grid_size.width == 0 || grid_size.height == 0) { - return true; +fml::Status ComputePassMTL::Compute(const ISize& grid_size) { + if (grid_size.IsEmpty()) { + return fml::Status(fml::StatusCode::kUnknown, + "Invalid grid size for compute command."); } - - ComputePassBindingsCache pass_bindings(encoder); - - fml::closure pop_debug_marker = [encoder]() { [encoder popDebugGroup]; }; - for (const ComputeCommand& command : commands_) { -#ifdef IMPELLER_DEBUG - fml::ScopedCleanupClosure auto_pop_debug_marker(pop_debug_marker); - if (!command.label.empty()) { - [encoder pushDebugGroup:@(command.label.c_str())]; - } else { - auto_pop_debug_marker.Release(); - } -#endif - - pass_bindings.SetComputePipelineState( - ComputePipelineMTL::Cast(*command.pipeline) - .GetMTLComputePipelineState()); - - for (const BufferAndUniformSlot& buffer : command.bindings.buffers) { - if (!Bind(pass_bindings, *allocator, buffer.slot.ext_res_0, - buffer.view.resource)) { - return false; - } - } - - for (const TextureAndSampler& data : command.bindings.sampled_images) { - if (!Bind(pass_bindings, data.slot.texture_index, *data.sampler, - *data.texture.resource)) { - return false; - } + // TODO(dnfield): use feature detection to support non-uniform threadgroup + // sizes. + // https://github.com/flutter/flutter/issues/110619 + auto width = grid_size.width; + auto height = grid_size.height; + + auto maxTotalThreadsPerThreadgroup = static_cast( + pass_bindings_cache_.GetPipeline().maxTotalThreadsPerThreadgroup); + + // Special case for linear processing. + if (height == 1) { + int64_t threadGroups = std::max( + static_cast( + std::ceil(width * 1.0 / maxTotalThreadsPerThreadgroup * 1.0)), + 1LL); + [encoder_ + dispatchThreadgroups:MTLSizeMake(threadGroups, 1, 1) + threadsPerThreadgroup:MTLSizeMake(maxTotalThreadsPerThreadgroup, 1, 1)]; + } else { + while (width * height > maxTotalThreadsPerThreadgroup) { + width = std::max(1LL, width / 2); + height = std::max(1LL, height / 2); } - // TODO(dnfield): use feature detection to support non-uniform threadgroup - // sizes. - // https://github.com/flutter/flutter/issues/110619 - auto width = grid_size.width; - auto height = grid_size.height; - - auto maxTotalThreadsPerThreadgroup = static_cast( - pass_bindings.GetPipeline().maxTotalThreadsPerThreadgroup); - - // Special case for linear processing. - if (height == 1) { - int64_t threadGroups = std::max( - static_cast( - std::ceil(width * 1.0 / maxTotalThreadsPerThreadgroup * 1.0)), - 1LL); - [encoder dispatchThreadgroups:MTLSizeMake(threadGroups, 1, 1) - threadsPerThreadgroup:MTLSizeMake(maxTotalThreadsPerThreadgroup, - 1, 1)]; - } else { - while (width * height > maxTotalThreadsPerThreadgroup) { - width = std::max(1LL, width / 2); - height = std::max(1LL, height / 2); - } + auto size = MTLSizeMake(width, height, 1); + [encoder_ dispatchThreadgroups:size threadsPerThreadgroup:size]; + } - auto size = MTLSizeMake(width, height, 1); - [encoder dispatchThreadgroups:size threadsPerThreadgroup:size]; - } +#ifdef IMPELLER_DEBUG + if (has_label_) { + [encoder_ popDebugGroup]; } + has_label_ = false; +#endif // IMPELLER_DEBUG + return fml::Status(); +} +bool ComputePassMTL::EncodeCommands() const { + [encoder_ endEncoding]; return true; } diff --git a/impeller/renderer/backend/vulkan/BUILD.gn b/impeller/renderer/backend/vulkan/BUILD.gn index 7a7822cc9f92c..63cf211048970 100644 --- a/impeller/renderer/backend/vulkan/BUILD.gn +++ b/impeller/renderer/backend/vulkan/BUILD.gn @@ -14,7 +14,6 @@ impeller_component("vulkan_unittests") { "context_vk_unittests.cc", "descriptor_pool_vk_unittests.cc", "fence_waiter_vk_unittests.cc", - "pass_bindings_cache_unittests.cc", "resource_manager_vk_unittests.cc", "test/gpu_tracer_unittests.cc", "test/mock_vulkan.cc", @@ -35,8 +34,6 @@ impeller_component("vulkan") { "android_hardware_buffer_texture_source_vk.h", "barrier_vk.cc", "barrier_vk.h", - "binding_helpers_vk.cc", - "binding_helpers_vk.h", "blit_command_vk.cc", "blit_command_vk.h", "blit_pass_vk.cc", @@ -68,8 +65,6 @@ impeller_component("vulkan") { "gpu_tracer_vk.cc", "gpu_tracer_vk.h", "limits_vk.h", - "pass_bindings_cache.cc", - "pass_bindings_cache.h", "pipeline_cache_vk.cc", "pipeline_cache_vk.h", "pipeline_library_vk.cc", diff --git a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc b/impeller/renderer/backend/vulkan/binding_helpers_vk.cc deleted file mode 100644 index 92b30313f5f3a..0000000000000 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.cc +++ /dev/null @@ -1,162 +0,0 @@ -// 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/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" -#include "impeller/renderer/backend/vulkan/command_pool_vk.h" -#include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" -#include "impeller/renderer/backend/vulkan/context_vk.h" -#include "impeller/renderer/backend/vulkan/sampler_vk.h" -#include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "impeller/renderer/command.h" -#include "impeller/renderer/compute_command.h" -#include "vulkan/vulkan_core.h" - -namespace impeller { - -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) { - 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) || - !encoder->Track(sampler.GetSharedSampler())) { - return false; - } - - const SampledImageSlot& slot = data.slot; - - vk::DescriptorImageInfo image_info; - image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal; - image_info.sampler = sampler.GetSampler(); - image_info.imageView = texture_vk.GetImageView(); - 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 = &image_workspace[image_offset - 1]; - - 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::array& buffer_workspace, - size_t& buffer_offset, - std::array& - write_workspace, - size_t& write_offset) { - for (const BufferAndUniformSlot& data : bindings.buffers) { - const std::shared_ptr& device_buffer = - data.view.resource.buffer; - - auto buffer = DeviceBufferVK::Cast(*device_buffer).GetBuffer(); - if (!buffer) { - return false; - } - - if (!encoder->Track(device_buffer)) { - return false; - } - - uint32_t offset = data.view.resource.range.offset; - - vk::DescriptorBufferInfo buffer_info; - buffer_info.buffer = buffer; - buffer_info.offset = offset; - buffer_info.range = data.view.resource.range.length; - buffer_workspace[buffer_offset++] = buffer_info; - - // TODO(jonahwilliams): remove this part by storing more data in - // ShaderUniformSlot. - const ShaderUniformSlot& uniform = data.slot; - auto layout_it = - std::find_if(desc_set.begin(), desc_set.end(), - [&uniform](const DescriptorSetLayout& layout) { - return layout.binding == uniform.binding; - }); - if (layout_it == desc_set.end()) { - VALIDATION_LOG << "Failed to get descriptor set layout for binding " - << uniform.binding; - return false; - } - auto layout = *layout_it; - - vk::WriteDescriptorSet write_set; - write_set.dstSet = vk_desc_set; - write_set.dstBinding = uniform.binding; - write_set.descriptorCount = 1u; - write_set.descriptorType = ToVKDescriptorType(layout.descriptor_type); - write_set.pBufferInfo = &buffer_workspace[buffer_offset - 1]; - - write_workspace[write_offset++] = write_set; - } - return true; -} - -fml::StatusOr AllocateAndBindDescriptorSets( - const ContextVK& context, - const std::shared_ptr& encoder, - 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_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, {}); - - 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 deleted file mode 100644 index be35567a77e3b..0000000000000 --- a/impeller/renderer/backend/vulkan/binding_helpers_vk.h +++ /dev/null @@ -1,32 +0,0 @@ -// 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_RENDERER_BACKEND_VULKAN_BINDING_HELPERS_VK_H_ -#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BINDING_HELPERS_VK_H_ - -#include "fml/status_or.h" -#include "impeller/renderer/backend/vulkan/context_vk.h" -#include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "impeller/renderer/command.h" -#include "impeller/renderer/compute_command.h" - -namespace impeller { - -// 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, - Allocator& allocator, - const ComputeCommand& command, - std::array& image_workspace, - std::array& buffer_workspace, - std::array& - write_workspace); - -} // namespace impeller - -#endif // FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_BINDING_HELPERS_VK_H_ diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 6b7b14f243fdb..c63dbec45dd9d 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -100,8 +100,8 @@ std::shared_ptr CommandBufferVK::OnCreateComputePass() { return nullptr; } auto pass = - std::shared_ptr(new ComputePassVK(context, // - weak_from_this() // + std::shared_ptr(new ComputePassVK(context, // + shared_from_this() // )); if (!pass->IsValid()) { return nullptr; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 4788ec1a42451..5c5a4a02a812d 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -4,19 +4,24 @@ #include "impeller/renderer/backend/vulkan/compute_pass_vk.h" -#include "flutter/fml/trace_event.h" -#include "impeller/renderer/backend/vulkan/binding_helpers_vk.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" +#include "impeller/renderer/backend/vulkan/formats_vk.h" +#include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" -#include "impeller/renderer/command.h" namespace impeller { -ComputePassVK::ComputePassVK(std::weak_ptr context, - std::weak_ptr command_buffer) +ComputePassVK::ComputePassVK(std::shared_ptr context, + std::shared_ptr command_buffer) : ComputePass(std::move(context)), command_buffer_(std::move(command_buffer)) { + // TOOD(dnfield): This should be moved to caps. But for now keeping this + // in parallel with Metal. + max_wg_size_ = ContextVK::Cast(*context) + .GetPhysicalDevice() + .getProperties() + .limits.maxComputeWorkGroupSize; is_valid_ = true; } @@ -33,125 +38,177 @@ void ComputePassVK::OnSetLabel(const std::string& label) { label_ = label; } -static bool UpdateBindingLayouts(const Bindings& bindings, - const vk::CommandBuffer& buffer) { - BarrierVK barrier; - barrier.cmd_buffer = buffer; - barrier.src_access = vk::AccessFlagBits::eTransferWrite; - barrier.src_stage = vk::PipelineStageFlagBits::eTransfer; - barrier.dst_access = vk::AccessFlagBits::eShaderRead; - barrier.dst_stage = vk::PipelineStageFlagBits::eComputeShader; - - barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal; +// |RenderPass| +void ComputePassVK::SetCommandLabel(std::string_view label) { +#ifdef IMPELLER_DEBUG + command_buffer_->GetEncoder()->PushDebugGroup(label); + has_label_ = true; +#endif // IMPELLER_DEBUG +} - for (const TextureAndSampler& data : bindings.sampled_images) { - if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) { - return false; - } +// |ComputePass| +void ComputePassVK::SetPipeline( + const std::shared_ptr>& pipeline) { + const auto& pipeline_vk = ComputePipelineVK::Cast(*pipeline); + const vk::CommandBuffer& command_buffer_vk = + command_buffer_->GetEncoder()->GetCommandBuffer(); + command_buffer_vk.bindPipeline(vk::PipelineBindPoint::eCompute, + pipeline_vk.GetPipeline()); + pipeline_layout_ = pipeline_vk.GetPipelineLayout(); + + auto descriptor_result = + command_buffer_->GetEncoder()->AllocateDescriptorSets( + pipeline_vk.GetDescriptorSetLayout(), ContextVK::Cast(*context_)); + if (!descriptor_result.ok()) { + return; } - return true; + descriptor_set_ = descriptor_result.value(); + pipeline_valid_ = true; } -static bool UpdateBindingLayouts(const ComputeCommand& command, - const vk::CommandBuffer& buffer) { - return UpdateBindingLayouts(command.bindings, buffer); -} +// |ComputePass| +fml::Status ComputePassVK::Compute(const ISize& grid_size) { + if (grid_size.IsEmpty() || !pipeline_valid_) { + return fml::Status(fml::StatusCode::kCancelled, + "Invalid pipeline or empty grid."); + } + + const ContextVK& context_vk = ContextVK::Cast(*context_); + for (auto i = 0u; i < descriptor_write_offset_; i++) { + write_workspace_[i].dstSet = descriptor_set_; + } -static bool UpdateBindingLayouts(const std::vector& commands, - const vk::CommandBuffer& buffer) { - for (const ComputeCommand& command : commands) { - if (!UpdateBindingLayouts(command, buffer)) { - return false; + context_vk.GetDevice().updateDescriptorSets(descriptor_write_offset_, + write_workspace_.data(), 0u, {}); + const vk::CommandBuffer& command_buffer_vk = + command_buffer_->GetEncoder()->GetCommandBuffer(); + + command_buffer_vk.bindDescriptorSets( + vk::PipelineBindPoint::eCompute, // bind point + pipeline_layout_, // layout + 0, // first set + 1, // set count + &descriptor_set_, // sets + 0, // offset count + nullptr // offsets + ); + + int64_t width = grid_size.width; + int64_t height = grid_size.height; + + // Special case for linear processing. + if (height == 1) { + int64_t minimum = 1; + int64_t threadGroups = std::max( + static_cast(std::ceil(width * 1.0 / max_wg_size_[0] * 1.0)), + minimum); + command_buffer_vk.dispatch(threadGroups, 1, 1); + } else { + while (width > max_wg_size_[0]) { + width = std::max(static_cast(1), width / 2); + } + while (height > max_wg_size_[1]) { + height = std::max(static_cast(1), height / 2); } + command_buffer_vk.dispatch(width, height, 1); } - return true; -} -bool ComputePassVK::OnEncodeCommands(const Context& context, - const ISize& grid_size, - const ISize& thread_group_size) const { - TRACE_EVENT0("impeller", "ComputePassVK::EncodeCommands"); - if (!IsValid()) { - return false; +#ifdef IMPELLER_DEBUG + if (has_label_) { + command_buffer_->GetEncoder()->PopDebugGroup(); } + has_label_ = false; +#endif // IMPELLER_DEBUG - FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty()); + size_t bound_image_offset_ = 0u; + size_t bound_buffer_offset_ = 0u; + size_t descriptor_write_offset_ = 0u; + bool has_label_ = false; + bool pipeline_valid_ = false; + + return fml::Status(); +} + +// |ResourceBinder| +bool ComputePassVK::BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) { + return BindResource(slot.binding, type, view); +} - const auto& vk_context = ContextVK::Cast(context); - auto command_buffer = command_buffer_.lock(); - if (!command_buffer) { - VALIDATION_LOG << "Command buffer died before commands could be encoded."; +// |ResourceBinder| +bool ComputePassVK::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) { + if (bound_buffer_offset_ >= kMaxBindings) { return false; } - auto encoder = command_buffer->GetEncoder(); - if (!encoder) { + const TextureVK& texture_vk = TextureVK::Cast(*texture); + const SamplerVK& sampler_vk = SamplerVK::Cast(*sampler); + + if (!command_buffer_->GetEncoder()->Track(texture) || + !command_buffer_->GetEncoder()->Track(sampler_vk.GetSharedSampler())) { return false; } - fml::ScopedCleanupClosure pop_marker( - [&encoder]() { encoder->PopDebugGroup(); }); - if (!label_.empty()) { - encoder->PushDebugGroup(label_.c_str()); - } else { - pop_marker.Release(); + 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::WriteDescriptorSet write_set; + write_set.dstBinding = slot.binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = ToVKDescriptorType(type); + write_set.pImageInfo = &image_workspace_[bound_image_offset_ - 1]; + + write_workspace_[descriptor_write_offset_++] = write_set; + return true; +} + +bool ComputePassVK::BindResource(size_t binding, + DescriptorType type, + const BufferView& view) { + if (bound_buffer_offset_ >= kMaxBindings) { + return false; } - auto cmd_buffer = encoder->GetCommandBuffer(); - if (!UpdateBindingLayouts(commands_, cmd_buffer)) { - VALIDATION_LOG << "Could not update binding layouts for compute pass."; + const std::shared_ptr& device_buffer = view.buffer; + auto buffer = DeviceBufferVK::Cast(*device_buffer).GetBuffer(); + if (!buffer) { return false; } - auto& allocator = *context.GetResourceAllocator(); - - TRACE_EVENT0("impeller", "EncodeComputePassCommands"); - 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_set}}, // sets - nullptr // offsets - ); - - // TOOD(dnfield): This should be moved to caps. But for now keeping this - // in parallel with Metal. - auto device_properties = vk_context.GetPhysicalDevice().getProperties(); - - auto max_wg_size = device_properties.limits.maxComputeWorkGroupSize; - - int64_t width = grid_size.width; - int64_t height = grid_size.height; - - // Special case for linear processing. - if (height == 1) { - int64_t minimum = 1; - int64_t threadGroups = std::max( - static_cast(std::ceil(width * 1.0 / max_wg_size[0] * 1.0)), - minimum); - cmd_buffer.dispatch(threadGroups, 1, 1); - } else { - while (width > max_wg_size[0]) { - width = std::max(static_cast(1), width / 2); - } - while (height > max_wg_size[1]) { - height = std::max(static_cast(1), height / 2); - } - cmd_buffer.dispatch(width, height, 1); - } + + if (!command_buffer_->GetEncoder()->Track(device_buffer)) { + return false; } + 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; + + vk::WriteDescriptorSet write_set; + write_set.dstBinding = binding; + write_set.descriptorCount = 1u; + write_set.descriptorType = ToVKDescriptorType(type); + write_set.pBufferInfo = &buffer_workspace_[bound_buffer_offset_ - 1]; + + write_workspace_[descriptor_write_offset_++] = write_set; + return true; +} + +// |ComputePass| +bool ComputePassVK::EncodeCommands() const { return true; } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index 2e2259c8121dc..34de35bccccd0 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -5,8 +5,6 @@ #ifndef FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_COMPUTE_PASS_VK_H_ #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" @@ -22,16 +20,27 @@ class ComputePassVK final : public ComputePass { private: friend class CommandBufferVK; - std::weak_ptr command_buffer_; + std::shared_ptr command_buffer_; std::string label_; + std::array max_wg_size_ = {}; bool is_valid_ = false; - mutable std::array image_workspace_; - mutable std::array buffer_workspace_; - mutable std::array + + // Per-command state. + std::array image_workspace_; + std::array buffer_workspace_; + std::array write_workspace_; + size_t bound_image_offset_ = 0u; + size_t bound_buffer_offset_ = 0u; + size_t descriptor_write_offset_ = 0u; + bool has_label_ = false; + bool pipeline_valid_ = false; + vk::Pipeline last_pipeline_; + vk::DescriptorSet descriptor_set_ = {}; + vk::PipelineLayout pipeline_layout_ = {}; - ComputePassVK(std::weak_ptr context, - std::weak_ptr command_buffer); + ComputePassVK(std::shared_ptr context, + std::shared_ptr command_buffer); // |ComputePass| bool IsValid() const override; @@ -40,9 +49,36 @@ class ComputePassVK final : public ComputePass { void OnSetLabel(const std::string& label) override; // |ComputePass| - bool OnEncodeCommands(const Context& context, - const ISize& grid_size, - const ISize& thread_group_size) const override; + bool EncodeCommands() const override; + + // |ComputePass| + void SetCommandLabel(std::string_view label) override; + + // |ComputePass| + void SetPipeline(const std::shared_ptr>& + pipeline) override; + + // |ComputePass| + fml::Status Compute(const ISize& grid_size) override; + + // |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, + std::shared_ptr sampler) override; + + bool BindResource(size_t binding, + DescriptorType type, + const BufferView& view); }; } // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pass_bindings_cache.cc b/impeller/renderer/backend/vulkan/pass_bindings_cache.cc deleted file mode 100644 index 00d8774443824..0000000000000 --- a/impeller/renderer/backend/vulkan/pass_bindings_cache.cc +++ /dev/null @@ -1,72 +0,0 @@ -// 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/renderer/backend/vulkan/pass_bindings_cache.h" - -namespace impeller { -void PassBindingsCache::BindPipeline(vk::CommandBuffer command_buffer, - vk::PipelineBindPoint pipeline_bind_point, - vk::Pipeline pipeline) { - switch (pipeline_bind_point) { - case vk::PipelineBindPoint::eGraphics: - if (graphics_pipeline_.has_value() && - graphics_pipeline_.value() == pipeline) { - return; - } - graphics_pipeline_ = pipeline; - break; - case vk::PipelineBindPoint::eCompute: - if (compute_pipeline_.has_value() && - compute_pipeline_.value() == pipeline) { - return; - } - compute_pipeline_ = pipeline; - break; - default: - break; - } - command_buffer.bindPipeline(pipeline_bind_point, pipeline); -} - -void PassBindingsCache::SetStencilReference(vk::CommandBuffer command_buffer, - vk::StencilFaceFlags face_mask, - uint32_t reference) { - if (stencil_face_flags_.has_value() && - face_mask == stencil_face_flags_.value() && - reference == stencil_reference_) { - return; - } - stencil_face_flags_ = face_mask; - stencil_reference_ = reference; - command_buffer.setStencilReference(face_mask, reference); -} - -void PassBindingsCache::SetScissor(vk::CommandBuffer command_buffer, - uint32_t first_scissor, - uint32_t scissor_count, - const vk::Rect2D* scissors) { - if (first_scissor == 0 && scissor_count == 1) { - if (scissors_.has_value() && scissors_.value() == scissors[0]) { - return; - } - scissors_ = scissors[0]; - } - command_buffer.setScissor(first_scissor, scissor_count, scissors); -} - -void PassBindingsCache::SetViewport(vk::CommandBuffer command_buffer, - uint32_t first_viewport, - uint32_t viewport_count, - const vk::Viewport* viewports) { - if (first_viewport == 0 && viewport_count == 1) { - // Note that this is doing equality checks on floating point numbers. - if (viewport_.has_value() && viewport_.value() == viewports[0]) { - return; - } - viewport_ = viewports[0]; - } - command_buffer.setViewport(first_viewport, viewport_count, viewports); -} - -} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pass_bindings_cache.h b/impeller/renderer/backend/vulkan/pass_bindings_cache.h deleted file mode 100644 index 9eccee0ee0ee0..0000000000000 --- a/impeller/renderer/backend/vulkan/pass_bindings_cache.h +++ /dev/null @@ -1,50 +0,0 @@ -// 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_RENDERER_BACKEND_VULKAN_PASS_BINDINGS_CACHE_H_ -#define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_PASS_BINDINGS_CACHE_H_ - -#include -#include - -#include "flutter/impeller/renderer/backend/vulkan/vk.h" - -namespace impeller { - -class PassBindingsCache { - public: - void BindPipeline(vk::CommandBuffer command_buffer, - vk::PipelineBindPoint pipeline_bind_point, - vk::Pipeline pipeline); - - void SetStencilReference(vk::CommandBuffer command_buffer, - vk::StencilFaceFlags face_mask, - uint32_t reference); - - void SetScissor(vk::CommandBuffer command_buffer, - uint32_t first_scissor, - uint32_t scissor_count, - const vk::Rect2D* scissors); - - void SetViewport(vk::CommandBuffer command_buffer, - uint32_t first_viewport, - uint32_t viewport_count, - const vk::Viewport* viewports); - - private: - // bindPipeline - std::optional graphics_pipeline_; - std::optional compute_pipeline_; - // setStencilReference - std::optional stencil_face_flags_; - uint32_t stencil_reference_ = 0; - // setScissor - std::optional scissors_; - // setViewport - std::optional viewport_; -}; - -} // namespace impeller - -#endif // FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_PASS_BINDINGS_CACHE_H_ diff --git a/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc b/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc deleted file mode 100644 index e76213c200493..0000000000000 --- a/impeller/renderer/backend/vulkan/pass_bindings_cache_unittests.cc +++ /dev/null @@ -1,82 +0,0 @@ -// 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 "flutter/testing/testing.h" -#include "impeller/renderer/backend/vulkan/command_encoder_vk.h" -#include "impeller/renderer/backend/vulkan/pass_bindings_cache.h" -#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h" - -namespace impeller { -namespace testing { - -namespace { -int32_t CountStringViewInstances(const std::vector& strings, - std::string_view target) { - int32_t count = 0; - for (const std::string& str : strings) { - if (str == target) { - count++; - } - } - return count; -} -} // namespace - -TEST(PassBindingsCacheTest, bindPipeline) { - auto context = MockVulkanContextBuilder().Build(); - PassBindingsCache cache; - auto encoder = std::make_unique(context)->Create(); - auto buffer = encoder->GetCommandBuffer(); - VkPipeline vk_pipeline = reinterpret_cast(0xfeedface); - vk::Pipeline pipeline(vk_pipeline); - cache.BindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline); - cache.BindPipeline(buffer, vk::PipelineBindPoint::eGraphics, pipeline); - std::shared_ptr> functions = - GetMockVulkanFunctions(context->GetDevice()); - EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdBindPipeline"), 1); -} - -TEST(PassBindingsCacheTest, setStencilReference) { - auto context = MockVulkanContextBuilder().Build(); - PassBindingsCache cache; - auto encoder = std::make_unique(context)->Create(); - auto buffer = encoder->GetCommandBuffer(); - cache.SetStencilReference( - buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123); - cache.SetStencilReference( - buffer, vk::StencilFaceFlagBits::eVkStencilFrontAndBack, 123); - std::shared_ptr> functions = - GetMockVulkanFunctions(context->GetDevice()); - EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetStencilReference"), - 1); -} - -TEST(PassBindingsCacheTest, setScissor) { - auto context = MockVulkanContextBuilder().Build(); - PassBindingsCache cache; - auto encoder = std::make_unique(context)->Create(); - auto buffer = encoder->GetCommandBuffer(); - vk::Rect2D scissors; - cache.SetScissor(buffer, 0, 1, &scissors); - cache.SetScissor(buffer, 0, 1, &scissors); - std::shared_ptr> functions = - GetMockVulkanFunctions(context->GetDevice()); - EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetScissor"), 1); -} - -TEST(PassBindingsCacheTest, setViewport) { - auto context = MockVulkanContextBuilder().Build(); - PassBindingsCache cache; - auto encoder = std::make_unique(context)->Create(); - auto buffer = encoder->GetCommandBuffer(); - vk::Viewport viewports; - cache.SetViewport(buffer, 0, 1, &viewports); - cache.SetViewport(buffer, 0, 1, &viewports); - std::shared_ptr> functions = - GetMockVulkanFunctions(context->GetDevice()); - EXPECT_EQ(CountStringViewInstances(*functions, "vkCmdSetViewport"), 1); -} - -} // namespace testing -} // namespace impeller diff --git a/impeller/renderer/backend/vulkan/pipeline_vk.h b/impeller/renderer/backend/vulkan/pipeline_vk.h index ffe245152c4da..c27d18643347f 100644 --- a/impeller/renderer/backend/vulkan/pipeline_vk.h +++ b/impeller/renderer/backend/vulkan/pipeline_vk.h @@ -14,6 +14,10 @@ namespace impeller { +// 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; + class PipelineVK final : public Pipeline, public BackendCast> { diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.cc b/impeller/renderer/backend/vulkan/render_pass_vk.cc index c4104329e2af0..0496554593657 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/render_pass_vk.cc @@ -14,7 +14,6 @@ #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" #include "impeller/renderer/backend/vulkan/command_encoder_vk.h" #include "impeller/renderer/backend/vulkan/context_vk.h" diff --git a/impeller/renderer/backend/vulkan/render_pass_vk.h b/impeller/renderer/backend/vulkan/render_pass_vk.h index c7da1ae37348e..ad9e89d5a83e0 100644 --- a/impeller/renderer/backend/vulkan/render_pass_vk.h +++ b/impeller/renderer/backend/vulkan/render_pass_vk.h @@ -6,7 +6,6 @@ #define FLUTTER_IMPELLER_RENDERER_BACKEND_VULKAN_RENDER_PASS_VK_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/pipeline_vk.h" #include "impeller/renderer/backend/vulkan/shared_object_vk.h" diff --git a/impeller/renderer/compute_command.cc b/impeller/renderer/compute_command.cc deleted file mode 100644 index e1a06015fa611..0000000000000 --- a/impeller/renderer/compute_command.cc +++ /dev/null @@ -1,59 +0,0 @@ -// 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/renderer/compute_command.h" - -#include - -#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) { - if (stage != ShaderStage::kCompute) { - VALIDATION_LOG << "Use Command for non-compute shader stages."; - return false; - } - if (!view) { - return false; - } - - bindings.buffers.emplace_back( - BufferAndUniformSlot{.slot = slot, .view = {&metadata, std::move(view)}}); - return true; -} - -bool ComputeCommand::BindResource(ShaderStage stage, - DescriptorType type, - const SampledImageSlot& slot, - const ShaderMetadata& metadata, - std::shared_ptr texture, - std::shared_ptr sampler) { - if (stage != ShaderStage::kCompute) { - VALIDATION_LOG << "Use Command for non-compute shader stages."; - return false; - } - if (!sampler || !sampler->IsValid()) { - return false; - } - if (!texture || !texture->IsValid()) { - return false; - } - - bindings.sampled_images.emplace_back(TextureAndSampler{ - .slot = slot, - .texture = {&metadata, std::move(texture)}, - .sampler = std::move(sampler), - }); - - return false; -} - -} // namespace impeller diff --git a/impeller/renderer/compute_command.h b/impeller/renderer/compute_command.h deleted file mode 100644 index c0c56e9f698f6..0000000000000 --- a/impeller/renderer/compute_command.h +++ /dev/null @@ -1,76 +0,0 @@ -// 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_RENDERER_COMPUTE_COMMAND_H_ -#define FLUTTER_IMPELLER_RENDERER_COMPUTE_COMMAND_H_ - -#include -#include - -#include "impeller/core/buffer_view.h" -#include "impeller/core/formats.h" -#include "impeller/core/resource_binder.h" -#include "impeller/core/sampler.h" -#include "impeller/core/shader_types.h" -#include "impeller/core/texture.h" -#include "impeller/renderer/command.h" -#include "impeller/renderer/compute_pipeline_descriptor.h" -#include "impeller/renderer/pipeline.h" - -namespace impeller { - -//------------------------------------------------------------------------------ -/// @brief An object used to specify compute work to the GPU along with -/// references to resources the GPU will used when doing said work. -/// -/// To construct a valid command, follow these steps: -/// * Specify a valid pipeline. -/// * (Optional) Specify a debug label. -/// -/// Command are very lightweight objects and can be created -/// frequently and on demand. The resources referenced in commands -/// views into buffers managed by other allocators and resource -/// managers. -/// -struct ComputeCommand : public ResourceBinder { - //---------------------------------------------------------------------------- - /// The pipeline to use for this command. - /// - std::shared_ptr> pipeline; - //---------------------------------------------------------------------------- - /// The buffer, texture, and sampler bindings used by the compute pipeline - /// stage. - /// - Bindings bindings; - -#ifdef IMPELLER_DEBUG - //---------------------------------------------------------------------------- - /// The debugging label to use for the command. - /// - std::string label; -#endif // IMPELLER_DEBUG - - // |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, - std::shared_ptr sampler) override; - - constexpr explicit operator bool() const { - return pipeline && pipeline->IsValid(); - } -}; - -} // namespace impeller - -#endif // FLUTTER_IMPELLER_RENDERER_COMPUTE_COMMAND_H_ diff --git a/impeller/renderer/compute_pass.cc b/impeller/renderer/compute_pass.cc index 8446fffdc53a2..e069aca32e1cc 100644 --- a/impeller/renderer/compute_pass.cc +++ b/impeller/renderer/compute_pass.cc @@ -3,14 +3,10 @@ // found in the LICENSE file. #include "impeller/renderer/compute_pass.h" -#include - -#include "fml/logging.h" -#include "impeller/base/validation.h" namespace impeller { -ComputePass::ComputePass(std::weak_ptr context) +ComputePass::ComputePass(std::shared_ptr context) : context_(std::move(context)) {} ComputePass::~ComputePass() = default; @@ -22,37 +18,4 @@ void ComputePass::SetLabel(const std::string& label) { OnSetLabel(label); } -void ComputePass::SetGridSize(const ISize& size) { - grid_size_ = size; -} - -void ComputePass::SetThreadGroupSize(const ISize& size) { - thread_group_size_ = size; -} - -bool ComputePass::AddCommand(ComputeCommand command) { - if (!command) { - VALIDATION_LOG - << "Attempted to add an invalid command to the compute pass."; - return false; - } - - commands_.emplace_back(std::move(command)); - return true; -} - -bool ComputePass::EncodeCommands() const { - if (grid_size_.IsEmpty() || thread_group_size_.IsEmpty()) { - FML_DLOG(WARNING) << "Attempted to encode a compute pass with an empty " - "grid or thread group size."; - return false; - } - auto context = context_.lock(); - // The context could have been collected in the meantime. - if (!context) { - return false; - } - return OnEncodeCommands(*context, grid_size_, thread_group_size_); -} - } // namespace impeller diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 4ba6e802c3275..40c313ecf203c 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -7,20 +7,20 @@ #include -#include "impeller/renderer/compute_command.h" +#include "fml/status.h" +#include "impeller/core/resource_binder.h" +#include "impeller/renderer/compute_pipeline_descriptor.h" +#include "impeller/renderer/pipeline_descriptor.h" namespace impeller { -class HostBuffer; -class Allocator; - //------------------------------------------------------------------------------ /// @brief Compute passes encode compute shader into the underlying command /// buffer. /// /// @see `CommandBuffer` /// -class ComputePass { +class ComputePass : public ResourceBinder { public: virtual ~ComputePass(); @@ -28,48 +28,29 @@ class ComputePass { void SetLabel(const std::string& label); - void SetGridSize(const ISize& size); + virtual void SetCommandLabel(std::string_view label) = 0; - void SetThreadGroupSize(const ISize& size); + virtual void SetPipeline( + const std::shared_ptr>& pipeline) = 0; - //---------------------------------------------------------------------------- - /// @brief Record a command for subsequent encoding to the underlying - /// command buffer. No work is encoded into the command buffer at - /// this time. - /// - /// @param[in] command The command - /// - /// @return If the command was valid for subsequent commitment. - /// - bool AddCommand(ComputeCommand command); + virtual fml::Status Compute(const ISize& grid_size) = 0; //---------------------------------------------------------------------------- /// @brief Encode the recorded commands to the underlying command buffer. /// - /// @param transients_allocator The transients allocator. - /// /// @return If the commands were encoded to the underlying command /// buffer. /// - bool EncodeCommands() const; + virtual bool EncodeCommands() const = 0; protected: - const std::weak_ptr context_; - std::vector commands_; + const std::shared_ptr context_; - explicit ComputePass(std::weak_ptr context); + explicit ComputePass(std::shared_ptr context); virtual void OnSetLabel(const std::string& label) = 0; - virtual bool OnEncodeCommands(const Context& context, - const ISize& grid_size, - const ISize& thread_group_size) const = 0; - private: - std::shared_ptr transients_buffer_; - ISize grid_size_ = ISize(32, 32); - ISize thread_group_size_ = ISize(32, 32); - ComputePass(const ComputePass&) = delete; ComputePass& operator=(const ComputePass&) = delete; diff --git a/impeller/renderer/compute_subgroup_unittests.cc b/impeller/renderer/compute_subgroup_unittests.cc index 441edd9b0181a..757fde04af584 100644 --- a/impeller/renderer/compute_subgroup_unittests.cc +++ b/impeller/renderer/compute_subgroup_unittests.cc @@ -24,7 +24,6 @@ #include "impeller/geometry/path_component.h" #include "impeller/playground/compute_playground_test.h" #include "impeller/renderer/command_buffer.h" -#include "impeller/renderer/compute_command.h" #include "impeller/renderer/compute_pipeline_builder.h" #include "impeller/renderer/compute_tessellator.h" #include "impeller/renderer/path_polyline.comp.h" diff --git a/impeller/renderer/compute_tessellator.cc b/impeller/renderer/compute_tessellator.cc index ebcf76d615d74..5cd9877489a7b 100644 --- a/impeller/renderer/compute_tessellator.cc +++ b/impeller/renderer/compute_tessellator.cc @@ -121,21 +121,17 @@ ComputeTessellator::Status ComputeTessellator::Tessellate( context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get(); FML_DCHECK(compute_pipeline); - pass->SetGridSize(ISize(line_count, 1)); - pass->SetThreadGroupSize(ISize(line_count, 1)); + pass->SetPipeline(compute_pipeline); + pass->SetCommandLabel("Generate Polyline"); - ComputeCommand cmd; - DEBUG_COMMAND_INFO(cmd, "Generate Polyline"); - cmd.pipeline = compute_pipeline; + PS::BindConfig(*pass, host_buffer.EmplaceUniform(config)); + PS::BindCubics(*pass, host_buffer.EmplaceStorageBuffer(cubics)); + PS::BindQuads(*pass, host_buffer.EmplaceStorageBuffer(quads)); + PS::BindLines(*pass, host_buffer.EmplaceStorageBuffer(lines)); + PS::BindComponents(*pass, host_buffer.EmplaceStorageBuffer(components)); + PS::BindPolyline(*pass, DeviceBuffer::AsBufferView(polyline_buffer)); - PS::BindConfig(cmd, host_buffer.EmplaceUniform(config)); - PS::BindCubics(cmd, host_buffer.EmplaceStorageBuffer(cubics)); - PS::BindQuads(cmd, host_buffer.EmplaceStorageBuffer(quads)); - PS::BindLines(cmd, host_buffer.EmplaceStorageBuffer(lines)); - PS::BindComponents(cmd, host_buffer.EmplaceStorageBuffer(components)); - PS::BindPolyline(cmd, DeviceBuffer::AsBufferView(polyline_buffer)); - - if (!pass->AddCommand(std::move(cmd))) { + if (!pass->Compute(ISize(line_count, 1)).ok()) { return Status::kCommandInvalid; } } @@ -149,12 +145,8 @@ ComputeTessellator::Status ComputeTessellator::Tessellate( context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get(); FML_DCHECK(compute_pipeline); - pass->SetGridSize(ISize(line_count, 1)); - pass->SetThreadGroupSize(ISize(line_count, 1)); - - ComputeCommand cmd; - DEBUG_COMMAND_INFO(cmd, "Compute Stroke"); - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); + pass->SetCommandLabel("Compute Stroke"); SS::Config config{ .width = stroke_width_, @@ -162,13 +154,13 @@ ComputeTessellator::Status ComputeTessellator::Tessellate( .join = static_cast(stroke_join_), .miter_limit = miter_limit_, }; - SS::BindConfig(cmd, host_buffer.EmplaceUniform(config)); + SS::BindConfig(*pass, host_buffer.EmplaceUniform(config)); - SS::BindPolyline(cmd, DeviceBuffer::AsBufferView(polyline_buffer)); - SS::BindVertexBufferCount(cmd, std::move(vertex_buffer_count)); - SS::BindVertexBuffer(cmd, std::move(vertex_buffer)); + SS::BindPolyline(*pass, DeviceBuffer::AsBufferView(polyline_buffer)); + SS::BindVertexBufferCount(*pass, std::move(vertex_buffer_count)); + SS::BindVertexBuffer(*pass, std::move(vertex_buffer)); - if (!pass->AddCommand(std::move(cmd))) { + if (!pass->Compute(ISize(line_count, 1)).ok()) { return Status::kCommandInvalid; } } diff --git a/impeller/renderer/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index ce416c65a0d2d..d8fd55d875c0f 100644 --- a/impeller/renderer/compute_unittests.cc +++ b/impeller/renderer/compute_unittests.cc @@ -3,20 +3,14 @@ // found in the LICENSE file. #include "flutter/fml/synchronization/waitable_event.h" -#include "flutter/fml/time/time_point.h" #include "flutter/testing/testing.h" #include "gmock/gmock.h" -#include "impeller/base/strings.h" -#include "impeller/core/formats.h" #include "impeller/core/host_buffer.h" #include "impeller/fixtures/sample.comp.h" #include "impeller/fixtures/stage1.comp.h" #include "impeller/fixtures/stage2.comp.h" -#include "impeller/geometry/path.h" -#include "impeller/geometry/path_component.h" #include "impeller/playground/compute_playground_test.h" #include "impeller/renderer/command_buffer.h" -#include "impeller/renderer/compute_command.h" #include "impeller/renderer/compute_pipeline_builder.h" #include "impeller/renderer/pipeline_library.h" #include "impeller/renderer/prefix_sum_test.comp.h" @@ -54,11 +48,7 @@ TEST_P(ComputeTest, CanCreateComputePass) { static constexpr size_t kCount = 5; - pass->SetGridSize(ISize(kCount, 1)); - pass->SetThreadGroupSize(ISize(kCount, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); CS::Info info{.count = kCount}; CS::Input0 input_0; @@ -76,12 +66,12 @@ TEST_P(ComputeTest, CanCreateComputePass) { auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindInfo(cmd, host_buffer->EmplaceUniform(info)); - CS::BindInput0(cmd, host_buffer->EmplaceStorageBuffer(input_0)); - CS::BindInput1(cmd, host_buffer->EmplaceStorageBuffer(input_1)); - CS::BindOutput(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindInfo(*pass, host_buffer->EmplaceUniform(info)); + CS::BindInput0(*pass, host_buffer->EmplaceStorageBuffer(input_0)); + CS::BindInput1(*pass, host_buffer->EmplaceStorageBuffer(input_1)); + CS::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + ASSERT_TRUE(pass->Compute(ISize(kCount, 1)).ok()); ASSERT_TRUE(pass->EncodeCommands()); fml::AutoResetWaitableEvent latch; @@ -131,11 +121,7 @@ TEST_P(ComputeTest, CanComputePrefixSum) { static constexpr size_t kCount = 5; - pass->SetGridSize(ISize(kCount, 1)); - pass->SetThreadGroupSize(ISize(kCount, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); CS::InputData input_data; input_data.count = kCount; @@ -146,10 +132,10 @@ TEST_P(ComputeTest, CanComputePrefixSum) { auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindInputData(cmd, host_buffer->EmplaceStorageBuffer(input_data)); - CS::BindOutputData(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindInputData(*pass, host_buffer->EmplaceStorageBuffer(input_data)); + CS::BindOutputData(*pass, DeviceBuffer::AsBufferView(output_buffer)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + ASSERT_TRUE(pass->Compute(ISize(kCount, 1)).ok()); ASSERT_TRUE(pass->EncodeCommands()); fml::AutoResetWaitableEvent latch; @@ -195,18 +181,14 @@ TEST_P(ComputeTest, 1DThreadgroupSizingIsCorrect) { static constexpr size_t kCount = 2048; - pass->SetGridSize(ISize(kCount, 1)); - pass->SetThreadGroupSize(ISize(kCount, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindOutputData(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindOutputData(*pass, DeviceBuffer::AsBufferView(output_buffer)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + ASSERT_TRUE(pass->Compute(ISize(kCount, 1)).ok()); ASSERT_TRUE(pass->EncodeCommands()); fml::AutoResetWaitableEvent latch; @@ -248,10 +230,7 @@ TEST_P(ComputeTest, CanComputePrefixSumLargeInteractive) { static constexpr size_t kCount = 1023; - pass->SetGridSize(ISize(kCount, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); CS::InputData input_data; input_data.count = kCount; @@ -262,10 +241,10 @@ TEST_P(ComputeTest, CanComputePrefixSumLargeInteractive) { auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindInputData(cmd, host_buffer->EmplaceStorageBuffer(input_data)); - CS::BindOutputData(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindInputData(*pass, host_buffer->EmplaceStorageBuffer(input_data)); + CS::BindOutputData(*pass, DeviceBuffer::AsBufferView(output_buffer)); - pass->AddCommand(std::move(cmd)); + pass->Compute(ISize(kCount, 1)); pass->EncodeCommands(); host_buffer->Reset(); return cmd_buffer->SubmitCommands(); @@ -305,9 +284,6 @@ TEST_P(ComputeTest, MultiStageInputAndOutput) { static constexpr size_t kCount1 = 5; static constexpr size_t kCount2 = kCount1 * 2; - pass->SetGridSize(ISize(512, 1)); - pass->SetThreadGroupSize(ISize(512, 1)); - CS1::Input input_1; input_1.count = kCount1; for (size_t i = 0; i < kCount1; i++) { @@ -326,22 +302,20 @@ TEST_P(ComputeTest, MultiStageInputAndOutput) { context, "Output Buffer Stage 2"); { - ComputeCommand cmd; - cmd.pipeline = compute_pipeline_1; + pass->SetPipeline(compute_pipeline_1); - CS1::BindInput(cmd, host_buffer->EmplaceStorageBuffer(input_1)); - CS1::BindOutput(cmd, DeviceBuffer::AsBufferView(output_buffer_1)); + CS1::BindInput(*pass, host_buffer->EmplaceStorageBuffer(input_1)); + CS1::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer_1)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + ASSERT_TRUE(pass->Compute(ISize(512, 1)).ok()); } { - ComputeCommand cmd; - cmd.pipeline = compute_pipeline_2; + pass->SetPipeline(compute_pipeline_2); - CS1::BindInput(cmd, DeviceBuffer::AsBufferView(output_buffer_1)); - CS2::BindOutput(cmd, DeviceBuffer::AsBufferView(output_buffer_2)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + CS1::BindInput(*pass, DeviceBuffer::AsBufferView(output_buffer_1)); + CS2::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer_2)); + ASSERT_TRUE(pass->Compute(ISize(512, 1)).ok()); } ASSERT_TRUE(pass->EncodeCommands()); @@ -393,10 +367,7 @@ TEST_P(ComputeTest, CanCompute1DimensionalData) { static constexpr size_t kCount = 5; - pass->SetGridSize(ISize(kCount, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); CS::Info info{.count = kCount}; CS::Input0 input_0; @@ -414,12 +385,12 @@ TEST_P(ComputeTest, CanCompute1DimensionalData) { auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindInfo(cmd, host_buffer->EmplaceUniform(info)); - CS::BindInput0(cmd, host_buffer->EmplaceStorageBuffer(input_0)); - CS::BindInput1(cmd, host_buffer->EmplaceStorageBuffer(input_1)); - CS::BindOutput(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindInfo(*pass, host_buffer->EmplaceUniform(info)); + CS::BindInput0(*pass, host_buffer->EmplaceStorageBuffer(input_0)); + CS::BindInput1(*pass, host_buffer->EmplaceStorageBuffer(input_1)); + CS::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); + ASSERT_TRUE(pass->Compute(ISize(kCount, 1)).ok()); ASSERT_TRUE(pass->EncodeCommands()); fml::AutoResetWaitableEvent latch; @@ -469,13 +440,7 @@ TEST_P(ComputeTest, ReturnsEarlyWhenAnyGridDimensionIsZero) { static constexpr size_t kCount = 5; - // Intentionally making the grid size zero in one dimension. No GPU will - // tolerate this. - pass->SetGridSize(ISize(0, 1)); - pass->SetThreadGroupSize(ISize(0, 1)); - - ComputeCommand cmd; - cmd.pipeline = compute_pipeline; + pass->SetPipeline(compute_pipeline); CS::Info info{.count = kCount}; CS::Input0 input_0; @@ -493,13 +458,14 @@ TEST_P(ComputeTest, ReturnsEarlyWhenAnyGridDimensionIsZero) { auto output_buffer = CreateHostVisibleDeviceBuffer>( context, "Output Buffer"); - CS::BindInfo(cmd, host_buffer->EmplaceUniform(info)); - CS::BindInput0(cmd, host_buffer->EmplaceStorageBuffer(input_0)); - CS::BindInput1(cmd, host_buffer->EmplaceStorageBuffer(input_1)); - CS::BindOutput(cmd, DeviceBuffer::AsBufferView(output_buffer)); + CS::BindInfo(*pass, host_buffer->EmplaceUniform(info)); + CS::BindInput0(*pass, host_buffer->EmplaceStorageBuffer(input_0)); + CS::BindInput1(*pass, host_buffer->EmplaceStorageBuffer(input_1)); + CS::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer)); - ASSERT_TRUE(pass->AddCommand(std::move(cmd))); - ASSERT_FALSE(pass->EncodeCommands()); + // Intentionally making the grid size zero in one dimension. No GPU will + // tolerate this. + ASSERT_FALSE(pass->Compute(ISize(0, 1)).ok()); } } // namespace testing From 94e274e3849f96c8c90cf0053a8302922f535e9f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 20 Jan 2024 20:32:39 -0800 Subject: [PATCH 02/12] [Impeller] fix compute barriers for Vulkan. --- .../entity/geometry/point_field_geometry.cc | 5 +- .../renderer/backend/metal/compute_pass_mtl.h | 7 ++ .../backend/metal/compute_pass_mtl.mm | 14 +++- .../backend/vulkan/capabilities_vk.cc | 11 +-- .../renderer/backend/vulkan/capabilities_vk.h | 3 +- .../backend/vulkan/compute_pass_vk.cc | 67 ++++++++++++++++++- .../renderer/backend/vulkan/compute_pass_vk.h | 7 ++ .../renderer/backend/vulkan/context_vk.cc | 13 ++-- impeller/renderer/compute_pass.h | 13 ++++ 9 files changed, 126 insertions(+), 14 deletions(-) diff --git a/impeller/entity/geometry/point_field_geometry.cc b/impeller/entity/geometry/point_field_geometry.cc index 0e7bf69476ce8..20e7013856f25 100644 --- a/impeller/entity/geometry/point_field_geometry.cc +++ b/impeller/entity/geometry/point_field_geometry.cc @@ -200,6 +200,7 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( using UV = UvComputeShader; + compute_pass->AddBufferMemoryBarrier(geometry_buffer); compute_pass->SetCommandLabel("UV Geometry"); compute_pass->SetPipeline(renderer.GetUvComputePipeline()); @@ -267,9 +268,7 @@ GeometryVertexType PointFieldGeometry::GetVertexType() const { // Compute is disabled for Vulkan because the barriers are incorrect, see // also: https://github.com/flutter/flutter/issues/140798 . bool PointFieldGeometry::CanUseCompute(const ContentContext& renderer) { - return renderer.GetDeviceCapabilities().SupportsCompute() && - renderer.GetContext()->GetBackendType() == - Context::BackendType::kMetal; + return renderer.GetDeviceCapabilities().SupportsCompute(); } // |Geometry| diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.h b/impeller/renderer/backend/metal/compute_pass_mtl.h index 757e2f1127c61..c273b46ebb4a2 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.h +++ b/impeller/renderer/backend/metal/compute_pass_mtl.h @@ -150,6 +150,13 @@ class ComputePassMTL final : public ComputePass { // |ComputePass| bool EncodeCommands() const override; + // |ComputePass| + void AddBufferMemoryBarrier(const BufferView& view) override; + + // |ComputePass| + void AddTextureMemoryBarrier( + const std::shared_ptr& texture) override; + ComputePassMTL(const ComputePassMTL&) = delete; ComputePassMTL& operator=(const ComputePassMTL&) = delete; diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index 6b3bf3a63341d..b6db408469089 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -32,7 +32,8 @@ if (!buffer_) { return; } - encoder_ = [buffer_ computeCommandEncoder]; + encoder_ = [buffer_ computeCommandEncoderWithDispatchType: + MTLDispatchType::MTLDispatchTypeConcurrent]; if (!encoder_) { return; } @@ -67,6 +68,17 @@ ComputePipelineMTL::Cast(*pipeline).GetMTLComputePipelineState()); } +// |ComputePass| +void ComputePassMTL::AddBufferMemoryBarrier(const BufferView& view) { + [encoder_ memoryBarrierWithScope:MTLBarrierScopeBuffers]; +} + +// |ComputePass| +void ComputePassMTL::AddTextureMemoryBarrier( + const std::shared_ptr& texture) { + [encoder_ memoryBarrierWithScope:MTLBarrierScopeTextures]; +} + // |ComputePass| bool ComputePassMTL::BindResource(ShaderStage stage, DescriptorType type, diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 4d83152795af7..1646d6a8930c8 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -65,7 +65,8 @@ bool CapabilitiesVK::AreValidationsEnabled() const { std::optional> CapabilitiesVK::GetEnabledLayers() const { - std::vector required; + // VK_LAYER_KHRONOS_synchronization2 ? + std::vector required = {}; if (validations_enabled_) { // The presence of this layer is already checked in the ctor. @@ -161,6 +162,8 @@ static const char* GetDeviceExtensionName(OptionalDeviceExtensionVK ext) { return VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess: return VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; + case OptionalDeviceExtensionVK::kKHRSynchronization2: + return VK_KHR_SYNCHRONIZATION_2_EXTENSION_NAME; case OptionalDeviceExtensionVK::kLast: return "Unknown"; } @@ -287,7 +290,7 @@ static bool HasRequiredQueues(const vk::PhysicalDevice& physical_device) { vk::QueueFlagBits::eTransfer)); } -std::optional +std::optional CapabilitiesVK::GetEnabledDeviceFeatures( const vk::PhysicalDevice& device) const { if (!PhysicalDeviceSupportsRequiredFormats(device)) { @@ -312,11 +315,11 @@ CapabilitiesVK::GetEnabledDeviceFeatures( const auto device_features = device.getFeatures(); - vk::PhysicalDeviceFeatures required; + vk::PhysicalDeviceFeatures2 required; // We require this for enabling wireframes in the playground. But its not // necessarily a big deal if we don't have this feature. - required.fillModeNonSolid = device_features.fillModeNonSolid; + required.features.fillModeNonSolid = device_features.fillModeNonSolid; return required; } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index 4cb7ce8996600..c1d09ff89a323 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -24,6 +24,7 @@ enum class OptionalDeviceExtensionVK : uint32_t { kEXTPipelineCreationFeedback, kARMRasterizationOrderAttachmentAccess, kEXTRasterizationOrderAttachmentAccess, + kKHRSynchronization2, kLast, }; @@ -50,7 +51,7 @@ class CapabilitiesVK final : public Capabilities, std::optional> GetEnabledDeviceExtensions( const vk::PhysicalDevice& physical_device) const; - std::optional GetEnabledDeviceFeatures( + std::optional GetEnabledDeviceFeatures( const vk::PhysicalDevice& physical_device) const; [[nodiscard]] bool SetPhysicalDevice( diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 5c5a4a02a812d..4f37d804b1a65 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -4,11 +4,15 @@ #include "impeller/renderer/backend/vulkan/compute_pass_vk.h" +#include "impeller/renderer/backend/vulkan/barrier_vk.h" #include "impeller/renderer/backend/vulkan/command_buffer_vk.h" #include "impeller/renderer/backend/vulkan/compute_pipeline_vk.h" +#include "impeller/renderer/backend/vulkan/device_buffer_vk.h" #include "impeller/renderer/backend/vulkan/formats_vk.h" #include "impeller/renderer/backend/vulkan/sampler_vk.h" #include "impeller/renderer/backend/vulkan/texture_vk.h" +#include "vulkan/vulkan_enums.hpp" +#include "vulkan/vulkan_structs.hpp" namespace impeller { @@ -18,7 +22,7 @@ ComputePassVK::ComputePassVK(std::shared_ptr context, command_buffer_(std::move(command_buffer)) { // TOOD(dnfield): This should be moved to caps. But for now keeping this // in parallel with Metal. - max_wg_size_ = ContextVK::Cast(*context) + max_wg_size_ = ContextVK::Cast(GetContext()) .GetPhysicalDevice() .getProperties() .limits.maxComputeWorkGroupSize; @@ -207,8 +211,69 @@ bool ComputePassVK::BindResource(size_t binding, return true; } +// Note: +// https://github.com/KhronosGroup/Vulkan-Docs/wiki/Synchronization-Examples +// Seems to suggest that anything more finely grained than a global memory +// barrier is likely to be ignored. Confirming this on mobile devices will +// require some experimentation. + +// |ComputePass| +void ComputePassVK::AddBufferMemoryBarrier(const BufferView& view) { + vk::MemoryBarrier2KHR barrier; + barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; + barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; + barrier.dstStageMask = vk::PipelineStageFlagBits2::eComputeShader; + barrier.dstAccessMask = vk::AccessFlagBits2::eShaderRead; + + vk::DependencyInfo dependency_info; + dependency_info.setMemoryBarrierCount(1); + dependency_info.pMemoryBarriers = &barrier; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( + dependency_info); +} + +// |ComputePass| +void ComputePassVK::AddTextureMemoryBarrier( + const std::shared_ptr& texture) { + vk::MemoryBarrier2KHR barrier; + barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; + barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; + barrier.dstStageMask = vk::PipelineStageFlagBits2::eComputeShader; + barrier.dstAccessMask = vk::AccessFlagBits2::eShaderRead; + + vk::DependencyInfo dependency_info; + dependency_info.setMemoryBarrierCount(1); + dependency_info.pMemoryBarriers = &barrier; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( + dependency_info); +} + // |ComputePass| bool ComputePassVK::EncodeCommands() const { + // Since we only use global memory barrier, we don't have to worry about + // compute to compute dependencies across cmd buffers. Instead, we pessimize + // here and assume that we wrote to a storage image or buffer and that a + // render pass will read from it. if there are ever scenarios where we end up + // with compute to compute dependencies this should be revisited. + + // This does not currently handle image barriers as we do not use them + // for anything. + vk::MemoryBarrier2KHR barrier; + barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; + barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; + barrier.dstStageMask = vk::PipelineStageFlagBits2::eVertexInput; + barrier.dstAccessMask = vk::AccessFlagBits2::eIndexRead | + vk::AccessFlagBits2::eVertexAttributeRead; + + vk::DependencyInfo dependency_info; + dependency_info.setMemoryBarrierCount(1); + dependency_info.pMemoryBarriers = &barrier; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( + dependency_info); + return true; } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index 34de35bccccd0..006dd3feca585 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -58,6 +58,13 @@ class ComputePassVK final : public ComputePass { void SetPipeline(const std::shared_ptr>& pipeline) override; + // |ComputePass| + void AddBufferMemoryBarrier(const BufferView& view) override; + + // |ComputePass| + void AddTextureMemoryBarrier( + const std::shared_ptr& texture) override; + // |ComputePass| fml::Status Compute(const ISize& grid_size) override; diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index e573d6aa5bfb9..261c72b642024 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; @@ -308,19 +308,24 @@ void ContextVK::Setup(Settings settings) { const auto queue_create_infos = GetQueueCreateInfos( {graphics_queue.value(), compute_queue.value(), transfer_queue.value()}); - const auto enabled_features = + const auto enabled_features_x = caps->GetEnabledDeviceFeatures(device_holder->physical_device); - if (!enabled_features.has_value()) { + if (!enabled_features_x.has_value()) { // This shouldn't happen since the device can't be picked if this was not // true. But doesn't hurt to check. return; } + vk::PhysicalDeviceFeatures2 enabled_features = enabled_features_x.value(); + vk::PhysicalDeviceSynchronization2Features physical_features; + physical_features.synchronization2 = true; + enabled_features.pNext = &physical_features; + vk::DeviceCreateInfo device_info; device_info.setQueueCreateInfos(queue_create_infos); device_info.setPEnabledExtensionNames(enabled_device_extensions_c); - device_info.setPEnabledFeatures(&enabled_features.value()); + device_info.setPNext(&enabled_features); // Device layers are deprecated and ignored. { diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 40c313ecf203c..2069a186f033c 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -35,6 +35,17 @@ class ComputePass : public ResourceBinder { virtual fml::Status Compute(const ISize& grid_size) = 0; + /// @brief Add a barrier that ensures all previously encoded commands have + /// finished reading or writing to the provided buffer before + /// subsequent commands will access it. + virtual void AddBufferMemoryBarrier(const BufferView& view) = 0; + + /// @brief Add a barrier that ensures all previously encoded commands have + /// finished reading or writing to the provided texture before + /// subsequent commands will access it. + virtual void AddTextureMemoryBarrier( + const std::shared_ptr& texture) = 0; + //---------------------------------------------------------------------------- /// @brief Encode the recorded commands to the underlying command buffer. /// @@ -43,6 +54,8 @@ class ComputePass : public ResourceBinder { /// virtual bool EncodeCommands() const = 0; + const Context& GetContext() const { return *context_; } + protected: const std::shared_ptr context_; From e04973020a1c3b93308b925be43824bc568b12f8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 21 Jan 2024 10:08:48 -0800 Subject: [PATCH 03/12] use old synchronization. --- .../backend/vulkan/capabilities_vk.cc | 3 - .../renderer/backend/vulkan/capabilities_vk.h | 1 - .../backend/vulkan/compute_pass_vk.cc | 59 +++++++------------ .../renderer/backend/vulkan/context_vk.cc | 2 +- 4 files changed, 23 insertions(+), 42 deletions(-) diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index 1646d6a8930c8..b0d0a825786d1 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -65,7 +65,6 @@ bool CapabilitiesVK::AreValidationsEnabled() const { std::optional> CapabilitiesVK::GetEnabledLayers() const { - // VK_LAYER_KHRONOS_synchronization2 ? std::vector required = {}; if (validations_enabled_) { @@ -162,8 +161,6 @@ static const char* GetDeviceExtensionName(OptionalDeviceExtensionVK ext) { return VK_ARM_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; case OptionalDeviceExtensionVK::kEXTRasterizationOrderAttachmentAccess: return VK_EXT_RASTERIZATION_ORDER_ATTACHMENT_ACCESS_EXTENSION_NAME; - case OptionalDeviceExtensionVK::kKHRSynchronization2: - return VK_KHR_SYNCHRONIZATION_2_EXTENSION_NAME; case OptionalDeviceExtensionVK::kLast: return "Unknown"; } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index c1d09ff89a323..dc2dd2c487bf1 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -24,7 +24,6 @@ enum class OptionalDeviceExtensionVK : uint32_t { kEXTPipelineCreationFeedback, kARMRasterizationOrderAttachmentAccess, kEXTRasterizationOrderAttachmentAccess, - kKHRSynchronization2, kLast, }; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 4f37d804b1a65..1a11a3a13620c 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -219,35 +219,25 @@ bool ComputePassVK::BindResource(size_t binding, // |ComputePass| void ComputePassVK::AddBufferMemoryBarrier(const BufferView& view) { - vk::MemoryBarrier2KHR barrier; - barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; - barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; - barrier.dstStageMask = vk::PipelineStageFlagBits2::eComputeShader; - barrier.dstAccessMask = vk::AccessFlagBits2::eShaderRead; - - vk::DependencyInfo dependency_info; - dependency_info.setMemoryBarrierCount(1); - dependency_info.pMemoryBarriers = &barrier; - - command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( - dependency_info); + vk::MemoryBarrier barrier; + barrier.srcAccessMask = vk::AccessFlagBits::eShaderWrite; + barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier( + vk::PipelineStageFlagBits::eComputeShader, + vk::PipelineStageFlagBits::eComputeShader, {}, 1, &barrier, 0, {}, 0, {}); } // |ComputePass| void ComputePassVK::AddTextureMemoryBarrier( const std::shared_ptr& texture) { - vk::MemoryBarrier2KHR barrier; - barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; - barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; - barrier.dstStageMask = vk::PipelineStageFlagBits2::eComputeShader; - barrier.dstAccessMask = vk::AccessFlagBits2::eShaderRead; - - vk::DependencyInfo dependency_info; - dependency_info.setMemoryBarrierCount(1); - dependency_info.pMemoryBarriers = &barrier; - - command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( - dependency_info); + vk::MemoryBarrier barrier; + barrier.srcAccessMask = vk::AccessFlagBits::eShaderWrite; + barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier( + vk::PipelineStageFlagBits::eComputeShader, + vk::PipelineStageFlagBits::eComputeShader, {}, 1, &barrier, 0, {}, 0, {}); } // |ComputePass| @@ -260,19 +250,14 @@ bool ComputePassVK::EncodeCommands() const { // This does not currently handle image barriers as we do not use them // for anything. - vk::MemoryBarrier2KHR barrier; - barrier.srcStageMask = vk::PipelineStageFlagBits2::eComputeShader; - barrier.srcAccessMask = vk::AccessFlagBits2::eShaderWrite; - barrier.dstStageMask = vk::PipelineStageFlagBits2::eVertexInput; - barrier.dstAccessMask = vk::AccessFlagBits2::eIndexRead | - vk::AccessFlagBits2::eVertexAttributeRead; - - vk::DependencyInfo dependency_info; - dependency_info.setMemoryBarrierCount(1); - dependency_info.pMemoryBarriers = &barrier; - - command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier2KHR( - dependency_info); + vk::MemoryBarrier barrier; + barrier.srcAccessMask = vk::AccessFlagBits::eShaderWrite; + barrier.dstAccessMask = + vk::AccessFlagBits::eIndexRead | vk::AccessFlagBits::eVertexAttributeRead; + + command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier( + vk::PipelineStageFlagBits::eComputeShader, + vk::PipelineStageFlagBits::eVertexShader, {}, 1, &barrier, 0, {}, 0, {}); return true; } diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 261c72b642024..37b7869ed7503 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 30e474c8e8800c29fce6a0be97da29115ae40bab Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 21 Jan 2024 10:18:40 -0800 Subject: [PATCH 04/12] fix stage masks. --- impeller/renderer/backend/vulkan/compute_pass_vk.cc | 2 +- impeller/renderer/backend/vulkan/context_vk.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 1a11a3a13620c..25721fcedf0db 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -257,7 +257,7 @@ bool ComputePassVK::EncodeCommands() const { command_buffer_->GetEncoder()->GetCommandBuffer().pipelineBarrier( vk::PipelineStageFlagBits::eComputeShader, - vk::PipelineStageFlagBits::eVertexShader, {}, 1, &barrier, 0, {}, 0, {}); + vk::PipelineStageFlagBits::eVertexInput, {}, 1, &barrier, 0, {}, 0, {}); return true; } diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 37b7869ed7503..b92f3dcb0fc78 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 = settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; From 802de2142edbe4595b8331e2ce6c572686eb22d9 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 16:14:27 -0800 Subject: [PATCH 05/12] cleanup --- .../renderer/backend/metal/compute_pass_mtl.h | 86 ------------------- .../backend/vulkan/capabilities_vk.cc | 8 +- .../renderer/backend/vulkan/capabilities_vk.h | 2 +- 3 files changed, 5 insertions(+), 91 deletions(-) diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.h b/impeller/renderer/backend/metal/compute_pass_mtl.h index 1539af3f9efb4..41fe3c41c2c2b 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.h +++ b/impeller/renderer/backend/metal/compute_pass_mtl.h @@ -14,92 +14,6 @@ namespace impeller { -//----------------------------------------------------------------------------- -/// @brief Ensures that bindings on the pass are not redundantly set or -/// updated. Avoids making the driver do additional checks and makes -/// the frame insights during profiling and instrumentation not -/// complain about the same. -/// -/// There should be no change to rendering if this caching was -/// absent. -/// -struct ComputePassBindingsCache { - explicit ComputePassBindingsCache() {} - - ComputePassBindingsCache(const ComputePassBindingsCache&) = delete; - - ComputePassBindingsCache(ComputePassBindingsCache&&) = delete; - - void SetComputePipelineState(id pipeline) { - if (pipeline == pipeline_) { - return; - } - pipeline_ = pipeline; - [encoder_ setComputePipelineState:pipeline_]; - } - - id GetPipeline() const { return pipeline_; } - - void SetEncoder(id encoder) { encoder_ = encoder; } - - void SetBuffer(uint64_t index, uint64_t offset, id buffer) { - auto found = buffers_.find(index); - if (found != buffers_.end() && found->second.buffer == buffer) { - // The right buffer is bound. Check if its offset needs to be updated. - if (found->second.offset == offset) { - // Buffer and its offset is identical. Nothing to do. - return; - } - - // Only the offset needs to be updated. - found->second.offset = offset; - - [encoder_ setBufferOffset:offset atIndex:index]; - return; - } - - buffers_[index] = {buffer, static_cast(offset)}; - [encoder_ setBuffer:buffer offset:offset atIndex:index]; - } - - void SetTexture(uint64_t index, id texture) { - auto found = textures_.find(index); - if (found != textures_.end() && found->second == texture) { - // Already bound. - return; - } - textures_[index] = texture; - [encoder_ setTexture:texture atIndex:index]; - return; - } - - void SetSampler(uint64_t index, id sampler) { - auto found = samplers_.find(index); - if (found != samplers_.end() && found->second == sampler) { - // Already bound. - return; - } - samplers_[index] = sampler; - [encoder_ setSamplerState:sampler atIndex:index]; - return; - } - - private: - struct BufferOffsetPair { - id buffer = nullptr; - size_t offset = 0u; - }; - using BufferMap = std::map; - using TextureMap = std::map>; - using SamplerMap = std::map>; - - id encoder_; - id pipeline_ = nullptr; - BufferMap buffers_; - TextureMap textures_; - SamplerMap samplers_; -}; - class ComputePassMTL final : public ComputePass { public: // |RenderPass| diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.cc b/impeller/renderer/backend/vulkan/capabilities_vk.cc index b0d0a825786d1..4d83152795af7 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.cc +++ b/impeller/renderer/backend/vulkan/capabilities_vk.cc @@ -65,7 +65,7 @@ bool CapabilitiesVK::AreValidationsEnabled() const { std::optional> CapabilitiesVK::GetEnabledLayers() const { - std::vector required = {}; + std::vector required; if (validations_enabled_) { // The presence of this layer is already checked in the ctor. @@ -287,7 +287,7 @@ static bool HasRequiredQueues(const vk::PhysicalDevice& physical_device) { vk::QueueFlagBits::eTransfer)); } -std::optional +std::optional CapabilitiesVK::GetEnabledDeviceFeatures( const vk::PhysicalDevice& device) const { if (!PhysicalDeviceSupportsRequiredFormats(device)) { @@ -312,11 +312,11 @@ CapabilitiesVK::GetEnabledDeviceFeatures( const auto device_features = device.getFeatures(); - vk::PhysicalDeviceFeatures2 required; + vk::PhysicalDeviceFeatures required; // We require this for enabling wireframes in the playground. But its not // necessarily a big deal if we don't have this feature. - required.features.fillModeNonSolid = device_features.fillModeNonSolid; + required.fillModeNonSolid = device_features.fillModeNonSolid; return required; } diff --git a/impeller/renderer/backend/vulkan/capabilities_vk.h b/impeller/renderer/backend/vulkan/capabilities_vk.h index dc2dd2c487bf1..4cb7ce8996600 100644 --- a/impeller/renderer/backend/vulkan/capabilities_vk.h +++ b/impeller/renderer/backend/vulkan/capabilities_vk.h @@ -50,7 +50,7 @@ class CapabilitiesVK final : public Capabilities, std::optional> GetEnabledDeviceExtensions( const vk::PhysicalDevice& physical_device) const; - std::optional GetEnabledDeviceFeatures( + std::optional GetEnabledDeviceFeatures( const vk::PhysicalDevice& physical_device) const; [[nodiscard]] bool SetPhysicalDevice( From ec946062fe282ac5212f3edc1d3fe775b2ea0a66 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 16:14:55 -0800 Subject: [PATCH 06/12] revert context changes. --- impeller/renderer/backend/vulkan/context_vk.cc | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index b92f3dcb0fc78..e573d6aa5bfb9 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -308,24 +308,19 @@ void ContextVK::Setup(Settings settings) { const auto queue_create_infos = GetQueueCreateInfos( {graphics_queue.value(), compute_queue.value(), transfer_queue.value()}); - const auto enabled_features_x = + const auto enabled_features = caps->GetEnabledDeviceFeatures(device_holder->physical_device); - if (!enabled_features_x.has_value()) { + if (!enabled_features.has_value()) { // This shouldn't happen since the device can't be picked if this was not // true. But doesn't hurt to check. return; } - vk::PhysicalDeviceFeatures2 enabled_features = enabled_features_x.value(); - vk::PhysicalDeviceSynchronization2Features physical_features; - physical_features.synchronization2 = true; - enabled_features.pNext = &physical_features; - vk::DeviceCreateInfo device_info; device_info.setQueueCreateInfos(queue_create_infos); device_info.setPEnabledExtensionNames(enabled_device_extensions_c); - device_info.setPNext(&enabled_features); + device_info.setPEnabledFeatures(&enabled_features.value()); // Device layers are deprecated and ignored. { From e5f44ceb5f119506c7bea3e6494b3db5ed17eb37 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 16:53:31 -0800 Subject: [PATCH 07/12] remove unused argument, add barrier. --- impeller/entity/geometry/point_field_geometry.cc | 2 +- .../renderer/backend/metal/compute_pass_mtl.h | 5 ++--- .../renderer/backend/metal/compute_pass_mtl.mm | 5 ++--- .../renderer/backend/vulkan/compute_pass_vk.cc | 5 ++--- .../renderer/backend/vulkan/compute_pass_vk.h | 5 ++--- impeller/renderer/compute_pass.h | 15 +++++++-------- impeller/renderer/compute_unittests.cc | 1 + 7 files changed, 17 insertions(+), 21 deletions(-) diff --git a/impeller/entity/geometry/point_field_geometry.cc b/impeller/entity/geometry/point_field_geometry.cc index 20e7013856f25..ed9a3c0af11ce 100644 --- a/impeller/entity/geometry/point_field_geometry.cc +++ b/impeller/entity/geometry/point_field_geometry.cc @@ -200,7 +200,7 @@ GeometryResult PointFieldGeometry::GetPositionBufferGPU( using UV = UvComputeShader; - compute_pass->AddBufferMemoryBarrier(geometry_buffer); + compute_pass->AddBufferMemoryBarrier(); compute_pass->SetCommandLabel("UV Geometry"); compute_pass->SetPipeline(renderer.GetUvComputePipeline()); diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.h b/impeller/renderer/backend/metal/compute_pass_mtl.h index 41fe3c41c2c2b..3c190ca4bcaee 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.h +++ b/impeller/renderer/backend/metal/compute_pass_mtl.h @@ -67,11 +67,10 @@ class ComputePassMTL final : public ComputePass { bool EncodeCommands() const override; // |ComputePass| - void AddBufferMemoryBarrier(const BufferView& view) override; + void AddBufferMemoryBarrier() override; // |ComputePass| - void AddTextureMemoryBarrier( - const std::shared_ptr& texture) override; + void AddTextureMemoryBarrier() override; ComputePassMTL(const ComputePassMTL&) = delete; diff --git a/impeller/renderer/backend/metal/compute_pass_mtl.mm b/impeller/renderer/backend/metal/compute_pass_mtl.mm index bf19d800b37d0..63d7ff4b695e5 100644 --- a/impeller/renderer/backend/metal/compute_pass_mtl.mm +++ b/impeller/renderer/backend/metal/compute_pass_mtl.mm @@ -69,13 +69,12 @@ } // |ComputePass| -void ComputePassMTL::AddBufferMemoryBarrier(const BufferView& view) { +void ComputePassMTL::AddBufferMemoryBarrier() { [encoder_ memoryBarrierWithScope:MTLBarrierScopeBuffers]; } // |ComputePass| -void ComputePassMTL::AddTextureMemoryBarrier( - const std::shared_ptr& texture) { +void ComputePassMTL::AddTextureMemoryBarrier() { [encoder_ memoryBarrierWithScope:MTLBarrierScopeTextures]; } diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index d54df9e752a30..9d79eac6675e7 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -220,7 +220,7 @@ bool ComputePassVK::BindResource(size_t binding, // require some experimentation. // |ComputePass| -void ComputePassVK::AddBufferMemoryBarrier(const BufferView& view) { +void ComputePassVK::AddBufferMemoryBarrier() { vk::MemoryBarrier barrier; barrier.srcAccessMask = vk::AccessFlagBits::eShaderWrite; barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead; @@ -231,8 +231,7 @@ void ComputePassVK::AddBufferMemoryBarrier(const BufferView& view) { } // |ComputePass| -void ComputePassVK::AddTextureMemoryBarrier( - const std::shared_ptr& texture) { +void ComputePassVK::AddTextureMemoryBarrier() { vk::MemoryBarrier barrier; barrier.srcAccessMask = vk::AccessFlagBits::eShaderWrite; barrier.dstAccessMask = vk::AccessFlagBits::eShaderRead; diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.h b/impeller/renderer/backend/vulkan/compute_pass_vk.h index e7401ca92f5f4..8395794809f7f 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.h +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.h @@ -58,11 +58,10 @@ class ComputePassVK final : public ComputePass { pipeline) override; // |ComputePass| - void AddBufferMemoryBarrier(const BufferView& view) override; + void AddBufferMemoryBarrier() override; // |ComputePass| - void AddTextureMemoryBarrier( - const std::shared_ptr& texture) override; + void AddTextureMemoryBarrier() override; // |ComputePass| fml::Status Compute(const ISize& grid_size) override; diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index d9582418a36be..da639f986cf5e 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -35,19 +35,18 @@ class ComputePass : public ResourceBinder { virtual fml::Status Compute(const ISize& grid_size) = 0; - /// @brief Add a barrier that ensures all previously encoded commands have - /// finished reading or writing to the provided buffer before - /// subsequent commands will access it. + /// @brief Ensures all previously encoded commands have finished reading or + /// writing to any buffer before subsequent commands will access it. /// /// This only handles read after write hazards. - virtual void AddBufferMemoryBarrier(const BufferView& view) = 0; + virtual void AddBufferMemoryBarrier() = 0; /// @brief Add a barrier that ensures all previously encoded commands have - /// finished reading or writing to the provided texture before - /// subsequent commands will access it. + /// finished reading or writing to any textures subsequent commands + /// will access it. + /// /// This only handles read after write hazards. - virtual void AddTextureMemoryBarrier( - const std::shared_ptr& texture) = 0; + virtual void AddTextureMemoryBarrier() = 0; //---------------------------------------------------------------------------- /// @brief Encode the recorded commands to the underlying command buffer. diff --git a/impeller/renderer/compute_unittests.cc b/impeller/renderer/compute_unittests.cc index d8fd55d875c0f..8a3cb8769412b 100644 --- a/impeller/renderer/compute_unittests.cc +++ b/impeller/renderer/compute_unittests.cc @@ -308,6 +308,7 @@ TEST_P(ComputeTest, MultiStageInputAndOutput) { CS1::BindOutput(*pass, DeviceBuffer::AsBufferView(output_buffer_1)); ASSERT_TRUE(pass->Compute(ISize(512, 1)).ok()); + pass->AddBufferMemoryBarrier(); } { From 0c2adf58febfe8b08cb5a8f86e2d1b82efc51228 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 17:11:26 -0800 Subject: [PATCH 08/12] fix doc comment. --- impeller/entity/geometry/point_field_geometry.cc | 10 ++-------- impeller/entity/geometry/point_field_geometry.h | 3 --- impeller/renderer/compute_pass.h | 14 +++++++------- 3 files changed, 9 insertions(+), 18 deletions(-) diff --git a/impeller/entity/geometry/point_field_geometry.cc b/impeller/entity/geometry/point_field_geometry.cc index ed9a3c0af11ce..5b5cd8caf7813 100644 --- a/impeller/entity/geometry/point_field_geometry.cc +++ b/impeller/entity/geometry/point_field_geometry.cc @@ -17,7 +17,7 @@ GeometryResult PointFieldGeometry::GetPositionBuffer( const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { - if (CanUseCompute(renderer)) { + if (renderer.GetDeviceCapabilities().SupportsCompute()) { return GetPositionBufferGPU(renderer, entity, pass); } auto vtx_builder = GetPositionBufferCPU(renderer, entity, pass); @@ -40,7 +40,7 @@ GeometryResult PointFieldGeometry::GetPositionUVBuffer( const ContentContext& renderer, const Entity& entity, RenderPass& pass) const { - if (CanUseCompute(renderer)) { + if (renderer.GetDeviceCapabilities().SupportsCompute()) { return GetPositionBufferGPU(renderer, entity, pass, texture_coverage, effect_transform); } @@ -265,12 +265,6 @@ GeometryVertexType PointFieldGeometry::GetVertexType() const { return GeometryVertexType::kPosition; } -// Compute is disabled for Vulkan because the barriers are incorrect, see -// also: https://github.com/flutter/flutter/issues/140798 . -bool PointFieldGeometry::CanUseCompute(const ContentContext& renderer) { - return renderer.GetDeviceCapabilities().SupportsCompute(); -} - // |Geometry| std::optional PointFieldGeometry::GetCoverage( const Matrix& transform) const { diff --git a/impeller/entity/geometry/point_field_geometry.h b/impeller/entity/geometry/point_field_geometry.h index a8c296adfc3c6..9d43f07cb9fbe 100644 --- a/impeller/entity/geometry/point_field_geometry.h +++ b/impeller/entity/geometry/point_field_geometry.h @@ -17,9 +17,6 @@ class PointFieldGeometry final : public Geometry { static size_t ComputeCircleDivisions(Scalar scaled_radius, bool round); - /// If the platform can use compute safely. - static bool CanUseCompute(const ContentContext& renderer); - private: // |Geometry| GeometryResult GetPositionBuffer(const ContentContext& renderer, diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index da639f986cf5e..06bf3510aff95 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -35,17 +35,17 @@ class ComputePass : public ResourceBinder { virtual fml::Status Compute(const ISize& grid_size) = 0; - /// @brief Ensures all previously encoded commands have finished reading or - /// writing to any buffer before subsequent commands will access it. + /// @brief Ensures all previously encoded commands have finished writing to + /// buffers before any following commands can read from those buffer. /// - /// This only handles read after write hazards. + /// In other words, this handles "read after write" memory hazards. virtual void AddBufferMemoryBarrier() = 0; - /// @brief Add a barrier that ensures all previously encoded commands have - /// finished reading or writing to any textures subsequent commands - /// will access it. + /// @brief Ensures all previously encoded commands have finished writing to + /// textures before any following commands can read from those + /// textures. /// - /// This only handles read after write hazards. + /// In other words, this handles "read after write" memory hazards. virtual void AddTextureMemoryBarrier() = 0; //---------------------------------------------------------------------------- From 0ec1f76aaa9742c966c8d1f9a5914df86c1bb628 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 17:42:19 -0800 Subject: [PATCH 09/12] ++ --- impeller/entity/entity_unittests.cc | 5 ----- impeller/renderer/compute_pass.h | 10 ++++++++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index fc14e47e4cf12..c4fd09d848a1c 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2498,11 +2498,6 @@ TEST_P(EntityTest, PointFieldGeometryCoverage) { Rect::MakeLTRB(35, 15, 135, 205)); } -TEST_P(EntityTest, PointFieldCanUseCompute) { - EXPECT_EQ(PointFieldGeometry::CanUseCompute(*GetContentContext()), - GetContext()->GetBackendType() == Context::BackendType::kMetal); -} - TEST_P(EntityTest, ColorFilterContentsWithLargeGeometry) { Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 06bf3510aff95..37196d90a4dd9 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -38,14 +38,20 @@ class ComputePass : public ResourceBinder { /// @brief Ensures all previously encoded commands have finished writing to /// buffers before any following commands can read from those buffer. /// - /// In other words, this handles "read after write" memory hazards. + /// This method after encoding a command that writes to a storage + /// buffer that a subsequent command will read from, provided that + /// is a compute command and not a render pass command. It does not + /// matter if the compute command is in a different command buffer. virtual void AddBufferMemoryBarrier() = 0; /// @brief Ensures all previously encoded commands have finished writing to /// textures before any following commands can read from those /// textures. /// - /// In other words, this handles "read after write" memory hazards. + /// This method after encoding a command that writes to a storage + /// texture that a subsequent command will read from, provided that + /// is a compute command and not a render pass command. It does not + /// matter if the compute command is in a different command buffer. virtual void AddTextureMemoryBarrier() = 0; //---------------------------------------------------------------------------- From 555e92dde5bc9219eb0628ea15cc36ee46e8137b Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 22 Jan 2024 19:28:06 -0800 Subject: [PATCH 10/12] ++ --- .../backend/vulkan/compute_pass_vk.cc | 4 ++-- impeller/renderer/compute_pass.h | 24 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 9d79eac6675e7..0ea8e0bbbbed3 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -216,8 +216,8 @@ bool ComputePassVK::BindResource(size_t binding, // Note: // https://github.com/KhronosGroup/Vulkan-Docs/wiki/Synchronization-Examples // Seems to suggest that anything more finely grained than a global memory -// barrier is likely to be ignored. Confirming this on mobile devices will -// require some experimentation. +// barrier is likely to be weakened into a global barrier. Confirming this on +// mobile devices will require some experimentation. // |ComputePass| void ComputePassVK::AddBufferMemoryBarrier() { diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 37196d90a4dd9..47d81a7edd012 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -35,23 +35,21 @@ class ComputePass : public ResourceBinder { virtual fml::Status Compute(const ISize& grid_size) = 0; - /// @brief Ensures all previously encoded commands have finished writing to - /// buffers before any following commands can read from those buffer. + /// @brief Ensures all previously encoded command's buffer writes are visible + /// to + /// any subsequent compute commands. /// - /// This method after encoding a command that writes to a storage - /// buffer that a subsequent command will read from, provided that - /// is a compute command and not a render pass command. It does not - /// matter if the compute command is in a different command buffer. + /// On Vulkan, it does not matter if the compute command is in a + /// different command buffer, only that it is executed later in queue + /// order. virtual void AddBufferMemoryBarrier() = 0; - /// @brief Ensures all previously encoded commands have finished writing to - /// textures before any following commands can read from those - /// textures. + /// @brief Ensures all previously encoded command's texture writes are visible + /// to any subsequent compute commands. /// - /// This method after encoding a command that writes to a storage - /// texture that a subsequent command will read from, provided that - /// is a compute command and not a render pass command. It does not - /// matter if the compute command is in a different command buffer. + /// On Vulkan, it does not matter if the compute command is in a + /// different command buffer, only that it is executed later in queue + /// order. virtual void AddTextureMemoryBarrier() = 0; //---------------------------------------------------------------------------- From 8220ebfa8891f13a3e0238c89433d10655d50b86 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 23 Jan 2024 08:25:59 -0800 Subject: [PATCH 11/12] formatting. --- impeller/renderer/compute_pass.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/impeller/renderer/compute_pass.h b/impeller/renderer/compute_pass.h index 47d81a7edd012..50b65c432e86b 100644 --- a/impeller/renderer/compute_pass.h +++ b/impeller/renderer/compute_pass.h @@ -35,17 +35,16 @@ class ComputePass : public ResourceBinder { virtual fml::Status Compute(const ISize& grid_size) = 0; - /// @brief Ensures all previously encoded command's buffer writes are visible - /// to - /// any subsequent compute commands. + /// @brief Ensures all previously encoded compute command's buffer writes are + /// visible to any subsequent compute commands. /// /// On Vulkan, it does not matter if the compute command is in a /// different command buffer, only that it is executed later in queue /// order. virtual void AddBufferMemoryBarrier() = 0; - /// @brief Ensures all previously encoded command's texture writes are visible - /// to any subsequent compute commands. + /// @brief Ensures all previously encoded compute command's texture writes are + /// visible to any subsequent compute commands. /// /// On Vulkan, it does not matter if the compute command is in a /// different command buffer, only that it is executed later in queue From 18f6f54a7ff4b0a4bd90f11ad91f33d46740843e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 26 Jan 2024 11:09:18 -0800 Subject: [PATCH 12/12] test --- impeller/renderer/backend/vulkan/compute_pass_vk.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/impeller/renderer/backend/vulkan/compute_pass_vk.cc b/impeller/renderer/backend/vulkan/compute_pass_vk.cc index 7e193c45d76b7..2b5ac2eece620 100644 --- a/impeller/renderer/backend/vulkan/compute_pass_vk.cc +++ b/impeller/renderer/backend/vulkan/compute_pass_vk.cc @@ -104,11 +104,7 @@ fml::Status ComputePassVK::Compute(const ISize& grid_size) { // Special case for linear processing. if (height == 1) { - int64_t minimum = 1; - int64_t threadGroups = std::max( - static_cast(std::ceil(width * 1.0 / max_wg_size_[0] * 1.0)), - minimum); - command_buffer_vk.dispatch(threadGroups, 1, 1); + command_buffer_vk.dispatch(width, 1, 1); } else { while (width > max_wg_size_[0]) { width = std::max(static_cast(1), width / 2);