[PATCH libinput 1/2] Change axis events to carry all directions
Hans de Goede
hdegoede at redhat.com
Mon Jan 5 04:20:53 PST 2015
Hi,
On 05-01-15 06:20, Peter Hutterer wrote:
> Sending separate axis events instead of one unified events is limiting,
> especially when simultaneously scrolling in both directions and the caller
> tries to implement kinetic scrolling.
>
> Take a page from the tablet-support branch and instead implement the axis
> event as a generic event that can contain multiple axes simultaneously.
>
> This is an API and ABI break.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
Ack to the concept (and most of the code) nack to the implementation wrt
the evdev-mt-touchpad-edge-scroll.c changes, see below.
> ---
> src/evdev-mt-touchpad-edge-scroll.c | 21 ++++++++-------
> src/evdev.c | 53 ++++++++++++++-----------------------
> src/libinput-private.h | 3 +--
> src/libinput.c | 31 +++++++++++++---------
> src/libinput.h | 17 ++----------
> test/litest.c | 19 ++++++-------
> test/pointer.c | 11 +++++---
> test/touchpad.c | 16 ++++++-----
> tools/event-debug.c | 22 +++++----------
> tools/event-gui.c | 18 ++++++-------
> 10 files changed, 93 insertions(+), 118 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c
> index a4dc093..b82c966 100644
> --- a/src/evdev-mt-touchpad-edge-scroll.c
> +++ b/src/evdev-mt-touchpad-edge-scroll.c
> @@ -314,7 +314,7 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
> struct libinput_device *device = &tp->device->base;
> struct tp_touch *t;
> enum libinput_pointer_axis axis;
> - double dx, dy, *delta;
> + double dx, dy;
>
> tp_for_each_touch(tp, t) {
> if (!t->dirty)
> @@ -325,19 +325,16 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
> if (t->scroll.direction != -1) {
> /* Send stop scroll event */
> pointer_notify_axis(device, time,
> - t->scroll.direction,
> LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> - 0.0);
> + 0.0, 0.0);
> t->scroll.direction = -1;
> }
> continue;
> case EDGE_RIGHT:
> axis = LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL;
> - delta = &dy;
> break;
> case EDGE_BOTTOM:
> axis = LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL;
> - delta = &dx;
> break;
> default: /* EDGE_RIGHT | EDGE_BOTTOM */
> continue; /* Don't know direction yet, skip */
> @@ -346,12 +343,17 @@ tp_edge_scroll_post_events(struct tp_dispatch *tp, uint64_t time)
> tp_get_delta(t, &dx, &dy);
> tp_filter_motion(tp, &dx, &dy, NULL, NULL, time);
>
> - if (fabs(*delta) < t->scroll.threshold)
> + if (fabs(dx) < t->scroll.threshold)
> + dx = 0.0;
> + if (fabs(dy) < t->scroll.threshold)
> + dy = 0.0;
> +
> + if (dx == 0.0 && dy == 0.0)
> continue;
>
> - pointer_notify_axis(device, time, axis,
> + pointer_notify_axis(device, time,
> LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> - *delta);
> + dx, dy);
> t->scroll.direction = axis;
>
> tp_edge_scroll_handle_event(tp, t, SCROLL_EVENT_POSTED);
This change leads to potentially sending horizontal scroll events while
scrolling along the right edge, or vertical scroll events while
scrolling along the bottom edge, which is undesirable.
This needs to 0 out the not used axis depending on the t->scroll.edge
value. Also since you now effectively no longer use the axis local variable
it can be removed, and t->scroll.direction should be replaced with a
t->scroll.scrolling boolean.
Regards,
Hans
> @@ -369,9 +371,8 @@ tp_edge_scroll_stop_events(struct tp_dispatch *tp, uint64_t time)
> tp_for_each_touch(tp, t) {
> if (t->scroll.direction != -1) {
> pointer_notify_axis(device, time,
> - t->scroll.direction,
> LIBINPUT_POINTER_AXIS_SOURCE_FINGER,
> - 0.0);
> + 0.0, 0.0);
> t->scroll.direction = -1;
> }
> }
> diff --git a/src/evdev.c b/src/evdev.c
> index 1718491..5d22af2 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -539,18 +539,18 @@ evdev_process_absolute_motion(struct evdev_device *device,
> static void
> evdev_notify_axis(struct evdev_device *device,
> uint64_t time,
> - enum libinput_pointer_axis axis,
> enum libinput_pointer_axis_source source,
> - double value)
> + double x, double y)
> {
> - if (device->scroll.natural_scrolling_enabled)
> - value *= -1;
> + if (device->scroll.natural_scrolling_enabled) {
> + x *= -1;
> + y *= -1;
> + }
>
> pointer_notify_axis(&device->base,
> time,
> - axis,
> source,
> - value);
> + x, y);
> }
>
> static inline void
> @@ -575,8 +575,8 @@ evdev_process_relative(struct evdev_device *device,
> evdev_notify_axis(
> device,
> time,
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL,
> LIBINPUT_POINTER_AXIS_SOURCE_WHEEL,
> + 0,
> -1 * e->value * DEFAULT_AXIS_STEP_DISTANCE);
> break;
> case REL_HWHEEL:
> @@ -584,9 +584,9 @@ evdev_process_relative(struct evdev_device *device,
> evdev_notify_axis(
> device,
> time,
> - LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL,
> LIBINPUT_POINTER_AXIS_SOURCE_WHEEL,
> - e->value * DEFAULT_AXIS_STEP_DISTANCE);
> + e->value * DEFAULT_AXIS_STEP_DISTANCE,
> + 0);
> break;
> }
> }
> @@ -1832,25 +1832,19 @@ evdev_post_scroll(struct evdev_device *device,
> /* We use the trigger to enable, but the delta from this event for
> * the actual scroll movement. Otherwise we get a jump once
> * scrolling engages */
> - if (dy != 0.0 &&
> - evdev_is_scrolling(device,
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)) {
> + if (!evdev_is_scrolling(device,
> + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
> + dy = 0.0;
> + if (!evdev_is_scrolling(device,
> + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
> + dx = 0.0;
> +
> + if (dx != 0.0 || dy != 0.0)
> evdev_notify_axis(device,
> time,
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL,
> source,
> + dx,
> dy);
> - }
> -
> - if (dx != 0.0 &&
> - evdev_is_scrolling(device,
> - LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL)) {
> - evdev_notify_axis(device,
> - time,
> - LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL,
> - source,
> - dx);
> - }
> }
>
> void
> @@ -1859,18 +1853,11 @@ evdev_stop_scroll(struct evdev_device *device,
> enum libinput_pointer_axis_source source)
> {
> /* terminate scrolling with a zero scroll event */
> - if (device->scroll.direction & (1 << LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
> + if (device->scroll.direction != 0)
> pointer_notify_axis(&device->base,
> time,
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL,
> source,
> - 0);
> - if (device->scroll.direction & (1 << LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
> - pointer_notify_axis(&device->base,
> - time,
> - LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL,
> - source,
> - 0);
> + 0.0, 0.0);
>
> device->scroll.buildup_horizontal = 0;
> device->scroll.buildup_vertical = 0;
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 84a0d44..5ffab40 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -278,9 +278,8 @@ pointer_notify_button(struct libinput_device *device,
> void
> pointer_notify_axis(struct libinput_device *device,
> uint64_t time,
> - enum libinput_pointer_axis axis,
> enum libinput_pointer_axis_source source,
> - double value);
> + double x, double y);
>
> void
> touch_notify_touch_down(struct libinput_device *device,
> diff --git a/src/libinput.c b/src/libinput.c
> index 426c306..f43799f 100644
> --- a/src/libinput.c
> +++ b/src/libinput.c
> @@ -64,9 +64,7 @@ struct libinput_event_pointer {
> uint32_t button;
> uint32_t seat_button_count;
> enum libinput_button_state state;
> - enum libinput_pointer_axis axis;
> enum libinput_pointer_axis_source source;
> - double value;
> };
>
> struct libinput_event_touch {
> @@ -379,16 +377,24 @@ libinput_event_pointer_get_seat_button_count(
> return event->seat_button_count;
> }
>
> -LIBINPUT_EXPORT enum libinput_pointer_axis
> -libinput_event_pointer_get_axis(struct libinput_event_pointer *event)
> -{
> - return event->axis;
> -}
>
> LIBINPUT_EXPORT double
> -libinput_event_pointer_get_axis_value(struct libinput_event_pointer *event)
> +libinput_event_pointer_get_axis_value(struct libinput_event_pointer *event,
> + enum libinput_pointer_axis axis)
> {
> - return event->value;
> + double value = 0;
> +
> + if (event->base.type == LIBINPUT_EVENT_POINTER_AXIS) {
> + switch (axis) {
> + case LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL:
> + value = event->x;
> + break;
> + case LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL:
> + value = event->y;
> + break;
> + }
> + }
> + return value;
> }
>
> LIBINPUT_EXPORT enum libinput_pointer_axis_source
> @@ -992,9 +998,8 @@ pointer_notify_button(struct libinput_device *device,
> void
> pointer_notify_axis(struct libinput_device *device,
> uint64_t time,
> - enum libinput_pointer_axis axis,
> enum libinput_pointer_axis_source source,
> - double value)
> + double x, double y)
> {
> struct libinput_event_pointer *axis_event;
>
> @@ -1004,8 +1009,8 @@ pointer_notify_axis(struct libinput_device *device,
>
> *axis_event = (struct libinput_event_pointer) {
> .time = time,
> - .axis = axis,
> - .value = value,
> + .x = x,
> + .y = y,
> .source = source,
> };
>
> diff --git a/src/libinput.h b/src/libinput.h
> index 56b77c2..408cdcc 100644
> --- a/src/libinput.h
> +++ b/src/libinput.h
> @@ -650,20 +650,6 @@ uint32_t
> libinput_event_pointer_get_seat_button_count(
> struct libinput_event_pointer *event);
>
> -/**
> - * @ingroup event_pointer
> - *
> - * Return the axis that triggered this event.
> - * For pointer events that are not of type @ref LIBINPUT_EVENT_POINTER_AXIS,
> - * this function returns 0.
> - *
> - * @note It is an application bug to call this function for events other than
> - * @ref LIBINPUT_EVENT_POINTER_AXIS.
> - *
> - * @return the axis triggering this event
> - */
> -enum libinput_pointer_axis
> -libinput_event_pointer_get_axis(struct libinput_event_pointer *event);
>
> /**
> * @ingroup event_pointer
> @@ -685,7 +671,8 @@ libinput_event_pointer_get_axis(struct libinput_event_pointer *event);
> * @return the axis value of this event
> */
> double
> -libinput_event_pointer_get_axis_value(struct libinput_event_pointer *event);
> +libinput_event_pointer_get_axis_value(struct libinput_event_pointer *event,
> + enum libinput_pointer_axis axis);
>
> /**
> * @ingroup event_pointer
> diff --git a/test/litest.c b/test/litest.c
> index 392ffe2..757f445 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -983,11 +983,11 @@ litest_print_event(struct libinput_event *event)
> case LIBINPUT_EVENT_POINTER_AXIS:
> p = libinput_event_get_pointer_event(event);
> fprintf(stderr,
> - "axis %s value %.2f",
> - libinput_event_pointer_get_axis(p) ==
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL ?
> - "vert" : "horiz",
> - libinput_event_pointer_get_axis_value(p));
> + "vert %.f horiz %.2f",
> + libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL),
> + libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL));
> break;
> default:
> break;
> @@ -1198,23 +1198,24 @@ litest_assert_scroll(struct libinput *li,
> LIBINPUT_EVENT_POINTER_AXIS);
> ptrev = libinput_event_get_pointer_event(event);
> ck_assert(ptrev != NULL);
> - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev), axis);
>
> if (next_event) {
> /* Normal scroll event, check dir */
> if (minimum_movement > 0) {
> ck_assert_int_ge(
> - libinput_event_pointer_get_axis_value(ptrev),
> + libinput_event_pointer_get_axis_value(ptrev,
> + axis),
> minimum_movement);
> } else {
> ck_assert_int_le(
> - libinput_event_pointer_get_axis_value(ptrev),
> + libinput_event_pointer_get_axis_value(ptrev,
> + axis),
> minimum_movement);
> }
> } else {
> /* Last scroll event, must be 0 */
> ck_assert_int_eq(
> - libinput_event_pointer_get_axis_value(ptrev),
> + libinput_event_pointer_get_axis_value(ptrev, axis),
> 0);
> }
> libinput_event_destroy(event);
> diff --git a/test/pointer.c b/test/pointer.c
> index b9bd3cc..120e165 100644
> --- a/test/pointer.c
> +++ b/test/pointer.c
> @@ -348,6 +348,7 @@ test_wheel_event(struct litest_device *dev, int which, int amount)
> struct libinput *li = dev->libinput;
> struct libinput_event *event;
> struct libinput_event_pointer *ptrev;
> + enum libinput_pointer_axis axis;
>
> /* the current evdev implementation scales the scroll wheel events
> up by a factor 10 */
> @@ -372,11 +373,13 @@ test_wheel_event(struct litest_device *dev, int which, int amount)
>
> ptrev = libinput_event_get_pointer_event(event);
> ck_assert(ptrev != NULL);
> - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev),
> - which == REL_WHEEL ?
> +
> + axis = (which == REL_WHEEL) ?
> LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL :
> - LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
> - ck_assert_int_eq(libinput_event_pointer_get_axis_value(ptrev), expected);
> + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL;
> +
> + ck_assert_int_eq(libinput_event_pointer_get_axis_value(ptrev, axis),
> + expected);
> ck_assert_int_eq(libinput_event_pointer_get_axis_source(ptrev),
> LIBINPUT_POINTER_AXIS_SOURCE_WHEEL);
> libinput_event_destroy(event);
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 422e8fe..dbe16a3 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -1439,18 +1439,19 @@ START_TEST(touchpad_2fg_scroll_slow_distance)
>
> /* last event is value 0, tested elsewhere */
> while (libinput_next_event_type(li) != LIBINPUT_EVENT_NONE) {
> + double axisval;
> ck_assert_int_eq(libinput_event_get_type(event),
> LIBINPUT_EVENT_POINTER_AXIS);
> ptrev = libinput_event_get_pointer_event(event);
>
> - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev),
> - LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
> - ck_assert(libinput_event_pointer_get_axis_value(ptrev) > 0.0);
> + axisval = libinput_event_pointer_get_axis_value(ptrev,
> + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
> + ck_assert(axisval > 0.0);
>
> /* this is to verify we test the right thing, if the value
> is greater than scroll.threshold we triggered the wrong
> condition */
> - ck_assert(libinput_event_pointer_get_axis_value(ptrev) < 5.0);
> + ck_assert(axisval < 5.0);
>
> libinput_event_destroy(event);
> event = libinput_get_event(li);
> @@ -1627,18 +1628,19 @@ START_TEST(touchpad_edge_scroll_slow_distance)
> ck_assert_notnull(event);
>
> while (libinput_next_event_type(li) != LIBINPUT_EVENT_NONE) {
> + double axisval;
> ck_assert_int_eq(libinput_event_get_type(event),
> LIBINPUT_EVENT_POINTER_AXIS);
> ptrev = libinput_event_get_pointer_event(event);
>
> - ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev),
> + axisval = libinput_event_pointer_get_axis_value(ptrev,
> LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
> - ck_assert(libinput_event_pointer_get_axis_value(ptrev) > 0.0);
> + ck_assert(axisval > 0.0);
>
> /* this is to verify we test the right thing, if the value
> is greater than scroll.threshold we triggered the wrong
> condition */
> - ck_assert(libinput_event_pointer_get_axis_value(ptrev) < 5.0);
> + ck_assert(axisval < 5.0);
>
> libinput_event_destroy(event);
> event = libinput_get_event(li);
> diff --git a/tools/event-debug.c b/tools/event-debug.c
> index 8fbea24..4a84d67 100644
> --- a/tools/event-debug.c
> +++ b/tools/event-debug.c
> @@ -225,24 +225,14 @@ static void
> print_axis_event(struct libinput_event *ev)
> {
> struct libinput_event_pointer *p = libinput_event_get_pointer_event(ev);
> - enum libinput_pointer_axis axis = libinput_event_pointer_get_axis(p);
> - const char *ax;
> - double val;
> -
> - switch (axis) {
> - case LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL:
> - ax = "vscroll";
> - break;
> - case LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL:
> - ax = "hscroll";
> - break;
> - default:
> - abort();
> - }
> + double v, h;
>
> + v = libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
> + h = libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
> print_event_time(libinput_event_pointer_get_time(p));
> - val = libinput_event_pointer_get_axis_value(p);
> - printf("%s %.2f\n", ax, val);
> + printf("vert %.2f horiz %.2f\n", v, h);
> }
>
> static void
> diff --git a/tools/event-gui.c b/tools/event-gui.c
> index 9a08d8e..4f9d7e6 100644
> --- a/tools/event-gui.c
> +++ b/tools/event-gui.c
> @@ -358,20 +358,20 @@ static void
> handle_event_axis(struct libinput_event *ev, struct window *w)
> {
> struct libinput_event_pointer *p = libinput_event_get_pointer_event(ev);
> - enum libinput_pointer_axis axis = libinput_event_pointer_get_axis(p);
> - double v = libinput_event_pointer_get_axis_value(p);
> + double v, h;
>
> - switch (axis) {
> - case LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL:
> + v = libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
> + h = libinput_event_pointer_get_axis_value(p,
> + LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
> +
> + if (v != 0.0) {
> w->vy += (int)v;
> w->vy = clip(w->vy, 0, w->height);
> - break;
> - case LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL:
> + }
> + if (h != 0.0) {
> w->hx += (int)v;
> w->hx = clip(w->hx, 0, w->width);
> - break;
> - default:
> - abort();
> }
> }
>
>
More information about the wayland-devel
mailing list