[PATCH v2 weston] libinput: normalize WL_CALIBRATION before passing it to libinput
Jonas Ådahl
jadahl at gmail.com
Mon Sep 8 11:16:07 PDT 2014
On Fri, Sep 05, 2014 at 11:25:25AM +1000, Peter Hutterer wrote:
> WL_CALIBRATION, introduced in weston-1.1, requires the translation component
> of the calibration matrix to be in screen coordinates. libinput does not have
> access to this and it's not a very generic way to do this anyway. So with
> the libinput backend, WL_CALIBRATION support is currently broken (#82742).
> This cannot be fixed in libinput without changing its API for this specific
> use-case.
>
> This patch lets weston take care of WL_CALIBRATION. It takes the original
> format and normalizes it before passing it to libinput. This way libinput
> still does the coordinate transformation, weston just needs to provide the
> initial configuration.
>
> Note that this needs an updated libinput, otherwise libinput will try to
> transform coordinates as well.
Should we also deprecate the WL_CALIBRATION udev parameter and warn
about when using it, as well as having weston-calibrator output the
normalized calibration matrix? We could also move weston-calibrator to
libinput, but then we'd need to port it to something else than toytoolkit
(or by not using a toolkit), which naturally is less trivial. Keeping
weston-calibrator that outputs the non-normalized matrix, at least after
the release, is probably not a good idea.
In anyway, this patch can be considered
Reviewed-by: Jonas Ådahl <jadahl at gmail.com> with some minor style nits
below.
Jonas
> ---
> Changes to v1:
> - fix copy/paste error assigning height to the screen width
>
> https://bugs.freedesktop.org/show_bug.cgi?id=82742 has positive test results
> so we don't break exisint setups with WL_CALIBRATION set.
>
> Can be merged once libinput 0.6 is released.
>
> configure.ac | 2 +-
> src/libinput-device.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index bc5c88a..fb19fb2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -160,7 +160,7 @@ AC_ARG_ENABLE(libinput-backend, [ --disable-libinput-backend],,
> AM_CONDITIONAL([ENABLE_LIBINPUT_BACKEND], [test x$enable_libinput_backend = xyes])
> if test x$enable_libinput_backend = xyes; then
> AC_DEFINE([BUILD_LIBINPUT_BACKEND], [1], [Build the libinput input device backend])
> - PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.5.0])
> + PKG_CHECK_MODULES(LIBINPUT_BACKEND, [libinput >= 0.6.0])
> fi
>
>
> diff --git a/src/libinput-device.c b/src/libinput-device.c
> index 6e50eeb..fded34c 100644
> --- a/src/libinput-device.c
> +++ b/src/libinput-device.c
> @@ -282,6 +282,90 @@ notify_output_destroy(struct wl_listener *listener, void *data)
> }
> }
>
> +/**
> + * The WL_CALIBRATION property requires a pixel-specific matrix to be
> + * applied after scaling device coordinates to screen coordinates. libinput
> + * can't do that, so we need to convert the calibration to the normalized
> + * format libinput expects.
> + */
> +static void
> +evdev_device_set_calibration(struct evdev_device *device)
> +{
> + struct udev *udev;
> + struct udev_device *udev_device = NULL;
> + const char *sysname = libinput_device_get_sysname(device->device);
> + const char *calibration_values;
> + uint32_t width, height;
> + float calibration[6];
> + enum libinput_config_status status;
> +
> + if (!device->output)
> + return;
> +
> + width = device->output->width;
> + height = device->output->height;
> + if (width == 0 || height == 0)
> + return;
> +
> + /* If libinput has a pre-set calibration matrix, don't override it */
> + if (!libinput_device_config_calibration_has_matrix(device->device) ||
> + libinput_device_config_calibration_get_default_matrix(
> + device->device,
> + calibration) != 0)
> + return;
> +
> + udev = udev_new();
> + if (!udev)
> + return;
> +
> + udev_device = udev_device_new_from_subsystem_sysname(udev,
> + "input",
> + sysname);
> + if (!udev_device)
> + goto out;
> +
> + calibration_values =
> + udev_device_get_property_value(udev_device,
> + "WL_CALIBRATION");
These two lines should be indented.
> +
> + if (!calibration_values || sscanf(calibration_values,
> + "%f %f %f %f %f %f",
> + &calibration[0],
> + &calibration[1],
> + &calibration[2],
> + &calibration[3],
> + &calibration[4],
> + &calibration[5]) != 6)
> + goto out;
> +
> + weston_log("Applying calibration: %f %f %f %f %f %f "
> + "(normalized %f %f)\n",
> + calibration[0],
> + calibration[1],
> + calibration[2],
> + calibration[3],
> + calibration[4],
> + calibration[5],
> + calibration[2]/width,
> + calibration[5]/height);
There should be spaces between the binary operators.
> +
> + /* normalize to a format libinput can use. There is a chance of
> + this being wrong if the width/height don't match the device
> + width/height but I'm not sure how to fix that */
> + calibration[2] /= width;
> + calibration[5] /= height;
> +
> + status = libinput_device_config_calibration_set_matrix(device->device,
> + calibration);
> + if (status != LIBINPUT_CONFIG_STATUS_SUCCESS)
> + weston_log("Failed to apply calibration.\n");
> +
> +out:
> + if (udev_device)
> + udev_device_unref(udev_device);
> + udev_unref(udev);
> +}
> +
> void
> evdev_device_set_output(struct evdev_device *device,
> struct weston_output *output)
> @@ -295,6 +379,7 @@ evdev_device_set_output(struct evdev_device *device,
> device->output_destroy_listener.notify = notify_output_destroy;
> wl_signal_add(&output->destroy_signal,
> &device->output_destroy_listener);
> + evdev_device_set_calibration(device);
> }
>
> static void
> @@ -318,6 +403,8 @@ configure_device(struct evdev_device *device)
> libinput_device_config_tap_set_enabled(device->device,
> enable_tap);
> }
> +
> + evdev_device_set_calibration(device);
> }
>
> struct evdev_device *
> --
> 1.9.3
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list