[PATCH libinput v2 4/4] touchpad: Implement pinch gesture support
Peter Hutterer
peter.hutterer at who-t.net
Wed Apr 8 22:25:49 PDT 2015
On Thu, Mar 26, 2015 at 10:04:40AM +0100, Hans de Goede wrote:
> Implement touchpad pinch (and rotate) gesture support.
>
> Note that two two-finger scrolling tests are slightly tweaked to assure that
> there is enough touch movement to allow the scroll-or-pinch detect code to do
> its works.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> src/evdev-mt-touchpad-gestures.c | 268 ++++++++++++++++++++++++++++++++++++++-
> src/evdev-mt-touchpad.h | 17 +++
> src/libinput-private.h | 33 +++++
> test/touchpad.c | 4 +-
> 4 files changed, 315 insertions(+), 7 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad-gestures.c b/src/evdev-mt-touchpad-gestures.c
> index 017354c..3d3dcaf 100644
> --- a/src/evdev-mt-touchpad-gestures.c
> +++ b/src/evdev-mt-touchpad-gestures.c
> @@ -22,7 +22,6 @@
>
> #include "config.h"
>
> -#include <assert.h>
> #include <math.h>
> #include <stdbool.h>
> #include <limits.h>
> @@ -30,6 +29,7 @@
> #include "evdev-mt-touchpad.h"
>
> #define DEFAULT_GESTURE_SWITCH_TIMEOUT 100 /* ms */
> +#define DEFAULT_GESTURE_2FG_SCROLL_TIMEOUT 1000 /* ms */
>
> static struct normalized_coords
> tp_get_touches_delta(struct tp_dispatch *tp, bool average)
> @@ -75,6 +75,7 @@ tp_get_average_touches_delta(struct tp_dispatch *tp)
> static void
> tp_gesture_start(struct tp_dispatch *tp, uint64_t time)
> {
> + struct libinput *libinput = tp->device->base.seat->libinput;
> const struct normalized_coords zero = { 0.0, 0.0 };
>
> if (tp->gesture.started)
> @@ -82,7 +83,21 @@ tp_gesture_start(struct tp_dispatch *tp, uint64_t time)
>
> switch (tp->gesture.finger_count) {
> case 2:
> - /* NOP */
> + switch (tp->gesture.twofinger_state) {
> + case GESTURE_2FG_STATE_NONE:
> + case GESTURE_2FG_STATE_UNKNOWN:
> + log_bug_libinput(libinput,
> + "%s in unknown gesture mode\n", __func__);
nitpick: for multi-line calls please line up the arguments with the opening
parenthesis where possible and use one arg per line. So this would become:
log_bug_libinput(libinput,
"%s in unknown gesture mode\n",
__func__);
> + break;
> + case GESTURE_2FG_STATE_SCROLL:
> + /* NOP */
> + break;
> + case GESTURE_2FG_STATE_PINCH:
> + gesture_notify_pinch(&tp->device->base, time,
> + LIBINPUT_EVENT_GESTURE_PINCH_START,
> + &zero, &zero, 0.0, 0.0);
> + break;
> + }
> break;
> case 3:
> case 4:
> @@ -114,8 +129,172 @@ tp_gesture_post_pointer_motion(struct tp_dispatch *tp, uint64_t time)
> }
> }
>
> +static unsigned int
> +tp_gesture_get_active_touches(struct tp_dispatch *tp,
> + struct tp_touch **touches,
> + unsigned int count)
> +{
> + unsigned int i, n = 0;
> + struct tp_touch *t;
> +
> + memset(touches, 0, count * sizeof(struct tp_touch *));
> +
> + for (i = 0; i < tp->real_touches; i++) {
> + t = &tp->touches[i];
> + if (tp_touch_active(tp, t)) {
> + touches[n++] = t;
> + if (n == count)
> + return count;
> + }
> + }
> +
> + /*
> + * This can happen when the user does .e.g:
> + * 1) Put down 1st finger in center (so active)
> + * 2) Put down 2nd finger in a button area (so inactive)
> + * 3) Put down 3th finger somewhere, gets reported as a fake finger,
> + * so gets same coordinates as 1st -> active
> + *
> + * We could avoid this by looking at all touches, be we really only
> + * want to look at real touches.
> + */
> + return n;
> +}
> +
> +static int
> +tp_gesture_get_direction(struct tp_dispatch *tp, int touch)
> +{
> + struct normalized_coords normalized;
> + double move_threshold;
> +
> + /*
> + * Semi-mt touchpads have somewhat inaccurate coordinates when
> + * 2 fingers are down, so use a slightly larger threshold.
> + */
> + if (tp->semi_mt)
> + move_threshold = TP_MM_TO_DPI_NORMALIZED(4);
> + else
> + move_threshold = TP_MM_TO_DPI_NORMALIZED(3);
> +
> + normalized = tp_normalize_delta(tp,
> + device_delta(tp->gesture.touches[touch]->point,
> + tp->gesture.touches[touch]->gesture.initial));
temp. variable please, with the indentation it's not immediately obvious
what calls what, so this would be better as:
struct tp_touch *t = &tp->gesture.touches[touch];
struct device_float_coords delta;
delta = device_delta(t->point, t->gesture.initial);
normalized = tp_normalize_delta(tp, &delta);
also - any reason you're passing in the touch index rather than a
struct tp_touch directly?
> + if (normalized_length(normalized) < move_threshold)
> + return UNDEFINED_DIRECTION;
> +
> + return normalized_get_direction(normalized);
> +}
> +
> +static void
> +tp_gesture_get_pinch_info(struct tp_dispatch *tp,
> + double *distance,
> + double *angle,
> + struct device_float_coords *center)
> +{
> + struct normalized_coords normalized;
> +
> + normalized = tp_normalize_delta(tp,
> + device_delta(tp->gesture.touches[0]->point,
> + tp->gesture.touches[1]->point));
same as above, except for the function argument.
struct tp_touch *first = &tp->gestures.touches[0],
*second = &tp->gestures.touches[1];
delta = device_delta(first->point, second->point);
normalized = ...
> + *distance = normalized_length(normalized);
> +
> + if (!tp->semi_mt)
> + *angle = atan2(normalized.y, normalized.x) * 180.0 / M_PI;
> + else
> + *angle = 0.0;
> +
> + *center = device_average(tp->gesture.touches[0]->point,
> + tp->gesture.touches[1]->point);
and re-use first/second here.
> +}
> +
> +static void
> +tp_gesture_set_scroll_buildup(struct tp_dispatch *tp)
> +{
> + struct device_float_coords d0, d1;
> +
> + d0 = device_delta(tp->gesture.touches[0]->point,
> + tp->gesture.touches[0]->gesture.initial);
> + d1 = device_delta(tp->gesture.touches[1]->point,
> + tp->gesture.touches[1]->gesture.initial);
here too.
> +
> + tp->device->scroll.buildup =
> + tp_normalize_delta(tp, device_float_average(d0, d1));
> +}
> +
> +static void
> +tp_gesture_twofinger_state_none(struct tp_dispatch *tp, uint64_t time)
> +{
> + if (tp_gesture_get_active_touches(tp, tp->gesture.touches, 2) != 2)
> + return;
> +
> + tp->gesture.initial_time = time;
> + tp->gesture.touches[0]->gesture.initial =
> + tp->gesture.touches[0]->point;
> + tp->gesture.touches[1]->gesture.initial =
> + tp->gesture.touches[1]->point;
here too.
> +
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_UNKNOWN;
> +}
> +
> +static void
> +tp_gesture_twofinger_state_unknown(struct tp_dispatch *tp, uint64_t time)
> +{
> + struct normalized_coords normalized;
> + int dir0, dir1;
> +
> + normalized = tp_normalize_delta(tp,
> + device_delta(tp->gesture.touches[0]->point,
> + tp->gesture.touches[1]->point));
and here, which then also measns you can re-use first/second in the
tp_gesture_get_direction() call, which goes nicely with one of my comments
above.
> +
> + /* If fingers are further than 3 cm apart assume pinch */
> + if (normalized_length(normalized) > TP_MM_TO_DPI_NORMALIZED(30)) {
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_PINCH;
> + tp_gesture_get_pinch_info(tp,
> + &tp->gesture.distance,
> + &tp->gesture.angle,
> + &tp->gesture.center);
> + return;
> + }
> +
> + /* Elif fingers have been close together for a while, scroll */
> + if (time > (tp->gesture.initial_time + DEFAULT_GESTURE_2FG_SCROLL_TIMEOUT)) {
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_SCROLL;
> + tp_gesture_set_scroll_buildup(tp);
> + return;
> + }
> +
> + /* Else wait for both fingers to have moved */
> + dir0 = tp_gesture_get_direction(tp, 0);
> + dir1 = tp_gesture_get_direction(tp, 1);
> + if (dir0 == UNDEFINED_DIRECTION || dir1 == UNDEFINED_DIRECTION)
> + return;
> +
> + /*
> + * If both touches are moving in the same direction assume scroll.
> + *
> + * In some cases (semi-mt touchpads) We may seen one finger move
> + * e.g. N/NE and the other W/NW so we not only check for overlapping
> + * directions, but also for neighboring bits being set.
> + * The ((dira & 0x80) && (dirb & 0x01)) checks are to check for bit 0
> + * and 7 being set as they also represent neighboring directions.
> + */
> + if (((dir0 | (dir0 >> 1)) & dir1) ||
> + ((dir1 | (dir1 >> 1)) & dir0) ||
> + ((dir0 & 0x80) && (dir1 & 0x01)) ||
> + ((dir1 & 0x80) && (dir0 & 0x01))) {
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_SCROLL;
> + tp_gesture_set_scroll_buildup(tp);
> + } else {
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_PINCH;
> + tp_gesture_get_pinch_info(tp,
> + &tp->gesture.distance,
> + &tp->gesture.angle,
> + &tp->gesture.center);
> + }
> +}
> +
> static void
> -tp_gesture_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time)
> +tp_gesture_twofinger_state_scroll(struct tp_dispatch *tp, uint64_t time)
> {
> struct normalized_coords delta;
>
> @@ -133,6 +312,65 @@ tp_gesture_post_twofinger_scroll(struct tp_dispatch *tp, uint64_t time)
> }
>
> static void
> +tp_gesture_twofinger_state_pinch(struct tp_dispatch *tp, uint64_t time)
> +{
> + double angle, d, distance;
> + struct device_float_coords center;
> + struct normalized_coords delta, unaccel;
> +
> + tp_gesture_get_pinch_info(tp, &distance, &angle, ¢er);
> +
> + d = distance - tp->gesture.distance;
> + tp->gesture.distance = distance;
> + distance = d;
> +
> + d = angle - tp->gesture.angle;
> + if (d > 180.0)
> + d -= 360.0;
> + else if (d < -180.0)
> + d += 360.0;
> + tp->gesture.angle = angle;
> + angle = d;
> +
> + unaccel = tp_normalize_delta(tp,
> + device_float_delta(center, tp->gesture.center));
same as above
> + tp->gesture.center = center;
> + delta = tp_filter_motion(tp, &unaccel, time);
> +
> + if (normalized_is_zero(delta) && normalized_is_zero(unaccel) &&
> + distance == 0.0 && angle == 0.0)
> + return;
> +
> + tp_gesture_start(tp, time);
> + gesture_notify_pinch(&tp->device->base, time,
> + LIBINPUT_EVENT_GESTURE_PINCH_UPDATE,
> + &delta, &unaccel, distance, angle);
> +}
> +
> +static void
> +tp_gesture_post_twofinger(struct tp_dispatch *tp, uint64_t time)
> +{
> + switch (tp->gesture.twofinger_state) {
> + case GESTURE_2FG_STATE_NONE:
> + tp_gesture_twofinger_state_none(tp, time);
> + if (tp->gesture.twofinger_state != GESTURE_2FG_STATE_UNKNOWN)
> + break;
> + /* fall through */
> + case GESTURE_2FG_STATE_UNKNOWN:
> + tp_gesture_twofinger_state_unknown(tp, time);
> + if (tp->gesture.twofinger_state != GESTURE_2FG_STATE_SCROLL)
> + break;
> + /* fall through */
> + case GESTURE_2FG_STATE_SCROLL:
> + tp_gesture_twofinger_state_scroll(tp, time);
> + break;
> + case GESTURE_2FG_STATE_PINCH:
> + tp_gesture_twofinger_state_pinch(tp, time);
> + break;
> + }
I don't think a switch statement is good for this type of logic. the
fallthroughs and behind-our-back state changes are not really obvious.
Better to use a if chain instead:
if (state == NONE)
handle_state_none();
if (state == unknown)
handle state_unknown();
etc. ideally you'd return the new state to make it even more obvious rather
than setting the state in the struct and re-checking that.
also, right now if you switch from none to scroll you post an event
immediately, but for pinch you don't until the next run. That seems
unbalanced and a bit odd, is there a reason for this?
> +}
> +
> +static void
> tp_gesture_post_swipe(struct tp_dispatch *tp, uint64_t time)
> {
> struct normalized_coords delta, unaccel;
> @@ -171,7 +409,7 @@ tp_gesture_post_events(struct tp_dispatch *tp, uint64_t time)
> tp_gesture_post_pointer_motion(tp, time);
> break;
> case 2:
> - tp_gesture_post_twofinger_scroll(tp, time);
> + tp_gesture_post_twofinger(tp, time);
> break;
> case 3:
> case 4:
> @@ -191,14 +429,32 @@ tp_gesture_stop_twofinger_scroll(struct tp_dispatch *tp, uint64_t time)
> void
> tp_gesture_stop(struct tp_dispatch *tp, uint64_t time)
> {
> + struct libinput *libinput = tp->device->base.seat->libinput;
> + enum tp_gesture_2fg_state twofinger_state = tp->gesture.twofinger_state;
> const struct normalized_coords zero = { 0.0, 0.0 };
>
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_NONE;
> +
> if (!tp->gesture.started)
> return;
>
> switch (tp->gesture.finger_count) {
> case 2:
> - tp_gesture_stop_twofinger_scroll(tp, time);
> + switch (twofinger_state) {
> + case GESTURE_2FG_STATE_NONE:
> + case GESTURE_2FG_STATE_UNKNOWN:
> + log_bug_libinput(libinput,
> + "%s in unknown gesture mode\n", __func__);
> + break;
> + case GESTURE_2FG_STATE_SCROLL:
> + tp_gesture_stop_twofinger_scroll(tp, time);
> + break;
> + case GESTURE_2FG_STATE_PINCH:
> + gesture_notify_pinch(&tp->device->base, time,
> + LIBINPUT_EVENT_GESTURE_PINCH_END,
> + &zero, &zero, 0.0, 0.0);
> + break;
> + }
> break;
> case 3:
> case 4:
> @@ -258,6 +514,8 @@ tp_gesture_handle_state(struct tp_dispatch *tp, uint64_t time)
> int
> tp_init_gesture(struct tp_dispatch *tp)
> {
> + tp->gesture.twofinger_state = GESTURE_2FG_STATE_NONE;
> +
> libinput_timer_init(&tp->gesture.finger_count_switch_timer,
> tp->device->base.seat->libinput,
> tp_gesture_finger_count_switch_timeout, tp);
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 19a262e..83f0492 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -120,6 +120,13 @@ enum tp_edge_scroll_touch_state {
> EDGE_SCROLL_TOUCH_STATE_AREA,
> };
>
> +enum tp_gesture_2fg_state {
> + GESTURE_2FG_STATE_NONE,
> + GESTURE_2FG_STATE_UNKNOWN,
> + GESTURE_2FG_STATE_SCROLL,
> + GESTURE_2FG_STATE_PINCH,
> +};
> +
> struct tp_touch {
> struct tp_dispatch *tp;
> enum touch_state state;
> @@ -171,6 +178,10 @@ struct tp_touch {
> struct device_coords first; /* first coordinates if is_palm == true */
> uint32_t time; /* first timestamp if is_palm == true */
> } palm;
> +
> + struct {
> + struct device_coords initial;
> + } gesture;
> };
>
> struct tp_dispatch {
> @@ -205,6 +216,12 @@ struct tp_dispatch {
> unsigned int finger_count;
> unsigned int finger_count_pending;
> struct libinput_timer finger_count_switch_timer;
> + enum tp_gesture_2fg_state twofinger_state;
> + struct tp_touch *touches[2];
> + uint64_t initial_time;
> + double distance;
> + double angle;
> + struct device_float_coords center;
> } gesture;
>
> struct {
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 2c9f42b..b433126 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -391,6 +391,39 @@ device_delta(struct device_coords a, struct device_coords b)
> return delta;
> }
>
> +static inline struct device_float_coords
> +device_average(struct device_coords a, struct device_coords b)
> +{
> + struct device_float_coords average;
> +
> + average.x = (a.x + b.x) / 2.0;
> + average.y = (a.y - b.y) / 2.0;
this doesn't look right, is this intended?
> +
> + return average;
> +}
> +
> +static inline struct device_float_coords
> +device_float_delta(struct device_float_coords a, struct device_float_coords b)
> +{
> + struct device_float_coords delta;
> +
> + delta.x = a.x - b.x;
> + delta.y = a.y - b.y;
> +
> + return delta;
> +}
> +
> +static inline struct device_float_coords
> +device_float_average(struct device_float_coords a, struct device_float_coords b)
> +{
> + struct device_float_coords average;
> +
> + average.x = (a.x + b.x) / 2.0;
> + average.y = (a.y - b.y) / 2.0;
same here
Cheers,
Peter
> +
> + return average;
> +}
> +
> static inline double
> normalized_length(struct normalized_coords norm)
> {
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 6fa2301..62b5381 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -1872,7 +1872,7 @@ START_TEST(touchpad_2fg_scroll_slow_distance)
> y_move = 7.0 * y->resolution /
> (y->maximum - y->minimum) * 100;
> } else {
> - y_move = 10.0;
> + y_move = 20.0;
> }
>
> litest_drain_events(li);
> @@ -1921,7 +1921,7 @@ START_TEST(touchpad_2fg_scroll_source)
>
> litest_drain_events(li);
>
> - test_2fg_scroll(dev, 0, 20, 0);
> + test_2fg_scroll(dev, 0, 30, 0);
> litest_wait_for_event_of_type(li, LIBINPUT_EVENT_POINTER_AXIS, -1);
>
> while ((event = libinput_get_event(li))) {
> --
> 2.3.3
>
> _______________________________________________
> 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