Skip to content

Commit 2bf705d

Browse files
jnschulzecfontas
authored andcommitted
Windows: Texture Registrar: Destroy textures on raster thread (flutter#33688)
1 parent ce1b620 commit 2bf705d

16 files changed

+176
-57
lines changed

shell/platform/common/client_wrapper/core_implementations.cc

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,32 @@ bool TextureRegistrarImpl::MarkTextureFrameAvailable(int64_t texture_id) {
195195
texture_registrar_ref_, texture_id);
196196
}
197197

198+
void TextureRegistrarImpl::UnregisterTexture(int64_t texture_id,
199+
std::function<void()> callback) {
200+
if (callback == nullptr) {
201+
FlutterDesktopTextureRegistrarUnregisterExternalTexture(
202+
texture_registrar_ref_, texture_id, nullptr, nullptr);
203+
return;
204+
}
205+
206+
struct Captures {
207+
std::function<void()> callback;
208+
};
209+
auto captures = new Captures();
210+
captures->callback = std::move(callback);
211+
FlutterDesktopTextureRegistrarUnregisterExternalTexture(
212+
texture_registrar_ref_, texture_id,
213+
[](void* opaque) {
214+
auto captures = reinterpret_cast<Captures*>(opaque);
215+
captures->callback();
216+
delete captures;
217+
},
218+
captures);
219+
}
220+
198221
bool TextureRegistrarImpl::UnregisterTexture(int64_t texture_id) {
199-
return FlutterDesktopTextureRegistrarUnregisterExternalTexture(
200-
texture_registrar_ref_, texture_id);
222+
UnregisterTexture(texture_id, nullptr);
223+
return true;
201224
}
202225

203226
} // namespace flutter

shell/platform/common/client_wrapper/include/flutter/texture_registrar.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,13 @@ class TextureRegistrar {
9595
// the callback that was provided upon creating the texture.
9696
virtual bool MarkTextureFrameAvailable(int64_t texture_id) = 0;
9797

98-
// Unregisters an existing Texture object.
99-
// Textures must not be unregistered while they're in use.
98+
// Asynchronously unregisters an existing texture object.
99+
// Upon completion, the optional |callback| gets invoked.
100+
virtual void UnregisterTexture(int64_t texture_id,
101+
std::function<void()> callback) = 0;
102+
103+
// Unregisters an existing texture object.
104+
// DEPRECATED: Use UnregisterTexture(texture_id, optional_callback) instead.
100105
virtual bool UnregisterTexture(int64_t texture_id) = 0;
101106
};
102107

shell/platform/common/client_wrapper/testing/stub_flutter_api.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -109,15 +109,17 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
109109
return result;
110110
}
111111

112-
bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
112+
void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
113113
FlutterDesktopTextureRegistrarRef texture_registrar,
114-
int64_t texture_id) {
115-
bool result = false;
114+
int64_t texture_id,
115+
void (*callback)(void* user_data),
116+
void* user_data) {
116117
if (s_stub_implementation) {
117-
result = s_stub_implementation->TextureRegistrarUnregisterExternalTexture(
118-
texture_id);
118+
s_stub_implementation->TextureRegistrarUnregisterExternalTexture(
119+
texture_id, callback, user_data);
120+
} else if (callback) {
121+
callback(user_data);
119122
}
120-
return result;
121123
}
122124

123125
bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/common/client_wrapper/testing/stub_flutter_api.h

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,19 @@ class StubFlutterApi {
6565
FlutterDesktopMessageCallback callback,
6666
void* user_data) {}
6767

68-
// Called for FlutterDesktopRegisterExternalTexture.
68+
// Called for FlutterDesktopTextureRegistrarRegisterExternalTexture.
6969
virtual int64_t TextureRegistrarRegisterExternalTexture(
7070
const FlutterDesktopTextureInfo* info) {
7171
return -1;
7272
}
7373

