From 5a53ac39962c546b0bbff72542900d0647067ced Mon Sep 17 00:00:00 2001 From: Robert Ancell Date: Wed, 5 Feb 2025 12:10:27 +1300 Subject: [PATCH] Fix warnings on startup about display monitor (#162653) Create the display monitor at initialization, not engine start. Handle case where the window is not visible and the monitor can't be determined. Don't leak the monitor when the engine is disposed. --- .../flutter/shell/platform/linux/fl_engine.cc | 40 ++++++++++++------- .../shell/platform/linux/fl_engine_test.cc | 13 ++++++ .../flutter/shell/platform/linux/fl_view.cc | 21 +++++++--- 3 files changed, 54 insertions(+), 20 deletions(-) diff --git a/engine/src/flutter/shell/platform/linux/fl_engine.cc b/engine/src/flutter/shell/platform/linux/fl_engine.cc index e871d29af7..2fd7a6237c 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine.cc +++ b/engine/src/flutter/shell/platform/linux/fl_engine.cc @@ -467,6 +467,7 @@ static void fl_engine_dispose(GObject* object) { fl_texture_registrar_shutdown(self->texture_registrar); g_clear_object(&self->project); + g_clear_object(&self->display_monitor); g_clear_object(&self->renderer); g_clear_object(&self->texture_registrar); g_clear_object(&self->binary_messenger); @@ -519,27 +520,46 @@ static void fl_engine_init(FlEngine* self) { g_warning("Failed get get engine function pointers"); } + self->display_monitor = + fl_display_monitor_new(self, gdk_display_get_default()); + self->task_runner = fl_task_runner_new(self); + // Implicit view is 0, so start at 1. self->next_view_id = 1; self->texture_registrar = fl_texture_registrar_new(self); } -FlEngine* fl_engine_new_with_renderer(FlDartProject* project, - FlRenderer* renderer) { +static FlEngine* fl_engine_new_full(FlDartProject* project, + FlRenderer* renderer, + FlBinaryMessenger* binary_messenger) { g_return_val_if_fail(FL_IS_DART_PROJECT(project), nullptr); g_return_val_if_fail(FL_IS_RENDERER(renderer), nullptr); FlEngine* self = FL_ENGINE(g_object_new(fl_engine_get_type(), nullptr)); self->project = FL_DART_PROJECT(g_object_ref(project)); self->renderer = FL_RENDERER(g_object_ref(renderer)); - self->binary_messenger = fl_binary_messenger_new(self); - + if (binary_messenger != nullptr) { + self->binary_messenger = + FL_BINARY_MESSENGER(g_object_ref(binary_messenger)); + } else { + self->binary_messenger = fl_binary_messenger_new(self); + } + self->keyboard_manager = fl_keyboard_manager_new(self); + self->mouse_cursor_handler = + fl_mouse_cursor_handler_new(self->binary_messenger); fl_renderer_set_engine(self->renderer, self); return self; } +FlEngine* fl_engine_new_with_renderer(FlDartProject* project, + FlRenderer* renderer) { + g_return_val_if_fail(FL_IS_DART_PROJECT(project), nullptr); + g_return_val_if_fail(FL_IS_RENDERER(renderer), nullptr); + return fl_engine_new_full(project, renderer, nullptr); +} + G_MODULE_EXPORT FlEngine* fl_engine_new(FlDartProject* project) { g_autoptr(FlRendererGdk) renderer = fl_renderer_gdk_new(); return fl_engine_new_with_renderer(project, FL_RENDERER(renderer)); @@ -548,10 +568,8 @@ G_MODULE_EXPORT FlEngine* fl_engine_new(FlDartProject* project) { FlEngine* fl_engine_new_with_binary_messenger( FlBinaryMessenger* binary_messenger) { g_autoptr(FlDartProject) project = fl_dart_project_new(); - FlEngine* self = fl_engine_new(project); - g_object_unref(self->binary_messenger); - self->binary_messenger = FL_BINARY_MESSENGER(g_object_ref(binary_messenger)); - return self; + g_autoptr(FlRendererGdk) renderer = fl_renderer_gdk_new(); + return fl_engine_new_full(project, FL_RENDERER(renderer), binary_messenger); } G_MODULE_EXPORT FlEngine* fl_engine_new_headless(FlDartProject* project) { @@ -572,8 +590,6 @@ FlDisplayMonitor* fl_engine_get_display_monitor(FlEngine* self) { gboolean fl_engine_start(FlEngine* self, GError** error) { g_return_val_if_fail(FL_IS_ENGINE(self), FALSE); - self->task_runner = fl_task_runner_new(self); - FlutterRendererConfig config = {}; config.type = kOpenGL; config.open_gl.struct_size = sizeof(FlutterOpenGLRendererConfig); @@ -671,8 +687,6 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { fl_settings_handler_start(self->settings_handler, settings); self->platform_handler = fl_platform_handler_new(self->binary_messenger); - self->mouse_cursor_handler = - fl_mouse_cursor_handler_new(self->binary_messenger); setup_keyboard(self); @@ -681,8 +695,6 @@ gboolean fl_engine_start(FlEngine* self, GError** error) { g_warning("Failed to enable accessibility features on Flutter engine"); } - self->display_monitor = - fl_display_monitor_new(self, gdk_display_get_default()); fl_display_monitor_start(self->display_monitor); return TRUE; 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 5f183b0945..cdf0a06b75 100644 --- a/engine/src/flutter/shell/platform/linux/fl_engine_test.cc +++ b/engine/src/flutter/shell/platform/linux/fl_engine_test.cc @@ -946,4 +946,17 @@ TEST(FlEngineTest, SendKeyEventError) { EXPECT_TRUE(called); } +TEST(FlEngineTest, ChildObjects) { + g_autoptr(FlDartProject) project = fl_dart_project_new(); + g_autoptr(FlEngine) engine = fl_engine_new(project); + + // Check objects exist before engine started. + EXPECT_NE(fl_engine_get_binary_messenger(engine), nullptr); + EXPECT_NE(fl_engine_get_renderer(engine), nullptr); + EXPECT_NE(fl_engine_get_display_monitor(engine), nullptr); + EXPECT_NE(fl_engine_get_task_runner(engine), nullptr); + EXPECT_NE(fl_engine_get_keyboard_manager(engine), nullptr); + EXPECT_NE(fl_engine_get_mouse_cursor_handler(engine), nullptr); +} + // NOLINTEND(clang-analyzer-core.StackAddressEscape) diff --git a/engine/src/flutter/shell/platform/linux/fl_view.cc b/engine/src/flutter/shell/platform/linux/fl_view.cc index ef0ede9046..425250184e 100644 --- a/engine/src/flutter/shell/platform/linux/fl_view.cc +++ b/engine/src/flutter/shell/platform/linux/fl_view.cc @@ -211,13 +211,22 @@ static void handle_geometry_changed(FlView* self) { GdkWindow* window = gtk_widget_get_window(gtk_widget_get_toplevel(GTK_WIDGET(self))); - GdkMonitor* monitor = gdk_display_get_monitor_at_window( - gtk_widget_get_display(GTK_WIDGET(self)), window); + // NOTE(robert-ancell) If we haven't got a window we default to display 0. + // This is probably indicating a problem with this code in that we + // shouldn't be generating anything until the window is created. + // Another event with the correct display ID is generated soon after. + // I haven't changed this code in case there are side-effects but we + // probably shouldn't call handle_geometry_changed after the view is + // added but only when the window is realized. + FlutterEngineDisplayId display_id = 0; + if (window != nullptr) { + GdkMonitor* monitor = gdk_display_get_monitor_at_window( + gtk_widget_get_display(GTK_WIDGET(self)), window); + display_id = fl_display_monitor_get_display_id( + fl_engine_get_display_monitor(self->engine), monitor); + } fl_engine_send_window_metrics_event( - self->engine, - fl_display_monitor_get_display_id( - fl_engine_get_display_monitor(self->engine), monitor), - self->view_id, allocation.width * scale_factor, + self->engine, display_id, self->view_id, allocation.width * scale_factor, allocation.height * scale_factor, scale_factor); // Make sure the view has been realized and its size has been allocated before