[Linuxwacom-devel] [PATCH 2/2] Further reduce arbitration priority of cursor devices

[Jason Gerecke]

Cursor (puck) devices are typically left on the sensor and can emit
spurious events that can potentially cause the driver to steal "active"
status from another tool. In the past we'd only considered the case where
the active tool was a touch but it is reasonable to extend this logic to
other tool types. If a cursor emits a spurious event in the middle of a
pen dragging, for example, the drag will momentarily stop as the pen is
temporarily sent out of prox -- with potentially disasterous results.
Additionally, when the pen "returns" in prox on the next event, any
currently-applied pressure will be taken as the preload and result in
incorrect pressure scaling until the pen is removed from contact.

This commit removes the "IsTouch" conditions from the handling of
spurious cursor events in check_arbitrated_control. The result is that a
cursor will not be granted "active" status in preference to any other
tool (and will drop "active" status if gained after 100ms and while
no buttons are pressed).

Signed-off-by: Jason Gerecke <***@wacom.com>
---
src/wcmCommon.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/wcmCommon.c b/src/wcmCommon.c
index 4fbe775..1471fa5 100644
--- a/src/wcmCommon.c
+++ b/src/wcmCommon.c
@@ -854,29 +854,38 @@ static Bool check_arbitrated_control(InputInfoPtr pInfo, WacomDeviceStatePtr ds)

if (IsPad(priv)) {
/* Pad may never be the "active" pointer controller */
+ DBG(6, priv, "Event from pad; not yielding pointer control\n.");
return FALSE;
}

if (active == NULL || active->oldState.device_id == ds->device_id) {
- DBG(11, priv, "Same device ID as active; allowing access.\n");
+ DBG(11, priv, "Event from active device; maintaining pointer control.\n");
return TRUE;
}
- else if (IsCursor(active) && IsTouch(priv)) {
- /* Cursor devices are often left idle in range, so allow touch to
- * grab control if the tool has not been used for some time.
+ else if (IsCursor(active)) {
+ /* Cursor devices are often left idle in range, so allow other devices
+ * to grab control if the tool has not been used for some time.
*/
- return (ds->time - active->oldState.time > 100) && !active->oldState.buttons;
+ Bool yield = (ds->time - active->oldState.time > 100) && !active->oldState.buttons;
+ DBG(6, priv, "Currently-active cursor %s idle; %s pointer control.\n",
+ yield ? "is" : "is not", yield ? "yielding" : "not yielding");
+ return yield;
}
- else if (IsTouch(active) && IsCursor(priv)) {
+ else if (IsCursor(priv)) {
/* An otherwise idle cursor may still occasionally jitter and send
- * events while the user is making active touches. Do not allow
- * the cursor to grab control in this particular case.
+ * events while the user is actively using other tools or touching
+ * the device. Do not allow the cursor to grab control in this
+ * particular case.
*/
+ DBG(6, priv, "Event from non-active cursor; not yielding pointer control.\n");
return FALSE;
}
else {
/* Non-touch input has priority over touch in general */
- return !IsTouch(priv);
+ Bool yield = !IsTouch(priv);
+ DBG(6, priv, "Event from non-active %s device; %s pointer control.\n",
+ yield ? "non-touch" : "touch", yield ? "yielding" : "not yielding");
+ return yield;
}
}

[Peter Hutterer]

Cursor (puck) tools tend to be left on the sensor, preventing us from
being able to rely solely on proximity information to determine if
they are being actively used. In the past we've used the amount of time
since the last event as an indicator of activity and allowed other
devices to grab control of the pointer if more than 100 milliseconds had
elapsed since the cursor's last event. Although this seems to work well,
button state. If a user is pressing a button on their cursor tool, it
should be considered active even if the 100ms timeout has been exceeded.
Not doing so could potentially allow another tool to grab "active"
status and have our driver send a button-up message and stop an in-
progress drag.
---
src/wcmCommon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/wcmCommon.c b/src/wcmCommon.c
index 28e5488..4fbe775 100644
--- a/src/wcmCommon.c
+++ b/src/wcmCommon.c
@@ -865,7 +865,7 @@ static Bool check_arbitrated_control(InputInfoPtr pInfo, WacomDeviceStatePtr ds)
/* Cursor devices are often left idle in range, so allow touch to
* grab control if the tool has not been used for some time.
*/
- return (ds->time - active->oldState.time > 100);
+ return (ds->time - active->oldState.time > 100) && !active->oldState.buttons;

