From 66a4cb4a49072c44888b8112cb12468c4c7bedae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberts=20Muktup=C4=81vels?= Date: Sat, 8 Sep 2018 18:24:42 +0300 Subject: [PATCH 1/3] display: ensure that we ignore our own focus events for focus predictions When we set the input focus, we first set the predicted window, and then try to process focus events. But as FocusOut on the existing window comes before FocusIn on the new window, we'll see the focus out on the old window and think the focus is going to nothing, which makes metacity think the prediction failed. Fix this by making sure that we ignore focus window changes of our own cause when updating the focus window field, by ignoring all focus events that have a serial the same as the focus request or lower. Note that if metacity doens't make any requests after the focus request, this could be racy, as another client could steal the focus, but metacity would ignore it as the serial was the same. Bump the serial by making a dummy ChangeProperty request to a metactiy-controlled window in this case. Based on mutter commit: https://gitlab.gnome.org/GNOME/mutter/commit/7fdfbad6d495ede1632588e528801443846e5f6d --- src/core/atomnames.h | 2 ++ src/core/display.c | 59 +++++++++++++++++++++++++++++++++----------- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/core/atomnames.h b/src/core/atomnames.h index 6e142196..bab2630b 100644 --- a/src/core/atomnames.h +++ b/src/core/atomnames.h @@ -62,6 +62,8 @@ item(_GTK_FRAME_EXTENTS) item(_GNOME_PANEL_ACTION) item(_GNOME_PANEL_ACTION_MAIN_MENU) item(_GNOME_PANEL_ACTION_RUN_DIALOG) +item(_METACITY_TIMESTAMP_PING) +item(_METACITY_FOCUS_SET) item(_METACITY_SENTINEL) item(_METACITY_VERSION) item(WM_CLIENT_MACHINE) diff --git a/src/core/display.c b/src/core/display.c index 7f0698e4..f4338da7 100644 --- a/src/core/display.c +++ b/src/core/display.c @@ -1210,6 +1210,17 @@ meta_display_get_current_time (MetaDisplay *display) return display->current_time; } +static Bool +find_timestamp_predicate (Display *xdisplay, + XEvent *ev, + XPointer arg) +{ + MetaDisplay *display = (MetaDisplay *) arg; + + return (ev->type == PropertyNotify && + ev->xproperty.atom == display->atom__METACITY_TIMESTAMP_PING); +} + /* Get a timestamp, even if it means a roundtrip */ guint32 meta_display_get_current_time_roundtrip (MetaDisplay *display) @@ -1221,17 +1232,13 @@ meta_display_get_current_time_roundtrip (MetaDisplay *display) { XEvent property_event; - /* Using the property XA_PRIMARY because it's safe; nothing - * would use it as a property. The type doesn't matter. - */ - XChangeProperty (display->xdisplay, - display->timestamp_pinging_window, - XA_PRIMARY, XA_STRING, 8, - PropModeAppend, NULL, 0); - XWindowEvent (display->xdisplay, - display->timestamp_pinging_window, - PropertyChangeMask, - &property_event); + XChangeProperty (display->xdisplay, display->timestamp_pinging_window, + display->atom__METACITY_TIMESTAMP_PING, + XA_STRING, 8, PropModeAppend, NULL, 0); + XIfEvent (display->xdisplay, + &property_event, + find_timestamp_predicate, + (XPointer) display); timestamp = property_event.xproperty.time; } @@ -1485,6 +1492,7 @@ request_xserver_input_focus_change (MetaDisplay *display, guint32 timestamp) { MetaWindow *meta_window; + gulong serial; if (timestamp_too_old (display, ×tamp)) return; @@ -1492,14 +1500,35 @@ request_xserver_input_focus_change (MetaDisplay *display, meta_window = meta_display_lookup_x_window (display, xwindow); meta_error_trap_push (display); - update_focus_window (display, - meta_window, - XNextRequest (display->xdisplay)); + + /* In order for mutter to know that the focus request succeeded, we track + * the serial of the "focus request" we made, but if we take the serial + * of the XSetInputFocus request, then there's no way to determine the + * difference between focus events as a result of the SetInputFocus and + * focus events that other clients send around the same time. Ensure that + * we know which is which by making two requests that the server will + * process at the same time. + */ + XGrabServer (display->xdisplay); + + serial = XNextRequest (display->xdisplay); XSetInputFocus (display->xdisplay, xwindow, RevertToPointerRoot, timestamp); + + XChangeProperty (display->xdisplay, display->timestamp_pinging_window, + display->atom__METACITY_FOCUS_SET, + XA_STRING, 8, PropModeAppend, NULL, 0); + + XUngrabServer (display->xdisplay); + XFlush (display->xdisplay); + + update_focus_window (display, + meta_window, + serial); + meta_error_trap_pop (display); display->last_focus_time = timestamp; @@ -1608,7 +1637,7 @@ handle_window_focus_event (MetaDisplay *display, else g_return_if_reached (); - if (display->server_focus_serial >= display->focus_serial) + if (display->server_focus_serial > display->focus_serial) { update_focus_window (display, focus_window, display->server_focus_serial); -- GitLab From 19c5732a24e89f405365efcfd749c55b4c9c215f Mon Sep 17 00:00:00 2001 From: "Owen W. Taylor" Date: Mon, 16 Dec 2013 16:01:04 -0500 Subject: [PATCH 2/3] fix problems with focus tracking When a client spontaneously focuses their window, perhaps in response to WM_TAKE_FOCUS we'll get a FocusOut/FocusIn pair with same serial. Updating display->focus_serial in response to FocusOut then was causing us to ignore FocusIn and think that the focus was not on any window. We need to distinguish this spontaneous case from the case where we set the focus ourselves - when we set the focus ourselves, we're careful to combine the SetFocus with a property change so that we know definitively what focus events we have already accounted for. https://bugzilla.gnome.org/show_bug.cgi?id=720558 --- src/core/display-private.h | 8 ++++++++ src/core/display.c | 25 +++++++++++++++++++------ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/core/display-private.h b/src/core/display-private.h index b7246ec9..8a2ee3ed 100644 --- a/src/core/display-private.h +++ b/src/core/display-private.h @@ -127,6 +127,14 @@ struct _MetaDisplay */ guint allow_terminal_deactivation : 1; + /* If true, server->focus_serial refers to us changing the focus; in + * this case, we can ignore focus events that have exactly focus_serial, + * since we take care to make another request immediately afterwards. + * But if focus is being changed by another client, we have to accept + * multiple events with the same serial. + */ + guint focused_by_us : 1; + /*< private-ish >*/ guint error_trap_synced_at_last_pop : 1; MetaScreen *screen; diff --git a/src/core/display.c b/src/core/display.c index f4338da7..b048f5f4 100644 --- a/src/core/display.c +++ b/src/core/display.c @@ -1415,9 +1415,11 @@ button_press_event_new (XEvent *xevent, static void update_focus_window (MetaDisplay *display, MetaWindow *window, - gulong serial) + gulong serial, + gboolean focused_by_us) { display->focus_serial = serial; + display->focused_by_us = focused_by_us; if (window == display->focus_window) return; @@ -1527,7 +1529,8 @@ request_xserver_input_focus_change (MetaDisplay *display, update_focus_window (display, meta_window, - serial); + serial, + TRUE); meta_error_trap_pop (display); @@ -1637,10 +1640,18 @@ handle_window_focus_event (MetaDisplay *display, else g_return_if_reached (); - if (display->server_focus_serial > display->focus_serial) + /* If display->focused_by_us, then the focus_serial will be used only + * for a focus change we made and have already accounted for. + * (See request_xserver_input_focus_change().) Otherwise, we can get + * multiple focus events with the same serial. + */ + if (display->server_focus_serial > display->focus_serial || + (!display->focused_by_us && + display->server_focus_serial == display->focus_serial)) { update_focus_window (display, focus_window, - display->server_focus_serial); + display->server_focus_serial, + FALSE); } } @@ -1685,7 +1696,8 @@ event_callback (XEvent *event, display->current_time = event_get_time (display, event); display->monitor_cache_invalidated = TRUE; - if (event->xany.serial > display->focus_serial && + if (display->focused_by_us && + event->xany.serial > display->focus_serial && display->focus_window && display->focus_window->xwindow != display->server_focus_window) { @@ -1693,7 +1705,8 @@ event_callback (XEvent *event, display->focus_window->desc); update_focus_window (display, meta_display_lookup_x_window (display, display->server_focus_window), - display->server_focus_serial); + display->server_focus_serial, + FALSE); } modified = event_get_modified_window (display, event); -- GitLab From d69fba052cdd0aa8348dc1bbbbb1ecbadfbebbd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alberts=20Muktup=C4=81vels?= Date: Sun, 9 Sep 2018 18:22:15 +0300 Subject: [PATCH 3/3] bump version to 3.30.1, update NEWS --- NEWS | 4 ++++ configure.ac | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 0f74d385..1d48e6bf 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +Version 3.30.1 +============== +- Improve focus tracking + Version 3.30.0 ============== - Fix build warnings diff --git a/configure.ac b/configure.ac index 6d76b1bd..27c4707d 100644 --- a/configure.ac +++ b/configure.ac @@ -4,7 +4,7 @@ dnl ************************************************************************** m4_define([m_major_version], [3]) m4_define([m_minor_version], [30]) -m4_define([m_micro_version], [0]) +m4_define([m_micro_version], [1]) m4_define([m_version], [m_major_version.m_minor_version.m_micro_version]) dnl ************************************************************************** -- GitLab