[PATCH evdev] evdev: added property Evdev Axes Rotation. #27688

[Peter Hutterer]

On Sun, Oct 24, 2010 at 01:45:03PM +0200, Paolo D'Apice wrote:
> The evdev driver does not allow to set a custom axes rotation
> as the mousedrv driver does with the option "AngleOffset".
> This option is necessary for some trackballs, for example the
> Logitech Cordless Optical TrackMan which has the optical
> sensor off-axes (in MS Windows, the Logitech proprietary driver
> adjusts the offset).
> 
> X.Org Bug 27688 <http://bugs.freedesktop.org/show_bug.cgi?id=27688>
> 
> Signed-off-by: Paolo D'Apice <dapicester at gmail.com>

this patch seems simple enough that we could add it to the server as a
standard property for pointer devices, isn't it?  I'd much prefer that so we
don't have to re-implement it for the various drivers.

> ---
>  include/evdev-properties.h |    4 ++++
>  man/evdev.man              |   12 ++++++++++++
>  src/evdev.c                |   25 +++++++++++++++++++++++++
>  src/evdev.h                |    1 +
>  4 files changed, 42 insertions(+), 0 deletions(-)
> 
> diff --git a/include/evdev-properties.h b/include/evdev-properties.h
> index 7df2876..a658e2b 100644
> --- a/include/evdev-properties.h
> +++ b/include/evdev-properties.h
> @@ -66,4 +66,8 @@
>  /* BOOL */
>  #define EVDEV_PROP_SWAP_AXES "Evdev Axes Swap"
>  
> +/* Axes Rotation */
> +/* CARD16 */
> +#define EVDEV_PROP_AXES_ROTATION "Evdev Axes Rotation"
> +

note that CARD16 is unsigned, this should probably read INT16?

>  #endif
> diff --git a/man/evdev.man b/man/evdev.man
> index adb3f8d..f7c5e9f 100644
> --- a/man/evdev.man
> +++ b/man/evdev.man
> @@ -177,6 +177,15 @@ This option has no effect on devices without absolute axes.
>  .BI "Option \*qSwapAxes\*q \*q" Bool \*q
>  Swap x/y axes. Default: off. Property: "Evdev Axes Swap".
>  .TP 7
> +.BI "Option \*qAxesRotation\*q \*q" integer \*q
> +Clockwise axes rotation (in degrees) to apply to the pointer motion. 
> +This transformation is applied before the 
> +.BR SwapAxes , 
> +.BR InvertX 
> +and
> +.B InvertY 
> +transformations. Default: 0. Property: "Evdev Axes Rotation".
> +.TP 7
>  .BI "Option \*qXAxisMapping\*q \*q" "N1 N2" \*q
>  Specifies which buttons are mapped to motion in the X direction in wheel
>  emulation mode.  Button number
> @@ -210,6 +219,9 @@ in-driver axis calibration.
>  .BI "Evdev Axes Swap"
>  1 boolean value (8 bit, 0 or 1). 1 swaps x/y axes.
>  .TP 7
> +.BI "Evdev Axes Rotation"
> +1 16-bit positive and negative value. 0 disable rotation.

How about "1 16-bit signed value". hard to have a non-zero positive _and_
negative value anyway :)

> +.TP 7
>  .BI "Evdev Drag Lock Buttons"
>  8-bit. Either 1 value or pairs of values. Value range 0-32, 0 disables a
>  value.
> diff --git a/src/evdev.c b/src/evdev.c
> index 9e1fb10..fc8918e 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -47,6 +47,7 @@
>  #include <exevents.h>
>  #include <xorgVersion.h>
>  #include <xkbsrv.h>
> +#include <math.h>
>  
>  #ifdef HAVE_PROPERTIES
>  #include <X11/Xatom.h>
> @@ -114,6 +115,7 @@ static Atom prop_calibration = 0;
>  static Atom prop_swap = 0;
>  static Atom prop_axis_label = 0;
>  static Atom prop_btn_label = 0;
> +static Atom prop_axes_rotation = 0;
>  #endif
>  
>  /* All devices the evdev driver has allocated and knows about.
> @@ -383,6 +385,15 @@ EvdevProcessValuators(InputInfoPtr pInfo, int v[MAX_VALUATORS], int *num_v,
>          int first = REL_CNT, last = 0;
>          int i;
>  
> +        if (pEvdev->axes_rotation) {
> +            float rotation = (pEvdev->axes_rotation % 360) * M_PI / 180.0; // degrees to radians 

see comment below about storing the radians.

> +            float rot_cos = cos(rotation), rot_sin = sin(rotation);

please split this line up, I read over it the first time and then got
confused.

> +   
> +            tmp = pEvdev->delta[REL_X];
> +            pEvdev->delta[REL_X] = (int)(pEvdev->delta[REL_X] * rot_cos - pEvdev->delta[REL_Y] * rot_sin);
> +            pEvdev->delta[REL_Y] = (int)(pEvdev->delta[REL_Y] * rot_cos + tmp * rot_sin);
> +        }
> +    
>          if (pEvdev->swap_axes) {
>              tmp = pEvdev->delta[REL_X];
>              pEvdev->delta[REL_X] = pEvdev->delta[REL_Y];
> @@ -2516,6 +2527,13 @@ EvdevInitProperty(DeviceIntPtr dev)
>  
>          XISetDevicePropertyDeletable(dev, prop_swap, FALSE);
>  
> +        prop_axes_rotation = MakeAtom(EVDEV_PROP_AXES_ROTATION,
> +                strlen(EVDEV_PROP_AXES_ROTATION), TRUE);
> +        rc = XIChangeDeviceProperty(dev, prop_axes_rotation, XA_INTEGER, 16,
> +                PropModeReplace, 1, &pEvdev->axes_rotation, FALSE);
> +        if (rc != Success) 
> +            return;
> +
>  #ifdef HAVE_LABELS
>          /* Axis labelling */
>          if ((pEvdev->num_vals > 0) && (prop_axis_label = XIGetKnownProperty(AXIS_LABEL_PROP)))
> @@ -2575,6 +2593,13 @@ EvdevSetProperty(DeviceIntPtr dev, Atom atom, XIPropertyValuePtr val,
>  
>          if (!checkonly)
>              pEvdev->swap_axes = *((BOOL*)val->data);
> +    } else if (atom == prop_axes_rotation)
> +    {
> +        if (val->format != 16 || val->type != XA_INTEGER || val->size != 1)
> +            return BadMatch;
> +
> +       if (!checkonly)
> +	   pEvdev->axes_rotation = *((CARD16*)val->data);

The X server keeps track of the property value for us and thus the internal
storage format can be arbitrary. I'd say it's better to calculate the
radians here and store that so we only do it once per configuration, not
once per event.

Cheers,
  Peter

>      } else if (atom == prop_axis_label || atom == prop_btn_label)
>          return BadAccess; /* Axis/Button labels can't be changed */
>  
> diff --git a/src/evdev.h b/src/evdev.h
> index b382670..17d607a 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -129,6 +129,7 @@ typedef struct {
>      BOOL swap_axes;
>      BOOL invert_x;
>      BOOL invert_y;
> +    int axes_rotation;
>  
>      int delta[REL_CNT];
>      unsigned int abs, rel;
> -- 
> 1.7.1
>