I'd prefer == 0 here but

Reviewed-by: Peter Hutterer <***@who-t.net>

Cheers,
Peter

}
else if (IsTouch(active) && IsCursor(priv)) {
/* An otherwise idle cursor may still occasionally jitter and send
--
2.10.1

[Peter Hutterer]

Post by Jason Gerecke
Cursor (puck) devices are typically left on the sensor and can emit
spurious events that can potentially cause the driver to steal "active"
status from another tool. In the past we'd only considered the case where
the active tool was a touch but it is reasonable to extend this logic to
other tool types. If a cursor emits a spurious event in the middle of a
pen dragging, for example, the drag will momentarily stop as the pen is
temporarily sent out of prox -- with potentially disasterous results.
Additionally, when the pen "returns" in prox on the next event, any
currently-applied pressure will be taken as the preload and result in
incorrect pressure scaling until the pen is removed from contact.
This commit removes the "IsTouch" conditions from the handling of
spurious cursor events in check_arbitrated_control. The result is that a
cursor will not be granted "active" status in preference to any other
tool (and will drop "active" status if gained after 100ms and while
no buttons are pressed).

Reviewed-by: Peter Hutterer <***@who-t.net>

Cheers,
Peter

Post by Jason Gerecke
---
src/wcmCommon.c | 27 ++++++++++++++++++---------
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/wcmCommon.c b/src/wcmCommon.c
index 4fbe775..1471fa5 100644
--- a/src/wcmCommon.c
+++ b/src/wcmCommon.c
@@ -854,29 +854,38 @@ static Bool check_arbitrated_control(InputInfoPtr pInfo, WacomDeviceStatePtr ds)
if (IsPad(priv)) {
/* Pad may never be the "active" pointer controller */
+ DBG(6, priv, "Event from pad; not yielding pointer control\n.");
return FALSE;
}
if (active == NULL || active->oldState.device_id == ds->device_id) {
- DBG(11, priv, "Same device ID as active; allowing access.\n");
+ DBG(11, priv, "Event from active device; maintaining pointer control.\n");
return TRUE;
}
- else if (IsCursor(active) && IsTouch(priv)) {
- /* Cursor devices are often left idle in range, so allow touch to
- * grab control if the tool has not been used for some time.
+ else if (IsCursor(active)) {
+ /* Cursor devices are often left idle in range, so allow other devices
+ * to grab control if the tool has not been used for some time.
*/
- return (ds->time - active->oldState.time > 100) && !active->oldState.buttons;
+ Bool yield = (ds->time - active->oldState.time > 100) && !active->oldState.buttons;
+ DBG(6, priv, "Currently-active cursor %s idle; %s pointer control.\n",
+ yield ? "is" : "is not", yield ? "yielding" : "not yielding");
+ return yield;
}
- else if (IsTouch(active) && IsCursor(priv)) {
+ else if (IsCursor(priv)) {
/* An otherwise idle cursor may still occasionally jitter and send
- * events while the user is making active touches. Do not allow
- * the cursor to grab control in this particular case.
+ * events while the user is actively using other tools or touching
+ * the device. Do not allow the cursor to grab control in this
+ * particular case.
*/
+ DBG(6, priv, "Event from non-active cursor; not yielding pointer control.\n");
return FALSE;
}
else {
/* Non-touch input has priority over touch in general */
- return !IsTouch(priv);
+ Bool yield = !IsTouch(priv);
+ DBG(6, priv, "Event from non-active %s device; %s pointer control.\n",
+ yield ? "non-touch" : "touch", yield ? "yielding" : "not yielding");
+ return yield;
}
}
--
2.10.1