[PATCH] dix: the input matrix must work on pixels, not device ranges

[Keith Packard]

Peter Hutterer <peter.hutterer at who-t.net> writes:

> The screen width is for most devices smaller than the device range.
> Previously, we applied the matrix to the device coordinate range, then
> converting into screen (subpixel) coordinates. The half-way point of a
> device is likely somewhere inside a subpixel, just to the left of the pixel
> we want (in a 50:50 configuration). Thus, the mapping is off by one pixel.
> In a 50/50 mapping with a 50% offset this means that we hit the right-most
> pixel of the left screen, even when we should only be bound to the right
> screen.

Can you provide some concrete numbers so that we can see the effect of
the different operation ordering? From a brief reading of the code, it
seems like you're just changing the order of operations from:

    device -> transformed_device -> clipped_device -> desktop -> screen

to

    device -> desktop -> screen

I'm obviously missing something important here; is there a float->int
truncation that I skipped?

> -    sx = dev->valuator->axes[0].max_value - dev->valuator->axes[0].min_value;
> -    sy = dev->valuator->axes[1].max_value - dev->valuator->axes[1].min_value;
> +    sx = screenInfo.width - screenInfo.x;
> +    sy = screenInfo.height - screenInfo.y;

Shouldn't the scale just be the overall screen size, not the screen size
minus the upper left corner?

And, given that you want screen coordinates coming out of this function,
shouldn't the translation be screenInfo.x and screenInfo.y for the
invscale matrix?

>  
>      /* invscale */
>      pixman_f_transform_init_scale(&scale, sx, sy);

> @@ -129,8 +129,8 @@ DeviceSetTransform(DeviceIntPtr dev, float *transform_data)
>  
>      /* scale */
>      pixman_f_transform_init_scale(&scale, 1.0 / sx, 1.0 / sy);
> -    scale.m[0][2] = -dev->valuator->axes[0].min_value / sx;
> -    scale.m[1][2] = -dev->valuator->axes[1].min_value / sy;
> +    scale.m[0][2] = -screenInfo.x / sx;
> +    scale.m[1][2] = -screenInfo.y / sy;

This looks correct -- get the screen origin out of the coordinate after
scaling it.

> +        clipAbsolute(pDev, &mask); /* set to axis boundaries */
> +        if ((flags & POINTER_NORAW) == 0)

Do we need to clip the screen coordinates at any point in this
computation? They were implicitly clipped before as they were taken from
the clipped device coordinates.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20130424/6f164a94/attachment.pgp>