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.

Also, AFAICT this patch changes the behaviour to reset on all four being -1,
I'll note that in the commit message.

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

Reply via email to