[PATCH libinput 6/8] tablet: support tool-specific pressure offsets
Peter Hutterer
peter.hutterer at who-t.net
Wed Dec 2 17:03:40 PST 2015
On Wed, Dec 02, 2015 at 04:21:56PM -0800, Jason Gerecke wrote:
> 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.
a couple of questions here:
* how common is this pressure preload?
* does it affect all pens or only specific pens?
* is there a maximum observed threshold for this pen preload?
The last one is specifically: is there a point where we can throw up our
hands and tell the user to just buy a new pen rather than needing automatic
detection *and* the udev property for anything that exceeds that threshold?
> 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.
at least on the tablets that do distance we can combine the maximum pressure
threshold with the distance measurement. It's harder to do that on tablets
without distance though.
> 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...
long-term I want the tip down/up to match a distance 0 and have anything
with distance not send any pressure. The current BTN_TOUCH handling isn't
ideal because it discards lower pressure values, I think we should use a
libinput-internal pressure threshold here and fake the BTN_TOUCH based on
that. And the range between max(threshold, preload)-axis_max will then be
mapped into the normalised range.
So the logic will likely be tilted towards the pressure, with finer-grained
distance measuring:
* if the distance > threshold and pressure != 0 -> pressure preload
* if the distance < other threshold and pressure > preload -> distance 0
this makes BTN_TOUCH relatively obsolete, and also requires a minimum
distance threshold. The big drawback is that the stylus behaviour will
change over time, but hopefully this is in such a subtle measurement that it
won't be perceived by the user.
Cheers,
Peter
>
>
> [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