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

[Bryce Harrington]

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

Verified with this patch applied on a Nexus 7 tablet running Ubuntu
Xserver 1.13 and rotated into portrait mode using xrandr + xinput, that
it makes touch-based input function smoothly.

Specifically, the following reported issues no longer appear for me with
this patch applied:

  https://bugs.launchpad.net/ubuntu-nexus7/+bug/1075415
  https://bugs.launchpad.net/ubuntu-nexus7/+bug/1076567
  https://bugs.launchpad.net/ubuntu-nexus7/+bug/1076627

Tested-by: Bryce Harrington <bryce at canonical.com>

> ---
>  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
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel