Bug 66532 - AreaBottomEdge is not always ignored and sometimes causes other touches to be ignored
AreaBottomEdge is not always ignored and sometimes causes other touches to be...
Status: NEW
Product: xorg
Classification: Unclassified
Component: Input/synaptics
unspecified
Other All
: medium normal
Assigned To: Peter Hutterer
:
Depends on: 66534
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-03 06:17 UTC by Jeremy Huddleston
Modified: 2015-02-03 01:40 UTC (History)
8 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Xorg log (15.02 KB, text/plain)
2013-07-03 06:30 UTC, Jeremy Huddleston
no flags Details
updated patch for ignoring touches in click area (2.02 KB, patch)
2014-06-08 06:40 UTC, Kit Westneat
no flags Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Huddleston 2013-07-03 06:17:38 UTC
I have a clickpad, and I'm trying to get synaptics to behave well with it.


I really want an "IgnoreThumb" option which would allow me to have one finger (my thumb) pressing down on the button while the other finger drags (eg an icon).

To approximate that, I decided to try defining AreaBottomEdge (to 85%).  If I move my finger into that region, the mouse cursor stops moving.  Great.  The only problem is that if I have a finger in that region, *ALL* movement is ignored (not just the one within that region).

For example:
  1) If I am moving my finger in the region above the bottom, the cursor will move, but as soon as I touch the bottom region with another finger, the cursor will stop moving, and scroll events will be generated instead (because it sees two fingers).
  2) In the opposite case, if I start with one finger in the bottom region and then place another finger above it and start moving it, there is no cursor movement nor scroll events.

In both of those example cases, I would expect the touch in the AreaBottomEdge to be ignored and the movement above the edge to translate the cursor.
Comment 1 Jeremy Huddleston 2013-07-03 06:29:19 UTC
Ignore my "*ALL* movement is ignored" line as I realized this wasn't quite the case when I was providing the two examples.  The examples are the correct cases.

synaptics 1.6.2
Comment 2 Jeremy Huddleston 2013-07-03 06:30:00 UTC
Created attachment 81937 [details]
Xorg log
Comment 3 Peter Hutterer 2013-07-04 06:59:42 UTC
the basic problem here is that (for historical reasons) the driver doesn't always deal with to separate points but rather one x/y coordinate and the extra info that there's another one or two fingers down. there are touchpads that won't give you more than that anyway.

so the code is optimised for that and no-one found the time yet to fix this, sorry. it's not really a bug as such, it's a missing feature.

culprit is "if (!inside_active_area) reset_hw_state(hw)" in HandleState(). That's the starting point. The end point will likely be drinking island.
Comment 4 Vaughn Cato 2013-12-18 23:07:18 UTC
This simple change seems to make it much more usable:

