From 5cf496f9d940217f21b517c9b94d8d58e07c5b08 Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Thu, 8 Sep 2022 12:37:56 +0200 Subject: [PATCH 1/7] Windows: Texture Registrar: Destroy textures on raster thread --- .../client_wrapper/core_implementations.cc | 27 ++++++++++++++++-- .../include/flutter/texture_registrar.h | 9 ++++-- .../testing/stub_flutter_api.cc | 14 ++++++---- .../client_wrapper/testing/stub_flutter_api.h | 13 +++++---- .../client_wrapper/texture_registrar_impl.h | 4 +++ .../texture_registrar_unittests.cc | 24 ++++++++++------ .../common/public/flutter_texture_registrar.h | 12 ++++---- shell/platform/glfw/flutter_glfw.cc | 14 +++++----- shell/platform/windows/flutter_windows.cc | 15 +++++++--- .../windows/flutter_windows_engine.cc | 21 ++++++++++++++ .../platform/windows/flutter_windows_engine.h | 3 ++ .../flutter_windows_engine_unittests.cc | 16 +++++++++++ .../flutter_windows_texture_registrar.cc | 28 +++++++++++-------- .../flutter_windows_texture_registrar.h | 5 ++-- ...ter_windows_texture_registrar_unittests.cc | 14 ++++++++-- 15 files changed, 163 insertions(+), 56 deletions(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index e90b26cd4d6e5..3053980500ef2 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -195,9 +195,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 = std::make_unique(); + captures->callback = callback; + FlutterDesktopTextureRegistrarUnregisterExternalTexture( + texture_registrar_ref_, texture_id, + [](void* opaque) { + auto captures = reinterpret_cast(opaque); + captures->callback(); + delete captures; + }, + captures.release()); +} + 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/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h index 7a2078f13daad..05f6adeb06c60 100644 --- a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h +++ b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h @@ -95,9 +95,12 @@ 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. - virtual bool UnregisterTexture(int64_t texture_id) = 0; + // Asynchronously unregisters an existing texture object. + virtual void UnregisterTexture(int64_t texture_id, + std::function callback) = 0; + + // Unregisters an existing texture object. + [[deprecated]] virtual bool UnregisterTexture(int64_t texture_id) = 0; }; } // namespace flutter diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc b/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc index 0612e8249b3b2..b9a18ba660b95 100644 --- a/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc +++ b/shell/platform/common/client_wrapper/testing/stub_flutter_api.cc @@ -109,15 +109,17 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture( return result; } -bool FlutterDesktopTextureRegistrarUnregisterExternalTexture( +void FlutterDesktopTextureRegistrarUnregisterExternalTexture( FlutterDesktopTextureRegistrarRef texture_registrar, - int64_t texture_id) { - bool result = false; + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data) { if (s_stub_implementation) { - result = s_stub_implementation->TextureRegistrarUnregisterExternalTexture( - texture_id); + s_stub_implementation->TextureRegistrarUnregisterExternalTexture( + texture_id, callback, user_data); + } else if (callback) { + callback(user_data); } - return result; } bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable( diff --git a/shell/platform/common/client_wrapper/testing/stub_flutter_api.h b/shell/platform/common/client_wrapper/testing/stub_flutter_api.h index 8676cab6c2808..0c1c94b8d27ae 100644 --- a/shell/platform/common/client_wrapper/testing/stub_flutter_api.h +++ b/shell/platform/common/client_wrapper/testing/stub_flutter_api.h @@ -65,18 +65,19 @@ class StubFlutterApi { FlutterDesktopMessageCallback callback, void* user_data) {} - // Called for FlutterDesktopRegisterExternalTexture. + // Called for FlutterDesktopTextureRegistrarRegisterExternalTexture. virtual int64_t TextureRegistrarRegisterExternalTexture( const FlutterDesktopTextureInfo* info) { return -1; } - // Called for FlutterDesktopUnregisterExternalTexture. - virtual bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) { - return false; - } + // Called for FlutterDesktopTextureRegistrarUnregisterExternalTexture. + virtual void TextureRegistrarUnregisterExternalTexture( + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data) {} - // Called for FlutterDesktopMarkExternalTextureFrameAvailable. + // Called for FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable. virtual bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) { return false; } diff --git a/shell/platform/common/client_wrapper/texture_registrar_impl.h b/shell/platform/common/client_wrapper/texture_registrar_impl.h index df51f6ddb5232..bd01839d40f42 100644 --- a/shell/platform/common/client_wrapper/texture_registrar_impl.h +++ b/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/shell/platform/common/client_wrapper/texture_registrar_unittests.cc b/shell/platform/common/client_wrapper/texture_registrar_unittests.cc index 02ccc6fa76891..e770f06611c77 100644 --- a/shell/platform/common/client_wrapper/texture_registrar_unittests.cc +++ b/shell/platform/common/client_wrapper/texture_registrar_unittests.cc @@ -8,6 +8,7 @@ #include #include +#include "flutter/fml/synchronization/waitable_event.h" #include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h" #include "flutter/shell/platform/common/client_wrapper/testing/stub_flutter_api.h" #include "gtest/gtest.h" @@ -41,13 +42,17 @@ class TestApi : public testing::StubFlutterApi { return last_texture_id_; } - bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) override { + void TextureRegistrarUnregisterExternalTexture( + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data) override { auto it = textures_.find(texture_id); if (it != textures_.end()) { textures_.erase(it); - return true; } - return false; + if (callback) { + callback(user_data); + } } bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) override { @@ -110,15 +115,17 @@ TEST(TextureRegistrarTest, RegisterUnregisterTexture) { EXPECT_TRUE(success); EXPECT_EQ(texture->mark_count, 3); - success = textures->UnregisterTexture(texture_id); - EXPECT_TRUE(success); + fml::AutoResetWaitableEvent unregister_latch; + textures->UnregisterTexture(texture_id, [&]() { unregister_latch.Signal(); }); + unregister_latch.Wait(); texture = test_api->GetFakeTexture(texture_id); EXPECT_EQ(texture, nullptr); EXPECT_EQ(test_api->textures_size(), static_cast(0)); } -// Tests that unregistering a texture with an unknown id returns false. +// Tests that the unregister callback gets also invoked when attempting to +// unregister a texture with an unknown id. TEST(TextureRegistrarTest, UnregisterInvalidTexture) { auto dummy_registrar_handle = reinterpret_cast(1); @@ -126,8 +133,9 @@ TEST(TextureRegistrarTest, UnregisterInvalidTexture) { TextureRegistrar* textures = registrar.texture_registrar(); - bool success = textures->UnregisterTexture(42); - EXPECT_FALSE(success); + fml::AutoResetWaitableEvent latch; + textures->UnregisterTexture(42, [&]() { latch.Signal(); }); + latch.Wait(); } // Tests that claiming a new frame being available for an unknown texture diff --git a/shell/platform/common/public/flutter_texture_registrar.h b/shell/platform/common/public/flutter_texture_registrar.h index 45921613833e3..9979e9a2c15fc 100644 --- a/shell/platform/common/public/flutter_texture_registrar.h +++ b/shell/platform/common/public/flutter_texture_registrar.h @@ -162,13 +162,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/shell/platform/glfw/flutter_glfw.cc b/shell/platform/glfw/flutter_glfw.cc index 5079f8c7c2d3f..a388d218f1823 100644 --- a/shell/platform/glfw/flutter_glfw.cc +++ b/shell/platform/glfw/flutter_glfw.cc @@ -728,10 +728,9 @@ static void SetUpLocales(FlutterDesktopEngineState* state) { // 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; }); FlutterEngineResult result = FlutterEngineUpdateLocales( state->flutter_engine, flutter_locale_list.data(), flutter_locale_list.size()); @@ -1114,11 +1113,12 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture( return -1; } -bool FlutterDesktopTextureRegistrarUnregisterExternalTexture( +void FlutterDesktopTextureRegistrarUnregisterExternalTexture( FlutterDesktopTextureRegistrarRef texture_registrar, - int64_t texture_id) { + int64_t texture_id, + void (*callback)(void* user_data), + void* user_data) { std::cerr << "GLFW Texture support is not implemented yet." << std::endl; - return false; } bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable( diff --git a/shell/platform/windows/flutter_windows.cc b/shell/platform/windows/flutter_windows.cc index c5db187e926ba..0d8ca85fb7225 100644 --- a/shell/platform/windows/flutter_windows.cc +++ b/shell/platform/windows/flutter_windows.cc @@ -302,11 +302,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/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 8eabd6d1f9e72..5ccc20485fafa 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -557,6 +557,27 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable( engine_, texture_id) == kSuccess); } +bool FlutterWindowsEngine::PostRasterThreadTask( + std::function callback) { + struct Captures { + std::function callback; + }; + auto captures = std::make_unique(); + captures->callback = callback; + if (embedder_api_.PostRenderThreadTask( + engine_, + [](void* opaque) { + auto captures = reinterpret_cast(opaque); + captures->callback(); + delete captures; + }, + captures.get()) == kSuccess) { + captures.release(); + return true; + } + return false; +} + bool FlutterWindowsEngine::DispatchSemanticsAction( uint64_t target, FlutterSemanticsAction action, diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index 2e261917ce2fa..fe72016c327e2 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -190,6 +190,9 @@ class FlutterWindowsEngine { // given |texture_id|. bool MarkExternalTextureFrameAvailable(int64_t texture_id); + // Posts the given callback onto the raster thread. + bool PostRasterThreadTask(std::function callback); + // Invoke on the embedder's vsync callback to schedule a frame. void OnVsync(intptr_t baton); diff --git a/shell/platform/windows/flutter_windows_engine_unittests.cc b/shell/platform/windows/flutter_windows_engine_unittests.cc index 87482e1988e09..30a91f0e9959a 100644 --- a/shell/platform/windows/flutter_windows_engine_unittests.cc +++ b/shell/platform/windows/flutter_windows_engine_unittests.cc @@ -446,5 +446,21 @@ TEST(FlutterWindowsEngine, UpdateHighContrastFeature) { EXPECT_FALSE(engine->high_contrast_enabled()); } +TEST(FlutterWindowsEngine, PostRasterThreadTask) { + std::unique_ptr engine = GetTestEngine(); + EngineModifier modifier(engine.get()); + + modifier.embedder_api().PostRenderThreadTask = MOCK_ENGINE_PROC( + PostRenderThreadTask, ([](auto engine, auto callback, auto context) { + callback(context); + return kSuccess; + })); + + bool called = false; + engine->PostRasterThreadTask([&called]() { called = true; }); + + EXPECT_TRUE(called); +} + } // namespace testing } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_texture_registrar.cc b/shell/platform/windows/flutter_windows_texture_registrar.cc index 0abc73b75afe3..f74464350fcf0 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.cc +++ b/shell/platform/windows/flutter_windows_texture_registrar.cc @@ -77,20 +77,26 @@ int64_t FlutterWindowsTextureRegistrar::EmplaceTexture( return texture_id; } -bool FlutterWindowsTextureRegistrar::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 FlutterWindowsTextureRegistrar::UnregisterTexture( + int64_t texture_id, + std::function callback) { engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() { engine->UnregisterExternalTexture(texture_id); }); - return true; + + engine_->PostRasterThreadTask( + [this, texture_id, callback = std::move(callback)]() { + { + std::lock_guard lock(map_mutex_); + auto it = textures_.find(texture_id); + if (it != textures_.end()) { + textures_.erase(it); + } + } + if (callback) { + callback(); + } + }); } bool FlutterWindowsTextureRegistrar::MarkTextureFrameAvailable( diff --git a/shell/platform/windows/flutter_windows_texture_registrar.h b/shell/platform/windows/flutter_windows_texture_registrar.h index f1d2ac0a3b25c..4b7cca97e5b72 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.h +++ b/shell/platform/windows/flutter_windows_texture_registrar.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_ +#include #include #include #include @@ -28,8 +29,8 @@ class FlutterWindowsTextureRegistrar { 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, + std::function callback = nullptr); // Notifies the engine about a new frame being available. // Returns true on success. diff --git a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc index f0bddc1ae9f3f..90896e4d01ae0 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc +++ b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc @@ -4,6 +4,7 @@ #include +#include "flutter/fml/synchronization/waitable_event.h" #include "flutter/shell/platform/embedder/test_utils/proc_table_replacement.h" #include "flutter/shell/platform/windows/flutter_windows_engine.h" #include "flutter/shell/platform/windows/flutter_windows_texture_registrar.h" @@ -113,6 +114,13 @@ TEST(FlutterWindowsTextureRegistrarTest, RegisterUnregisterTexture) { return kSuccess; })); + modifier.embedder_api().PostRenderThreadTask = + MOCK_ENGINE_PROC(PostRenderThreadTask, + [](auto engine, auto callback, void* callback_data) { + callback(callback_data); + return kSuccess; + }); + auto texture_id = registrar.RegisterTexture(&texture_info); EXPECT_TRUE(register_called); EXPECT_NE(texture_id, -1); @@ -121,8 +129,10 @@ TEST(FlutterWindowsTextureRegistrarTest, RegisterUnregisterTexture) { EXPECT_TRUE(registrar.MarkTextureFrameAvailable(texture_id)); EXPECT_TRUE(mark_frame_available_called); - EXPECT_TRUE(registrar.UnregisterTexture(texture_id)); - EXPECT_TRUE(unregister_called); + fml::AutoResetWaitableEvent latch; + registrar.UnregisterTexture(texture_id, [&]() { latch.Signal(); }); + latch.Wait(); + ASSERT_TRUE(unregister_called); } TEST(FlutterWindowsTextureRegistrarTest, RegisterUnknownTextureType) { From 96d2120dc6bb735763092e0ef0fd13fefb91ffdf Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Thu, 8 Sep 2022 14:30:43 +0200 Subject: [PATCH 2/7] Ensure FlutterWindowsTextureRegistrar::UnregisterTexture callback gets always called --- .../flutter_windows_texture_registrar.cc | 29 ++++++++++--------- ...ter_windows_texture_registrar_unittests.cc | 12 ++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/shell/platform/windows/flutter_windows_texture_registrar.cc b/shell/platform/windows/flutter_windows_texture_registrar.cc index f74464350fcf0..9fec1901c4a6c 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.cc +++ b/shell/platform/windows/flutter_windows_texture_registrar.cc @@ -84,19 +84,22 @@ void FlutterWindowsTextureRegistrar::UnregisterTexture( engine->UnregisterExternalTexture(texture_id); }); - engine_->PostRasterThreadTask( - [this, texture_id, callback = std::move(callback)]() { - { - std::lock_guard lock(map_mutex_); - auto it = textures_.find(texture_id); - if (it != textures_.end()) { - textures_.erase(it); - } - } - if (callback) { - callback(); - } - }); + 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 FlutterWindowsTextureRegistrar::MarkTextureFrameAvailable( diff --git a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc index 90896e4d01ae0..af12b41fde674 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc +++ b/shell/platform/windows/flutter_windows_texture_registrar_unittests.cc @@ -305,5 +305,17 @@ TEST(FlutterWindowsTextureRegistrarTest, PopulateInvalidTexture) { EXPECT_FALSE(result); } +TEST(FlutterWindowsTextureRegistrarTest, + UnregisterTextureWithEngineDownInvokesCallback) { + std::unique_ptr engine = GetTestEngine(); + std::unique_ptr gl = std::make_unique(); + + FlutterWindowsTextureRegistrar registrar(engine.get(), gl->gl_procs()); + + fml::AutoResetWaitableEvent latch; + registrar.UnregisterTexture(1234, [&]() { latch.Signal(); }); + latch.Wait(); +} + } // namespace testing } // namespace flutter From 80659e55252e7b68338e3b56e257f2e8b7071710 Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Thu, 8 Sep 2022 14:55:50 +0200 Subject: [PATCH 3/7] Improve comments --- .../client_wrapper/include/flutter/texture_registrar.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h index 05f6adeb06c60..e8b06c90babd7 100644 --- a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h +++ b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h @@ -96,11 +96,15 @@ class TextureRegistrar { virtual bool MarkTextureFrameAvailable(int64_t texture_id) = 0; // 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]] virtual bool UnregisterTexture(int64_t texture_id) = 0; + [[deprecated( + "Use UnregisterTexture(texture_id, optional_callback) " + "instead.")]] virtual bool + UnregisterTexture(int64_t texture_id) = 0; }; } // namespace flutter From b8bdac5021d617658ff8ab967d65a3cf02ce0ab0 Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Thu, 8 Sep 2022 23:59:21 +0200 Subject: [PATCH 4/7] Address review comments --- .../platform/common/client_wrapper/core_implementations.cc | 4 ++-- .../client_wrapper/include/flutter/texture_registrar.h | 6 ++---- shell/platform/windows/flutter_windows_engine.cc | 7 +++---- shell/platform/windows/flutter_windows_engine.h | 2 +- .../platform/windows/flutter_windows_texture_registrar.cc | 5 ++--- shell/platform/windows/flutter_windows_texture_registrar.h | 5 ++--- 6 files changed, 12 insertions(+), 17 deletions(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index 3053980500ef2..7531273b10649 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -206,7 +206,7 @@ void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id, struct Captures { std::function callback; }; - auto captures = std::make_unique(); + auto captures = new Captures(); captures->callback = callback; FlutterDesktopTextureRegistrarUnregisterExternalTexture( texture_registrar_ref_, texture_id, @@ -215,7 +215,7 @@ void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id, captures->callback(); delete captures; }, - captures.release()); + captures); } bool TextureRegistrarImpl::UnregisterTexture(int64_t texture_id) { diff --git a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h index e8b06c90babd7..47daf7c42fbcd 100644 --- a/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h +++ b/shell/platform/common/client_wrapper/include/flutter/texture_registrar.h @@ -101,10 +101,8 @@ class TextureRegistrar { 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; + // DEPRECATED: Use UnregisterTexture(texture_id, optional_callback) instead. + virtual bool UnregisterTexture(int64_t texture_id) = 0; }; } // namespace flutter diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 5ccc20485fafa..13ccc810fc9d8 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -557,13 +557,12 @@ bool FlutterWindowsEngine::MarkExternalTextureFrameAvailable( engine_, texture_id) == kSuccess); } -bool FlutterWindowsEngine::PostRasterThreadTask( - std::function callback) { +bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) { struct Captures { - std::function callback; + fml::closure callback; }; auto captures = std::make_unique(); - captures->callback = callback; + captures->callback = std::move(callback); if (embedder_api_.PostRenderThreadTask( engine_, [](void* opaque) { diff --git a/shell/platform/windows/flutter_windows_engine.h b/shell/platform/windows/flutter_windows_engine.h index fe72016c327e2..579212183d657 100644 --- a/shell/platform/windows/flutter_windows_engine.h +++ b/shell/platform/windows/flutter_windows_engine.h @@ -191,7 +191,7 @@ class FlutterWindowsEngine { bool MarkExternalTextureFrameAvailable(int64_t texture_id); // Posts the given callback onto the raster thread. - bool PostRasterThreadTask(std::function callback); + bool PostRasterThreadTask(fml::closure callback); // Invoke on the embedder's vsync callback to schedule a frame. void OnVsync(intptr_t baton); diff --git a/shell/platform/windows/flutter_windows_texture_registrar.cc b/shell/platform/windows/flutter_windows_texture_registrar.cc index 9fec1901c4a6c..c3371b8d7b5ca 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.cc +++ b/shell/platform/windows/flutter_windows_texture_registrar.cc @@ -77,9 +77,8 @@ int64_t FlutterWindowsTextureRegistrar::EmplaceTexture( return texture_id; } -void FlutterWindowsTextureRegistrar::UnregisterTexture( - int64_t texture_id, - std::function callback) { +void FlutterWindowsTextureRegistrar::UnregisterTexture(int64_t texture_id, + fml::closure callback) { engine_->task_runner()->RunNowOrPostTask([engine = engine_, texture_id]() { engine->UnregisterExternalTexture(texture_id); }); diff --git a/shell/platform/windows/flutter_windows_texture_registrar.h b/shell/platform/windows/flutter_windows_texture_registrar.h index 4b7cca97e5b72..60534328eee4e 100644 --- a/shell/platform/windows/flutter_windows_texture_registrar.h +++ b/shell/platform/windows/flutter_windows_texture_registrar.h @@ -5,11 +5,11 @@ #ifndef FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_ #define FLUTTER_SHELL_PLATFORM_WINDOWS_FLUTTER_WINDOWS_TEXTURE_REGISTRAR_H_ -#include #include #include #include +#include "flutter/fml/closure.h" #include "flutter/shell/platform/common/public/flutter_texture_registrar.h" #include "flutter/shell/platform/windows/external_texture.h" @@ -29,8 +29,7 @@ class FlutterWindowsTextureRegistrar { int64_t RegisterTexture(const FlutterDesktopTextureInfo* texture_info); // Attempts to unregister the texture identified by |texture_id|. - void UnregisterTexture(int64_t texture_id, - std::function callback = nullptr); + void UnregisterTexture(int64_t texture_id, fml::closure callback = nullptr); // Notifies the engine about a new frame being available. // Returns true on success. From 01293543381f327121cd3bd1cb67f139935dc19b Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Fri, 9 Sep 2022 00:02:35 +0200 Subject: [PATCH 5/7] core_implementations.cc: Move callback to Captures --- shell/platform/common/client_wrapper/core_implementations.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/common/client_wrapper/core_implementations.cc b/shell/platform/common/client_wrapper/core_implementations.cc index 7531273b10649..41be7515ca9e0 100644 --- a/shell/platform/common/client_wrapper/core_implementations.cc +++ b/shell/platform/common/client_wrapper/core_implementations.cc @@ -207,7 +207,7 @@ void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id, std::function callback; }; auto captures = new Captures(); - captures->callback = callback; + captures->callback = std::move(callback); FlutterDesktopTextureRegistrarUnregisterExternalTexture( texture_registrar_ref_, texture_id, [](void* opaque) { From a9b3fac32f62aac585560255818b08359fb4387c Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Fri, 9 Sep 2022 00:24:26 +0200 Subject: [PATCH 6/7] Use new/free instead of unique_ptr --- shell/platform/windows/flutter_windows_engine.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shell/platform/windows/flutter_windows_engine.cc b/shell/platform/windows/flutter_windows_engine.cc index 13ccc810fc9d8..e862a94029b11 100644 --- a/shell/platform/windows/flutter_windows_engine.cc +++ b/shell/platform/windows/flutter_windows_engine.cc @@ -561,7 +561,7 @@ bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) { struct Captures { fml::closure callback; }; - auto captures = std::make_unique(); + auto captures = new Captures(); captures->callback = std::move(callback); if (embedder_api_.PostRenderThreadTask( engine_, @@ -570,10 +570,10 @@ bool FlutterWindowsEngine::PostRasterThreadTask(fml::closure callback) { captures->callback(); delete captures; }, - captures.get()) == kSuccess) { - captures.release(); + captures) == kSuccess) { return true; } + delete captures; return false; } From f105e5ea0812fa1b113c66fac37ad3e75a9c5f4a Mon Sep 17 00:00:00 2001 From: Niklas Schulze Date: Fri, 9 Sep 2022 11:57:02 +0200 Subject: [PATCH 7/7] Cleanup --- shell/platform/windows/angle_surface_manager.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/platform/windows/angle_surface_manager.cc b/shell/platform/windows/angle_surface_manager.cc index 95eb1d1224873..cd8973dad6d16 100644 --- a/shell/platform/windows/angle_surface_manager.cc +++ b/shell/platform/windows/angle_surface_manager.cc @@ -301,8 +301,6 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle( } bool AngleSurfaceManager::GetDevice(ID3D11Device** device) { - using Microsoft::WRL::ComPtr; - if (!resolved_device_) { PFNEGLQUERYDISPLAYATTRIBEXTPROC egl_query_display_attrib_EXT = reinterpret_cast(