From 615beab2e61e78493b8e871c87c3bd3dd41d1bb4 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Thu, 12 Jan 2023 11:57:56 +0100 Subject: [PATCH] Seat: rework Xwayland Override Redirect focus Instead of stashing the active view and setting Seat.focused to the Xwayland OR surface when a child OR surface of a currently focused Xwayland view is given keyboard focus, keep Seat.focused set to the Xwayland view. Such Override Redirect surfaces are commonly used for drop down menus and the like, and river should behave as if the parent Xwayland view still has focus. This ensures that the riverctl focus-view next/prev commands continue to work as expected while a popup is open, the correct focused view title will be sent over river status, etc. It's also cleaner to centralize this logic in XwaylandOverrideRedirect and keep it out of Seat.zig. --- river/Cursor.zig | 10 ++---- river/Seat.zig | 52 ++++++++---------------------- river/XwaylandOverrideRedirect.zig | 35 ++++++++++++++++---- river/XwaylandView.zig | 10 ------ 4 files changed, 44 insertions(+), 63 deletions(-) diff --git a/river/Cursor.zig b/river/Cursor.zig index 38eb6f6..8fd669d 100644 --- a/river/Cursor.zig +++ b/river/Cursor.zig @@ -367,12 +367,8 @@ fn updateKeyboardFocus(self: Self, result: SurfaceAtResult) void { self.seat.setFocusRaw(.{ .lock_surface = lock_surface }); }, .xwayland_override_redirect => |override_redirect| { - if (!build_options.xwayland) unreachable; - if (override_redirect.xwayland_surface.overrideRedirectWantsFocus() and - override_redirect.xwayland_surface.icccmInputModel() != .none) - { - self.seat.setFocusRaw(.{ .xwayland_override_redirect = override_redirect }); - } + assert(server.lock_manager.state != .unlocked); + override_redirect.focusIfDesired(); }, } } @@ -663,7 +659,7 @@ const SurfaceAtResult = struct { view: *View, layer_surface: *LayerSurface, lock_surface: *LockSurface, - xwayland_override_redirect: if (build_options.xwayland) *XwaylandOverrideRedirect else void, + xwayland_override_redirect: if (build_options.xwayland) *XwaylandOverrideRedirect else noreturn, }, }; diff --git a/river/Seat.zig b/river/Seat.zig index 441ede3..baf1c74 100644 --- a/river/Seat.zig +++ b/river/Seat.zig @@ -80,9 +80,6 @@ focused_output: *Output, focused: FocusTarget = .none, -/// Currently activated Xwayland view (used to handle override redirect menus) -activated_xwayland_view: if (build_options.xwayland) ?*View else void = if (build_options.xwayland) null else {}, - /// Stack of views in most recently focused order /// If there is a currently focused view, it is on top. focus_stack: ViewStack(*View) = .{}, @@ -233,36 +230,11 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void { .none => null, }; - if (build_options.xwayland) { - // Keep the parent top-level Xwayland view of any override redirect surface - // activated while that override redirect surface is focused. This ensures - // override redirect menus do not disappear as a result of deactivating - // their parent window. - if (new_focus == .xwayland_override_redirect and - self.focused == .view and - self.focused.view.impl == .xwayland_view and - self.focused.view.impl.xwayland_view.xwayland_surface.pid == new_focus.xwayland_override_redirect.xwayland_surface.pid) - { - self.activated_xwayland_view = self.focused.view; - } else if (self.activated_xwayland_view) |active_view| { - if (!(new_focus == .view and new_focus.view == active_view) and - !(new_focus == .xwayland_override_redirect and new_focus.xwayland_override_redirect.xwayland_surface.pid == active_view.impl.xwayland_view.xwayland_surface.pid)) - { - if (active_view.pending.focus == 0) active_view.setActivated(false); - self.activated_xwayland_view = null; - } - } - } - // First clear the current focus switch (self.focused) { .view => |view| { view.pending.focus -= 1; - if (view.pending.focus == 0 and - (!build_options.xwayland or view != self.activated_xwayland_view)) - { - view.setActivated(false); - } + if (view.pending.focus == 0) view.setActivated(false); }, .xwayland_override_redirect, .layer, .lock_surface, .none => {}, } @@ -272,11 +244,7 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void { .view => |target_view| { assert(server.lock_manager.state == .unlocked); assert(self.focused_output == target_view.output); - if (target_view.pending.focus == 0 and - (!build_options.xwayland or target_view != self.activated_xwayland_view)) - { - target_view.setActivated(true); - } + if (target_view.pending.focus == 0) target_view.setActivated(true); target_view.pending.focus += 1; target_view.pending.urgent = false; }, @@ -289,7 +257,17 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void { } self.focused = new_focus; - // Send keyboard enter/leave events and handle pointer constraints + self.keyboardEnterOrLeave(target_surface); + + // Inform any clients tracking status of the change + var it = self.status_trackers.first; + while (it) |node| : (it = node.next) node.data.sendFocusedView(); +} + +/// Send keyboard enter/leave events and handle pointer constraints +/// This should never normally be called from outside of setFocusRaw(), but we make an exception for +/// XwaylandOverrideRedirect surfaces as they don't conform to the Wayland focus model. +pub fn keyboardEnterOrLeave(self: *Self, target_surface: ?*wlr.Surface) void { if (target_surface) |wlr_surface| { self.keyboardNotifyEnter(wlr_surface); @@ -317,10 +295,6 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void { self.cursor.may_need_warp = true; } } - - // Inform any clients tracking status of the change - var it = self.status_trackers.first; - while (it) |node| : (it = node.next) node.data.sendFocusedView(); } fn keyboardNotifyEnter(self: *Self, wlr_surface: *wlr.Surface) void { diff --git a/river/XwaylandOverrideRedirect.zig b/river/XwaylandOverrideRedirect.zig index 6b61100..224ad65 100644 --- a/river/XwaylandOverrideRedirect.zig +++ b/river/XwaylandOverrideRedirect.zig @@ -97,10 +97,26 @@ pub fn handleMap(listener: *wl.Listener(*wlr.XwaylandSurface), xwayland_surface: xwayland_surface.surface.?.events.commit.add(&self.commit); + self.focusIfDesired(); +} + +pub fn focusIfDesired(self: *Self) void { if (self.xwayland_surface.overrideRedirectWantsFocus() and self.xwayland_surface.icccmInputModel() != .none) { - server.input_manager.defaultSeat().setFocusRaw(.{ .xwayland_override_redirect = self }); + const seat = server.input_manager.defaultSeat(); + // Keep the parent top-level Xwayland view of any override redirect surface + // activated while that override redirect surface is focused. This ensures + // override redirect menus do not disappear as a result of deactivating + // their parent window. + if (seat.focused == .view and + seat.focused.view.impl == .xwayland_view and + seat.focused.view.impl.xwayland_view.xwayland_surface.pid == self.xwayland_surface.pid) + { + seat.keyboardEnterOrLeave(self.xwayland_surface.surface); + } else { + seat.setFocusRaw(.{ .xwayland_override_redirect = self }); + } } } @@ -113,15 +129,20 @@ fn handleUnmap(listener: *wl.Listener(*wlr.XwaylandSurface), _: *wlr.XwaylandSur self.commit.link.remove(); - // If the unmapped surface is currently focused, reset focus to the most - // appropriate view. + // If the unmapped surface is currently focused, pass keyboard focus + // to the most appropriate surface. var seat_it = server.input_manager.seats.first; while (seat_it) |seat_node| : (seat_it = seat_node.next) { const seat = &seat_node.data; - if (seat.focused == .xwayland_override_redirect and - seat.focused.xwayland_override_redirect == self) - { - seat.focus(null); + switch (seat.focused) { + .view => |focused| if (focused.impl == .xwayland_view and + focused.impl.xwayland_view.xwayland_surface.pid == self.xwayland_surface.pid and + seat.wlr_seat.keyboard_state.focused_surface == self.xwayland_surface.surface) + { + seat.keyboardEnterOrLeave(focused.surface.?); + }, + .xwayland_override_redirect => |focused| if (focused == self) seat.focus(null), + .layer, .lock_surface, .none => {}, } } diff --git a/river/XwaylandView.zig b/river/XwaylandView.zig index 55681e6..0eef7a4 100644 --- a/river/XwaylandView.zig +++ b/river/XwaylandView.zig @@ -175,16 +175,6 @@ pub fn getConstraints(self: Self) View.Constraints { /// Called when the xwayland surface is destroyed fn handleDestroy(listener: *wl.Listener(*wlr.XwaylandSurface), _: *wlr.XwaylandSurface) void { const self = @fieldParentPtr(Self, "destroy", listener); - const view = self.view; - - // Ensure no seat will attempt to access this view after it is destroyed. - var seat_it = server.input_manager.seats.first; - while (seat_it) |seat_node| : (seat_it = seat_node.next) { - const seat = &seat_node.data; - if (seat.activated_xwayland_view == view) { - seat.activated_xwayland_view = null; - } - } // Remove listeners that are active for the entire lifetime of the view self.destroy.link.remove();