From ec8f57e704f802eaa98275785f66618a7652fd11 Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Thu, 11 Jan 2024 15:06:33 -0600 Subject: [PATCH] Keyboard: check translated keysyms for mappings If our current approch without xkbcommon translation does not match any mapping on a key event attempt to match the translated keysym as well. This makes e.g. the keypad number keys (e.g. KP_1) work intuitively as they may require translation with numlock active. The reason we stopped doing this in I7c02ebcbc was due to layout where e.g. Super+Shift+Space is translated as Space with the Shift modifier consumed, thereby conflicting with a separate mapping for Super+Space. This should not be a issue anymore though as we now only run a maximum of one mapping per key event and we attemt to match mappings without xkbcommon translation before attempting with translation. --- river/Mapping.zig | 57 +++++++++++++++++++++++++++++++---------------- river/Seat.zig | 36 +++++++++++++++++++++++++----- 2 files changed, 68 insertions(+), 25 deletions(-) diff --git a/river/Mapping.zig b/river/Mapping.zig index 0805d51..e769181 100644 --- a/river/Mapping.zig +++ b/river/Mapping.zig @@ -69,6 +69,7 @@ pub fn match( modifiers: wlr.Keyboard.ModifierMask, released: bool, xkb_state: *xkb.State, + method: enum { no_translate, translate }, ) bool { if (released != self.options.release) return false; @@ -79,28 +80,46 @@ pub fn match( // will fall back to the active layout if so. const layout_index = self.options.layout_index orelse xkb_state.keyGetLayout(keycode); - // Get keysyms from the base layer, as if modifiers didn't change keysyms. - // E.g. pressing `Super+Shift 1` does not translate to `Super Exclam`. - const keysyms = keymap.keyGetSymsByLevel( - keycode, - layout_index, - 0, - ); + switch (method) { + .no_translate => { + // Get keysyms from the base layer, as if modifiers didn't change keysyms. + // E.g. pressing `Super+Shift 1` does not translate to `Super Exclam`. + const keysyms = keymap.keyGetSymsByLevel( + keycode, + layout_index, + 0, + ); - if (std.meta.eql(modifiers, self.modifiers)) { - for (keysyms) |sym| { - if (sym == self.keysym) { - return true; + if (@as(u32, @bitCast(modifiers)) == @as(u32, @bitCast(self.modifiers))) { + for (keysyms) |sym| { + if (sym == self.keysym) { + return true; + } + } } - } - } + }, + .translate => { + // Keysyms and modifiers as translated by xkb. + // Modifiers used to translate the key are consumed. + // E.g. pressing `Super+Shift 1` translates to `Super Exclam`. + const keysyms_translated = keymap.keyGetSymsByLevel( + keycode, + layout_index, + xkb_state.keyGetLevel(keycode, layout_index), + ); - // We deliberately choose not to translate keysyms and modifiers with xkb, - // because of strange behavior that xkb shows for some layouts and keys. - // When pressing `Shift Space` on some layouts (Swedish among others), - // xkb reports `Shift` as consumed. This leads to the case that we cannot - // distinguish between `Space` and `Shift Space` presses when doing a - // correct translation with xkb. + const consumed = xkb_state.keyGetConsumedMods2(keycode, .xkb); + const modifiers_translated = @as(u32, @bitCast(modifiers)) & ~consumed; + + if (modifiers_translated == @as(u32, @bitCast(self.modifiers))) { + for (keysyms_translated) |sym| { + if (sym == self.keysym) { + return true; + } + } + } + }, + } return false; } diff --git a/river/Seat.zig b/river/Seat.zig index e90b0c8..03d6968 100644 --- a/river/Seat.zig +++ b/river/Seat.zig @@ -359,7 +359,7 @@ pub fn enterMode(self: *Self, mode_id: u32) void { } /// Handle any user-defined mapping for passed keycode, modifiers and keyboard state -/// Returns true if at least one mapping was run +/// Returns true if a mapping was run pub fn handleMapping( self: *Self, keycode: xkb.Keycode, @@ -368,16 +368,40 @@ pub fn handleMapping( xkb_state: *xkb.State, ) bool { const modes = &server.config.modes; - // It is possible for more than one mapping to be matched due to the existence of - // layout-independent mappings. In this case run the first match and log a warning - // if there are other matches. + + // It is possible for more than one mapping to be matched due to the + // existence of layout-independent mappings. It is also possible due to + // translation by xkbcommon consuming modifiers. On the swedish layout + // for example, translating Super+Shift+Space may consume the Shift + // modifier and confict with a mapping for Super+Space. For this reason, + // matching wihout xkbcommon translation is done first and after a match + // has been found all further matches are ignored. var found: ?*Mapping = null; + + // First check for matches without translating keysyms with xkbcommon. + // That is, if the physical keys Mod+Shift+1 are pressed on a US layout don't + // translate the keysym 1 to an exclamation mark. This behavior is generally + // what is desired. for (modes.items[self.mode_id].mappings.items) |*mapping| { - if (mapping.match(keycode, modifiers, released, xkb_state)) { + if (mapping.match(keycode, modifiers, released, xkb_state, .no_translate)) { if (found == null) { found = mapping; } else { - log.warn("already found a matching mapping, ignoring additional match", .{}); + log.debug("already found a matching mapping, ignoring additional match", .{}); + } + } + } + + // There are however some cases where it is necessary to translate keysyms + // with xkbcommon for intuitive behavior. For example, layouts may require + // translation with the numlock modifier to obtain keypad number keysyms + // (e.g. KP_1). + for (modes.items[self.mode_id].mappings.items) |*mapping| { + if (mapping.match(keycode, modifiers, released, xkb_state, .translate)) { + if (found == null) { + found = mapping; + } else { + log.debug("already found a matching mapping, ignoring additional match", .{}); } } }