Bug 86807

Created attachment 110289 [details] [review]
[PATCH] touchpad: Allow the center of pinned fingers to drift over time

Hi,

It seems that what you're describing here are 2 distinct issues:

(In reply to Peter Hutterer from comment #0)
> Once two-finger scrolling is triggered, pointer movement stops until all
> fingers are lifted off the touchpad. This is problematic during
> drag-and-drop, if the finger holding the button down moves enough to trigger
> a single scroll event libinput switches to scroll and you can't get out of
> that mode without lifting the finger (and ending the drag).

The problem here is not 2fg scrolling triggering, the problem is the finger getting unpinned, once unpinned it counts for 2fg scrolling, and as such as long as 2 fg are down you will be in a 2fg scrolling situation. Note that this is only a problem when doing click + drag outside the button areas.

We could potentially fix this by doing something like the attached patch, can you give this a try ?

> another way to directly reproduce this as well:
> 
> 10:45 < Jasper> whot, what I do: press down index on clickpad, move index 
>                 around a bit
> 10:46 < Jasper> Realize I need more drag range, press middle down and move 
>                 middle
> 
> What libinput should do is recognise that there's only one finger left and
> go back to normal pointer motion.

This is a different problem, here the clicking finger is also the one doing the dragging, so here the unpin is deliberate. 2fg scrolling may or may not trigger here, the problem is that when the 2nd finger comes down the 1st one is the pointer, and we do not look for a new pointer candidate when the 1st finger lifts. AFAIK this is deliberate, we don't want which finger is the pointer to be jumping between fingers when there is an (accidental) lift.

What we could do here is set a timer of 300 ms when the finger which is the pointer lifts, and if no new pointer is assigned when that timer expires, search for the first active finger (so not pinned and not in a soft button area) and make that the pointer.

I think that that is a good solution, if you agree I can implement this.

Regards,

Hans

(In reply to Hans de Goede from comment #1
> This is a different problem, here the clicking finger is also the one doing
> the dragging, so here the unpin is deliberate. 2fg scrolling may or may not
> trigger here, the problem is that when the 2nd finger comes down the 1st one
> is the pointer, and we do not look for a new pointer candidate when the 1st
> finger lifts. AFAIK this is deliberate, we don't want which finger is the
> pointer to be jumping between fingers when there is an (accidental) lift.

I do not lift the first pointer. Sequence:

Put index finger down on touchpad.
Click touchpad down with index to start drag.
Move index around on touchpad.

Realize I don't have enough range. Push down middle, and move middle around index while index is still on touchpad. When I want to drop, I release middle and then index.

I notice here that xf86-input-synaptics never emits scrolling events while the clickpad is pushed down. That could be a solution.

Hi,

(In reply to Jasper St. Pierre from comment #2)
> (In reply to Hans de Goede from comment #1
> > This is a different problem, here the clicking finger is also the one doing
> > the dragging, so here the unpin is deliberate. 2fg scrolling may or may not
> > trigger here, the problem is that when the 2nd finger comes down the 1st one
> > is the pointer, and we do not look for a new pointer candidate when the 1st
> > finger lifts. AFAIK this is deliberate, we don't want which finger is the
> > pointer to be jumping between fingers when there is an (accidental) lift.
> 
> I do not lift the first pointer. Sequence:
> 
> Put index finger down on touchpad.
> Click touchpad down with index to start drag.
> Move index around on touchpad.
> 
> Realize I don't have enough range. Push down middle, and move middle around
> index while index is still on touchpad. When I want to drop, I release
> middle and then index.

Ah, I thought you used your middle finger to take over from the index finger, but now I see what you're doing. 

> I notice here that xf86-input-synaptics never emits scrolling events while
> the clickpad is pushed down. That could be a solution.

Hm, yes that should actually work for both your and Peter's problem, but it won't fix the variant of your problem as I thought you were doing things :)

So I think there is probably still value in both my suggested fixes too. Peter what do you think ?

(In reply to Hans de Goede from comment #3)
> > I do not lift the first pointer. Sequence:
> > 
> > Put index finger down on touchpad.
> > Click touchpad down with index to start drag.
> > Move index around on touchpad.
> > 
> > Realize I don't have enough range. Push down middle, and move middle around
> > index while index is still on touchpad. When I want to drop, I release
> > middle and then index.
> 
> Ah, I thought you used your middle finger to take over from the index
> finger, but now I see what you're doing. 
> 
> > I notice here that xf86-input-synaptics never emits scrolling events while
> > the clickpad is pushed down. That could be a solution.
> 
> Hm, yes that should actually work for both your and Peter's problem

Ok, so thinking more about this, I take this back, not doing 2fg scrolling when the clickpad is clicked down will not help for your scenario. We always make one touch the "pointer", and in your case that is the index finger, and it will stay the pointer as long as its down.

The reason this works with synaptics is because it lacks proper multi-touch support, so as soon as a clickpad is clicked down we switch to relative mode, and all fingers move the pointer at the same time, which is a mess.

This does not mean your suggestion does not have merits, but I'm afraid that your use-case is one which we simply do not want to support. The "expected" way to use the touchpad for click + drag from a libinput pov is to click with one finger, and then select with another, which will make the other finger the pointer from the get go, switching which finger is the pointer halfway through is not supported, and I don't think we want to support that.

I'm sorry my muscle memory works the way it does. I guess I won't be using Wayland any time soon then.

(In reply to Hans de Goede from comment #1)
> The problem here is not 2fg scrolling triggering, the problem is the finger
> getting unpinned, once unpinned it counts for 2fg scrolling, and as such as
> long as 2 fg are down you will be in a 2fg scrolling situation. Note that
> this is only a problem when doing click + drag outside the button areas.

correct, but "outside" also means "moves outside". And that was what happened to me when I was using my thumb to click. As the touchpoint changes shape during hand movement, the actual touch point may move outside the buttons and trigger scrolling. User-visible change is that movement stops suddenly.


 +------------------------+
 |          __            |
 |         /Y \           |
 |         \   \          |
 |          \   \  ______ |
 |     __    \   \/   \   \
 +----/  \---+\   \    \   \
 |    \ X \  | | 
 +-----\   \-+-|
        \   \__|

As Y moves around, the angle of the thumb changes (rolls sideways or forward)
and X moves up and down. That can move it outside the button area. I'll attach evemu recordings to illustrate the point.

fwiw, with the patch I get occasional cursor jumps and errors of "invalid event, no fingers are down" (the errors seem to be unrelated to the patch, see #86917).

I think not scrolling while dragging is a good solution for now. We originally allowed it because it's not that uncommon to have to scroll while dragging but if it breaks a more common use-case then we should go for the more common one.

I'd like us to bounce around some more ideas on how we can fix Jasper's use-case though, I don't think it's that uncommon. Jasper, can you attach a few evemu recordings with common uses, preferable in all directions? maybe there's some common data that we can easily detect.

Created attachment 110328 [details]
unintentional-scroll.evemu

Created attachment 110329 [details]
unintentional-scroll2.evemu

(In reply to Peter Hutterer from comment #6)
> (In reply to Hans de Goede from comment #1)
> > The problem here is not 2fg scrolling triggering, the problem is the finger
> > getting unpinned, once unpinned it counts for 2fg scrolling, and as such as
> > long as 2 fg are down you will be in a 2fg scrolling situation. Note that
> > this is only a problem when doing click + drag outside the button areas.
> 
> correct, but "outside" also means "moves outside". And that was what
> happened to me when I was using my thumb to click. As the touchpoint changes
> shape during hand movement, the actual touch point may move outside the
> buttons and trigger scrolling. User-visible change is that movement stops
> suddenly.
> 
> 
>  +------------------------+
>  |          __            |
>  |         /Y \           |
>  |         \   \          |
>  |          \   \  ______ |
>  |     __    \   \/   \   \
>  +----/  \---+\   \    \   \
>  |    \ X \  | | 
>  +-----\   \-+-|
>         \   \__|
> 
> As Y moves around, the angle of the thumb changes (rolls sideways or forward)
> and X moves up and down. That can move it outside the button area. I'll
> attach evemu recordings to illustrate the point.
> 
> fwiw, with the patch I get occasional cursor jumps and errors of "invalid
> event, no fingers are down" (the errors seem to be unrelated to the patch,
> see #86917).
> 
> I think not scrolling while dragging is a good solution for now.

On clickpads yes, ack. That should solve your use-case.

I still think the patch I attached is a good idea regardless though, shall I post it to the list for review ?

> We originally allowed it because it's not that uncommon to have to scroll while
> dragging but if it breaks a more common use-case then we should go for the
> more common one.

Ok.

> I'd like us to bounce around some more ideas on how we can fix Jasper's
> use-case though, I don't think it's that uncommon. Jasper, can you attach a
> few evemu recordings with common uses, preferable in all directions? maybe
> there's some common data that we can easily detect.

How about simply counting (combined) movement from all "active" (so not pinned not in button area) fingers when a clickpad is clicked down. Or maybe go one step further and simply always report movement from all active fingers ?

(In reply to Hans de Goede from comment #9)
> I still think the patch I attached is a good idea regardless though, shall I
> post it to the list for review ?

yeah, fine with me, just double-check it for cursor jumps please. I saw a few of them while testing with this patch and though I don't see why this patch would've triggered them make sure anyway. Usage was as illustrated above, but I don't know the exact sequence triggering jumps, they were sporadic (but frequent enough).


> > I'd like us to bounce around some more ideas on how we can fix Jasper's
> > use-case though, I don't think it's that uncommon. Jasper, can you attach a
> > few evemu recordings with common uses, preferable in all directions? maybe
> > there's some common data that we can easily detect.
> 
> How about simply counting (combined) movement from all "active" (so not
> pinned not in button area) fingers when a clickpad is clicked down. Or maybe
> go one step further and simply always report movement from all active
> fingers ?

I think combined movement should work well enough, though it needs testing whether finger movement on the 'holding finger' would interfere with actual movement. There's likely some more complex solution to switch the pointer finger once a movement threshold is met but let's see if the simple solution is enough (synaptics would indicate it is)

(In reply to Peter Hutterer from comment #10)
> > How about simply counting (combined) movement from all "active" (so not
> > pinned not in button area) fingers when a clickpad is clicked down. Or maybe
> > go one step further and simply always report movement from all active
> > fingers ?
> 
> I think combined movement should work well enough, though it needs testing
> whether finger movement on the 'holding finger' would interfere with actual
> movement. There's likely some more complex solution to switch the pointer
> finger once a movement threshold is met but let's see if the simple solution
> is enough (synaptics would indicate it is)

Ok, before I implement this, one question, do you only want to do combined movement when a clickpad is "clicked", or do you want to do it always and drop the whole notion of one touch being the pointer?

In case it is not clear I'm not 100% sure which way is best, which is why I'm asking :)

(In reply to Peter Hutterer from comment #10)
> I think combined movement should work well enough, though it needs testing
> whether finger movement on the 'holding finger' would interfere with actual
> movement. There's likely some more complex solution to switch the pointer
> finger once a movement threshold is met but let's see if the simple solution
> is enough (synaptics would indicate it is)

Just realized I did not reply to your worries about the holding finger interfering, that is why we've pinning as well as ignoring motion in the button areas, I think those two combined should solve that issue nicely.

(In reply to Hans de Goede from comment #11)
> Ok, before I implement this, one question, do you only want to do combined
> movement when a clickpad is "clicked", or do you want to do it always and
> drop the whole notion of one touch being the pointer?
> 
> In case it is not clear I'm not 100% sure which way is best, which is why
> I'm asking :)

when the clickpad is clicked. generally, I don't think there's a big use-case for truly combining the motion, I don't think people intentionally move two fingers. so having the concept of one touch being the pointer seems to be right.

so it really comes down to getting the right movement when two fingers move and one movement is unintentional and outside of the button area. That's where the move combination is the simplest approach, and afaict this use-case only happens during drag and drop. 

The only other case where there are two fingers on the touchpad is when one of them is in the button area _waiting_ for a click. But that should already work anyway.

Preferrably: if two fingers have contact while the touchpad is clicked, then if one moves together with the other, it's a scroll, else, the delta is sent as a mouse movement.

So if I keep one still and move the other, it moves the mouse, no matter which finger is moving. If there's two down and they move at the same time in the same direction, that's a scroll. If they both move in opposite directions or whatever, I don't care what you do.

Sorry about not being able to send the evemu-record, I've been fairly busy recently and have been away from my laptop with the touchpad.

Jasper,

I've just posted a libinput patch-series to wayland-devel which fixes your use-case. At least it does for me, feedback appreciated.

Note this does not differentiate between scrolling and pointer motion, it simply turns all motion into pointer motion when a clickpad is clicked (at least for now).

Regards,

Hans

commit 6664144611dce94ae42e0e88495d3955f5823de8
Author: Hans de Goede <hdegoede@redhat.com>
Date:   Tue Dec 9 12:47:10 2014 +0100

    touchpad: When a clickpad is clicked post combined motion of all touches

commit 6028e126fe006d0099aee965db669e8752f7bd42
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Dec 18 10:37:38 2014 +1000

    touchpad: revert to pointer movement when stopping twofinger scrolling