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 <[email protected]> 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
