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.

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),
+            );
+
+            const consumed = xkb_state.keyGetConsumedMods2(keycode, .xkb);
+            const modifiers_translated = @as(u32, @bitCast(modifiers)) & ~consumed;
 
-    // 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.
+            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, .no_translate)) {
+            if (found == null) {
+                found = mapping;
+            } else {
+                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)) {
+        if (mapping.match(keycode, modifiers, released, xkb_state, .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", .{});
             }
         }
     }