Mir

Merge lp:~mir-team/mir/fix-initial-key-repeat-timeout into lp:mir

  1. fix-initial-key-repeat-timeout
  2. Merge into development-branch
Proposed by Robert Carr
Status: Merged
Approved by: Chris Halse Rogers
Approved revision: no longer in the source branch.
Merged at revision: 2732
Proposed branch: lp:~mir-team/mir/fix-initial-key-repeat-timeout
Merge into: lp:mir
Diff against target: 105 lines (+16/-10)
4 files modified
src/server/input/default_configuration.cpp (+3/-2)
src/server/input/key_repeat_dispatcher.cpp (+5/-3)
src/server/input/key_repeat_dispatcher.h (+3/-1)
tests/unit-tests/input/test_key_repeat_dispatcher.cpp (+5/-4)
To merge this branch: bzr merge lp:~mir-team/mir/fix-initial-key-repeat-timeout
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Chris Halse Rogers Approve
Alberto Aguirre (community) Approve
Review via email: mp+264205@code.launchpad.net

Commit message

A single timeout isn't enough for reasonable key repeat behavior. We need a longer delay before sending the FIRST repeat or keys will start repeating too quickly. I observed this issue in autopilot.

This branch implements such a timeout.

This also bumps the key repeat delay (the time inbetween repeats once they are being sent) to 50ms, or the same value as in the old InputDispatcher

Description of the change

A single timeout isn't enough for reasonable key repeat behavior. We need a longer delay before sending the FIRST repeat or keys will start repeating too quickly. I observed this issue in autopilot.

This branch implements such a timeout.

This also bumps the key repeat delay (the time inbetween repeats once they are being sent) to 50ms, or the same value as in the old InputDispatcher

To post a comment you must log in.
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

OK.

review: Approve
Revision history for this message
Chris Halse Rogers (raof) wrote :

