[PATCH libinput] touchpad: fix clickfinger behavior with a thumb being present

[Peter Hutterer]

Hi Friedrich,

On Tue, May 16, 2017 at 10:44:22PM +0200, Friedrich Schöller wrote:
> With a thumb on the touchpad, a two-finger click was incorrectly
> treated as a middle-click. This patch takes the thumb into account and
> treats the click as a right-click.

fwiw, this was intentional behaviour, introduced in 316f30d2f28b62 with the
comment:
  It's reasonable to expect a thumb (or the other hand's index finger) to click
  a button while a finger is down for movement. It's less reasonable to expect
  this when two fingers are interacting with the touchpad, or when two fingers
  click the touchpad (even on a large touchpad that's an awkward position).

The problem is that while a index down + thumb is always [1] a one-finger
click, it's not as well defined for two fingers down. But that doesn't
matter, I'm happy to change that behaviour if we can detect the position
correctly. The patch works, but it'll need a change please:
on a touchpad with num_slots 2 the third finger position is based on the
finger position of the first two fingers (see tp_position_fake_touches() or
so).

with the patch applied, *when* the thumb is put down now matters:
if the thumb is the first or second finger, a two-finger + thumb
click is BTN_RIGHT, if the thumb is the third finger, a two-finger + thumb
click is BTN_MIDDLE. That's inconsistent, so we need an extra for num_slots
so this behaviour only gets enabled for devices that can reliably track 3 fingers.

Thanks

Cheers,
   Peter

[1] fsvo 'always'

> ---
>  src/evdev-mt-touchpad-buttons.c | 44 ++++++++++++++++++-----------------------
>  1 file changed, 19 insertions(+), 25 deletions(-)
> 
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index 895cf0e..ffb6a58 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -981,23 +981,24 @@ out:
>  static uint32_t
>  tp_clickfinger_set_button(struct tp_dispatch *tp)
>  {
> -	uint32_t button;
> +	bool thumb_detected = false;
>  	unsigned int nfingers = tp->nfingers_down;
>  	struct tp_touch *t;
>  	struct tp_touch *first = NULL,
>  			*second = NULL;
>  
> -	if (nfingers != 2)
> -		goto out;
> +	if (nfingers < 2)
> +		return BTN_LEFT;
>  
> -	/* two fingers down on the touchpad. Check for distance
> -	 * between the fingers. */
>  	tp_for_each_touch(tp, t) {
>  		if (t->state != TOUCH_BEGIN && t->state != TOUCH_UPDATE)
>  			continue;
>  
> -		if (t->thumb.state == THUMB_STATE_YES)
> +		if (t->thumb.state == THUMB_STATE_YES) {
> +			thumb_detected = true;
> +			nfingers--;
>  			continue;
> +		}
>  
>  		if (!first)
>  			first = t;
> @@ -1005,27 +1006,20 @@ tp_clickfinger_set_button(struct tp_dispatch *tp)
>  			second = t;
>  	}
>  
> -	if (!first || !second) {
> -		nfingers = 1;
> -		goto out;
> -	}
> +	if (nfingers < 2)
> +		return BTN_LEFT;
> +	else if (nfingers > 2)
> +		return BTN_MIDDLE;
>  
> -	if (tp_clickfinger_within_distance(tp, first, second))
> -		nfingers = 2;
> +	/* Two fingers (not counting thumbs) are on the touchpad. If no
> +	 * additional thumb was detected, we check the distance between the
> +	 * touches. Some touchpads can not report more than two finger positions
> +	 * so we do not check the distance if an additional thumb was detected.
> +	 */
> +	if (thumb_detected || tp_clickfinger_within_distance(tp, first, second))
> +		return BTN_RIGHT;
>  	else
> -		nfingers = 1;
> -
> -out:
> -	switch (nfingers) {
> -	case 0:
> -	case 1: button = BTN_LEFT; break;
> -	case 2: button = BTN_RIGHT; break;
> -	default:
> -		button = BTN_MIDDLE; break;
> -		break;
> -	}
> -
> -	return button;
> +		return BTN_LEFT;
>  }
>  
>  static int
> -- 
> 2.9.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>