On Mon, Sep 08, 2014 at 08:16:07PM +0200, Jonas Ådahl wrote: > 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?
tbh, I'd rather leave it as-is. I'm not a big fan of the configuration back-channel in libinput, I'd prefer this to be set by the compositor as required. with WL_CALIBRATION as-is that works fine, having a weston tool spit out a udev configuration that is then read by libinput to give weston calibrated values seems a bit all over the place to me. > 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. thanks for the review, updated patch is on the list. Cheers, Peter > > > 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
