[PATCH weston] evdev: Handle SYN_DROPPED, atomically count buttons
Giulio Camuffo
giuliocamuffo at gmail.com
Wed Mar 27 03:50:07 PDT 2013
Hi Martin,
Apart some style issues this seems good to me, but i'ts the first time
i look into evdev.c, so i might have missed something.
I could not reproduce the two issues you describe, with 20 mpv
instances running a full hd movie in the background.
2013/3/27 Martin Minarik <minarik11 at student.fiit.stuba.sk>:
> When the kernel event queue overruns, the evdev.c will:
> 1. Skip events until and up to the next SYN_REPORT
> 2. Count depressed grab buttons on seat devices.
> 3. If the depressed grab button count is different, update
> it atomically. If it changed to zero, call grab cancelation.
>
> Potential pitfalls:
> a) During the testing, button isn't counted properly on one device.
> This causes grab resets when the system is under heavy load. Under
> normal operation it's ok.
> b) A mysterious notify_button() call that didn't arrive via evdev. (?)
>
> The evdev_notify_keyboard_focus() has been reworked to make the
> similiarity obvious. (still works exactly as before). The functions
> are now almost the same:
>
> evdev_keys_states_count()
> evdev_keys_states_query()
> ---
> src/compositor.c | 21 ++++++--
> src/compositor.h | 6 ++-
> src/evdev.c | 145 +++++++++++++++++++++++++++++++++++++++++++++---------
> src/evdev.h | 15 ++++++
> 4 files changed, 158 insertions(+), 29 deletions(-)
>
> diff --git a/src/compositor.c b/src/compositor.c
> index d6a45ba..580366c 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1715,7 +1715,15 @@ weston_compositor_idle_inhibit(struct weston_compositor *compositor)
> static void
> weston_compositor_idle_release(struct weston_compositor *compositor)
> {
> - compositor->idle_inhibit--;
> + if (compositor->idle_inhibit)
I'd do "if (compositor->idle_inhibit > 0)" here, to be more consistent
with places where you do "if (some_var == $n)", and to make the
purpose more evident. There is another occurrence below.
> + compositor->idle_inhibit--;
> + weston_compositor_wake(compositor);
> +}
> +
> +WL_EXPORT void
> +weston_compositor_idle_reset(struct weston_compositor *compositor)
> +{
> + compositor->idle_inhibit = 0;
> weston_compositor_wake(compositor);
> }
>
> @@ -1867,7 +1875,7 @@ weston_surface_activate(struct weston_surface *surface,
> }
>
> WL_EXPORT void
> -notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
> +notify_button(struct weston_seat *seat, uint32_t time, uint32_t button,
> enum wl_pointer_button_state state)
> {
> struct weston_compositor *compositor = seat->compositor;
> @@ -1886,10 +1894,12 @@ notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
> pointer->grab_x = pointer->x;
> pointer->grab_y = pointer->y;
> }
> - pointer->button_count++;
> + if (button == pointer->grab_button)
> + pointer->button_count++;
> } else {
> weston_compositor_idle_release(compositor);
> - pointer->button_count--;
> + if ((button == pointer->grab_button) && pointer->button_count)
> + pointer->button_count--;
> }
>
> weston_compositor_run_button_binding(compositor, seat, time, button,
> @@ -1897,7 +1907,8 @@ notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
>
> pointer->grab->interface->button(pointer->grab, time, button, state);
>
> - if (pointer->button_count == 1)
> + if ((pointer->button_count == 1) &&
> + (state == WL_POINTER_BUTTON_STATE_PRESSED))
> pointer->grab_serial =
> wl_display_get_serial(compositor->wl_display);
> }
> diff --git a/src/compositor.h b/src/compositor.h
> index 48ec57c..777596c 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -570,7 +570,9 @@ void
> notify_motion(struct weston_seat *seat, uint32_t time,
> wl_fixed_t x, wl_fixed_t y);
> void
> -notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
> +notify_lag(struct weston_seat *seat, uint32_t time);
> +void
> +notify_button(struct weston_seat *seat, uint32_t time, uint32_t button,
> enum wl_pointer_button_state state);
> void
> notify_axis(struct weston_seat *seat, uint32_t time, uint32_t axis,
> @@ -693,6 +695,8 @@ weston_compositor_run_debug_binding(struct weston_compositor *compositor,
> struct weston_seat *seat, uint32_t time,
> uint32_t key,
> enum wl_keyboard_key_state state);
> +void
> +weston_compositor_idle_reset(struct weston_compositor *compositor);
>
> int
> weston_environment_get_fd(const char *env);
> diff --git a/src/evdev.c b/src/evdev.c
> index d2954b5..7a4966c 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -60,6 +60,24 @@ evdev_led_update(struct evdev_device *device, enum weston_led leds)
> (void)i; /* no, we really don't care about the return value */
> }
>
> +struct evdev_dispatch_interface fallback_interface;
> +struct evdev_dispatch_interface syn_drop_interface;
> +
> +static inline void
> +evdev_process_syn(struct evdev_device *device, struct input_event *e, int time)
> +{
> + switch (e->code) {
> + case SYN_DROPPED:
> + if (device->dispatch->interface == &fallback_interface)
> + device->dispatch->interface = &syn_drop_interface;
> + break;
> + case SYN_REPORT:
> + default:
> + device->pending_events |= EVDEV_SYN;
> + break;
> + }
> +}
> +
> static inline void
> evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> {
> @@ -80,7 +98,6 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> e->value ? WL_POINTER_BUTTON_STATE_PRESSED :
> WL_POINTER_BUTTON_STATE_RELEASED);
> break;
> -
> default:
> notify_key(device->seat,
> time, e->code,
> @@ -308,7 +325,7 @@ fallback_process(struct evdev_dispatch *dispatch,
> evdev_process_key(device, event, time);
> break;
> case EV_SYN:
> - device->pending_events |= EVDEV_SYN;
> + evdev_process_syn(device, event, time);
> break;
> }
> }
> @@ -324,6 +341,50 @@ struct evdev_dispatch_interface fallback_interface = {
> fallback_destroy
> };
>
> +static void
> +syn_drop_process(struct evdev_dispatch *dispatch,
> + struct evdev_device *device,
> + struct input_event *event,
> + uint32_t time)
> +{
> + if ((event->code != EV_SYN) || (event->code != SYN_REPORT))
> + return;
> +
> + if (device->dispatch->interface == &syn_drop_interface)
> + device->dispatch->interface = &fallback_interface;
> +
> + struct weston_seat *seat = device->seat;
> + struct wl_pointer *pointer = seat->seat.pointer;
> + struct wl_list *devices_list = &device->link;
> +
> + uint8_t button_count_new = 0;
> + evdev_keys_states_count(devices_list, &button_count_new,
> + pointer->grab_button,
> + pointer->grab_button+1);
> +
> + if (button_count_new == pointer->button_count)
> + return;
> +
> + int stop_grab = (button_count_new == 0) && (pointer->button_count > 0);
The declaration of stop_grab should go at the top, with the others.
> +
> + weston_compositor_idle_reset(seat->compositor);
> + pointer->button_count = button_count_new;
> + if (stop_grab)
> + pointer->grab->interface->button(pointer->grab, time,
> + pointer->grab_button, WL_POINTER_BUTTON_STATE_RELEASED);
> +}
> +
> +static void
> +syn_drop_destroy(struct evdev_dispatch *dispatch)
> +{
> + free(dispatch);
> +}
> +
> +struct evdev_dispatch_interface syn_drop_interface = {
> + syn_drop_process,
> + syn_drop_destroy
> +};
> +
> static struct evdev_dispatch *
> fallback_dispatch_create(void)
> {
> @@ -607,44 +668,82 @@ evdev_device_destroy(struct evdev_device *device)
> }
>
> void
> -evdev_notify_keyboard_focus(struct weston_seat *seat,
> - struct wl_list *evdev_devices)
> +evdev_keys_states_count(struct wl_list *evdev_devices, uint8_t *keys_counts,
> + uint32_t key_min, uint32_t key_max)
> {
> struct evdev_device *device;
> - struct wl_array keys;
> - unsigned int i, set;
> - char evdev_keys[(KEY_CNT + 7) / 8];
> - char all_keys[(KEY_CNT + 7) / 8];
> - uint32_t *k;
> - int ret;
> + uint32_t key_range = key_max - key_min;
>
> - if (!seat->seat.keyboard)
> + if (key_range > KEY_MAX || key_min > KEY_CNT || key_max > KEY_MAX)
> return;
>
> - memset(all_keys, 0, sizeof all_keys);
> wl_list_for_each(device, evdev_devices, link) {
> - 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);
> + unsigned long evdev_keys[NBITS(KEY_CNT)];
> + uint32_t i;
> + int ret;
These should be outside of the loop, at the top of the function.
> + memset(evdev_keys, 0, sizeof(evdev_keys));
> +
> + ret = ioctl(device->fd, EVIOCGKEY(KEY_CNT), evdev_keys);
> +
> + if (ret < 0)
> continue;
> +
> + for (i = 0; i < key_range; i++) {
> + int value = TEST_BIT(evdev_keys, (i + key_min));
> + if (value)
> + keys_counts[i]++;
> }
> + }
> +}
> +
> +void
> +evdev_keys_states_query(struct wl_list *evdev_devices,
> + unsigned long all_keys[NBITS(KEY_CNT)])
> +{
> + struct evdev_device *device;
> +
> + wl_list_for_each(device, evdev_devices, link) {
> + unsigned long evdev_keys[NBITS(KEY_CNT)];
> + uint32_t i;
> + int ret;
Same here.
> + memset(evdev_keys, 0, sizeof(evdev_keys));
> +
> + ret = ioctl(device->fd, EVIOCGKEY(KEY_CNT), evdev_keys);
> +
> + if (ret < 0)
> + continue;
> +
> for (i = 0; i < ARRAY_LENGTH(evdev_keys); i++)
> all_keys[i] |= evdev_keys[i];
> }
> +}
> +
> +void
> +evdev_keys_depressed_array(struct wl_list *evdev_devices, struct wl_array *keys)
> +{
> + unsigned long all_keys[NBITS(KEY_CNT)];
> + uint32_t i;
> + uint32_t *k;
> +
> + memset(all_keys, 0, sizeof(all_keys));
> + evdev_keys_states_query(evdev_devices, all_keys);
>
> - 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);
> + if (TEST_BIT(all_keys,i)) {
> + k = wl_array_add(keys, sizeof *k);
> *k = i;
> }
> }
> +}
>
> - notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC);
> +void
> +evdev_notify_keyboard_focus(struct weston_seat *seat,
> + struct wl_list *evdev_devices)
> +{
> + struct wl_array keys;
>
> + wl_array_init(&keys);
> + evdev_keys_depressed_array(evdev_devices, &keys);
> + notify_keyboard_focus_in(seat, &keys, STATE_UPDATE_AUTOMATIC);
> wl_array_release(&keys);
> }
> diff --git a/src/evdev.h b/src/evdev.h
> index eb5c868..ad68081 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -121,7 +121,22 @@ void
> evdev_device_destroy(struct evdev_device *device);
>
> void
> +evdev_keys_states_count(struct wl_list *evdev_devices, uint8_t *keys_counts,
> + uint32_t key_min, uint32_t key_max);
> +void
> +evdev_keys_states_query(struct wl_list *evdev_devices,
> + unsigned long all_keys[NBITS(KEY_CNT)]);
> +void
> +evdev_keys_depressed_array(struct wl_list *evdev_devices,struct wl_array *keys);
> +
> +void
> evdev_notify_keyboard_focus(struct weston_seat *seat,
> struct wl_list *evdev_devices);
>
> +void
> +evdev_notify_key_status(struct weston_seat *seat,
> + struct wl_list *evdev_devices,
> + uint32_t key_min, uint32_t key_max);
> +
> +
> #endif /* EVDEV_H */
> --
> 1.7.10.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list