[PATCH libinput 11/11] touchpad: put a movement threshold on thumb detection

[Peter Hutterer]

On Wed, Jul 22, 2015 at 03:54:02PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-07-15 07:09, Peter Hutterer wrote:
> >If a thumb moves around, it's not resting and we should consider it a normal
> >touch.
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> >---
> >  src/evdev-mt-touchpad.c | 16 ++++++++++++++++
> >  src/evdev-mt-touchpad.h |  1 +
> >  test/touchpad.c         | 31 ++++++++++++++++++++++++++++---
> >  3 files changed, 45 insertions(+), 3 deletions(-)
> >
> >diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> >index 0b39bf6..63ce488 100644
> >--- a/src/evdev-mt-touchpad.c
> >+++ b/src/evdev-mt-touchpad.c
> >@@ -636,6 +636,22 @@ tp_thumb_detect(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
> >  		goto out;
> >  	}
> >
> >+	/* If the thumb moves by more than 7mm, it's not a resting thumb */
> >+	if (t->state == TOUCH_BEGIN)
> >+		t->thumb.initial = t->point;
> >+	else if (t->state == TOUCH_UPDATE) {
> >+		struct device_float_coords delta;
> >+		struct normalized_coords normalized;
> >+
> >+		delta = device_delta(t->point, t->thumb.initial);
> >+		normalized = tp_normalize_delta(tp, delta);
> >+		if (normalized_length(normalized) >
> >+			TP_MM_TO_DPI_NORMALIZED(7)) {
> >+			t->thumb.state = THUMB_STATE_NO;
> >+			goto out;
> >+		}
> >+	}
> >+
> >  	/* Note: a thumb at the edge of the touchpad won't trigger the
> >  	 * threshold, the surface area is usually too small. So we have a
> >  	 * two-stage detection: pressure and time within the area.
> 
> If you want this to mark thouches with THUMB_STATE_YES as THUMB_STATE_NO
> this needs to be above the:
> 
>     /* once a thumb, always a thumb, once ruled out always ruled out */
>     if (!tp->thumb.detect_thumbs ||
>         t->thumb.state != THUMB_STATE_MAYBE)
>          return;
> 
> Block, and maybe the commentline above the if() of that block needs to
> be updated too ...

see my comment to the other patch, for now I'd like to leave it as "once a
thumb, always a thumb".

> Also does this commit not require updates to the svg with the thumb detect
> statemachine ?

the svg in the other patch is a graphic for the documentation page, not a
state diagram. the state machine is simple enough (and constrained to a
single function) that I didn't bother with a state diagram.

Cheers,
   Peter


> 
> Regards,
> 
> Hans
> 
> 
> 
> >diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> >index 89801d6..0dfc6ac 100644
> >--- a/src/evdev-mt-touchpad.h
> >+++ b/src/evdev-mt-touchpad.h
> >@@ -204,6 +204,7 @@ struct tp_touch {
> >  	struct {
> >  		enum tp_thumb_state state;
> >  		uint64_t first_touch_time;
> >+		struct device_coords initial;
> >  	} thumb;
> >  };
> >
> >diff --git a/test/touchpad.c b/test/touchpad.c
> >index 875bc58..25db43f 100644
> >--- a/test/touchpad.c
> >+++ b/test/touchpad.c
> >@@ -4054,15 +4054,39 @@ START_TEST(touchpad_thumb_update_no_motion)
> >
> >  	litest_drain_events(li);
> >
> >+	litest_touch_down(dev, 0, 59, 99);
> >+	litest_touch_move_extended(dev, 0, 59, 99, axes);
> >+	litest_touch_move_to(dev, 0, 60, 99, 80, 99, 10, 0);
> >+	litest_touch_up(dev, 0);
> >+
> >+	litest_assert_empty_queue(li);
> >+}
> >+END_TEST
> >+
> >+START_TEST(touchpad_thumb_moving)
> >+{
> >+	struct litest_device *dev = litest_current_device();
> >+	struct libinput *li = dev->libinput;
> >+	struct axis_replacement axes[] = {
> >+		{ ABS_MT_PRESSURE, 190 },
> >+		{ -1, 0 }
> >+	};
> >+
> >+	litest_disable_tap(dev->libinput_device);
> >+	enable_clickfinger(dev);
> >+
> >+	if (!has_thumb_detect(dev))
> >+		return;
> >+
> >+	litest_drain_events(li);
> >+
> >  	litest_touch_down(dev, 0, 50, 99);
> >  	litest_touch_move_to(dev, 0, 50, 99, 60, 99, 10, 0);
> >-	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> >-
> >  	litest_touch_move_extended(dev, 0, 65, 99, axes);
> >  	litest_touch_move_to(dev, 0, 65, 99, 80, 99, 10, 0);
> >  	litest_touch_up(dev, 0);
> >
> >-	litest_assert_empty_queue(li);
> >+	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> >  }
> >  END_TEST
> >
> >@@ -4520,6 +4544,7 @@ litest_setup_tests(void)
> >
> >  	litest_add("touchpad:thumb", touchpad_thumb_begin_no_motion, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:thumb", touchpad_thumb_update_no_motion, LITEST_CLICKPAD, LITEST_ANY);
> >+	litest_add("touchpad:thumb", touchpad_thumb_moving, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:thumb", touchpad_thumb_clickfinger, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:thumb", touchpad_thumb_btnarea, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:thumb", touchpad_thumb_edgescroll, LITEST_CLICKPAD, LITEST_ANY);
> >