Bug 94379

Created attachment 122090 [details]
record-jumps.txt

recap from the IRC conversation we had about this: the main cause for the finger motion seems to be the pressure on the touchpad, lifting the finger causes a jump as well. light touches don't see the jerky movements.

Created attachment 122095 [details]
reproducer on the t450s

I think I can reproduce it here as well.
This is a reproducer for Peter to try a patch

the common pattern in attachment 122095 [details] is: a series of events with x and/or y updates, then a set of updates for pressure and major only, followed by at least one x/y event. That event usually has a jump of ~20 device units and afaict from this event is then immediately followed by other small motion events.

see the events at 9.710062. X is at 850, pressure only until 10.797246 where X jumps to 870.

Created attachment 122133 [details] [review]
0001-touchpad-add-quirk-for-the-T450-generation-hardware.patch

Possible fix. This would need some extra work to only apply to this touchpad but for you guys this doesn't matter.

It avoids the jumps in benjamin's recording but I have no ideal if it feels right, the recording obviously won't show me that. Please test this and let me know how it works.

(In reply to Peter Hutterer from comment #5)
> It avoids the jumps in benjamin's recording but I have no ideal if it feels
> right, the recording obviously won't show me that. Please test this and let
> me know how it works.

It doesn't feel right unfortunately:

At low speed (when trying to reach pixels), the feeling is good IMO. It's not perfect but at least there are no more jumps. The pointer seams to be stuck from time to time, but that's pretty much the best we can do given the incoming data.

However, at regular speed of use, this patch is going to annoy users. The events are now jumpy and and the cursor is definitively not as smooth as it was.

We should try to apply the heuristic only when the speed is low and disable it most of the time...

Created attachment 122153 [details] [review]
0001-touchpad-add-quirk-for-the-T450-generation-hardware.patch

v2 of the patch, this time with a timer but more importantly, the event counting/resetting was fixed. before it would accidentally reset every so often during normal processing.

If this patch works better please also try taking out the time check (third hunk, remove the line:

+                   time - tp->quirks.nonmotion_first_time > ms2us(500)) {

Created attachment 122196 [details] [review]
0001-touchpad-add-quirk-for-the-T450-generation-hardware.patch

Ok, version 3, I just took the timeout out again. I tested this on a T450 here with the 4.4.3-300.fc23 kernel and it works pretty much as I'd expect. I still get the occasional jump on slow motion, fast motion and medium motion is fine.

OK, this one works for me. Thanks for doing the fine tuning.

You can add my tested by. We still need to make this t450s specific, and probably also test the X1 carbon3, W541 and the other laptops with similar touchpad.

https://lists.freedesktop.org/archives/wayland-devel/2016-March/027424.html

Seems that the change leads to a new bug: https://bugs.freedesktop.org/show_bug.cgi?id=94601

Patch doesn't fix the issue for me. Cursor still jumps when I remove a finger.

Fix for the issue identified in comment 3 was committed, but that's not the original issue reported (which has the jump on removing the finger)

commit a608d9dc2c70c7915fc94466ed27c1684f65409e
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Mon Mar 7 16:05:25 2016 +1000

    touchpad: add quirk for the T450 and T460 generation hardware

Created attachment 122601 [details] [review]
0001-touchpad-only-post-motion-events-if-we-have-motion.patch

Follow-up to attachment 122196 [details] [review], this should correct bug 94601 (I think). Right now I'm mostly looking for feedback if that changes the motion in some unexpected way that I haven't noticed yet, it's likely not complete yet (I don't have tests yet). If you can give it a try, that'd be much appreciated.

Probably won't fix the jump on release either, just the jerky motion when moving slowly.

Good news: I've tested attachment 122601 [details] [review] (patched version 1.2.2 with it) for about 10 hours. Bug 94601 does not occur anymore (neither the visible effect that bugs the user nor the gdm log entry).

On the other hand: I've never experienced the original bug 94379 (or didn't pay close attention).

Created attachment 122628 [details]
evemu-record-jumps-of-finger-removal.txt

Created attachment 122632 [details] [review]
0001-touchpad-reset-the-motion-history-on-significant-neg.patch

