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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
