On Mon, Dec 21, 2009 at 05:53:26PM -0800, Ping wrote:
> On Mon, Dec 21, 2009 at 5:25 PM, Peter Hutterer
> <[email protected]> wrote:
> > On Mon, Dec 21, 2009 at 05:00:07PM -0800, Ping wrote:
> >> On Sun, Dec 20, 2009 at 10:31 PM, Peter Hutterer
> >> <[email protected]> wrote:
> >> > On Fri, Dec 18, 2009 at 09:26:11PM -0800, Ping wrote:
> >> >> Ok, another try by following the property principle. Comments are added
> >> >> into the patch without repeating them in the commit message. Feel free
> >> >> to
> >> >> change them the way that works for you.
> >>
> >> Hope this patch puts the issue to an end. I tested it and it works.
> >> However, I do have one question (I think I asked this it before): how
> >> are we going to let the driver "remember" the changes an user made?
> >> The old way that we used to store xsetwacom in ~/.xinitrc or something
> >> else?
> >
> > The driver doesn't remember anything past its lifetime, i.e. if you unplug
> > the device and re-plug it, it starts with the defaults/xorg.conf options.
> >
> > any run-time options deviating from the defaults must be handled by a
> > session manager or by the xsetwacom in the .xinitrc (note that the latter
> > isn't really working in light of hotplugging).
> > we really need some desktop integration for that.
> >
> >> From b2ad0f639ee720a00ba8cb249cce042d0fbe17f1 Mon Sep 17 00:00:00 2001
> >> From: Ping Cheng <[email protected]>
> >> Date: Mon, 21 Dec 2009 16:52:28 -0800
> >> Subject: [PATCH] Remove area overlap check for area property
> >>
> >> Signed-off-by: Ping Cheng <[email protected]>
> >> ---
> >> src/wcmXCommand.c | 51
> >> +++++++++++++++++++++++++++------------------------
> >> 1 files changed, 27 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> >> index 31b9945..d0120ff 100644
> >> --- a/src/wcmXCommand.c
> >> +++ b/src/wcmXCommand.c
> >> @@ -304,45 +304,48 @@ int xf86WcmSetProperty(DeviceIntPtr dev, Atom
> >> property, XIPropertyValuePtr prop,
> >> if (property == prop_tablet_area)
> >> {
> >> INT32 *values = (INT32*)prop->data;
> >> - WacomToolArea area;
> >> + WacomToolAreaPtr area = priv->toolarea;
> >>
> >> if (prop->size != 4 || prop->format != 32)
> >> return BadValue;
> >>
> >> - area.topX = values[0];
> >> - area.topY = values[1];
> >> - area.bottomX = values[2];
> >> - area.bottomY = values[3];
> >> + /* value validation is unnecessary since we let utility
> >> programs, such as
> >> + * xsetwacom and userland control panel take care of the
> >> validation role.
> >> + * when all four values are set to -1, it is an area reset
> >> (xydefault) */
> >> + if ((values[0] != -1) || (values[1] != -1) ||
> >> + (values[2] != -1) || (values[3] != -1))
> >> + {
> >> + area->topX = values[0];
> >> + area->topY = values[1];
> >> + area->bottomX = values[2];
> >> + area->bottomY = values[3];
> >>
> >> - /* FIXME: this will always be the case? */
> >> - if (WcmAreaListOverlap(&area, priv->tool->arealist))
> >> - return BadValue;
> >> + /* validate the area */
> >> + if (WcmAreaListOverlap(area, priv->tool->arealist))
> >> + return BadValue;
> >> + }
> >>
> >> if (!checkonly)
> >> {
> >> - /* Invalid range resets axis to defaults */
> >> - if (values[0] >= values[2])
> >> + if ((values[0] == -1) && (values[1] == -1) &&
> >> + (values[2] == -1) && (values[3] == -1))
> >> {
> >> values[0] = 0;
> >> - if (!IsTouch(priv))
> >> + values[1] = 0;
> >> + if (!IsTouch(priv))
> >> values[2] = common->wcmMaxX;
> >> - else
> >> + else
> >> values[2] = common->wcmMaxTouchX;
> >> - }
> >> -
> >> - if (values[1] >= values[3])
> >> - {
> >> - values[1] = 0;
> >> - if (!IsTouch(priv))
> >> + if (!IsTouch(priv))
> >> values[3] = common->wcmMaxY;
> >> - else
> >> + else
> >> values[3] = common->wcmMaxTouchY;
> >> }
> >>
> >> - priv->topX = values[0];
> >> - priv->topY = values[1];
> >> - priv->bottomX = values[2];
> >> - priv->bottomY = values[3];
> >> + priv->topX = area->topX = values[0];
> >> + priv->topY = area->topY = values[1];
> >> + priv->bottomX = area->bottomX = values[2];
> >> + priv->bottomY = area->bottomY = values[3];
> >> xf86WcmInitialCoordinates(local, 0);
> >> xf86WcmInitialCoordinates(local, 1);
> >> }
> >> --
> >> 1.6.5.2
> >
> > thanks, that works now. the diff below is needed on top to handle errors
> > correctly though.
> >
> > diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
> > index 919e946..802c2f5 100644
> > --- a/src/wcmXCommand.c
> > +++ b/src/wcmXCommand.c
> > @@ -315,6 +315,8 @@ int xf86WcmSetProperty(DeviceIntPtr dev, Atom property,
> > XIPropertyValuePtr prop,
> > if ((values[0] != -1) || (values[1] != -1) ||
> > (values[2] != -1) || (values[3] != -1))
> > {
> > + WacomToolArea tmp_area = *area;
> > +
> > area->topX = values[0];
> > area->topY = values[1];
> > area->bottomX = values[2];
> > @@ -322,7 +324,10 @@ int xf86WcmSetProperty(DeviceIntPtr dev, Atom
> > property, XIPropertyValuePtr prop,
> >
> > /* validate the area */
> > if (WcmAreaListOverlap(area, priv->tool->arealist))
> > + {
> > + *area = tmp_area;
> > return BadValue;
> > + }
> > }
> >
> > if (!checkonly)
> >
> >
> > otherwise, if a BadValue is returned the client thinks the operation has
> > failed but the driver will have updated the area regardless.
> > I'll squash that into your patch before pushing.
>
> I was wondering what I was doing in introuding the tmp_area in my
> previous patch when I worked on this patch. I thought I must have
> lost my mind. Looks like I missed something in both cases. Thank you
> for catching the ball on the other end.
This extra line is needed as well. The previous one didn't reset on success but
in theory another property handler may fail on that property, leaving us with
inconsistent settings. We need to reset in all cases.
diff --git a/src/wcmXCommand.c b/src/wcmXCommand.c
index 802c2f5..7b40685 100644
--- a/src/wcmXCommand.c
+++ b/src/wcmXCommand.c
@@ -328,6 +328,7 @@ int xf86WcmSetProperty(DeviceIntPtr dev, Atom property,
XIPropertyValuePtr prop,
*area = tmp_area;
return BadValue;
}
+ *area = tmp_area;
}
if (!checkonly)
>
> >
> > Also, AFAICT this patch changes the behaviour to reset on all four being -1,
> > I'll note that in the commit message.
>
> Sure, you can do that. The comments have been added in the code though:
right, commit logs are IMO equally important though, I look at them all day,
more often than at the actual code.
Cheers,
Peter
------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
