[PATCH] weston: Filter keys by id bit
Martin Minarik
minarik11 at student.fiit.stuba.sk
Thu Jun 13 04:42:32 PDT 2013
Since evdev keys are unreliable, they might randomly get dropped, such
as, on SYN_DROPPED. Even SYN_DROPPED is sometimes not delivered.
Clients, compositor are not able to recover from duplicate press/release.
Multiple wl_keyboard attached devices are not distinguished properly.
This fixes this bug, thereby making the compositor and clients useable
even under critical system conditions such as, but not limited to, high
system load, device malfunction. This improves reliability.
This fixes the compositor, so that it delivers the correct events, when attached
multiple keyboards to the same seat. This is tested under the evdev backend,
perhaps it works under multiseat X.
The way to verify this:
1. press A on keyboard 1
2. press A on keyboard 2
3. release A on keyboard 1
4. release A on keyboard 2
The actual sequence:
[2134420.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 1)
[2134421.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 1)
[2134422.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 0)
[2134423.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 0)
The correct sequence - after applying this patch:
[2134420.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 1)
[2134423.1337] -> wl_keyboard at 39.key(2316, 899782584, 30, 0)
The reason is, because the wl_keyboard device must from the client's
perspective act like single input device, as discussed.
Every seat device is assigned a different id_bit. Up to 64 devices
can be recognized per seat this way. The notify_key() will
use this bit to recognize from which device the event originated.
This is necessary because it filters keys double press double release,
improving reliability. In case multiple
This adds a second stack where keys are filtered, each time
a press or release occurs, the originating device is marked.
An event is propagated further only if the key is held or released
on 1 or 0 devices thereby giving the impression to the clients
that the wl_keyboard is 1 physical device
Disadvantages of this approach
none
---
src/compositor-wayland.c | 14 ++++++--
src/compositor-x11.c | 25 ++++++++-----
src/compositor.h | 5 ++-
src/evdev.c | 57 +++++++++++++++++++++++-------
src/evdev.h | 4 +++
src/input.c | 88 +++++++++++++++++++++++++++++++++++++---------
tests/weston-test.c | 3 +-
7 files changed, 153 insertions(+), 43 deletions(-)
diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
index f3a98a8..0304937 100644
--- a/src/compositor-wayland.c
+++ b/src/compositor-wayland.c
@@ -529,11 +529,21 @@ input_handle_keyboard_enter(void *data,
struct wl_array *keys)
{
struct wayland_input *input = data;
+ struct wl_array key_states;
+ unsigned int *k_cnt;
+
+ wl_array_init(&key_states);
+ wl_array_add(&key_states, keys->size);
+ wl_array_for_each(k_cnt, &key_states) {
+ *k_cnt = 1;
+ }
/* XXX: If we get a modifier event immediately before the focus,
* we should try to keep the same serial. */
- notify_keyboard_focus_in(&input->base, keys,
+ notify_keyboard_focus_in(&input->base, keys, &key_states,
STATE_UPDATE_AUTOMATIC);
+
+ wl_array_release(&key_states);
}
static void
@@ -557,7 +567,7 @@ input_handle_key(void *data, struct wl_keyboard *keyboard,
notify_key(&input->base, time, key,
state ? WL_KEYBOARD_KEY_STATE_PRESSED :
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_NONE);
+ STATE_UPDATE_NONE, 1);
}
static void
diff --git a/src/compositor-x11.c b/src/compositor-x11.c
index 5a0bcf0..09de5c3 100644
--- a/src/compositor-x11.c
+++ b/src/compositor-x11.c
@@ -65,7 +65,6 @@ struct x11_compositor {
xcb_connection_t *conn;
xcb_screen_t *screen;
xcb_cursor_t null_cursor;
- struct wl_array keys;
struct wl_event_source *xcb_source;
struct xkb_keymap *xkb_keymap;
unsigned int has_xkb;
@@ -1181,6 +1180,8 @@ static int
x11_compositor_handle_event(int fd, uint32_t mask, void *data)
{
struct x11_compositor *c = data;
+ struct wl_array keys;
+ struct wl_array key_states;
struct x11_output *output;
xcb_generic_event_t *event, *prev;
xcb_client_message_event_t *client_message;
@@ -1224,7 +1225,8 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
weston_compositor_get_time(),
key_release->detail - 8,
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_AUTOMATIC);
+ STATE_UPDATE_AUTOMATIC, 1);
+
free(prev);
prev = NULL;
break;
@@ -1233,13 +1235,16 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
case XCB_FOCUS_IN:
assert(response_type == XCB_KEYMAP_NOTIFY);
keymap_notify = (xcb_keymap_notify_event_t *) event;
- c->keys.size = 0;
+ wl_array_init(&keys);
+ wl_array_init(&key_states);
for (i = 0; i < ARRAY_LENGTH(keymap_notify->keys) * 8; i++) {
set = keymap_notify->keys[i >> 3] &
(1 << (i & 7));
if (set) {
- k = wl_array_add(&c->keys, sizeof *k);
+ k = wl_array_add(&keys, sizeof *k);
*k = i;
+ k = wl_array_add(&key_states, sizeof *k);
+ *k = 1;
}
}
@@ -1247,9 +1252,12 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
* event, rather than with the focus event. I'm not
* sure of the exact semantics around it and whether
* we can ensure that we get both? */
- notify_keyboard_focus_in(&c->core_seat, &c->keys,
+ notify_keyboard_focus_in(&c->core_seat, &keys, &key_states,
STATE_UPDATE_AUTOMATIC);
+ wl_array_release(&keys);
+ wl_array_release(&key_states);
+
free(prev);
prev = NULL;
break;
@@ -1269,7 +1277,7 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
key_press->detail - 8,
WL_KEYBOARD_KEY_STATE_PRESSED,
c->has_xkb ? STATE_UPDATE_NONE :
- STATE_UPDATE_AUTOMATIC);
+ STATE_UPDATE_AUTOMATIC, 1);
break;
case XCB_KEY_RELEASE:
/* If we don't have XKB, we need to use the lame
@@ -1283,7 +1291,7 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
weston_compositor_get_time(),
key_release->detail - 8,
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_NONE);
+ STATE_UPDATE_NONE, 1);
break;
case XCB_BUTTON_PRESS:
x11_compositor_deliver_button_event(c, event, 1);
@@ -1364,7 +1372,7 @@ x11_compositor_handle_event(int fd, uint32_t mask, void *data)
weston_compositor_get_time(),
key_release->detail - 8,
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_AUTOMATIC);
+ STATE_UPDATE_AUTOMATIC, 1);
free(prev);
prev = NULL;
break;
@@ -1540,7 +1548,6 @@ x11_compositor_create(struct wl_display *display,
s = xcb_setup_roots_iterator(xcb_get_setup(c->conn));
c->screen = s.data;
- wl_array_init(&c->keys);
x11_compositor_get_resources(c);
x11_compositor_get_wm_info(c);
diff --git a/src/compositor.h b/src/compositor.h
index 22700b7..6f21c89 100644
--- a/src/compositor.h
+++ b/src/compositor.h
@@ -403,6 +403,7 @@ struct weston_keyboard {
uint32_t grab_time;
struct wl_array keys;
+ struct wl_array keys_where;
struct {
uint32_t mods_depressed;
@@ -839,7 +840,8 @@ notify_axis(struct weston_seat *seat, uint32_t time, uint32_t axis,
void
notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
enum wl_keyboard_key_state state,
- enum weston_key_state_update update_state);
+ enum weston_key_state_update update_state,
+ uint64_t seat_device_bit);
void
notify_modifiers(struct weston_seat *seat, uint32_t serial);
@@ -849,6 +851,7 @@ notify_pointer_focus(struct weston_seat *seat, struct weston_output *output,
void
notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
+ struct wl_array *keys_where,
enum weston_key_state_update update_state);
void
notify_keyboard_focus_out(struct weston_seat *seat);
diff --git a/src/evdev.c b/src/evdev.c
index 122a2d9..417c266 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -88,7 +88,7 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
time, e->code,
e->value ? WL_KEYBOARD_KEY_STATE_PRESSED :
WL_KEYBOARD_KEY_STATE_RELEASED,
- STATE_UPDATE_AUTOMATIC);
+ STATE_UPDATE_AUTOMATIC, device->id_bit);
break;
}
}
@@ -520,6 +520,12 @@ evdev_configure_device(struct evdev_device *device)
return 0;
}
+void
+evdev_device_assign_id_bit(struct evdev_device *device, unsigned int id)
+{
+ device->id_bit = 1 << (id % 64);
+}
+
struct evdev_device *
evdev_device_create(struct weston_seat *seat, const char *path, int device_fd)
{
@@ -545,6 +551,7 @@ evdev_device_create(struct weston_seat *seat, const char *path, int device_fd)
device->rel.dy = 0;
device->dispatch = NULL;
device->fd = device_fd;
+ device->id_bit = 1;
ioctl(device->fd, EVIOCGNAME(sizeof(devname)), devname);
device->devname = strdup(devname);
@@ -613,40 +620,64 @@ evdev_notify_keyboard_focus(struct weston_seat *seat,
struct wl_list *evdev_devices)
{
struct evdev_device *device;
- struct wl_array keys;
+ struct wl_array keys, keys_where;
unsigned int i, set;
char evdev_keys[(KEY_CNT + 7) / 8];
char all_keys[(KEY_CNT + 7) / 8];
uint32_t *k;
- int ret;
+ uint64_t *k_where;
+ int new, ret, id = 0;
if (!seat->keyboard)
return;
+ wl_array_init(&keys_where);
+ wl_array_init(&keys);
+
memset(all_keys, 0, sizeof all_keys);
wl_list_for_each(device, evdev_devices, link) {
+ evdev_device_assign_id_bit(device, id++);
+
memset(evdev_keys, 0, sizeof evdev_keys);
ret = ioctl(device->fd,
EVIOCGKEY(sizeof evdev_keys), evdev_keys);
+
if (ret < 0) {
weston_log("failed to get keys for device %s\n",
device->devnode);
continue;
}
- for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++)
- all_keys[i] |= evdev_keys[i];
- }
- wl_array_init(&keys);
- for (i = 0; i < KEY_CNT; i++) {
- set = all_keys[i >> 3] & (1 << (i & 7));
- if (set) {
- k = wl_array_add(&keys, sizeof *k);
- *k = i;
+ for (i = 0; i < KEY_CNT; i++) {
+ new = evdev_keys[i >> 3] & (1 << (i & 7));
+ if (!new)
+ continue;
+
+ set = all_keys[i >> 3] & (1 << (i & 7));
+
+ if (set) {
+ k_where = keys_where.data;
+ wl_array_for_each(k, &keys) {
+ if (*k == i)
+ break;
+ k_where++;
+ }
+ *k_where |= device->id_bit;
+ } else {
+ k = wl_array_add(&keys, sizeof *k);
+ k_where = wl_array_add(&keys_where, sizeof *k_where);
+ *k = i;
+ *k_where = device->id_bit;
+ }
}
+
+ for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++)
+ all_keys[i] |= evdev_keys[i];
}
- notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC);
+ notify_keyboard_focus_in(seat, &keys, &keys_where, STATE_UPDATE_AUTOMATIC);
wl_array_release(&keys);
+ wl_array_release(&keys_where);
}
+
diff --git a/src/evdev.h b/src/evdev.h
index eb5c868..acf02a2 100644
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -77,6 +77,7 @@ struct evdev_device {
enum evdev_device_capability caps;
int is_mt;
+ uint64_t id_bit;
};
/* copied from udev/extras/input_id/input_id.c */
@@ -114,6 +115,9 @@ evdev_touchpad_create(struct evdev_device *device);
void
evdev_led_update(struct evdev_device *device, enum weston_led leds);
+void
+evdev_device_assign_id_bit(struct evdev_device *device, unsigned int id);
+
struct evdev_device *
evdev_device_create(struct weston_seat *seat, const char *path, int device_fd);
diff --git a/src/input.c b/src/input.c
index d299d98..a28b5eb 100644
--- a/src/input.c
+++ b/src/input.c
@@ -357,6 +357,7 @@ weston_keyboard_create(void)
memset(keyboard, 0, sizeof *keyboard);
wl_list_init(&keyboard->resource_list);
wl_array_init(&keyboard->keys);
+ wl_array_init(&keyboard->keys_where);
keyboard->focus_listener.notify = lose_keyboard_focus;
keyboard->default_grab.interface = &default_keyboard_grab_interface;
keyboard->default_grab.keyboard = keyboard;
@@ -373,6 +374,7 @@ weston_keyboard_destroy(struct weston_keyboard *keyboard)
if (keyboard->focus_resource)
wl_list_remove(&keyboard->focus_listener.link);
wl_array_release(&keyboard->keys);
+ wl_array_release(&keyboard->keys_where);
free(keyboard);
}
@@ -827,18 +829,81 @@ update_modifier_state(struct weston_seat *seat, uint32_t serial, uint32_t key,
notify_modifiers(seat, serial);
}
+static uint64_t
+update_key_state(struct weston_keyboard *keyboard, uint64_t seat_device_bit,
+ uint32_t key, enum wl_keyboard_key_state state)
+{
+ uint32_t *k;
+ uint64_t *k_where;
+
+ const uint32_t *end = keyboard->keys.data + keyboard->keys.size;
+ const uint64_t *end_where = keyboard->keys_where.data + keyboard->keys_where.size;
+
+ k_where = keyboard->keys_where.data;
+
+ wl_array_for_each(k, &keyboard->keys) {
+ if (*k == key)
+ break;
+ k_where++;
+ }
+
+ if (k >= end) {
+ if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
+ goto err_multi_release;
+ k = wl_array_add(&keyboard->keys, sizeof *k);
+ k_where = wl_array_add(&keyboard->keys_where, sizeof *k_where);
+ *k = key;
+ *k_where = seat_device_bit;
+ return seat_device_bit;
+ } else {
+ if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
+ if (seat_device_bit & *k_where)
+ goto err_multi_press;
+ } else {
+ if (!(seat_device_bit & *k_where))
+ goto err_multi_release;
+ }
+
+ *k_where = seat_device_bit ^ *k_where;
+ if (0 == *k_where) {
+ *k = *(end-1);
+ *k_where = *(end_where-1);
+ keyboard->keys.size -= sizeof *k;
+ keyboard->keys_where.size -= sizeof *k_where;
+ return 0;
+ }
+
+ return *k_where;
+ }
+
+err_multi_release:
+ return 0xFFFFFFFF;
+err_multi_press:
+ return 0;
+}
+
WL_EXPORT void
notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
enum wl_keyboard_key_state state,
- enum weston_key_state_update update_state)
+ enum weston_key_state_update update_state,
+ uint64_t seat_device_bit)
{
struct weston_compositor *compositor = seat->compositor;
struct weston_keyboard *keyboard = seat->keyboard;
struct weston_surface *focus =
(struct weston_surface *) keyboard->focus;
struct weston_keyboard_grab *grab = keyboard->grab;
- uint32_t serial = wl_display_next_serial(compositor->wl_display);
- uint32_t *k, *end;
+ uint32_t serial;
+ uint64_t held_where = update_key_state(keyboard, seat_device_bit, key, state);
+ uint64_t held_multiple = held_where & (held_where - 1);
+
+ if (state == !held_where)
+ return;
+
+ if (held_multiple)
+ return;
+
+ serial = wl_display_next_serial(compositor->wl_display);
if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
if (compositor->ping_handler && focus)
@@ -851,20 +916,6 @@ notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
weston_compositor_idle_release(compositor);
}
- end = keyboard->keys.data + keyboard->keys.size;
- for (k = keyboard->keys.data; k < end; k++) {
- if (*k == key) {
- /* Ignore server-generated repeats. */
- if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
- return;
- *k = *--end;
- }
- }
- keyboard->keys.size = (void *) end - keyboard->keys.data;
- if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
- k = wl_array_add(&keyboard->keys, sizeof *k);
- *k = key;
- }
if (grab == &keyboard->default_grab ||
grab == &keyboard->input_method_grab) {
@@ -912,6 +963,7 @@ destroy_device_saved_kbd_focus(struct wl_listener *listener, void *data)
WL_EXPORT void
notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
+ struct wl_array *keys_where,
enum weston_key_state_update update_state)
{
struct weston_compositor *compositor = seat->compositor;
@@ -920,6 +972,8 @@ notify_keyboard_focus_in(struct weston_seat *seat, struct wl_array *keys,
uint32_t *k, serial;
serial = wl_display_next_serial(compositor->wl_display);
+
+ wl_array_copy(&keyboard->keys_where, keys_where);
wl_array_copy(&keyboard->keys, keys);
wl_array_for_each(k, &keyboard->keys) {
weston_compositor_idle_inhibit(compositor);
diff --git a/tests/weston-test.c b/tests/weston-test.c
index b629c86..872446c 100644
--- a/tests/weston-test.c
+++ b/tests/weston-test.c
@@ -156,6 +156,7 @@ activate_surface(struct wl_client *client, struct wl_resource *resource,
if (surface) {
weston_surface_activate(surface, seat);
notify_keyboard_focus_in(seat, &seat->keyboard->keys,
+ &seat->keyboard->keys_where,
STATE_UPDATE_AUTOMATIC);
}
else {
@@ -173,7 +174,7 @@ send_key(struct wl_client *client, struct wl_resource *resource,
test->compositor->focus = 1;
- notify_key(seat, 100, key, state, STATE_UPDATE_AUTOMATIC);
+ notify_key(seat, 100, key, state, STATE_UPDATE_AUTOMATIC, 1);
}
static const struct wl_test_interface test_implementation = {
--
1.7.10.4
More information about the wayland-devel
mailing list