[PATCH libinput] touchpad: disable MT for all semi-mt devices

[Peter Hutterer]

On Fri, Jan 22, 2016 at 11:15:55AM +0100, Hans de Goede wrote:
> Hi,
> 
> On 22-01-16 03:06, Peter Hutterer wrote:
> >On Wed, Jan 20, 2016 at 11:31:42AM +0100, Hans de Goede wrote:
> >>Hi,
> >>
> 
> <snip>
> 
> >>Then we can also revert:
> >>
> >>http://cgit.freedesktop.org/wayland/libinput/commit/?id=0d40aefe
> >>
> >>Making tp_gesture_get_active_touches() look only at real touches
> >>again as intended.
> >
> >
> >I don't think this is correct, we still need to count fake fingers on an ST
> >touchpad for two-finger scrolling to trigger correctly.
> 
> That is because you've put the:
> 
> +	if (!tp->gesture.enabled)
> +		return GESTURE_2FG_STATE_SCROLL;
> +
> 
> Check at the end of tp_gesture_twofinger_handle_state_none(), so after the
> call to tp_gesture_get_active_touches().
> 
> None of the variables set in tp_gesture_twofinger_handle_state_none() are
> used when jumping straight to GESTURE_2FG_STATE_SCROLL, so you could do
> the check at the beginning of tp_gesture_twofinger_handle_state_none()
> (before calling tp_gesture_get_active_touches()), and then you no longer
> need to worry about ST touchpads in there.

Note: I've since merged the 3-finger pinch gesture branch, so merging this
set on top caused a couple of conflicts and things look a bit different now.

tp_gesture_twofinger_handle_state_none() was renamed and is now invoked for
3 and 4-finger states as well, so we do need a finger check to bail out.
what I will add though is a direct jump to swipe for a >2 finger gesture on
an ST touchpad, patch coming up.

> The problem I've with checking all touches, not just real touches
> in tp_gesture_get_active_touches() is that you may end up returning
> a fake touch as one of the 2 touches, and then use the coordinates
> of this fake touch to decide whether the user is doing a pinch or
> a 2fg scroll. IMHO it would be better to just jump straight to 2FG
> scrolling in this case (note currently we stay in GESTURE_2FG_STATE_NONE
> if tp_gesture_get_active_touches() fails).

the touchpad code is so that fake touches get the same
positional coordinates as the first touch points. So you can't ever trigger
a pinch on an ST device anyway, they'll always move in the same direction.
I've tried to avoid putting more semi-mt conditions in than necessary,
relying on that functionality to give us the correct coordinates.

but I think the patch for the above addresses that anyway, so I think we're
good here.

> ###
> 
> An unrelated thing which I noticed while reviewing your patches for this
> is that I'm not sure if we're handling things right in tp_get_average_touches_delta()
> currently we are counting dirty touches there and dividing the reported deltas
> by the number of dirty touches, this means that if 2fg are moving and movement
> gets reported from both in a single frame we report the averaged result of
> both moves (good), but if 2 fingers are moving, but first we get a move event
> for fg1 only, and then for fg2 only we now end up reporting double the
> move of what we would report if both moves were reported in a single frame.
> 
> This is wrong, and doubly so because we are likely to get movements in separate
> frames when the user is trying to e.g. do a slow 2fg scroll, which we now
> speed up by a factor of 2.
> 
> I believe that we should change to counting active_touches in tp_get_touches_delta
> rather then changed touches, and dividing by that when averaging.
> 
> We cannot simply use tp->gesture.finger_count since that may be different from
> the actual number of active touches when tap+dragging.

well spotted, thanks. It's hard to tell the difference, but I *think* this
is indeed the cause of the slight scroll jumps that are visible every so
often. I still get the odd one but it appears to be severely reduced.

> ###
> 
> Yet another unrelated remark, it seems we've an unused "int i" in
> tp_gesture_handle_state().

easy fix, pushed, thanks.

Cheers,
   Peter