evdev: use distance triggers to start scrolling

The previous code used delta/event as scroll trigger which roughly translates
to speed, but depends on the sampling rate of the device.

For slow two-finger motion, a user may move the height of the touchpad without
ever triggering scrolling. Change the _initial_ trigger to a cumulative
trigger, i.e. once the user moved past the threshold distance, scrolling
starts regardless of the speed.

Once scrolling is engaged, the original trigger of threshold/event is
required to engange the second scroll direction.

Note that except for really slow movements, it's very easy to engage both
scroll directions on a touchpad. This is intentional, libinput does not have
enough semantic knowledge to know if horizontal scrolling is needed. So we
provide some direction locking but not much, it's up to the
client/toolkit/widget to decide if both scroll directions should be handled.
Add a comment to clarify that in the public doc.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

diff --git a/src/evdev.c b/src/evdev.c
index 89a39ca..94daea8 100644 (file)
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -1517,14 +1517,47 @@ evdev_post_scroll(struct evdev_device *device,
                  double dx,
                  double dy)
 {
-       if (fabs(dy) >= device->scroll.threshold)
-               evdev_start_scrolling(device,
+       double trigger_horiz, trigger_vert;
+
+       if (!evdev_is_scrolling(device,
+                               LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
+               device->scroll.buildup_vertical += dy;
+       if (!evdev_is_scrolling(device,
+                               LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
+               device->scroll.buildup_horizontal += dx;
+
+       trigger_vert = device->scroll.buildup_vertical;
+       trigger_horiz = device->scroll.buildup_horizontal;
+
+       /* If we're not scrolling yet, use a distance trigger: moving
+          past a certain distance starts scrolling */
+       if (!evdev_is_scrolling(device,
+                               LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL) &&
+           !evdev_is_scrolling(device,
+                               LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)) {
+               if (fabs(trigger_vert) >= device->scroll.threshold)
+                       evdev_start_scrolling(device,
+                                             LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
+               if (fabs(trigger_horiz) >= device->scroll.threshold)
+                       evdev_start_scrolling(device,
+                                             LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
+       /* We're already scrolling in one direction. Require some
+          trigger speed to start scrolling in the other direction */
+       } else if (!evdev_is_scrolling(device,
+                              LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)) {
+               if (fabs(dy) >= device->scroll.threshold)
+                       evdev_start_scrolling(device,
                                      LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
-
-       if (fabs(dx) >= device->scroll.threshold)
-               evdev_start_scrolling(device,
+       } else if (!evdev_is_scrolling(device,
+                               LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL)) {
+               if (fabs(dx) >= device->scroll.threshold)
+                       evdev_start_scrolling(device,
                                      LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
+       }
 
+       /* We use the trigger to enable, but the delta from this event for
+        * the actual scroll movement. Otherwise we get a jump once
+        * scrolling engages */
        if (dy != 0.0 &&
            evdev_is_scrolling(device,
                               LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)) {
@@ -1559,6 +1592,8 @@ evdev_stop_scroll(struct evdev_device *device, uint64_t time)
                                    LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL,
                                    0);
 
+       device->scroll.buildup_horizontal = 0;
+       device->scroll.buildup_vertical = 0;
        device->scroll.direction = 0;
 }
 

diff --git a/src/evdev.h b/src/evdev.h
index eefbb79..279a3ae 100644 (file)
--- a/src/evdev.h
+++ b/src/evdev.h
@@ -104,6 +104,8 @@ struct evdev_device {
                bool middle_button_scroll_active;
                double threshold;
                uint32_t direction;
+               double buildup_vertical;
+               double buildup_horizontal;
        } scroll;
 
        enum evdev_event_type pending_event;

diff --git a/src/libinput.h b/src/libinput.h
index 049e3b8..f416afd 100644 (file)
--- a/src/libinput.h
+++ b/src/libinput.h
@@ -195,6 +195,12 @@ enum libinput_button_state {
  * @ingroup device
  *
  * Axes on a device that are not x or y coordinates.
+ *
+ * The two scroll axes @ref LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL and
+ * @ref LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL are engaged separately,
+ * depending on the device. libinput provides some scroll direction locking
+ * but it is up to the caller to determine which axis is needed and
+ * appropriate in the current interaction
  */
 enum libinput_pointer_axis {
        LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL = 0,

diff --git a/test/touchpad.c b/test/touchpad.c
index f9031dd..2b79aaf 100644 (file)
--- a/test/touchpad.c
+++ b/test/touchpad.c
@@ -1455,6 +1455,50 @@ START_TEST(touchpad_2fg_scroll)
 }
 END_TEST
 
+START_TEST(touchpad_2fg_scroll_slow_distance)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+       struct libinput_event *event;
+       struct libinput_event_pointer *ptrev;
+
+       litest_drain_events(li);
+
+       litest_touch_down(dev, 0, 20, 30);
+       litest_touch_down(dev, 1, 40, 30);
+       litest_touch_move_to(dev, 0, 20, 30, 20, 50, 60, 10);
+       litest_touch_move_to(dev, 1, 40, 30, 40, 50, 60, 10);
+       litest_touch_up(dev, 1);
+       litest_touch_up(dev, 0);
+       libinput_dispatch(li);
+
+       event = libinput_get_event(li);
+       ck_assert_notnull(event);
+
+       /* last event is value 0, tested elsewhere */
+       while (libinput_next_event_type(li) != LIBINPUT_EVENT_NONE) {
+               ck_assert_int_eq(libinput_event_get_type(event),
+                                LIBINPUT_EVENT_POINTER_AXIS);
+               ptrev = libinput_event_get_pointer_event(event);
+
+               ck_assert_int_eq(libinput_event_pointer_get_axis(ptrev),
+                                LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
+               ck_assert(libinput_event_pointer_get_axis_value(ptrev) > 0.0);
+
+               /* this is to verify we test the right thing, if the value
+                  is greater than scroll.threshold we triggered the wrong
+                  condition */
+               ck_assert(libinput_event_pointer_get_axis_value(ptrev) < 5.0);
+
+               libinput_event_destroy(event);
+               event = libinput_get_event(li);
+       }
+
+       litest_assert_empty_queue(li);
+       libinput_event_destroy(event);
+}
+END_TEST
+
 START_TEST(touchpad_scroll_natural_defaults)
 {
        struct litest_device *dev = litest_current_device();
@@ -2073,6 +2117,7 @@ int main(int argc, char **argv) {
        litest_add("touchpad:topsoftbuttons", clickpad_topsoftbuttons_move_out_ignore, LITEST_TOPBUTTONPAD, LITEST_ANY);
 
        litest_add("touchpad:scroll", touchpad_2fg_scroll, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
+       litest_add("touchpad:scroll", touchpad_2fg_scroll_slow_distance, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);
        litest_add("touchpad:scroll", touchpad_scroll_natural_defaults, LITEST_TOUCHPAD, LITEST_ANY);
        litest_add("touchpad:scroll", touchpad_scroll_natural_enable_config, LITEST_TOUCHPAD, LITEST_ANY);
        litest_add("touchpad:scroll", touchpad_scroll_natural, LITEST_TOUCHPAD, LITEST_SINGLE_TOUCH);