From 4ff54997b6f3b8f9783c14092f09730548e59a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Mon, 11 Sep 2023 23:30:22 +0800 Subject: [PATCH] cursor-renderer/native: Create all view objects before realizing Realizing a cursor will assume view related state objects are valid so they can mark them as dirty. This assumption broke when there were a scale changed that happened with multiple CRTCs, as we'd create view object by view object as we realized the texture. Realizing the texture would trigger a signal that had the handler assuming the validity of all view objects, but if we only had gotten to the first, the second view would not be there yet, thus we'd be doing a NULL pointer dereference. Creating the view objects first, then handling the updating avoids this problem by making the already done assumption valid on hotplugs. The test case added tests exactly this series of events, and uses a virtual monitor as a cheap trick to make the KMS CRTC based view the first one, and an arbitrary view the second that previously had its view object initialized too late. Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/3012 Part-of: --- src/backends/native/meta-crtc-virtual.h | 1 + .../native/meta-cursor-renderer-native.c | 5 +- .../monitor-configs/kms-cursor-scale.xml | 41 +++++++++++ src/tests/native-kms-cursor-hotplug.c | 73 +++++++++++++++++++ 4 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 src/tests/monitor-configs/kms-cursor-scale.xml diff --git a/src/backends/native/meta-crtc-virtual.h b/src/backends/native/meta-crtc-virtual.h index 039ee8b50..b62f61b68 100644 --- a/src/backends/native/meta-crtc-virtual.h +++ b/src/backends/native/meta-crtc-virtual.h @@ -20,6 +20,7 @@ #include "backends/native/meta-crtc-native.h" #define META_TYPE_CRTC_VIRTUAL (meta_crtc_virtual_get_type ()) +META_EXPORT_TEST G_DECLARE_FINAL_TYPE (MetaCrtcVirtual, meta_crtc_virtual, META, CRTC_VIRTUAL, MetaCrtcNative) diff --git a/src/backends/native/meta-cursor-renderer-native.c b/src/backends/native/meta-cursor-renderer-native.c index c7c2ce2a7..a98dea7e7 100644 --- a/src/backends/native/meta-cursor-renderer-native.c +++ b/src/backends/native/meta-cursor-renderer-native.c @@ -315,6 +315,8 @@ meta_cursor_renderer_native_update_cursor (MetaCursorRenderer *cursor_renderer, cursor_changed = priv->current_cursor != cursor_sprite; views = meta_renderer_get_views (renderer); + g_list_foreach (views, (GFunc) ensure_cursor_stage_view, NULL); + for (l = views; l; l = l->next) { MetaStageView *view = l->data; @@ -324,7 +326,8 @@ meta_cursor_renderer_native_update_cursor (MetaCursorRenderer *cursor_renderer, CursorStageView *cursor_stage_view = NULL; gboolean has_hw_cursor = FALSE; - cursor_stage_view = ensure_cursor_stage_view (view); + cursor_stage_view = get_cursor_stage_view (view); + g_assert (cursor_stage_view); if (!META_IS_CRTC_KMS (crtc) || !meta_crtc_native_is_hw_cursor_supported (crtc_native)) diff --git a/src/tests/monitor-configs/kms-cursor-scale.xml b/src/tests/monitor-configs/kms-cursor-scale.xml new file mode 100644 index 000000000..ea1c9fda0 --- /dev/null +++ b/src/tests/monitor-configs/kms-cursor-scale.xml @@ -0,0 +1,41 @@ + + + + 0 + 0 + 2 + yes + + + Virtual-1 + unknown + unknown + unknown + + + 1024 + 768 + 60.003841 + + + + + 512 + 0 + no + + + Meta-0 + MetaTestVendor + MetaVirtualMonitor + 0x1234 + + + 100 + 100 + 60 + + + + + diff --git a/src/tests/native-kms-cursor-hotplug.c b/src/tests/native-kms-cursor-hotplug.c index 2578ad4cd..0d7f56281 100644 --- a/src/tests/native-kms-cursor-hotplug.c +++ b/src/tests/native-kms-cursor-hotplug.c @@ -20,6 +20,10 @@ #include "backends/meta-monitor-config-manager.h" #include "backends/meta-virtual-monitor.h" +#include "backends/native/meta-backend-native.h" +#include "backends/native/meta-crtc-kms.h" +#include "backends/native/meta-crtc-virtual.h" +#include "backends/native/meta-udev.h" #include "core/window-private.h" #include "meta-test/meta-context-test.h" #include "meta/meta-backend.h" @@ -35,6 +39,12 @@ static MetaContext *test_context; +static void +set_true_cb (gboolean *done) +{ + *done = TRUE; +} + static void meta_test_cursor_hotplug (void) { @@ -139,11 +149,74 @@ meta_test_cursor_hotplug (void) meta_wait_for_paint (test_context); } +static void +meta_test_hotplug_multi_view_invalidation (void) +{ + MetaBackend *backend = meta_context_get_backend (test_context); + MetaMonitorManager *monitor_manager = + meta_backend_get_monitor_manager (backend); + MetaRenderer *renderer = meta_backend_get_renderer (backend); + MetaCursorRenderer *cursor_renderer = meta_backend_get_cursor_renderer (backend); + ClutterSeat *seat; + g_autoptr (MetaVirtualMonitorInfo) monitor_info = NULL; + MetaVirtualMonitor *virtual_monitor; + g_autoptr (ClutterVirtualInputDevice) virtual_pointer = NULL; + MetaCursorSprite *cursor_sprite; + gboolean texture_changed; + gulong texture_changed_handler_id; + g_autoptr (GError) error = NULL; + GList *views; + + seat = meta_backend_get_default_seat (backend); + virtual_pointer = clutter_seat_create_virtual_device (seat, + CLUTTER_POINTER_DEVICE); + + monitor_info = meta_virtual_monitor_info_new (100, 100, 60.0, + "MetaTestVendor", + "MetaVirtualMonitor", + "0x1234"); + virtual_monitor = meta_monitor_manager_create_virtual_monitor (monitor_manager, + monitor_info, + &error); + g_assert_no_error (error); + + meta_monitor_manager_reload (monitor_manager); + views = meta_renderer_get_views (renderer); + g_assert_true (META_IS_CRTC_KMS (meta_renderer_view_get_crtc (views->data))); + g_assert_true (META_IS_CRTC_VIRTUAL (meta_renderer_view_get_crtc (views->next->data))); + + meta_wait_for_paint (test_context); + + cursor_sprite = meta_cursor_renderer_get_cursor (cursor_renderer); + g_assert_nonnull (cursor_sprite); + texture_changed_handler_id = + g_signal_connect_swapped (cursor_sprite, "texture-changed", + G_CALLBACK (set_true_cb), &texture_changed); + + /* Trigger a cursor scale change, that causes invalidation on a non-first + * KMS CRTC based cursor renderer view auxiliary object. + */ + texture_changed = FALSE; + meta_set_custom_monitor_config_full (backend, "kms-cursor-scale.xml", + META_MONITORS_CONFIG_FLAG_NONE); + meta_monitor_manager_reload (monitor_manager); + views = meta_renderer_get_views (renderer); + g_assert_true (META_IS_CRTC_KMS (meta_renderer_view_get_crtc (views->data))); + g_assert_true (META_IS_CRTC_VIRTUAL (meta_renderer_view_get_crtc (views->next->data))); + g_assert_true (texture_changed); + + g_signal_handler_disconnect (cursor_sprite, texture_changed_handler_id); + g_clear_object (&virtual_monitor); + meta_wait_for_paint (test_context); +} + static void init_tests (void) { g_test_add_func ("/wayland/cursor-hotplug", meta_test_cursor_hotplug); + g_test_add_func ("/hotplug/multi-view-invalidation", + meta_test_hotplug_multi_view_invalidation); } int