[PATCH libinput 11/20] touchpad: Rework is_pointer handling

[Jonas Ådahl]

On Thu, Apr 24, 2014 at 03:28:26PM +1000, Peter Hutterer wrote:
> On Tue, Apr 15, 2014 at 02:28:08PM +0200, Hans de Goede wrote:
> > Move scanning for a suitable touch to be the pointer to tp_process_state
> > and take tp_button_touch_active into account.
> > 
> > Note this adds a tp_touch_active helper since we want to do the same checks in
> > other places too (ie to see if a finger should count for 2 finger scrolling).
> > 
> > Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> > Acked-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  src/evdev-mt-touchpad.c | 47 +++++++++++++++++++++++------------------------
> >  1 file changed, 23 insertions(+), 24 deletions(-)
> > 
> > diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> > index 9df4a78..b671211 100644
> > --- a/src/evdev-mt-touchpad.c
> > +++ b/src/evdev-mt-touchpad.c
> > @@ -152,8 +152,6 @@ tp_get_touch(struct tp_dispatch *tp, unsigned int slot)
> >  static inline void
> >  tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t)
> >  {
> > -	struct tp_touch *tmp;
> > -
> >  	if (t->state != TOUCH_UPDATE) {
> >  		tp_motion_history_reset(t);
> >  		t->dirty = true;
> > @@ -161,15 +159,6 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t)
> >  		tp->nfingers_down++;
> >  		assert(tp->nfingers_down >= 1);
> >  		tp->queued |= TOUCHPAD_EVENT_MOTION;
> > -
> > -		tp_for_each_touch(tp, tmp) {
> > -			if (tmp->is_pointer)
> > -				break;
> > -		}
> > -
> > -		if (!tmp->is_pointer) {
> > -			t->is_pointer = true;
> > -		}
> >  	}
> >  }
> >  
> > @@ -341,7 +330,6 @@ static void
> >  tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
> >  {
> >  	unsigned int xdist, ydist;
> > -	struct tp_touch *tmp;
> >  
> >  	if (!t->pinned.is_pinned)
> >  		return;
> > @@ -349,19 +337,9 @@ tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
> >  	xdist = abs(t->x - t->pinned.center_x);
> >  	ydist = abs(t->y - t->pinned.center_y);
> >  
> > -	if (xdist * xdist + ydist * ydist <
> > +	if (xdist * xdist + ydist * ydist >=
> >  			tp->buttons.motion_dist * tp->buttons.motion_dist)
> > -		return;
> > -
> > -	t->pinned.is_pinned = false;
> > -
> > -	tp_for_each_touch(tp, tmp) {
> > -		if (tmp->is_pointer)
> > -			break;
> > -	}
> > -
> > -	if (t->state != TOUCH_END && !tmp->is_pointer)
> > -		t->is_pointer = true;
> > +		t->pinned.is_pinned = false;
> >  }
> >  
> >  static void
> > @@ -377,6 +355,13 @@ tp_pin_fingers(struct tp_dispatch *tp)
> >  	}
> >  }
> >  
> > +static int
> > +tp_touch_active(struct tp_dispatch *tp, struct tp_touch *t)
> > +{
> > +	return (t->state == TOUCH_BEGIN || t->state == TOUCH_UPDATE) &&
> > +		!t->pinned.is_pinned && tp_button_touch_active(tp, t);
> > +}
> > +
> >  static void
> >  tp_process_state(struct tp_dispatch *tp, uint32_t time)
> >  {
> > @@ -409,6 +394,20 @@ tp_process_state(struct tp_dispatch *tp, uint32_t time)
> >  	if ((tp->queued & TOUCHPAD_EVENT_BUTTON_PRESS) &&
> >  	    !tp->buttons.has_buttons)
> >  		tp_pin_fingers(tp);
> > +
> > +	/* If we don't have a touch as pointer find a suitable one */
> > +	tp_for_each_touch(tp, t) {
> > +		if (t->is_pointer)
> > +			break;
> > +	}
> > +	if (!t->is_pointer) {
> > +		tp_for_each_touch(tp, t) {
> > +			if (tp_touch_active(tp, t)) {
> > +				t->is_pointer = true;
> > +				break;
> > +			}
> > +		}
> > +	}
> >  }
> >  
> 
> this changes the way the touchpad behaves for scrolling (it breaks the
> touchpad_2fg_no_motion test). Before, a touch is marked as pointer touch on
> begin. Now a touch can be marked pointer touch as soon as it is the last
> touch and that causes a spurious motion event in the above test: as soon as
> the first finger is lifted, the second finger generates a motion event.

It seems this patch also breaks the touchpad_1fg_motion test. Applying
it I start to get:

touchpad.c:50:F:synaptics ST:func:0: Assertion 'event != ((void *)0)'
failed


Jonas

> 
> This is a bit of a side-effect pinning all fingers, I think we need
> something more sophisticated to tell which touchpoint is the controlling
> one.
> 
> Cheers,
>    Peter
>  
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel