[PATCH] dix: Save touchpoint last coordinates before transform. #49347

[Peter Hutterer]

sorry about the delay, I wanted to get a test-case for this first to
reliably reproduce it and that took me a while.

On Thu, Nov 08, 2012 at 05:13:39PM -0500, Yuly Novikov wrote:
> A touchpoint used to store the entire valuators structure,
> and coordinates there were stored post-transformation,
> which resulted in Coordinate Transformation Matrix
> being applied multiple times to the last coordinates,
> in the case when only pressure changes in the last touch event.
> 
> This commit gets rid of the valuators, as only X and Y coordinates
> are actually used, and stores only the relevant values.
> The variable name was changed to indicate
> that the values are stored before transformation,
> to avoid confusion with last.valuators, which are after transformation.

NACK to that part, the reason we use the ValuatorMask here is because this
way we always deal with valuator masks when dealing with input coordinates.
last.valuator not being a VM yet is a leftover from the time before the VMs
were introduced.

please leave that as-is, even if it feels clunky.

Cheers,
   Peter

> 
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=49347
> 
> Signed-off-by: Yuly Novikov <ynovikov at chromium.org>

> ---
>  dix/devices.c      |    2 --
>  dix/getevents.c    |   19 ++++++-------------
>  dix/touch.c        |    1 -
>  include/inputstr.h |    3 +--
>  4 files changed, 7 insertions(+), 18 deletions(-)
> 
> diff --git a/dix/devices.c b/dix/devices.c
> index 613323f..d23b1b4 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -967,8 +967,6 @@ CloseDevice(DeviceIntPtr dev)
>      free(dev->deviceGrab.sync.event);
>      free(dev->config_info);     /* Allocated in xf86ActivateDevice. */
>      free(dev->last.scroll);
> -    for (j = 0; j < dev->last.num_touches; j++)
> -        free(dev->last.touches[j].valuators);
>      free(dev->last.touches);
>      dev->config_info = NULL;
>      dixFreePrivates(dev->devPrivates, PRIVATE_DEVICE);
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 8b4379d..fe60ac3 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -1951,32 +1951,25 @@ GetTouchEvents(InternalEvent *events, DeviceIntPtr dev, uint32_t ddx_touchid,
>      default:
>          return 0;
>      }
> -    if (t->mode == XIDirectTouch && !(flags & TOUCH_CLIENT_ID)) {
> -        if (!valuator_mask_isset(&mask, 0))
> -            valuator_mask_set_double(&mask, 0,
> -                                     valuator_mask_get_double(touchpoint.ti->
> -                                                              valuators, 0));
> -        if (!valuator_mask_isset(&mask, 1))
> -            valuator_mask_set_double(&mask, 1,
> -                                     valuator_mask_get_double(touchpoint.ti->
> -                                                              valuators, 1));
> -    }
>  
>      /* Get our screen event co-ordinates (root_x/root_y/event_x/event_y):
>       * these come from the touchpoint in Absolute mode, or the sprite in
>       * Relative. */
>      if (t->mode == XIDirectTouch) {
> -        transformAbsolute(dev, &mask);
>  
>          if (!(flags & TOUCH_CLIENT_ID)) {
> -            for (i = 0; i < valuator_mask_size(&mask); i++) {
> +            for (i = 0; i < 2; i++) {
>                  double val;
>  
>                  if (valuator_mask_fetch_double(&mask, i, &val))
> -                    valuator_mask_set_double(touchpoint.ti->valuators, i, val);
> +                    touchpoint.ti->last_raw_axes[i] = val;
> +                else
> +                    valuator_mask_set_double(&mask, i,
> +                                             touchpoint.ti->last_raw_axes[i]);
>              }
>          }
>  
> +        transformAbsolute(dev, &mask);
>          clipAbsolute(dev, &mask);
>      }
>      else {
> diff --git a/dix/touch.c b/dix/touch.c
> index 5f77be5..5074ec7 100644
> --- a/dix/touch.c
> +++ b/dix/touch.c
> @@ -226,7 +226,6 @@ void
>  TouchInitDDXTouchPoint(DeviceIntPtr dev, DDXTouchPointInfoPtr ddxtouch)
>  {
>      memset(ddxtouch, 0, sizeof(*ddxtouch));
> -    ddxtouch->valuators = valuator_mask_new(dev->valuator->numAxes);
>  }
>  
>  Bool
> diff --git a/include/inputstr.h b/include/inputstr.h
> index 5a38924..c06f9fe 100644
> --- a/include/inputstr.h
> +++ b/include/inputstr.h
> @@ -330,8 +330,7 @@ typedef struct _DDXTouchPointInfo {
>      Bool active;                /* whether or not the touch is active */
>      uint32_t ddx_id;            /* touch ID given by the DDX */
>      Bool emulate_pointer;
> -
> -    ValuatorMask *valuators;    /* last recorded axis values */
> +    double last_raw_axes[2];    /* last X/Y axis valuator data as posted */
>  } DDXTouchPointInfoRec;
>  
>  typedef struct _TouchClassRec {
> -- 
> 1.7.7.3
>