[PATCH libinput] evdev: fix device_transform_ functions

[Jonas Ådahl]

On Tue, Feb 18, 2014 at 02:28:53PM +1000, Peter Hutterer wrote:
> On Mon, Feb 17, 2014 at 01:42:52PM -0500, Benjamin Tissoires wrote:
> > X and Y are li_fixed_t, which is 24.8 fixed point real number.
> > li_fixed_t max is thus ~8388607.
> > 
> > On a touchscreen with a range of 32767 values (like a 3M sensor), and
> > mapped on monitor with a resolution of 1920x1080, we currently have:
> > (x - li_fixed_from_int(device->abs.min_x)) * width == 62912640
> > 
> > which is 7 times bigger than li_fixed_t max.
> > 
> > To keep the precision of the sensor, first compute the uniformized
> > coordinate (in range 0 .. 1.0) of the touch point, then multiply it
> > by the screen dimension, and revert it to a li_fixed_t.
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
> > ---
> > 
> > Hi,
> > 
> > I have hit this problem by playing with a touchscreen reporting 4096 values, on
> > xf86-input-libinput. xf86-input-libinput does not use the real screen size, but
> > 0xffff instead. This allows to report a touchscreen with a range of 128 values
> > to work properly :(
> > 
> > I went through the multitouch device database, and took one example of a more
> > real use-case (xf86-input-libinput is still in an early shape).
> 
> fwiw, this is exactly the type of use-case where it would be simple and
> worth it to knock up a test for a single device and make sure that the
> coordinates are correct. which gives us a nice reproducer and prevents us
> from errors like this in the future.
> 
> >  src/evdev.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/evdev.c b/src/evdev.c
> > index d8dff65..0d033b8 100644
> > --- a/src/evdev.c
> > +++ b/src/evdev.c
> > @@ -91,8 +91,9 @@ evdev_device_transform_x(struct evdev_device *device,
> >  			 li_fixed_t x,
> >  			 uint32_t width)
> >  {
> > -	return (x - li_fixed_from_int(device->abs.min_x)) * width /
> > +	double x_scaled = (li_fixed_to_double(x) - device->abs.min_x) /
> >  		(device->abs.max_x - device->abs.min_x + 1);
> > +	return li_fixed_from_double(x_scaled * width);
> 
> A simple 1L * .... should suffice, right?

It would if 1L means 64 bit, but that is not the case on all platforms.
A cast to uint64_t is enough for this case though.

Jonas

> 
> Cheers,
>    Peter
> 
> >  }
> >  
> >  li_fixed_t
> > @@ -100,8 +101,9 @@ evdev_device_transform_y(struct evdev_device *device,
> >  			 li_fixed_t y,
> >  			 uint32_t height)
> >  {
> > -	return (y - li_fixed_from_int(device->abs.min_y)) * height /
> > +	double y_scaled = (li_fixed_to_double(y) - device->abs.min_y) /
> >  		(device->abs.max_y - device->abs.min_y + 1);
> > +	return li_fixed_from_double(y_scaled * height);
> >  }
> >  
> >  static void
> > -- 
> > 1.8.5.3
> >