This patch should fix the cursor jump on release, at least for most cases. It goes on top of attachment 122601 [details] [review] which goes on top of today's master (455498e9).

It uses negative pressure changes to detect releases and swallows the motion. Please give it a test.

Created attachment 122633 [details]
another_evemu_record.txt

I've reduced the threshold to <= -5 in the patch above, but the recording still shows some downwards movement at the end of the touch. The issue is largely that we don't see the pressure change until the downwards finger movement has already started. 

the events starting 1.333662 show this nicely. the y movement goes from 2463 to 2473 before the final event with a pressure change which we now skip. That movement though is undetectable and indistinguishable from a normal finger movement. I don't know how to fix this short of obviously reverting the motion history patch, but doing so makes the pointer terribly unresponsive to slow motions.

just for the archives (conversation on IRC) if you can record a video of your finger position and interaction with the touchpad that'd be appreciated. it may show why you trigger this, even though I fear that there may a "you're holding it wrong" outcome...

(In reply to Peter Hutterer from comment #20)
> just for the archives (conversation on IRC) if you can record a video of
> your finger position and interaction with the touchpad that'd be
> appreciated. it may show why you trigger this, even though I fear that there
> may a "you're holding it wrong" outcome...

Here's link to a video:

https://www.dropbox.com/s/c08mgg81rvdnl1p/VID_20160414_111037094.mp4?dl=0

Created attachment 122947 [details]
evemu-record-for-video.txt

hmm, when I replay this on the current libinput master I get almost no pointer motion at the end of a touch. there may be still a pixel or two, but not on all those touches. In fact, I think it's only the first touch sequence that may have some extra movement, can you confirm this?

(In reply to Peter Hutterer from comment #23)
> hmm, when I replay this on the current libinput master I get almost no
> pointer motion at the end of a touch. there may be still a pixel or two, but
> not on all those touches. In fact, I think it's only the first touch
> sequence that may have some extra movement, can you confirm 

First 3 motions and last 3 definitely have motion at the end of touch with libinput-1.2.3. Do you want me to retry with current master?

yes please

(In reply to Peter Hutterer from comment #25)
> yes please

There's a jump in 1st, 2nd, 4th, 8th, 12th movements. It's enough to jump out of edges of some button, or change chosen menu item to another one.

ok, after a fair bit of trying I think I can reproduce this now, ca once in five attempts. I also pushed a patch to display a grid in the event gui, that makes the movement a bit easier to spot.

what I can see is a 1-2 pixel movement on finger release, when I hold my finger in a specific position and exert some pressure (more than I would usually, but not excessive). Unfortunately, that movement is impossible to spot from the event stream, pressure doesn't change significantly and it still looks the same as normal movements. So yeah, I'm almost resigned to having to re-introduce the hysteresis, the main question is now just whether it's needed for other touchpads too, haven't tested that yet.

(In reply to Peter Hutterer from comment #27)
> ok, after a fair bit of trying I think I can reproduce this now, ca once in
> five attempts. I also pushed a patch to display a grid in the event gui,
> that makes the movement a bit easier to spot.
> 
> what I can see is a 1-2 pixel movement on finger release, when I hold my
> finger in a specific position and exert some pressure (more than I would
> usually, but not excessive). Unfortunately, that movement is impossible to
> spot from the event stream, pressure doesn't change significantly and it
> still looks the same as normal movements. So yeah, I'm almost resigned to
> having to re-introduce the hysteresis, the main question is now just whether
> it's needed for other touchpads too, haven't tested that yet.

Is it possible to introduce a knob for hysteresis? Some people may want to keep it disabled.

Not for hysteresis directly, but *maybe* a more generic knob for accuracy vs. steadiness. I'm loath to expose internal implementation as a config knob.

(In reply to Peter Hutterer from comment #29)
> Not for hysteresis directly, but *maybe* a more generic knob for accuracy
> vs. steadiness. I'm loath to expose internal implementation as a config knob.

Sounds good.

long story short, hysteresis is re-enabled for all devices now. all other attempts failed.

commit 48473994c8e60189356feae7b7eae25288e5ac28
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Jun 16 16:11:56 2016 +1000

    touchpad: re-enable hysteresis by default for all devices