From 8490558b8b416c91060c6c858d28696a4dac38f9 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sun, 16 Mar 2025 13:37:57 +0100 Subject: [PATCH 1/2] Cursor: restore previous image on unhide If client A has an xdg_popup open and the user moves the cursor over a surface of client B and waits for the cursor to be hidden after a timeout, the cursor will not be shown on movement until the (invisible) cursor is moved back into a surface of client A or somewhere the compositor is responsible for rendering the cursor. This is due to the (flawed) xdg popup grab interface of wlroots which prevents wlr_seat_pointer_notify_enter() from sending events to clients other than the one with the active xdg popup. Closes: https://codeberg.org/river/river/issues/1192 --- river/Cursor.zig | 70 +++++++++++++++++++++++++++++++++++++----------- river/Server.zig | 2 +- 2 files changed, 56 insertions(+), 16 deletions(-) diff --git a/river/Cursor.zig b/river/Cursor.zig index bb6334f..7a9eeb0 100644 --- a/river/Cursor.zig +++ b/river/Cursor.zig @@ -108,6 +108,19 @@ const LayoutPoint = struct { ly: f64, }; +const Image = union(enum) { + /// No cursor image + none, + /// Name of the current Xcursor shape + xcursor: [*:0]const u8, + /// Cursor surface configured by a client + client: struct { + surface: *wlr.Surface, + hotspot_x: i32, + hotspot_y: i32, + }, +}; + const log = std.log.scoped(.cursor); /// Current cursor mode as well as any state needed to implement that mode @@ -124,9 +137,8 @@ wlr_cursor: *wlr.Cursor, /// Xcursor manager for the currently configured Xcursor theme. xcursor_manager: *wlr.XcursorManager, -/// Name of the current Xcursor shape, or null if a client has configured a -/// surface to be used as the cursor shape instead. -xcursor_name: ?[*:0]const u8 = null, +image: Image = .none, +image_surface_destroy: wl.Listener(*wlr.Surface) = wl.Listener(*wlr.Surface).init(handleImageSurfaceDestroy), /// Number of distinct buttons currently pressed pressed_count: u32 = 0, @@ -286,18 +298,40 @@ pub fn setTheme(cursor: *Cursor, theme: ?[*:0]const u8, _size: ?u32) !void { cursor.xcursor_manager.destroy(); cursor.xcursor_manager = xcursor_manager; - if (cursor.xcursor_name) |name| { - cursor.setXcursor(name); + switch (cursor.image) { + .none, .client => {}, + .xcursor => |name| cursor.wlr_cursor.setXcursor(xcursor_manager, name), } } -pub fn setXcursor(cursor: *Cursor, name: [*:0]const u8) void { - cursor.wlr_cursor.setXcursor(cursor.xcursor_manager, name); - cursor.xcursor_name = name; +pub fn setImage(cursor: *Cursor, image: Image) void { + switch (cursor.image) { + .none, .xcursor => {}, + .client => { + cursor.image_surface_destroy.link.remove(); + }, + } + cursor.image = image; + switch (cursor.image) { + .none => cursor.wlr_cursor.unsetImage(), + .xcursor => |name| cursor.wlr_cursor.setXcursor(cursor.xcursor_manager, name), + .client => |client| { + cursor.wlr_cursor.setSurface(client.surface, client.hotspot_x, client.hotspot_y); + client.surface.events.destroy.add(&cursor.image_surface_destroy); + }, + } +} + +fn handleImageSurfaceDestroy(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void { + const cursor: *Cursor = @fieldParentPtr("image_surface_destroy", listener); + // wlroots calls wlr_cursor_unset_image() automatically + // when the cursor surface is destroyed. + cursor.image = .none; + cursor.image_surface_destroy.link.remove(); } fn clearFocus(cursor: *Cursor) void { - cursor.setXcursor("default"); + cursor.setImage(.{ .xcursor = "default" }); cursor.seat.wlr_seat.pointerNotifyClearFocus(); } @@ -740,8 +774,15 @@ fn handleRequestSetCursor( // on the output that it's currently on and continue to do so as the // cursor moves between outputs. log.debug("focused client set cursor", .{}); - cursor.wlr_cursor.setSurface(event.surface, event.hotspot_x, event.hotspot_y); - cursor.xcursor_name = null; + if (event.surface) |surface| { + cursor.setImage(.{ .client = .{ + .surface = surface, + .hotspot_x = event.hotspot_x, + .hotspot_y = event.hotspot_y, + } }); + } else { + cursor.setImage(.none); + } } } @@ -757,8 +798,6 @@ pub fn hide(cursor: *Cursor) void { cursor.hidden = true; cursor.wlr_cursor.unsetImage(); - cursor.xcursor_name = null; - cursor.seat.wlr_seat.pointerNotifyClearFocus(); cursor.hide_cursor_timer.timerUpdate(0) catch { log.err("failed to update cursor hide timeout", .{}); }; @@ -770,6 +809,7 @@ pub fn unhide(cursor: *Cursor) void { }; if (!cursor.hidden) return; cursor.hidden = false; + cursor.setImage(cursor.image); cursor.updateState(); } @@ -868,7 +908,7 @@ fn computeEdges(cursor: *const Cursor, view: *const View) wlr.Edges { } } -fn enterMode(cursor: *Cursor, mode: Mode, view: *View, xcursor_name: [*:0]const u8) void { +fn enterMode(cursor: *Cursor, mode: Mode, view: *View, xcursor: [*:0]const u8) void { assert(cursor.mode == .passthrough or cursor.mode == .down); assert(mode == .move or mode == .resize); @@ -884,7 +924,7 @@ fn enterMode(cursor: *Cursor, mode: Mode, view: *View, xcursor_name: [*:0]const } cursor.seat.wlr_seat.pointerNotifyClearFocus(); - cursor.setXcursor(xcursor_name); + cursor.setImage(.{ .xcursor = xcursor }); server.root.applyPending(); } diff --git a/river/Server.zig b/river/Server.zig index ea06842..05bc96b 100644 --- a/river/Server.zig +++ b/river/Server.zig @@ -519,7 +519,7 @@ fn handleRequestSetCursorShape( // actually has pointer focus first. if (focused_client == event.seat_client) { const name = wlr.CursorShapeManagerV1.shapeName(event.shape); - seat.cursor.setXcursor(name); + seat.cursor.setImage(.{ .xcursor = name }); } } } From 46f77f30dcce06b7af0ec8dff5ae3e4fbc73176f Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sat, 29 Mar 2025 10:54:29 +0100 Subject: [PATCH 2/2] Seat: put all keyboards in a single group Deprecate and ignore the riverctl commands for creating explicit keyboard groups. In my mind, the only reason to have more than one keyboard group is if different keyboard devices are assigned different keymaps or repeat rates. River does not currently allow such things to be configured however. When river eventually makes it possible to configure different keymaps and repeat rates per keyboard device, there is no reason we can't 100% automatically group keyboards based on the keymap/repeat rate. Exposing this keyboard group abstraction to the user is just bad UX. Failing to group keyboards automatically also creates confusing/buggy behavior for the user if the hardware, for example, exposes some of the the XF86 buttons on a laptop as a separate keyboard device from the main keyboard. Creating keybindings for these XF86 buttons that use modifiers doesn't work by default, but there's no reason it shouldn't just work. Closes: https://codeberg.org/river/river/issues/1138 --- completions/bash/riverctl | 4 - completions/fish/riverctl.fish | 5 -- completions/zsh/_riverctl | 5 -- doc/riverctl.1.scd | 19 ----- river/Keyboard.zig | 13 +-- river/KeyboardGroup.zig | 141 ------------------------------- river/Seat.zig | 10 +-- river/command/keyboard_group.zig | 78 ++--------------- 8 files changed, 15 insertions(+), 260 deletions(-) delete mode 100644 river/KeyboardGroup.zig diff --git a/completions/bash/riverctl b/completions/bash/riverctl index a889d4c..891f13c 100644 --- a/completions/bash/riverctl +++ b/completions/bash/riverctl @@ -4,10 +4,6 @@ function __riverctl_completion () if [ "${COMP_CWORD}" -eq 1 ] then OPTS=" \ - keyboard-group-create \ - keyboard-group-destroy \ - keyboard-group-add \ - keyboard-group-remove \ keyboard-layout \ keyboard-layout-file \ close \ diff --git a/completions/fish/riverctl.fish b/completions/fish/riverctl.fish index 50458b4..70aceb7 100644 --- a/completions/fish/riverctl.fish +++ b/completions/fish/riverctl.fish @@ -72,11 +72,6 @@ complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'hide-cursor' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'set-repeat' -d 'Set the keyboard repeat rate and repeat delay' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'set-cursor-warp' -d 'Set the cursor warp mode' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'xcursor-theme' -d 'Set the xcursor theme' -# Keyboardgroups -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-create' -d 'Create a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-destroy' -d 'Destroy a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-add' -d 'Add a keyboard to a keyboard group' -complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-group-remove' -d 'Remove a keyboard from a keyboard group' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-layout' -d 'Set the keyboard layout' complete -c riverctl -n '__fish_riverctl_complete_arg 1' -a 'keyboard-layout-file' -d 'Set the keyboard layout from a file.' diff --git a/completions/zsh/_riverctl b/completions/zsh/_riverctl index ad372d3..8bfd8f0 100644 --- a/completions/zsh/_riverctl +++ b/completions/zsh/_riverctl @@ -62,11 +62,6 @@ _riverctl_commands() 'set-repeat:Set the keyboard repeat rate and repeat delay' 'set-cursor-warp:Set the cursor warp mode.' 'xcursor-theme:Set the xcursor theme' - # Keyboard groups - 'keyboard-group-create:Create a keyboard group' - 'keyboard-group-destroy:Destroy a keyboard group' - 'keyboard-group-add:Add a keyboard to a keyboard group' - 'keyboard-group-remove:Remove a keyboard from a keyboard group' 'keyboard-layout:Set the keyboard layout' 'keyboard-layout-file:Set the keyboard layout from a file' # Input diff --git a/doc/riverctl.1.scd b/doc/riverctl.1.scd index d0476ea..9c1f223 100644 --- a/doc/riverctl.1.scd +++ b/doc/riverctl.1.scd @@ -454,25 +454,6 @@ matches everything while _\*\*_ and the empty string are invalid. following URL: https://xkbcommon.org/doc/current/keymap-text-format-v1.html -*keyboard-group-create* _group_name_ - Create a keyboard group. A keyboard group collects multiple keyboards in - a single logical keyboard. This means that all state, like the active - modifiers, is shared between the keyboards in a group. - -*keyboard-group-destroy* _group_name_ - Destroy the keyboard group with the given name. All attached keyboards - will be released, making them act as separate devices again. - -*keyboard-group-add* _group_name_ _input_device_name_ - Add a keyboard to a keyboard group, identified by the keyboard's - input device name. Any currently connected and future keyboards with - the given name will be added to the group. Simple globbing patterns are - supported, see the rules section for further information on globs. - -*keyboard-group-remove* _group_name_ _input_device_name_ - Remove a keyboard from a keyboard group, identified by the keyboard's - input device name. - The _input_ command can be used to create a configuration rule for an input device identified by its _name_. The _name_ of an input device consists of its type, its decimal vendor id, diff --git a/river/Keyboard.zig b/river/Keyboard.zig index 3c659d8..ef43043 100644 --- a/river/Keyboard.zig +++ b/river/Keyboard.zig @@ -101,16 +101,9 @@ pub fn init(keyboard: *Keyboard, seat: *Seat, wlr_device: *wlr.InputDevice) !voi // wlroots will log a more detailed error if this fails. if (!wlr_keyboard.setKeymap(server.config.keymap)) return error.OutOfMemory; - // Add to keyboard-group, if applicable. - var group_it = seat.keyboard_groups.first; - outer: while (group_it) |group_node| : (group_it = group_node.next) { - for (group_node.data.globs.items) |glob| { - if (globber.match(glob, keyboard.device.identifier)) { - // wlroots will log an error if this fails explaining the reason. - _ = group_node.data.wlr_group.addKeyboard(wlr_keyboard); - break :outer; - } - } + if (wlr.KeyboardGroup.fromKeyboard(wlr_keyboard) == null) { + // wlroots will log an error on failure + _ = seat.keyboard_group.addKeyboard(wlr_keyboard); } wlr_keyboard.setRepeatInfo(server.config.repeat_rate, server.config.repeat_delay); diff --git a/river/KeyboardGroup.zig b/river/KeyboardGroup.zig deleted file mode 100644 index 5f79608..0000000 --- a/river/KeyboardGroup.zig +++ /dev/null @@ -1,141 +0,0 @@ -// This file is part of river, a dynamic tiling wayland compositor. -// -// Copyright 2022 The River Developers -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, version 3. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -const KeyboardGroup = @This(); - -const std = @import("std"); -const assert = std.debug.assert; -const mem = std.mem; - -const globber = @import("globber"); -const wlr = @import("wlroots"); -const wl = @import("wayland").server.wl; -const xkb = @import("xkbcommon"); - -const log = std.log.scoped(.keyboard_group); - -const server = &@import("main.zig").server; -const util = @import("util.zig"); - -const Seat = @import("Seat.zig"); -const Keyboard = @import("Keyboard.zig"); - -seat: *Seat, -wlr_group: *wlr.KeyboardGroup, -name: []const u8, -globs: std.ArrayListUnmanaged([]const u8) = .{}, - -pub fn create(seat: *Seat, name: []const u8) !void { - log.debug("new keyboard group: '{s}'", .{name}); - - const node = try util.gpa.create(std.TailQueue(KeyboardGroup).Node); - errdefer util.gpa.destroy(node); - - const wlr_group = try wlr.KeyboardGroup.create(); - errdefer wlr_group.destroy(); - - const owned_name = try util.gpa.dupe(u8, name); - errdefer util.gpa.free(owned_name); - - node.data = .{ - .wlr_group = wlr_group, - .name = owned_name, - .seat = seat, - }; - - seat.addDevice(&wlr_group.keyboard.base); - seat.keyboard_groups.append(node); -} - -pub fn destroy(group: *KeyboardGroup) void { - log.debug("destroying keyboard group: '{s}'", .{group.name}); - - util.gpa.free(group.name); - - for (group.globs.items) |glob| { - util.gpa.free(glob); - } - group.globs.deinit(util.gpa); - - group.wlr_group.destroy(); - - const node: *std.TailQueue(KeyboardGroup).Node = @fieldParentPtr("data", group); - group.seat.keyboard_groups.remove(node); - util.gpa.destroy(node); -} - -pub fn addIdentifier(group: *KeyboardGroup, new_id: []const u8) !void { - for (group.globs.items) |glob| { - if (mem.eql(u8, glob, new_id)) return; - } - - log.debug("keyboard group '{s}' adding identifier: '{s}'", .{ group.name, new_id }); - - const owned_id = try util.gpa.dupe(u8, new_id); - errdefer util.gpa.free(owned_id); - - // Glob is validated in the command handler. - try group.globs.append(util.gpa, owned_id); - errdefer { - // Not used now, but if at any point this function is modified to that - // it may return an error after the glob pattern is added to the list, - // the list will have a pointer to freed memory in its last position. - _ = group.globs.pop(); - } - - // Add any existing matching keyboards to the group. - var it = server.input_manager.devices.iterator(.forward); - while (it.next()) |device| { - if (device.seat != group.seat) continue; - if (device.wlr_device.type != .keyboard) continue; - - if (globber.match(device.identifier, new_id)) { - log.debug("found existing matching keyboard; adding to group", .{}); - - if (!group.wlr_group.addKeyboard(device.wlr_device.toKeyboard())) { - // wlroots logs an error message to explain why this failed. - continue; - } - } - - // Continue, because we may have more than one device with the exact - // same identifier. That is in fact one reason for the keyboard group - // feature to exist in the first place. - } -} - -pub fn removeIdentifier(group: *KeyboardGroup, id: []const u8) !void { - for (group.globs.items, 0..) |glob, index| { - if (mem.eql(u8, glob, id)) { - _ = group.globs.orderedRemove(index); - break; - } - } else { - return; - } - - var it = server.input_manager.devices.iterator(.forward); - while (it.next()) |device| { - if (device.seat != group.seat) continue; - if (device.wlr_device.type != .keyboard) continue; - - if (globber.match(device.identifier, id)) { - const wlr_keyboard = device.wlr_device.toKeyboard(); - assert(wlr_keyboard.group == group.wlr_group); - group.wlr_group.removeKeyboard(wlr_keyboard); - } - } -} diff --git a/river/Seat.zig b/river/Seat.zig index bb8e827..0dc0376 100644 --- a/river/Seat.zig +++ b/river/Seat.zig @@ -33,7 +33,6 @@ const InputDevice = @import("InputDevice.zig"); const InputManager = @import("InputManager.zig"); const InputRelay = @import("InputRelay.zig"); const Keyboard = @import("Keyboard.zig"); -const KeyboardGroup = @import("KeyboardGroup.zig"); const LayerSurface = @import("LayerSurface.zig"); const LockSurface = @import("LockSurface.zig"); const Mapping = @import("Mapping.zig"); @@ -84,7 +83,7 @@ mapping_repeat_timer: *wl.EventSource, /// Currently repeating mapping, if any repeating_mapping: ?*const Mapping = null, -keyboard_groups: std.TailQueue(KeyboardGroup) = .{}, +keyboard_group: *wlr.KeyboardGroup, /// Currently focused output. Null only when there are no outputs at all. focused_output: ?*Output = null, @@ -121,12 +120,15 @@ pub fn init(seat: *Seat, name: [*:0]const u8) !void { .cursor = undefined, .relay = undefined, .mapping_repeat_timer = mapping_repeat_timer, + .keyboard_group = try wlr.KeyboardGroup.create(), }; seat.wlr_seat.data = @intFromPtr(seat); try seat.cursor.init(seat); seat.relay.init(); + try seat.tryAddDevice(&seat.keyboard_group.keyboard.base); + seat.wlr_seat.events.request_set_selection.add(&seat.request_set_selection); seat.wlr_seat.events.request_start_drag.add(&seat.request_start_drag); seat.wlr_seat.events.start_drag.add(&seat.start_drag); @@ -142,9 +144,7 @@ pub fn deinit(seat: *Seat) void { seat.cursor.deinit(); seat.mapping_repeat_timer.remove(); - while (seat.keyboard_groups.first) |node| { - node.data.destroy(); - } + seat.keyboard_group.destroy(); seat.request_set_selection.link.remove(); seat.request_start_drag.link.remove(); diff --git a/river/command/keyboard_group.zig b/river/command/keyboard_group.zig index 442fbdc..95a8153 100644 --- a/river/command/keyboard_group.zig +++ b/river/command/keyboard_group.zig @@ -24,77 +24,13 @@ const util = @import("../util.zig"); const Error = @import("../command.zig").Error; const Seat = @import("../Seat.zig"); -const KeyboardGroup = @import("../KeyboardGroup.zig"); -pub fn keyboardGroupCreate( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 2) return Error.NotEnoughArguments; - if (args.len > 2) return Error.TooManyArguments; +pub const keyboardGroupCreate = keyboardGroupDeprecated; +pub const keyboardGroupDestroy = keyboardGroupDeprecated; +pub const keyboardGroupAdd = keyboardGroupDeprecated; +pub const keyboardGroupRemove = keyboardGroupDeprecated; - if (keyboardGroupFromName(seat, args[1]) != null) { - const msg = try util.gpa.dupe(u8, "error: failed to create keybaord group: group of same name already exists\n"); - out.* = msg; - return; - } - - try KeyboardGroup.create(seat, args[1]); -} - -pub fn keyboardGroupDestroy( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 2) return Error.NotEnoughArguments; - if (args.len > 2) return Error.TooManyArguments; - const group = keyboardGroupFromName(seat, args[1]) orelse { - const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n"); - out.* = msg; - return; - }; - group.destroy(); -} - -pub fn keyboardGroupAdd( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 3) return Error.NotEnoughArguments; - if (args.len > 3) return Error.TooManyArguments; - - const group = keyboardGroupFromName(seat, args[1]) orelse { - const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n"); - out.* = msg; - return; - }; - try globber.validate(args[2]); - try group.addIdentifier(args[2]); -} - -pub fn keyboardGroupRemove( - seat: *Seat, - args: []const [:0]const u8, - out: *?[]const u8, -) Error!void { - if (args.len < 3) return Error.NotEnoughArguments; - if (args.len > 3) return Error.TooManyArguments; - - const group = keyboardGroupFromName(seat, args[1]) orelse { - const msg = try util.gpa.dupe(u8, "error: no keyboard group with that name exists\n"); - out.* = msg; - return; - }; - try group.removeIdentifier(args[2]); -} - -fn keyboardGroupFromName(seat: *Seat, name: []const u8) ?*KeyboardGroup { - var it = seat.keyboard_groups.first; - while (it) |node| : (it = node.next) { - if (mem.eql(u8, node.data.name, name)) return &node.data; - } - return null; +fn keyboardGroupDeprecated(_: *Seat, _: []const [:0]const u8, out: *?[]const u8) Error!void { + out.* = try util.gpa.dupe(u8, "warning: explicit keyboard groups are deprecated, " ++ + "all keyboards are now automatically added to a single group\n"); }