uterm: input: uxkb: properly handle changed keyboard state on wake up

Fix the problems caused by the keyboard state getting out-of-sync with
the xkb_state while the device is asleep (instead of the current hack).
The problem is described in a comment and in the removed FIXME.

Signed-off-by: Ran Benita <ran234@gmail.com>
Signed-off-by: David Herrmann <dh.herrmann@googlemail.com>

diff --git a/src/shl_misc.h b/src/shl_misc.h
index f05434d..f53beae 100644 (file)
--- a/src/shl_misc.h
+++ b/src/shl_misc.h
@@ -39,6 +39,7 @@
 #include <xkbcommon/xkbcommon.h>
 
 #define SHL_HAS_BITS(_bitmask, _bits) (((_bitmask) & (_bits)) == (_bits))
+#define SHL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
 
 static inline int shl_strtou(const char *input, unsigned int *output)
 {

diff --git a/src/uterm_input.c b/src/uterm_input.c
index 49719ad..ae6f1a2 100644 (file)
--- a/src/uterm_input.c
+++ b/src/uterm_input.c
@@ -110,8 +110,7 @@ static int input_wake_up_dev(struct uterm_input_dev *dev)
                return -EFAULT;
        }
 
-       /* rediscover the keyboard state if sth changed during sleep */
-       uxkb_dev_reset(dev);
+       uxkb_dev_wake_up(dev);
 
        ret = ev_eloop_new_fd(dev->input->eloop, &dev->fd, dev->rfd,
                              EV_READABLE, input_data_dev, dev);
@@ -129,6 +128,8 @@ static void input_sleep_dev(struct uterm_input_dev *dev)
        if (dev->rfd < 0)
                return;
 
+       uxkb_dev_sleep(dev);
+
        dev->repeating = false;
        ev_timer_update(dev->repeat_timer, NULL);
        ev_eloop_rm_fd(dev->fd);

diff --git a/src/uterm_input.h b/src/uterm_input.h
index bd04302..8800397 100644 (file)
--- a/src/uterm_input.h
+++ b/src/uterm_input.h
@@ -35,6 +35,7 @@
 #include <xkbcommon/xkbcommon-keysyms.h>
 #include "eloop.h"
 #include "shl_dlist.h"
+#include "shl_misc.h"
 #include "uterm.h"
 
 enum uterm_input_device_capability {
@@ -51,6 +52,8 @@ struct uterm_input_dev {
        char *node;
        struct ev_fd *fd;
        struct xkb_state *state;
+       /* Used in sleep/wake up to store the key's pressed/released state. */
+       char key_state_bits[SHL_DIV_ROUND_UP(KEY_CNT, CHAR_BIT)];
 
        unsigned int num_syms;
        struct uterm_input_event event;
@@ -91,6 +94,7 @@ void uxkb_dev_destroy(struct uterm_input_dev *dev);
 int uxkb_dev_process(struct uterm_input_dev *dev,
                     uint16_t key_state,
                     uint16_t code);
-void uxkb_dev_reset(struct uterm_input_dev *dev);
+void uxkb_dev_sleep(struct uterm_input_dev *dev);
+void uxkb_dev_wake_up(struct uterm_input_dev *dev);
 
 #endif /* UTERM_INPUT_H */

diff --git a/src/uterm_input_uxkb.c b/src/uterm_input_uxkb.c
index 1bf48ac..0be66fa 100644 (file)
--- a/src/uterm_input_uxkb.c
+++ b/src/uterm_input_uxkb.c
@@ -339,28 +339,58 @@ int uxkb_dev_process(struct uterm_input_dev *dev,
        return 0;
 }
 
-/*
- * Call this when we regain control of the keyboard after losing it.
- * We don't reset the locked group, this should survive a VT switch, etc.
- */
-void uxkb_dev_reset(struct uterm_input_dev *dev)
+void uxkb_dev_sleep(struct uterm_input_dev *dev)
 {
-       struct xkb_state *state;
+       /*
+        * While the device is asleep, we don't receive key events. This
+        * means that when we wake up, the keyboard state may be different
+        * (e.g. some key is pressed but we don't know about it). This can
+        * cause various problems, like stuck modifiers: consider if we
+        * miss a release of the left Shift key. When the user presses it
+        * again, xkb_state_update_key() will think there is *another* left
+        * Shift key that was pressed. When the key is released, it's as if
+        * this "second" key was released, but the "first" is still left
+        * pressed.
+        * To handle this, when the device goes to sleep, we save our
+        * current knowledge of the keyboard's press/release state. On wake
+        * up, we compare the states before and after, and just feed
+        * xkb_state_update_key() the deltas.
+        */
+       memset(dev->key_state_bits, 0, sizeof(dev->key_state_bits));
+       errno = 0;
+       ioctl(dev->rfd, EVIOCGKEY(sizeof(dev->key_state_bits)),
+             dev->key_state_bits);
+       if (errno)
+               log_warn("failed to save keyboard state (%d): %m", errno);
+}
 
-       /* TODO: Urghs, while the input device was closed we might have missed
-        * some events that affect internal state. As xkbcommon does not provide
-        * a way to reset the internal state, we simply recreate the state. This
-        * should have the same effect.
-        * It also has a bug that if the CTRL-Release event is skipped, then
-        * every further release will never perform a _real_ release. Kind of
-        * buggy so we should fix it upstream. */
-       state = xkb_state_new(dev->input->keymap);
-       if (!state) {
-               log_warning("cannot recreate xkb-state");
+void uxkb_dev_wake_up(struct uterm_input_dev *dev)
+{
+       uint32_t code;
+       char *old_bits, cur_bits[sizeof(dev->key_state_bits)];
+       char old_bit, cur_bit;
+
+       old_bits = dev->key_state_bits;
+
+       memset(cur_bits, 0, sizeof(cur_bits));
+       errno = 0;
+       ioctl(dev->rfd, EVIOCGKEY(sizeof(cur_bits)), cur_bits);
+       if (errno) {
+               log_warn("failed to get current keyboard state (%d): %m",
+                        errno);
                return;
        }
-       xkb_state_unref(dev->state);
-       dev->state = state;
+
+       for (code = 0; code < KEY_CNT; code++) {
+               old_bit = (old_bits[code / 8] & (1 << (code % 8)));
+               cur_bit = (cur_bits[code / 8] & (1 << (code % 8)));
+
+               if (old_bit == cur_bit)
+                       continue;
+
+               xkb_state_update_key(dev->state, code + EVDEV_KEYCODE_OFFSET,
+                                    cur_bit ? XKB_KEY_DOWN : XKB_KEY_UP);
+       }
 
        uxkb_dev_update_keyboard_leds(dev);
 }