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 80220f5e3a..808918fe8e 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/handle_gles.h @@ -79,6 +79,7 @@ class HandleGLES { HandleType type_ = HandleType::kUnknown; std::optional name_; std::size_t hash_; + std::optional untracked_id_; friend class ReactorGLES; 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 a86b2119ab..4f869fe050 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.cc @@ -126,6 +126,10 @@ const ProcTableGLES& ReactorGLES::GetProcTable() const { std::optional ReactorGLES::GetHandle( const HandleGLES& handle) const { + if (handle.untracked_id_.has_value()) { + return ReactorGLES::GLStorage{.integer = handle.untracked_id_.value()}; + } + ReaderLock handles_lock(handles_mutex_); if (auto found = handles_.find(handle); found != handles_.end()) { if (found->second.pending_collection) { @@ -187,6 +191,7 @@ bool ReactorGLES::RegisterCleanupCallback(const HandleGLES& handle, if (handle.IsDead()) { return false; } + FML_DCHECK(!handle.untracked_id_.has_value()); WriterLock handles_lock(handles_mutex_); if (auto found = handles_.find(handle); found != handles_.end()) { found->second.callback = fml::ScopedCleanupClosure(callback); @@ -195,6 +200,17 @@ bool ReactorGLES::RegisterCleanupCallback(const HandleGLES& handle, return false; } +HandleGLES ReactorGLES::CreateUntrackedHandle(HandleType type) { + FML_DCHECK(CanReactOnCurrentThread()); + auto new_handle = HandleGLES::Create(type); + std::optional gl_handle = + CreateGLHandle(GetProcTable(), type); + if (gl_handle.has_value()) { + new_handle.untracked_id_ = gl_handle.value().integer; + } + return new_handle; +} + HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) { if (type == HandleType::kUnknown) { return HandleGLES::DeadHandle(); @@ -217,12 +233,19 @@ HandleGLES ReactorGLES::CreateHandle(HandleType type, GLuint external_handle) { } void ReactorGLES::CollectHandle(HandleGLES handle) { - WriterLock handles_lock(handles_mutex_); - if (auto found = handles_.find(handle); found != handles_.end()) { - if (!found->second.pending_collection) { - handles_to_collect_count_ += 1; + if (handle.untracked_id_.has_value()) { + LiveHandle live_handle(GLStorage{.integer = handle.untracked_id_.value()}); + live_handle.pending_collection = true; + WriterLock handles_lock(handles_mutex_); + handles_[handle] = std::move(live_handle); + } else { + WriterLock handles_lock(handles_mutex_); + if (auto found = handles_.find(handle); found != handles_.end()) { + if (!found->second.pending_collection) { + handles_to_collect_count_ += 1; + } + found->second.pending_collection = true; } - found->second.pending_collection = true; } } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h index 09dc092097..49191932e0 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/reactor_gles.h @@ -174,6 +174,16 @@ class ReactorGLES { /// HandleGLES CreateHandle(HandleType type, GLuint external_handle = GL_NONE); + /// @brief Create a handle that is not managed by `ReactorGLES`. + /// @details This behaves just like `CreateHandle` but it doesn't add the + /// handle to ReactorGLES::handles_ and the creation is executed + /// synchronously, so it must be called from a proper thread. The benefit of + /// this is that it avoid synchronization and hash table lookups when + /// creating/accessing the handle. + /// @param type The type of handle to create. + /// @return The reactor handle. + HandleGLES CreateUntrackedHandle(HandleType type); + //---------------------------------------------------------------------------- /// @brief Collect a reactor handle. /// @@ -255,8 +265,10 @@ class ReactorGLES { union { GLuint handle; GLsync sync; + uint64_t integer; }; }; + static_assert(sizeof(GLStorage) == sizeof(uint64_t)); struct LiveHandle { std::optional name; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc index e94e74dc28..2296351ed9 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc @@ -171,6 +171,17 @@ void mockUniform1fv(GLint location, GLsizei count, const GLfloat* value) { RecordGLCall("glUniform1fv"); } +void mockGenTextures(GLsizei n, GLuint* textures) { + RecordGLCall("glGenTextures"); + if (auto mock_gles = g_mock_gles.lock()) { + std::optional next_texture; + std::swap(mock_gles->next_texture_, next_texture); + if (next_texture.has_value()) { + textures[0] = next_texture.value(); + } + } +} + static_assert(CheckSameSignature::value); @@ -217,6 +228,8 @@ const ProcTableGLES::Resolver kMockResolverGLES = [](const char* name) { return reinterpret_cast(mockGetQueryObjectuivEXT); } else if (strcmp(name, "glUniform1fv") == 0) { return reinterpret_cast(mockUniform1fv); + } else if (strcmp(name, "glGenTextures") == 0) { + return reinterpret_cast(mockGenTextures); } else { return reinterpret_cast(&doNothing); } diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h index 0ce2bcb6e3..aa5a66c296 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h +++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.h @@ -50,8 +50,11 @@ class MockGLES final { ~MockGLES(); + void SetNextTexture(uint64_t next_texture) { next_texture_ = next_texture; } + private: friend void RecordGLCall(const char* name); + friend void mockGenTextures(GLsizei n, GLuint* textures); explicit MockGLES(ProcTableGLES::Resolver resolver = kMockResolverGLES); @@ -59,6 +62,7 @@ class MockGLES final { ProcTableGLES proc_table_; std::vector captured_calls_; + std::optional next_texture_; MockGLES(const MockGLES&) = delete; 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 c43b9b1b96..e83ffc2aaa 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 @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include #include "flutter/fml/synchronization/waitable_event.h" #include "flutter/testing/testing.h" // IWYU pragma: keep @@ -61,6 +62,35 @@ TEST(ReactorGLES, DeletesHandlesDuringShutdown) { calls.end()); } +TEST(ReactorGLES, UntrackedHandle) { + std::shared_ptr 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); + + mock_gles->SetNextTexture(1234u); + HandleGLES handle = reactor->CreateUntrackedHandle(HandleType::kTexture); + EXPECT_FALSE(handle.IsDead()); + std::optional glint = reactor->GetGLHandle(handle); + EXPECT_TRUE(glint.has_value()); + if (glint.has_value()) { + EXPECT_EQ(1234u, *glint); + } + mock_gles->GetCapturedCalls(); + reactor->CollectHandle(handle); + std::vector calls = mock_gles->GetCapturedCalls(); + EXPECT_TRUE(std::find(calls.begin(), calls.end(), "glDeleteTextures") == + calls.end()); + // Without an operation nothing is cleaned up. + EXPECT_TRUE(reactor->AddOperation([&](const ReactorGLES&) {})); + EXPECT_TRUE(reactor->React()); + calls = mock_gles->GetCapturedCalls(); + EXPECT_FALSE(std::find(calls.begin(), calls.end(), "glDeleteTextures") == + calls.end()); +} + TEST(ReactorGLES, PerThreadOperationQueues) { auto mock_gles = MockGLES::Init(); ProcTableGLES::Resolver resolver = kMockResolverGLES; diff --git a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc index 748fb5ee25..0b96430638 100644 --- a/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc +++ b/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.cc @@ -193,7 +193,7 @@ TextureGLES::TextureGLES(std::shared_ptr reactor, type_(GetTextureTypeFromDescriptor(GetTextureDescriptor())), handle_(external_handle.has_value() ? external_handle.value() - : reactor_->CreateHandle(ToHandleType(type_))), + : reactor_->CreateUntrackedHandle(ToHandleType(type_))), is_wrapped_(fbo.has_value() || external_handle.has_value()), wrapped_fbo_(fbo) { // Ensure the texture descriptor itself is valid. @@ -407,7 +407,7 @@ void TextureGLES::InitializeContentsIfNecessary() const { } const auto& gl = reactor_->GetProcTable(); - auto handle = reactor_->GetGLHandle(handle_); + std::optional handle = reactor_->GetGLHandle(handle_); if (!handle.has_value()) { VALIDATION_LOG << "Could not initialize the contents of texture."; return;