From 4c78a15883a01556e85906c4616223df290d5e12 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 28 Mar 2023 07:13:30 +0900 Subject: [PATCH 1/2] Daemon: Workaround broken window menu for some apps (#1574) Co-authored-by: Ryan Kornheisl --- daemon/MenuDaemon.vala | 25 ++++++++++++++++--------- data/gala.appdata.xml.in | 2 ++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/daemon/MenuDaemon.vala b/daemon/MenuDaemon.vala index 92beb3c10..5029aab46 100644 --- a/daemon/MenuDaemon.vala +++ b/daemon/MenuDaemon.vala @@ -276,14 +276,21 @@ namespace Gala { close_accellabel.accel_string = keybind_settings.get_strv ("close")[0]; } - window_menu.popup (null, null, (m, ref px, ref py, out push_in) => { - var scale = m.scale_factor; - px = x / scale; - // Move the menu 1 pixel outside of the pointer or else it closes instantly - // on the mouse up event - py = (y / scale) + 1; - push_in = true; - }, 3, Gdk.CURRENT_TIME); + // `opened` is used as workaround for https://github.com/elementary/gala/issues/1387 + var opened = false; + Idle.add (() => { + window_menu.popup (null, null, (m, ref px, ref py, out push_in) => { + var scale = m.scale_factor; + px = x / scale; + // Move the menu 1 pixel outside of the pointer or else it closes instantly + // on the mouse up event + py = (y / scale) + 1; + push_in = true; + opened = true; + }, Gdk.BUTTON_SECONDARY, Gdk.CURRENT_TIME); + + return opened ? Source.REMOVE : Source.CONTINUE; + }); } public void show_desktop_menu (int x, int y) throws DBusError, IOError { @@ -354,7 +361,7 @@ namespace Gala { // on the mouse up event py = (y / scale) + 1; push_in = false; - }, 3, Gdk.CURRENT_TIME); + }, Gdk.BUTTON_SECONDARY, Gdk.CURRENT_TIME); } } } diff --git a/data/gala.appdata.xml.in b/data/gala.appdata.xml.in index 2ef9906dd..31cc2d1ec 100644 --- a/data/gala.appdata.xml.in +++ b/data/gala.appdata.xml.in @@ -22,6 +22,8 @@ Holding on keyboard shortcut to activate the action only once Notification bubble appears in wrong corner on one workspace + Parity between right-clicking titlebars/headerbars on mouse and touchpad + Window menu sluggish/inoperative for apps with flathub origin Closing a window in multitasking view closes multitasking view When 2 windows are tiled and then resized, the inactive one gets glitched, leaving its full-sized picture as an artifact when minimized From f75159568aedd1bf584d6517d804d0d276aea114 Mon Sep 17 00:00:00 2001 From: Leo Date: Tue, 28 Mar 2023 16:13:03 +0900 Subject: [PATCH 2/2] WindowOverview and WindowCloneContainer: Cleanup (#1587) * Code cleanup * Fixes an issue with window stacking in window overview where windows on other workspaces were higher than windows on current workspace * Prevents switching workspaces when window overview is opened --- src/InternalUtils.vala | 28 ++++- src/Widgets/WindowCloneContainer.vala | 161 ++++++++++++++------------ src/Widgets/WindowOverview.vala | 142 ++++++++++------------- src/WindowManager.vala | 3 +- 4 files changed, 172 insertions(+), 162 deletions(-) diff --git a/src/InternalUtils.vala b/src/InternalUtils.vala index af442e478..31184ad7e 100644 --- a/src/InternalUtils.vala +++ b/src/InternalUtils.vala @@ -314,8 +314,34 @@ namespace Gala { return result; } + /* + * Sorts the windows by stacking order so that the window on active workspaces come first. + */ + public static SList sort_windows (Meta.Display display, List windows) { + var windows_on_active_workspace = new SList (); + var windows_on_other_workspaces = new SList (); + unowned var active_workspace = display.get_workspace_manager ().get_active_workspace (); + foreach (unowned var window in windows) { + if (window.get_workspace () == active_workspace) { + windows_on_active_workspace.append (window); + } else { + windows_on_other_workspaces.append (window); + } + } + + var sorted_windows = new SList (); + var windows_on_active_workspace_sorted = display.sort_windows_by_stacking (windows_on_active_workspace); + windows_on_active_workspace_sorted.reverse (); + var windows_on_other_workspaces_sorted = display.sort_windows_by_stacking (windows_on_other_workspaces); + windows_on_other_workspaces_sorted.reverse (); + sorted_windows.concat ((owned) windows_on_active_workspace_sorted); + sorted_windows.concat ((owned) windows_on_other_workspaces_sorted); + + return sorted_windows; + } + public static inline bool get_window_is_normal (Meta.Window window) { - switch (window.get_window_type ()) { + switch (window.window_type) { case Meta.WindowType.NORMAL: case Meta.WindowType.DIALOG: case Meta.WindowType.MODAL_DIALOG: diff --git a/src/Widgets/WindowCloneContainer.vala b/src/Widgets/WindowCloneContainer.vala index 1a66ff269..4ac7b3e24 100644 --- a/src/Widgets/WindowCloneContainer.vala +++ b/src/Widgets/WindowCloneContainer.vala @@ -31,23 +31,18 @@ namespace Gala { public GestureTracker? gesture_tracker { get; construct; } public bool overview_mode { get; construct; } - private bool opened; + private bool opened = false; /** * The window that is currently selected via keyboard shortcuts. It is not * necessarily the same as the active window. */ - private WindowClone? current_window; + private WindowClone? current_window = null; public WindowCloneContainer (WindowManager wm, GestureTracker? gesture_tracker, bool overview_mode = false) { Object (wm: wm, gesture_tracker: gesture_tracker, overview_mode: overview_mode); } - construct { - opened = false; - current_window = null; - } - /** * Create a WindowClone for a MetaWindow and add it to the group * @@ -55,27 +50,24 @@ namespace Gala { */ public void add_window (Meta.Window window) { unowned Meta.Display display = window.get_display (); - var children = get_children (); - GLib.SList windows = new GLib.SList (); - foreach (unowned Clutter.Actor child in children) { - unowned WindowClone tw = (WindowClone) child; - windows.prepend (tw.window); + var windows = new List (); + foreach (unowned var child in get_children ()) { + unowned var clone = (WindowClone) child; + windows.append (clone.window); } - windows.prepend (window); - windows.reverse (); + windows.append (window); - var windows_ordered = display.sort_windows_by_stacking (windows); + var windows_ordered = InternalUtils.sort_windows (display, windows); var new_window = new WindowClone (wm, window, gesture_tracker, overview_mode); new_window.selected.connect (window_selected_cb); new_window.destroy.connect (window_destroyed); - new_window.request_reposition.connect (() => reflow ()); + new_window.request_reposition.connect (window_request_reposition); - var added = false; unowned Meta.Window? target = null; - foreach (unowned Meta.Window w in windows_ordered) { + foreach (unowned var w in windows_ordered) { if (w != window) { target = w; continue; @@ -83,19 +75,19 @@ namespace Gala { break; } - foreach (unowned Clutter.Actor child in children) { - unowned WindowClone tw = (WindowClone) child; - if (target == tw.window) { - insert_child_above (new_window, tw); - added = true; + // top most or no other children + if (target == null) { + add_child (new_window); + } + + foreach (unowned var child in get_children ()) { + unowned var clone = (WindowClone) child; + if (target == clone.window) { + insert_child_below (new_window, clone); break; } } - // top most or no other children - if (!added) - add_child (new_window); - reflow (); } @@ -103,7 +95,7 @@ namespace Gala { * Find and remove the WindowClone for a MetaWindow */ public void remove_window (Meta.Window window) { - foreach (var child in get_children ()) { + foreach (unowned var child in get_children ()) { if (((WindowClone) child).window == window) { remove_child (child); reflow (); @@ -112,43 +104,42 @@ namespace Gala { } } - private void window_selected_cb (WindowClone tiled) { - window_selected (tiled.window); + private void window_selected_cb (WindowClone clone) { + window_selected (clone.window); } private void window_destroyed (Clutter.Actor actor) { - var window = actor as WindowClone; - if (window == null) - return; + unowned var clone = (WindowClone) actor; - window.destroy.disconnect (window_destroyed); - window.selected.disconnect (window_selected_cb); + clone.destroy.disconnect (window_destroyed); + clone.selected.disconnect (window_selected_cb); + clone.request_reposition.disconnect (window_request_reposition); - Idle.add (() => { - reflow (); - return false; - }); + reflow (); + } + + private void window_request_reposition () { + reflow (); } /** * Sort the windows z-order by their actual stacking to make intersections * during animations correct. */ - public void restack_windows (Meta.Display display) { + public void restack_windows () { var children = get_children (); - GLib.SList windows = new GLib.SList (); + var windows = new List (); foreach (unowned Clutter.Actor child in children) { - unowned WindowClone tw = (WindowClone) child; - windows.prepend (tw.window); + windows.prepend (((WindowClone) child).window); } - var windows_ordered = display.sort_windows_by_stacking (windows); + var windows_ordered = InternalUtils.sort_windows (wm.get_display (), windows); windows_ordered.reverse (); - foreach (unowned Meta.Window window in windows_ordered) { - var i = 0; - foreach (unowned Clutter.Actor child in children) { + var i = 0; + foreach (unowned var window in windows_ordered) { + foreach (unowned var child in children) { if (((WindowClone) child).window == window) { set_child_at_index (child, i); children.remove (child); @@ -164,17 +155,19 @@ namespace Gala { * the resulting spots. */ public void reflow (bool with_gesture = false, bool is_cancel_animation = false) { - if (!opened) + if (!opened) { return; + } var windows = new List (); - foreach (var child in get_children ()) { - unowned WindowClone window = (WindowClone) child; - windows.prepend ({ window.window.get_frame_rect (), window }); + foreach (unowned var child in get_children ()) { + unowned var clone = (WindowClone) child; + windows.prepend ({ clone.window.get_frame_rect (), clone }); } - if (windows.length () < 1) + if (windows.is_empty ()) { return; + } // make sure the windows are always in the same order so the algorithm // doesn't give us different slots based on stacking order, which can lead @@ -195,9 +188,9 @@ namespace Gala { var window_positions = InternalUtils.calculate_grid_placement (area, windows); foreach (var tilable in window_positions) { - unowned WindowClone window = (WindowClone) tilable.id; - window.take_slot (tilable.rect, with_gesture, is_cancel_animation); - window.place_widgets (tilable.rect.width, tilable.rect.height); + unowned var clone = (WindowClone) tilable.id; + clone.take_slot (tilable.rect, with_gesture, is_cancel_animation); + clone.place_widgets (tilable.rect.width, tilable.rect.height); } } @@ -208,8 +201,9 @@ namespace Gala { * @param direction The MetaMotionDirection in which to search for windows for. */ public void select_next_window (Meta.MotionDirection direction) { - if (get_n_children () < 1) + if (get_n_children () < 1) { return; + } if (current_window == null) { current_window = (WindowClone) get_child_at_index (0); @@ -219,63 +213,76 @@ namespace Gala { var current_rect = current_window.slot; WindowClone? closest = null; - foreach (var window in get_children ()) { - if (window == current_window) + foreach (unowned var child in get_children ()) { + if (child == current_window) { continue; + } - var window_rect = ((WindowClone) window).slot; + var window_rect = ((WindowClone) child).slot; switch (direction) { case Meta.MotionDirection.LEFT: - if (window_rect.x > current_rect.x) + if (window_rect.x > current_rect.x) { continue; + } // test for vertical intersection if (window_rect.y + window_rect.height > current_rect.y && window_rect.y < current_rect.y + current_rect.height) { if (closest == null - || closest.slot.x < window_rect.x) - closest = (WindowClone) window; + || closest.slot.x < window_rect.x) { + + closest = (WindowClone) child; + } } break; case Meta.MotionDirection.RIGHT: - if (window_rect.x < current_rect.x) + if (window_rect.x < current_rect.x) { continue; + } // test for vertical intersection if (window_rect.y + window_rect.height > current_rect.y && window_rect.y < current_rect.y + current_rect.height) { if (closest == null - || closest.slot.x > window_rect.x) - closest = (WindowClone) window; + || closest.slot.x > window_rect.x) { + + closest = (WindowClone) child; + } } break; case Meta.MotionDirection.UP: - if (window_rect.y > current_rect.y) + if (window_rect.y > current_rect.y) { continue; + } // test for horizontal intersection if (window_rect.x + window_rect.width > current_rect.x && window_rect.x < current_rect.x + current_rect.width) { if (closest == null - || closest.slot.y < window_rect.y) - closest = (WindowClone) window; + || closest.slot.y < window_rect.y) { + + closest = (WindowClone) child; + } } break; case Meta.MotionDirection.DOWN: - if (window_rect.y < current_rect.y) + if (window_rect.y < current_rect.y) { continue; + } // test for horizontal intersection if (window_rect.x + window_rect.width > current_rect.x && window_rect.x < current_rect.x + current_rect.width) { if (closest == null - || closest.slot.y > window_rect.y) - closest = (WindowClone) window; + || closest.slot.y > window_rect.y) { + + closest = (WindowClone) child; + } } break; default: @@ -283,11 +290,13 @@ namespace Gala { } } - if (closest == null) + if (closest == null) { return; + } - if (current_window != null) + if (current_window != null) { current_window.active = false; + } closest.active = true; current_window = closest; @@ -318,9 +327,9 @@ namespace Gala { // hide the highlight when opened if (selected_window != null) { foreach (var child in get_children ()) { - unowned WindowClone tiled_window = (WindowClone) child; - if (tiled_window.window == selected_window) { - current_window = tiled_window; + unowned var clone = (WindowClone) child; + if (clone.window == selected_window) { + current_window = clone; break; } } diff --git a/src/Widgets/WindowOverview.vala b/src/Widgets/WindowOverview.vala index e23bca076..4adc71227 100644 --- a/src/Widgets/WindowOverview.vala +++ b/src/Widgets/WindowOverview.vala @@ -12,9 +12,7 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { public WindowManager wm { get; construct; } - private Meta.Display display; private ModalProxy modal_proxy; - private bool ready; // the workspaces which we expose right now private List workspaces; @@ -24,19 +22,10 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } construct { - display = wm.get_display (); - display.get_workspace_manager ().workspace_switched.connect (() => { close (); }); - display.restacked.connect (restack_windows); - visible = false; - ready = true; reactive = true; } - ~WindowOverview () { - display.restacked.disconnect (restack_windows); - } - public override bool key_press_event (Clutter.KeyEvent event) { if (event.keyval == Clutter.Key.Escape) { close (); @@ -62,93 +51,73 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } /** - * {@inheritDoc} - */ + * {@inheritDoc} + */ public bool is_opened () { return visible; } /** - * {@inheritDoc} - * You may specify 'all-windows' in hints to expose all windows - */ + * {@inheritDoc} + * You may specify 'all-windows' in hints to expose all windows + */ public void open (HashTable? hints = null) { - if (!ready) { - return; - } - - if (visible) { - close (); - return; - } - var all_windows = hints != null && "all-windows" in hints; - var used_windows = new SList (); - workspaces = new List (); - - unowned Meta.WorkspaceManager manager = display.get_workspace_manager (); + unowned var manager = wm.get_display ().get_workspace_manager (); if (all_windows) { - for (int i = 0; i < manager.get_n_workspaces (); i++) { - workspaces.append (manager.get_workspace_by_index (i)); + foreach (unowned var workspace in manager.get_workspaces ()) { + workspaces.append (workspace); } } else { workspaces.append (manager.get_active_workspace ()); } + var windows = new List (); foreach (var workspace in workspaces) { - foreach (var window in workspace.list_windows ()) { + foreach (unowned var window in workspace.list_windows ()) { + if (window.window_type == Meta.WindowType.DOCK) { + continue; + } + if (window.window_type != Meta.WindowType.NORMAL && - window.window_type != Meta.WindowType.DOCK && window.window_type != Meta.WindowType.DIALOG || window.is_attached_dialog ()) { unowned var actor = (Meta.WindowActor) window.get_compositor_private (); - if (actor != null) { - actor.hide (); - } - continue; - } - if (window.window_type == Meta.WindowType.DOCK) { + actor.hide (); + continue; } // skip windows that are on all workspace except we're currently // processing the workspace it actually belongs to - if (window.is_on_all_workspaces () && window.get_workspace () != workspace) { + if (window.on_all_workspaces && window.get_workspace () != workspace) { continue; } - used_windows.append (window); + windows.append (window); } } - var n_windows = used_windows.length (); - if (n_windows == 0) { + if (windows.is_empty ()) { return; } - ready = false; - foreach (var workspace in workspaces) { workspace.window_added.connect (add_window); workspace.window_removed.connect (remove_window); } - display.window_left_monitor.connect (window_left_monitor); - - // sort windows by stacking order - var windows = display.sort_windows_by_stacking (used_windows); + wm.get_display ().window_left_monitor.connect (window_left_monitor); grab_key_focus (); modal_proxy = wm.push_modal (this); modal_proxy.set_keybinding_filter (keybinding_filter); - visible = true; - - for (var i = 0; i < display.get_n_monitors (); i++) { - var geometry = display.get_monitor_geometry (i); + for (var i = 0; i < wm.get_display ().get_n_monitors (); i++) { + var geometry = wm.get_display ().get_monitor_geometry (i); var container = new WindowCloneContainer (wm, null, true) { padding_top = TOP_GAP, @@ -163,11 +132,11 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { add_child (container); } - foreach (var window in windows) { + visible = true; + + foreach (unowned var window in windows) { unowned var actor = (Meta.WindowActor) window.get_compositor_private (); - if (actor != null) { - actor.hide (); - } + actor.hide (); unowned var container = (WindowCloneContainer) get_child_at_index (window.get_monitor ()); if (container == null) { @@ -175,13 +144,8 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } container.add_window (window); + container.open (); } - - foreach (var child in get_children ()) { - ((WindowCloneContainer) child).open (); - } - - ready = true; } private bool keybinding_filter (Meta.KeyBinding binding) { @@ -198,9 +162,9 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { return true; } - private void restack_windows (Meta.Display display) { + private void restack_windows () { foreach (var child in get_children ()) { - ((WindowCloneContainer) child).restack_windows (display); + ((WindowCloneContainer) child).restack_windows (); } } @@ -220,8 +184,18 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } private void add_window (Meta.Window window) { - if (!visible - || (window.window_type != Meta.WindowType.NORMAL && window.window_type != Meta.WindowType.DIALOG)) { + if (!visible) { + return; + } + if (window.window_type == Meta.WindowType.DOCK) { + return; + } + if (window.window_type != Meta.WindowType.NORMAL && + window.window_type != Meta.WindowType.DIALOG || + window.is_attached_dialog ()) { + unowned var actor = (Meta.WindowActor) window.get_compositor_private (); + actor.hide (); + return; } @@ -249,42 +223,41 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } private void thumb_selected (Meta.Window window) { - if (window.get_workspace () == display.get_workspace_manager ().get_active_workspace ()) { - window.activate (display.get_current_time ()); + if (window.get_workspace () == wm.get_display ().get_workspace_manager ().get_active_workspace ()) { + window.activate (window.get_display ().get_current_time ()); close (); } else { close (); - //wait for the animation to finish before switching - Timeout.add (400, () => { - window.get_workspace ().activate_with_focus (window, display.get_current_time ()); + + // wait for the animation to finish before switching + Timeout.add (MultitaskingView.ANIMATION_DURATION, () => { + window.get_workspace ().activate_with_focus (window, window.get_display ().get_current_time ()); return Source.REMOVE; }); } } /** - * {@inheritDoc} - */ + * {@inheritDoc} + */ public void close (HashTable? hints = null) { - if (!visible || !ready) { + if (!visible) { return; } + restack_windows (); + foreach (var workspace in workspaces) { workspace.window_added.disconnect (add_window); workspace.window_removed.disconnect (remove_window); } + wm.get_display ().window_left_monitor.disconnect (window_left_monitor); - display.window_left_monitor.disconnect (window_left_monitor); - ready = false; - - wm.pop_modal (modal_proxy); - - foreach (var child in get_children ()) { + foreach (unowned var child in get_children ()) { ((WindowCloneContainer) child).close (); } - Clutter.Threads.Timeout.add (300, () => { + Clutter.Threads.Timeout.add (MultitaskingView.ANIMATION_DURATION, () => { cleanup (); return Source.REMOVE; @@ -292,10 +265,11 @@ public class Gala.WindowOverview : Clutter.Actor, ActivatableComponent { } private void cleanup () { - ready = true; visible = false; - foreach (var window in display.get_workspace_manager ().get_active_workspace ().list_windows ()) { + wm.pop_modal (modal_proxy); + + foreach (var window in wm.get_display ().get_workspace_manager ().get_active_workspace ().list_windows ()) { if (window.showing_on_its_workspace ()) { ((Clutter.Actor) window.get_compositor_private ()).show (); } diff --git a/src/WindowManager.vala b/src/WindowManager.vala index 5cd26c5fa..8e29ac695 100644 --- a/src/WindowManager.vala +++ b/src/WindowManager.vala @@ -1724,7 +1724,8 @@ namespace Gala { || AnimationDuration.WORKSPACE_SWITCH == 0 || (direction != Meta.MotionDirection.LEFT && direction != Meta.MotionDirection.RIGHT) || animating_switch_workspace - || workspace_view.is_opened ()) { + || workspace_view.is_opened () + || window_overview.is_opened ()) { animating_switch_workspace = false; switch_workspace_completed (); return;