Merge lp:~andreas-pokorny/mir/alternative-fix-1522673 into lp:mir

[Andreas Pokorny]

Adjusts how the scroll axis are interpreted and changes the positive direction of scroll values

Instead of ticks (that do not map well for touchpad based scrolling) scroll units of libinput are used as reference. Hence button events in X11 platform are scaled now too. Evdev platform now uses the same positive scroll direction as android-input: 'up'-'right'.

This is an alternative fix, described here as last option
https://code.launchpad.net/~vanvugt/mir/fix-1522673/+merge/279551/comments/708020

So with libinput, instead of using device dependent ticks, we are using scroll units that may be affected by device specific settings. Thus have a better chance to be normalized. This is the reason why the values differ compared to andoird-input. A change was also made in in qtmir to no longer scale the values reported by mir.

[Alan Griffiths (alan-griffiths)]

Changing the meaning of mir_pointer_event_axis_value() is surely an API break?

Do we really know the effect on all downstreams?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:3177
http://jenkins.qa.ubuntu.com/job/mir-ci/5777/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5208
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4114
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5156
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/103/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/106
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/106/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/106
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/106/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5156
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5156/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7686
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25822
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/101/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/5777/rebuild

review: Approve (continuous-integration)

[Andreas Pokorny (andreas-pokorny)]

Agreed going through the different downstreams. This is probably not the right way to do. We should instead come up with a separate axis.

[Andreas Pokorny (andreas-pokorny)]

meh .. i will resubmit this mp

=== modified file 'src/platforms/evdev/libinput_device.cpp'
--- src/platforms/evdev/libinput_device.cpp	2015-11-30 16:34:46 +0000
+++ src/platforms/evdev/libinput_device.cpp	2015-12-07 01:18:22 +0000
@@ -202,14 +202,34 @@
     auto const action = mir_pointer_action_motion;
     auto const relative_x_value = 0.0f;
     auto const relative_y_value = 0.0f;
