[PATCH libinput 3/5] tablet: add pressure threshold handling

Peter Hutterer peter.hutterer at who-t.net
Tue Jan 5 17:42:53 PST 2016


On Tue, Jan 05, 2016 at 11:39:15AM -0800, Jason Gerecke wrote:
> 'On Mon, Jan 4, 2016 at 5:21 PM, Peter Hutterer
> <peter.hutterer at who-t.net> wrote:
> > On tablets with ABS_PRESSURE use a pressure value to determine tip state, not
> > BTN_TOUCH. This enables us (down the road) to have device-specific pressure
> > thresholds. For now we use a 5% default for all devices.
> >
> > The threshold is a range, if we go past the upper range we initiate the tip
> > down, if we go below the lower range we release the tip again.
> >
> > This affects two current tests:
> > * Once we have pressure offsets and pressure thresholds, we're biased towards
> > pressure. So we can only check that distance is zero when there is a pressure
> > value, not the other way round.
> > * When the pressure threshold is exceeded on proximity in with a nonzero
> > distance, we can only warn and handle the pressure as normal. Since this is a
> > niche case anyway anything fancier is likely unnecessary.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  doc/tablet-support.dox | 16 +++++++++
> >  src/evdev-tablet.c     | 94 ++++++++++++++++++++++++++++++++++++++++----------
> >  src/libinput-private.h |  8 +++++
> >  test/tablet.c          | 67 ++++++++++++-----------------------
> >  4 files changed, 122 insertions(+), 63 deletions(-)
> >
> > diff --git a/doc/tablet-support.dox b/doc/tablet-support.dox
> > index fbe778d..a3d4d7b 100644
> > --- a/doc/tablet-support.dox
> > +++ b/doc/tablet-support.dox
> > @@ -33,6 +33,22 @@ Tablet tools may send button events; these are exclusively for extra buttons
> >  unrelated to the tip. A button event is independent of the tip and occur
> >  at any time.
> >
> > +Some tablet tools' pressure detection is too sensitive, causing phantom
> > +touches when the user only slightly brushes the surfaces. For example, some
> > +tools are capable of detecting 1 gram of pressure.
> > +
> > +libinput uses a device-specific pressure threshold to determine when the tip
> > +is considered logically down. As a result, libinput may send a nonzero
> > +pressure value while the tip is logically up. Most application can and
> > +should ignore pressure information until they receive the event of type @ref
> > +LIBINPUT_EVENT_TABLET_TOOL_TIP. Applications that require extremely
> > +fine-grained pressure sensitivity should use the pressure data instead of
> > +the tip events.
> > +
> > +Note that the pressure threshold to trigger a logical tip event may be zero
> > +on some devices. On tools without pressure sensitivity, determining when a
> > +tip is down is device-specific.
> > +
> >  @section tablet-axes Special axes on tablet tools
> >
> >  A tablet tool usually provides additional information beyond x/y positional
> > diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
> > index 7f6c860..2f57805 100644
> > --- a/src/evdev-tablet.c
> > +++ b/src/evdev-tablet.c
> > @@ -617,10 +617,15 @@ tablet_process_key(struct tablet_dispatch *tablet,
> >                                    e->value);
> >                 break;
> >         case BTN_TOUCH:
> > -               if (e->value)
> > -                       tablet_set_status(tablet, TABLET_TOOL_ENTERING_CONTACT);
> > -               else
> > -                       tablet_set_status(tablet, TABLET_TOOL_LEAVING_CONTACT);
> > +               if (!bit_is_set(tablet->axis_caps,
> > +                               LIBINPUT_TABLET_TOOL_AXIS_PRESSURE)) {
> > +                       if (e->value)
> > +                               tablet_set_status(tablet,
> > +                                                 TABLET_TOOL_ENTERING_CONTACT);
> > +                       else
> > +                               tablet_set_status(tablet,
> > +                                                 TABLET_TOOL_LEAVING_CONTACT);
> > +               }
> >                 break;
> >         case BTN_LEFT:
> >         case BTN_RIGHT:
> > @@ -844,6 +849,12 @@ tool_set_bits(const struct tablet_dispatch *tablet,
> >         }
> >  }
> >
> > +static inline int
> > +axis_range_percentage(const struct input_absinfo *a, double percent)
> > +{
> > +       return (a->maximum - a->minimum) * percent/100.0 + a->minimum;
> > +}
> > +
> >  static struct libinput_tablet_tool *
> >  tablet_get_tool(struct tablet_dispatch *tablet,
> >                 enum libinput_tablet_tool_type type,
> > @@ -894,12 +905,22 @@ tablet_get_tool(struct tablet_dispatch *tablet,
> >
> >                 tool->pressure_offset = 0;
> >                 tool->has_pressure_offset = false;
> > +               tool->pressure_threshold.lower = 0;
> > +               tool->pressure_threshold.upper = 1;
> >
> >                 pressure = libevdev_get_abs_info(tablet->device->evdev,
> >                                                  ABS_PRESSURE);
> > -               if (pressure)
> > +               if (pressure) {
> >                         tool->pressure_offset = pressure->minimum;
> >
> > +                       /* 5% of the pressure range */
> > +                       tool->pressure_threshold.upper =
> > +                               pressure->minimum +
> > +                               axis_range_percentage(pressure, 5);
> 
> The addition here seems incorrect. 'axis_range_percentage' already
> accounts for the minimum, so you're effectively double-counting.

oops. fixed, thanks.


> 
> > +                       tool->pressure_threshold.lower =
> > +                               pressure->minimum;
> > +               }
> > +
> >                 tool_set_bits(tablet, tool);
> >
> >                 list_insert(tool_list, &tool->link);
> > @@ -965,7 +986,8 @@ tablet_notify_buttons(struct tablet_dispatch *tablet,
> >  }
> >
> >  static void
> > -sanitize_pressure_distance(struct tablet_dispatch *tablet)
> > +sanitize_pressure_distance(struct tablet_dispatch *tablet,
> > +                          struct libinput_tablet_tool *tool)
> >  {
> >         bool tool_in_contact;
> >         const struct input_absinfo *distance,
> > @@ -974,9 +996,14 @@ sanitize_pressure_distance(struct tablet_dispatch *tablet)
> >         distance = libevdev_get_abs_info(tablet->device->evdev, ABS_DISTANCE);
> >         pressure = libevdev_get_abs_info(tablet->device->evdev, ABS_PRESSURE);
> >
> > -       tool_in_contact = (tablet_has_status(tablet, TABLET_TOOL_IN_CONTACT) ||
> > -                          tablet_has_status(tablet,
> > -                                            TABLET_TOOL_ENTERING_CONTACT));
> > +       if (!pressure || !distance)
> > +               return;
> > +
> > +       if (!bit_is_set(tablet->changed_axes, LIBINPUT_TABLET_TOOL_AXIS_DISTANCE) &&
> > +           !bit_is_set(tablet->changed_axes, LIBINPUT_TABLET_TOOL_AXIS_PRESSURE))
> > +               return;
> > +
> > +       tool_in_contact = (pressure->value > tool->pressure_offset);
> >
> >         /* Keep distance and pressure mutually exclusive */
> >         if (distance &&
> > @@ -1017,16 +1044,11 @@ sanitize_mouse_lens_rotation(struct tablet_dispatch *tablet)
> >                 set_bit(tablet->changed_axes, LIBINPUT_TABLET_TOOL_AXIS_ROTATION_Z);
> >  }
> >
> > -static inline int
> > -axis_range_percentage(const struct input_absinfo *a, int percent)
> > -{
> > -       return (a->maximum - a->minimum) * percent/100 + a->minimum;
> > -}
> > -
> >  static void
> > -sanitize_tablet_axes(struct tablet_dispatch *tablet)
> > +sanitize_tablet_axes(struct tablet_dispatch *tablet,
> > +                    struct libinput_tablet_tool *tool)
> >  {
> > -       sanitize_pressure_distance(tablet);
> > +       sanitize_pressure_distance(tablet, tool);
> >         sanitize_mouse_lens_rotation(tablet);
> >  }
> >
> > @@ -1086,6 +1108,41 @@ detect_pressure_offset(struct tablet_dispatch *tablet,
> >  }
> >
> >  static void
> > +detect_tool_contact(struct tablet_dispatch *tablet,
> > +                   struct evdev_device *device,
> > +                   struct libinput_tablet_tool *tool)
> > +{
> > +       int pressure;
> > +
> > +       if (!bit_is_set(tool->axis_caps, LIBINPUT_TABLET_TOOL_AXIS_PRESSURE))
> > +               return;
> > +
> > +       /* if we have pressure, always use that for contact, not BTN_TOUCH */
> > +       if (tablet_has_status(tablet, TABLET_TOOL_ENTERING_CONTACT))
> > +               log_bug_libinput(device->base.seat->libinput,
> > +                                "Invalid status: entering contact\n");
> > +       if (tablet_has_status(tablet, TABLET_TOOL_LEAVING_CONTACT) &&
> > +           !tablet_has_status(tablet, TABLET_TOOL_LEAVING_PROXIMITY))
> > +               log_bug_libinput(device->base.seat->libinput,
> > +                                "Invalid status: leaving contact\n");
> > +
> > +       pressure = libevdev_get_event_value(tablet->device->evdev,
> > +                                           EV_ABS,
> > +                                           ABS_PRESSURE);
> > +
> > +       if (tool->has_pressure_offset)
> > +               pressure -= tool->pressure_offset;
> > +
> > +       if (pressure <= tool->pressure_threshold.lower &&
> > +           tablet_has_status(tablet, TABLET_TOOL_IN_CONTACT)) {
> > +               tablet_set_status(tablet, TABLET_TOOL_LEAVING_CONTACT);
> > +       } else if (pressure >= tool->pressure_threshold.upper &&
> > +                  !tablet_has_status(tablet, TABLET_TOOL_IN_CONTACT)) {
> > +               tablet_set_status(tablet, TABLET_TOOL_ENTERING_CONTACT);
> > +       }
> 
> This probably doesn't work as expected if the device has a non-zero
> minimum ABS_PRESSURE. For example, a tablet with a pressure range of
> [100..200] will have both its lower threshold and pressure offset set
> to 100, but because 'pressure' has the offset subtracted away it will
> only ever take on values from [0..100]. Even maximum pressure would
> not be sufficient to have the tool enter contact.
> 
> I believe you would either want to subtract away the ABS_PRESSURE
> minimum from the thresholds here or when setting them in
> 'tablet_get_tool'.

good point, thanks. I've gone with the simplest approach here, subtracting
minimum from the offset. That's only in one place, everything stays in
device coordinates and that calculation is only required if we have a
pressure offset. v2 coming up.

Cheers,
   Peter
> 
> > +}
> > +
> > +static void
> >  tablet_mark_all_axes_changed(struct tablet_dispatch *tablet,
> >                              struct libinput_tablet_tool *tool)
> >  {
> > @@ -1188,7 +1245,8 @@ tablet_flush(struct tablet_dispatch *tablet,
> >                                       TABLET_TOOL_ENTERING_PROXIMITY))
> >                         tablet_mark_all_axes_changed(tablet, tool);
> >                 detect_pressure_offset(tablet, device, tool);
> > -               sanitize_tablet_axes(tablet);
> > +               detect_tool_contact(tablet, device, tool);
> > +               sanitize_tablet_axes(tablet, tool);
> >                 tablet_check_notify_axes(tablet, device, time, tool);
> >
> >                 tablet_unset_status(tablet, TABLET_TOOL_ENTERING_PROXIMITY);
> > diff --git a/src/libinput-private.h b/src/libinput-private.h
> > index 3d714af..5238e64 100644
> > --- a/src/libinput-private.h
> > +++ b/src/libinput-private.h
> > @@ -63,6 +63,12 @@ struct normalized_range_coords {
> >         double x, y;
> >  };
> >
> > +/* A threshold with an upper and lower limit */
> > +struct threshold {
> > +       int upper;
> > +       int lower;
> > +};
> > +
> >  struct libinput_interface_backend {
> >         int (*resume)(struct libinput *libinput);
> >         void (*suspend)(struct libinput *libinput);
> > @@ -277,6 +283,8 @@ struct libinput_tablet_tool {
> >         int refcount;
> >         void *user_data;
> >
> > +       /* The pressure threshold assumes a pressure_offset of 0 */
> > +       struct threshold pressure_threshold;
> >         int pressure_offset; /* in device coordinates */
> >         bool has_pressure_offset;
> >  };
> > diff --git a/test/tablet.c b/test/tablet.c
> > index fc6d1df..0ddad77 100644
> > --- a/test/tablet.c
> > +++ b/test/tablet.c
> > @@ -2528,7 +2528,7 @@ START_TEST(tablet_pressure_distance_exclusive)
> >         struct libinput_event_tablet_tool *tev;
> >         struct axis_replacement axes[] = {
> >                 { ABS_DISTANCE, 10 },
> > -               { ABS_PRESSURE, 20 }, /* see the litest device */
> > +               { ABS_PRESSURE, 0 },
> >                 { -1, -1 },
> >         };
> >         double pressure, distance;
> > @@ -2536,6 +2536,7 @@ START_TEST(tablet_pressure_distance_exclusive)
> >         litest_tablet_proximity_in(dev, 5, 100, axes);
> >         litest_drain_events(li);
> >
> > +       litest_axis_set_value(axes, ABS_PRESSURE, 2);
> >         litest_tablet_motion(dev, 70, 70, axes);
> >         libinput_dispatch(li);
> >
> > @@ -2545,28 +2546,8 @@ START_TEST(tablet_pressure_distance_exclusive)
> >         pressure = libinput_event_tablet_tool_get_pressure(tev);
> >         distance = libinput_event_tablet_tool_get_distance(tev);
> >
> > -       ck_assert_double_eq(pressure, 0.0);
> > -       ck_assert_double_ne(distance, 0.0);
> > -
> > -       libinput_event_destroy(event);
> > -
> > -       litest_axis_set_value(axes, ABS_DISTANCE, 15);
> > -       litest_axis_set_value(axes, ABS_PRESSURE, 25);
> > -       litest_event(dev, EV_KEY, BTN_TOUCH, 1);
> > -       litest_tablet_motion(dev, 30, 30, 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_pressure(tev);
> > -       distance = libinput_event_tablet_tool_get_distance(tev);
> > -
> > -       ck_assert_double_eq(distance, 0.0);
> >         ck_assert_double_ne(pressure, 0.0);
> > +       ck_assert_double_eq(distance, 0.0);
> >
> >         libinput_event_destroy(event);
> >  }
> > @@ -2933,6 +2914,18 @@ START_TEST(tablet_pressure_offset_increase)
> >  }
> >  END_TEST
> >
> > +static void pressure_threshold_warning(struct libinput *libinput,
> > +                                      enum libinput_log_priority priority,
> > +                                      const char *format,
> > +                                      va_list args)
> > +{
> > +       int *warning_triggered = (int*)libinput_get_user_data(libinput);
> > +
> > +       if (priority == LIBINPUT_LOG_PRIORITY_ERROR &&
> > +           strstr(format, "pressure offset greater"))
> > +               (*warning_triggered)++;
> > +}
> > +
> >  START_TEST(tablet_pressure_offset_exceed_threshold)
> >  {
> >         struct litest_device *dev = litest_current_device();
> > @@ -2945,40 +2938,24 @@ START_TEST(tablet_pressure_offset_exceed_threshold)
> >                 { -1, -1 },
> >         };
> >         double pressure;
> > +       int warning_triggered = 0;
> >
> >         litest_drain_events(li);
> >
> > -       litest_disable_log_handler(li);
> > +       libinput_set_user_data(li, &warning_triggered);
> > +
> > +       libinput_log_set_handler(li, pressure_threshold_warning);
> >         litest_tablet_proximity_in(dev, 5, 100, axes);
> >         libinput_dispatch(li);
> >         event = libinput_get_event(li);
> >         tev = litest_is_tablet_event(event,
> >                                      LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY);
> >         pressure = libinput_event_tablet_tool_get_pressure(tev);
> > -       ck_assert_double_eq(pressure, 0.0);
> > +       ck_assert_double_gt(pressure, 0.0);
> >         libinput_event_destroy(event);
> > +
> > +       ck_assert_int_eq(warning_triggered, 1);
> >         litest_restore_log_handler(li);
> > -
> > -       litest_axis_set_value(axes, ABS_DISTANCE, 0);
> > -       litest_axis_set_value(axes, ABS_PRESSURE, 31);
> > -       litest_push_event_frame(dev);
> > -       litest_tablet_motion(dev, 70, 70, axes);
> > -       litest_event(dev, EV_KEY, BTN_TOUCH, 1);
> > -       litest_pop_event_frame(dev);
> > -       libinput_dispatch(li);
> > -       litest_drain_events(li);
> > -
> > -       litest_axis_set_value(axes, ABS_PRESSURE, 30);
> > -       litest_tablet_motion(dev, 70, 70, axes);
> > -       libinput_dispatch(li);
> > -
> > -       event = libinput_get_event(li);
> > -       tev = litest_is_tablet_event(event,
> > -                                    LIBINPUT_EVENT_TABLET_TOOL_AXIS);
> > -       pressure = libinput_event_tablet_tool_get_pressure(tev);
> > -       ck_assert_double_gt(pressure, 0.0);
> > -
> > -       libinput_event_destroy(event);
> >  }
> >  END_TEST
> >
> > --
> > 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