Re: [PATCH xf86-input-synaptics 8/8] Wait for *new* coordinates on a clickpad click before reporting the click
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 # SYN_REPORT (0) --
E: 3.997419 0003 0039 -001# EV_ABS / ABS_MT_TRACKING_ID -1
E: 3.997419 0001 014a # EV_KEY / BTN_TOUCH0
E: 3.997419 0003 0018 # EV_ABS / ABS_PRESSURE 0
E: 3.997419 0001 0145 # EV_KEY / BTN_TOOL_FINGER 0
E: 3.997419 # SYN_REPORT (0) --
E: 5.117881 0001 0110 0001# EV_KEY / BTN_LEFT 1
E: 5.117881 # 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_X3098
E: 5.133422 0003 0036 3282# EV_ABS / ABS_MT_POSITION_Y3282
E: 5.133422 0003 003a 0046# EV_ABS / ABS_MT_PRESSURE 46
E: 5.133422 0001 014a 0001# EV_KEY / BTN_TOUCH1
E: 5.133422 0003 3102# EV_ABS / ABS_X3102
E: 5.133422 0003 0001 3282# EV_ABS / ABS_Y3282
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 # 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 peter.hutte...@who-t.net 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 hdego...@redhat.com
---
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
[PATCH xf86-input-synaptics 8/8] Wait for *new* coordinates on a clickpad click before reporting the click
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 # SYN_REPORT (0) --
E: 3.997419 0003 0039 -001 # EV_ABS / ABS_MT_TRACKING_ID -1
E: 3.997419 0001 014a # EV_KEY / BTN_TOUCH0
E: 3.997419 0003 0018 # EV_ABS / ABS_PRESSURE 0
E: 3.997419 0001 0145 # EV_KEY / BTN_TOOL_FINGER 0
E: 3.997419 # SYN_REPORT (0) --
E: 5.117881 0001 0110 0001 # EV_KEY / BTN_LEFT 1
E: 5.117881 # 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_X3098
E: 5.133422 0003 0036 3282 # EV_ABS / ABS_MT_POSITION_Y3282
E: 5.133422 0003 003a 0046 # EV_ABS / ABS_MT_PRESSURE 46
E: 5.133422 0001 014a 0001 # EV_KEY / BTN_TOUCH1
E: 5.133422 0003 3102 # EV_ABS / ABS_X3102
E: 5.133422 0003 0001 3282 # EV_ABS / ABS_Y3282
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 # 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.
Signed-off-by: Hans de Goede hdego...@redhat.com
---
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
___
xorg-devel@lists.x.org: X.Org development
Archives:
