From 1d85ce2e419790cc6fa74719035ece5ea85e43f4 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Fri, 19 Jan 2024 18:57:02 -0800 Subject: [PATCH] [Impeller] Encode directly to command buffer for Metal. (flutter/engine#49785) Part of https://github.com/flutter/flutter/issues/140804 Rather than using impeller::Command, the impeller::RenderPass records most state directly into the Vulkan command buffer. This should remove allocation/free overhead of the intermediary structures and make further improvements to the backend even easier. This completely removes the background worker threads used for encoding, which should lower overall CPU usage without decreasing performance by much (though it may be a tad slower or a tad faster). --- .../ci/licenses_golden/licenses_flutter | 4 + .../checkerboard_contents_unittests.cc | 4 + .../contents/test/recording_render_pass.cc | 2 +- .../tiled_texture_contents_unittests.cc | 4 + .../contents/vertices_contents_unittests.cc | 4 + .../impeller/renderer/backend/metal/BUILD.gn | 2 + .../backend/metal/command_buffer_mtl.h | 7 - .../backend/metal/command_buffer_mtl.mm | 85 --- .../renderer/backend/metal/context_mtl.h | 3 - .../renderer/backend/metal/context_mtl.mm | 28 +- .../backend/metal/pass_bindings_cache_mtl.h | 75 +++ .../backend/metal/pass_bindings_cache_mtl.mm | 152 ++++++ .../renderer/backend/metal/render_pass_mtl.h | 73 ++- .../renderer/backend/metal/render_pass_mtl.mm | 499 ++++++------------ .../impeller/renderer/command_buffer.cc | 2 - .../flutter/impeller/renderer/render_pass.h | 3 + .../impeller/renderer/renderer_unittests.cc | 1 + 17 files changed, 495 insertions(+), 453 deletions(-) create mode 100644 engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h create mode 100644 engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index e4b8b44cad..bb19c77d15 100644 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -5401,6 +5401,8 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.h + ../. ORIGIN: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.mm + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h + ../../../flutter/LICENSE +ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm + ../../../flutter/LICENSE ORIGIN: ../../../flutter/impeller/renderer/backend/metal/pipeline_mtl.h + ../../../flutter/LICENSE @@ -8243,6 +8245,8 @@ FILE: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.h FILE: ../../../flutter/impeller/renderer/backend/metal/gpu_tracer_mtl.mm FILE: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.h FILE: ../../../flutter/impeller/renderer/backend/metal/lazy_drawable_holder.mm +FILE: ../../../flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h +FILE: ../../../flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.h FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_library_mtl.mm FILE: ../../../flutter/impeller/renderer/backend/metal/pipeline_mtl.h diff --git a/engine/src/flutter/impeller/entity/contents/checkerboard_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/checkerboard_contents_unittests.cc index 97e1dae427..61f5aa1ce5 100644 --- a/engine/src/flutter/impeller/entity/contents/checkerboard_contents_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/checkerboard_contents_unittests.cc @@ -47,6 +47,10 @@ TEST_P(EntityTest, RendersWithoutError) { ASSERT_TRUE(recording_pass->GetCommands().empty()); ASSERT_TRUE(contents->Render(*content_context, entity, *recording_pass)); ASSERT_FALSE(recording_pass->GetCommands().empty()); + + if (GetParam() == PlaygroundBackend::kMetal) { + recording_pass->EncodeCommands(); + } } #endif // IMPELLER_DEBUG diff --git a/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc b/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc index ef22d86d69..efbacd0745 100644 --- a/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc +++ b/engine/src/flutter/impeller/entity/contents/test/recording_render_pass.cc @@ -78,7 +78,7 @@ void RecordingRenderPass::OnSetLabel(std::string label) { // |RenderPass| bool RecordingRenderPass::OnEncodeCommands(const Context& context) const { - return true; + return delegate_->EncodeCommands(); } // |RenderPass| diff --git a/engine/src/flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc index ddaedc3efc..ac0b9a3030 100644 --- a/engine/src/flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/tiled_texture_contents_unittests.cc @@ -46,6 +46,10 @@ TEST_P(EntityTest, TiledTextureContentsRendersWithCorrectPipeline) { ASSERT_EQ(commands.size(), 1u); ASSERT_STREQ(commands[0].pipeline->GetDescriptor().GetLabel().c_str(), "TextureFill Pipeline V#1"); + + if (GetParam() == PlaygroundBackend::kMetal) { + recording_pass->EncodeCommands(); + } } // GL_OES_EGL_image_external isn't supported on MacOS hosts. diff --git a/engine/src/flutter/impeller/entity/contents/vertices_contents_unittests.cc b/engine/src/flutter/impeller/entity/contents/vertices_contents_unittests.cc index 4f1e778986..67b6e43174 100644 --- a/engine/src/flutter/impeller/entity/contents/vertices_contents_unittests.cc +++ b/engine/src/flutter/impeller/entity/contents/vertices_contents_unittests.cc @@ -78,6 +78,10 @@ TEST_P(EntityTest, RendersDstPerColorWithAlpha) { ASSERT_TRUE(frag_uniforms); ASSERT_EQ(frag_uniforms->alpha, 0.5); + + if (GetParam() == PlaygroundBackend::kMetal) { + recording_pass->EncodeCommands(); + } } } // namespace testing diff --git a/engine/src/flutter/impeller/renderer/backend/metal/BUILD.gn b/engine/src/flutter/impeller/renderer/backend/metal/BUILD.gn index e038782fb4..4e64976688 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/BUILD.gn +++ b/engine/src/flutter/impeller/renderer/backend/metal/BUILD.gn @@ -28,6 +28,8 @@ impeller_component("metal") { "gpu_tracer_mtl.mm", "lazy_drawable_holder.h", "lazy_drawable_holder.mm", + "pass_bindings_cache_mtl.h", + "pass_bindings_cache_mtl.mm", "pipeline_library_mtl.h", "pipeline_library_mtl.mm", "pipeline_mtl.h", diff --git a/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.h index 735142b59a..f9db1c1b4a 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.h @@ -38,13 +38,6 @@ class CommandBufferMTL final : public CommandBuffer { // |CommandBuffer| void OnWaitUntilScheduled() override; - // |CommandBuffer| - bool EncodeAndSubmit(const std::shared_ptr& render_pass) override; - - // |CommandBuffer| - bool EncodeAndSubmit(const std::shared_ptr& blit_ass, - const std::shared_ptr& allocator) override; - // |CommandBuffer| std::shared_ptr OnCreateRenderPass(RenderTarget target) override; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.mm index c7899d4898..d9c43d1b4e 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -6,7 +6,6 @@ #include "flutter/fml/make_copyable.h" #include "flutter/fml/synchronization/semaphore.h" -#include "flutter/fml/trace_event.h" #include "impeller/renderer/backend/metal/blit_pass_mtl.h" #include "impeller/renderer/backend/metal/compute_pass_mtl.h" @@ -183,90 +182,6 @@ bool CommandBufferMTL::OnSubmitCommands(CompletionCallback callback) { return true; } -bool CommandBufferMTL::EncodeAndSubmit( - const std::shared_ptr& render_pass) { - TRACE_EVENT0("impeller", "CommandBufferMTL::EncodeAndSubmit"); - if (!IsValid() || !render_pass->IsValid()) { - return false; - } - auto context = context_.lock(); - if (!context) { - return false; - } - [buffer_ enqueue]; - auto buffer = buffer_; - buffer_ = nil; - -#ifdef IMPELLER_DEBUG - ContextMTL::Cast(*context).GetGPUTracer()->RecordCmdBuffer(buffer); -#endif // IMPELLER_DEBUG - - auto worker_task_runner = ContextMTL::Cast(*context).GetWorkerTaskRunner(); - auto mtl_render_pass = static_cast(render_pass.get()); - - // Render command encoder creation has been observed to exceed the stack size - // limit for worker threads, and therefore is intentionally constructed on the - // raster thread. - auto render_command_encoder = - [buffer renderCommandEncoderWithDescriptor:mtl_render_pass->desc_]; - if (!render_command_encoder) { - return false; - } - - auto task = fml::MakeCopyable( - [render_pass, buffer, render_command_encoder, weak_context = context_]() { - auto context = weak_context.lock(); - if (!context) { - [render_command_encoder endEncoding]; - return; - } - - auto mtl_render_pass = static_cast(render_pass.get()); - if (!mtl_render_pass->label_.empty()) { - [render_command_encoder setLabel:@(mtl_render_pass->label_.c_str())]; - } - - auto result = mtl_render_pass->EncodeCommands( - context->GetResourceAllocator(), render_command_encoder); - [render_command_encoder endEncoding]; - if (result) { - [buffer commit]; - } else { - VALIDATION_LOG << "Failed to encode command buffer"; - } - }); - worker_task_runner->PostTask(task); - return true; -} - -bool CommandBufferMTL::EncodeAndSubmit( - const std::shared_ptr& blit_pass, - const std::shared_ptr& allocator) { - if (!IsValid() || !blit_pass->IsValid()) { - return false; - } - auto context = context_.lock(); - if (!context) { - return false; - } - [buffer_ enqueue]; - auto buffer = buffer_; - buffer_ = nil; - - auto worker_task_runner = ContextMTL::Cast(*context).GetWorkerTaskRunner(); - auto task = fml::MakeCopyable( - [blit_pass, buffer, weak_context = context_, allocator]() { - auto context = weak_context.lock(); - if (!blit_pass->EncodeCommands(allocator)) { - VALIDATION_LOG << "Failed to encode blit pass."; - return; - } - [buffer commit]; - }); - worker_task_runner->PostTask(task); - return true; -} - void CommandBufferMTL::OnWaitUntilScheduled() {} std::shared_ptr CommandBufferMTL::OnCreateRenderPass( diff --git a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.h index d7561f52c3..f0735a3219 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.h @@ -93,8 +93,6 @@ class ContextMTL final : public Context, id CreateMTLCommandBuffer(const std::string& label) const; - const std::shared_ptr GetWorkerTaskRunner() const; - std::shared_ptr GetIsGpuDisabledSyncSwitch() const; #ifdef IMPELLER_DEBUG @@ -122,7 +120,6 @@ class ContextMTL final : public Context, std::shared_ptr sampler_library_; std::shared_ptr resource_allocator_; std::shared_ptr device_capabilities_; - std::shared_ptr raster_message_loop_; std::shared_ptr is_gpu_disabled_sync_switch_; #ifdef IMPELLER_DEBUG std::shared_ptr gpu_tracer_; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm index 90054aa21d..9c2342d302 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/context_mtl.mm @@ -4,7 +4,6 @@ #include "impeller/renderer/backend/metal/context_mtl.h" -#include #include #include "flutter/fml/concurrent_message_loop.h" @@ -88,24 +87,6 @@ ContextMTL::ContextMTL( sync_switch_observer_.reset(new SyncSwitchObserver(*this)); is_gpu_disabled_sync_switch_->AddObserver(sync_switch_observer_.get()); - // Worker task runner. - { - raster_message_loop_ = fml::ConcurrentMessageLoop::Create( - std::min(4u, std::thread::hardware_concurrency())); - raster_message_loop_->PostTaskToAllWorkers([]() { - // See https://github.com/flutter/flutter/issues/65752 - // Intentionally opt out of QoS for raster task workloads. - [[NSThread currentThread] setThreadPriority:1.0]; - sched_param param; - int policy; - pthread_t thread = pthread_self(); - if (!pthread_getschedparam(thread, &policy, ¶m)) { - param.sched_priority = 50; - pthread_setschedparam(thread, policy, ¶m); - } - }); - } - // Setup the shader library. { if (shader_libraries == nil) { @@ -330,9 +311,7 @@ std::shared_ptr ContextMTL::CreateCommandBuffer() const { } // |Context| -void ContextMTL::Shutdown() { - raster_message_loop_.reset(); -} +void ContextMTL::Shutdown() {} #ifdef IMPELLER_DEBUG std::shared_ptr ContextMTL::GetGPUTracer() const { @@ -340,11 +319,6 @@ std::shared_ptr ContextMTL::GetGPUTracer() const { } #endif // IMPELLER_DEBUG -const std::shared_ptr -ContextMTL::GetWorkerTaskRunner() const { - return raster_message_loop_->GetTaskRunner(); -} - std::shared_ptr ContextMTL::GetIsGpuDisabledSyncSwitch() const { return is_gpu_disabled_sync_switch_; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h new file mode 100644 index 0000000000..4728bee4b8 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.h @@ -0,0 +1,75 @@ +// 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_METAL_PASS_BINDINGS_CACHE_MTL_H_ +#define FLUTTER_IMPELLER_RENDERER_BACKEND_METAL_PASS_BINDINGS_CACHE_MTL_H_ + +#include + +#include "impeller/renderer/render_pass.h" +#include "impeller/renderer/render_target.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 PassBindingsCacheMTL { + explicit PassBindingsCacheMTL() {} + + ~PassBindingsCacheMTL() = default; + + PassBindingsCacheMTL(const PassBindingsCacheMTL&) = delete; + + PassBindingsCacheMTL(PassBindingsCacheMTL&&) = delete; + + void SetEncoder(id encoder); + + void SetRenderPipelineState(id pipeline); + + void SetDepthStencilState(id depth_stencil); + + bool SetBuffer(ShaderStage stage, + uint64_t index, + uint64_t offset, + id buffer); + + bool SetTexture(ShaderStage stage, uint64_t index, id texture); + + bool SetSampler(ShaderStage stage, + uint64_t index, + id sampler); + + void SetViewport(const Viewport& viewport); + + void SetScissor(const IRect& scissor); + + 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; + id depth_stencil_ = nullptr; + std::map buffers_; + std::map textures_; + std::map samplers_; + std::optional viewport_; + std::optional scissor_; +}; + +} // namespace impeller + +#endif // FLUTTER_IMPELLER_RENDERER_BACKEND_METAL_PASS_BINDINGS_CACHE_MTL_H_ diff --git a/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm new file mode 100644 index 0000000000..ab5370a7a1 --- /dev/null +++ b/engine/src/flutter/impeller/renderer/backend/metal/pass_bindings_cache_mtl.mm @@ -0,0 +1,152 @@ +// 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/metal/pass_bindings_cache_mtl.h" + +namespace impeller { + +void PassBindingsCacheMTL::SetEncoder(id encoder) { + encoder_ = encoder; +} + +void PassBindingsCacheMTL::SetRenderPipelineState( + id pipeline) { + if (pipeline == pipeline_) { + return; + } + pipeline_ = pipeline; + [encoder_ setRenderPipelineState:pipeline_]; +} + +void PassBindingsCacheMTL::SetDepthStencilState( + id depth_stencil) { + if (depth_stencil_ == depth_stencil) { + return; + } + depth_stencil_ = depth_stencil; + [encoder_ setDepthStencilState:depth_stencil_]; +} + +bool PassBindingsCacheMTL::SetBuffer(ShaderStage stage, + uint64_t index, + uint64_t offset, + id buffer) { + auto& buffers_map = buffers_[stage]; + auto found = buffers_map.find(index); + if (found != buffers_map.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 true; + } + + // Only the offset needs to be updated. + found->second.offset = offset; + + switch (stage) { + case ShaderStage::kVertex: + [encoder_ setVertexBufferOffset:offset atIndex:index]; + return true; + case ShaderStage::kFragment: + [encoder_ setFragmentBufferOffset:offset atIndex:index]; + return true; + default: + VALIDATION_LOG << "Cannot update buffer offset of an unknown stage."; + return false; + } + return true; + } + buffers_map[index] = {buffer, static_cast(offset)}; + switch (stage) { + case ShaderStage::kVertex: + [encoder_ setVertexBuffer:buffer offset:offset atIndex:index]; + return true; + case ShaderStage::kFragment: + [encoder_ setFragmentBuffer:buffer offset:offset atIndex:index]; + return true; + default: + VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; + return false; + } + return false; +} + +bool PassBindingsCacheMTL::SetTexture(ShaderStage stage, + uint64_t index, + id texture) { + auto& texture_map = textures_[stage]; + auto found = texture_map.find(index); + if (found != texture_map.end() && found->second == texture) { + // Already bound. + return true; + } + texture_map[index] = texture; + switch (stage) { + case ShaderStage::kVertex: + [encoder_ setVertexTexture:texture atIndex:index]; + return true; + case ShaderStage::kFragment: + [encoder_ setFragmentTexture:texture atIndex:index]; + return true; + default: + VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; + return false; + } + return false; +} + +bool PassBindingsCacheMTL::SetSampler(ShaderStage stage, + uint64_t index, + id sampler) { + auto& sampler_map = samplers_[stage]; + auto found = sampler_map.find(index); + if (found != sampler_map.end() && found->second == sampler) { + // Already bound. + return true; + } + sampler_map[index] = sampler; + switch (stage) { + case ShaderStage::kVertex: + [encoder_ setVertexSamplerState:sampler atIndex:index]; + return true; + case ShaderStage::kFragment: + [encoder_ setFragmentSamplerState:sampler atIndex:index]; + return true; + default: + VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; + return false; + } + return false; +} + +void PassBindingsCacheMTL::SetViewport(const Viewport& viewport) { + if (viewport_.has_value() && viewport_.value() == viewport) { + return; + } + [encoder_ setViewport:MTLViewport{ + .originX = viewport.rect.GetX(), + .originY = viewport.rect.GetY(), + .width = viewport.rect.GetWidth(), + .height = viewport.rect.GetHeight(), + .znear = viewport.depth_range.z_near, + .zfar = viewport.depth_range.z_far, + }]; + viewport_ = viewport; +} + +void PassBindingsCacheMTL::SetScissor(const IRect& scissor) { + if (scissor_.has_value() && scissor_.value() == scissor) { + return; + } + [encoder_ + setScissorRect:MTLScissorRect{ + .x = static_cast(scissor.GetX()), + .y = static_cast(scissor.GetY()), + .width = static_cast(scissor.GetWidth()), + .height = static_cast(scissor.GetHeight()), + }]; + scissor_ = scissor; +} + +} // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.h b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.h index 00240d4524..4548f4424e 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.h +++ b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.h @@ -8,6 +8,7 @@ #include #include "flutter/fml/macros.h" +#include "impeller/renderer/backend/metal/pass_bindings_cache_mtl.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" @@ -22,14 +23,35 @@ class RenderPassMTL final : public RenderPass { friend class CommandBufferMTL; id buffer_ = nil; + id encoder_ = nil; MTLRenderPassDescriptor* desc_ = nil; std::string label_; + bool is_metal_trace_active_ = false; bool is_valid_ = false; + // Many parts of the codebase will start writing to a render pass but + // never submit them. This boolean is used to track if a submit happened + // so that in the dtor we can always ensure the render pass is finished. + mutable bool did_finish_encoding_ = false; + + PassBindingsCacheMTL pass_bindings_; + + // Per-command state + size_t instance_count_ = 1u; + size_t base_vertex_ = 0u; + size_t vertex_count_ = 0u; + bool has_valid_pipeline_ = false; + bool has_label_ = false; + BufferView index_buffer_ = {}; + PrimitiveType primitive_type_ = {}; + MTLIndexType index_type_ = {}; RenderPassMTL(std::shared_ptr context, const RenderTarget& target, id buffer); + // |RenderPass| + void ReserveCommands(size_t command_count) override {} + // |RenderPass| bool IsValid() const override; @@ -39,8 +61,55 @@ class RenderPassMTL final : public RenderPass { // |RenderPass| bool OnEncodeCommands(const Context& context) const override; - bool EncodeCommands(const std::shared_ptr& transients_allocator, - id pass) const; + // |RenderPass| + void SetPipeline( + const std::shared_ptr>& pipeline) override; + + // |RenderPass| + void SetCommandLabel(std::string_view label) override; + + // |RenderPass| + void SetStencilReference(uint32_t value) override; + + // |RenderPass| + void SetBaseVertex(uint64_t value) override; + + // |RenderPass| + void SetViewport(Viewport viewport) override; + + // |RenderPass| + void SetScissor(IRect scissor) override; + + // |RenderPass| + void SetInstanceCount(size_t count) override; + + // |RenderPass| + bool SetVertexBuffer(VertexBuffer buffer) override; + + // |RenderPass| + fml::Status Draw() override; + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) override; + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view) override; + + // |RenderPass| + bool BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) override; RenderPassMTL(const RenderPassMTL&) = delete; diff --git a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm index 396127d598..05d608f3a3 100644 --- a/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm +++ b/engine/src/flutter/impeller/renderer/backend/metal/render_pass_mtl.mm @@ -7,7 +7,8 @@ #include "flutter/fml/closure.h" #include "flutter/fml/logging.h" #include "flutter/fml/make_copyable.h" -#include "flutter/fml/trace_event.h" +#include "fml/status.h" + #include "impeller/base/backend_cast.h" #include "impeller/core/formats.h" #include "impeller/core/host_buffer.h" @@ -143,220 +144,49 @@ RenderPassMTL::RenderPassMTL(std::shared_ptr context, if (!buffer_ || !desc_ || !render_target_.IsValid()) { return; } + encoder_ = [buffer_ renderCommandEncoderWithDescriptor:desc_]; + + if (!encoder_) { + return; + } +#ifdef IMPELLER_DEBUG + is_metal_trace_active_ = + [[MTLCaptureManager sharedCaptureManager] isCapturing]; +#endif // IMPELLER_DEBUG + pass_bindings_.SetEncoder(encoder_); + pass_bindings_.SetViewport( + Viewport{.rect = Rect::MakeSize(GetRenderTargetSize())}); + pass_bindings_.SetScissor(IRect::MakeSize(GetRenderTargetSize())); is_valid_ = true; } -RenderPassMTL::~RenderPassMTL() = default; +RenderPassMTL::~RenderPassMTL() { + if (!did_finish_encoding_) { + [encoder_ endEncoding]; + did_finish_encoding_ = true; + } +} bool RenderPassMTL::IsValid() const { return is_valid_; } void RenderPassMTL::OnSetLabel(std::string label) { +#ifdef IMPELLER_DEBUG if (label.empty()) { return; } - label_ = std::move(label); + encoder_.label = @(std::string(label).c_str()); +#endif // IMPELLER_DEBUG } bool RenderPassMTL::OnEncodeCommands(const Context& context) const { - TRACE_EVENT0("impeller", "RenderPassMTL::EncodeCommands"); - if (!IsValid()) { - return false; - } - auto render_command_encoder = - [buffer_ renderCommandEncoderWithDescriptor:desc_]; - - if (!render_command_encoder) { - return false; - } - - if (!label_.empty()) { - [render_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( - [render_command_encoder]() { [render_command_encoder endEncoding]; }); - - return EncodeCommands(context.GetResourceAllocator(), render_command_encoder); + did_finish_encoding_ = true; + [encoder_ endEncoding]; + return true; } -//----------------------------------------------------------------------------- -/// @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 PassBindingsCache { - explicit PassBindingsCache(id encoder) - : encoder_(encoder) {} - - PassBindingsCache(const PassBindingsCache&) = delete; - - PassBindingsCache(PassBindingsCache&&) = delete; - - void SetRenderPipelineState(id pipeline) { - if (pipeline == pipeline_) { - return; - } - pipeline_ = pipeline; - [encoder_ setRenderPipelineState:pipeline_]; - } - - void SetDepthStencilState(id depth_stencil) { - if (depth_stencil_ == depth_stencil) { - return; - } - depth_stencil_ = depth_stencil; - [encoder_ setDepthStencilState:depth_stencil_]; - } - - bool SetBuffer(ShaderStage stage, - uint64_t index, - uint64_t offset, - id buffer) { - auto& buffers_map = buffers_[stage]; - auto found = buffers_map.find(index); - if (found != buffers_map.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 true; - } - - // Only the offset needs to be updated. - found->second.offset = offset; - - switch (stage) { - case ShaderStage::kVertex: - [encoder_ setVertexBufferOffset:offset atIndex:index]; - return true; - case ShaderStage::kFragment: - [encoder_ setFragmentBufferOffset:offset atIndex:index]; - return true; - default: - VALIDATION_LOG << "Cannot update buffer offset of an unknown stage."; - return false; - } - return true; - } - buffers_map[index] = {buffer, static_cast(offset)}; - switch (stage) { - case ShaderStage::kVertex: - [encoder_ setVertexBuffer:buffer offset:offset atIndex:index]; - return true; - case ShaderStage::kFragment: - [encoder_ setFragmentBuffer:buffer offset:offset atIndex:index]; - return true; - default: - VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; - return false; - } - return false; - } - - bool SetTexture(ShaderStage stage, uint64_t index, id texture) { - auto& texture_map = textures_[stage]; - auto found = texture_map.find(index); - if (found != texture_map.end() && found->second == texture) { - // Already bound. - return true; - } - texture_map[index] = texture; - switch (stage) { - case ShaderStage::kVertex: - [encoder_ setVertexTexture:texture atIndex:index]; - return true; - case ShaderStage::kFragment: - [encoder_ setFragmentTexture:texture atIndex:index]; - return true; - default: - VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; - return false; - } - return false; - } - - bool SetSampler(ShaderStage stage, - uint64_t index, - id sampler) { - auto& sampler_map = samplers_[stage]; - auto found = sampler_map.find(index); - if (found != sampler_map.end() && found->second == sampler) { - // Already bound. - return true; - } - sampler_map[index] = sampler; - switch (stage) { - case ShaderStage::kVertex: - [encoder_ setVertexSamplerState:sampler atIndex:index]; - return true; - case ShaderStage::kFragment: - [encoder_ setFragmentSamplerState:sampler atIndex:index]; - return true; - default: - VALIDATION_LOG << "Cannot bind buffer to unknown shader stage."; - return false; - } - return false; - } - - void SetViewport(const Viewport& viewport) { - if (viewport_.has_value() && viewport_.value() == viewport) { - return; - } - [encoder_ setViewport:MTLViewport{ - .originX = viewport.rect.GetX(), - .originY = viewport.rect.GetY(), - .width = viewport.rect.GetWidth(), - .height = viewport.rect.GetHeight(), - .znear = viewport.depth_range.z_near, - .zfar = viewport.depth_range.z_far, - }]; - viewport_ = viewport; - } - - void SetScissor(const IRect& scissor) { - if (scissor_.has_value() && scissor_.value() == scissor) { - return; - } - [encoder_ - setScissorRect:MTLScissorRect{ - .x = static_cast(scissor.GetX()), - .y = static_cast(scissor.GetY()), - .width = static_cast(scissor.GetWidth()), - .height = - static_cast(scissor.GetHeight()), - }]; - scissor_ = scissor; - } - - 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; - id depth_stencil_ = nullptr; - std::map buffers_; - std::map textures_; - std::map samplers_; - std::optional viewport_; - std::optional scissor_; -}; - -static bool Bind(PassBindingsCache& pass, - Allocator& allocator, +static bool Bind(PassBindingsCacheMTL& pass, ShaderStage stage, size_t bind_index, const BufferView& view) { @@ -378,7 +208,7 @@ static bool Bind(PassBindingsCache& pass, return pass.SetBuffer(stage, bind_index, view.range.offset, buffer); } -static bool Bind(PassBindingsCache& pass, +static bool Bind(PassBindingsCacheMTL& pass, ShaderStage stage, size_t bind_index, const Sampler& sampler, @@ -403,147 +233,164 @@ static bool Bind(PassBindingsCache& pass, SamplerMTL::Cast(sampler).GetMTLSamplerState()); } -bool RenderPassMTL::EncodeCommands(const std::shared_ptr& allocator, - id encoder) const { - PassBindingsCache pass_bindings(encoder); - auto bind_stage_resources = [&allocator, &pass_bindings]( - const Bindings& bindings, - ShaderStage stage) -> bool { - for (const BufferAndUniformSlot& buffer : bindings.buffers) { - if (!Bind(pass_bindings, *allocator, stage, buffer.slot.ext_res_0, - buffer.view.resource)) { - return false; - } - } - for (const TextureAndSampler& data : bindings.sampled_images) { - if (!Bind(pass_bindings, stage, data.slot.texture_index, *data.sampler, - *data.texture.resource)) { - return false; - } - } - return true; - }; +// |RenderPass| +void RenderPassMTL::SetPipeline( + const std::shared_ptr>& pipeline) { + const PipelineDescriptor& pipeline_desc = pipeline->GetDescriptor(); + primitive_type_ = pipeline_desc.GetPrimitiveType(); + pass_bindings_.SetRenderPipelineState( + PipelineMTL::Cast(*pipeline).GetMTLRenderPipelineState()); + pass_bindings_.SetDepthStencilState( + PipelineMTL::Cast(*pipeline).GetMTLDepthStencilState()); - const auto target_sample_count = render_target_.GetSampleCount(); + [encoder_ setFrontFacingWinding:pipeline_desc.GetWindingOrder() == + WindingOrder::kClockwise + ? MTLWindingClockwise + : MTLWindingCounterClockwise]; + [encoder_ setCullMode:ToMTLCullMode(pipeline_desc.GetCullMode())]; + [encoder_ setTriangleFillMode:ToMTLTriangleFillMode( + pipeline_desc.GetPolygonMode())]; + has_valid_pipeline_ = true; +} - fml::closure pop_debug_marker = [encoder]() { [encoder popDebugGroup]; }; - for (const auto& command : commands_) { +// |RenderPass| +void RenderPassMTL::SetCommandLabel(std::string_view label) { #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(); - } + if (is_metal_trace_active_) { + has_label_ = true; + std::string label_copy(label); + [encoder_ pushDebugGroup:@(label_copy.c_str())]; + } #endif // IMPELLER_DEBUG +} - const auto& pipeline_desc = command.pipeline->GetDescriptor(); - if (target_sample_count != pipeline_desc.GetSampleCount()) { - VALIDATION_LOG << "Pipeline for command and the render target disagree " - "on sample counts (target was " - << static_cast(target_sample_count) - << " but pipeline wanted " - << static_cast(pipeline_desc.GetSampleCount()) - << ")."; - return false; - } +// |RenderPass| +void RenderPassMTL::SetStencilReference(uint32_t value) { + [encoder_ setStencilReferenceValue:value]; +} - pass_bindings.SetRenderPipelineState( - PipelineMTL::Cast(*command.pipeline).GetMTLRenderPipelineState()); - pass_bindings.SetDepthStencilState( - PipelineMTL::Cast(*command.pipeline).GetMTLDepthStencilState()); - pass_bindings.SetViewport(command.viewport.value_or( - {.rect = Rect::MakeSize(GetRenderTargetSize())})); - pass_bindings.SetScissor( - command.scissor.value_or(IRect::MakeSize(GetRenderTargetSize()))); +// |RenderPass| +void RenderPassMTL::SetBaseVertex(uint64_t value) { + base_vertex_ = value; +} - [encoder setFrontFacingWinding:pipeline_desc.GetWindingOrder() == - WindingOrder::kClockwise - ? MTLWindingClockwise - : MTLWindingCounterClockwise]; - [encoder setCullMode:ToMTLCullMode(pipeline_desc.GetCullMode())]; - [encoder setTriangleFillMode:ToMTLTriangleFillMode( - pipeline_desc.GetPolygonMode())]; - [encoder setStencilReferenceValue:command.stencil_reference]; +// |RenderPass| +void RenderPassMTL::SetViewport(Viewport viewport) { + pass_bindings_.SetViewport(viewport); +} - if (!Bind(pass_bindings, *allocator, ShaderStage::kVertex, - VertexDescriptor::kReservedVertexBufferIndex, - command.vertex_buffer.vertex_buffer)) { - return false; - } +// |RenderPass| +void RenderPassMTL::SetScissor(IRect scissor) { + pass_bindings_.SetScissor(scissor); +} - if (!bind_stage_resources(command.vertex_bindings, ShaderStage::kVertex)) { - return false; - } - if (!bind_stage_resources(command.fragment_bindings, - ShaderStage::kFragment)) { - return false; - } +// |RenderPass| +void RenderPassMTL::SetInstanceCount(size_t count) { + instance_count_ = count; +} - const PrimitiveType primitive_type = pipeline_desc.GetPrimitiveType(); - if (command.vertex_buffer.index_type == IndexType::kNone) { - if (command.instance_count != 1u) { -#if TARGET_OS_SIMULATOR - VALIDATION_LOG << "iOS Simulator does not support instanced rendering."; - return false; -#else // TARGET_OS_SIMULATOR - [encoder drawPrimitives:ToMTLPrimitiveType(primitive_type) - vertexStart:command.base_vertex - vertexCount:command.vertex_buffer.vertex_count - instanceCount:command.instance_count - baseInstance:0u]; -#endif // TARGET_OS_SIMULATOR - } else { - [encoder drawPrimitives:ToMTLPrimitiveType(primitive_type) - vertexStart:command.base_vertex - vertexCount:command.vertex_buffer.vertex_count]; - } - continue; - } +// |RenderPass| +bool RenderPassMTL::SetVertexBuffer(VertexBuffer buffer) { + if (buffer.index_type == IndexType::kUnknown) { + return false; + } - if (command.vertex_buffer.index_type == IndexType::kUnknown) { - return false; - } - auto index_buffer = command.vertex_buffer.index_buffer.buffer; - if (!index_buffer) { - return false; - } - auto mtl_index_buffer = DeviceBufferMTL::Cast(*index_buffer).GetMTLBuffer(); - if (!mtl_index_buffer) { - return false; - } + if (!Bind(pass_bindings_, ShaderStage::kVertex, + VertexDescriptor::kReservedVertexBufferIndex, + buffer.vertex_buffer)) { + return false; + } - FML_DCHECK( - command.vertex_buffer.vertex_count * - (command.vertex_buffer.index_type == IndexType::k16bit ? 2 : 4) == - command.vertex_buffer.index_buffer.range.length); - - if (command.instance_count != 1u) { -#if TARGET_OS_SIMULATOR - VALIDATION_LOG << "iOS Simulator does not support instanced rendering."; - return false; -#else // TARGET_OS_SIMULATOR - [encoder - drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type) - indexCount:command.vertex_buffer.vertex_count - indexType:ToMTLIndexType(command.vertex_buffer.index_type) - indexBuffer:mtl_index_buffer - indexBufferOffset:command.vertex_buffer.index_buffer.range.offset - instanceCount:command.instance_count - baseVertex:command.base_vertex - baseInstance:0u]; -#endif // TARGET_OS_SIMULATOR - } else { - [encoder - drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type) - indexCount:command.vertex_buffer.vertex_count - indexType:ToMTLIndexType(command.vertex_buffer.index_type) - indexBuffer:mtl_index_buffer - indexBufferOffset:command.vertex_buffer.index_buffer.range - .offset]; - } + vertex_count_ = buffer.vertex_count; + if (buffer.index_type != IndexType::kNone) { + index_type_ = ToMTLIndexType(buffer.index_type); + index_buffer_ = std::move(buffer.index_buffer); } return true; } +// |RenderPass| +fml::Status RenderPassMTL::Draw() { + if (!has_valid_pipeline_) { + return fml::Status(fml::StatusCode::kCancelled, "Invalid pipeline."); + } + + if (!index_buffer_) { + if (instance_count_ != 1u) { + [encoder_ drawPrimitives:ToMTLPrimitiveType(primitive_type_) + vertexStart:base_vertex_ + vertexCount:vertex_count_ + instanceCount:instance_count_ + baseInstance:0u]; + } else { + [encoder_ drawPrimitives:ToMTLPrimitiveType(primitive_type_) + vertexStart:base_vertex_ + vertexCount:vertex_count_]; + } + } else { + id mtl_index_buffer = + DeviceBufferMTL::Cast(*index_buffer_.buffer).GetMTLBuffer(); + if (instance_count_ != 1u) { + [encoder_ drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type_) + indexCount:vertex_count_ + indexType:index_type_ + indexBuffer:mtl_index_buffer + indexBufferOffset:index_buffer_.range.offset + instanceCount:instance_count_ + baseVertex:base_vertex_ + baseInstance:0u]; + } else { + [encoder_ drawIndexedPrimitives:ToMTLPrimitiveType(primitive_type_) + indexCount:vertex_count_ + indexType:index_type_ + indexBuffer:mtl_index_buffer + indexBufferOffset:index_buffer_.range.offset]; + } + } + +#ifdef IMPELLER_DEBUG + if (has_label_) { + [encoder_ popDebugGroup]; + } +#endif // IMPELLER_DEBUG + + vertex_count_ = 0u; + base_vertex_ = 0u; + instance_count_ = 1u; + index_buffer_ = {}; + has_valid_pipeline_ = false; + has_label_ = false; + + return fml::Status(); +} + +// |RenderPass| +bool RenderPassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const ShaderMetadata& metadata, + BufferView view) { + return Bind(pass_bindings_, stage, slot.ext_res_0, view); +} + +// |RenderPass| +bool RenderPassMTL::BindResource( + ShaderStage stage, + DescriptorType type, + const ShaderUniformSlot& slot, + const std::shared_ptr& metadata, + BufferView view) { + return Bind(pass_bindings_, stage, slot.ext_res_0, view); +} + +// |RenderPass| +bool RenderPassMTL::BindResource(ShaderStage stage, + DescriptorType type, + const SampledImageSlot& slot, + const ShaderMetadata& metadata, + std::shared_ptr texture, + std::shared_ptr sampler) { + return Bind(pass_bindings_, stage, slot.texture_index, *sampler, *texture); +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/command_buffer.cc b/engine/src/flutter/impeller/renderer/command_buffer.cc index b25ab3af00..7a675f560d 100644 --- a/engine/src/flutter/impeller/renderer/command_buffer.cc +++ b/engine/src/flutter/impeller/renderer/command_buffer.cc @@ -4,7 +4,6 @@ #include "impeller/renderer/command_buffer.h" -#include "flutter/fml/trace_event.h" #include "impeller/renderer/compute_pass.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" @@ -17,7 +16,6 @@ CommandBuffer::CommandBuffer(std::weak_ptr context) CommandBuffer::~CommandBuffer() = default; bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) { - TRACE_EVENT0("impeller", "CommandBuffer::SubmitCommands"); if (!IsValid()) { // Already committed or was never valid. Either way, this is caller error. if (callback) { diff --git a/engine/src/flutter/impeller/renderer/render_pass.h b/engine/src/flutter/impeller/renderer/render_pass.h index cae0fe3046..6655d18ec1 100644 --- a/engine/src/flutter/impeller/renderer/render_pass.h +++ b/engine/src/flutter/impeller/renderer/render_pass.h @@ -46,6 +46,9 @@ class RenderPass : public ResourceBinder { void SetLabel(std::string label); + /// @brief Reserve [command_count] commands in the HAL command buffer. + /// + /// Note: this is not the native command buffer. virtual void ReserveCommands(size_t command_count) { commands_.reserve(command_count); } diff --git a/engine/src/flutter/impeller/renderer/renderer_unittests.cc b/engine/src/flutter/impeller/renderer/renderer_unittests.cc index b01f8ed328..222e335d41 100644 --- a/engine/src/flutter/impeller/renderer/renderer_unittests.cc +++ b/engine/src/flutter/impeller/renderer/renderer_unittests.cc @@ -1272,6 +1272,7 @@ TEST_P(RendererTest, CanLookupRenderTargetProperties) { render_target.GetStencilAttachment().has_value()); EXPECT_EQ(render_pass->GetRenderTargetSize(), render_target.GetRenderTargetSize()); + render_pass->EncodeCommands(); } } // namespace testing