From 0fb0618cd3b01e697f657c82fe0fb4f0907641c9 Mon Sep 17 00:00:00 2001 From: barribarrier <98932277+barribarrier@users.noreply.github.com> Date: Sun, 29 Sep 2024 22:05:04 -0700 Subject: [PATCH] Destroy textures on raster thread Signed-off-by: Bari Rao --- src/flutter/fml/closure.h | 77 +++++++++++++++++++ src/flutter/fml/macros.h | 44 +++++++++++ .../client_wrapper/core_implementations.cc | 27 ++++++- .../include/flutter/texture_registrar.h | 9 ++- .../client_wrapper/texture_registrar_impl.h | 4 + .../common/public/flutter_texture_registrar.h | 12 +-- .../platform/linux_embedded/flutter_elinux.cc | 15 +++- .../linux_embedded/flutter_elinux_engine.cc | 30 ++++++-- .../linux_embedded/flutter_elinux_engine.h | 3 + .../flutter_elinux_texture_registrar.cc | 30 +++++--- .../flutter_elinux_texture_registrar.h | 4 +- 11 files changed, 223 insertions(+), 32 deletions(-) create mode 100644 src/flutter/fml/closure.h create mode 100644 src/flutter/fml/macros.h diff --git a/src/flutter/fml/closure.h b/src/flutter/fml/closure.h new file mode 100644 index 00000000..264a95f7 --- /dev/null +++ b/src/flutter/fml/closure.h @@ -0,0 +1,77 @@ +// 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_FML_CLOSURE_H_ +#define FLUTTER_FML_CLOSURE_H_ + +#include + +#include "flutter/fml/macros.h" + +namespace fml { + +using closure = std::function; + +//------------------------------------------------------------------------------ +/// @brief Wraps a closure that is invoked in the destructor unless +/// released by the caller. +/// +/// This is especially useful in dealing with APIs that return a +/// resource by accepting ownership of a sub-resource and a closure +/// that releases that resource. When such APIs are chained, each +/// link in the chain must check that the next member in the chain +/// has accepted the resource. If not, it must invoke the closure +/// eagerly. Not doing this results in a resource leak in the +/// erroneous case. Using this wrapper, the closure can be released +/// once the next call in the chain has successfully accepted +/// ownership of the resource. If not, the closure gets invoked +/// automatically at the end of the scope. This covers the cases +/// where there are early returns as well. +/// +class ScopedCleanupClosure final { + public: + ScopedCleanupClosure() = default; + + ScopedCleanupClosure(ScopedCleanupClosure&& other) { + closure_ = other.Release(); + } + + ScopedCleanupClosure& operator=(ScopedCleanupClosure&& other) { + closure_ = other.Release(); + return *this; + } + + explicit ScopedCleanupClosure(const fml::closure& closure) + : closure_(closure) {} + + ~ScopedCleanupClosure() { Reset(); } + + fml::closure SetClosure(const fml::closure& closure) { + auto old_closure = closure_; + closure_ = closure; + return old_closure; + } + + fml::closure Release() { + fml::closure closure = closure_; + closure_ = nullptr; + return closure; + } + + void Reset() { + if (closure_) { + closure_(); + closure_ = nullptr; + } + } + + private: + fml::closure closure_; + + FML_DISALLOW_COPY_AND_ASSIGN(ScopedCleanupClosure); +}; + +} // namespace fml + +#endif // FLUTTER_FML_CLOSURE_H_ diff --git a/src/flutter/fml/macros.h b/src/flutter/fml/macros.h new file mode 100644 index 00000000..7de4ddf1 --- /dev/null +++ b/src/flutter/fml/macros.h @@ -0,0 +1,44 @@ +// 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_FML_MACROS_H_ +#define FLUTTER_FML_MACROS_H_ + +#ifndef FML_USED_ON_EMBEDDER + +#define FML_EMBEDDER_ONLY [[deprecated]] + +#else // FML_USED_ON_EMBEDDER + +#define FML_EMBEDDER_ONLY + +#endif // FML_USED_ON_EMBEDDER + +#define FML_DISALLOW_COPY(TypeName) TypeName(const TypeName&) = delete + +#define FML_DISALLOW_ASSIGN(TypeName) \ + TypeName& operator=(const TypeName&) = delete + +#define FML_DISALLOW_MOVE(TypeName) \ + TypeName(TypeName&&) = delete; \ + TypeName& operator=(TypeName&&) = delete + +#define FML_DISALLOW_COPY_AND_ASSIGN(TypeName) \ + TypeName(const TypeName&) = delete; \ + TypeName& operator=(const TypeName&) = delete + +#define FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName) \ + TypeName(const TypeName&) = delete; \ + TypeName(TypeName&&) = delete; \ + TypeName& operator=(const TypeName&) = delete; \ + TypeName& operator=(TypeName&&) = delete + +#define FML_DISALLOW_IMPLICIT_CONSTRUCTORS(TypeName) \ + TypeName() = delete; \ + FML_DISALLOW_COPY_ASSIGN_AND_MOVE(TypeName) + +#define FML_FRIEND_TEST(test_case_name, test_name) \ + friend class test_case_name##_##test_name##_Test + +#endif // FLUTTER_FML_MACROS_H_ diff --git a/src/flutter/shell/platform/common/client_wrapper/core_implementations.cc b/src/flutter/shell/platform/common/client_wrapper/core_implementations.cc index 983feb3e..ec0d6dd9 100644 --- a/src/flutter/shell/platform/common/client_wrapper/core_implementations.cc +++ b/src/flutter/shell/platform/common/client_wrapper/core_implementations.cc @@ -210,9 +210,32 @@ bool TextureRegistrarImpl::MarkTextureFrameAvailable(int64_t texture_id) { texture_registrar_ref_, texture_id); } +void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id, + std::function callback) { + if (callback == nullptr) { + FlutterDesktopTextureRegistrarUnregisterExternalTexture( + texture_registrar_ref_, texture_id, nullptr, nullptr); + return; + } + + struct Captures { + std::function callback; + }; + auto captures = new Captures(); + captures->callback = std::move(callback); + FlutterDesktopTextureRegistrarUnregisterExternalTexture( + texture_registrar_ref_, texture_id, + [](void* opaque) { + auto captures = reinterpret_cast(opaque); + captures->callback(); + delete captures; + }, + captures); +} + bool TextureRegistrarImpl::UnregisterTexture(int64_t texture_id) { - return FlutterDesktopTextureRegistrarUnregisterExternalTexture( - texture_registrar_ref_, texture_id); + UnregisterTexture(texture_id, nullptr); + return true; } } // namespace flutter diff --git a/src/flutter/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/src/flutter/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h index 0777e528..9b13025f 100644 --- a/src/flutter/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h +++ b/src/flutter/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h @@ -128,8 +128,13 @@ class TextureRegistrar { // the callback that was provided upon creating the texture. virtual bool MarkTextureFrameAvailable(int64_t texture_id) = 0; - // Unregisters an existing Texture object. - // Textures must not be unregistered while they're in use. + // Asynchronously unregisters an existing texture object. + // Upon completion, the optional |callback| gets invoked. + virtual void UnregisterTexture(int64_t texture_id, + std::function callback) = 0; + + // Unregisters an existing texture object. + // DEPRECATED: Use UnregisterTexture(texture_id, optional_callback) instead. virtual bool UnregisterTexture(int64_t texture_id) = 0; }; diff --git a/src/flutter/shell/platform/common/client_wrapper/texture_registrar_impl.h b/src/flutter/shell/platform/common/client_wrapper/texture_registrar_impl.h index df51f6dd..bd01839d 100644 --- a/src/flutter/shell/platform/common/client_wrapper/texture_registrar_impl.h +++ b/src/flutter/shell/platform/common/client_wrapper/texture_registrar_impl.h @@ -27,6 +27,10 @@ class TextureRegistrarImpl : public TextureRegistrar { // |flutter::TextureRegistrar| bool MarkTextureFrameAvailable(int64_t texture_id) override; + // |flutter::TextureRegistrar| + void UnregisterTexture(int64_t texture_id, + std::function callback) override; + // |flutter::TextureRegistrar| bool UnregisterTexture(int64_t texture_id) override; diff --git a/src/flutter/shell/platform/common/public/flutter_texture_registrar.h b/src/flutter/shell/platform/common/public/flutter_texture_registrar.h index fb564382..a56c3107 100644 --- a/src/flutter/shell/platform/common/public/flutter_texture_registrar.h +++ b/src/flutter/shell/platform/common/public/flutter_texture_registrar.h @@ -195,13 +195,15 @@ FLUTTER_EXPORT int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture( FlutterDesktopTextureRegistrarRef texture_registrar, const FlutterDesktopTextureInfo* info); -// Unregisters an existing texture from the Flutter engine for a |texture_id|. -// Returns true on success or false if the specified texture doesn't exist. +// Asynchronously unregisters the texture identified by |texture_id| from the +// Flutter engine. +// An optional |callback| gets invoked upon completion. // This function can be called from any thread. -// However, textures must not be unregistered while they're in use. -FLUTTER_EXPORT bool FlutterDesktopTextureRegistrarUnregisterExternalTexture( +FLUTTER_EXPORT void FlutterDesktopTextureRegistrarUnregisterExternalTexture( FlutterDesktopTextureRegistrarRef texture_registrar, - int64_t texture_id); + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data); // Marks that a new texture frame is available for a given |texture_id|. // Returns true on success or false if the specified texture doesn't exist. diff --git a/src/flutter/shell/platform/linux_embedded/flutter_elinux.cc b/src/flutter/shell/platform/linux_embedded/flutter_elinux.cc index fd3754a4..2d3587f3 100644 --- a/src/flutter/shell/platform/linux_embedded/flutter_elinux.cc +++ b/src/flutter/shell/platform/linux_embedded/flutter_elinux.cc @@ -284,11 +284,18 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture( ->RegisterTexture(texture_info); } -bool FlutterDesktopTextureRegistrarUnregisterExternalTexture( +void FlutterDesktopTextureRegistrarUnregisterExternalTexture( FlutterDesktopTextureRegistrarRef texture_registrar, - int64_t texture_id) { - return TextureRegistrarFromHandle(texture_registrar) - ->UnregisterTexture(texture_id); + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data) { + auto registrar = TextureRegistrarFromHandle(texture_registrar); + if (callback) { + registrar->UnregisterTexture( + texture_id, [callback, user_data]() { callback(user_data); }); + return; + } + registrar->UnregisterTexture(texture_id); } bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable( diff --git a/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.cc b/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.cc index 9e49c9e4..45a922e2 100644 --- a/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.cc +++ b/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.cc @@ -371,8 +371,7 @@ void FlutterELinuxEngine::HandlePlatformMessage( auto message = ConvertToDesktopMessage(*engine_message); - message_dispatcher_->HandleMessage( - message, [this] {}, [this] {}); + message_dispatcher_->HandleMessage(message, [this] {}, [this] {}); } void FlutterELinuxEngine::ReloadSystemFonts() { @@ -398,10 +397,9 @@ void FlutterELinuxEngine::SendSystemLocales() { // Convert the locale list to the locale pointer list that must be provided. std::vector flutter_locale_list; flutter_locale_list.reserve(flutter_locales.size()); - std::transform( - flutter_locales.begin(), flutter_locales.end(), - std::back_inserter(flutter_locale_list), - [](const auto& arg) -> const auto* { return &arg; }); + std::transform(flutter_locales.begin(), flutter_locales.end(), + std::back_inserter(flutter_locale_list), + [](const auto& arg) -> const auto* { return &arg; }); auto result = embedder_api_.UpdateLocales(engine_, flutter_locale_list.data(), flutter_locale_list.size()); if (result != kSuccess) { @@ -425,6 +423,26 @@ bool FlutterELinuxEngine::MarkExternalTextureFrameAvailable( engine_, texture_id) == kSuccess); } +bool FlutterELinuxEngine::PostRasterThreadTask(fml::closure callback) { + struct Captures { + fml::closure callback; + }; + auto captures = new Captures(); + captures->callback = std::move(callback); + if (embedder_api_.PostRenderThreadTask( + engine_, + [](void* opaque) { + auto captures = reinterpret_cast(opaque); + captures->callback(); + delete captures; + }, + captures) == kSuccess) { + return true; + } + delete captures; + return false; +} + void FlutterELinuxEngine::OnVsync(uint64_t last_frame_time_nanos, uint64_t vsync_interval_time_nanos) { uint64_t current_time_nanos = embedder_api_.GetCurrentTime(); diff --git a/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.h b/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.h index b705941e..4bcc7f10 100644 --- a/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.h +++ b/src/flutter/shell/platform/linux_embedded/flutter_elinux_engine.h @@ -120,6 +120,9 @@ class FlutterELinuxEngine { // given |texture_id|. bool MarkExternalTextureFrameAvailable(int64_t texture_id); + // Posts the given callback onto the raster thread. + bool PostRasterThreadTask(fml::closure callback); + // Notifies the engine about the vsync event. void OnVsync(uint64_t last_frame_time_nanos, uint64_t vsync_interval_time_nanos); diff --git a/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.cc b/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.cc index be4899fe..66a603c3 100644 --- a/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.cc +++ b/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.cc @@ -72,20 +72,28 @@ int64_t FlutterELinuxTextureRegistrar::EmplaceTexture( return texture_id; } -bool FlutterELinuxTextureRegistrar::UnregisterTexture(int64_t texture_id) { - { - std::lock_guard lock(map_mutex_); - auto it = textures_.find(texture_id); - if (it == textures_.end()) { - return false; - } - textures_.erase(it); - } - +void FlutterELinuxTextureRegistrar::UnregisterTexture(int64_t texture_id, + fml::closure callback) { engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() { engine->UnregisterExternalTexture(texture_id); }); - return true; + + bool posted = engine_->PostRasterThreadTask([this, texture_id, callback]() { + { + std::lock_guard lock(map_mutex_); + auto it = textures_.find(texture_id); + if (it != textures_.end()) { + textures_.erase(it); + } + } + if (callback) { + callback(); + } + }); + + if (!posted && callback) { + callback(); + } } bool FlutterELinuxTextureRegistrar::MarkTextureFrameAvailable( diff --git a/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.h b/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.h index 7ad57a2f..8770ad57 100644 --- a/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.h +++ b/src/flutter/shell/platform/linux_embedded/flutter_elinux_texture_registrar.h @@ -9,6 +9,7 @@ #include #include +#include "flutter/fml/closure.h" #include "flutter/shell/platform/common/public/flutter_texture_registrar.h" #include "flutter/shell/platform/linux_embedded/external_texture.h" @@ -28,8 +29,7 @@ class FlutterELinuxTextureRegistrar { int64_t RegisterTexture(const FlutterDesktopTextureInfo* texture_info); // Attempts to unregister the texture identified by |texture_id|. - // Returns true if the texture was successfully unregistered. - bool UnregisterTexture(int64_t texture_id); + void UnregisterTexture(int64_t texture_id, fml::closure callback = nullptr); // Notifies the engine about a new frame being available. // Returns true on success.