--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -2825,6 +2825,10 @@ HandleState(InputInfoPtr pInfo, struct SynapticsHwState *hw, CARD32 n
 
     inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
 
+    if (para->clickpad && hw->left) {
+      inside_active_area = TRUE;
+    }
+
     /* these two just update hw->left, right, etc. */
     update_hw_button_state(pInfo, hw, priv->old_hw_state, now, &delay);
 

It simply pretends like the finger is inside the active area when the clickpad is clicked whether it actually is or not.  It's not perfect.  True multi-touch handling would be better, but at least it is usable.
Comment 5 Kit Westneat 2014-01-06 02:40:01 UTC
This seems to work too:
@@ -2821,7 +2827,35 @@ HandleState(InputInfoPtr pInfo, struct S
      * like flicker in scrolling or noise motion. */
     filter_jitter(priv, &hw->x, &hw->y);
 
+    /* validate all the multitouches and remove touches outside active area */
     inside_active_area = is_inside_active_area(priv, hw->x, hw->y);
+    if (para->clickpad) {
+	hw->numFingers = 0;
+        for (i = 0; i < hw->num_mt_mask; i++) {
+            ValuatorMask *f1;
+            Bool mt_inside;
+            double x1, y1;
+
+            if (hw->slot_state[i] == SLOTSTATE_EMPTY ||
+                    hw->slot_state[i] == SLOTSTATE_CLOSE)
+                continue;
+
+            f1 = hw->mt_mask[i];
+            x1 = valuator_mask_get_double(f1, 0);
+            y1 = valuator_mask_get_double(f1, 1);
+            mt_inside = is_inside_active_area(priv, x1, y1);
+            if (!mt_inside) {
+                hw->slot_state[i] = SLOTSTATE_EMPTY;
+                continue;
+            }
+            if (!inside_active_area) {
+                inside_active_area = TRUE;
+                hw->x = x1;
+                hw->y = y1;
+            }
+	    hw->numFingers++;
+        }
+    }
 
     /* these two just update hw->left, right, etc. */
     update_hw_button_state(pInfo, hw, priv->old_hw_state, now, &delay);

This builds on the previous patch, but attempts to throw away all touches in the inactive zone. If it finds any touches in the active zone, it sets the inside_active_area boolean. It seems to work pretty well for my uses.
Comment 6 Niklas Kullberg 2014-02-02 11:06:21 UTC
I can confirm that kit.westneat's patch works. It makes my MacBook Pro touchpad completely usable.

I also removed "BUG_WARN(priv->num_active_touches == 0);" from line 2657 in synaptics.c to prevent spamming the log with messages.
Comment 7 Gabriele 2014-02-24 16:55:37 UTC
Unfortunately http://cgit.freedesktop.org/xorg/driver/xf86-input-synaptics/commit/?id=945acfc261707be78cbc5438c22b061e20076004 breaks kit.westneat's patch.

I'd like also to point out that when kit.westneat's patch is used, there are three behaviours depending on the order in which fingers are positioned:

First finger: active area
Second finger: inactive area
Third finger: active area
Everything works as expected.

First finger: inactive area
Second finger: active area
Third finger: active area
Two finger scrolling doesn't work.

First finger: active area
Second finger: active area
Third finger: inactive area
A fourth finger in the active area is required.

Apart from that, the patch makes trackpads a lot more usable.
Comment 8 Peter Hutterer 2014-03-13 06:40:18 UTC
Can you guys give the current git master a35b2d629d85d7a8c82621a5098a17e5ffb341dc a try please?

Based on Jeremy's original description to "I really want an "IgnoreThumb" option which would allow me to have one finger (my thumb) pressing down on the button while the other finger drags (eg an icon)." this bug should be fixed now, without any special configuration or extra patches required.
Comment 9 Jeremy Huddleston 2014-03-13 16:51:45 UTC
Yeah, I'll give it a whirl as soon as I figure out where I stashed the relevant system...
Comment 10 Jeremy Huddleston 2014-03-14 08:40:48 UTC
It's unfortunately not behaving as I'd hope.  This is using 4122db68f6 which includes the "(fixed version)" and default options.

When the button is pressed, the behavior is as I would expect (the thumb pressing the button is ignored).
When I release the button and leave my thumb in contact with the in that button region, the thumb is not ignored and two finger events are sent (at a higher level, usually scrolling instead of pointer movement).

Log to show detected / default configurations:

[  3769.774] (II) config/udev: Adding input device Cypress APA Trackpad (cyapa) (/dev/input/event5)
[  3769.774] (**) Cypress APA Trackpad (cyapa): Applying InputClass "evdev touchpad catchall"
[  3769.774] (**) Cypress APA Trackpad (cyapa): Applying InputClass "touchpad catchall"
[  3769.774] (**) Cypress APA Trackpad (cyapa): Applying InputClass "Default clickpad buttons"
[  3769.775] (**) Cypress APA Trackpad (cyapa): Applying InputClass "Cypress APA Trackpad (Samsung ARM Chromebook) Quirks for Synaptics"
[  3769.775] (II) LoadModule: "synaptics"
[  3769.775] (II) Loading /usr/lib/xorg/modules/input/synaptics_drv.so
[  3769.776] (II) Module synaptics: vendor="X.Org Foundation"
[  3769.776] 	compiled for 1.14.5, module version = 1.7.99
[  3769.776] 	Module class: X.Org XInput Driver
[  3769.776] 	ABI class: X.Org XInput driver, version 19.1
[  3769.776] (II) Using input driver 'synaptics' for 'Cypress APA Trackpad (cyapa)'
[  3769.776] (**) Cypress APA Trackpad (cyapa): always reports core events
[  3769.776] (**) Option "Device" "/dev/input/event5"
[  3769.970] (II) synaptics: Cypress APA Trackpad (cyapa): found clickpad property
[  3769.971] (--) synaptics: Cypress APA Trackpad (cyapa): x-axis range 0 - 1280 (res 13)
[  3769.972] (--) synaptics: Cypress APA Trackpad (cyapa): y-axis range 0 - 680 (res 12)
[  3769.972] (--) synaptics: Cypress APA Trackpad (cyapa): pressure range 0 - 255
[  3769.972] (II) synaptics: Cypress APA Trackpad (cyapa): device does not report finger width.
[  3769.972] (--) synaptics: Cypress APA Trackpad (cyapa): buttons: left double triple
[  3769.973] (--) synaptics: Cypress APA Trackpad (cyapa): Vendor 0 Product 0
[  3769.973] (--) synaptics: Cypress APA Trackpad (cyapa): invalid finger width range.  defaulting to 0 - 15
[  3769.973] (**) Option "FingerLow" "5"
[  3769.974] (**) Option "FingerHigh" "5"
[  3769.975] (**) Option "ClickPad" "true"
[  3769.976] (**) Option "TapButton1" "0"
[  3769.976] (**) Option "TapButton2" "0"
[  3769.977] (**) Option "TapButton3" "0"
[  3769.977] (**) Option "PalmDetect" "true"
[  3769.978] (**) Option "SoftButtonAreas" "65% 0 85% 0 0 0 0 0"
[  3769.978] (--) synaptics: Cypress APA Trackpad (cyapa): touchpad found
[  3769.979] (**) Cypress APA Trackpad (cyapa): always reports core events
[  3770.065] (**) Option "config_info" "udev:/sys/devices/s3c2440-i2c.1/i2c-1/1-0067/input/input5/event5"
[  3770.066] (II) XINPUT: Adding extended input device "Cypress APA Trackpad (cyapa)" (type: TOUCHPAD, id 7)
[  3770.067] (**) synaptics: Cypress APA Trackpad (cyapa): (accel) MinSpeed is now constant deceleration 2.5
[  3770.067] (**) synaptics: Cypress APA Trackpad (cyapa): (accel) MaxSpeed is now 1.75
[  3770.067] (**) synaptics: Cypress APA Trackpad (cyapa): (accel) AccelFactor is now 0.138
[  3770.071] (**) Cypress APA Trackpad (cyapa): (accel) keeping acceleration scheme 1
[  3770.072] (**) Cypress APA Trackpad (cyapa): (accel) acceleration profile 1
[  3770.072] (**) Cypress APA Trackpad (cyapa): (accel) acceleration factor: 2.000
[  3770.072] (**) Cypress APA Trackpad (cyapa): (accel) acceleration threshold: 4
[  3770.074] (--) synaptics: Cypress APA Trackpad (cyapa): touchpad found
Comment 11 Jeremy Huddleston 2014-03-14 08:43:05 UTC
(In reply to comment #10)
> Log to show detected / default configurations:
I take that back.  I did have an xorg.conf section setting that up, but those options match the experience reported.
Comment 12 Kit Westneat 2014-06-08 06:40:28 UTC
Created attachment 100632 [details] [review]
updated patch for ignoring touches in click area

I updated the previous patch I pasted, and added it as an attachment. I'm not sure if this patch takes the best approach, but I thought I'd share it in case it helps anyone else. Click and drag is still not working great in 1.7.6 for me without this patch. BTW this version no longer requires removing the BUG_WARN.