Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 03, 2011 at 08:38:37PM -0700, Ping Cheng wrote:
> On Sun, Apr 3, 2011 at 7:51 PM, Peter Hutterer
> wrote:
>
> > On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
> > > On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer > >wrote:
> > >
> > > > >
> > > > > What information can the user use for Uniq to associate the devices,
> > > > which
> > > > > we do not have in the driver? Both devices have the same product ID
> > and
> > > > > name.
> > > >
> > > > I only have one Bamboo, but the test code below gives me the right
> > answer.
> > > > argv[1,2] are the two device nodes (/dev/input/eventX and
> > > > /dev/input/eventY).
> > > >
> > > > static struct udev_device *udev_from_file(struct udev *udev, const char
> > > > *filename)
> > > > {
> > > >struct stat st;
> > > >stat(filename, &st);
> > > >return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> > > > }
> > > >
> > > > int main(int argc, char** argv)
> > > > {
> > > >int rc = 1;
> > > >struct udev *udev = NULL;
> > > >struct udev_device *dev1, *dev2 = NULL;
> > > >struct udev_device *parent1, *parent2;
> > > >const char *syspath1, *syspath2;
> > > >
> > > >if (argc < 3)
> > > >goto out;
> > > >
> > > >udev = udev_new();
> > > >dev1 = udev_from_file(udev, argv[1]);
> > > >dev2 = udev_from_file(udev, argv[2]);
> > > >
> > > >if (!dev1 || !dev2)
> > > >goto out;
> > > >
> > > >parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, "usb",
> > > > "usb_device");
> > > >parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, "usb",
> > > > "usb_device");
> > > >
> > > >syspath1 = udev_device_get_syspath(parent1);
> > > >syspath2 = udev_device_get_syspath(parent2);
> > > >
> > >
> > >
> > > This looks very promising. So, we do not need anything from the user.
> > > Everything could be done inside the driver. Are you going to make a
> > patch?
> >
> > can't right now, got too many other things on my slate, sorry.
> >
>
> That's fine. I may be able to get to it after next week.
>
> Can you test the code with two of your Intuos4's? If we get "Nope, different
> devices" for two identical devices, we are ready to move forward. Otherwise,
> ...
>
> I will not be in the office next whole week. So, I do not have two identical
> devices to play with.
yeah, as expected, it says that the devices are different.
Cheers,
Peter
--
Xperia(TM) PLAY
It's a major breakthrough. An authentic gaming
smartphone on the nation's most reliable network.
And it wants your games.
http://p.sf.net/sfu/verizon-sfdev
___
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 3, 2011 at 7:51 PM, Peter Hutterer wrote:
> On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
> > On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer >wrote:
> >
> > > >
> > > > What information can the user use for Uniq to associate the devices,
> > > which
> > > > we do not have in the driver? Both devices have the same product ID
> and
> > > > name.
> > >
> > > I only have one Bamboo, but the test code below gives me the right
> answer.
> > > argv[1,2] are the two device nodes (/dev/input/eventX and
> > > /dev/input/eventY).
> > >
> > > static struct udev_device *udev_from_file(struct udev *udev, const char
> > > *filename)
> > > {
> > >struct stat st;
> > >stat(filename, &st);
> > >return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> > > }
> > >
> > > int main(int argc, char** argv)
> > > {
> > >int rc = 1;
> > >struct udev *udev = NULL;
> > >struct udev_device *dev1, *dev2 = NULL;
> > >struct udev_device *parent1, *parent2;
> > >const char *syspath1, *syspath2;
> > >
> > >if (argc < 3)
> > >goto out;
> > >
> > >udev = udev_new();
> > >dev1 = udev_from_file(udev, argv[1]);
> > >dev2 = udev_from_file(udev, argv[2]);
> > >
> > >if (!dev1 || !dev2)
> > >goto out;
> > >
> > >parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, "usb",
> > > "usb_device");
> > >parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, "usb",
> > > "usb_device");
> > >
> > >syspath1 = udev_device_get_syspath(parent1);
> > >syspath2 = udev_device_get_syspath(parent2);
> > >
> >
> >
> > This looks very promising. So, we do not need anything from the user.
> > Everything could be done inside the driver. Are you going to make a
> patch?
>
> can't right now, got too many other things on my slate, sorry.
>
That's fine. I may be able to get to it after next week.
Can you test the code with two of your Intuos4's? If we get "Nope, different
devices" for two identical devices, we are ready to move forward. Otherwise,
...
I will not be in the office next whole week. So, I do not have two identical
devices to play with.
Ping
--
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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 03, 2011 at 07:23:34PM -0700, Ping Cheng wrote:
> On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer
> wrote:
>
> > >
> > > What information can the user use for Uniq to associate the devices,
> > which
> > > we do not have in the driver? Both devices have the same product ID and
> > > name.
> >
> > I only have one Bamboo, but the test code below gives me the right answer.
> > argv[1,2] are the two device nodes (/dev/input/eventX and
> > /dev/input/eventY).
> >
> > static struct udev_device *udev_from_file(struct udev *udev, const char
> > *filename)
> > {
> >struct stat st;
> >stat(filename, &st);
> >return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> > }
> >
> > int main(int argc, char** argv)
> > {
> >int rc = 1;
> >struct udev *udev = NULL;
> >struct udev_device *dev1, *dev2 = NULL;
> >struct udev_device *parent1, *parent2;
> >const char *syspath1, *syspath2;
> >
> >if (argc < 3)
> >goto out;
> >
> >udev = udev_new();
> >dev1 = udev_from_file(udev, argv[1]);
> >dev2 = udev_from_file(udev, argv[2]);
> >
> >if (!dev1 || !dev2)
> >goto out;
> >
> >parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, "usb",
> > "usb_device");
> >parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, "usb",
> > "usb_device");
> >
> >syspath1 = udev_device_get_syspath(parent1);
> >syspath2 = udev_device_get_syspath(parent2);
> >
>
>
> This looks very promising. So, we do not need anything from the user.
> Everything could be done inside the driver. Are you going to make a patch?
can't right now, got too many other things on my slate, sorry.
Cheers,
Peter
--
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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Sun, Apr 3, 2011 at 6:35 PM, Peter Hutterer wrote:
> >
> > What information can the user use for Uniq to associate the devices,
> which
> > we do not have in the driver? Both devices have the same product ID and
> > name.
>
> I only have one Bamboo, but the test code below gives me the right answer.
> argv[1,2] are the two device nodes (/dev/input/eventX and
> /dev/input/eventY).
>
> static struct udev_device *udev_from_file(struct udev *udev, const char
> *filename)
> {
>struct stat st;
>stat(filename, &st);
>return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
> }
>
> int main(int argc, char** argv)
> {
>int rc = 1;
>struct udev *udev = NULL;
>struct udev_device *dev1, *dev2 = NULL;
>struct udev_device *parent1, *parent2;
>const char *syspath1, *syspath2;
>
>if (argc < 3)
>goto out;
>
>udev = udev_new();
>dev1 = udev_from_file(udev, argv[1]);
>dev2 = udev_from_file(udev, argv[2]);
>
>if (!dev1 || !dev2)
>goto out;
>
>parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, "usb",
> "usb_device");
>parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, "usb",
> "usb_device");
>
>syspath1 = udev_device_get_syspath(parent1);
>syspath2 = udev_device_get_syspath(parent2);
>
This looks very promising. So, we do not need anything from the user.
Everything could be done inside the driver. Are you going to make a patch?
Thank you for the info.
Ping
> if (strcmp(syspath1, syspath2) == 0)
>printf("Yep, they're the same physical device\n");
>else
>printf("Nope, different devices\n");
>
> out:
>udev_device_unref(dev1);
>udev_device_unref(dev2);
>udev_unref(udev);
>return rc;
> }
>
> Cheers,
> Peter
>
--
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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Thu, Mar 31, 2011 at 11:13:36PM -0700, Ping Cheng wrote:
> On Thu, Mar 31, 2011 at 10:57 PM, Peter Hutterer
> wrote:
>
> > On Thu, Mar 31, 2011 at 10:37:54PM -0700, Ping Cheng wrote:
> > > On Thu, Mar 31, 2011 at 4:43 PM, Peter Hutterer <
> > peter.hutte...@who-t.net>wrote:
> > >
> > > >/* FIXME: why strstr and not strcmp? */
> > > > if (!strstr(device->drv->driverName, "wacom"))
> > > > continue;
> > > >
> > >
> > > I guess, the intention was to allow driverName has extra characters
> > instead
> > > of exact "wacom". As long as it has "wacom" in it, we take care of it.
> > Not
> > > sure if it is necessary though.
> >
> > I'd rather be precise on the match. I don't think anyone has a driver that
> > could possibly conflict but substring matches have a nasty habit of finding
> > things humans don't necessarily see.
> >
> > > > would it make sense to have them share the common struct once we've
> > linked
> > > > them?
> > > >
> > >
> > > It makes sense. One issue, even with the current code, is how to tell the
> > > difference between two identical tablets in the driver. How do we know if
> > we
> > > are linking the proper devices together?
> > >
> > > So far, I assume all devices attached to the same system have different
> > > product IDs.
> >
> > maybe we need an Option Uniq then to let the user associate the devices?
> >
>
> What information can the user use for Uniq to associate the devices, which
> we do not have in the driver? Both devices have the same product ID and
> name.
I only have one Bamboo, but the test code below gives me the right answer.
argv[1,2] are the two device nodes (/dev/input/eventX and /dev/input/eventY).
static struct udev_device *udev_from_file(struct udev *udev, const char
*filename)
{
struct stat st;
stat(filename, &st);
return udev_device_new_from_devnum(udev, 'c', st.st_rdev);
}
int main(int argc, char** argv)
{
int rc = 1;
struct udev *udev = NULL;
struct udev_device *dev1, *dev2 = NULL;
struct udev_device *parent1, *parent2;
const char *syspath1, *syspath2;
if (argc < 3)
goto out;
udev = udev_new();
dev1 = udev_from_file(udev, argv[1]);
dev2 = udev_from_file(udev, argv[2]);
if (!dev1 || !dev2)
goto out;
parent1 = udev_device_get_parent_with_subsystem_devtype(dev1, "usb",
"usb_device");
parent2 = udev_device_get_parent_with_subsystem_devtype(dev2, "usb",
"usb_device");
syspath1 = udev_device_get_syspath(parent1);
syspath2 = udev_device_get_syspath(parent2);
if (strcmp(syspath1, syspath2) == 0)
printf("Yep, they're the same physical device\n");
else
printf("Nope, different devices\n");
out:
udev_device_unref(dev1);
udev_device_unref(dev2);
udev_unref(udev);
return rc;
}
Cheers,
Peter
--
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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Thu, Mar 31, 2011 at 10:57 PM, Peter Hutterer wrote: > On Thu, Mar 31, 2011 at 10:37:54PM -0700, Ping Cheng wrote: > > On Thu, Mar 31, 2011 at 4:43 PM, Peter Hutterer < > peter.hutte...@who-t.net>wrote: > > > > >/* FIXME: why strstr and not strcmp? */ > > > if (!strstr(device->drv->driverName, "wacom")) > > > continue; > > > > > > > I guess, the intention was to allow driverName has extra characters > instead > > of exact "wacom". As long as it has "wacom" in it, we take care of it. > Not > > sure if it is necessary though. > > I'd rather be precise on the match. I don't think anyone has a driver that > could possibly conflict but substring matches have a nasty habit of finding > things humans don't necessarily see. > > > > would it make sense to have them share the common struct once we've > linked > > > them? > > > > > > > It makes sense. One issue, even with the current code, is how to tell the > > difference between two identical tablets in the driver. How do we know if > we > > are linking the proper devices together? > > > > So far, I assume all devices attached to the same system have different > > product IDs. > > maybe we need an Option Uniq then to let the user associate the devices? > What information can the user use for Uniq to associate the devices, which we do not have in the driver? Both devices have the same product ID and name. Ping -- 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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Thu, Mar 31, 2011 at 04:25:23PM -0700, Ping Cheng wrote:
> On Wed, Mar 30, 2011 at 11:17 PM, Peter Hutterer
> > > + 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.
> >
>
> Not really. Although priv can be set to tmppriv, both common and tmpcommon
> are needed which can not be used with one name. Plus, since it is a if/else
> if, there would be cases outside of those statements that we should not
> assign any values except continue.
for (; device != NULL; device = device->next)
{
WacomCommonPtr tmpcommon = NULL;
WacomDevicePtr tmppriv = NULL;
/* FIXME: why strstr and not strcmp? */
if (!strstr(device->drv->driverName, "wacom"))
continue;
tmppriv = (WacomDevicePtr) device->private;
tmpcommon = tmppriv->common;
if (tmppriv == priv)
continue;
if ((tmpcommon->tablet_id == common->tablet_id))
{
WacomCommonPtr ptr_dev = NULL;
WacomDevicePtr touch_dev = NULL;
if (IsTouch(tmppriv) && IsPen(priv))
{
ptr_dev = common;
touch_dev = tmppriv;
}
else if (IsTouch(priv) && IsPen(tmppriv))
{
ptr_dev = tmpcommon;
touch_dev = priv;
}
if (ptr_dev && touch_dev)
{
ptr_dev->wcmPointerToTouch = touch_dev;
common->tablet_type |= WCM_PENTOUCH;
tmpcommon->tablet_type |= WCM_PENTOUCH;
}
}
}
and while writing this I noticed another thing. IIRC touch and pen are two
kernel devices (this is what you refer to as port, right?). would it make
sense to have them share the common struct once we've linked them?
> > + }
> > > + }
> > > + }
> > > +}
> > > +
> > > /* 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 (0x0200 | WCM_LCD) /* TabletPC
> > (special
> > > button handling,
> > > always an LCD) */
> > > +#define WCM_PENTOUCH 0x0400 /* Tablet supports pen and touch
> > */
> > > #define TabletHasFeature(common, feature) (((common)->tablet_type &
> > (feature)) != 0)
> > >
> > > #define ABSOLUTE_FLAG0x0100
> > > @@ -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
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
On Wed, Mar 30, 2011 at 11:17 PM, Peter Hutterer
wrote:
> > {
> > - 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
>
Right, it should be outside of the if-statement.
> }
> >
> > 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
>
Oh, I only remembered there is no need to specify pInfo and common. I forgot
this detail.
> + * 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.
>
No problem. Will do.
> > + 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.
>
Not really. Although priv can be set to tmppriv, both common and tmpcommon
are needed which can not be used with one name. Plus, since it is a if/else
if, there would be cases outside of those statements that we should not
assign any values except continue.
> + }
> > + }
> > + }
> > +}
> > +
> > /* 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 (0x0200 | WCM_LCD) /* TabletPC
> (
Re: [Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
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
> ---
> 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
[Linuxwacom-devel] [PATCH 3/3] Centralize pen and touch arbitration
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.
Signed-off-by: Ping Cheng
---
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)
{
- 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;
}
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
+ * 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;
+
+ 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;
+
