On Tue, Aug 5, 2014 at 9:51 PM, Peter Hutterer <peter.hutte...@who-t.net> wrote:
> On Thu, Jul 24, 2014 at 12:04:02PM -0700, Jason Gerecke wrote:
>> Currently pointer arbitration is taken care of through the use of
>> a 'wcomPenInProx' variable in the WacomCommonRec. If two devices
>> share a 'common' variable, the commonDispatchDevice function will
>> ensure that touches do not move the pointer while the pen is in
>> proximity.
>>
>> This patch adds support for cross-device pointer arbitration to
>> prevent other touches known to the driver from moving the pointer.
>> This is achieved with the introduction of a 'WACOM_DRIVER' structure
>> that is shared by all tools. Inside this structure is an 'active'
>> variable that keeps track of the last tool which was actively
>> controlling the pointer. When an event from any tool comes in, the
>> driver will attempt to determine if it should be filtered or not.
>> In some cases, the new device will be allowed to take control of
>> the pointer, and become the new active device.
>>
>> Note that arbitration occurs *after* we store the device state
>> to pChannel->valid.state. The code in wcmTouchFilter.c assumes
>> that states are not missing, and can become desynchronized from
>> reality if (for example) a finger were to go out of prox without
>> a corresponding state update.
>>
>> Signed-off-by: Jason Gerecke <killert...@gmail.com>
>> ---
>> src/wcmCommon.c | 69
>> ++++++++++++++++++++++++++++++++++++-----------------
>> src/xf86WacomDefs.h | 8 ++++++-
>> 2 files changed, 54 insertions(+), 23 deletions(-)
>>
>> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
>> index 5bcd755..4e7098a 100644
>> --- a/src/wcmCommon.c
>> +++ b/src/wcmCommon.c
>> @@ -28,6 +28,11 @@
>> #include <xkbsrv.h>
>> #include <xf86_OSproc.h>
>>
>> +
>> +struct _WacomDriverRec WACOM_DRIVER = {
>> + .active = NULL,
>> +};
>
> just to make sure: you're intending to block events from other, unrelated
> devices if multiple tablets are connected? say, block touch from an I5 while
> a pen is active on a cintiq.
>
That's correct. At the moment the X driver can only do arbitration if
the devices share the same USB ID. This isn't always sufficient, and
we rely on the kernel to work out the trickier cases. This isn't as
clever as the kernel's arbitration logic, but its brute-force approach
can actually work out well for tablet PC users... If an xorg.conf.d
rule is added to route all touchscreens to xf86-input-wacom,
arbitration will (finally) occur between the pen and touch even if
they're from different manufacturers. I'd like to know what your
thoughts are on that...
>> +
>> /* X servers pre 1.9 didn't copy data passed into xf86Post*Event.
>> * Data passed in would be modified, requiring the driver to copy the
>> * data beforehand.
>> @@ -832,6 +837,34 @@ static WacomToolPtr findTool(const WacomCommonPtr
>> common,
>> return tool;
>> }
>>
>> +/**
>> + * Check if the given device should grab control of the pointer in
>> + * preference to whatever tool currently has access.
>> + *
>> + * @param pInfo The device to check for access
>> + * @param ds The current state of the device
>> + * @returns 'TRUE' if control of the pointer should be granted, FALSE
>> otherwise
>> + */
>> +static Bool check_arbitrated_control(InputInfoPtr pInfo,
>> WacomDeviceStatePtr ds)
>> +{
>> + WacomDevicePtr active = WACOM_DRIVER.active;
>> + WacomDevicePtr priv = pInfo->private;
>> +
>> + if (active == NULL || active->oldState.device_id == ds->device_id) {
>> + //DBG(-1, priv, "Same device ID as active; allowing
>> access.\n");
>
> /* c-style comments please */
>
That particular statement actually shouldn't be commented out at all.
I've changed it to a DBG(11) if you don't mind. The others have been
fixed.
>> + return TRUE;
>> + }
>> +
>> + if (IsPad(priv)) {
>> + // Pad may never be the "active" pointer controller
>> + return FALSE;
>> + }
>> + else {
>> + // EMR events may take control from touch at any time
>
> EMR?
Clarified: s/EMR/Pen and cursor/
>
> Cheers,
> Peter
>
>> + return !IsTouch(priv);
>> + }
>> +}
>> +
>>
>> /*****************************************************************************
>> * wcmEvent -
>> * Handles suppression, transformation, filtering, and event dispatch.
>> @@ -933,6 +966,20 @@ void wcmEvent(WacomCommonPtr common, unsigned int
>> channel,
>> pChannel->valid.state = ds; /*save last raw sample */
>> if (pChannel->nSamples < common->wcmRawSample) ++pChannel->nSamples;
>>
>> + /* arbitrate pointer control */
>> + if (check_arbitrated_control(pInfo, &ds)) {
>> + if (WACOM_DRIVER.active != NULL && priv !=
>> WACOM_DRIVER.active) {
>> + wcmSoftOutEvent(WACOM_DRIVER.active->pInfo);
>> + }
>> + if (ds.proximity)
>> + WACOM_DRIVER.active = priv;
>> + else
>> + WACOM_DRIVER.active = NULL;
>> + }
>> + else if (!IsPad(priv)) {
>> + return;
>> + }
>> +
>> if ((ds.device_type == TOUCH_ID) && common->wcmTouch)
>> {
>> wcmGestureFilter(priv, ds.serial_num - 1);
>> @@ -1125,28 +1172,6 @@ static void commonDispatchDevice(InputInfoPtr pInfo,
>> return;
>> }
>>
>> - if (TabletHasFeature(common, WCM_PENTOUCH))
>> - {
>> - if (IsTablet(priv))
>> - {
>> - common->wcmTouchDevice->common->wcmPenInProx =
>> filtered.proximity;
>> -
>> - /* send touch out when tablet tool coming in-prox for
>> - * devices that provide both pen/puck and touch events
>> - * so system cursor won't jump between tools.
>> - */
>> - if (common->wcmTouchDevice->oldState.proximity)
>> - {
>> - common->wcmGestureMode = 0;
>> - wcmSoftOutEvent(common->wcmTouchDevice->pInfo);
>> - return;
>> - }
>> - }
>> - else if (IsTouch(priv) && common->wcmPenInProx)
>> - /* Ignore touch events when tablet tool is in prox */
>> - return;
>> - }
>> -
>> if ((IsPen(priv) || IsTouch(priv)) && common->wcmMaxZ)
>> {
>> int prev_min_pressure = priv->oldState.proximity ?
>> priv->minPressure : 0;
>> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
>> index 9b81fe0..af1df4e 100644
>> --- a/src/xf86WacomDefs.h
>> +++ b/src/xf86WacomDefs.h
>> @@ -378,6 +378,12 @@ enum WacomProtocol {
>> WCM_PROTOCOL_5
>> };
>>
>> +struct _WacomDriverRec
>> +{
>> + WacomDevicePtr active; /* Arbitrate motion through this pointer */
>> +};
>> +extern struct _WacomDriverRec WACOM_DRIVER; // Defined in wcmCommon.c
>> +
>> struct _WacomCommonRec
>> {
>> /* Do not move device_path, same offset as priv->name. Used by DBG
>> macro */
>> @@ -393,7 +399,7 @@ struct _WacomCommonRec
>> unsigned long wcmKeys[NBITS(KEY_MAX)]; /* supported tool types for the
>> device */
>> WacomDevicePtr wcmTouchDevice; /* The pointer for pen to access the
>> touch tool of the same device id */
>> - Bool wcmPenInProx; /* Keep pen in-prox state for touch tool */
>> +
>> Bool wcmHasHWTouchSwitch; /* Tablet has a touch on/off switch */
>> int wcmHWTouchSwitchState; /* touch event disable/enabled by
>> hardware switch */
>>
>> --
>> 2.0.2
>>
>>
>> ------------------------------------------------------------------------------
>> Want fast and easy access to all the code in your enterprise? Index and
>> search up to 200,000 lines of code with a free copy of Black Duck
>> Code Sight - the same software that powers the world's largest code
>> search on Ohloh, the Black Duck Open Hub! Try it now.
>> http://p.sf.net/sfu/bds
>> _______________________________________________
>> Linuxwacom-devel mailing list
>> Linuxwacom-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
>>
------------------------------------------------------------------------------
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
