From e32ddabf3127b3ca90c593e246dba76930da2f96 Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 15 Dec 2025 10:55:09 -0800 Subject: [PATCH] [Impeller] Delete GLES framebuffer objects only on the thread where they were created (#179768) OpenGL does not allow cross-context sharing of container objects (such as framebuffers) that hold references to other objects. ReactorGLES must ensure that framebuffers are deleted using the thread and context where the object was created. Fixes https://github.com/flutter/flutter/issues/178276 --- .../renderer/backend/gles/handle_gles.cc | 4 +++ .../renderer/backend/gles/handle_gles.h | 13 +++++-- .../renderer/backend/gles/reactor_gles.cc | 8 +++++ .../backend/gles/test/reactor_unittests.cc | 35 +++++++++++++++++++ 4 files changed, 58 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.cc index f1ee08ac9b4..4791424d159 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.cc @@ -28,4 +28,8 @@ std::string HandleTypeToString(HandleType type) { FML_UNREACHABLE(); } +bool HandleTypeIsShareable(HandleType type) { + return type != HandleType::kFrameBuffer; +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h index 9d71b4c6e34..e6f51d2caae 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "flutter/fml/hash_combine.h" @@ -27,6 +28,9 @@ enum class HandleType { std::string HandleTypeToString(HandleType type); +// Returns whether this type of handle can be shared across OpenGL contexts. +bool HandleTypeIsShareable(HandleType type); + class ReactorGLES; //------------------------------------------------------------------------------ @@ -80,10 +84,11 @@ class HandleGLES { std::optional name_; std::size_t hash_; std::optional untracked_id_; + std::optional owner_thread_; friend class ReactorGLES; - HandleGLES(HandleType p_type, UniqueID p_name) + HandleGLES(HandleType p_type, UniqueID p_name, std::thread::id p_owner_thread) : type_(p_type), name_(p_name), hash_(fml::HashCombine( @@ -98,7 +103,11 @@ class HandleGLES { p_name)) {} static HandleGLES Create(HandleType type) { - return HandleGLES{type, UniqueID{}}; + HandleGLES handle{type, UniqueID{}}; + if (!HandleTypeIsShareable(type)) { + handle.owner_thread_ = std::this_thread::get_id(); + } + return handle; } }; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc index c4a35799a7a..926b0bc9db5 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc @@ -296,6 +296,7 @@ bool ReactorGLES::ReactOnce() { bool ReactorGLES::ConsolidateHandles() { TRACE_EVENT0("impeller", __FUNCTION__); const auto& gl = GetProcTable(); + std::thread::id current_thread = std::this_thread::get_id(); std::vector>> handles_to_delete; std::vector> @@ -307,6 +308,13 @@ bool ReactorGLES::ConsolidateHandles() { for (auto& handle : handles_) { // Collect dead handles. if (handle.second.pending_collection) { + // Some objects can only be deleted on a specific thread. Collection + // of these objects will be deferred until the owner thead executes + // a reaction and triggers collection of handles. + const auto& owner_thread = handle.first.owner_thread_; + if (owner_thread.has_value() && *owner_thread != current_thread) { + continue; + } handles_to_delete.emplace_back( std::make_tuple(handle.first, handle.second.name)); continue; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/reactor_unittests.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/reactor_unittests.cc index 3010e4c4806..c5e3adbcbc4 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/test/reactor_unittests.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/reactor_unittests.cc @@ -180,5 +180,40 @@ TEST(ReactorGLES, CanDeferOperations) { EXPECT_TRUE(did_run); } +TEST(ReactorGLES, FramebufferDeletedOnOwnerThread) { + auto mock_gles_impl = std::make_unique(); + + EXPECT_CALL(*mock_gles_impl, GenFramebuffers(1, _)) + .WillOnce([](GLsizei size, GLuint* queries) { queries[0] = 1234; }); + + std::shared_ptr mock_gles = + MockGLES::Init(std::move(mock_gles_impl)); + 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); + + HandleGLES handle = reactor->CreateHandle(HandleType::kFrameBuffer); + + std::thread::id cleanup_thread; + reactor->RegisterCleanupCallback( + handle, [&]() { cleanup_thread = std::this_thread::get_id(); }); + + reactor->CollectHandle(handle); + + std::thread thread([&] { + EXPECT_TRUE(reactor->AddOperation([](const ReactorGLES& reactor) {})); + EXPECT_TRUE(reactor->React()); + }); + thread.join(); + + EXPECT_EQ(cleanup_thread, std::thread::id()); + + EXPECT_TRUE(reactor->AddOperation([](const ReactorGLES& reactor) {})); + EXPECT_TRUE(reactor->React()); + EXPECT_EQ(cleanup_thread, std::this_thread::get_id()); +} + } // namespace testing } // namespace impeller