From 6a028639b84ea7c6e98f7831876457e64d54cb6a Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Fri, 30 Dec 2022 23:11:04 +0100 Subject: [PATCH] Config: use a single xkb keymap for all keyboards This is nice simplification and allows us to abort startup if the default xkb configuration (perhaps influenced by XKB_DEFAULT_* environment variables) is invalid. --- river/Config.zig | 22 ++++++++++++++++------ river/Keyboard.zig | 12 ++---------- river/Seat.zig | 3 --- river/command/keyboard.zig | 36 ++++++++++++++++++------------------ river/util.zig | 6 ------ 5 files changed, 36 insertions(+), 43 deletions(-) diff --git a/river/Config.zig b/river/Config.zig index de8474b..5b5e387 100644 --- a/river/Config.zig +++ b/river/Config.zig @@ -102,13 +102,24 @@ cursor_hide_timeout: u31 = 0, /// Hide the cursor while typing cursor_hide_when_typing: HideCursorWhenTypingMode = .disabled, -/// Keyboard layout configuration -keyboard_layout: ?xkb.RuleNames = null, +xkb_context: *xkb.Context, +/// The xkb keymap used for all keyboards +keymap: *xkb.Keymap, pub fn init() !Self { + const xkb_context = xkb.Context.new(.no_flags) orelse return error.XkbContextFailed; + defer xkb_context.unref(); + + // Passing null here indicates that defaults from libxkbcommon and + // its XKB_DEFAULT_LAYOUT, XKB_DEFAULT_OPTIONS, etc. should be used. + const keymap = xkb.Keymap.newFromNames(xkb_context, null, .no_flags) orelse return error.XkbKeymapFailed; + defer keymap.unref(); + var self = Self{ .mode_to_id = std.StringHashMap(u32).init(util.gpa), .modes = try std.ArrayListUnmanaged(Mode).initCapacity(util.gpa, 2), + .xkb_context = xkb_context.ref(), + .keymap = keymap.ref(), }; errdefer self.deinit(); @@ -134,10 +145,6 @@ pub fn deinit(self: *Self) void { for (self.modes.items) |*mode| mode.deinit(); self.modes.deinit(util.gpa); - if (self.keyboard_layout) |kl| { - util.free_xkb_rule_names(kl); - } - { var it = self.float_filter_app_ids.keyIterator(); while (it.next()) |key| util.gpa.free(key.*); @@ -163,6 +170,9 @@ pub fn deinit(self: *Self) void { } util.gpa.free(self.default_layout_namespace); + + self.keymap.unref(); + self.xkb_context.unref(); } pub fn shouldFloat(self: Self, view: *View) bool { diff --git a/river/Keyboard.zig b/river/Keyboard.zig index 4827ffa..55efde8 100644 --- a/river/Keyboard.zig +++ b/river/Keyboard.zig @@ -46,19 +46,11 @@ pub fn init(self: *Self, seat: *Seat, wlr_device: *wlr.InputDevice) !void { try self.device.init(seat, wlr_device); errdefer self.device.deinit(); - const context = xkb.Context.new(.no_flags) orelse return error.XkbContextFailed; - defer context.unref(); - - // Passing null here indicates that defaults from libxkbcommon and - // its XKB_DEFAULT_LAYOUT, XKB_DEFAULT_OPTIONS, etc. should be used. - const layout_config = if (server.config.keyboard_layout) |kl| &kl else null; - const keymap = xkb.Keymap.newFromNames(context, layout_config, .no_flags) orelse return error.XkbKeymapFailed; - defer keymap.unref(); - const wlr_keyboard = self.device.wlr_device.toKeyboard(); wlr_keyboard.data = @ptrToInt(self); - if (!wlr_keyboard.setKeymap(keymap)) return error.SetKeymapFailed; + // wlroots will log a more detailed error if this fails. + if (!wlr_keyboard.setKeymap(server.config.keymap)) return error.OutOfMemory; wlr_keyboard.setRepeatInfo(server.config.repeat_rate, server.config.repeat_delay); diff --git a/river/Seat.zig b/river/Seat.zig index ec18662..9518864 100644 --- a/river/Seat.zig +++ b/river/Seat.zig @@ -464,9 +464,6 @@ fn handleMappingRepeatTimeout(self: *Self) callconv(.C) c_int { pub fn addDevice(self: *Self, wlr_device: *wlr.InputDevice) void { self.tryAddDevice(wlr_device) catch |err| switch (err) { error.OutOfMemory => log.err("out of memory", .{}), - error.XkbContextFailed => log.err("failed to create xkbcommon context", .{}), - error.XkbKeymapFailed => log.err("failed to create xkbcommon keymap", .{}), - error.SetKeymapFailed => log.err("failed to set wlroots keymap", .{}), }; } diff --git a/river/command/keyboard.zig b/river/command/keyboard.zig index bfb7e26..5e0d9b5 100644 --- a/river/command/keyboard.zig +++ b/river/command/keyboard.zig @@ -42,31 +42,31 @@ pub fn keyboardLayout( if (result.args.len < 1) return Error.NotEnoughArguments; if (result.args.len > 1) return Error.TooManyArguments; - const new_layout = xkb.RuleNames{ - .layout = try util.gpa.dupeZ(u8, result.args[0]), - .rules = if (result.flags.rules) |s| try util.gpa.dupeZ(u8, s) else null, - .model = if (result.flags.model) |s| try util.gpa.dupeZ(u8, s) else null, - .variant = if (result.flags.variant) |s| try util.gpa.dupeZ(u8, s) else null, - .options = if (result.flags.options) |s| try util.gpa.dupeZ(u8, s) else null, + const rule_names = xkb.RuleNames{ + .layout = result.args[0], + // TODO(zig) these should coerce without this hack with the selfhosted compiler. + .rules = if (result.flags.rules) |s| s else null, + .model = if (result.flags.model) |s| s else null, + .variant = if (result.flags.variant) |s| s else null, + .options = if (result.flags.options) |s| s else null, }; - errdefer util.free_xkb_rule_names(new_layout); - const context = xkb.Context.new(.no_flags) orelse return error.OutOfMemory; - defer context.unref(); + const new_keymap = xkb.Keymap.newFromNames( + server.config.xkb_context, + &rule_names, + .no_flags, + ) orelse return error.InvalidValue; + defer new_keymap.unref(); - const keymap = xkb.Keymap.newFromNames(context, &new_layout, .no_flags) orelse return error.InvalidValue; - defer keymap.unref(); - - // Wait until after successfully creating the keymap to save the new layout options. - // Otherwise we may store invalid layout options which could cause keyboards to become - // unusable. - if (server.config.keyboard_layout) |old_layout| util.free_xkb_rule_names(old_layout); - server.config.keyboard_layout = new_layout; + server.config.keymap.unref(); + server.config.keymap = new_keymap.ref(); var it = server.input_manager.devices.iterator(.forward); while (it.next()) |device| { if (device.wlr_device.type != .keyboard) continue; const wlr_keyboard = device.wlr_device.toKeyboard(); - if (!wlr_keyboard.setKeymap(keymap)) return error.OutOfMemory; + // wlroots will log an error if this fails and there's unfortunately + // nothing we can really do in the case of failure. + _ = wlr_keyboard.setKeymap(new_keymap); } } diff --git a/river/util.zig b/river/util.zig index e568104..fd1ac74 100644 --- a/river/util.zig +++ b/river/util.zig @@ -35,9 +35,3 @@ pub fn post_fork_pre_execve() void { }; os.sigaction(os.SIG.PIPE, &sig_dfl, null); } - -pub fn free_xkb_rule_names(rule_names: xkb.RuleNames) void { - inline for (std.meta.fields(xkb.RuleNames)) |field| { - if (@field(rule_names, field.name)) |s| gpa.free(std.mem.span(s)); - } -}