touchpad: be smarter about clickfinger thumb detection

Watching a colleague try clickfinger right-click after enabling it the first
time showed that the vertical distance is too small. Increase it to 30mm
instead.

Increase the allowed spread between fingers to 40x30mm, but check if one of
the fingers is in the bottom-most 20mm of the touchpad. If that's the case,
and the touchpad is large enough to be feasable for resting a thumb on it,
discard the finger for clickfinger count.

If both fingers are in that area or one finger is in the area and they're
really close together, the fingers count separately and are not regarded as
thumb.

https://bugs.freedesktop.org/show_bug.cgi?id=91046

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
index 3352c66..771d71d 100644 (file)
--- a/src/evdev-mt-touchpad-buttons.c
+++ b/src/evdev-mt-touchpad-buttons.c
@@ -825,6 +825,9 @@ tp_check_clickfinger_distance(struct tp_dispatch *tp,
                              struct tp_touch *t2)
 {
        double x, y;
+       int within_distance = 0;
+       int xres, yres;
+       int bottom_threshold;
 
        if (!t1 || !t2)
                return 0;
@@ -839,19 +842,44 @@ tp_check_clickfinger_distance(struct tp_dispatch *tp,
                w = tp->device->abs.dimensions.x;
                h = tp->device->abs.dimensions.y;
 
-               return (x < w * 0.3 && y < h * 0.3) ? 1 : 0;
-       } else {
-               /* maximum spread is 40mm horiz, 20mm vert. Anything wider than that
-                * is probably a gesture. The y spread is small so we ignore clicks
-                * with thumbs at the bottom of the touchpad while the pointer
-                * moving finger is still on the pad */
+               within_distance = (x < w * 0.3 && y < h * 0.3) ? 1 : 0;
+               goto out;
+       }
 
-               x /= tp->device->abs.absinfo_x->resolution;
-               y /= tp->device->abs.absinfo_y->resolution;
+       xres = tp->device->abs.absinfo_x->resolution;
+       yres = tp->device->abs.absinfo_y->resolution;
+       x /= xres;
+       y /= yres;
 
-               return (x < 40 && y < 20) ? 1 : 0;
-       }
+       /* maximum horiz spread is 40mm horiz, 30mm vert, anything wider
+        * than that is probably a gesture. */
+       if (x > 40 || y > 30)
+               goto out;
+
+       within_distance = 1;
+
+       /* if y spread is <= 20mm, they're definitely together. */
+       if (y <= 20)
+               goto out;
+
+       /* if they're vertically spread between 20-40mm, they're not
+        * together if:
+        * - the touchpad's vertical size is >50mm, anything smaller is
+        *   unlikely to have a thumb resting on it
+        * - and one of the touches is in the bottom 20mm of the touchpad
+        *   and the other one isn't
+        */
 
+       if (tp->device->abs.dimensions.y/yres < 50)
+               goto out;
+
+       bottom_threshold = tp->device->abs.absinfo_y->maximum - 20 * yres;
+       if ((t1->point.y > bottom_threshold) !=
+                   (t2->point.y > bottom_threshold))
+               within_distance = 0;
+
+out:
+       return within_distance;
 }
 
 static uint32_t

diff --git a/test/touchpad.c b/test/touchpad.c
index a9e9e4e..4aa9f4e 100644 (file)
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -291,6 +291,13 @@ START_TEST(touchpad_2fg_clickfinger_distance)
 {
        struct litest_device *dev = litest_current_device();
        struct libinput *li = dev->libinput;
+       double w, h;
+       bool small_touchpad = false;
+       unsigned int expected_button;
+
+       if (libinput_device_get_size(dev->libinput_device, &w, &h) == 0 &&
+           h < 50.0)
+               small_touchpad = true;
 
        enable_clickfinger(dev);
 
@@ -323,12 +330,86 @@ START_TEST(touchpad_2fg_clickfinger_distance)
        litest_touch_up(dev, 0);
        litest_touch_up(dev, 1);
 
+       /* if the touchpad is small enough, we expect all fingers to count
+        * for clickfinger */
+       if (small_touchpad)
+               expected_button = BTN_RIGHT;
+       else
+               expected_button = BTN_LEFT;
+
+       litest_assert_button_event(li,
+                                  expected_button,
+                                  LIBINPUT_BUTTON_STATE_PRESSED);
+       litest_assert_button_event(li,
+                                  expected_button,
+                                  LIBINPUT_BUTTON_STATE_RELEASED);
+}
+END_TEST
+
+START_TEST(touchpad_2fg_clickfinger_bottom)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+
+       /* this test is run for the T440s touchpad only, makes getting the
+        * mm correct easier */
+
+       libinput_device_config_click_set_method(dev->libinput_device,
+                                               LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER);
+       litest_drain_events(li);
+
+       /* one above, one below the magic line, vert spread ca 27mm */
+       litest_touch_down(dev, 0, 40, 60);
+       litest_touch_down(dev, 1, 60, 100);
+       litest_event(dev, EV_KEY, BTN_LEFT, 1);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_event(dev, EV_KEY, BTN_LEFT, 0);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_touch_up(dev, 0);
+       litest_touch_up(dev, 1);
+
        litest_assert_button_event(li,
                                   BTN_LEFT,
                                   LIBINPUT_BUTTON_STATE_PRESSED);
        litest_assert_button_event(li,
                                   BTN_LEFT,
                                   LIBINPUT_BUTTON_STATE_RELEASED);
+
+       litest_assert_empty_queue(li);
+
+       /* both below the magic line */
+       litest_touch_down(dev, 0, 40, 100);
+       litest_touch_down(dev, 1, 60, 95);
+       litest_event(dev, EV_KEY, BTN_LEFT, 1);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_event(dev, EV_KEY, BTN_LEFT, 0);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_touch_up(dev, 0);
+       litest_touch_up(dev, 1);
+
+       litest_assert_button_event(li,
+                                  BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_PRESSED);
+       litest_assert_button_event(li,
+                                  BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_RELEASED);
+
+       /* one above, one below the magic line, vert spread 17mm */
+       litest_touch_down(dev, 0, 50, 75);
+       litest_touch_down(dev, 1, 55, 100);
+       litest_event(dev, EV_KEY, BTN_LEFT, 1);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_event(dev, EV_KEY, BTN_LEFT, 0);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       litest_touch_up(dev, 0);
+       litest_touch_up(dev, 1);
+
+       litest_assert_button_event(li,
+                                  BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_PRESSED);
+       litest_assert_button_event(li,
+                                  BTN_RIGHT,
+                                  LIBINPUT_BUTTON_STATE_RELEASED);
 }
 END_TEST
 
@@ -3676,6 +3757,7 @@ litest_setup_tests(void)
        litest_add("touchpad:clickfinger", touchpad_1fg_clickfinger_no_touch, LITEST_CLICKPAD, LITEST_ANY);
        litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger, LITEST_CLICKPAD, LITEST_ANY);
        litest_add("touchpad:clickfinger", touchpad_2fg_clickfinger_distance, LITEST_CLICKPAD, LITEST_ANY);
+       litest_add_for_device("touchpad:clickfinger", touchpad_2fg_clickfinger_bottom, LITEST_SYNAPTICS_TOPBUTTONPAD);
        litest_add("touchpad:clickfinger", touchpad_clickfinger_to_area_method, LITEST_CLICKPAD, LITEST_ANY);
        litest_add("touchpad:clickfinger",
                   touchpad_clickfinger_to_area_method_while_down, LITEST_CLICKPAD, LITEST_ANY);