[PATCH weston] evdev: Handle SYN_DROPPED, query the state of buttons
Jonas Ådahl
jadahl at gmail.com
Sat Mar 23 08:20:57 PDT 2013
On Mon, Mar 18, 2013 at 10:35 AM, Martin Minarik
<minarik11 at student.fiit.stuba.sk> wrote:
> When the kernel event queue overruns, the evdev.c will:
> 1. Skip events until and up to the next SYN_REPORT
> 2. Notify the compositor of the lag, the compositor can specify
> a key range of a key buttons to query the state of. The compositor
> will also invalidate the button event counters, if any.
> 3. Replay the button states and key states to the compositor via
> the usual api with a custom ACTUAL_STATE flag set.
> 4. Notify the compositor that the lag is over. The compositor
> will check, in case no button is held on the lagged device
> it will prematurely terminate the grab. This can be maybe
> coded even more clever to query all the seat devices, in that case,
> compositor button counter would be restored completely, and grabs
> terminated.
>
> This introduces a new field, enum weston_button_state_update:
> STATE_CHANGE_EVENT The state has changed, can count these events
> STATE_ACTUAL_UPDATE The actual state, it might not have changed
>
> This fixes:
> https://bugs.freedesktop.org/show_bug.cgi?id=62368
Hi Martin,
It seems that in this patch you let the compositor do the
synchronization for something evdev specific. I think this approach is
incorrect as well, as it is not up to the compositor to do this. The
correct approach is to let evdev do the synchronization, and the
missing part to do this is making button presses etc transactional,
i.e. we should not commit a change (press/release button or key, move
to (x, y)) until we are sure it's not to be aborted. When we do that,
we can properly abort a transaction and synchronize the state upon
SYN_DROPPED.
Jonas
> ---
> src/compositor-wayland.c | 2 +-
> src/compositor-x11.c | 3 +-
> src/compositor.c | 60 ++++++++++++++++++++++++++----
> src/compositor.h | 16 +++++++-
> src/evdev-touchpad.c | 9 +++--
> src/evdev.c | 91 ++++++++++++++++++++++++++++++++++++++++++++--
> tests/weston-test.c | 2 +-
> 7 files changed, 165 insertions(+), 18 deletions(-)
>
> diff --git a/src/compositor-wayland.c b/src/compositor-wayland.c
> index ceb912a..6a105b3 100644
> --- a/src/compositor-wayland.c
> +++ b/src/compositor-wayland.c
> @@ -375,7 +375,7 @@ input_handle_button(void *data, struct wl_pointer *pointer,
> struct wayland_input *input = data;
> enum wl_pointer_button_state state = state_w;
>
> - notify_button(&input->base, time, button, state);
> + notify_button(&input->base, time, button, state, STATE_CHANGE_EVENT);
> }
>
> static void
> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
> index 8e052dd..346e1ec 100644
> --- a/src/compositor-x11.c
> +++ b/src/compositor-x11.c
> @@ -943,7 +943,8 @@ x11_compositor_deliver_button_event(struct x11_compositor *c,
> notify_button(&c->core_seat,
> weston_compositor_get_time(), button,
> state ? WL_POINTER_BUTTON_STATE_PRESSED :
> - WL_POINTER_BUTTON_STATE_RELEASED);
> + WL_POINTER_BUTTON_STATE_RELEASED,
> + STATE_CHANGE_EVENT);
> }
>
> static void
> diff --git a/src/compositor.c b/src/compositor.c
> index 248d3b4..cbfb8cc 100644
> --- a/src/compositor.c
> +++ b/src/compositor.c
> @@ -1635,7 +1635,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)
> + compositor->idle_inhibit--;
> + weston_compositor_wake(compositor);
> +}
> +
> +static void
> +weston_compositor_idle_reset(struct weston_compositor *compositor)
> +{
> + compositor->idle_inhibit = 0;
> weston_compositor_wake(compositor);
> }
>
> @@ -1776,8 +1784,35 @@ weston_surface_activate(struct weston_surface *surface,
> }
>
> WL_EXPORT void
> -notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
> - enum wl_pointer_button_state state)
> +notify_lag(struct weston_seat *seat, uint32_t time,
> + int32_t *query_key_min, int32_t *query_key_max)
> +{
> + struct wl_pointer *pointer = seat->seat.pointer;
> + struct weston_compositor *compositor = seat->compositor;
> +
> + weston_compositor_idle_reset(compositor);
> +
> + if (pointer->button_count) {
> + pointer->button_count = 0;
> + *query_key_min = pointer->grab_button;
> + *query_key_max = pointer->grab_button + 1;
> + }
> +}
> +
> +WL_EXPORT void
> +notify_lag_end(struct weston_seat *seat, uint32_t time)
> +{
> + struct wl_pointer *pointer = seat->seat.pointer;
> +
> + if (pointer->button_count == 0)
> + pointer->grab->interface->button(pointer->grab, time,
> + pointer->grab_button, WL_POINTER_BUTTON_STATE_RELEASED);
> +}
> +
> +WL_EXPORT void
> +notify_button(struct weston_seat *seat, uint32_t time, uint32_t button,
> + enum wl_pointer_button_state state,
> + enum weston_button_state_update update_state)
> {
> struct weston_compositor *compositor = seat->compositor;
> struct wl_pointer *pointer = seat->seat.pointer;
> @@ -1789,16 +1824,25 @@ notify_button(struct weston_seat *seat, uint32_t time, int32_t button,
> if (compositor->ping_handler && focus)
> compositor->ping_handler(focus, serial);
> weston_compositor_idle_inhibit(compositor);
> - if (pointer->button_count == 0) {
> +
> + if ((pointer->button_count == 0) &&
> + (update_state == STATE_CHANGE_EVENT)) {
> pointer->grab_button = button;
> pointer->grab_time = time;
> 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 (pointer->button_count == 0)
> + return;
> +
> + if ((button == pointer->grab_button) &&
> + (update_state == STATE_CHANGE_EVENT))
> + pointer->button_count--;
> }
>
> weston_compositor_run_button_binding(compositor, seat, time, button,
> @@ -1806,7 +1850,9 @@ 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 ((state == WL_POINTER_BUTTON_STATE_PRESSED) &&
> + (pointer->button_count == 1) &&
> + (update_state == STATE_CHANGE_EVENT))
> pointer->grab_serial =
> wl_display_get_serial(compositor->wl_display);
> }
> diff --git a/src/compositor.h b/src/compositor.h
> index 4a0c1e3..a614ee9 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -491,6 +491,11 @@ enum weston_key_state_update {
> STATE_UPDATE_NONE,
> };
>
> +enum weston_button_state_update {
> + STATE_CHANGE_EVENT,
> + STATE_ACTUAL_UPDATE,
> +};
> +
> void
> weston_version(int *major, int *minor, int *micro);
>
> @@ -542,8 +547,15 @@ 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,
> - enum wl_pointer_button_state state);
> +notify_lag(struct weston_seat *seat, uint32_t time,
> + int32_t *query_key_min, int32_t *query_key_max);
> +void
> +notify_lag_end(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,
> + enum weston_button_state_update update_state);
> void
> notify_axis(struct weston_seat *seat, uint32_t time, uint32_t axis,
> wl_fixed_t value);
> diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
> index c25a199..aad5dfc 100644
> --- a/src/evdev-touchpad.c
> +++ b/src/evdev-touchpad.c
> @@ -275,7 +275,8 @@ notify_button_pressed(struct touchpad_dispatch *touchpad, uint32_t time)
> {
> notify_button(touchpad->device->seat, time,
> DEFAULT_TOUCHPAD_SINGLE_TAP_BUTTON,
> - WL_POINTER_BUTTON_STATE_PRESSED);
> + WL_POINTER_BUTTON_STATE_PRESSED,
> + STATE_CHANGE_EVENT);
> }
>
> static void
> @@ -283,7 +284,8 @@ notify_button_released(struct touchpad_dispatch *touchpad, uint32_t time)
> {
> notify_button(touchpad->device->seat, time,
> DEFAULT_TOUCHPAD_SINGLE_TAP_BUTTON,
> - WL_POINTER_BUTTON_STATE_RELEASED);
> + WL_POINTER_BUTTON_STATE_RELEASED,
> + STATE_CHANGE_EVENT);
> }
>
> static void
> @@ -581,7 +583,8 @@ process_key(struct touchpad_dispatch *touchpad,
> notify_button(device->seat,
> time, e->code,
> e->value ? WL_POINTER_BUTTON_STATE_PRESSED :
> - WL_POINTER_BUTTON_STATE_RELEASED);
> + WL_POINTER_BUTTON_STATE_RELEASED,
> + STATE_CHANGE_EVENT);
> break;
> case BTN_TOOL_PEN:
> case BTN_TOOL_RUBBER:
> diff --git a/src/evdev.c b/src/evdev.c
> index d2954b5..6d140ce 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -60,6 +60,34 @@ 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 int
> +evdev_query_eviocg_button_range(struct evdev_device *device, int time,
> + int32_t key_min, int32_t key_max);
> +
> +static inline void
> +evdev_process_syn_skip(struct evdev_device *d, struct input_event *e, int time)
> +{
> +
> +}
> +
> static inline void
> evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> {
> @@ -78,9 +106,9 @@ evdev_process_key(struct evdev_device *device, struct input_event *e, int time)
> notify_button(device->seat,
> time, e->code,
> e->value ? WL_POINTER_BUTTON_STATE_PRESSED :
> - WL_POINTER_BUTTON_STATE_RELEASED);
> + WL_POINTER_BUTTON_STATE_RELEASED,
> + STATE_CHANGE_EVENT);
> break;
> -
> default:
> notify_key(device->seat,
> time, e->code,
> @@ -308,7 +336,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 +352,39 @@ 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)
> +{
> + int32_t key_min = KEY_MAX, key_max = 0;
> +
> + if ((event->code != EV_SYN) || (event->code != SYN_REPORT))
> + return;
> +
> + if (device->dispatch->interface == &syn_drop_interface)
> + device->dispatch->interface = &fallback_interface;
> +
> + notify_lag(device->seat, time, &key_min, &key_max);
> +
> + if (key_min < key_max)
> + evdev_query_eviocg_button_range(device, time, key_min, key_max);
> +
> + notify_lag_end(device->seat, time);
> +}
> +
> +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)
> {
> @@ -399,6 +460,30 @@ evdev_device_data(int fd, uint32_t mask, void *data)
> }
>
> static int
> +evdev_query_eviocg_button_range(struct evdev_device *device, int time,
> + int32_t key_min, int32_t key_max)
> +{
> + unsigned long key_bits[NBITS(KEY_MAX)];
> + int key;
> +
> + memset(key_bits, 0, sizeof(key_bits));
> +
> + if (ioctl(device->fd, EVIOCGKEY(KEY_MAX), key_bits) < 0)
> + return -1;
> +
> + for (key = key_min; key < key_max; key++) {
> + int value = TEST_BIT(key_bits, key);
> + notify_button(device->seat,
> + time, key,
> + value ? WL_POINTER_BUTTON_STATE_PRESSED :
> + WL_POINTER_BUTTON_STATE_RELEASED,
> + STATE_ACTUAL_UPDATE);
> + }
> +
> + return 0;
> +}
> +
> +static int
> evdev_handle_device(struct evdev_device *device)
> {
> struct input_absinfo absinfo;
> diff --git a/tests/weston-test.c b/tests/weston-test.c
> index b29b64f..6782205 100644
> --- a/tests/weston-test.c
> +++ b/tests/weston-test.c
> @@ -142,7 +142,7 @@ send_button(struct wl_client *client, struct wl_resource *resource,
>
> test->compositor->focus = 1;
>
> - notify_button(seat, 100, button, state);
> + notify_button(seat, 100, button, state, STATE_CHANGE_EVENT);
> }
>
> static void
> --
> 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