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

[Hans de Goede]

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.

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",
>