[PATCH libinput 07/11] evdev: switch to a normalized transformation matrix

[Hans de Goede]

Hi,

On 08/29/2014 02:04 AM, Peter Hutterer wrote:
> On Thu, Aug 28, 2014 at 03:07:26PM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 08/27/2014 06:31 AM, Peter Hutterer wrote:
>>> The big change here is the requirement to have the translation component in a
>>> device-normalized coordinate space. Without that, we cannot reliably rotate as
>>> the coordinate space is effectively unknown and may differ between the axes.
>>> This affects any rotation matrix or translation matrix, pure scale matrices
>>> were working just fine since they're unit-less.
>>>
>>> Requiring the matrix in device-normalized space makes it possible for libinput
>>> to rotate or otherwise handle the matrix independent of the screen resolution.
>>> The rotation matrix is documented in a bit more detail to make it easier for
>>> users to figure it out.
>>>
>>> This changes the definition of the WL_CALIBRATION property (which is currently
>>> broken).
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>  src/evdev.c    | 71 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>>>  src/evdev.h    |  2 +-
>>>  src/libinput.h | 26 +++++++++++++++++++++
>>>  3 files changed, 85 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/src/evdev.c b/src/evdev.c
>>> index a029887..4cd3cfa 100644
>>> --- a/src/evdev.c
>>> +++ b/src/evdev.c
>>> @@ -150,20 +150,10 @@ evdev_device_led_update(struct evdev_device *device, enum libinput_led leds)
>>>  static void
>>>  transform_absolute(struct evdev_device *device, int32_t *x, int32_t *y)
>>>  {
>>> -	int32_t tx, ty;
>>> -
>>>  	if (!device->abs.apply_calibration)
>>>  		return;
>>>  
>>> -	tx = *x * device->abs.calibration[0] +
>>> -		*y * device->abs.calibration[1] +
>>> -		device->abs.calibration[2];
>>> -
>>> -	ty = *x * device->abs.calibration[3] +
>>> -		*y * device->abs.calibration[4] +
>>> -		device->abs.calibration[5];
>>> -	*x = tx;
>>> -	*y = ty;
>>> +	matrix_mult_vec(&device->abs.calibration, x, y);
>>>  }
>>>  
>>>  static inline double
>>> @@ -913,6 +903,8 @@ evdev_device_create(struct libinput_seat *seat,
>>>  	device->pending_event = EVDEV_NONE;
>>>  	device->devname = libevdev_get_name(device->evdev);
>>>  
>>> +	matrix_init_identity(&device->abs.calibration);
>>> +
>>>  	if (evdev_configure_device(device) == -1)
>>>  		goto err;
>>>  
>>> @@ -986,8 +978,61 @@ void
>>>  evdev_device_calibrate(struct evdev_device *device,
>>>  		       const float calibration[6])
>>>  {
>>> -	device->abs.apply_calibration = 1;
>>> -	memcpy(device->abs.calibration, calibration, sizeof device->abs.calibration);
>>> +	struct matrix scale,
>>> +		      translate,
>>> +		      transform;
>>> +	double sx, sy;
>>> +
>>> +	matrix_from_farray6(&transform, calibration);
>>> +	device->abs.apply_calibration = !matrix_is_identity(&transform);
>>> +
>>> +	if (!device->abs.apply_calibration) {
>>> +		matrix_init_identity(&device->abs.calibration);
>>
>> Erm, "!device->abs.apply_calibration" == "!!matrix_is_identity(&transform)" ==
>> "matrix_is_identity(&transform)", so your initializing the matrix to identity
>> here when it already is identity.
> 
> not quite. the matrix may have been set by previous calibration setting
> (either by the caller or by the udev property). Now a caller sets the matrix
> to the identity matrix.
> 
> it's technically not needed since we have the separate apply_calibration
> toggle but I'd rather have the matrix in a known good state than in whatever
> it was before.

Ah the is_identity and and init_identity are done on different matrices, I missed that,
in that case this is:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans