Mir

Merge lp:~mir-team/mir/fix-1528109 into lp:mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 3464
Proposed branch: lp:~mir-team/mir/fix-1528109
Merge into: lp:mir
Diff against target: 153 lines (+58/-18)
6 files modified
include/test/mir/test/event_matchers.h (+8/-3)
src/platforms/evdev/libinput_device.cpp (+4/-5)
src/server/input/seat_input_device_tracker.cpp (+11/-9)
src/server/input/seat_input_device_tracker.h (+4/-1)
tests/unit-tests/input/evdev/test_libinput_device.cpp (+14/-0)
tests/unit-tests/input/test_seat_input_device_tracker.cpp (+17/-0)
To merge this branch: bzr merge lp:~mir-team/mir/fix-1528109
Reviewer Review Type Date Requested Status
Andreas Pokorny (community) Approve
Alan Griffiths Approve
Mir CI Bot continuous-integration Approve
Review via email: mp+291984@code.launchpad.net

Commit message

Fix unresponsive cursor to slow mouse movement (LP: #1528109)

Since we started using libinput and its acceleration curve, that actually
reduces the amount of motion we receive for slow mouse movements. A device
increment of 1 actually results in us receiving a dx or dy from libinput
of less than 1.0. So this fix ensures that such small movements no longer
get truncated to zero.

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:3468
https://mir-jenkins.ubuntu.com/job/mir-ci/846/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/857/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/894
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/885
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/885
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/867/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/867
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/867/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/867/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/867/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/867
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/867/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/846/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

+ auto confined{old};

See https://herbsutter.com/2013/06/05/gotw-92-auto-variables-part-1/

(part 4 applies)

review: Needs Fixing
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3469
https://mir-jenkins.ubuntu.com/job/mir-ci/848/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/862
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/899
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/890
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/890
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/872
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/872/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/872
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/872/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/872
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/872/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/872
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/872/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/872
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/872/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/848/rebuild

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> + auto confined{old};
>
> See https://herbsutter.com/2013/06/05/gotw-92-auto-variables-part-1/
>
> (part 4 applies)

Fixed

review: Approve
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote :

yes .. lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/test/mir/test/event_matchers.h'
--- include/test/mir/test/event_matchers.h 2016-03-23 06:39:56 +0000
+++ include/test/mir/test/event_matchers.h 2016-04-15 15:11:30 +0000
@@ -383,16 +383,21 @@
383 return false;383 return false;
384}384}
385385
386MATCHER_P2(PointerEventWithDiff, dx, dy, "")386MATCHER_P2(PointerEventWithDiff, expect_dx, expect_dy, "")
387{387{
388 auto pev = maybe_pointer_event(to_address(arg));388 auto pev = maybe_pointer_event(to_address(arg));
389 if (pev == nullptr)389 if (pev == nullptr)
390 return false;390 return false;
391 if (mir_pointer_event_action(pev) != mir_pointer_action_motion)391 if (mir_pointer_event_action(pev) != mir_pointer_action_motion)
392 return false;392 return false;
393 if (mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_x) != dx)393 auto const error = 0.00001f;
394 auto const actual_dx = mir_pointer_event_axis_value(pev,
395 mir_pointer_axis_relative_x);
396 if (std::abs(expect_dx - actual_dx) > error)
394 return false;397 return false;
395 if (mir_pointer_event_axis_value(pev, mir_pointer_axis_relative_y) != dy)398 auto const actual_dy = mir_pointer_event_axis_value(pev,
399 mir_pointer_axis_relative_y);
400 if (std::abs(expect_dy - actual_dy) > error)
396 return false;401 return false;
397 return true;402 return true;
398}403}
399404
=== modified file 'src/platforms/evdev/libinput_device.cpp'
--- src/platforms/evdev/libinput_device.cpp 2016-03-23 06:39:56 +0000
+++ src/platforms/evdev/libinput_device.cpp 2016-04-15 15:11:30 +0000
@@ -171,11 +171,10 @@
171171
172 report->received_event_from_kernel(time.count(), EV_REL, 0, 0);172 report->received_event_from_kernel(time.count(), EV_REL, 0, 0);
173173
174 mir::geometry::Displacement const movement{libinput_event_pointer_get_dx(pointer),174 return builder->pointer_event(time, action, button_state,
175 libinput_event_pointer_get_dy(pointer)};175 hscroll_value, vscroll_value,
176176 libinput_event_pointer_get_dx(pointer),
177 return builder->pointer_event(time, action, button_state, hscroll_value, vscroll_value, movement.dx.as_float(),177 libinput_event_pointer_get_dy(pointer));
178 movement.dy.as_float());
179}178}
180179
181mir::EventUPtr mie::LibInputDevice::convert_absolute_motion_event(libinput_event_pointer* pointer)180mir::EventUPtr mie::LibInputDevice::convert_absolute_motion_event(libinput_event_pointer* pointer)
182181
=== modified file 'src/server/input/seat_input_device_tracker.cpp'
--- src/server/input/seat_input_device_tracker.cpp 2016-03-23 06:39:56 +0000
+++ src/server/input/seat_input_device_tracker.cpp 2016-04-15 15:11:30 +0000
@@ -200,7 +200,7 @@
200200
201mir::geometry::Point mi::SeatInputDeviceTracker::cursor_position() const201mir::geometry::Point mi::SeatInputDeviceTracker::cursor_position() const
202{202{
203 return cursor_pos;203 return {cursor_x, cursor_y};
204}204}
205205
206MirPointerButtons mi::SeatInputDeviceTracker::button_state() const206MirPointerButtons mi::SeatInputDeviceTracker::button_state() const
@@ -210,12 +210,14 @@
210210
211void mi::SeatInputDeviceTracker::update_cursor(MirPointerEvent const* event)211void mi::SeatInputDeviceTracker::update_cursor(MirPointerEvent const* event)
212{212{
213 mir::geometry::Displacement movement{213 cursor_x += mir_pointer_event_axis_value(event, mir_pointer_axis_relative_x);
214 mir_pointer_event_axis_value(event, mir_pointer_axis_relative_x),214 cursor_y += mir_pointer_event_axis_value(event, mir_pointer_axis_relative_y);
215 mir_pointer_event_axis_value(event, mir_pointer_axis_relative_y),215
216 };216 mir::geometry::Point const old{cursor_x, cursor_y};
217 cursor_pos = cursor_pos + movement;217 auto confined = old;
218 input_region->confine(cursor_pos);218 input_region->confine(confined);
219219 if (confined.x != old.x) cursor_x = confined.x.as_int();
220 cursor_listener->cursor_moved_to(cursor_pos.x.as_float(), cursor_pos.y.as_float());220 if (confined.y != old.y) cursor_y = confined.y.as_int();
221
222 cursor_listener->cursor_moved_to(cursor_x, cursor_y);
221}223}
222224
=== modified file 'src/server/input/seat_input_device_tracker.h'
--- src/server/input/seat_input_device_tracker.h 2016-03-23 06:39:56 +0000
+++ src/server/input/seat_input_device_tracker.h 2016-04-15 15:11:30 +0000
@@ -80,7 +80,10 @@
80 std::vector<TouchVisualizer::Spot> spots;80 std::vector<TouchVisualizer::Spot> spots;
81 };81 };
8282
83 mir::geometry::Point cursor_pos;83 // Libinput's acceleration curve means the cursor moves by non-integer
84 // increments, and often less than 1.0, so float is required...
85 float cursor_x = 0.0f, cursor_y = 0.0f;
86
84 MirInputEventModifiers modifier;87 MirInputEventModifiers modifier;
85 MirPointerButtons buttons;88 MirPointerButtons buttons;
86 std::unordered_map<MirInputDeviceId, DeviceData> device_data;89 std::unordered_map<MirInputDeviceId, DeviceData> device_data;
8790
=== modified file 'tests/unit-tests/input/evdev/test_libinput_device.cpp'
--- tests/unit-tests/input/evdev/test_libinput_device.cpp 2016-03-23 06:39:56 +0000
+++ tests/unit-tests/input/evdev/test_libinput_device.cpp 2016-04-15 15:11:30 +0000
@@ -407,6 +407,20 @@
407 process_events(mouse);407 process_events(mouse);
408}408}
409409
410TEST_F(LibInputDeviceOnMouse, notices_slow_relative_movements)
411{ // Regression test for LP: #1528109
412 float x1 = 0.5f, x2 = 0.6f;
413 float y1 = 1.7f, y2 = 1.8f;
414
415 EXPECT_CALL(mock_sink, handle_input(mt::PointerEventWithDiff(x1,y1)));
416 EXPECT_CALL(mock_sink, handle_input(mt::PointerEventWithDiff(x2,y2)));
417
418 mouse.start(&mock_sink, &mock_builder);
419 env.mock_libinput.setup_pointer_event(fake_device, event_time_1, x1, y1);
420 env.mock_libinput.setup_pointer_event(fake_device, event_time_2, x2, y2);
421 process_events(mouse);
422}
423
410TEST_F(LibInputDeviceOnMouse, process_event_handles_press_and_release)424TEST_F(LibInputDeviceOnMouse, process_event_handles_press_and_release)
411{425{
412 float const x = 0;426 float const x = 0;
413427
=== modified file 'tests/unit-tests/input/test_seat_input_device_tracker.cpp'
--- tests/unit-tests/input/test_seat_input_device_tracker.cpp 2016-02-05 10:19:39 +0000
+++ tests/unit-tests/input/test_seat_input_device_tracker.cpp 2016-04-15 15:11:30 +0000
@@ -149,6 +149,23 @@
149 move_x, move_y));149 move_x, move_y));
150}150}
151151
152TEST_F(SeatInputDeviceTracker, slow_pointer_movement_updates_cursor)
153{ // Regression test for LP: #1528109
154 float const step = 0.25f;
155 float const target = 3.0f;
156
157 for (float x = step; x <= target; x += step)
158 EXPECT_CALL(mock_cursor_listener,
159 cursor_moved_to(FloatEq(x), FloatEq(x))).Times(1);
160
161 tracker.add_device(some_device);
162
163 for (float x = step; x <= target; x += step)
164 tracker.dispatch(*some_device_builder.pointer_event(
165 arbitrary_timestamp, mir_pointer_action_motion, 0, 0.0f, 0.0f,
166 step, step));
167}
168
152TEST_F(SeatInputDeviceTracker, key_strokes_of_modifier_key_update_modifier)169TEST_F(SeatInputDeviceTracker, key_strokes_of_modifier_key_update_modifier)
153{170{
154 const MirInputEventModifiers shift_left = mir_input_event_modifier_shift_left | mir_input_event_modifier_shift;171 const MirInputEventModifiers shift_left = mir_input_event_modifier_shift_left | mir_input_event_modifier_shift;

Subscribers

People subscribed via source and target branches