tablet: require a minimum pressure before we process pressure events

Tools default to 1% lower threshold (tip up) and 5% upper threshold (tip
down). But our distance vs pressure exclusion would reset the distance
for *any* pressure value, regardless how low that value was and how high
distance was in comparison.

A very low pressure value of less than 1% would then result in a
normalized pressure of 0, so we'd effectively just reset the distance to
zero and do nothing with the pressure. This can cause distance jumps
when the tool arbitrarily sends low pressure values while hovering as
seen in https://github.com/libsdl-org/SDL/pull/5481#issuecomment-1118969064

Commit 61bdc05fb0f84303f97daaba6ae6b49c976dbfbf from Dec 2017
  "tablet: set the tip-up pressure threshold to 1%"
was presumably to address this but no longer (?) works.

Fix this by addressing multiple issues at the same time:
- anything under that 1% threshold is now considered as zero pressure
  and any distance value is kept as-is. Once pressure reaches 1%,
  distance is always zero.
- axis normalization is now from 1% to 100% (previously: upper threshold
  to 100%). So a tip down event should always have ~4% pressure and we
  may get tablet motion events with nonzero pressure before the tip down
  event.
  From memory, this was always intended anyway since a tip event should
  require some significant pressure, maybe too high compared to e.g.
  pressure-sensitive painting
- where a tablet has an offset, add the same 1%/5% thresholds, on top of
  that offset. And keep adjusting those thresholds as we change the
  offset. Assuming that the offset is the absolute minimum a worn-out
  pen can reach, this gives us the same behaviour as a new pen. The
  calculation here uses a simple approach so the actual range is
  slightly larger than 5% but it'll do.

  Previously, the lower threshold for an offset pen was the axis minimum
  but that can never be reached. So there was probably an undiscovered
  bug in there.

And fix a bunch of comments that were either wrong, confusing or
incomplete, e.g. the pressure thresholds were already in device
coordinates.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

diff --git a/src/evdev-tablet.c b/src/evdev-tablet.c
index 21963cc..62c23b8 100644 (file)
--- a/src/evdev-tablet.c
+++ b/src/evdev-tablet.c
@@ -353,30 +353,20 @@ static inline double
 normalize_pressure(const struct input_absinfo *absinfo,
                   struct libinput_tablet_tool *tool)
 {
-       int offset;
-       double range;
-       double value;
-
        /**
-        * If the tool has a pressure offset, we use that as the lower bound
-        * for the scaling range. If not, we use the upper threshold as the
-        * lower bound, so once we get past that minimum physical pressure
-        * we have logical 0 pressure.
+        * Note: the upper threshold takes the offset into account so that
+        *            |- 4% -|
+        * min |------X------X-------------------------| max
+        *            |      |
+        *            |      + upper threshold / tip trigger
+        *            +- offset and lower threshold
         *
-        * This means that there is a small range (lower-upper) where
-        * different physical pressure (default: 1-5%) result in the same
-        * logical pressure. This is, hopefully, not noticeable.
-        *
-        * Note that that lower-upper range gives us a negative pressure, so
-        * we have to clip to 0 for those.
+        * The axis is scaled into the range [lower, max] so that the lower
+        * threshold is 0 pressure.
         */
-
-       if (tool->pressure.has_offset)
-               offset = tool->pressure.offset;
-       else
-               offset = tool->pressure.threshold.upper;
-       range = absinfo->maximum - offset;
-       value = (absinfo->value - offset) / range;
+       int base = tool->pressure.threshold.lower;
+       double range = absinfo->maximum - base;
+       double value = (absinfo->value - base) / range;
 
        return max(0.0, value);
 }