-    auto const hscroll_value = libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL)
-        ? horizontal_scroll_scale * libinput_event_pointer_get_axis_value(pointer,
-                                                                          LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL)
-        : 0.0f;
-    auto const vscroll_value = libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)
-        ? vertical_scroll_scale * libinput_event_pointer_get_axis_value(pointer,
-                                                                        LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL)
-        : 0.0f;
+
+    auto hscroll_value = 0.0f;
+    auto vscroll_value = 0.0f;
+    if (libinput_event_pointer_get_axis_source(pointer) == LIBINPUT_POINTER_AXIS_SOURCE_WHEEL)
+    {
+        if (libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
+            hscroll_value = horizontal_scroll_scale * libinput_event_pointer_get_axis_value_discrete(
+                                                          pointer, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL);
+        if (libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
+            vscroll_value = -vertical_scroll_scale * libinput_event_pointer_get_axis_value_discrete(
+                                                        pointer, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL);
+    }
+    else
+    {
+        // by default libinput assumes that wheel ticks represent a rotation of 15 degrees. This relation
+        // is used to map wheel rotations to 'scroll units'. To map the immediate scroll units received
+        // from gesture based scrolling we invert that transformation here.
+        auto const scroll_units_to_ticks = 15.0f;
+        if (libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
+            hscroll_value = horizontal_scroll_scale *
+                            libinput_event_pointer_get_axis_value(pointer, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL) /
+                            scroll_units_to_ticks;
+
+        if (libinput_event_pointer_has_axis(pointer, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
+            vscroll_value = -vertical_scroll_scale *
+                            libinput_event_pointer_get_axis_value(pointer, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL) /
+                            scroll_units_to_ticks;
+    }
 
     report->received_event_from_kernel(time.count(), EV_REL, 0, 0);
     return builder->pointer_event(time, action, button_state, hscroll_value, vscroll_value, relative_x_value,

=== modified file 'tests/include/mir/test/doubles/mock_libinput.h'
--- tests/include/mir/test/doubles/mock_libinput.h	2015-10-05 02:18:18 +0000
+++ tests/include/mir/test/doubles/mock_libinput.h	2015-12-07 01:18:22 +0000
@@ -71,6 +71,8 @@
     MOCK_METHOD1(libinput_event_pointer_get_seat_button_count, uint32_t(libinput_event_pointer*));
     MOCK_METHOD1(libinput_event_pointer_get_axis, libinput_pointer_axis(libinput_event_pointer*));
     MOCK_METHOD2(libinput_event_pointer_get_axis_value, double(libinput_event_pointer*, libinput_pointer_axis));
+    MOCK_METHOD1(libinput_event_pointer_get_axis_source, libinput_pointer_axis_source(libinput_event_pointer*));
+    MOCK_METHOD2(libinput_event_pointer_get_axis_value_discrete, double(libinput_event_pointer*, libinput_pointer_axis));
     MOCK_METHOD2(libinput_event_pointer_has_axis,int(libinput_event_pointer *,libinput_pointer_axis));
 
     MOCK_METHOD1(libinput_event_touch_get_time, uint32_t(libinput_event_touch*));

=== modified file 'tests/mir_test_doubles/mock_libinput.cpp'
--- tests/mir_test_doubles/mock_libinput.cpp	2015-10-05 02:18:18 +0000
+++ tests/mir_test_doubles/mock_libinput.cpp	2015-12-07 01:18:22 +0000
@@ -197,11 +197,21 @@
     return global_libinput->libinput_event_pointer_get_axis(event);
 }
 
+libinput_pointer_axis_source libinput_event_pointer_get_axis_source(libinput_event_pointer* event)
+{
+    return global_libinput->libinput_event_pointer_get_axis_source(event);
+}
+
 double libinput_event_pointer_get_axis_value(libinput_event_pointer* event, libinput_pointer_axis axis)
 {
     return global_libinput->libinput_event_pointer_get_axis_value(event, axis);
 }
 
+double libinput_event_pointer_get_axis_value_discrete(libinput_event_pointer* event, libinput_pointer_axis axis)
+{
+    return global_libinput->libinput_event_pointer_get_axis_value_discrete(event, axis);
+}
+
 int libinput_event_pointer_has_axis(libinput_event_pointer* event, libinput_pointer_axis axis)
 {
     return global_libinput->libinput_event_pointer_has_axis(event, axis);

=== modified file 'tests/unit-tests/input/evdev/test_libinput_device.cpp'
--- tests/unit-tests/input/evdev/test_libinput_device.cpp	2015-11-30 16:34:46 +0000
+++ tests/unit-tests/input/evdev/test_libinput_device.cpp	2015-12-07 01:18:22 +0000
@@ -338,6 +338,30 @@
             .WillByDefault(Return(pointer_event));
         ON_CALL(mock_libinput, libinput_event_pointer_get_time_usec(pointer_event))
             .WillByDefault(Return(event_time));
+        ON_CALL(mock_libinput, libinput_event_pointer_get_axis_source(pointer_event))
+            .WillByDefault(Return(LIBINPUT_POINTER_AXIS_SOURCE_WHEEL));
+        ON_CALL(mock_libinput, libinput_event_pointer_has_axis(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
+            .WillByDefault(Return(horizontal!=0.0));
+        ON_CALL(mock_libinput, libinput_event_pointer_has_axis(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
+            .WillByDefault(Return(vertical!=0.0));
+        ON_CALL(mock_libinput, libinput_event_pointer_get_axis_value_discrete(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
+            .WillByDefault(Return(vertical));
+        ON_CALL(mock_libinput, libinput_event_pointer_get_axis_value_discrete(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
+            .WillByDefault(Return(horizontal));
+    }
+
+    void setup_finger_axis_event(libinput_event* event, uint64_t event_time, double horizontal, double vertical)
+    {
+        auto pointer_event = reinterpret_cast<libinput_event_pointer*>(event);
+
+        ON_CALL(mock_libinput, libinput_event_get_type(event))
+            .WillByDefault(Return(LIBINPUT_EVENT_POINTER_AXIS));
+        ON_CALL(mock_libinput, libinput_event_get_pointer_event(event))
+            .WillByDefault(Return(pointer_event));
+        ON_CALL(mock_libinput, libinput_event_pointer_get_time_usec(pointer_event))
+            .WillByDefault(Return(event_time));
+        ON_CALL(mock_libinput, libinput_event_pointer_get_axis_source(pointer_event))
+            .WillByDefault(Return(LIBINPUT_POINTER_AXIS_SOURCE_FINGER));
         ON_CALL(mock_libinput, libinput_event_pointer_has_axis(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_HORIZONTAL))
             .WillByDefault(Return(horizontal!=0.0));
         ON_CALL(mock_libinput, libinput_event_pointer_has_axis(pointer_event, LIBINPUT_POINTER_AXIS_SCROLL_VERTICAL))
@@ -616,8 +640,8 @@
     InSequence seq;
     // expect two scroll events..
     EXPECT_CALL(mock_builder,
-                pointer_event(time_stamp_1, mir_pointer_action_motion, 0, 0.0f, 20.0f, 0.0f, 0.0f));
-    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_vscroll, 20.0f)));
+                pointer_event(time_stamp_1, mir_pointer_action_motion, 0, 0.0f, -20.0f, 0.0f, 0.0f));
+    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_vscroll, -20.0f)));
     EXPECT_CALL(mock_builder,
                 pointer_event(time_stamp_2, mir_pointer_action_motion, 0, 5.0f, 0.0f, 0.0f, 0.0f));
     EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_hscroll, 5.0f)));
@@ -807,7 +831,7 @@
     setup_axis_event(fake_event_2, event_time_2, -2.0, 0.0);
 
     // expect two scroll events..
-    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_vscroll, -3.0f)));
+    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_vscroll, 3.0f)));
     EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_hscroll, -10.0f)));
 
     setup_pointer_configuration(mouse.device(), 1, mir_pointer_handedness_right);
@@ -829,6 +853,26 @@
     EXPECT_THAT(val.is_set(), Eq(false));
 }
 
+TEST_F(LibInputDeviceOnTouchpad, process_event_handles_scroll)
+{
+    setup_finger_axis_event(fake_event_1, event_time_1, 0.0, 150.0);
+    setup_finger_axis_event(fake_event_2, event_time_2, 15.0, 0.0);
+
+    InSequence seq;
+    // expect two scroll events..
+    EXPECT_CALL(mock_builder,
+                pointer_event(time_stamp_1, mir_pointer_action_motion, 0, 0.0f, -10.0f, 0.0f, 0.0f));
+    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_vscroll, -10.0f)));
+    EXPECT_CALL(mock_builder,
+                pointer_event(time_stamp_2, mir_pointer_action_motion, 0, 1.0f, 0.0f, 0.0f, 0.0f));
+    EXPECT_CALL(mock_sink, handle_input(mt::PointerAxisChange(mir_pointer_axis_hscroll, 1.0f)));
+
+    touchpad.start(&mock_sink, &mock_builder);
+    touchpad.process_event(fake_event_1);
+    touchpad.process_event(fake_event_2);
+
+}
+
 TEST_F(LibInputDeviceOnTouchpad, reads_touchpad_settings_from_libinput)
 {
     setup_touchpad_configuration(fake_device, mir_touchpad_click_mode_finger_count,