[RFC] dix: Re-introduce rescaling on motion events for extended event reporting

[Peter Hutterer]

Magnus Vigerlöf wrote:
> Since the work to correct this is postponed for some time I propose
> an interim solution where we get back the scaling so devices can
> register any values on X&Y and not be tied to the changing values of
> the screen size.
> 
> The patch below is tested with a Wacom tablet and works well with my
> Volito 2 both as a pointer on the desktop and as a pressure-sensitive,
> high-resolution pen in Gimp. At the same time my synaptic touchpad and
> USB mouse worked as before.
> 
> This time I haven't introduced any interface changes. The values that
> should be reported for the device are the same as in xserver 1.3,
> including the max-values on X&Y. The drivers that has been adapted to
> cope with the current situation will continue to work as the max-value
> registered by the driver is used in the scaling.


one thing: in the proposed patch, pDev->valuator->lastx/lasty are set
before the call to miPointerSetPosition. lastx/lasty of the VCP however
afterwards.
miPSP may change x/y if there was a screen switch. so you may end up
with different coordinates here.

could this be a problem? I've looked at the code several times but I'm
still unsure.

aside from that, I'd say its fine.

> The only worry I have with this patch is the check for if we should
> scale or not. The devices I have in my system that shouldn't scale all
> have '-1' as max and this is what's used to detect this situation. Is
> that safe?

this should be fine. I vaguely remember the server relying on -1
somewhere as well. although I don't know where that was anymore or
whether it was just a feverish nightmare.

> [1] https://bugs.freedesktop.org/show_bug.cgi?id=10324
> 
> ---
>  dix/getevents.c |   54 +++++++++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 41 insertions(+), 13 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 6e840d4..d562e18 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -528,6 +528,7 @@ GetPointerEvents(xEvent *events, DeviceIntPtr pDev, int type, int buttons,
>      DeviceIntPtr cp = inputInfo.pointer;
>      int x = 0, y = 0;
>      Bool coreOnly = (pDev == inputInfo.pointer);
> +    ScreenPtr scr = miPointerGetScreen(pDev);
>  
>      /* Sanity checks. */
>      if (type != MotionNotify && type != ButtonPress && type != ButtonRelease)
> @@ -593,15 +594,33 @@ GetPointerEvents(xEvent *events, DeviceIntPtr pDev, int type, int buttons,
>                                valuators);
>  
>          if (pDev->coreEvents) {
> -            if (first_valuator == 0 && num_valuators >= 1)
> -                x = cp->valuator->lastx + valuators[0];
> +            /* Get and convert the core pointer coordinate space into
> +             * device coordinates. Use the device coords if it translates
> +             * into the same position as the core to preserve relative sub-
> +             * pixel movements from the device. */
> +            int max = pDev->valuator->axes[0].max_value;
> +            if(max > 0) {
> +                x = pDev->valuator->lastx;
> +                if((int)((float)(x)*scr->width/(max+1)) != cp->valuator->lastx)
> +                    x = (int)((float)(cp->valuator->lastx)*(max+1)/scr->width);
> +            }
>              else
>                  x = cp->valuator->lastx;
>  
> -            if (first_valuator <= 1 && num_valuators >= (2 - first_valuator))
> -                y = cp->valuator->lasty + valuators[1 - first_valuator];
> +            max = pDev->valuator->axes[1].max_value;
> +            if(max > 0) {
> +                y = pDev->valuator->lasty;
> +                if((int)((float)(y)*scr->height/(max+1)) != cp->valuator->lasty)
> +                    y = (int)((float)(cp->valuator->lasty)*(max+1)/scr->height);
> +            }
>              else
>                  y = cp->valuator->lasty;
> +
> +            /* Add relative movement */
> +            if (first_valuator == 0 && num_valuators >= 1)
> +                x += valuators[0];
> +            if (first_valuator <= 1 && num_valuators >= (2 - first_valuator))
> +                y += valuators[1 - first_valuator];
>          }
>          else {
>              if (first_valuator == 0 && num_valuators >= 1)
> @@ -620,11 +639,6 @@ GetPointerEvents(xEvent *events, DeviceIntPtr pDev, int type, int buttons,
>      clipAxis(pDev, 0, &x);
>      clipAxis(pDev, 1, &y);
>  
> -    /* This takes care of crossing screens for us, as well as clipping
> -     * to the current screen.  Right now, we only have one history buffer,
> -     * so we don't set this for both the device and core.*/
> -    miPointerSetPosition(pDev, &x, &y, ms);
> -
>      /* Drop x and y back into the valuators list, if they were originally
>       * present. */
>      if (first_valuator == 0 && num_valuators >= 1)
> @@ -634,12 +648,26 @@ GetPointerEvents(xEvent *events, DeviceIntPtr pDev, int type, int buttons,
>  
>      updateMotionHistory(pDev, ms, first_valuator, num_valuators, valuators);
>  
> +    pDev->valuator->lastx = x;
> +    pDev->valuator->lasty = y;
> +    /* Convert the dev coord back to screen coord if we're
> +     * sending core events */
> +    if (pDev->coreEvents) {
> +        if(pDev->valuator->axes[0].max_value > 0)
> +            x = (int)((float)(x)*scr->width/(pDev->valuator->axes[0].max_value+1));
> +        if(pDev->valuator->axes[1].max_value > 0)
> +            y = (int)((float)(y)*scr->height/(pDev->valuator->axes[1].max_value+1));
> +    }
> +
> +    /* This takes care of crossing screens for us, as well as clipping
> +     * to the current screen.  Right now, we only have one history buffer,
> +     * so we don't set this for both the device and core.*/
> +    miPointerSetPosition(pDev, &x, &y, ms);
> +
>      if (pDev->coreEvents) {
>          cp->valuator->lastx = x;
>          cp->valuator->lasty = y;
>      }
> -    pDev->valuator->lastx = x;
> -    pDev->valuator->lasty = y;
>  
>      /* for some reason inputInfo.pointer does not have coreEvents set */
>      if (coreOnly || pDev->coreEvents) {
> @@ -677,8 +705,8 @@ GetPointerEvents(xEvent *events, DeviceIntPtr pDev, int type, int buttons,
>              kbp->detail = pDev->button->map[buttons];
>          }
>  
> -        kbp->root_x = x;
> -        kbp->root_y = y;
> +        kbp->root_x = pDev->valuator->lastx;
> +        kbp->root_y = pDev->valuator->lasty;
>  
>          events++;
>          if (num_valuators) {