Yup, seems good.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:2732
http://jenkins.qa.ubuntu.com/job/mir-ci/4286/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3114
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/636
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3062
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/435
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/435/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3062
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3062/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5867
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21737

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/input/default_configuration.cpp'
2--- src/server/input/default_configuration.cpp 2015-06-26 08:00:59 +0000
3+++ src/server/input/default_configuration.cpp 2015-07-08 21:38:15 +0000
4@@ -163,14 +163,15 @@
5 return input_dispatcher(
6 [this]()
7 {
8- std::chrono::milliseconds const key_repeat_timeout{20};
9+ std::chrono::milliseconds const key_repeat_timeout{500};
10+ std::chrono::milliseconds const key_repeat_delay{50};
11
12 auto const options = the_options();
13 auto enable_repeat = options->get<bool>(options::enable_key_repeat_opt);
14
15 return std::make_shared<mi::KeyRepeatDispatcher>(
16 the_event_filter_chain_dispatcher(), the_main_loop(), enable_repeat,
17- key_repeat_timeout);
18+ key_repeat_timeout, key_repeat_delay);
19 });
20 }
21
22
23=== modified file 'src/server/input/key_repeat_dispatcher.cpp'
24--- src/server/input/key_repeat_dispatcher.cpp 2015-06-12 06:20:34 +0000
25+++ src/server/input/key_repeat_dispatcher.cpp 2015-07-08 21:38:15 +0000
26@@ -33,11 +33,13 @@
27 std::shared_ptr<mi::InputDispatcher> const& next_dispatcher,
28 std::shared_ptr<mir::time::AlarmFactory> const& factory,
29 bool repeat_enabled,
30- std::chrono::milliseconds repeat_timeout)
31+ std::chrono::milliseconds repeat_timeout,
32+ std::chrono::milliseconds repeat_delay)
33 : next_dispatcher(next_dispatcher),
34 alarm_factory(factory),
35 repeat_enabled(repeat_enabled),
36- repeat_timeout(repeat_timeout)
37+ repeat_timeout(repeat_timeout),
38+ repeat_delay(repeat_delay)
39 {
40 }
41
42@@ -116,7 +118,7 @@
43 ev.key.event_time = std::chrono::high_resolution_clock::now().time_since_epoch();
44 next_dispatcher->dispatch(ev);
45
46- capture_alarm->reschedule_in(repeat_timeout);
47+ capture_alarm->reschedule_in(repeat_delay);
48 });
49 alarm->reschedule_in(repeat_timeout);
50 device_state.repeat_alarms_by_scancode[scan_code] = {alarm};
51
52=== modified file 'src/server/input/key_repeat_dispatcher.h'
53--- src/server/input/key_repeat_dispatcher.h 2015-06-12 06:20:34 +0000
54+++ src/server/input/key_repeat_dispatcher.h 2015-07-08 21:38:15 +0000
55@@ -42,7 +42,8 @@
56 KeyRepeatDispatcher(std::shared_ptr<InputDispatcher> const& next_dispatcher,
57 std::shared_ptr<time::AlarmFactory> const& factory,
58 bool repeat_enabled,
59- std::chrono::milliseconds repeat_timeout);
60+ std::chrono::milliseconds repeat_timeout, /* timeout before sending first repeat */
61+ std::chrono::milliseconds repeat_delay /* delay between repeated keys */);
62
63 // InputDispatcher
64 bool dispatch(MirEvent const& event) override;
65@@ -56,6 +57,7 @@
66 std::shared_ptr<time::AlarmFactory> const alarm_factory;
67 bool const repeat_enabled;
68 std::chrono::milliseconds repeat_timeout;
69+ std::chrono::milliseconds repeat_delay;
70
71 struct KeyboardState
72 {
73
74=== modified file 'tests/unit-tests/input/test_key_repeat_dispatcher.cpp'
75--- tests/unit-tests/input/test_key_repeat_dispatcher.cpp 2015-06-26 08:00:59 +0000
76+++ tests/unit-tests/input/test_key_repeat_dispatcher.cpp 2015-07-08 21:38:15 +0000
77@@ -61,12 +61,13 @@
78 struct KeyRepeatDispatcher : public testing::Test
79 {
80 KeyRepeatDispatcher()
81- : dispatcher(mock_next_dispatcher, mock_alarm_factory, true, repeat_time)
82+ : dispatcher(mock_next_dispatcher, mock_alarm_factory, true, repeat_time, repeat_delay)
83 {
84 }
85 std::shared_ptr<mtd::MockInputDispatcher> mock_next_dispatcher = std::make_shared<mtd::MockInputDispatcher>();
86 std::shared_ptr<MockAlarmFactory> mock_alarm_factory = std::make_shared<MockAlarmFactory>();
87- std::chrono::milliseconds const repeat_time{1};
88+ std::chrono::milliseconds const repeat_time{2};
89+ std::chrono::milliseconds const repeat_delay{1};
90 mi::KeyRepeatDispatcher dispatcher;
91 };
92 }
93@@ -106,10 +107,10 @@
94 EXPECT_CALL(*mock_alarm_factory, create_alarm_adapter(_)).Times(1).
95 WillOnce(DoAll(SaveArg<0>(&alarm_function), Return(mock_alarm)));
96 // Once for initial down and again when invoked
97- EXPECT_CALL(*mock_alarm, reschedule_in(_)).Times(1).WillOnce(Return(true));
98+ EXPECT_CALL(*mock_alarm, reschedule_in(repeat_time)).Times(1).WillOnce(Return(true));
99 EXPECT_CALL(*mock_next_dispatcher, dispatch(mt::KeyDownEvent())).Times(1);
100 EXPECT_CALL(*mock_next_dispatcher, dispatch(mt::KeyRepeatEvent())).Times(1);
101- EXPECT_CALL(*mock_alarm, reschedule_in(_)).Times(1).WillOnce(Return(true));
102+ EXPECT_CALL(*mock_alarm, reschedule_in(repeat_delay)).Times(1).WillOnce(Return(true));
103 EXPECT_CALL(*mock_next_dispatcher, dispatch(mt::KeyUpEvent())).Times(1);
104
105 // Schedule the repeat

Subscribers

People subscribed via source and target branches