On Fri, Feb 21, 2014 at 10:31:44AM +0100, Hans de Goede wrote: > It is possible for a click to get reported before any related touch events > get reported, here is the relevant part of an evemu-record session on a T440s: > > E: 3.985585 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- > E: 3.997419 0003 0039 -001 # EV_ABS / ABS_MT_TRACKING_ID -1 > E: 3.997419 0001 014a 0000 # EV_KEY / BTN_TOUCH 0 > E: 3.997419 0003 0018 0000 # EV_ABS / ABS_PRESSURE 0 > E: 3.997419 0001 0145 0000 # EV_KEY / BTN_TOOL_FINGER 0 > E: 3.997419 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- > E: 5.117881 0001 0110 0001 # EV_KEY / BTN_LEFT 1 > E: 5.117881 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- > E: 5.133422 0003 0039 0187 # EV_ABS / ABS_MT_TRACKING_ID 187 > E: 5.133422 0003 0035 3098 # EV_ABS / ABS_MT_POSITION_X 3098 > E: 5.133422 0003 0036 3282 # EV_ABS / ABS_MT_POSITION_Y 3282 > E: 5.133422 0003 003a 0046 # EV_ABS / ABS_MT_PRESSURE 46 > E: 5.133422 0001 014a 0001 # EV_KEY / BTN_TOUCH 1 > E: 5.133422 0003 0000 3102 # EV_ABS / ABS_X 3102 > E: 5.133422 0003 0001 3282 # EV_ABS / ABS_Y 3282 > E: 5.133422 0003 0018 0046 # EV_ABS / ABS_PRESSURE 46 > E: 5.133422 0001 0145 0001 # EV_KEY / BTN_TOOL_FINGER 1 > E: 5.133422 0000 0000 0000 # ------------ SYN_REPORT (0) ---------- > > Notice the BTN_LEFT event all by itself! > > If this happens, it may lead to the following problem scenario: > -touch the touchpad in its right click area > -let go of the touchpad > -rapidly click in the middle area, so that BTN_LEFT gets reported before the > new coordinates (such as seen in the trace above, this may require some > practicing with evemu-record to reproduce) > -the driver registers the click as a right click because it uses the > old coordinates from the cumulative coordinates to determine the > click location > > This commit fixes this by: > 1) Resetting the cumulative coordinates not only when no button is pressed, > but also when there is no finger touching the touchpad, so that when > we do get a touch the cumulative coordinates start at the right place > 2) Delaying processing the BTN_LEFT down transition if there is no finger > touching the touchpad > > This approach has one downside, if we wrongly identify a touchpad as > a clickpad, then the left button won't work unless the user touches the > touchpad while clicking the left button. > > If we want we can fix this by doing something like this: > 1) Making update_hw_button_state return a delay; and > 2) Tracking that we've delayed BTN_LEFT down transition processing; and > 3) When we've delayed BTN_LEFT down transition return a small delay value; and > 4) If when we're called again we still don't have a finger down, just > treat the click as a BTN_LEFT > > But this is not worth the trouble IMHO, the proper thing to do in this > scenario is to fix the mis-identification of the touchpad as a clickpad.
indeed, this is not something we need to hack around. this series is Reviewed-by: Peter Hutterer <[email protected]> except for the property patch, I'll do some more testing with my clickpad over the week though. Cheers, Peter > > Signed-off-by: Hans de Goede <[email protected]> > --- > src/eventcomm.c | 5 +++-- > src/synaptics.c | 7 +++++++ > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/src/eventcomm.c b/src/eventcomm.c > index fe57aa8..231afbc 100644 > --- a/src/eventcomm.c > +++ b/src/eventcomm.c > @@ -675,8 +675,9 @@ EventReadHwState(InputInfoPtr pInfo, > > SynapticsResetTouchHwState(hw, FALSE); > > - /* Reset cumulative values if buttons were not previously pressed */ > - if (!hw->left && !hw->right && !hw->middle) { > + /* Reset cumulative values if buttons were not previously pressed, > + * or no finger was previously present. */ > + if ((!hw->left && !hw->right && !hw->middle) || hw->z < > para->finger_low) { > hw->cumulative_dx = hw->x; > hw->cumulative_dy = hw->y; > sync_cumulative = TRUE; > diff --git a/src/synaptics.c b/src/synaptics.c > index bd14100..59fcf48 100644 > --- a/src/synaptics.c > +++ b/src/synaptics.c > @@ -2810,6 +2810,12 @@ update_hw_button_state(const InputInfoPtr pInfo, > struct SynapticsHwState *hw, > if (para->clickpad) { > /* hw->left is down, but no other buttons were already down */ > if (!(priv->lastButtons & 7) && hw->left && !hw->right && > !hw->middle) { > + /* If the finger down event is delayed, the x and y > + * coordinates are stale so we delay processing the click */ > + if (hw->z < para->finger_low) { > + hw->left = 0; > + goto out; > + } > if (is_inside_rightbutton_area(para, hw->x, hw->y)) { > hw->left = 0; > hw->right = 1; > @@ -2841,6 +2847,7 @@ update_hw_button_state(const InputInfoPtr pInfo, struct > SynapticsHwState *hw, > if (hw->left && !(priv->lastButtons & 7) && hw->numFingers >= 1) > handle_clickfinger(priv, hw); > > +out: > /* Two finger emulation */ > if (hw->numFingers == 1 && hw->z >= para->emulate_twofinger_z && > hw->fingerWidth >= para->emulate_twofinger_w) { > -- > 1.9.0 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel > _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
