From 1a1cc2a24abd0890e99dda79c3a7fabd0f257935 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Sat, 19 Nov 2022 11:32:14 +0800 Subject: [PATCH 1/5] =?UTF-8?q?[Impeller]=20Support=20=E2=80=98SyncMode?= =?UTF-8?q?=E2=80=99=20in=20'CommandBuffer:::SubmitCommands'?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- impeller/aiks/aiks_context.cc | 7 +++- impeller/aiks/aiks_context.h | 4 +- impeller/aiks/picture.cc | 14 ++++--- impeller/aiks/picture.h | 8 +++- impeller/entity/entity_pass.cc | 40 ++++++++++++------- impeller/entity/entity_pass.h | 21 +++++----- impeller/entity/inline_pass_context.cc | 12 ++++-- impeller/entity/inline_pass_context.h | 4 +- .../backend/gles/command_buffer_gles.cc | 16 +++++++- .../backend/gles/command_buffer_gles.h | 3 +- .../renderer/backend/gles/proc_table_gles.h | 4 +- .../backend/metal/command_buffer_mtl.h | 3 +- .../backend/metal/command_buffer_mtl.mm | 14 +++++-- .../backend/vulkan/command_buffer_vk.cc | 3 +- .../backend/vulkan/command_buffer_vk.h | 3 +- impeller/renderer/command_buffer.cc | 13 ++++-- impeller/renderer/command_buffer.h | 14 ++++++- impeller/renderer/renderer_unittests.cc | 3 +- shell/common/snapshot_controller_impeller.cc | 4 +- 19 files changed, 132 insertions(+), 58 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index fd504cbe03ca3..b3b504db871c9 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -36,13 +36,16 @@ const ContentContext& AiksContext::GetContentContext() const { return *content_context_; } -bool AiksContext::Render(const Picture& picture, RenderTarget& render_target) { +bool AiksContext::Render(const Picture& picture, + RenderTarget& render_target, + bool wait_until_completed) { if (!IsValid()) { return false; } if (picture.pass) { - return picture.pass->Render(*content_context_, render_target); + return picture.pass->Render(*content_context_, render_target, + wait_until_completed); } return true; diff --git a/impeller/aiks/aiks_context.h b/impeller/aiks/aiks_context.h index d748f3156fcba..8cf502f018e15 100644 --- a/impeller/aiks/aiks_context.h +++ b/impeller/aiks/aiks_context.h @@ -28,7 +28,9 @@ class AiksContext { const ContentContext& GetContentContext() const; - bool Render(const Picture& picture, RenderTarget& render_target); + bool Render(const Picture& picture, + RenderTarget& render_target, + bool wait_until_completed = false); private: std::shared_ptr context_; diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index 29737411c0740..58e131656b7d7 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -22,24 +22,28 @@ std::optional Picture::Snapshot(AiksContext& context) { const auto translate = Matrix::MakeTranslation(-coverage.value().origin); auto texture = - RenderToTexture(context, ISize(coverage.value().size), translate); + RenderToTexture(context, ISize(coverage.value().size), translate, false); return impeller::Snapshot{ .texture = std::move(texture), .transform = Matrix::MakeTranslation(coverage.value().origin)}; } -std::shared_ptr Picture::ToImage(AiksContext& context, ISize size) { +std::shared_ptr Picture::ToImage(AiksContext& context, + ISize size, + bool wait_until_completed) { if (size.IsEmpty()) { return nullptr; } - auto texture = RenderToTexture(context, size); + auto texture = + RenderToTexture(context, size, std::nullopt, wait_until_completed); return texture ? std::make_shared(texture) : nullptr; } std::shared_ptr Picture::RenderToTexture( AiksContext& context, ISize size, - std::optional translate) { + std::optional translate, + bool wait_until_completed) { FML_DCHECK(!size.IsEmpty()); pass->IterateAllEntities([&translate](auto& entity) -> bool { @@ -64,7 +68,7 @@ std::shared_ptr Picture::RenderToTexture( return nullptr; } - if (!context.Render(*this, target)) { + if (!context.Render(*this, target, wait_until_completed)) { VALIDATION_LOG << "Could not render Picture to Texture."; return nullptr; } diff --git a/impeller/aiks/picture.h b/impeller/aiks/picture.h index 7224af3a7b1be..b69c2dc2cf1a7 100644 --- a/impeller/aiks/picture.h +++ b/impeller/aiks/picture.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include #include @@ -21,13 +22,16 @@ struct Picture { std::optional Snapshot(AiksContext& context); - std::shared_ptr ToImage(AiksContext& context, ISize size); + std::shared_ptr ToImage(AiksContext& context, + ISize size, + bool wait_until_completed = false); private: std::shared_ptr RenderToTexture( AiksContext& context, ISize size, - std::optional translate = std::nullopt); + std::optional translate = std::nullopt, + bool wait_until_completed = false); }; } // namespace impeller diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index b83c8480837ac..8d3f7d9c066e6 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -184,7 +184,8 @@ static RenderTarget CreateRenderTarget(ContentContext& renderer, } bool EntityPass::Render(ContentContext& renderer, - const RenderTarget& render_target) const { + const RenderTarget& render_target, + bool wait_until_completed) const { if (reads_from_pass_texture_ > 0) { auto offscreen_target = CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true); @@ -214,7 +215,11 @@ bool EntityPass::Render(ContentContext& renderer, if (!render_pass->EncodeCommands()) { return false; } - if (!command_buffer->SubmitCommands()) { + + auto sync_mode = wait_until_completed + ? CommandBuffer::SyncMode::kWaitUntilCompleted + : CommandBuffer::SyncMode::kDontCare; + if (!command_buffer->SubmitCommands(sync_mode)) { return false; } @@ -222,7 +227,7 @@ bool EntityPass::Render(ContentContext& renderer, } return OnRender(renderer, render_target.GetRenderTargetSize(), render_target, - Point(), Point(), 0); + Point(), Point(), 0, 0, nullptr, wait_until_completed); } EntityPass::EntityResult EntityPass::GetEntityForElement( @@ -378,20 +383,20 @@ struct StencilLayer { size_t stencil_depth; }; -bool EntityPass::OnRender( - ContentContext& renderer, - ISize root_pass_size, - const RenderTarget& render_target, - Point position, - Point parent_position, - uint32_t pass_depth, - size_t stencil_depth_floor, - std::shared_ptr backdrop_filter_contents) const { +bool EntityPass::OnRender(ContentContext& renderer, + ISize root_pass_size, + const RenderTarget& render_target, + Point position, + Point parent_position, + uint32_t pass_depth, + size_t stencil_depth_floor, + std::shared_ptr backdrop_filter_contents, + bool wait_until_completed) const { TRACE_EVENT0("impeller", "EntityPass::OnRender"); auto context = renderer.GetContext(); - InlinePassContext pass_context(context, render_target, - reads_from_pass_texture_); + InlinePassContext pass_context( + context, render_target, reads_from_pass_texture_, wait_until_completed); if (!pass_context.IsValid()) { return false; } @@ -401,7 +406,12 @@ bool EntityPass::OnRender( .stencil_depth = stencil_depth_floor}}; auto render_element = [&stencil_depth_floor, &pass_context, &pass_depth, - &renderer, &stencil_stack](Entity& element_entity) { + &renderer, &stencil_stack, + wait_until_completed](Entity& element_entity) { + // Make sure only one 'RenderPass' is created when 'wait_until_completed' is + // true. + FML_DCHECK(!wait_until_completed || + (wait_until_completed && pass_context.IsActive())); auto result = pass_context.GetRenderPass(pass_depth); if (!result.pass) { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index cb7c3b5a47a87..f118488756115 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -50,7 +50,8 @@ class EntityPass { EntityPass* GetSuperpass() const; bool Render(ContentContext& renderer, - const RenderTarget& render_target) const; + const RenderTarget& render_target, + bool wait_until_completed = false) const; void IterateAllEntities(const std::function& iterator); @@ -101,15 +102,15 @@ class EntityPass { uint32_t pass_depth, size_t stencil_depth_floor) const; - bool OnRender( - ContentContext& renderer, - ISize root_pass_size, - const RenderTarget& render_target, - Point position, - Point parent_position, - uint32_t pass_depth, - size_t stencil_depth_floor = 0, - std::shared_ptr backdrop_filter_contents = nullptr) const; + bool OnRender(ContentContext& renderer, + ISize root_pass_size, + const RenderTarget& render_target, + Point position, + Point parent_position, + uint32_t pass_depth, + size_t stencil_depth_floor = 0, + std::shared_ptr backdrop_filter_contents = nullptr, + bool wait_until_completed = false) const; std::vector elements_; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 99a1877c95442..d1fcc109ed6c7 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -15,10 +15,12 @@ namespace impeller { InlinePassContext::InlinePassContext(std::shared_ptr context, const RenderTarget& render_target, - uint32_t pass_texture_reads) + uint32_t pass_texture_reads, + bool wait_until_completed) : context_(std::move(context)), render_target_(render_target), - total_pass_reads_(pass_texture_reads) {} + total_pass_reads_(pass_texture_reads), + wait_until_completed_(wait_until_completed) {} InlinePassContext::~InlinePassContext() { EndPass(); @@ -47,8 +49,10 @@ bool InlinePassContext::EndPass() { if (!pass_->EncodeCommands()) { return false; } - - if (!command_buffer_->SubmitCommands()) { + auto sync_mode = wait_until_completed_ + ? CommandBuffer::SyncMode::kWaitUntilCompleted + : CommandBuffer::SyncMode::kDontCare; + if (!command_buffer_->SubmitCommands(sync_mode)) { return false; } diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index ee393bda6c4b2..59bd9fe75b3a6 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -19,7 +19,8 @@ class InlinePassContext { InlinePassContext(std::shared_ptr context, const RenderTarget& render_target, - uint32_t pass_texture_reads); + uint32_t pass_texture_reads, + bool wait_until_completed); ~InlinePassContext(); bool IsValid() const; @@ -38,6 +39,7 @@ class InlinePassContext { std::shared_ptr pass_; uint32_t pass_count_ = 0; uint32_t total_pass_reads_ = 0; + bool wait_until_completed_ = false; FML_DISALLOW_COPY_AND_ASSIGN(InlinePassContext); }; diff --git a/impeller/renderer/backend/gles/command_buffer_gles.cc b/impeller/renderer/backend/gles/command_buffer_gles.cc index c53c774330399..8686aa0f162fb 100644 --- a/impeller/renderer/backend/gles/command_buffer_gles.cc +++ b/impeller/renderer/backend/gles/command_buffer_gles.cc @@ -29,8 +29,22 @@ bool CommandBufferGLES::IsValid() const { } // |CommandBuffer| -bool CommandBufferGLES::OnSubmitCommands(CompletionCallback callback) { +bool CommandBufferGLES::OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) { const auto result = reactor_->React(); + if (result) { + const auto& gl = reactor_->GetProcTable(); + switch (sync_mode) { + case CommandBuffer::SyncMode::kWaitUntilScheduled: + gl.Flush(); + break; + case CommandBuffer::SyncMode::kWaitUntilCompleted: + gl.Finish(); + break; + case CommandBuffer::SyncMode::kDontCare: + break; + } + } if (callback) { callback(result ? CommandBuffer::Status::kCompleted : CommandBuffer::Status::kError); diff --git a/impeller/renderer/backend/gles/command_buffer_gles.h b/impeller/renderer/backend/gles/command_buffer_gles.h index 9aef71fa80842..c9f31575842cd 100644 --- a/impeller/renderer/backend/gles/command_buffer_gles.h +++ b/impeller/renderer/backend/gles/command_buffer_gles.h @@ -32,7 +32,8 @@ class CommandBufferGLES final : public CommandBuffer { bool IsValid() const override; // |CommandBuffer| - bool OnSubmitCommands(CompletionCallback callback) override; + bool OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) override; // |CommandBuffer| std::shared_ptr OnCreateRenderPass(RenderTarget target) override; diff --git a/impeller/renderer/backend/gles/proc_table_gles.h b/impeller/renderer/backend/gles/proc_table_gles.h index 6fb00e83c4a5a..9688ba4f65e53 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -163,7 +163,9 @@ struct GLProc { PROC(UseProgram); \ PROC(VertexAttribPointer); \ PROC(Viewport); \ - PROC(ReadPixels); + PROC(ReadPixels); \ + PROC(Flush); \ + PROC(Finish); #define FOR_EACH_IMPELLER_GLES3_PROC(PROC) PROC(BlitFramebuffer); diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.h b/impeller/renderer/backend/metal/command_buffer_mtl.h index 30dd3f6f5d6a4..895e5eb5058ed 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.h +++ b/impeller/renderer/backend/metal/command_buffer_mtl.h @@ -31,7 +31,8 @@ class CommandBufferMTL final : public CommandBuffer { bool IsValid() const override; // |CommandBuffer| - bool OnSubmitCommands(CompletionCallback callback) override; + bool OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) override; // |CommandBuffer| std::shared_ptr OnCreateRenderPass(RenderTarget target) override; diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.mm b/impeller/renderer/backend/metal/command_buffer_mtl.mm index 2e8a5f7d8a5af..79593b96c606f 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -151,7 +151,8 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { return CommandBufferMTL::Status::kError; } -bool CommandBufferMTL::OnSubmitCommands(CompletionCallback callback) { +bool CommandBufferMTL::OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) { if (callback) { [buffer_ addCompletedHandler:^(id buffer) { @@ -162,9 +163,16 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { callback(ToCommitResult(buffer.status)); }]; } - [buffer_ commit]; - [buffer_ waitUntilScheduled]; + + switch (sync_mode) { + case SyncMode::kWaitUntilCompleted: + [buffer_ waitUntilCompleted]; + case SyncMode::kWaitUntilScheduled: + case SyncMode::kDontCare: + [buffer_ waitUntilScheduled]; + } + buffer_ = nil; return true; } diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 6db5c36c2c391..ec0be04796bd3 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -64,7 +64,8 @@ bool CommandBufferVK::IsValid() const { return is_valid_; } -bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) { +bool CommandBufferVK::OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) { // TODO(https://github.com/flutter/flutter/issues/112387) // This needs to be the place where the command buffer, renderpass, // and the various descriptor sets in use by the command buffer are diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h index 2b004071d683e..5c835963b796f 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.h @@ -45,7 +45,8 @@ class CommandBufferVK final : public CommandBuffer { bool IsValid() const override; // |CommandBuffer| - bool OnSubmitCommands(CompletionCallback callback) override; + bool OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) override; // |CommandBuffer| std::shared_ptr OnCreateRenderPass(RenderTarget target) override; diff --git a/impeller/renderer/command_buffer.cc b/impeller/renderer/command_buffer.cc index c5e9c5920d180..0b38d305fc30f 100644 --- a/impeller/renderer/command_buffer.cc +++ b/impeller/renderer/command_buffer.cc @@ -16,7 +16,8 @@ CommandBuffer::CommandBuffer(std::weak_ptr context) CommandBuffer::~CommandBuffer() = default; -bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) { +bool CommandBuffer::SubmitCommands(SyncMode sync_mode, + const CompletionCallback& callback) { TRACE_EVENT0("impeller", "CommandBuffer::SubmitCommands"); if (!IsValid()) { // Already committed or was never valid. Either way, this is caller error. @@ -25,11 +26,15 @@ bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) { } return false; } - return OnSubmitCommands(callback); + return OnSubmitCommands(sync_mode, callback); +} + +bool CommandBuffer::SubmitCommands(const CompletionCallback& callback) { + return SubmitCommands(SyncMode::kDontCare, callback); } -bool CommandBuffer::SubmitCommands() { - return SubmitCommands(nullptr); +bool CommandBuffer::SubmitCommands(SyncMode sync_mode) { + return SubmitCommands(sync_mode, nullptr); } std::shared_ptr CommandBuffer::CreateRenderPass( diff --git a/impeller/renderer/command_buffer.h b/impeller/renderer/command_buffer.h index e043e165c2fd4..641ff5ea71b6d 100644 --- a/impeller/renderer/command_buffer.h +++ b/impeller/renderer/command_buffer.h @@ -45,6 +45,12 @@ class CommandBuffer { kCompleted, }; + enum class SyncMode { + kDontCare, + kWaitUntilScheduled, + kWaitUntilCompleted, + }; + using CompletionCallback = std::function; virtual ~CommandBuffer(); @@ -61,9 +67,12 @@ class CommandBuffer { /// /// @param[in] callback The completion callback. /// + [[nodiscard]] bool SubmitCommands(SyncMode sync_mode, + const CompletionCallback& callback); + [[nodiscard]] bool SubmitCommands(const CompletionCallback& callback); - [[nodiscard]] bool SubmitCommands(); + [[nodiscard]] bool SubmitCommands(SyncMode sync_mode = SyncMode::kDontCare); //---------------------------------------------------------------------------- /// @brief Create a render pass to record render commands into. @@ -100,7 +109,8 @@ class CommandBuffer { virtual std::shared_ptr OnCreateBlitPass() const = 0; - [[nodiscard]] virtual bool OnSubmitCommands(CompletionCallback callback) = 0; + [[nodiscard]] virtual bool OnSubmitCommands(SyncMode sync_mode, + CompletionCallback callback) = 0; virtual std::shared_ptr OnCreateComputePass() const = 0; diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index cfd3637f9a8c2..69284c1956ea7 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -622,7 +622,8 @@ TEST_P(RendererTest, CanBlitTextureToBuffer) { pass->EncodeCommands(context->GetResourceAllocator()); - if (!buffer->SubmitCommands()) { + if (!buffer->SubmitCommands( + CommandBuffer::SyncMode::kWaitUntilCompleted)) { return false; } } diff --git a/shell/common/snapshot_controller_impeller.cc b/shell/common/snapshot_controller_impeller.cc index 6e5d65b74769c..fdcf441079946 100644 --- a/shell/common/snapshot_controller_impeller.cc +++ b/shell/common/snapshot_controller_impeller.cc @@ -61,8 +61,8 @@ sk_sp SnapshotControllerImpeller::DoMakeRasterSnapshot( render_target_size.height *= scale_factor; } - std::shared_ptr image = - picture.ToImage(*context, render_target_size); + std::shared_ptr image = picture.ToImage( + *context, render_target_size, /**wait_until_completed=*/true); if (image) { return impeller::DlImageImpeller::Make(image->GetTexture()); } From 1b611f93f8405a41e1d93e676dc81042569ea1c2 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Sat, 19 Nov 2022 11:51:36 +0800 Subject: [PATCH 2/5] Clean code --- impeller/aiks/picture.h | 1 - impeller/entity/entity_pass.cc | 16 ++++++++-------- impeller/entity/inline_pass_context.cc | 10 ++++------ impeller/entity/inline_pass_context.h | 6 ++---- 4 files changed, 14 insertions(+), 19 deletions(-) diff --git a/impeller/aiks/picture.h b/impeller/aiks/picture.h index b69c2dc2cf1a7..c2497ad4e9e9f 100644 --- a/impeller/aiks/picture.h +++ b/impeller/aiks/picture.h @@ -4,7 +4,6 @@ #pragma once -#include #include #include #include diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 8d3f7d9c066e6..ac836fa9d6cbd 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -395,8 +395,8 @@ bool EntityPass::OnRender(ContentContext& renderer, TRACE_EVENT0("impeller", "EntityPass::OnRender"); auto context = renderer.GetContext(); - InlinePassContext pass_context( - context, render_target, reads_from_pass_texture_, wait_until_completed); + InlinePassContext pass_context(context, render_target, + reads_from_pass_texture_); if (!pass_context.IsValid()) { return false; } @@ -406,12 +406,7 @@ bool EntityPass::OnRender(ContentContext& renderer, .stencil_depth = stencil_depth_floor}}; auto render_element = [&stencil_depth_floor, &pass_context, &pass_depth, - &renderer, &stencil_stack, - wait_until_completed](Entity& element_entity) { - // Make sure only one 'RenderPass' is created when 'wait_until_completed' is - // true. - FML_DCHECK(!wait_until_completed || - (wait_until_completed && pass_context.IsActive())); + &renderer, &stencil_stack](Entity& element_entity) { auto result = pass_context.GetRenderPass(pass_depth); if (!result.pass) { @@ -558,6 +553,11 @@ bool EntityPass::OnRender(ContentContext& renderer, } } + if (wait_until_completed) { + FML_DCHECK(pass_context.IsActive()); + pass_context.EndPass(wait_until_completed); + } + return true; } diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index d1fcc109ed6c7..18625a5e786f1 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -15,12 +15,10 @@ namespace impeller { InlinePassContext::InlinePassContext(std::shared_ptr context, const RenderTarget& render_target, - uint32_t pass_texture_reads, - bool wait_until_completed) + uint32_t pass_texture_reads) : context_(std::move(context)), render_target_(render_target), - total_pass_reads_(pass_texture_reads), - wait_until_completed_(wait_until_completed) {} + total_pass_reads_(pass_texture_reads) {} InlinePassContext::~InlinePassContext() { EndPass(); @@ -41,7 +39,7 @@ std::shared_ptr InlinePassContext::GetTexture() { return render_target_.GetRenderTargetTexture(); } -bool InlinePassContext::EndPass() { +bool InlinePassContext::EndPass(bool wait_until_completed) { if (!IsActive()) { return true; } @@ -49,7 +47,7 @@ bool InlinePassContext::EndPass() { if (!pass_->EncodeCommands()) { return false; } - auto sync_mode = wait_until_completed_ + auto sync_mode = wait_until_completed ? CommandBuffer::SyncMode::kWaitUntilCompleted : CommandBuffer::SyncMode::kDontCare; if (!command_buffer_->SubmitCommands(sync_mode)) { diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index 59bd9fe75b3a6..c8b8455d4f83a 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -19,14 +19,13 @@ class InlinePassContext { InlinePassContext(std::shared_ptr context, const RenderTarget& render_target, - uint32_t pass_texture_reads, - bool wait_until_completed); + uint32_t pass_texture_reads); ~InlinePassContext(); bool IsValid() const; bool IsActive() const; std::shared_ptr GetTexture(); - bool EndPass(); + bool EndPass(bool wait_until_completed = false); const RenderTarget& GetRenderTarget() const; uint32_t GetPassCount() const; @@ -39,7 +38,6 @@ class InlinePassContext { std::shared_ptr pass_; uint32_t pass_count_ = 0; uint32_t total_pass_reads_ = 0; - bool wait_until_completed_ = false; FML_DISALLOW_COPY_AND_ASSIGN(InlinePassContext); }; From ac84daad2aa77feaab141db3c0cf84459fccf9c4 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Sat, 19 Nov 2022 12:10:29 +0800 Subject: [PATCH 3/5] Add a check --- impeller/entity/entity_pass.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index ac836fa9d6cbd..6b220c20ad71c 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -554,6 +554,7 @@ bool EntityPass::OnRender(ContentContext& renderer, } if (wait_until_completed) { + FML_DCHECK(reads_from_pass_texture_ == 0); FML_DCHECK(pass_context.IsActive()); pass_context.EndPass(wait_until_completed); } From 347007bb04463bc6bda2e6cc302c1e1f8d3cd160 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 22 Nov 2022 11:49:24 +0800 Subject: [PATCH 4/5] Use CommandBuffer::SyncMode as param instead of bool --- impeller/aiks/aiks_context.cc | 6 ++--- impeller/aiks/aiks_context.h | 8 +++--- impeller/aiks/picture.cc | 11 ++++---- impeller/aiks/picture.h | 9 ++++--- impeller/entity/entity_pass.cc | 14 ++++------ impeller/entity/entity_pass.h | 27 +++++++++++--------- impeller/entity/inline_pass_context.cc | 6 ++--- impeller/entity/inline_pass_context.h | 4 ++- shell/common/snapshot_controller_impeller.cc | 12 +++++---- 9 files changed, 50 insertions(+), 47 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index b3b504db871c9..1c7ad131c5563 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -5,6 +5,7 @@ #include "impeller/aiks/aiks_context.h" #include "impeller/aiks/picture.h" +#include "impeller/renderer/context.h" namespace impeller { @@ -38,14 +39,13 @@ const ContentContext& AiksContext::GetContentContext() const { bool AiksContext::Render(const Picture& picture, RenderTarget& render_target, - bool wait_until_completed) { + CommandBuffer::SyncMode sync_mode) { if (!IsValid()) { return false; } if (picture.pass) { - return picture.pass->Render(*content_context_, render_target, - wait_until_completed); + return picture.pass->Render(*content_context_, render_target, sync_mode); } return true; diff --git a/impeller/aiks/aiks_context.h b/impeller/aiks/aiks_context.h index 8cf502f018e15..c12c89a2d7416 100644 --- a/impeller/aiks/aiks_context.h +++ b/impeller/aiks/aiks_context.h @@ -8,6 +8,7 @@ #include "flutter/fml/macros.h" #include "impeller/entity/contents/content_context.h" +#include "impeller/renderer/command_buffer.h" #include "impeller/renderer/context.h" #include "impeller/renderer/render_target.h" @@ -28,9 +29,10 @@ class AiksContext { const ContentContext& GetContentContext() const; - bool Render(const Picture& picture, - RenderTarget& render_target, - bool wait_until_completed = false); + bool Render( + const Picture& picture, + RenderTarget& render_target, + CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare); private: std::shared_ptr context_; diff --git a/impeller/aiks/picture.cc b/impeller/aiks/picture.cc index 58e131656b7d7..6e1ac8c681df3 100644 --- a/impeller/aiks/picture.cc +++ b/impeller/aiks/picture.cc @@ -22,7 +22,7 @@ std::optional Picture::Snapshot(AiksContext& context) { const auto translate = Matrix::MakeTranslation(-coverage.value().origin); auto texture = - RenderToTexture(context, ISize(coverage.value().size), translate, false); + RenderToTexture(context, ISize(coverage.value().size), translate); return impeller::Snapshot{ .texture = std::move(texture), .transform = Matrix::MakeTranslation(coverage.value().origin)}; @@ -30,12 +30,11 @@ std::optional Picture::Snapshot(AiksContext& context) { std::shared_ptr Picture::ToImage(AiksContext& context, ISize size, - bool wait_until_completed) { + CommandBuffer::SyncMode sync_mode) { if (size.IsEmpty()) { return nullptr; } - auto texture = - RenderToTexture(context, size, std::nullopt, wait_until_completed); + auto texture = RenderToTexture(context, size, std::nullopt, sync_mode); return texture ? std::make_shared(texture) : nullptr; } @@ -43,7 +42,7 @@ std::shared_ptr Picture::RenderToTexture( AiksContext& context, ISize size, std::optional translate, - bool wait_until_completed) { + CommandBuffer::SyncMode sync_mode) { FML_DCHECK(!size.IsEmpty()); pass->IterateAllEntities([&translate](auto& entity) -> bool { @@ -68,7 +67,7 @@ std::shared_ptr Picture::RenderToTexture( return nullptr; } - if (!context.Render(*this, target, wait_until_completed)) { + if (!context.Render(*this, target, sync_mode)) { VALIDATION_LOG << "Could not render Picture to Texture."; return nullptr; } diff --git a/impeller/aiks/picture.h b/impeller/aiks/picture.h index c2497ad4e9e9f..27e1550a73dd9 100644 --- a/impeller/aiks/picture.h +++ b/impeller/aiks/picture.h @@ -21,16 +21,17 @@ struct Picture { std::optional Snapshot(AiksContext& context); - std::shared_ptr ToImage(AiksContext& context, - ISize size, - bool wait_until_completed = false); + std::shared_ptr ToImage( + AiksContext& context, + ISize size, + CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare); private: std::shared_ptr RenderToTexture( AiksContext& context, ISize size, std::optional translate = std::nullopt, - bool wait_until_completed = false); + CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare); }; } // namespace impeller diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 6b220c20ad71c..0a7fccce78498 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -185,7 +185,7 @@ static RenderTarget CreateRenderTarget(ContentContext& renderer, bool EntityPass::Render(ContentContext& renderer, const RenderTarget& render_target, - bool wait_until_completed) const { + CommandBuffer::SyncMode sync_mode) const { if (reads_from_pass_texture_ > 0) { auto offscreen_target = CreateRenderTarget(renderer, render_target.GetRenderTargetSize(), true); @@ -215,10 +215,6 @@ bool EntityPass::Render(ContentContext& renderer, if (!render_pass->EncodeCommands()) { return false; } - - auto sync_mode = wait_until_completed - ? CommandBuffer::SyncMode::kWaitUntilCompleted - : CommandBuffer::SyncMode::kDontCare; if (!command_buffer->SubmitCommands(sync_mode)) { return false; } @@ -227,7 +223,7 @@ bool EntityPass::Render(ContentContext& renderer, } return OnRender(renderer, render_target.GetRenderTargetSize(), render_target, - Point(), Point(), 0, 0, nullptr, wait_until_completed); + Point(), Point(), 0, 0, nullptr, sync_mode); } EntityPass::EntityResult EntityPass::GetEntityForElement( @@ -391,7 +387,7 @@ bool EntityPass::OnRender(ContentContext& renderer, uint32_t pass_depth, size_t stencil_depth_floor, std::shared_ptr backdrop_filter_contents, - bool wait_until_completed) const { + CommandBuffer::SyncMode sync_mode) const { TRACE_EVENT0("impeller", "EntityPass::OnRender"); auto context = renderer.GetContext(); @@ -553,10 +549,10 @@ bool EntityPass::OnRender(ContentContext& renderer, } } - if (wait_until_completed) { + if (sync_mode != CommandBuffer::SyncMode::kDontCare) { FML_DCHECK(reads_from_pass_texture_ == 0); FML_DCHECK(pass_context.IsActive()); - pass_context.EndPass(wait_until_completed); + pass_context.EndPass(sync_mode); } return true; diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index f118488756115..252262dcfd2fb 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -15,6 +15,7 @@ #include "impeller/entity/entity.h" #include "impeller/entity/entity_pass_delegate.h" #include "impeller/entity/inline_pass_context.h" +#include "impeller/renderer/context.h" #include "impeller/renderer/render_target.h" #include "impeller/typographer/lazy_glyph_atlas.h" @@ -49,9 +50,10 @@ class EntityPass { EntityPass* GetSuperpass() const; - bool Render(ContentContext& renderer, - const RenderTarget& render_target, - bool wait_until_completed = false) const; + bool Render( + ContentContext& renderer, + const RenderTarget& render_target, + CommandBuffer::SyncMode = CommandBuffer::SyncMode::kDontCare) const; void IterateAllEntities(const std::function& iterator); @@ -102,15 +104,16 @@ class EntityPass { uint32_t pass_depth, size_t stencil_depth_floor) const; - bool OnRender(ContentContext& renderer, - ISize root_pass_size, - const RenderTarget& render_target, - Point position, - Point parent_position, - uint32_t pass_depth, - size_t stencil_depth_floor = 0, - std::shared_ptr backdrop_filter_contents = nullptr, - bool wait_until_completed = false) const; + bool OnRender( + ContentContext& renderer, + ISize root_pass_size, + const RenderTarget& render_target, + Point position, + Point parent_position, + uint32_t pass_depth, + size_t stencil_depth_floor = 0, + std::shared_ptr backdrop_filter_contents = nullptr, + CommandBuffer::SyncMode = CommandBuffer::SyncMode::kDontCare) const; std::vector elements_; diff --git a/impeller/entity/inline_pass_context.cc b/impeller/entity/inline_pass_context.cc index 18625a5e786f1..75e9f0d4b8949 100644 --- a/impeller/entity/inline_pass_context.cc +++ b/impeller/entity/inline_pass_context.cc @@ -39,7 +39,7 @@ std::shared_ptr InlinePassContext::GetTexture() { return render_target_.GetRenderTargetTexture(); } -bool InlinePassContext::EndPass(bool wait_until_completed) { +bool InlinePassContext::EndPass(CommandBuffer::SyncMode sync_mode) { if (!IsActive()) { return true; } @@ -47,9 +47,7 @@ bool InlinePassContext::EndPass(bool wait_until_completed) { if (!pass_->EncodeCommands()) { return false; } - auto sync_mode = wait_until_completed - ? CommandBuffer::SyncMode::kWaitUntilCompleted - : CommandBuffer::SyncMode::kDontCare; + if (!command_buffer_->SubmitCommands(sync_mode)) { return false; } diff --git a/impeller/entity/inline_pass_context.h b/impeller/entity/inline_pass_context.h index c8b8455d4f83a..6bc5c4339e737 100644 --- a/impeller/entity/inline_pass_context.h +++ b/impeller/entity/inline_pass_context.h @@ -4,6 +4,7 @@ #pragma once +#include "impeller/renderer/command_buffer.h" #include "impeller/renderer/context.h" #include "impeller/renderer/render_pass.h" #include "impeller/renderer/render_target.h" @@ -25,7 +26,8 @@ class InlinePassContext { bool IsValid() const; bool IsActive() const; std::shared_ptr GetTexture(); - bool EndPass(bool wait_until_completed = false); + bool EndPass( + CommandBuffer::SyncMode sync_mode = CommandBuffer::SyncMode::kDontCare); const RenderTarget& GetRenderTarget() const; uint32_t GetPassCount() const; diff --git a/shell/common/snapshot_controller_impeller.cc b/shell/common/snapshot_controller_impeller.cc index fdcf441079946..d2f75e3fcff76 100644 --- a/shell/common/snapshot_controller_impeller.cc +++ b/shell/common/snapshot_controller_impeller.cc @@ -8,10 +8,11 @@ #include "flutter/flow/surface.h" #include "flutter/fml/trace_event.h" -#include "flutter/impeller/display_list/display_list_dispatcher.h" -#include "flutter/impeller/display_list/display_list_image_impeller.h" -#include "flutter/impeller/geometry/size.h" #include "flutter/shell/common/snapshot_controller.h" +#include "impeller/display_list/display_list_dispatcher.h" +#include "impeller/display_list/display_list_image_impeller.h" +#include "impeller/geometry/size.h" +#include "impeller/renderer/command_buffer.h" namespace flutter { @@ -61,8 +62,9 @@ sk_sp SnapshotControllerImpeller::DoMakeRasterSnapshot( render_target_size.height *= scale_factor; } - std::shared_ptr image = picture.ToImage( - *context, render_target_size, /**wait_until_completed=*/true); + std::shared_ptr image = + picture.ToImage(*context, render_target_size, + impeller::CommandBuffer::SyncMode::kWaitUntilCompleted); if (image) { return impeller::DlImageImpeller::Make(image->GetTexture()); } From 96f69a2c8346464459c9ed23bdee18ae9e779578 Mon Sep 17 00:00:00 2001 From: ColdPaleLight Date: Tue, 22 Nov 2022 13:08:45 +0800 Subject: [PATCH 5/5] Clean code --- impeller/aiks/aiks_context.cc | 1 - impeller/entity/entity_pass.h | 1 - 2 files changed, 2 deletions(-) diff --git a/impeller/aiks/aiks_context.cc b/impeller/aiks/aiks_context.cc index 1c7ad131c5563..b220717e7f5f9 100644 --- a/impeller/aiks/aiks_context.cc +++ b/impeller/aiks/aiks_context.cc @@ -5,7 +5,6 @@ #include "impeller/aiks/aiks_context.h" #include "impeller/aiks/picture.h" -#include "impeller/renderer/context.h" namespace impeller { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 252262dcfd2fb..81e583eda4746 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -15,7 +15,6 @@ #include "impeller/entity/entity.h" #include "impeller/entity/entity_pass_delegate.h" #include "impeller/entity/inline_pass_context.h" -#include "impeller/renderer/context.h" #include "impeller/renderer/render_target.h" #include "impeller/typographer/lazy_glyph_atlas.h"