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

@@ -102,13 +102,24 @@ cursor_hide_timeout: u31 = 0,
 /// Hide the cursor while typing
 cursor_hide_when_typing: HideCursorWhenTypingMode = .disabled,
 
-/// Keyboard layout configuration
+xkb_context: *xkb.Context,
-keyboard_layout: ?xkb.RuleNames = null,
+/// 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 {

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);
 

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", .{}),
     };
 }
 

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{
+    const rule_names = xkb.RuleNames{
-        .layout = try util.gpa.dupeZ(u8, result.args[0]),
+        .layout = result.args[0],
-        .rules = if (result.flags.rules) |s| try util.gpa.dupeZ(u8, s) else null,
+        // TODO(zig) these should coerce without this hack with the selfhosted compiler.
-        .model = if (result.flags.model) |s| try util.gpa.dupeZ(u8, s) else null,
+        .rules = if (result.flags.rules) |s| s else null,
-        .variant = if (result.flags.variant) |s| try util.gpa.dupeZ(u8, s) else null,
+        .model = if (result.flags.model) |s| s else null,
-        .options = if (result.flags.options) |s| try util.gpa.dupeZ(u8, 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;
+    const new_keymap = xkb.Keymap.newFromNames(
-    defer context.unref();
+        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;
+    server.config.keymap.unref();
-    defer keymap.unref();
+    server.config.keymap = new_keymap.ref();
-
-    // 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;
 
     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);
     }
 }

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));
-    }
-}