74-
// Called for FlutterDesktopUnregisterExternalTexture.
75-
virtual bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) {
76-
return false;
77-
}
74+
// Called for FlutterDesktopTextureRegistrarUnregisterExternalTexture.
75+
virtual void TextureRegistrarUnregisterExternalTexture(
76+
int64_t texture_id,
77+
void (*callback)(void* user_data),
78+
void* user_data) {}
7879

79-
// Called for FlutterDesktopMarkExternalTextureFrameAvailable.
80+
// Called for FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable.
8081
virtual bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) {
8182
return false;
8283
}

shell/platform/common/client_wrapper/texture_registrar_impl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ class TextureRegistrarImpl : public TextureRegistrar {
2727
// |flutter::TextureRegistrar|
2828
bool MarkTextureFrameAvailable(int64_t texture_id) override;
2929

30+
// |flutter::TextureRegistrar|
31+
void UnregisterTexture(int64_t texture_id,
32+
std::function<void()> callback) override;
33+
3034
// |flutter::TextureRegistrar|
3135
bool UnregisterTexture(int64_t texture_id) override;
3236

shell/platform/common/client_wrapper/texture_registrar_unittests.cc

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <memory>
99
#include <vector>
1010

11+
#include "flutter/fml/synchronization/waitable_event.h"
1112
#include "flutter/shell/platform/common/client_wrapper/include/flutter/plugin_registrar.h"
1213
#include "flutter/shell/platform/common/client_wrapper/testing/stub_flutter_api.h"
1314
#include "gtest/gtest.h"
@@ -41,13 +42,17 @@ class TestApi : public testing::StubFlutterApi {
4142
return last_texture_id_;
4243
}
4344

44-
bool TextureRegistrarUnregisterExternalTexture(int64_t texture_id) override {
45+
void TextureRegistrarUnregisterExternalTexture(
46+
int64_t texture_id,
47+
void (*callback)(void* user_data),
48+
void* user_data) override {
4549
auto it = textures_.find(texture_id);
4650
if (it != textures_.end()) {
4751
textures_.erase(it);
48-
return true;
4952
}
50-
return false;
53+
if (callback) {
54+
callback(user_data);
55+
}
5156
}
5257

5358
bool TextureRegistrarMarkTextureFrameAvailable(int64_t texture_id) override {
@@ -110,24 +115,27 @@ TEST(TextureRegistrarTest, RegisterUnregisterTexture) {
110115
EXPECT_TRUE(success);
111116
EXPECT_EQ(texture->mark_count, 3);
112117

113-
success = textures->UnregisterTexture(texture_id);
114-
EXPECT_TRUE(success);
118+
fml::AutoResetWaitableEvent unregister_latch;
119+
textures->UnregisterTexture(texture_id, [&]() { unregister_latch.Signal(); });
120+
unregister_latch.Wait();
115121

116122
texture = test_api->GetFakeTexture(texture_id);
117123
EXPECT_EQ(texture, nullptr);
118124
EXPECT_EQ(test_api->textures_size(), static_cast<size_t>(0));
119125
}
120126

121-
// Tests that unregistering a texture with an unknown id returns false.
127+
// Tests that the unregister callback gets also invoked when attempting to
128+
// unregister a texture with an unknown id.
122129
TEST(TextureRegistrarTest, UnregisterInvalidTexture) {
123130
auto dummy_registrar_handle =
124131
reinterpret_cast<FlutterDesktopPluginRegistrarRef>(1);
125132
PluginRegistrar registrar(dummy_registrar_handle);
126133

127134
TextureRegistrar* textures = registrar.texture_registrar();
128135

129-
bool success = textures->UnregisterTexture(42);
130-
EXPECT_FALSE(success);
136+
fml::AutoResetWaitableEvent latch;
137+
textures->UnregisterTexture(42, [&]() { latch.Signal(); });
138+
latch.Wait();
131139
}
132140

133141
// Tests that claiming a new frame being available for an unknown texture

shell/platform/common/public/flutter_texture_registrar.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,15 @@ FLUTTER_EXPORT int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
162162
FlutterDesktopTextureRegistrarRef texture_registrar,
163163
const FlutterDesktopTextureInfo* info);
164164

165-
// Unregisters an existing texture from the Flutter engine for a |texture_id|.
166-
// Returns true on success or false if the specified texture doesn't exist.
165+
// Asynchronously unregisters the texture identified by |texture_id| from the
166+
// Flutter engine.
167+
// An optional |callback| gets invoked upon completion.
167168
// This function can be called from any thread.
168-
// However, textures must not be unregistered while they're in use.
169-
FLUTTER_EXPORT bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
169+
FLUTTER_EXPORT void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
170170
FlutterDesktopTextureRegistrarRef texture_registrar,
171-
int64_t texture_id);
171+
int64_t texture_id,
172+
void (*callback)(void* user_data),
173+
void* user_data);
172174

