diff --git a/impeller/renderer/backend/gles/command_buffer_gles.cc b/impeller/renderer/backend/gles/command_buffer_gles.cc index d570cbc6e972e..2ade0db37f821 100644 --- a/impeller/renderer/backend/gles/command_buffer_gles.cc +++ b/impeller/renderer/backend/gles/command_buffer_gles.cc @@ -39,8 +39,8 @@ bool CommandBufferGLES::OnSubmitCommands(CompletionCallback callback) { } // |CommandBuffer| -void CommandBufferGLES::OnWaitUntilScheduled() { - reactor_->GetProcTable().Flush(); +void CommandBufferGLES::OnWaitUntilCompleted() { + reactor_->GetProcTable().Finish(); } // |CommandBuffer| diff --git a/impeller/renderer/backend/gles/command_buffer_gles.h b/impeller/renderer/backend/gles/command_buffer_gles.h index 2188ccdd3fa53..a1536d9433271 100644 --- a/impeller/renderer/backend/gles/command_buffer_gles.h +++ b/impeller/renderer/backend/gles/command_buffer_gles.h @@ -35,7 +35,7 @@ class CommandBufferGLES final : public CommandBuffer { bool OnSubmitCommands(CompletionCallback callback) override; // |CommandBuffer| - void OnWaitUntilScheduled() override; + void OnWaitUntilCompleted() 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 eadcb4f333057..f8503b52f9599 100644 --- a/impeller/renderer/backend/gles/proc_table_gles.h +++ b/impeller/renderer/backend/gles/proc_table_gles.h @@ -168,6 +168,7 @@ struct GLProc { PROC(DrawElements); \ PROC(Enable); \ PROC(EnableVertexAttribArray); \ + PROC(Finish); \ PROC(Flush); \ PROC(FramebufferRenderbuffer); \ PROC(FramebufferTexture2D); \ diff --git a/impeller/renderer/backend/gles/reactor_gles.cc b/impeller/renderer/backend/gles/reactor_gles.cc index 4ccbb47891b38..6f6e7288aa53c 100644 --- a/impeller/renderer/backend/gles/reactor_gles.cc +++ b/impeller/renderer/backend/gles/reactor_gles.cc @@ -105,8 +105,10 @@ bool ReactorGLES::RemoveWorker(WorkerID worker) { } bool ReactorGLES::HasPendingOperations() const { + auto thread_id = std::this_thread::get_id(); Lock ops_lock(ops_mutex_); - return !ops_.empty(); + auto it = ops_.find(thread_id); + return it != ops_.end() ? !it->second.empty() : false; } const ProcTableGLES& ReactorGLES::GetProcTable() const { @@ -136,9 +138,10 @@ bool ReactorGLES::AddOperation(Operation operation) { if (!operation) { return false; } + auto thread_id = std::this_thread::get_id(); { Lock ops_lock(ops_mutex_); - ops_.emplace_back(std::move(operation)); + ops_[thread_id].emplace_back(std::move(operation)); } // Attempt a reaction if able but it is not an error if this isn't possible. [[maybe_unused]] auto result = React(); @@ -191,10 +194,6 @@ bool ReactorGLES::React() { } TRACE_EVENT0("impeller", "ReactorGLES::React"); while (HasPendingOperations()) { - // Both the raster thread and the IO thread can flush queued operations. - // Ensure that execution of the ops is serialized. - Lock execution_lock(ops_execution_mutex_); - if (!ReactOnce()) { return false; } @@ -279,10 +278,11 @@ bool ReactorGLES::FlushOps() { // Do NOT hold the ops or handles locks while performing operations in case // the ops enqueue more ops. - decltype(ops_) ops; + decltype(ops_)::mapped_type ops; + auto thread_id = std::this_thread::get_id(); { Lock ops_lock(ops_mutex_); - std::swap(ops_, ops); + std::swap(ops_[thread_id], ops); } for (const auto& op : ops) { TRACE_EVENT0("impeller", "ReactorGLES::Operation"); diff --git a/impeller/renderer/backend/gles/reactor_gles.h b/impeller/renderer/backend/gles/reactor_gles.h index 80fb6877365bb..1b86a8ed328c1 100644 --- a/impeller/renderer/backend/gles/reactor_gles.h +++ b/impeller/renderer/backend/gles/reactor_gles.h @@ -260,9 +260,9 @@ class ReactorGLES { std::unique_ptr proc_table_; - Mutex ops_execution_mutex_; mutable Mutex ops_mutex_; - std::vector ops_ IPLR_GUARDED_BY(ops_mutex_); + std::map> ops_ IPLR_GUARDED_BY( + ops_mutex_); // Make sure the container is one where erasing items during iteration doesn't // invalidate other iterators. @@ -280,7 +280,7 @@ class ReactorGLES { bool can_set_debug_labels_ = false; bool is_valid_ = false; - bool ReactOnce() IPLR_REQUIRES(ops_execution_mutex_); + bool ReactOnce(); bool HasPendingOperations() const; diff --git a/impeller/renderer/backend/gles/test/reactor_unittests.cc b/impeller/renderer/backend/gles/test/reactor_unittests.cc index efa7b576be82b..fe589c7dbcaae 100644 --- a/impeller/renderer/backend/gles/test/reactor_unittests.cc +++ b/impeller/renderer/backend/gles/test/reactor_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include +#include "flutter/fml/synchronization/waitable_event.h" #include "flutter/testing/testing.h" // IWYU pragma: keep #include "gtest/gtest.h" #include "impeller/renderer/backend/gles/handle_gles.h" @@ -60,5 +61,37 @@ TEST(ReactorGLES, DeletesHandlesDuringShutdown) { calls.end()); } +TEST(ReactorGLES, PerThreadOperationQueues) { + auto mock_gles = MockGLES::Init(); + ProcTableGLES::Resolver resolver = kMockResolverGLES; + auto proc_table = std::make_unique(resolver); + auto worker = std::make_shared(); + auto reactor = std::make_shared(std::move(proc_table)); + reactor->AddWorker(worker); + + bool op1_called = false; + EXPECT_TRUE( + reactor->AddOperation([&](const ReactorGLES&) { op1_called = true; })); + + fml::AutoResetWaitableEvent event; + bool op2_called = false; + std::thread thread([&] { + EXPECT_TRUE( + reactor->AddOperation([&](const ReactorGLES&) { op2_called = true; })); + event.Wait(); + EXPECT_TRUE(reactor->React()); + }); + + // Reacting on the main thread should only run the main thread's operation. + EXPECT_TRUE(reactor->React()); + EXPECT_TRUE(op1_called); + EXPECT_FALSE(op2_called); + + // Reacting on the second thread will run the second thread's operation. + event.Signal(); + thread.join(); + EXPECT_TRUE(op2_called); +} + } // namespace testing } // namespace impeller diff --git a/impeller/renderer/backend/metal/command_buffer_mtl.h b/impeller/renderer/backend/metal/command_buffer_mtl.h index 6c5c948593713..c644ea114f457 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.h +++ b/impeller/renderer/backend/metal/command_buffer_mtl.h @@ -37,7 +37,7 @@ class CommandBufferMTL final : public CommandBuffer { bool OnSubmitCommands(CompletionCallback callback) override; // |CommandBuffer| - void OnWaitUntilScheduled() override; + void OnWaitUntilCompleted() 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 93bee9c7c1182..f6b907da54eff 100644 --- a/impeller/renderer/backend/metal/command_buffer_mtl.mm +++ b/impeller/renderer/backend/metal/command_buffer_mtl.mm @@ -187,7 +187,7 @@ static bool LogMTLCommandBufferErrorIfPresent(id buffer) { return true; } -void CommandBufferMTL::OnWaitUntilScheduled() {} +void CommandBufferMTL::OnWaitUntilCompleted() {} std::shared_ptr CommandBufferMTL::OnCreateRenderPass( RenderTarget target) { diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.cc b/impeller/renderer/backend/vulkan/command_buffer_vk.cc index 7abea7ab7604c..cbce517d435e9 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.cc +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.cc @@ -47,7 +47,7 @@ bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) { FML_UNREACHABLE() } -void CommandBufferVK::OnWaitUntilScheduled() {} +void CommandBufferVK::OnWaitUntilCompleted() {} std::shared_ptr CommandBufferVK::OnCreateRenderPass( RenderTarget target) { diff --git a/impeller/renderer/backend/vulkan/command_buffer_vk.h b/impeller/renderer/backend/vulkan/command_buffer_vk.h index 09bcadeca764b..e4ac9138859b1 100644 --- a/impeller/renderer/backend/vulkan/command_buffer_vk.h +++ b/impeller/renderer/backend/vulkan/command_buffer_vk.h @@ -100,7 +100,7 @@ class CommandBufferVK final bool OnSubmitCommands(CompletionCallback callback) override; // |CommandBuffer| - void OnWaitUntilScheduled() override; + void OnWaitUntilCompleted() override; // |CommandBuffer| std::shared_ptr OnCreateRenderPass(RenderTarget target) override; diff --git a/impeller/renderer/command_buffer.cc b/impeller/renderer/command_buffer.cc index 5ff1113c100ec..918e590da2beb 100644 --- a/impeller/renderer/command_buffer.cc +++ b/impeller/renderer/command_buffer.cc @@ -30,8 +30,8 @@ bool CommandBuffer::SubmitCommands() { return SubmitCommands(nullptr); } -void CommandBuffer::WaitUntilScheduled() { - return OnWaitUntilScheduled(); +void CommandBuffer::WaitUntilCompleted() { + return OnWaitUntilCompleted(); } std::shared_ptr CommandBuffer::CreateRenderPass( diff --git a/impeller/renderer/command_buffer.h b/impeller/renderer/command_buffer.h index 958b4d0bd41f1..cbfd406895c5f 100644 --- a/impeller/renderer/command_buffer.h +++ b/impeller/renderer/command_buffer.h @@ -61,9 +61,10 @@ class CommandBuffer { virtual void SetLabel(std::string_view label) const = 0; //---------------------------------------------------------------------------- - /// @brief Force execution of pending GPU commands. + /// @brief Block the current thread until the GPU has completed execution + /// of the commands. /// - void WaitUntilScheduled(); + void WaitUntilCompleted(); //---------------------------------------------------------------------------- /// @brief Create a render pass to record render commands into. @@ -102,7 +103,7 @@ class CommandBuffer { [[nodiscard]] virtual bool OnSubmitCommands(CompletionCallback callback) = 0; - virtual void OnWaitUntilScheduled() = 0; + virtual void OnWaitUntilCompleted() = 0; virtual std::shared_ptr OnCreateComputePass() = 0; diff --git a/impeller/renderer/testing/mocks.h b/impeller/renderer/testing/mocks.h index b389668135815..99464fa9e7597 100644 --- a/impeller/renderer/testing/mocks.h +++ b/impeller/renderer/testing/mocks.h @@ -128,7 +128,7 @@ class MockCommandBuffer : public CommandBuffer { OnSubmitCommands, (CompletionCallback callback), (override)); - MOCK_METHOD(void, OnWaitUntilScheduled, (), (override)); + MOCK_METHOD(void, OnWaitUntilCompleted, (), (override)); MOCK_METHOD(std::shared_ptr, OnCreateComputePass, (), diff --git a/lib/ui/painting/image_decoder_impeller.cc b/lib/ui/painting/image_decoder_impeller.cc index 1355653d59a7e..656b3a4aa165c 100644 --- a/lib/ui/painting/image_decoder_impeller.cc +++ b/lib/ui/painting/image_decoder_impeller.cc @@ -384,6 +384,10 @@ ImageDecoderImpeller::UnsafeUploadTextureToPrivate( return std::make_pair(nullptr, decode_error); } + // Flush the pending command buffer to ensure that its output becomes visible + // to the raster thread. + command_buffer->WaitUntilCompleted(); + context->DisposeThreadLocalCachedResources(); return std::make_pair(