[PATCH libinput] touchpad: allow BTN_LEFT in clickfinger mode without touches

[Peter Hutterer]

On Fri, Apr 24, 2015 at 08:30:28AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 24-04-15 06:20, Peter Hutterer wrote:
> >On the Logitech T650 it's quite easy to trigger a click without touching the
> >surface. For software buttons we discard those clicks because we can't tell
> >where the finger is to decide on left vs right click.
> >
> >It takes effort to trigger a click with two fingers without triggering a touch
> >though, so in clickfinger mode post a click without toucheas as single-finger
> >click.
> >
> >https://bugs.freedesktop.org/show_bug.cgi?id=90150
> >
> >Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> 
> I'm not sure about this one, the reason to wait to see a finger before continuing
> with processing a click is the same with click-finger, on some hardware the left-button
> event arrives before the finger info, so in this case if I quickly click the pad with
> 2 fingers to get a right click, then the button may get seen on the evdev node before
> the fingers do, resulting in a left click instead.
> 
> Now for 2fg clicking I would expect the user to take some time with both fingers on
> the surface before clicking to make sure both fingers have registered so I'm not sure
> this is a real issue, still I thought this should be pointed out.

Yeah, that's pretty much the reasoning. not touching the surface with 2
fingers while executing a click is quite hard so we can assume thats a rare
exception. otoh clicking with the thumb resting on the edge (and never
triggering a touch) is quite easy, so we should handle that case correctly.

thanks for the review.

Cheers,
   Peter

> 
> The code looks good, so if you still think this is a good idea, then this is:
> 
> Reviewed-by: Hans de Goede <hdegoede at redhat.com>
> 
> Regards,
> 
> Hans
> 
> 
> >---
> >  src/evdev-mt-touchpad-buttons.c |  6 ++++--
> >  test/touchpad.c                 | 30 ++++++++++++++++++++++++++++++
> >  2 files changed, 34 insertions(+), 2 deletions(-)
> >
> >diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> >index c907ecc..7f60a53 100644
> >--- a/src/evdev-mt-touchpad-buttons.c
> >+++ b/src/evdev-mt-touchpad-buttons.c
> >@@ -804,6 +804,7 @@ tp_notify_clickpadbutton(struct tp_dispatch *tp,
> >  	if (tp->buttons.click_method == LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER &&
> >  	    state == LIBINPUT_BUTTON_STATE_PRESSED) {
> >  		switch (tp->nfingers_down) {
> >+		case 0:
> >  		case 1: button = BTN_LEFT; break;
> >  		case 2: button = BTN_RIGHT; break;
> >  		case 3: button = BTN_MIDDLE; break;
> >@@ -865,7 +866,8 @@ tp_post_clickpadbutton_buttons(struct tp_dispatch *tp, uint64_t time)
> >  			}
> >  		}
> >
> >-		if (area == 0) {
> >+		if (area == 0 &&
> >+		    tp->buttons.click_method != LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER) {
> >  			/* No touches, wait for a touch before processing */
> >  			tp->buttons.click_pending = true;
> >  			return 0;
> >@@ -877,7 +879,7 @@ tp_post_clickpadbutton_buttons(struct tp_dispatch *tp, uint64_t time)
> >  			button = evdev_to_left_handed(tp->device, BTN_RIGHT);
> >  		else if (area & LEFT)
> >  			button = evdev_to_left_handed(tp->device, BTN_LEFT);
> >-		else /* main area is always BTN_LEFT */
> >+		else /* main or no area (for clickfinger) is always BTN_LEFT */
> >  			button = BTN_LEFT;
> >
> >  		tp->buttons.active = button;
> >diff --git a/test/touchpad.c b/test/touchpad.c
> >index c04ef11..db981dc 100644
> >--- a/test/touchpad.c
> >+++ b/test/touchpad.c
> >@@ -1506,6 +1506,32 @@ START_TEST(touchpad_1fg_clickfinger)
> >  }
> >  END_TEST
> >
> >+START_TEST(touchpad_1fg_clickfinger_no_touch)
> >+{
> >+	struct litest_device *dev = litest_current_device();
> >+	struct libinput *li = dev->libinput;
> >+	enum libinput_config_status status;
> >+
> >+	status = libinput_device_config_click_set_method(dev->libinput_device,
> >+							 LIBINPUT_CONFIG_CLICK_METHOD_CLICKFINGER);
> >+	ck_assert_int_eq(status, LIBINPUT_CONFIG_STATUS_SUCCESS);
> >+
> >+	litest_drain_events(li);
> >+
> >+	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);
> >+
> >+	libinput_dispatch(li);
> >+
> >+	litest_assert_button_event(li, BTN_LEFT,
> >+				   LIBINPUT_BUTTON_STATE_PRESSED);
> >+	litest_assert_button_event(li, BTN_LEFT,
> >+				   LIBINPUT_BUTTON_STATE_RELEASED);
> >+}
> >+END_TEST
> >+
> >  START_TEST(touchpad_2fg_clickfinger)
> >  {
> >  	struct litest_device *dev = litest_current_device();
> >@@ -1785,6 +1811,9 @@ START_TEST(clickpad_btn_left)
> >  	struct litest_device *dev = litest_current_device();
> >  	struct libinput *li = dev->libinput;
> >
> >+	libinput_device_config_click_set_method(dev->libinput_device,
> >+						LIBINPUT_CONFIG_CLICK_METHOD_BUTTON_AREAS);
> >+
> >  	litest_drain_events(li);
> >
> >  	/* A clickpad always needs a finger down to tell where the
> >@@ -4119,6 +4148,7 @@ int main(int argc, char **argv) {
> >  	litest_add("touchpad:tap", clickpad_2fg_tap_click, LITEST_CLICKPAD, LITEST_SINGLE_TOUCH|LITEST_APPLE_CLICKPAD);
> >
> >  	litest_add("touchpad:clickfinger", touchpad_1fg_clickfinger, LITEST_CLICKPAD, LITEST_ANY);
> >+	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_clickfinger_to_area_method, LITEST_CLICKPAD, LITEST_ANY);
> >  	litest_add("touchpad:clickfinger",
> >