On Wed, Mar 30, 2011 at 04:25:12PM -0700, Ping Cheng wrote:
> With the introduction of multi-touch, the chances of getting touch
> events while pen is in prox have been increased. One obvious use
> case is that the touch events could be used for gestures while pen
> is in prox. However, we do not want two cursors compete on the screen.
>
> Link the pen and touch device once during the initialization stage
> instead of every time when we receive a pen event. Then, centralize
> pen and touch arbitration process so we can store the touch data in
> wcmUSB.c instead of discarding them. The touch events will only be
> ignored if it is a single touch event that causes a cursor movement
> while pen is in prox.
>
> Some cleanup in wcmUSB.c is needed. It will be considered when we
> make MAX_CHANNEL a dynamic value based on MAX_FINGERS. The
> MAX_FINGERS is going to be the maximum of ABS_MT_SLOT that we
> retrieve from the kernel. That brings us to the state to support
> XInput 2.1 and devcies that have dynamic number of fingers.
^ typo, 'devcies'
> Signed-off-by: Ping Cheng <[email protected]>
> ---
> src/wcmCommon.c | 31 ++++++++++++-------------------
> src/wcmConfig.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> src/xf86WacomDefs.h | 4 ++++
> 3 files changed, 61 insertions(+), 19 deletions(-)
>
> diff --git a/src/wcmCommon.c b/src/wcmCommon.c
> index a370389..615ac6a 100644
> --- a/src/wcmCommon.c
> +++ b/src/wcmCommon.c
> @@ -1138,30 +1138,23 @@ static void commonDispatchDevice(WacomCommonPtr
> common, unsigned int channel,
> return;
> }
>
> - /* send a touch out for USB Tablet PCs */
> - if (IsUSBDevice(common) && !IsTouch(priv)
> - && common->wcmTouchDefault && !priv->oldProximity)
> + /* send touch out when pen coming in-prox for devices that provide
> + * both pen and touch events so system cursor won't jump between tools
> + */
> + if (IsPen(priv) && TabletHasFeature(common, WCM_PENTOUCH))
> {
> - InputInfoPtr device = xf86FirstLocalDevice();
> - WacomCommonPtr tempcommon = NULL;
> - WacomDevicePtr temppriv = NULL;
> -
> - /* Lookup to see if associated touch was enabled */
> - for (; device != NULL; device = device->next)
> + if (IsPen(priv))
> {
> - if (strstr(device->drv->driverName, "wacom"))
> + if (common->wcmPointerToTouch->oldProximity)
temporary variable please. as a general rule, if you have to use two -> to
get to the field you want (and you're doing so twice), you should use a
temporary variable to make the code easier to read.
> {
> - temppriv = (WacomDevicePtr) device->private;
> - tempcommon = temppriv->common;
> -
> - if ((tempcommon->tablet_id ==
> common->tablet_id) &&
> - IsTouch(temppriv) &&
> temppriv->oldProximity)
> - {
> - /* Send soft prox-out for touch first */
> - wcmSoftOutEvent(device);
> - }
> + /* Send touch out so pen takes control */
> +
> wcmSoftOutEvent(common->wcmPointerToTouch->pInfo);
> + return;
> }
> }
> + else if (IsTouch(priv) && common->wcmPenInProx)
> + /* Ignore touch events when pen is in prox */
> + return;
we won't ever get to this condition, IsPen(priv) is always true in this
block
> }
>
> if (IsPen(priv))
> diff --git a/src/wcmConfig.c b/src/wcmConfig.c
> index 6235d3c..21a124e 100644
> --- a/src/wcmConfig.c
> +++ b/src/wcmConfig.c
> @@ -397,6 +397,45 @@ wcmInitModel(InputInfoPtr pInfo)
> return TRUE;
> }
>
> +/* Link the touch tool to the pen of the same device
asciidoc /** please
> + * so we can arbitrate the events.
> + */
> +static void wcmLinkTouchAndPen(InputInfoPtr pInfo)
> +{
> + WacomDevicePtr priv = pInfo->private;
> + WacomCommonPtr common = priv->common;
> + InputInfoPtr device = xf86FirstLocalDevice();
> + WacomCommonPtr tmpcommon = NULL;
> + WacomDevicePtr tmppriv = NULL;
> +
> + /* Lookup to find the associated pen and touch */
> + for (; device != NULL; device = device->next)
> + {
> + if (strstr(device->drv->driverName, "wacom"))
> + {
> + tmppriv = (WacomDevicePtr) device->private;
> + tmpcommon = tmppriv->common;
> +
I'm a big fan of using continues for simple conditions to skip. in this
case, a
if (tmppriv == priv)
continue;
rather than adding this to the next condition that deals with the actual
comparison of the tools.
> + if ((tmpcommon->tablet_id == common->tablet_id) &&
> + tmppriv != priv)
> + {
> + if (IsTouch(tmppriv) && IsPen(priv))
> + {
> + common->wcmPointerToTouch = tmppriv;
> + common->tablet_type |= WCM_PENTOUCH;
> + tmpcommon->tablet_type |= WCM_PENTOUCH;
> + }
> + else if (IsTouch(priv) && IsPen(tmppriv))
> + {
> + tmpcommon->wcmPointerToTouch = priv;
> + common->tablet_type |= WCM_PENTOUCH;
> + tmpcommon->tablet_type |= WCM_PENTOUCH;
> + }
this can be simplified with the if/else if condition setting a tmp variable
for the new priv assignment and then a shared block for the three actual
assignments.
> + }
> + }
> + }
> +}
> +
> /* wcmPreInit - called for each input devices with the driver set to
> * "wacom" */
> #if GET_ABI_MAJOR(ABI_XINPUT_VERSION) < 12
> @@ -518,6 +557,12 @@ static int wcmPreInit(InputDriverPtr drv, InputInfoPtr
> pInfo, int flags)
> pInfo->fd = -1;
> }
>
> + /* only link them once per port. We need to try for both pen and touch
> + * since we do not know which tool (touch or pen) will be added first.
> + */
> + if (IsTouch(priv) || (IsPen(priv) && !common->wcmPointerToTouch))
> + wcmLinkTouchAndPen(pInfo);
> +
> return Success;
>
> SetupProc_fail:
> diff --git a/src/xf86WacomDefs.h b/src/xf86WacomDefs.h
> index 43348fc..60d6426 100644
> --- a/src/xf86WacomDefs.h
> +++ b/src/xf86WacomDefs.h
> @@ -173,6 +173,7 @@ struct _WacomModel
> #define WCM_TPC (0x00000200 | WCM_LCD) /* TabletPC
> (special
> button handling,
> always an LCD) */
> +#define WCM_PENTOUCH 0x00000400 /* Tablet supports pen and touch */
> #define TabletHasFeature(common, feature) (((common)->tablet_type &
> (feature)) != 0)
>
> #define ABSOLUTE_FLAG 0x00000100
> @@ -421,6 +422,9 @@ struct _WacomCommonRec
> int fd; /* file descriptor to tablet */
> int fd_refs; /* number of references to fd; if =0, fd
> is invalid */
> unsigned long wcmKeys[NBITS(KEY_MAX)]; /* supported tool types for the
> device */
> + WacomDevicePtr wcmPointerToTouch; /* The pointer for pen to access the
> + touch tool of the same device id */
the term pointer in X is so badly overloaded that I can't recommend using
it. how about just "wcmTouchDevice"?
keeping this pointer gives us a race condition similar to the hotplugging
one we had a while ago. if the touch device is removed first and tablet
still sends event, the new block in commonDispatchDevice may dereference the
already deleted pointer.
You need a matching wcmUnlinkTouchAndPen() called during DEVICE_OFF.
Cheers,
Peter
> + Bool wcmPenInProx; /* Keep pen in-prox state for touch tool */
>
> /* These values are in tablet coordinates */
> int wcmMaxX; /* tablet max X value */
> --
> 1.7.4
------------------------------------------------------------------------------
Create and publish websites with WebMatrix
Use the most popular FREE web apps or write code yourself;
WebMatrix provides all the features you need to develop and
publish your website. http://p.sf.net/sfu/ms-webmatrix-sf
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