173175
// Marks that a new texture frame is available for a given |texture_id|.
174176
// Returns true on success or false if the specified texture doesn't exist.

shell/platform/glfw/flutter_glfw.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -728,10 +728,9 @@ static void SetUpLocales(FlutterDesktopEngineState* state) {
728728
// Convert the locale list to the locale pointer list that must be provided.
729729
std::vector<const FlutterLocale*> flutter_locale_list;
730730
flutter_locale_list.reserve(flutter_locales.size());
731-
std::transform(
732-
flutter_locales.begin(), flutter_locales.end(),
733-
std::back_inserter(flutter_locale_list),
734-
[](const auto& arg) -> const auto* { return &arg; });
731+
std::transform(flutter_locales.begin(), flutter_locales.end(),
732+
std::back_inserter(flutter_locale_list),
733+
[](const auto& arg) -> const auto* { return &arg; });
735734
FlutterEngineResult result = FlutterEngineUpdateLocales(
736735
state->flutter_engine, flutter_locale_list.data(),
737736
flutter_locale_list.size());
@@ -1114,11 +1113,12 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
11141113
return -1;
11151114
}
11161115

1117-
bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
1116+
void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
11181117
FlutterDesktopTextureRegistrarRef texture_registrar,
1119-
int64_t texture_id) {
1118+
int64_t texture_id,
1119+
void (*callback)(void* user_data),
1120+
void* user_data) {
11201121
std::cerr << "GLFW Texture support is not implemented yet." << std::endl;
1121-
return false;
11221122
}
11231123

11241124
bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

shell/platform/windows/angle_surface_manager.cc

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,6 @@ EGLSurface AngleSurfaceManager::CreateSurfaceFromHandle(
301301
}
302302

303303
bool AngleSurfaceManager::GetDevice(ID3D11Device** device) {
304-
using Microsoft::WRL::ComPtr;
305-
306304
if (!resolved_device_) {
307305
PFNEGLQUERYDISPLAYATTRIBEXTPROC egl_query_display_attrib_EXT =
308306
reinterpret_cast<PFNEGLQUERYDISPLAYATTRIBEXTPROC>(

shell/platform/windows/flutter_windows.cc

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -302,11 +302,18 @@ int64_t FlutterDesktopTextureRegistrarRegisterExternalTexture(
302302
->RegisterTexture(texture_info);
303303
}
304304

305-
bool FlutterDesktopTextureRegistrarUnregisterExternalTexture(
305+
void FlutterDesktopTextureRegistrarUnregisterExternalTexture(
306306
FlutterDesktopTextureRegistrarRef texture_registrar,
307-
int64_t texture_id) {
308-
return TextureRegistrarFromHandle(texture_registrar)
309-
->UnregisterTexture(texture_id);
307+
int64_t texture_id,
308+
void (*callback)(void* user_data),
309+
void* user_data) {
310+
auto registrar = TextureRegistrarFromHandle(texture_registrar);
311+
if (callback) {
312+
registrar->UnregisterTexture(
313+
texture_id, [callback, user_data]() { callback(user_data); });
314+
return;
315+
}
316+
registrar->UnregisterTexture(texture_id);
310317
}
311318

312319
bool FlutterDesktopTextureRegistrarMarkExternalTextureFrameAvailable(

0 commit comments

Comments
 (0)