[PATCH libinput 2/2] touchpad: set the finger pin distance to 5mm where possible

Peter Hutterer peter.hutterer at who-t.net
Mon Jun 15 17:19:54 PDT 2015


On Mon, Jun 15, 2015 at 09:54:18AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 15-06-15 06:40, Peter Hutterer wrote:
> >On touchpads with resolutions, use a 5mm motion threshold before we unpin the
> >finger (allow motion events while a clickpad button is down). This should
> >remove any erroneous finger movements while clicking, at the cost of having to
> >move the finger a bit more for a single-finger click-and-drag (use two fingers
> >already!)
> >
> >And drop the finger drifting, it was per-event based rather than time-based.
> >So unless the motion threshold was hit in a single event it was possible to
> >move the finger around the whole touchpad without ever unpinning it.
> >
> >Drop the finger drifting altogether, if the touchpad drifts by more than 5mm
> >we have other issues.
> >
> >https://bugzilla.redhat.com/show_bug.cgi?id=1230462
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> 
> Why 5mm ? Have you done any tests to come to this, I've a feeling it is larger
> then it needs to be. Maybe we should try 3 first?

fair enough. no real reason other than it seemed like a good distance. I
tested with 3mm and it feels equally stable. So I changed it to 3mm now (and
the scale for resolution-less devices accordingly)

> Regardless of the above this looks good:
> 
> Reviewed-by: Hans de Goede <hdegoede at redhat.com>

thanks, much appreciated.

Cheers,
   Peter