@@ -1255,7 +1245,10 @@ sanitize_pressure_distance(struct tablet_dispatch *tablet,
        if (!pressure_changed && !distance_changed)
                return;
 
-       tool_in_contact = (pressure->value > tool->pressure.offset);
+       /* Note: this is an arbitrary "in contact" decision rather than "tip
+        * down". We use the lower threshold as minimum pressure value,
+        * anything less than that gets filtered away */
+       tool_in_contact = (pressure->value > tool->pressure.threshold.lower);
 
        /* Keep distance and pressure mutually exclusive */
        if (distance &&
@@ -1327,7 +1320,7 @@ detect_pressure_offset(struct tablet_dispatch *tablet,
         */
        if (tool->pressure.has_offset) {
                if (offset < tool->pressure.offset)
-                       tool->pressure.offset = offset;
+                       goto set_offset;
                return;
        }
 
@@ -1359,9 +1352,17 @@ detect_pressure_offset(struct tablet_dispatch *tablet,
                 tablet_tool_type_to_string(tool->type),
                 tool->serial,
                 HTTP_DOC_LINK);
+set_offset:
        tool->pressure.offset = offset;
        tool->pressure.has_offset = true;
-       tool->pressure.threshold.lower = pressure->minimum;
+
+       /* Adjust the tresholds accordingly - we use the same gap (4% in
+        * device coordinates) between upper and lower as before which isn't
+        * technically correct (our range shrunk) but it's easy to calculate.
+        */
+       int gap = tool->pressure.threshold.upper - tool->pressure.threshold.lower;
+       tool->pressure.threshold.lower = offset;
+       tool->pressure.threshold.upper = offset + gap;
 }
 
 static void
@@ -1392,9 +1393,6 @@ detect_tool_contact(struct tablet_dispatch *tablet,
        }
        pressure = p->value;
 
-       if (tool->pressure.has_offset)
-               pressure -= (tool->pressure.offset - p->minimum);
-
        if (pressure <= tool->pressure.threshold.lower &&
            tablet_has_status(tablet, TABLET_TOOL_IN_CONTACT)) {
                tablet_set_status(tablet, TABLET_TOOL_LEAVING_CONTACT);

diff --git a/src/libinput-private.h b/src/libinput-private.h
index 7abb384..aca4d96 100644 (file)
--- a/src/libinput-private.h
+++ b/src/libinput-private.h
@@ -407,10 +407,8 @@ struct libinput_tablet_tool {
        void *user_data;
 
        struct {
-               /* The pressure threshold assumes a pressure_offset of 0 */
-               struct threshold threshold;
-               /* pressure_offset includes axis->minimum */
-               int offset;
+               struct threshold threshold; /* in device coordinates */
+               int offset; /* in device coordinates */
                bool has_offset;
        } pressure;
 };

diff --git a/test/test-tablet.c b/test/test-tablet.c
index 35f835e..ce1c7b6 100644 (file)
--- a/test/test-tablet.c
+++ b/test/test-tablet.c
@@ -3588,8 +3588,9 @@ START_TEST(tablet_pressure_distance_exclusive)
        litest_tablet_proximity_in(dev, 5, 50, axes);
        litest_drain_events(li);
 
-       /* We have pressure but we're still below the tip threshold */
-       litest_axis_set_value(axes, ABS_PRESSURE, 1);
+       /* We have 0.1% pressure above minimum threshold but we're still below
+        * the tip threshold */
+       litest_axis_set_value(axes, ABS_PRESSURE, 1.1);
        litest_tablet_motion(dev, 70, 70, axes);
        libinput_dispatch(li);
 
@@ -3599,11 +3600,11 @@ 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_eq(pressure, 0.001);
        ck_assert_double_eq(distance, 0.0);
        libinput_event_destroy(event);
 
-       /* We have pressure and we're above the threshold now */
+       /* We have pressure and we're above the tip threshold now */
        litest_axis_set_value(axes, ABS_PRESSURE, 5.5);
        litest_tablet_motion(dev, 70, 70, axes);
        libinput_dispatch(li);
@@ -3820,18 +3821,23 @@ START_TEST(tablet_pressure_offset_set)
        };
        double pressure;
 
-       /* This activates the pressure thresholds */
+       /* This activates the pressure offset */
        litest_tablet_proximity_in(dev, 5, 100, axes);
        litest_drain_events(li);
 
        /* Put the pen down, with a pressure high enough to meet the
-        * threshold */
+        * new offset */
        litest_axis_set_value(axes, ABS_DISTANCE, 0);
-       litest_axis_set_value(axes, ABS_PRESSURE, 25);
+       litest_axis_set_value(axes, ABS_PRESSURE, 26);
 
-       litest_tablet_tip_down(dev, 70, 70, axes);
+       /* Tablet motion above threshold should trigger axis + tip down. Use
+        * the litest motion helper here to avoid false positives caused by
+        * BTN_TOUCH */
+       litest_tablet_motion(dev, 70, 70, axes);
        libinput_dispatch(li);
-       litest_drain_events(li);
+       event = libinput_get_event(li);
+       litest_is_tablet_event(event, LIBINPUT_EVENT_TABLET_TOOL_TIP);
+       libinput_event_destroy(event);
 
        /* Reduce pressure to just a tick over the offset, otherwise we get
         * the tip up event again */
@@ -3879,6 +3885,16 @@ START_TEST(tablet_pressure_offset_set)
        ck_assert_double_ge(pressure, 1.0);
        libinput_event_destroy(event);
 
+       /* Tablet motion at offset should trigger tip up. Use
+        * the litest motion helper here to avoid false positives caused by
+        * BTN_TOUCH */
+       litest_axis_set_value(axes, ABS_PRESSURE, 20);
+       litest_tablet_motion(dev, 71, 71, axes);
+       libinput_dispatch(li);
+       event = libinput_get_event(li);
+       litest_is_tablet_event(event, LIBINPUT_EVENT_TABLET_TOOL_TIP);
+       libinput_event_destroy(event);
+
 }
 END_TEST
 
@@ -3929,8 +3945,10 @@ START_TEST(tablet_pressure_offset_decrease)
 
        pressure = libinput_event_tablet_tool_get_pressure(tev);
 
-       /* offset is 10, value is 15, so that's a around 5% of the
-        * remaining range */
+       /* offset 10 + lower threshold of ~1% of original range,
+        * value 15 is 5% over original range but with the above taken into
+        * account it's closer to 5% into the remaining effective 89% range
+        */
        ck_assert_double_eq_tol(pressure, 0.05, 0.01);
        libinput_event_destroy(event);
 }
@@ -3973,7 +3991,11 @@ START_TEST(tablet_pressure_offset_increase)
        tev = litest_is_tablet_event(event,
                                     LIBINPUT_EVENT_TABLET_TOOL_AXIS);
        pressure = libinput_event_tablet_tool_get_pressure(tev);
-       /* offset 20, value 30 leaves us with 12% of the remaining 90 range */
+
+       /* offset 20 + lower threshold of 1% of original range,
+        * value 30 is 5% over original range but with the above taken into
+        * account it's closer to 12% into the remaining effective 79% range
+        */
        ck_assert_double_eq_tol(pressure, 0.12, 0.01);
        libinput_event_destroy(event);
 
@@ -4015,7 +4037,8 @@ START_TEST(tablet_pressure_min_max)
        libinput_dispatch(li);
 
        litest_axis_set_value(axes, ABS_DISTANCE, 0);
-       litest_axis_set_value(axes, ABS_PRESSURE, 1);
+       /* Default pressure threshold is 1% of range */
+       litest_axis_set_value(axes, ABS_PRESSURE, 1.1);
        litest_tablet_motion(dev, 5, 50, axes);
        libinput_dispatch(li);
        event = libinput_get_event(li);