[PATCH libinput 0/7] Handle jumping cursors

[Hans de Goede]

Hi,

On 09/18/2014 06:20 AM, Peter Hutterer wrote:
> 
> When a finger leaves the touchpad at the same time as another finger sets
> down, a touchpad may not notice the change in fingers but rather think the
> touchpoint moved to the new position. Bug 76722 has an event sequence but it
> really boils down to just a move of a touch, identifiable only by the
> distance moved.
> 
> https://bugs.freedesktop.org/show_bug.cgi?id=76722
> 
> I've done some measurements on a T440 and a x220 and the most one can
> reasonably move on those two is under 20mm. Use the script at the url below
> if you want to verify yourself.
> https://github.com/whot/input-data-analysis/tree/master/touchpad-max-delta
> 
> The idea of the fix was to consider a jumping touch a changing touch, i.e.
> end it and begin it at the new position. This also requires the new touch to
> be tap-capable, recognised by the softbutton code, etc.
> 
> Handling this is a bit tricky because we handle the state of the touchpad at
> EV_SYN time, so we can't easily split event handling. Since we don't know
> which touchpoint moves (could be the second one, in theory) we'd have to do
> double-processing of the state otherwise, i.e. pull all events up to the
> EV_SYN, check each touchpoint for jumps, release those, process the state,
> then re-process the events again. The approach I've picked here is to handle
> the state twice (rather than the events), ignoring certain things the first
> time and other things the second time.
> 
> Benjamin is currently looking into a (hopefully simpler) kernel fix so maybe
> we won't need this patchset anyway, but I prefer it to be archived on the
> list either way. All that aside, patches 1-3 are useful anyway.
> 
> All available on 
> https://github.com/whot/libinput/tree/wip/jumping-cursor

Overall this sets looks good, I've found 1 issue, and I've 1 nitpick,

First the issue in "[PATCH libinput 5/7] touchpad: only send button events if we have button events queued"
you seem to have forgotten to change tp_post_softbutton_buttons.

All other patches look good and are:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

And the nitpick is for "[PATCH libinput 6/7] touchpad: detect coordinate jumps on touchpads" :

+		/* ditch release events, re-instate press */
+		tp->queued &= ~TOUCHPAD_EVENT_BUTTON_RELEASE;
+		if (queued & TOUCHPAD_EVENT_BUTTON_PRESS)
+			tp->queued |= TOUCHPAD_EVENT_BUTTON_PRESS;

You may want to simplify this to:

+		/* ditch release events, re-instate press */
+		tp->queued = queued & ~TOUCHPAD_EVENT_BUTTON_RELEASE;

Regards,

Hans