[PATCH libinput 6/8] tablet: support tool-specific pressure offsets
Jason Gerecke
killertofu at gmail.com
Wed Dec 2 16:21:56 PST 2015
On Tue, Dec 1, 2015 at 5:46 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> If a tool wears out, it may have a pre-loaded pressure offset. In that case,
> even when the tool is not physically in contact with the tablet surface it
> will send pressure events.
>
> The X.Org wacom driver has automatic pressure preload detection, but it is
> unreliable. A quick tap can trigger the detection, making the pen unusable
> (see xf86-input-wacom-0.23.0-43-g10cc765).
>
> Since this is a hardware-specific property, add a new udev property with the
> prefix TABLET_TOOL_PRESSURE_OFFSET_ and let it be set system-wide through the
> hwdb or some other process. Use the value of this property to offset any
> incoming pressure values and scale into the original normalized axis range.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
I've been asked by Ping (who is going to be OOO for the next few days)
to try and express some of her concerns about how libinput will handle
pen pressure. The two of us have had some lengthy discussions, and
though I'm not sure I agree on all points, I'll try to argue them as
best I can.
The idea behind the X driver's automatic preload detection was to
provide a way to handle the pen-specific pressure offsets without
requiring the user to configure or calibrate anything. The minimum
pressure seen from a pen is a fairly reliable measure of the offset,
and the X driver attempts to measure this as the pen hovers in
proximity. This works well in most circumstances, but obviously fails
if the pen is "stabbed" at the tablet so quickly that it comes into
contact before it can report an unloaded pressure value. This kind of
wanton hardware abuse isn't something we encourage ;)
Making the pressure offset a configuration option like you propose
here doesn't suffer from mis-detection, but does require the user to
periodically run a tool to update the UDEV HWDB. Updates to the DB
won't take immediate effect[1], won't propagate across systems[2], and
are incapable of distinguishing between tools without a serial number.
One change that could be made to the automatic method that would make
it a bit more reliable would be to define a maximum pressure that it
would consider to be a sane preload. A 'stabbed' pen would almost
certainly overshoot the ceiling and we could simply assume an
arbitrary preload of our choosing (zero? the ceiling?). A very light
'stab' could still cause issues, but its quite difficult to both move
the pen quickly enough to enter and exit prox at a high enough speed
to not get an unloaded pressure reading *and* only lightly come into
contact with the surface.
A second issue that Ping brought up is the tying of pressure
information to the "down" state (patch 4). Its possible that a user
would want to have the maximum possible dynamic range for pressure
(after accounting for the preload) in a drawing app so that the
lightest strokes were useful, while simultaneously wanting UI elements
like buttons and menus to only respond to more deliberate (and
high-pressure) pen events. In terms of protocol and library, this
would mean allowing applications to see non-zero pressure values even
before the pen is "down". If we want to do this, we would have to be
very deliberate in documenting that the behavior so that programmers
understand the pressure data is useful and should not be discarded (or
if absolutely necessary, rescaled so that the click threshold is
reported as 0 pressure). Right now I don't think any toolkits or
applications make use of pressure prior to click, though TBH I haven't
actually checked...
[1]: I know you can use `udevadm --reload`, but that doesn't affect
already-connected devices
[2]: The automatic detection doesn't require re-calibrating a pen on
every system its used on
Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one /
(That is to say, eight) to the two, /
But you can’t take seven from three, /
So you look at the sixty-fours....
> ---
> doc/device-configuration-via-udev.dox | 4 +
> doc/tablet-support.dox | 30 ++++++
> src/evdev-tablet.c | 71 +++++++++++++-
> src/libinput-private.h | 2 +
> test/litest-device-wacom-intuos-tablet.c | 12 +++
> test/tablet.c | 155 ++++++++++++++++++++++++++++++-
> 6 files changed, 270 insertions(+), 4 deletions(-)
>
> diff --git a/doc/device-configuration-via-udev.dox b/doc/device-configuration-via-udev.dox
> index f615cc1..ba44e58 100644
> --- a/doc/device-configuration-via-udev.dox
> +++ b/doc/device-configuration-via-udev.dox
> @@ -63,6 +63,10 @@ libinput_pointer_get_axis_source() for details.
> <dt>POINTINGSTICK_CONST_ACCEL</dt>
> <dd>A constant (linear) acceleration factor to apply to pointingstick deltas
> to normalize them.
> +<dt>TABLET_TOOL_PRESSURE_OFFSET_*</dt>
> +<dd>Specifies the pressure offset in device coordinates for the tool whose
> +name is given as part of the property name. See @ref tablet-pressure-offset
> +for details.</dd>
> <dt>LIBINPUT_MODEL_*</dt>
> <dd><b>This prefix is reserved as private API, do not use.</b> See @ref
> model_specific_configuration for details.
> diff --git a/doc/tablet-support.dox b/doc/tablet-support.dox
> index 24d08d2..27f970a 100644
> --- a/doc/tablet-support.dox
> +++ b/doc/tablet-support.dox
> @@ -92,4 +92,34 @@ if (value < min) {
> }
> @endcode
>
> + at section tablet-pressure-offset Pressure offset on worn-out tools
> +
> +When a tool is used for an extended period it can wear down physically. A
> +worn-down tool may never return a zero pressure value. Even when hovering
> +above the surface, the pressure value returned by the tool is nonzero,
> +creating a fake surface touch and making interaction with the tablet less
> +predictable.
> +
> +libinput allows for a static configuration to be applied to negate this
> +effect. If set, the <b>TABLET_TOOL_PRESSURE_OFFSET_...</b> udev property
> +specifies the tool-specific offset to be applied to any pressure
> +calculation. The calculation is done transparent to the caller.
> +
> +The udev property name contains the type and serial number of the tool, the
> +value is the offset in device coordinate range. The type of the tool is the
> +string as defined in linux/input.h without the BTN_TOOL_ prefix (e.g. "PEN",
> +"RUBBER", ...). The serial number is the uppercase hexadecimal serial number
> +without the 0x prefix.
> +
> +For example, if a tool with the serial number ab1234 has a pressure offset
> +of 20 on the pen and 30 on the eraser tool, the properties should be
> +
> + at code
> +TABLET_TOOL_PRESSURE_OFFSET_PEN_AB1234=20
> +TABLET_TOOL_PRESSURE_OFFSET_RUBBER_AB1234=30
> + at endcode
> +
> +Pressure offsets are not available on @ref LIBINPUT_TABLET_TOOL_TYPE_MOUSE
> +and @ref LIBINPUT_TABLET_TOOL_TYPE_LENS tools.
> +
> */
> diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> index e9473e8..d3ec1da 100644
> --- a/src/evdev-tablet.c
> +++ b/src/evdev-tablet.c
> @@ -202,7 +202,7 @@ tablet_update_tool(struct tablet_dispatch *tablet,
> }
>
> static inline double
> -normalize_pressure_dist_slider(const struct input_absinfo *absinfo)
> +normalize_dist_slider(const struct input_absinfo *absinfo)
> {
> double range = absinfo->maximum - absinfo->minimum;
> double value = (absinfo->value - absinfo->minimum) / range;
> @@ -211,6 +211,17 @@ normalize_pressure_dist_slider(const struct input_absinfo *absinfo)
> }
>
> static inline double
> +normalize_pressure(const struct input_absinfo *absinfo,
> + struct libinput_tablet_tool *tool)
> +{
> + double range = absinfo->maximum - absinfo->minimum;
> + int offset_val = absinfo->value - tool->pressure_offset;
> + double value = (offset_val - absinfo->minimum) / range;
> +
> + return value;
> +}
> +
> +static inline double
> normalize_tilt(const struct input_absinfo *absinfo)
> {
> double range = absinfo->maximum - absinfo->minimum;
> @@ -368,10 +379,12 @@ tablet_check_notify_axes(struct tablet_dispatch *tablet,
> else
> tablet->axes[a] = absinfo->value;
> break;
> - case LIBINPUT_TABLET_TOOL_AXIS_DISTANCE:
> case LIBINPUT_TABLET_TOOL_AXIS_PRESSURE:
> + tablet->axes[a] = normalize_pressure(absinfo, tool);
> + break;
> + case LIBINPUT_TABLET_TOOL_AXIS_DISTANCE:
> case LIBINPUT_TABLET_TOOL_AXIS_SLIDER:
> - tablet->axes[a] = normalize_pressure_dist_slider(absinfo);
> + tablet->axes[a] = normalize_dist_slider(absinfo);
> break;
> case LIBINPUT_TABLET_TOOL_AXIS_TILT_X:
> case LIBINPUT_TABLET_TOOL_AXIS_TILT_Y:
> @@ -733,6 +746,57 @@ tool_set_bits(const struct tablet_dispatch *tablet,
> }
> }
>
> +static void
> +tool_set_pressure_offset(const struct tablet_dispatch *tablet,
> + struct libinput_tablet_tool *tool)
> +{
> + struct evdev_device *device = tablet->device;
> + char prop_name[100];
> + const char *strtype;
> + const char *prop_value;
> + int offset;
> +
> + switch (tool->type) {
> + case LIBINPUT_TABLET_TOOL_TYPE_PEN:
> + strtype = "PEN";
> + break;
> + case LIBINPUT_TABLET_TOOL_TYPE_BRUSH:
> + strtype = "BRUSH";
> + break;
> + case LIBINPUT_TABLET_TOOL_TYPE_AIRBRUSH:
> + strtype = "AIRBRUSH";
> + break;
> + case LIBINPUT_TABLET_TOOL_TYPE_PENCIL:
> + strtype = "PENCIL";
> + break;
> + case LIBINPUT_TABLET_TOOL_TYPE_ERASER:
> + strtype = "RUBBER";
> + break;
> + default:
> + return;
> + }
> +
> + snprintf(prop_name,
> + sizeof(prop_name),
> + "TABLET_TOOL_PRESSURE_OFFSET_%s_%X",
> + strtype,
> + tool->serial);
> +
> + prop_value = udev_device_get_property_value(device->udev_device,
> + prop_name);
> +
> + if (!prop_value)
> + return;
> +
> + if (!atoi_safe(prop_value, &offset)) {
> + log_error(device->base.seat->libinput,
> + "Invalid pressure offset value %s\n", prop_value);
> + return;
> + }
> +
> + tool->pressure_offset = offset;
> +}
> +
> static struct libinput_tablet_tool *
> tablet_get_tool(struct tablet_dispatch *tablet,
> enum libinput_tablet_tool_type type,
> @@ -780,6 +844,7 @@ tablet_get_tool(struct tablet_dispatch *tablet,
> };
>
> tool_set_bits(tablet, tool);
> + tool_set_pressure_offset(tablet, tool);
>
> list_insert(tool_list, &tool->link);
> }
> diff --git a/src/libinput-private.h b/src/libinput-private.h
> index 38a14b8..091fd98 100644
> --- a/src/libinput-private.h
> +++ b/src/libinput-private.h
> @@ -259,6 +259,8 @@ struct libinput_tablet_tool {
> unsigned char buttons[NCHARS(KEY_MAX) + 1];
> int refcount;
> void *user_data;
> +
> + int pressure_offset;
> };
>
> struct libinput_event {
> diff --git a/test/litest-device-wacom-intuos-tablet.c b/test/litest-device-wacom-intuos-tablet.c
> index e0e1d44..8b9b59e 100644
> --- a/test/litest-device-wacom-intuos-tablet.c
> +++ b/test/litest-device-wacom-intuos-tablet.c
> @@ -37,6 +37,7 @@ static struct input_event proximity_in[] = {
> { .type = EV_ABS, .code = ABS_X, .value = LITEST_AUTO_ASSIGN },
> { .type = EV_ABS, .code = ABS_Y, .value = LITEST_AUTO_ASSIGN },
> { .type = EV_ABS, .code = ABS_DISTANCE, .value = LITEST_AUTO_ASSIGN },
> + { .type = EV_ABS, .code = ABS_PRESSURE, .value = LITEST_AUTO_ASSIGN },
> { .type = EV_ABS, .code = ABS_TILT_X, .value = LITEST_AUTO_ASSIGN },
> { .type = EV_ABS, .code = ABS_TILT_Y, .value = LITEST_AUTO_ASSIGN },
> { .type = EV_ABS, .code = ABS_MISC, .value = 1050626 },
> @@ -127,6 +128,16 @@ static int events[] = {
> -1, -1,
> };
>
> +static const char udev_rule[] =
> +"ACTION==\"remove\", GOTO=\"intuos_end\"\n"
> +"KERNEL!=\"event*\", GOTO=\"intuos_end\"\n"
> +"\n"
> +"ATTRS{name}==\"Wacom Intuos5 touch M Pen\",\n"
> +" ENV{TABLET_TOOL_PRESSURE_OFFSET_PEN_123456}=\"40\",\n"
> +" ENV{TABLET_TOOL_PRESSURE_OFFSET_RUBBER_123456}=\"70\"\n"
> +"\n"
> +"LABEL=\"intuos_end\"";
> +
> struct litest_test_device litest_wacom_intuos_tablet_device = {
> .type = LITEST_WACOM_INTUOS,
> .features = LITEST_TABLET | LITEST_DISTANCE | LITEST_TOOL_SERIAL,
> @@ -138,4 +149,5 @@ struct litest_test_device litest_wacom_intuos_tablet_device = {
> .id = &input_id,
> .events = events,
> .absinfo = absinfo,
> + .udev_rule = udev_rule,
> };
> diff --git a/test/tablet.c b/test/tablet.c
> index 125ab4b..97740f7 100644
> --- a/test/tablet.c
> +++ b/test/tablet.c
> @@ -2313,7 +2313,7 @@ START_TEST(tablet_pressure_distance_exclusive)
> struct libinput_event_tablet_tool *tev;
> struct axis_replacement axes[] = {
> { ABS_DISTANCE, 10 },
> - { ABS_PRESSURE, 20 },
> + { ABS_PRESSURE, 20 }, /* see the litest device */
> { -1, -1 },
> };
> double pressure, distance;
> @@ -2364,6 +2364,156 @@ START_TEST(tablet_pressure_distance_exclusive)
> }
> END_TEST
>
> +START_TEST(tablet_pressure_offset_none_for_default_pen)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + struct libinput_event_tablet_tool *tev;
> + struct axis_replacement axes[] = {
> + { ABS_DISTANCE, 0 },
> + { ABS_PRESSURE, 1 },
> + { -1, -1 },
> + };
> + double pressure;
> +
> + litest_push_event_frame(dev);
> + litest_tablet_proximity_in(dev, 5, 100, axes);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 1);
> + litest_pop_event_frame(dev);
> + litest_drain_events(li);
> +
> + litest_tablet_motion(dev, 70, 70, axes);
> + libinput_dispatch(li);
> +
> + litest_wait_for_event_of_type(li,
> + LIBINPUT_EVENT_TABLET_TOOL_AXIS,
> + -1);
> + event = libinput_get_event(li);
> + tev = libinput_event_get_tablet_tool_event(event);
> + pressure = libinput_event_tablet_tool_get_axis_value(tev,
> + LIBINPUT_TABLET_TOOL_AXIS_PRESSURE);
> + ck_assert_double_eq(pressure, 0.0);
> +
> + libinput_event_destroy(event);
> +}
> +END_TEST
> +
> +START_TEST(tablet_pressure_offset_pen)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + struct libinput_event_tablet_tool *tev;
> + struct axis_replacement axes[] = {
> + { ABS_DISTANCE, 0 },
> + { ABS_PRESSURE, 40 }, /* see the litest device */
> + { -1, -1 },
> + };
> + double pressure;
> +
> + litest_push_event_frame(dev);
> + litest_tablet_proximity_in(dev, 5, 100, axes);
> + /* the serial number to trigger the pen offset */
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 1);
> + litest_pop_event_frame(dev);
> + litest_drain_events(li);
> +
> + litest_push_event_frame(dev);
> + litest_tablet_motion(dev, 70, 70, axes);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_pop_event_frame(dev);
> + libinput_dispatch(li);
> +
> + litest_wait_for_event_of_type(li,
> + LIBINPUT_EVENT_TABLET_TOOL_AXIS,
> + -1);
> + event = libinput_get_event(li);
> + tev = libinput_event_get_tablet_tool_event(event);
> + pressure = libinput_event_tablet_tool_get_axis_value(tev,
> + LIBINPUT_TABLET_TOOL_AXIS_PRESSURE);
> + ck_assert_double_eq(pressure, 0.0);
> +
> + libinput_event_destroy(event);
> +
> + axes[0].value = 0;
> + axes[1].value = 80;
> + litest_push_event_frame(dev);
> + litest_tablet_motion(dev, 30, 30, axes);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_pop_event_frame(dev);
> + libinput_dispatch(li);
> + event = libinput_get_event(li);
> + tev = libinput_event_get_tablet_tool_event(event);
> +
> + pressure = libinput_event_tablet_tool_get_axis_value(tev,
> + LIBINPUT_TABLET_TOOL_AXIS_PRESSURE);
> +
> + ck_assert_double_gt(pressure, 0.0);
> +
> + libinput_event_destroy(event);
> +}
> +END_TEST
> +
> +START_TEST(tablet_pressure_offset_rubber)
> +{
> + struct litest_device *dev = litest_current_device();
> + struct libinput *li = dev->libinput;
> + struct libinput_event *event;
> + struct libinput_event_tablet_tool *tev;
> + struct axis_replacement axes[] = {
> + { ABS_DISTANCE, 0 },
> + { ABS_PRESSURE, 70 },
> + { -1, -1 },
> + };
> + double pressure;
> +
> + litest_push_event_frame(dev);
> + litest_tablet_proximity_in(dev, 5, 100, axes);
> + /* the serial number to trigger the pen offset */
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_event(dev, EV_KEY, BTN_TOOL_RUBBER, 1);
> + litest_event(dev, EV_KEY, BTN_TOUCH, 1);
> + litest_pop_event_frame(dev);
> + litest_drain_events(li);
> +
> + litest_push_event_frame(dev);
> + litest_tablet_motion(dev, 70, 70, axes);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_pop_event_frame(dev);
> + libinput_dispatch(li);
> +
> + litest_wait_for_event_of_type(li,
> + LIBINPUT_EVENT_TABLET_TOOL_AXIS,
> + -1);
> + event = libinput_get_event(li);
> + tev = libinput_event_get_tablet_tool_event(event);
> + pressure = libinput_event_tablet_tool_get_axis_value(tev,
> + LIBINPUT_TABLET_TOOL_AXIS_PRESSURE);
> + ck_assert_double_eq(pressure, 0.0);
> +
> + libinput_event_destroy(event);
> +
> + axes[0].value = 0;
> + axes[1].value = 80;
> + litest_push_event_frame(dev);
> + litest_tablet_motion(dev, 30, 30, axes);
> + litest_event(dev, EV_MSC, MSC_SERIAL, 0x123456);
> + litest_pop_event_frame(dev);
> + libinput_dispatch(li);
> + event = libinput_get_event(li);
> + tev = libinput_event_get_tablet_tool_event(event);
> +
> + pressure = libinput_event_tablet_tool_get_axis_value(tev,
> + LIBINPUT_TABLET_TOOL_AXIS_PRESSURE);
> +
> + ck_assert_double_gt(pressure, 0.0);
> +
> + libinput_event_destroy(event);
> +}
> +END_TEST
> +
> void
> litest_setup_tests(void)
> {
> @@ -2408,4 +2558,7 @@ litest_setup_tests(void)
>
> litest_add("tablet:time", tablet_time_usec, LITEST_TABLET, LITEST_ANY);
> litest_add("tablet:pressure", tablet_pressure_distance_exclusive, LITEST_TABLET | LITEST_DISTANCE, LITEST_ANY);
> + litest_add_for_device("tablet:pressure", tablet_pressure_offset_none_for_default_pen, LITEST_WACOM_INTUOS);
> + litest_add_for_device("tablet:pressure", tablet_pressure_offset_pen, LITEST_WACOM_INTUOS);
> + litest_add_for_device("tablet:pressure", tablet_pressure_offset_rubber, LITEST_WACOM_INTUOS);
> }
> --
> 2.5.0
>
> _______________________________________________
> 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