From 740b8e8cce9b7523d4093b7692b079425eb08596 Mon Sep 17 00:00:00 2001 From: Carlos Garnacho Date: Wed, 7 Sep 2022 14:26:54 +0200 Subject: [PATCH] x11: Replace MetaWindow sync request handling with MetaSyncCounter Put the helper to use, in order to lift MetaWindow itself from this accounting. As a bonus, the data itself now moved to the MetaWindowX11 private struct, since this may only happen with X11 windows (or its Xwayland subclass). Part-of: --- src/compositor/meta-window-actor-x11.c | 7 +- src/core/window-private.h | 15 -- src/core/window.c | 11 +- src/x11/window-props.c | 22 +- src/x11/window-x11-private.h | 3 + src/x11/window-x11.c | 285 ++++--------------------- src/x11/window-x11.h | 7 + 7 files changed, 74 insertions(+), 276 deletions(-) diff --git a/src/compositor/meta-window-actor-x11.c b/src/compositor/meta-window-actor-x11.c index 4f01a2c9b..6791bac5f 100644 --- a/src/compositor/meta-window-actor-x11.c +++ b/src/compositor/meta-window-actor-x11.c @@ -501,6 +501,8 @@ meta_window_actor_x11_queue_frame_drawn (MetaWindowActor *actor, MetaWindowActorX11 *actor_x11 = META_WINDOW_ACTOR_X11 (actor); MetaWindow *window = meta_window_actor_get_meta_window (actor); + MetaSyncCounter *sync_counter = + meta_window_x11_get_sync_counter (window); FrameData *frame; if (meta_window_actor_is_destroyed (actor)) @@ -508,7 +510,7 @@ meta_window_actor_x11_queue_frame_drawn (MetaWindowActor *actor, frame = g_new0 (FrameData, 1); frame->frame_counter = -1; - frame->sync_request_serial = window->sync_request_serial; + frame->sync_request_serial = sync_counter->sync_request_serial; actor_x11->frames = g_list_prepend (actor_x11->frames, frame); @@ -1614,6 +1616,7 @@ meta_window_actor_x11_constructed (GObject *object) MetaWindowActorX11 *actor_x11 = META_WINDOW_ACTOR_X11 (object); MetaWindowActor *actor = META_WINDOW_ACTOR (actor_x11); MetaWindow *window = meta_window_actor_get_meta_window (actor); + MetaSyncCounter *sync_counter = meta_window_x11_get_sync_counter (window); /* * Start off with an empty shape region to maintain the invariant that it's @@ -1626,7 +1629,7 @@ meta_window_actor_x11_constructed (GObject *object) /* If a window doesn't start off with updates frozen, we should * we should send a _NET_WM_FRAME_DRAWN immediately after the first drawn. */ - if (window->extended_sync_request_counter && + if (sync_counter->extended_sync_request_counter && !meta_window_updates_are_frozen (window)) meta_window_actor_queue_frame_drawn (actor, FALSE); } diff --git a/src/core/window-private.h b/src/core/window-private.h index d7c33f821..e5d5a280f 100644 --- a/src/core/window-private.h +++ b/src/core/window-private.h @@ -257,14 +257,6 @@ struct _MetaWindow /* Note: can be NULL */ GSList *struts; - /* XSync update counter */ - XSyncCounter sync_request_counter; - gint64 sync_request_serial; - gint64 sync_request_wait_serial; - guint sync_request_timeout_id; - /* alarm monitoring client's _NET_WM_SYNC_REQUEST_COUNTER */ - XSyncAlarm sync_request_alarm; - /* Number of UnmapNotify that are caused by us, if * we get UnmapNotify with none pending then the client * is withdrawing the window. @@ -545,9 +537,6 @@ struct _MetaWindow /* if TRUE we have a grab on the focus click buttons */ guint have_focus_click_grab : 1; - /* if TRUE, application is buggy and SYNC resizing is turned off */ - guint disable_sync : 1; - /* if TRUE, window is attached to its parent */ guint attached : 1; @@ -560,10 +549,6 @@ struct _MetaWindow /* Whether the window is alive */ guint is_alive : 1; - /* if TRUE, the we have the new form of sync request counter which - * also handles application frames */ - guint extended_sync_request_counter : 1; - guint in_workspace_change : 1; }; diff --git a/src/core/window.c b/src/core/window.c index f69d55c2a..883f83691 100644 --- a/src/core/window.c +++ b/src/core/window.c @@ -1020,11 +1020,6 @@ meta_window_constructed (GObject *object) window->workspace = NULL; - window->sync_request_counter = None; - window->sync_request_serial = 0; - window->sync_request_timeout_id = 0; - window->sync_request_alarm = None; - meta_window_update_sandboxed_app_id (window); meta_window_update_desc (window); @@ -1085,7 +1080,6 @@ meta_window_constructed (GObject *object) window->calc_placement = FALSE; window->shaken_loose = FALSE; window->have_focus_click_grab = FALSE; - window->disable_sync = FALSE; window->unmaps_pending = 0; window->reparents_pending = 0; @@ -1521,8 +1515,6 @@ meta_window_unmanage (MetaWindow *window, invalidate_work_areas (window); } - g_clear_handle_id (&window->sync_request_timeout_id, g_source_remove); - if (window->display->grab_window == window) meta_display_end_grab_op (window->display, timestamp); @@ -6129,7 +6121,8 @@ update_resize (MetaWindow *window, * resize the window when the window responds, or when we time * the response out. */ - if (window->sync_request_timeout_id != 0) + if (window->client_type == META_WINDOW_CLIENT_TYPE_X11 && + meta_window_x11_is_awaiting_sync_response (window)) return; meta_window_get_frame_rect (window, &old_rect); diff --git a/src/x11/window-props.c b/src/x11/window-props.c index aa4fab99c..a0a30b39a 100644 --- a/src/x11/window-props.c +++ b/src/x11/window-props.c @@ -1052,31 +1052,29 @@ reload_update_counter (MetaWindow *window, { if (value->type != META_PROP_VALUE_INVALID) { - meta_window_x11_destroy_sync_request_alarm (window); - window->sync_request_counter = None; + MetaSyncCounter *sync_counter; + + sync_counter = meta_window_x11_get_sync_counter (window); if (value->v.xcounter_list.n_counters == 0) { meta_warning ("_NET_WM_SYNC_REQUEST_COUNTER is empty"); + meta_sync_counter_set_counter (sync_counter, None, FALSE); return; } if (value->v.xcounter_list.n_counters == 1) { - window->sync_request_counter = value->v.xcounter_list.counters[0]; - window->extended_sync_request_counter = FALSE; + meta_sync_counter_set_counter (sync_counter, + value->v.xcounter_list.counters[0], + FALSE); } else { - window->sync_request_counter = value->v.xcounter_list.counters[1]; - window->extended_sync_request_counter = TRUE; + meta_sync_counter_set_counter (sync_counter, + value->v.xcounter_list.counters[1], + TRUE); } - meta_verbose ("Window has _NET_WM_SYNC_REQUEST_COUNTER 0x%lx (extended=%s)", - window->sync_request_counter, - window->extended_sync_request_counter ? "true" : "false"); - - if (window->extended_sync_request_counter) - meta_window_x11_create_sync_request_alarm (window); } } diff --git a/src/x11/window-x11-private.h b/src/x11/window-x11-private.h index 20cb44528..255c73f1b 100644 --- a/src/x11/window-x11-private.h +++ b/src/x11/window-x11-private.h @@ -25,6 +25,7 @@ #include "core/window-private.h" #include "x11/iconcache.h" +#include "x11/meta-sync-counter.h" #include "x11/window-x11.h" G_BEGIN_DECLS @@ -83,6 +84,8 @@ struct _MetaWindowX11Private /* Bypass compositor hints */ MetaBypassCompositorHint bypass_compositor; + + MetaSyncCounter sync_counter; }; MetaWindowX11Private * meta_window_x11_get_private (MetaWindowX11 *window_x11); diff --git a/src/x11/window-x11.c b/src/x11/window-x11.c index 1566e1e4a..0538beb13 100644 --- a/src/x11/window-x11.c +++ b/src/x11/window-x11.c @@ -52,6 +52,7 @@ #include "wayland/meta-window-xwayland.h" #endif +#include "x11/meta-sync-counter.h" #include "x11/meta-x11-display-private.h" #include "x11/session.h" #include "x11/window-props.h" @@ -540,6 +541,7 @@ meta_window_x11_manage (MetaWindow *window) MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); + meta_sync_counter_init (&priv->sync_counter, window, window->xwindow); meta_icon_cache_init (&priv->icon_cache); meta_x11_display_register_x_window (display->x11_display, @@ -706,6 +708,8 @@ meta_window_x11_unmanage (MetaWindow *window) meta_window_destroy_frame (window); } + + meta_sync_counter_clear (&priv->sync_counter); } void @@ -1088,8 +1092,7 @@ meta_window_x11_grab_op_began (MetaWindow *window, if (meta_grab_op_is_resizing (op)) { - if (window->sync_request_counter != None) - meta_window_x11_create_sync_request_alarm (window); + meta_window_x11_create_sync_request_alarm (window); if (window->size_hints.width_inc > 2 || window->size_hints.height_inc > 2) { @@ -1227,88 +1230,6 @@ update_gtk_edge_constraints (MetaWindow *window) meta_x11_error_trap_pop (x11_display); } -static gboolean -sync_request_timeout (gpointer data) -{ - MetaWindow *window = data; - - window->sync_request_timeout_id = 0; - - /* We have now waited for more than a second for the - * application to respond to the sync request - */ - window->disable_sync = TRUE; - - /* Reset the wait serial, so we don't continue freezing - * window updates - */ - window->sync_request_wait_serial = 0; - meta_compositor_sync_updates_frozen (window->display->compositor, window); - - if (window == window->display->grab_window && - meta_grab_op_is_resizing (window->display->grab_op)) - { - meta_window_update_resize (window, - window->display->grab_last_edge_resistance_flags, - window->display->grab_latest_motion_x, - window->display->grab_latest_motion_y); - } - - return FALSE; -} - -static void -send_sync_request (MetaWindow *window) -{ - MetaX11Display *x11_display = window->display->x11_display; - XClientMessageEvent ev; - gint64 wait_serial; - - /* For the old style of _NET_WM_SYNC_REQUEST_COUNTER, we just have to - * increase the value, but for the new "extended" style we need to - * pick an even (unfrozen) value sufficiently ahead of the last serial - * that we received from the client; the same code still works - * for the old style. The increment of 240 is specified by the EWMH - * and is (1 second) * (60fps) * (an increment of 4 per frame). - */ - wait_serial = window->sync_request_serial + 240; - - window->sync_request_wait_serial = wait_serial; - - ev.type = ClientMessage; - ev.window = window->xwindow; - ev.message_type = x11_display->atom_WM_PROTOCOLS; - ev.format = 32; - ev.data.l[0] = x11_display->atom__NET_WM_SYNC_REQUEST; - /* FIXME: meta_display_get_current_time() is bad, but since calls - * come from meta_window_move_resize_internal (which in turn come - * from all over), I'm not sure what we can do to fix it. Do we - * want to use _roundtrip, though? - */ - ev.data.l[1] = meta_display_get_current_time (window->display); - ev.data.l[2] = wait_serial & G_GUINT64_CONSTANT(0xffffffff); - ev.data.l[3] = wait_serial >> 32; - ev.data.l[4] = window->extended_sync_request_counter ? 1 : 0; - - /* We don't need to trap errors here as we are already - * inside an error_trap_push()/pop() pair. - */ - XSendEvent (x11_display->xdisplay, - window->xwindow, False, 0, (XEvent*) &ev); - - /* We give the window 1 sec to respond to _NET_WM_SYNC_REQUEST; - * if this time expires, we consider the window unresponsive - * and resize it unsynchonized. - */ - window->sync_request_timeout_id = g_timeout_add (1000, - sync_request_timeout, - window); - g_source_set_name_by_id (window->sync_request_timeout_id, - "[mutter] sync_request_timeout"); - - meta_compositor_sync_updates_frozen (window->display->compositor, window); -} - static unsigned long meta_window_get_net_wm_desktop (MetaWindow *window) { @@ -1544,7 +1465,7 @@ meta_window_x11_move_resize_internal (MetaWindow *window, * will be left undisturbed for us to paint to the screen until * the client finishes redrawing. */ - if (window->extended_sync_request_counter) + if (priv->sync_counter.extended_sync_request_counter) configure_frame_first = TRUE; else configure_frame_first = size_dx + size_dy >= 0; @@ -1571,14 +1492,8 @@ meta_window_x11_move_resize_internal (MetaWindow *window, meta_x11_error_trap_push (window->display->x11_display); if (window == window->display->grab_window && - meta_grab_op_is_resizing (window->display->grab_op) && - !window->disable_sync && - window->sync_request_counter != None && - window->sync_request_alarm != None && - window->sync_request_timeout_id == 0) - { - send_sync_request (window); - } + meta_grab_op_is_resizing (window->display->grab_op)) + meta_sync_counter_send_request (&priv->sync_counter); XConfigureWindow (window->display->x11_display->xdisplay, window->xwindow, @@ -1976,14 +1891,10 @@ meta_window_x11_is_stackable (MetaWindow *window) static gboolean meta_window_x11_are_updates_frozen (MetaWindow *window) { - if (window->extended_sync_request_counter && - window->sync_request_serial % 2 == 1) - return TRUE; + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); - if (window->sync_request_serial < window->sync_request_wait_serial) - return TRUE; - - return FALSE; + return meta_sync_counter_is_waiting (&priv->sync_counter); } /* Get layer ignoring any transient or group relationships */ @@ -4170,158 +4081,29 @@ meta_window_x11_set_allowed_actions_hint (MetaWindow *window) void meta_window_x11_create_sync_request_alarm (MetaWindow *window) { - MetaX11Display *x11_display = window->display->x11_display; - XSyncAlarmAttributes values; - XSyncValue init; + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); - if (window->sync_request_counter == None || - window->sync_request_alarm != None) - return; - - meta_x11_error_trap_push (x11_display); - - /* In the new (extended style), the counter value is initialized by - * the client before mapping the window. In the old style, we're - * responsible for setting the initial value of the counter. - */ - if (window->extended_sync_request_counter) - { - if (!XSyncQueryCounter(x11_display->xdisplay, - window->sync_request_counter, - &init)) - { - meta_x11_error_trap_pop_with_return (x11_display); - window->sync_request_counter = None; - return; - } - - window->sync_request_serial = - XSyncValueLow32 (init) + ((gint64)XSyncValueHigh32 (init) << 32); - } - else - { - XSyncIntToValue (&init, 0); - XSyncSetCounter (x11_display->xdisplay, - window->sync_request_counter, init); - window->sync_request_serial = 0; - } - - values.trigger.counter = window->sync_request_counter; - values.trigger.test_type = XSyncPositiveComparison; - - /* Initialize to one greater than the current value */ - values.trigger.value_type = XSyncRelative; - XSyncIntToValue (&values.trigger.wait_value, 1); - - /* After triggering, increment test_value by this until - * until the test condition is false */ - XSyncIntToValue (&values.delta, 1); - - /* we want events (on by default anyway) */ - values.events = True; - - window->sync_request_alarm = XSyncCreateAlarm (x11_display->xdisplay, - XSyncCACounter | - XSyncCAValueType | - XSyncCAValue | - XSyncCATestType | - XSyncCADelta | - XSyncCAEvents, - &values); - - if (meta_x11_error_trap_pop_with_return (x11_display) == Success) - meta_x11_display_register_sync_alarm (x11_display, &window->sync_request_alarm, window); - else - { - window->sync_request_alarm = None; - window->sync_request_counter = None; - } + meta_sync_counter_create_sync_alarm (&priv->sync_counter); } void meta_window_x11_destroy_sync_request_alarm (MetaWindow *window) { - MetaX11Display *x11_display = window->display->x11_display; + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); - if (window->sync_request_alarm != None) - { - /* Has to be unregistered _before_ clearing the structure field */ - meta_x11_display_unregister_sync_alarm (x11_display, window->sync_request_alarm); - XSyncDestroyAlarm (x11_display->xdisplay, - window->sync_request_alarm); - window->sync_request_alarm = None; - } + meta_sync_counter_destroy_sync_alarm (&priv->sync_counter); } void meta_window_x11_update_sync_request_counter (MetaWindow *window, gint64 new_counter_value) { - gboolean needs_frame_drawn = FALSE; - gboolean no_delay_frame = FALSE; + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); - COGL_TRACE_BEGIN (MetaWindowSyncRequestCounter, "X11: Sync request counter"); - - if (window->extended_sync_request_counter && new_counter_value % 2 == 0) - { - needs_frame_drawn = TRUE; - no_delay_frame = new_counter_value == window->sync_request_serial + 1; - } - - window->sync_request_serial = new_counter_value; - meta_compositor_sync_updates_frozen (window->display->compositor, window); - - if (new_counter_value >= window->sync_request_wait_serial && - window->sync_request_timeout_id) - { - - if (!window->extended_sync_request_counter || - new_counter_value % 2 == 0) - g_clear_handle_id (&window->sync_request_timeout_id, g_source_remove); - - if (window == window->display->grab_window && - meta_grab_op_is_resizing (window->display->grab_op) && - (!window->extended_sync_request_counter || - new_counter_value % 2 == 0)) - { - meta_topic (META_DEBUG_RESIZING, - "Alarm event received last motion x = %d y = %d", - window->display->grab_latest_motion_x, - window->display->grab_latest_motion_y); - - /* This means we are ready for another configure; - * no pointer round trip here, to keep in sync */ - meta_window_update_resize (window, - window->display->grab_last_edge_resistance_flags, - window->display->grab_latest_motion_x, - window->display->grab_latest_motion_y); - } - } - - /* If sync was previously disabled, turn it back on and hope - * the application has come to its senses (maybe it was just - * busy with a pagefault or a long computation). - */ - window->disable_sync = FALSE; - - if (needs_frame_drawn) - meta_compositor_queue_frame_drawn (window->display->compositor, window, - no_delay_frame); - -#ifdef COGL_HAS_TRACING - if (G_UNLIKELY (cogl_is_tracing_enabled ())) - { - g_autofree char *description = NULL; - - description = - g_strdup_printf ("sync request serial: %" G_GINT64_FORMAT ", " - "needs frame drawn: %s", - new_counter_value, - needs_frame_drawn ? "yes" : "no"); - COGL_TRACE_DESCRIBE (MetaWindowSyncRequestCounter, description); - COGL_TRACE_END (MetaWindowSyncRequestCounter); - } -#endif + meta_sync_counter_update (&priv->sync_counter, new_counter_value); } Window @@ -4461,3 +4243,30 @@ meta_window_x11_can_unredirect (MetaWindowX11 *window_x11) return FALSE; } + +MetaSyncCounter * +meta_window_x11_get_sync_counter (MetaWindow *window) +{ + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); + + return &priv->sync_counter; +} + +gboolean +meta_window_x11_has_active_sync_alarms (MetaWindow *window) +{ + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); + + return meta_sync_counter_has_sync_alarm (&priv->sync_counter); +} + +gboolean +meta_window_x11_is_awaiting_sync_response (MetaWindow *window) +{ + MetaWindowX11 *window_x11 = META_WINDOW_X11 (window); + MetaWindowX11Private *priv = meta_window_x11_get_instance_private (window_x11); + + return meta_sync_counter_is_waiting_response (&priv->sync_counter); +} diff --git a/src/x11/window-x11.h b/src/x11/window-x11.h index 5e45adf57..8193fea3a 100644 --- a/src/x11/window-x11.h +++ b/src/x11/window-x11.h @@ -28,6 +28,7 @@ #include "core/window-private.h" #include "meta/compositor.h" #include "meta/window.h" +#include "x11/meta-sync-counter.h" G_BEGIN_DECLS @@ -100,4 +101,10 @@ MetaRectangle meta_window_x11_get_client_rect (MetaWindowX11 *window_x11); gboolean meta_window_x11_can_unredirect (MetaWindowX11 *window_x11); +MetaSyncCounter * meta_window_x11_get_sync_counter (MetaWindow *window); + +gboolean meta_window_x11_has_active_sync_alarms (MetaWindow *window); + +gboolean meta_window_x11_is_awaiting_sync_response (MetaWindow *window); + #endif