From b39232e610dafd0671edab5d22662ea14973c41d Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 26 Mar 2025 16:24:36 +1300 Subject: [PATCH] Move tracking of renderables from FlRenderer to FlEngine (#165848) Motivated by work on refactoring FlRenderer to FlCompositor. --- .../flutter/shell/platform/linux/fl_engine.cc | 34 +++++++++++++++ .../shell/platform/linux/fl_engine_private.h | 24 +++++++++++ .../shell/platform/linux/fl_engine_test.cc | 17 +++++--- .../shell/platform/linux/fl_renderer.cc | 42 +++---------------- .../shell/platform/linux/fl_renderer.h | 21 ---------- .../shell/platform/linux/fl_renderer_test.cc | 36 ++++++---------- .../flutter/shell/platform/linux/fl_view.cc | 15 ++----- 7 files changed, 91 insertions(+), 98 deletions(-) diff --git a/engine/src/flutter/shell/platform/linux/fl_engine.cc b/engine/src/flutter/shell/platform/linux/fl_engine.cc index d16d6a9bd8..66e0fd29a8 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine.cc +++ b/engine/src/flutter/shell/platform/linux/fl_engine.cc @@ -8,6 +8,7 @@ #include +#include "flutter/common/constants.h" #include "flutter/shell/platform/common/engine_switches.h" #include "flutter/shell/platform/embedder/embedder.h" #include "flutter/shell/platform/linux/fl_binary_messenger_private.h" @@ -93,6 +94,9 @@ struct _FlEngine { // Next ID to use for a view. FlutterViewId next_view_id; + // Objects rendering the views. + GHashTable* renderables_by_view_id; + // Function to call when a platform message is received. FlEnginePlatformMessageHandler platform_message_handler; gpointer platform_message_handler_data; @@ -489,6 +493,7 @@ static void fl_engine_dispose(GObject* object) { g_clear_object(&self->keyboard_handler); g_clear_object(&self->mouse_cursor_handler); g_clear_object(&self->task_runner); + g_clear_pointer(&self->renderables_by_view_id, g_hash_table_unref); if (self->platform_message_handler_destroy_notify) { self->platform_message_handler_destroy_notify( @@ -534,6 +539,12 @@ static void fl_engine_init(FlEngine* self) { // Implicit view is 0, so start at 1. self->next_view_id = 1; + self->renderables_by_view_id = g_hash_table_new_full( + g_direct_hash, g_direct_equal, nullptr, [](gpointer value) { + GWeakRef* ref = static_cast(value); + g_weak_ref_clear(ref); + free(ref); + }); self->texture_registrar = fl_texture_registrar_new(self); } @@ -734,7 +745,15 @@ void fl_engine_notify_display_update(FlEngine* self, } } +void fl_engine_set_implicit_view(FlEngine* self, FlRenderable* renderable) { + GWeakRef* ref = g_new(GWeakRef, 1); + g_weak_ref_init(ref, G_OBJECT(renderable)); + g_hash_table_insert(self->renderables_by_view_id, + GINT_TO_POINTER(flutter::kFlutterImplicitViewId), ref); +} + FlutterViewId fl_engine_add_view(FlEngine* self, + FlRenderable* renderable, size_t width, size_t height, double pixel_ratio, @@ -748,6 +767,11 @@ FlutterViewId fl_engine_add_view(FlEngine* self, FlutterViewId view_id = self->next_view_id; self->next_view_id++; + GWeakRef* ref = g_new(GWeakRef, 1); + g_weak_ref_init(ref, G_OBJECT(renderable)); + g_hash_table_insert(self->renderables_by_view_id, GINT_TO_POINTER(view_id), + ref); + // We don't know which display this view will open on, so set to zero and this // will be updated in a following FlutterWindowMetricsEvent FlutterEngineDisplayId display_id = 0; @@ -784,6 +808,14 @@ gboolean fl_engine_add_view_finish(FlEngine* self, return g_task_propagate_boolean(G_TASK(result), error); } +FlRenderable* fl_engine_get_renderable(FlEngine* self, FlutterViewId view_id) { + g_return_val_if_fail(FL_IS_ENGINE(self), nullptr); + + GWeakRef* ref = static_cast(g_hash_table_lookup( + self->renderables_by_view_id, GINT_TO_POINTER(view_id))); + return FL_RENDERABLE(g_weak_ref_get(ref)); +} + void fl_engine_remove_view(FlEngine* self, FlutterViewId view_id, GCancellable* cancellable, @@ -791,6 +823,8 @@ void fl_engine_remove_view(FlEngine* self, gpointer user_data) { g_return_if_fail(FL_IS_ENGINE(self)); + g_hash_table_remove(self->renderables_by_view_id, GINT_TO_POINTER(view_id)); + g_autoptr(GTask) task = g_task_new(self, cancellable, callback, user_data); FlutterRemoveViewInfo info; diff --git a/engine/src/flutter/shell/platform/linux/fl_engine_private.h b/engine/src/flutter/shell/platform/linux/fl_engine_private.h index d156493ab5..bceabf0b0e 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine_private.h +++ b/engine/src/flutter/shell/platform/linux/fl_engine_private.h @@ -11,6 +11,7 @@ #include "flutter/shell/platform/linux/fl_display_monitor.h" #include "flutter/shell/platform/linux/fl_keyboard_manager.h" #include "flutter/shell/platform/linux/fl_mouse_cursor_handler.h" +#include "flutter/shell/platform/linux/fl_renderable.h" #include "flutter/shell/platform/linux/fl_renderer.h" #include "flutter/shell/platform/linux/fl_task_runner.h" #include "flutter/shell/platform/linux/fl_text_input_handler.h" @@ -127,9 +128,19 @@ void fl_engine_notify_display_update(FlEngine* engine, const FlutterEngineDisplay* displays, size_t displays_length); +/** + * fl_engine_set_implicit_view: + * @engine: an #FlEngine. + * @renderable: the object that will render the implicit view. + * + * Sets the object to render the implicit view. + */ +void fl_engine_set_implicit_view(FlEngine* engine, FlRenderable* renderable); + /** * fl_engine_add_view: * @engine: an #FlEngine. + * @renderable: the object that will render this view. * @width: width of view in pixels. * @height: height of view in pixels. * @pixel_ratio: scale factor for view. @@ -144,6 +155,7 @@ void fl_engine_notify_display_update(FlEngine* engine, * Returns: the ID for the view. */ FlutterViewId fl_engine_add_view(FlEngine* engine, + FlRenderable* renderable, size_t width, size_t height, double pixel_ratio, @@ -166,6 +178,18 @@ gboolean fl_engine_add_view_finish(FlEngine* engine, GAsyncResult* result, GError** error); +/** + * fl_engine_get_renderable: + * @engine: an #FlEngine. + * @view_id: ID to check. + * + * Gets the renderable associated with the give view ID. + * + * Returns: (transfer full): a reference to an #FlRenderable or %NULL if none + * for this ID. + */ +FlRenderable* fl_engine_get_renderable(FlEngine* engine, FlutterViewId view_id); + /** * fl_engine_remove_view: * @engine: an #FlEngine. diff --git a/engine/src/flutter/shell/platform/linux/fl_engine_test.cc b/engine/src/flutter/shell/platform/linux/fl_engine_test.cc index fd5f20e7ca..25c67b3c3f 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_engine_test.cc @@ -10,6 +10,7 @@ #include "flutter/shell/platform/linux/public/flutter_linux/fl_engine.h" #include "flutter/shell/platform/linux/public/flutter_linux/fl_json_message_codec.h" #include "flutter/shell/platform/linux/public/flutter_linux/fl_string_codec.h" +#include "flutter/shell/platform/linux/testing/mock_renderer.h" // MOCK_ENGINE_PROC is leaky by design // NOLINTBEGIN(clang-analyzer-core.StackAddressEscape) @@ -651,8 +652,10 @@ TEST(FlEngineTest, AddView) { return kSuccess; })); + g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new(); FlutterViewId view_id = - fl_engine_add_view(engine, 123, 456, 2.0, nullptr, add_view_cb, loop); + fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0, + nullptr, add_view_cb, loop); EXPECT_GT(view_id, 0); EXPECT_TRUE(called); @@ -688,8 +691,10 @@ TEST(FlEngineTest, AddViewError) { return kSuccess; })); - FlutterViewId view_id = fl_engine_add_view(engine, 123, 456, 2.0, nullptr, - add_view_error_cb, loop); + g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new(); + FlutterViewId view_id = + fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0, + nullptr, add_view_error_cb, loop); EXPECT_GT(view_id, 0); // Blocks here until add_view_error_cb is called. @@ -718,8 +723,10 @@ TEST(FlEngineTest, AddViewEngineError) { return kInvalidArguments; })); - FlutterViewId view_id = fl_engine_add_view(engine, 123, 456, 2.0, nullptr, - add_view_engine_error_cb, loop); + g_autoptr(FlMockRenderable) renderable = fl_mock_renderable_new(); + FlutterViewId view_id = + fl_engine_add_view(engine, FL_RENDERABLE(renderable), 123, 456, 2.0, + nullptr, add_view_engine_error_cb, loop); EXPECT_GT(view_id, 0); // Blocks here until remove_view_engine_error_cb is called. diff --git a/engine/src/flutter/shell/platform/linux/fl_renderer.cc b/engine/src/flutter/shell/platform/linux/fl_renderer.cc index 518b0b2904..b86dc066bf 100644 --- a/engine/src/flutter/shell/platform/linux/fl_renderer.cc +++ b/engine/src/flutter/shell/platform/linux/fl_renderer.cc @@ -51,9 +51,6 @@ typedef struct { // The format used to create textures. GLint general_format; - // Views being rendered. - GHashTable* views; - // target dimension for resizing int target_width; int target_height; @@ -85,12 +82,6 @@ typedef struct { G_DEFINE_TYPE_WITH_PRIVATE(FlRenderer, fl_renderer, G_TYPE_OBJECT) -static void free_weak_ref(gpointer value) { - GWeakRef* ref = static_cast(value); - g_weak_ref_clear(ref); - free(ref); -} - // Check if running on an NVIDIA driver. static gboolean is_nvidia() { const gchar* vendor = reinterpret_cast(glGetString(GL_VENDOR)); @@ -349,10 +340,12 @@ static gboolean present_layers(FlRenderer* self, } } - GWeakRef* ref = static_cast( - g_hash_table_lookup(priv->views, GINT_TO_POINTER(view_id))); + g_autoptr(FlEngine) engine = FL_ENGINE(g_weak_ref_get(&priv->engine)); + if (engine == nullptr) { + return TRUE; + } g_autoptr(FlRenderable) renderable = - ref != nullptr ? FL_RENDERABLE(g_weak_ref_get(ref)) : nullptr; + fl_engine_get_renderable(engine, view_id); if (renderable == nullptr) { return TRUE; } @@ -462,7 +455,6 @@ static void fl_renderer_dispose(GObject* object) { fl_renderer_unblock_main_thread(self); g_weak_ref_clear(&priv->engine); - g_clear_pointer(&priv->views, g_hash_table_unref); g_clear_pointer(&priv->framebuffers_by_view_id, g_hash_table_unref); g_mutex_clear(&priv->present_mutex); g_cond_clear(&priv->present_condition); @@ -477,8 +469,6 @@ static void fl_renderer_class_init(FlRendererClass* klass) { static void fl_renderer_init(FlRenderer* self) { FlRendererPrivate* priv = reinterpret_cast( fl_renderer_get_instance_private(self)); - priv->views = g_hash_table_new_full(g_direct_hash, g_direct_equal, nullptr, - free_weak_ref); priv->framebuffers_by_view_id = g_hash_table_new_full( g_direct_hash, g_direct_equal, nullptr, reinterpret_cast(g_ptr_array_unref)); @@ -495,28 +485,6 @@ void fl_renderer_set_engine(FlRenderer* self, FlEngine* engine) { g_weak_ref_init(&priv->engine, engine); } -void fl_renderer_add_renderable(FlRenderer* self, - FlutterViewId view_id, - FlRenderable* renderable) { - FlRendererPrivate* priv = reinterpret_cast( - fl_renderer_get_instance_private(self)); - - g_return_if_fail(FL_IS_RENDERER(self)); - - GWeakRef* ref = g_new(GWeakRef, 1); - g_weak_ref_init(ref, G_OBJECT(renderable)); - g_hash_table_insert(priv->views, GINT_TO_POINTER(view_id), ref); -} - -void fl_renderer_remove_view(FlRenderer* self, FlutterViewId view_id) { - FlRendererPrivate* priv = reinterpret_cast( - fl_renderer_get_instance_private(self)); - - g_return_if_fail(FL_IS_RENDERER(self)); - - g_hash_table_remove(priv->views, GINT_TO_POINTER(view_id)); -} - void* fl_renderer_get_proc_address(FlRenderer* self, const char* name) { g_return_val_if_fail(FL_IS_RENDERER(self), NULL); diff --git a/engine/src/flutter/shell/platform/linux/fl_renderer.h b/engine/src/flutter/shell/platform/linux/fl_renderer.h index b4b5216c70..3d1a173ebd 100644 --- a/engine/src/flutter/shell/platform/linux/fl_renderer.h +++ b/engine/src/flutter/shell/platform/linux/fl_renderer.h @@ -66,27 +66,6 @@ struct _FlRendererClass { */ void fl_renderer_set_engine(FlRenderer* renderer, FlEngine* engine); -/** - * fl_renderer_add_renderable: - * @renderer: an #FlRenderer. - * @view_id: the ID of the view. - * @renderable: object that is to be rendered on. - * - * Add a view to render on. - */ -void fl_renderer_add_renderable(FlRenderer* renderer, - FlutterViewId view_id, - FlRenderable* renderable); - -/** - * fl_renderer_remove_view: - * @renderer: an #FlRenderer. - * @view_id: the ID of the view. - * - * Remove a view from the renderer. - */ -void fl_renderer_remove_view(FlRenderer* renderer, FlutterViewId view_id); - /** * fl_renderer_get_proc_address: * @renderer: an #FlRenderer. diff --git a/engine/src/flutter/shell/platform/linux/fl_renderer_test.cc b/engine/src/flutter/shell/platform/linux/fl_renderer_test.cc index ab300fad0d..07a9db6cc7 100644 --- a/engine/src/flutter/shell/platform/linux/fl_renderer_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_renderer_test.cc @@ -8,6 +8,7 @@ #include "flutter/common/constants.h" #include "flutter/fml/logging.h" #include "flutter/fml/synchronization/waitable_event.h" +#include "flutter/shell/platform/linux/fl_engine_private.h" #include "flutter/shell/platform/linux/fl_framebuffer.h" #include "flutter/shell/platform/linux/testing/mock_epoxy.h" #include "flutter/shell/platform/linux/testing/mock_renderer.h" @@ -30,9 +31,7 @@ TEST(FlRendererTest, BackgroundColor) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); FlutterBackingStoreConfig config = { .struct_size = sizeof(FlutterBackingStoreConfig), @@ -84,9 +83,7 @@ TEST(FlRendererTest, RestoresGLState) { g_autoptr(FlFramebuffer) framebuffer = fl_framebuffer_new(GL_RGB, kWidth, kHeight); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), kWidth, kHeight); FlutterBackingStore backing_store; @@ -151,9 +148,7 @@ TEST(FlRendererTest, BlitFramebuffer) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); FlutterBackingStoreConfig config = { .struct_size = sizeof(FlutterBackingStoreConfig), @@ -211,9 +206,7 @@ TEST(FlRendererTest, BlitFramebufferExtension) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); FlutterBackingStoreConfig config = { .struct_size = sizeof(FlutterBackingStoreConfig), @@ -264,9 +257,7 @@ TEST(FlRendererTest, NoBlitFramebuffer) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); FlutterBackingStoreConfig config = { .struct_size = sizeof(FlutterBackingStoreConfig), @@ -320,9 +311,7 @@ TEST(FlRendererTest, BlitFramebufferNvidia) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); FlutterBackingStoreConfig config = { .struct_size = sizeof(FlutterBackingStoreConfig), @@ -377,11 +366,10 @@ TEST(FlRendererTest, MultiView) { g_autoptr(FlMockRenderer) renderer = fl_mock_renderer_new(); fl_renderer_setup(FL_RENDERER(renderer)); fl_renderer_set_engine(FL_RENDERER(renderer), engine); - fl_renderer_add_renderable(FL_RENDERER(renderer), - flutter::kFlutterImplicitViewId, - FL_RENDERABLE(renderable)); - fl_renderer_add_renderable(FL_RENDERER(renderer), 1, - FL_RENDERABLE(secondary_renderable)); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(renderable)); + FlutterViewId view_id = + fl_engine_add_view(engine, FL_RENDERABLE(secondary_renderable), 1024, 768, + 1.0, nullptr, nullptr, nullptr); fl_renderer_wait_for_frame(FL_RENDERER(renderer), 1024, 1024); EXPECT_EQ(fl_mock_renderable_get_redraw_count(renderable), @@ -405,7 +393,7 @@ TEST(FlRendererTest, MultiView) { .backing_store = &backing_store, .size = {.width = 1024, .height = 1024}}; const FlutterLayer* layers[] = {&layer0}; - fl_renderer_present_layers(FL_RENDERER(renderer), 1, layers, 1); + fl_renderer_present_layers(FL_RENDERER(renderer), view_id, layers, 1); latch.Signal(); }).detach(); diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc index 539c58c94d..d548e4c9d1 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view.cc @@ -479,9 +479,6 @@ static void realize_cb(FlView* self) { g_signal_connect_swapped(toplevel_window, "delete-event", G_CALLBACK(window_delete_event_cb), self); - fl_renderer_add_renderable(FL_RENDERER(self->renderer), self->view_id, - FL_RENDERABLE(self)); - // Flutter engine will need to make the context current from raster thread // during initialization. fl_renderer_clear_current(FL_RENDERER(self->renderer)); @@ -558,9 +555,6 @@ static void fl_view_dispose(GObject* object) { self->cursor_changed_cb_id = 0; } - // Stop rendering. - fl_renderer_remove_view(FL_RENDERER(self->renderer), self->view_id); - // Release the view ID from the engine. fl_engine_remove_view(self->engine, self->view_id, nullptr, nullptr, nullptr); @@ -773,6 +767,8 @@ G_MODULE_EXPORT FlView* fl_view_new(FlDartProject* project) { g_signal_connect_swapped(self->gl_area, "unrealize", G_CALLBACK(unrealize_cb), self); + fl_engine_set_implicit_view(engine, FL_RENDERABLE(self)); + return self; } @@ -784,14 +780,11 @@ G_MODULE_EXPORT FlView* fl_view_new_for_engine(FlEngine* engine) { g_assert(FL_IS_RENDERER_GDK(renderer)); self->renderer = FL_RENDERER_GDK(g_object_ref(renderer)); - self->view_id = fl_engine_add_view(engine, 1, 1, 1.0, self->cancellable, - view_added_cb, self); + self->view_id = fl_engine_add_view(engine, FL_RENDERABLE(self), 1, 1, 1.0, + self->cancellable, view_added_cb, self); setup_engine(self); - fl_renderer_add_renderable(FL_RENDERER(self->renderer), self->view_id, - FL_RENDERABLE(self)); - g_signal_connect_swapped(self->gl_area, "realize", G_CALLBACK(secondary_realize_cb), self); return self;