> >---
> >  src/evdev-mt-touchpad-buttons.c | 19 ++++++++++------
> >  src/evdev-mt-touchpad.c         | 10 ++++-----
> >  src/evdev-mt-touchpad.h         |  5 ++++-
> >  src/libinput-util.h             |  6 ++++++
> >  test/touchpad.c                 | 48 +++++++++++++++++++++++++++++++++++++++++
> >  5 files changed, 75 insertions(+), 13 deletions(-)
> >
> >diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> >index 5786ea8..c84bcb0 100644
> >--- a/src/evdev-mt-touchpad-buttons.c
> >+++ b/src/evdev-mt-touchpad-buttons.c
> >@@ -31,7 +31,6 @@
> >
> >  #include "evdev-mt-touchpad.h"
> >
> >-#define DEFAULT_BUTTON_MOTION_THRESHOLD 0.02 /* 2% of size */
> >  #define DEFAULT_BUTTON_ENTER_TIMEOUT 100 /* ms */
> >  #define DEFAULT_BUTTON_LEAVE_TIMEOUT 300 /* ms */
> >
> >@@ -709,11 +708,19 @@ tp_init_buttons(struct tp_dispatch *tp,
> >  	absinfo_x = device->abs.absinfo_x;
> >  	absinfo_y = device->abs.absinfo_y;
> >
> >-	width = abs(absinfo_x->maximum - absinfo_x->minimum);
> >-	height = abs(absinfo_y->maximum - absinfo_y->minimum);
> >-	diagonal = sqrt(width*width + height*height);
> >-
> >-	tp->buttons.motion_dist = diagonal * DEFAULT_BUTTON_MOTION_THRESHOLD;
> >+	/* pinned-finger motion threshold, see tp_unpin_finger.
> >+	   The MAGIC for resolution-less touchpads ends up as 2% of the diagonal */
> >+	if (device->abs.fake_resolution) {
> >+		const int BUTTON_MOTION_MAGIC = 0.004;
> >+		width = abs(absinfo_x->maximum - absinfo_x->minimum);
> >+		height = abs(absinfo_y->maximum - absinfo_y->minimum);
> >+		diagonal = sqrt(width*width + height*height);
> >+		tp->buttons.motion_dist.x_scale_coeff = diagonal * BUTTON_MOTION_MAGIC;
> >+		tp->buttons.motion_dist.y_scale_coeff = diagonal * BUTTON_MOTION_MAGIC;
> >+	} else {
> >+		tp->buttons.motion_dist.x_scale_coeff = 1.0/absinfo_x->resolution;
> >+		tp->buttons.motion_dist.y_scale_coeff = 1.0/absinfo_y->resolution;
> >+	}
> >
> >  	tp->buttons.config_method.get_methods = tp_button_config_click_get_methods;
> >  	tp->buttons.config_method.set_method = tp_button_config_click_set_method;
> >diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> >index ce79530..ec4dd6d 100644
> >--- a/src/evdev-mt-touchpad.c
> >+++ b/src/evdev-mt-touchpad.c
> >@@ -431,17 +431,15 @@ tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
> >  		return;
> >
> >  	xdist = abs(t->point.x - t->pinned.center.x);
> >+	xdist *= tp->buttons.motion_dist.x_scale_coeff;
> >  	ydist = abs(t->point.y - t->pinned.center.y);
> >+	ydist *= tp->buttons.motion_dist.y_scale_coeff;
> >
> >-	if (xdist * xdist + ydist * ydist >=
> >-			tp->buttons.motion_dist * tp->buttons.motion_dist) {
> >+	/* 5mm movement -> unpin */
> >+	if (vector_length(xdist, ydist) >= 5.0) {
> >  		t->pinned.is_pinned = false;
> >  		return;
> >  	}
> >-
> >-	/* The finger may slowly drift, adjust the center */
> >-	t->pinned.center.x = (t->point.x + t->pinned.center.x)/2;
> >-	t->pinned.center.y = (t->point.y + t->pinned.center.y)/2;
> >  }
> >
> >  static void
> >diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> >index fef5cb3..bd2d163 100644
> >--- a/src/evdev-mt-touchpad.h
> >+++ b/src/evdev-mt-touchpad.h
> >@@ -223,7 +223,10 @@ struct tp_dispatch {
> >  		bool click_pending;
> >  		uint32_t state;
> >  		uint32_t old_state;
> >-		uint32_t motion_dist;		/* for pinned touches */
> >+		struct {
> >+			double x_scale_coeff;
> >+			double y_scale_coeff;
> >+		} motion_dist;			/* for pinned touches */
> >  		unsigned int active;		/* currently active button, for release event */
> >  		bool active_is_topbutton;	/* is active a top button? */
> >
> >diff --git a/src/libinput-util.h b/src/libinput-util.h
> >index 910406c..224e4b6 100644
> >--- a/src/libinput-util.h
> >+++ b/src/libinput-util.h
> >@@ -284,4 +284,10 @@ int parse_mouse_dpi_property(const char *prop);
> >  int parse_mouse_wheel_click_angle_property(const char *prop);
> >  double parse_trackpoint_accel_property(const char *prop);
> >
> >+static inline double
> >+vector_length(double x, double y)
> >+{
> >+	return sqrt(x * x + y * y);
> >+}
> >+
> >  #endif /* LIBINPUT_UTIL_H */
> >diff --git a/test/touchpad.c b/test/touchpad.c
> >index 692698c..1e5e97b 100644
> >--- a/test/touchpad.c
> >+++ b/test/touchpad.c
> >@@ -2153,6 +2153,53 @@ START_TEST(clickpad_click_n_drag)
> >  }
> >  END_TEST
> >
> >+START_TEST(clickpad_finger_pin)
> >+{
> >+	struct litest_device *dev = litest_current_device();
> >+	struct libinput *li = dev->libinput;
> >+	struct libevdev *evdev = dev->evdev;
> >+	const struct input_absinfo *abs;
> >+
> >+	abs = libevdev_get_abs_info(evdev, ABS_MT_POSITION_X);
> >+	ck_assert_notnull(abs);
> >+	if (abs->resolution == 0)
> >+		return;
> >+
> >+	litest_drain_events(li);
> >+
> >+	/* make sure the movement generates pointer events when
> >+	   not pinned */
> >+	litest_touch_down(dev, 0, 50, 50);
> >+	litest_touch_move_to(dev, 0, 50, 50, 52, 52, 10, 1);
> >+	litest_touch_move_to(dev, 0, 52, 52, 48, 48, 10, 1);
> >+	litest_touch_move_to(dev, 0, 48, 48, 50, 50, 10, 1);
> >+	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> >+
> >+	litest_button_click(dev, BTN_LEFT, true);
> >+	litest_drain_events(li);
> >+
> >+	litest_touch_move_to(dev, 0, 50, 50, 52, 52, 10, 1);
> >+	litest_touch_move_to(dev, 0, 52, 52, 48, 48, 10, 1);
> >+	litest_touch_move_to(dev, 0, 48, 48, 50, 50, 10, 1);
> >+
> >+	litest_assert_empty_queue(li);
> >+
> >+	litest_button_click(dev, BTN_LEFT, false);
> >+	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_BUTTON);
> >+
> >+	/* still pinned after release */
> >+	litest_touch_move_to(dev, 0, 50, 50, 52, 52, 10, 1);
> >+	litest_touch_move_to(dev, 0, 52, 52, 48, 48, 10, 1);
> >+	litest_touch_move_to(dev, 0, 48, 48, 50, 50, 10, 1);
> >+
> >+	litest_assert_empty_queue(li);
> >+
> >+	/* move to unpin */
> >+	litest_touch_move_to(dev, 0, 50, 50, 70, 70, 10, 1);
> >+	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> >+}
> >+END_TEST
> >+
> >  START_TEST(clickpad_softbutton_left)
> >  {
> >  	struct litest_device *dev = litest_current_device();
> >@@ -5144,6 +5191,7 @@ litest_setup_tests(void)
> >  	litest_add("touchpad:click", touchpad_btn_left, LITEST_TOUCHPAD|LITEST_BUTTON, LITEST_CLICKPAD);
> >  	litest_add("touchpad:click", clickpad_btn_left, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:click", clickpad_click_n_drag, LITEST_CLICKPAD, LITEST_SINGLE_TOUCH);
> >+	litest_add("touchpad:click", clickpad_finger_pin, LITEST_CLICKPAD, LITEST_ANY);
> >
> >  	litest_add("touchpad:softbutton", clickpad_softbutton_left, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
> >  	litest_add("touchpad:softbutton", clickpad_softbutton_right, LITEST_CLICKPAD, LITEST_APPLE_CLICKPAD);
> >


More information about the wayland-devel mailing list