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 <pingli...@gmail.com> > --- > 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 Linuxwacom-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel