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.

>
> 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:

>> +         * when all four values are set to -1, it is an area reset
>> (xydefault) */

Ping

------------------------------------------------------------------------------
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