Merge lp:~vanvugt/mir/remove-input-resampling-standalone into lp:mir
- remove-input-resampling-standalone
- Merge into development-branch
Status: | Merged |
---|---|
Approved by: | Daniel van Vugt |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3949 |
Proposed branch: | lp:~vanvugt/mir/remove-input-resampling-standalone |
Merge into: | lp:mir |
Diff against target: |
522 lines (+48/-274) 7 files modified
examples/fingerpaint.c (+0/-6) examples/target.c (+0/-6) src/client/input/android/android_input_receiver.cpp (+26/-98) src/client/input/android/android_input_receiver.h (+3/-12) tests/acceptance-tests/test_client_input.cpp (+0/-64) tests/acceptance-tests/test_confined_pointer.cpp (+0/-1) tests/unit-tests/client/input/test_android_input_receiver.cpp (+19/-87) |
To merge this branch: | bzr merge lp:~vanvugt/mir/remove-input-resampling-standalone |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mir CI Bot | continuous-integration | Approve | |
Andreas Pokorny (community) | Approve | ||
Alan Griffiths | Abstain | ||
Chris Halse Rogers | Approve | ||
Review via email: mp+314305@code.launchpad.net |
Commit message
Remove input resampling, finally.
Input events now get delivered from the kernel (via AndroidInput which we
can now retire) to clients without any resampling delay. That was [0,16.9]ms or an average of 8.4ms before, but is now around 0.2ms.
This fixes LP: #1570698 and LP: #1576600. Probably LP: #1394369 too.
Description of the change
I had believed until recently that we would need to land client-side vsync
before we could do this. However I have now invented a simpler
prerequisite which allows Mir to drop input resampling immediately:
lp:~vanvugt/unity8/fix-1497105
Mir CI Bot (mir-ci-bot) wrote : | # |
Daniel van Vugt (vanvugt) wrote : | # |
^^^
Bug 1616312
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3911
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Chris Halse Rogers (raof) wrote : | # |
The AndroidInputRec
I'm weakly against the change to dispatch multiple events in a single process_
When client-driven dispatch is available clients are going to expect to get a single event out of a single dispatch call.
**** So: ****
*) Rename or remove AndroidInputRec
*) Don't loop in process_
Optional:
*) Maybe rename process_
*) Test that you get at most one event sent per dispatch() on the AndroidInputRec
Daniel van Vugt (vanvugt) wrote : | # |
I knew you would say that. Unfortunately I did try exactly what you suggested as a first choice and it seemed broken (either was no longer readable or the Dispatchable ignored wakeups). Will need to dig deeper now to find out why...
Chris Halse Rogers (raof) wrote : | # |
Curious. MultiplexingDis
edge) triggering, the socket is SEQPACKET, so there's guaranteed to
only be whole events there, and consume() looks like it should only eat
one...
Andreas Pokorny (andreas-pokorny) wrote : | # |
> Curious. MultiplexingDis
> edge) triggering, the socket is SEQPACKET, so there's guaranteed to
> only be whole events there, and consume() looks like it should only eat
> one...
I think thats what the second parameter "consumeBatches" does .. passing false there should help.
Daniel van Vugt (vanvugt) wrote : | # |
No, consumeBatches should always be true now. We don't batch anything and want clients to receive all events immediately.
It was just our tests were overly simplistic and not able to consume wake notifications when they dispatch(), leaving the fd readable indefinitely. So it was just a problem with the tests. But instead of modifying the tests too much I have reworked the semaphore logic to deal with the simplistic test expectations so that dispatch() can also drain wake()'s.
Chris Halse Rogers (raof) wrote : | # |
On 10 Jan. 2017 18:30, Andreas Pokorny <email address hidden> wrote:
>
> > Curious. MultiplexingDis
> > edge) triggering, the socket is SEQPACKET, so there's guaranteed to
> > only be whole events there, and consume() looks like it should only eat
> > one...
>
> I think thats what the second parameter "consumeBatches" does .. passing false there should help.
Actually, I think it no longer matters what value use for consumeBatches - because we pass in a frame time of -1, I'm pretty sure we never generate any batches.
(Relatedly, I think this means we can delete almost all of AndroidInput)
Andreas Pokorny (andreas-pokorny) wrote : | # |
iirc conumeBatches = true means that we read stuff from the socket without handing it out to the caller.. So we need to wake() the consuming thread again.
I am a bit confused now. Is the intention to remove batching and resampling, or just resampling?
Last time I wallowed in that code resampling only affected touch motion and not pointer movement, so this will only affect that X11-Mouse behavior if we also drop batching.
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3915
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Andreas Pokorny (andreas-pokorny) wrote : | # |
Ah this MP also removes the timer fd! So it will affect the frequency of mouse events.
Then using false instead of true for consumeBatches should actually not drain the input channel fd. I assume it would be simpler to just serialize the MirEvent into a buffer in BasicSurface:
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3916
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
CI failures are bug 1655293 and bug 1639941. Is this branch aggravating those?...
Daniel van Vugt (vanvugt) wrote : | # |
Also bug 1646558.
Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3917
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3920
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
^^^
Bug 1646375
Chris Halse Rogers (raof) wrote : | # |
LGTM, thanks.
I continue to be surprised that the FD doesn't remain readable if there's still an event left after consume(), and we'll be woken up once more than is strictly necessary, but this is fine.
Chris Halse Rogers (raof) wrote : | # |
+ if (read(wake_fd, &dummy, sizeof(dummy)) != sizeof(dummy) &&
+ errno != EAGAIN)
Hm, now that I look at this again, I think you might be missing && errno != EINTR?
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3925
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
^^^
Bug 1649758
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3927
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
^^^
Bug 1523621
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3928
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Daniel van Vugt (vanvugt) wrote : | # |
Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3929
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Alan Griffiths (alan-griffiths) wrote : | # |
Alan Griffiths (alan-griffiths) wrote : | # |
I don't feel I understand this code well enough to have an opinion. (Would like to hear from Andreas.)
Andreas Pokorny (andreas-pokorny) wrote : | # |
I am in favor of this change - my only opinions here are that we could simplify more... Each further step raises the question - do we want to send input related MirEvents through the "normal" MirConnection fd?
From personal use on the phone I prefer not having the resampling, and also activate a different cpu frequency governor - so from that pov I am not concerned about removing the resampling..
But we can do that in a later MP.
Andreas Pokorny (andreas-pokorny) wrote : | # |
Poor editing of my last review comment, lead to some confusion. By "that" I meant the removal of the remaining android input parts and using the fd of the session.
Mir CI Bot (mir-ci-bot) : | # |
Preview Diff
1 | === modified file 'examples/fingerpaint.c' |
2 | --- examples/fingerpaint.c 2017-01-12 03:18:54 +0000 |
3 | +++ examples/fingerpaint.c 2017-01-16 08:35:54 +0000 |
4 | @@ -16,8 +16,6 @@ |
5 | * Author: Daniel van Vugt <daniel.van.vugt@canonical.com> |
6 | */ |
7 | |
8 | -#define _POSIX_C_SOURCE 200112L // for setenv() from stdlib.h |
9 | - |
10 | #include "mir_toolkit/mir_client_library.h" |
11 | #include "mir_toolkit/events/input/input_event.h" |
12 | |
13 | @@ -415,10 +413,6 @@ |
14 | } |
15 | } |
16 | |
17 | - // We do our own resampling now. We can keep up with raw input... |
18 | - // TODO: Replace setenv with a proper Mir function (LP: #1439590) |
19 | - setenv("MIR_CLIENT_INPUT_RATE", "0", 0); |
20 | - |
21 | conn = mir_connect_sync(mir_socket, argv[0]); |
22 | if (!mir_connection_is_valid(conn)) |
23 | { |
24 | |
25 | === modified file 'examples/target.c' |
26 | --- examples/target.c 2017-01-12 03:18:54 +0000 |
27 | +++ examples/target.c 2017-01-16 08:35:54 +0000 |
28 | @@ -16,7 +16,6 @@ |
29 | * Author: Daniel van Vugt <daniel.van.vugt@canonical.com> |
30 | */ |
31 | |
32 | -#define _POSIX_C_SOURCE 200112L // for setenv() from stdlib.h |
33 | #include "eglapp.h" |
34 | #include <assert.h> |
35 | #include <stdio.h> |
36 | @@ -265,11 +264,6 @@ |
37 | sigaddset(&sigs, SIGHUP); |
38 | pthread_sigmask(SIG_BLOCK, &sigs, NULL); |
39 | |
40 | - // Disable Mir's input resampling. We do our own here, in a way that |
41 | - // has even lower latency than Mir's default algorithm. |
42 | - // TODO: Make a proper client API function for this: |
43 | - setenv("MIR_CLIENT_INPUT_RATE", "0", 0); |
44 | - |
45 | static unsigned int width = 0, height = 0; |
46 | if (!mir_eglapp_init(argc, argv, &width, &height, NULL)) |
47 | return 1; |
48 | |
49 | === modified file 'src/client/input/android/android_input_receiver.cpp' |
50 | --- src/client/input/android/android_input_receiver.cpp 2016-10-31 02:37:31 +0000 |
51 | +++ src/client/input/android/android_input_receiver.cpp 2017-01-16 08:35:54 +0000 |
52 | @@ -53,54 +53,29 @@ |
53 | mircva::InputReceiver::InputReceiver(droidinput::sp<droidinput::InputChannel> const& input_channel, |
54 | std::shared_ptr<mircv::XKBMapper> const& keymapper, |
55 | std::function<void(MirEvent*)> const& event_handling_callback, |
56 | - std::shared_ptr<mircv::InputReceiverReport> const& report, |
57 | - AndroidClock clock) |
58 | - : timer_fd{valid_fd_or_system_error(timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC), |
59 | - "Failed to create IO timer")}, |
60 | - wake_fd{valid_fd_or_system_error(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK | EFD_SEMAPHORE), |
61 | + std::shared_ptr<mircv::InputReceiverReport> const& report) |
62 | + : wake_fd{valid_fd_or_system_error(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK), |
63 | "Failed to create IO wakeup notifier")}, |
64 | input_channel(input_channel), |
65 | handler{event_handling_callback}, |
66 | xkb_mapper(keymapper), |
67 | report(report), |
68 | - input_consumer(std::make_shared<droidinput::InputConsumer>(input_channel)), |
69 | - android_clock(clock) |
70 | + input_consumer(std::make_shared<droidinput::InputConsumer>(input_channel)) |
71 | { |
72 | - /* |
73 | - * 59Hz by default. This ensures the input rate never gets ahead of the |
74 | - * typical display rate, which would be seen as visible lag. |
75 | - */ |
76 | - event_rate_hz = 59; |
77 | - auto env = getenv("MIR_CLIENT_INPUT_RATE"); |
78 | - if (env != NULL) |
79 | - event_rate_hz = atoi(env); |
80 | - |
81 | - dispatcher.add_watch(timer_fd, [this]() |
82 | - { |
83 | - consume_wake_notification(timer_fd); |
84 | - process_and_maybe_send_event(); |
85 | - }); |
86 | - |
87 | - dispatcher.add_watch(wake_fd, [this]() |
88 | - { |
89 | - consume_wake_notification(wake_fd); |
90 | - process_and_maybe_send_event(); |
91 | - }); |
92 | - |
93 | + dispatcher.add_watch(wake_fd, |
94 | + [this]() { woke(); }); |
95 | dispatcher.add_watch(mir::Fd{mir::IntOwnedFd{input_channel->getFd()}}, |
96 | - [this]() { process_and_maybe_send_event(); }); |
97 | + [this]() { woke(); }); |
98 | } |
99 | |
100 | mircva::InputReceiver::InputReceiver(int fd, |
101 | std::shared_ptr<mircv::XKBMapper> const& keymapper, |
102 | std::function<void(MirEvent*)> const& event_handling_callback, |
103 | - std::shared_ptr<mircv::InputReceiverReport> const& report, |
104 | - AndroidClock clock) |
105 | + std::shared_ptr<mircv::InputReceiverReport> const& report) |
106 | : InputReceiver(new droidinput::InputChannel(droidinput::String8(""), fd), |
107 | keymapper, |
108 | event_handling_callback, |
109 | - report, |
110 | - clock) |
111 | + report) |
112 | { |
113 | } |
114 | |
115 | @@ -138,36 +113,28 @@ |
116 | |
117 | } |
118 | |
119 | -void mircva::InputReceiver::process_and_maybe_send_event() |
120 | +void mircva::InputReceiver::woke() |
121 | { |
122 | + uint64_t dummy; |
123 | + |
124 | + /* |
125 | + * We never care about the cause of the wakeup. Either real input or a |
126 | + * wake(). Either way the only requirement is that we woke() after each |
127 | + * wake() or real input channel event. |
128 | + */ |
129 | + if (read(wake_fd, &dummy, sizeof(dummy)) != sizeof(dummy) && |
130 | + errno != EAGAIN && |
131 | + errno != EINTR) |
132 | + { |
133 | + BOOST_THROW_EXCEPTION((std::system_error{errno, |
134 | + std::system_category(), |
135 | + "Failed to consume notification"})); |
136 | + } |
137 | + |
138 | droidinput::InputEvent *android_event; |
139 | uint32_t event_sequence_id; |
140 | |
141 | - /* |
142 | - * Enable "Project Butter" input resampling in InputConsumer::consume(): |
143 | - * consumeBatches = true, so as to ensure the "cooked" event rate that |
144 | - * clients experience is at least the minimum of event_rate_hz |
145 | - * and the raw device event rate. |
146 | - * frame_time = A regular interval. This provides a virtual frame |
147 | - * interval during which InputConsumer will collect raw events, |
148 | - * resample them and emit a "cooked" event back to us at roughly every |
149 | - * 60th of a second. "cooked" events are both smoothed and |
150 | - * extrapolated/predicted into the future (for tool=finger) giving the |
151 | - * appearance of lower latency. Getting a real frame time from the |
152 | - * graphics logic (which is messy) does not appear to be necessary to |
153 | - * gain significant benefit. |
154 | - */ |
155 | - |
156 | - auto frame_time = std::chrono::nanoseconds(-1); |
157 | - |
158 | - if (event_rate_hz > 0) |
159 | - { |
160 | - auto one_frame = std::chrono::nanoseconds(1000000000ULL / event_rate_hz); |
161 | - std::chrono::nanoseconds const |
162 | - now = android_clock(SYSTEM_TIME_MONOTONIC); |
163 | - frame_time = (now / one_frame) * one_frame; |
164 | - } |
165 | - |
166 | + auto const frame_time = std::chrono::nanoseconds(-1); |
167 | auto result = input_consumer->consume(&event_factory, |
168 | true, |
169 | frame_time, |
170 | @@ -197,45 +164,6 @@ |
171 | // So, we ensure we'll appear dispatchable by pushing an event to the wakeup pipe. |
172 | wake(); |
173 | } |
174 | - else if (input_consumer->hasPendingBatch() && event_rate_hz > 0) |
175 | - { |
176 | - using namespace std::chrono; |
177 | - // If we batch according to a fixed event rate - we wait until the next "frame" occurs. |
178 | - auto one_frame = nanoseconds(1000000000ULL / event_rate_hz); |
179 | - auto now = android_clock(SYSTEM_TIME_MONOTONIC); |
180 | - auto next_frame = frame_time + one_frame; |
181 | - |
182 | - if (next_frame <= now) |
183 | - { |
184 | - wake(); |
185 | - } |
186 | - else |
187 | - { |
188 | - auto full_sec = duration_cast<duration<long>>(next_frame); |
189 | - auto nano_sec = duration_cast<duration<long,std::nano>>(next_frame - full_sec); |
190 | - struct itimerspec const frame_timeout = { |
191 | - { 0, 0 }, |
192 | - { full_sec.count(), nano_sec.count()} |
193 | - }; |
194 | - if (timerfd_settime(timer_fd, TFD_TIMER_ABSTIME, &frame_timeout, NULL) < 0) |
195 | - { |
196 | - BOOST_THROW_EXCEPTION((std::system_error{errno, |
197 | - std::system_category(), |
198 | - "Failed to arm timer"})); |
199 | - } |
200 | - } |
201 | - } |
202 | -} |
203 | - |
204 | -void mircva::InputReceiver::consume_wake_notification(mir::Fd const& fd) |
205 | -{ |
206 | - uint64_t dummy; |
207 | - if (read(fd, &dummy, sizeof(dummy)) != sizeof(dummy)) |
208 | - { |
209 | - BOOST_THROW_EXCEPTION((std::system_error{errno, |
210 | - std::system_category(), |
211 | - "Failed to consume notification"})); |
212 | - } |
213 | } |
214 | |
215 | void mircva::InputReceiver::wake() |
216 | |
217 | === modified file 'src/client/input/android/android_input_receiver.h' |
218 | --- src/client/input/android/android_input_receiver.h 2016-01-29 08:18:22 +0000 |
219 | +++ src/client/input/android/android_input_receiver.h 2017-01-16 08:35:54 +0000 |
220 | @@ -56,18 +56,14 @@ |
221 | class InputReceiver : public dispatch::Dispatchable |
222 | { |
223 | public: |
224 | - typedef std::function<std::chrono::nanoseconds(int)> AndroidClock; |
225 | - |
226 | InputReceiver(droidinput::sp<droidinput::InputChannel> const& input_channel, |
227 | std::shared_ptr<XKBMapper> const& keymapper, |
228 | std::function<void(MirEvent*)> const& event_handling_callback, |
229 | - std::shared_ptr<InputReceiverReport> const& report, |
230 | - AndroidClock clock = systemTime); |
231 | + std::shared_ptr<InputReceiverReport> const& report); |
232 | InputReceiver(int fd, |
233 | std::shared_ptr<XKBMapper> const& keymapper, |
234 | std::function<void(MirEvent*)> const& event_handling_callback, |
235 | - std::shared_ptr<InputReceiverReport> const& report, |
236 | - AndroidClock clock = systemTime); |
237 | + std::shared_ptr<InputReceiverReport> const& report); |
238 | |
239 | virtual ~InputReceiver(); |
240 | |
241 | @@ -81,7 +77,6 @@ |
242 | |
243 | private: |
244 | dispatch::MultiplexingDispatchable dispatcher; |
245 | - Fd const timer_fd; |
246 | Fd const wake_fd; |
247 | |
248 | droidinput::sp<droidinput::InputChannel> input_channel; |
249 | @@ -92,11 +87,7 @@ |
250 | std::shared_ptr<droidinput::InputConsumer> input_consumer; |
251 | droidinput::PreallocatedInputEventFactory event_factory; |
252 | |
253 | - AndroidClock const android_clock; |
254 | - int event_rate_hz; |
255 | - |
256 | - void process_and_maybe_send_event(); |
257 | - static void consume_wake_notification(mir::Fd const& fd); |
258 | + void woke(); |
259 | void wake(); |
260 | }; |
261 | |
262 | |
263 | === modified file 'tests/acceptance-tests/test_client_input.cpp' |
264 | --- tests/acceptance-tests/test_client_input.cpp 2017-01-16 04:43:36 +0000 |
265 | +++ tests/acceptance-tests/test_client_input.cpp 2017-01-16 08:35:54 +0000 |
266 | @@ -330,8 +330,6 @@ |
267 | |
268 | TEST_F(TestClientInput, clients_receive_relative_pointer_events) |
269 | { |
270 | - mtf::TemporaryEnvironmentValue disable_batching("MIR_CLIENT_INPUT_RATE", "0"); |
271 | - |
272 | positions[first] = geom::Rectangle{{0,0}, {surface_width, surface_height}}; |
273 | Client first_client(new_connection(), first); |
274 | |
275 | @@ -648,68 +646,6 @@ |
276 | first_client.all_events_received.wait_for(2s); |
277 | } |
278 | |
279 | -TEST_F(TestClientInput, receives_one_touch_event_per_frame) |
280 | -{ |
281 | - positions[first] = screen_geometry; |
282 | - Client first_client(new_connection(), first); |
283 | - |
284 | - int const frame_rate = 60; |
285 | - int const input_rate = 500; |
286 | - int const nframes = 100; |
287 | - int const nframes_error = 80; |
288 | - int const inputs_per_frame = input_rate / frame_rate; |
289 | - int const ninputs = nframes * inputs_per_frame; |
290 | - auto const frame_time = 1000ms / frame_rate; |
291 | - auto const input_interval = std::chrono::duration<double>(1s) / input_rate; |
292 | - |
293 | - int received_input_events = 0; |
294 | - |
295 | - EXPECT_CALL(first_client, handle_input(_)) |
296 | - .Times(Between(nframes-nframes_error, nframes+nframes_error)) |
297 | - .WillRepeatedly(InvokeWithoutArgs( |
298 | - [&]() |
299 | - { |
300 | - ++received_input_events; |
301 | - if (received_input_events >= nframes-nframes_error) |
302 | - first_client.all_events_received.raise(); |
303 | - })); |
304 | - |
305 | - fake_touch_screen->emit_event(mis::a_touch_event() |
306 | - .at_position({0,0})); |
307 | - |
308 | - ASSERT_THAT(input_rate, Ge(2 * frame_rate)); |
309 | - ASSERT_THAT(ninputs, Gt(2 * nframes)); |
310 | - |
311 | - fake_touch_screen->emit_touch_sequence( |
312 | - [this](int i) |
313 | - { |
314 | - auto const x = i; |
315 | - auto const y = 2*i; |
316 | - return mis::a_touch_event() |
317 | - .with_action(mis::TouchParameters::Action::Move) |
318 | - .at_position({x,y}); |
319 | - }, |
320 | - ninputs, |
321 | - input_interval |
322 | - ); |
323 | - |
324 | - // The main thing we're testing for is that too many events don't arrive |
325 | - // so we wait a little to check the cooked event stream has stopped: |
326 | - std::this_thread::sleep_for(200 * frame_time); |
327 | - |
328 | - // Wait for the expected minimum number of events (should be quick but |
329 | - // some CI runs are actually incredibly slow to finish) |
330 | - ASSERT_TRUE(first_client.all_events_received.wait_for(120s)); |
331 | - |
332 | - // Remove reference to local received_input_events |
333 | - Mock::VerifyAndClearExpectations(&first_client); |
334 | - |
335 | - float const client_input_events_per_frame = |
336 | - (float)received_input_events / nframes; |
337 | - EXPECT_THAT(client_input_events_per_frame, Gt(0.0f)); |
338 | - EXPECT_THAT(client_input_events_per_frame, Lt(2.0f)); |
339 | -} |
340 | - |
341 | TEST_F(TestClientInput, send_mir_input_events_through_surface) |
342 | { |
343 | Client first_client(new_connection(), first); |
344 | |
345 | === modified file 'tests/acceptance-tests/test_confined_pointer.cpp' |
346 | --- tests/acceptance-tests/test_confined_pointer.cpp 2017-01-12 03:18:54 +0000 |
347 | +++ tests/acceptance-tests/test_confined_pointer.cpp 2017-01-16 08:35:54 +0000 |
348 | @@ -206,7 +206,6 @@ |
349 | std::string first{"first"}; |
350 | std::string second{"second"}; |
351 | mtf::ClientPositions positions; |
352 | - mtf::TemporaryEnvironmentValue disable_batching{"MIR_CLIENT_INPUT_RATE", "0"}; |
353 | }; |
354 | |
355 | TEST_F(PointerConfinement, test_we_hit_pointer_confined_boundary) |
356 | |
357 | === modified file 'tests/unit-tests/client/input/test_android_input_receiver.cpp' |
358 | --- tests/unit-tests/client/input/test_android_input_receiver.cpp 2016-11-11 06:59:42 +0000 |
359 | +++ tests/unit-tests/client/input/test_android_input_receiver.cpp 2017-01-16 08:35:54 +0000 |
360 | @@ -36,6 +36,7 @@ |
361 | #include <memory> |
362 | #include <system_error> |
363 | #include <boost/throw_exception.hpp> |
364 | +#include <atomic> |
365 | |
366 | namespace mircv = mir::input::receiver; |
367 | namespace mircva = mircv::android; |
368 | @@ -193,7 +194,7 @@ |
369 | EXPECT_TRUE(producer.must_receive_handled_signal()); |
370 | } |
371 | |
372 | -TEST_F(AndroidInputReceiverSetup, receiver_consumes_batched_motion_events) |
373 | +TEST_F(AndroidInputReceiverSetup, receiver_consumes_all_motion_events) |
374 | { |
375 | mircva::InputReceiver receiver{channel.client_fd(), |
376 | std::make_shared<mircv::XKBMapper>(), |
377 | @@ -203,14 +204,18 @@ |
378 | TestingInputProducer producer(channel.server_fd()); |
379 | |
380 | // Produce 3 motion events before client handles any. |
381 | - producer.produce_a_pointer_event(0, 0, std::chrono::nanoseconds(0)); |
382 | - producer.produce_a_pointer_event(0, 0, std::chrono::nanoseconds(0)); |
383 | - producer.produce_a_pointer_event(0, 0, std::chrono::nanoseconds(0)); |
384 | + int const nevents = 3; |
385 | + for (int i = 0; i < nevents; ++i) |
386 | + producer.produce_a_pointer_event(0, 0, std::chrono::nanoseconds(0)); |
387 | |
388 | flush_channels(); |
389 | |
390 | - EXPECT_TRUE(mt::fd_becomes_readable(receiver.watch_fd(), next_event_timeout)); |
391 | - receiver.dispatch(md::FdEvent::readable); |
392 | + for (int j = 0; j < nevents; ++j) |
393 | + { |
394 | + EXPECT_TRUE(mt::fd_becomes_readable(receiver.watch_fd(), |
395 | + next_event_timeout)); |
396 | + receiver.dispatch(md::FdEvent::readable); |
397 | + } |
398 | |
399 | // Now there should be no events |
400 | EXPECT_FALSE(mt::fd_is_readable(receiver.watch_fd())); |
401 | @@ -218,50 +223,33 @@ |
402 | |
403 | TEST_F(AndroidInputReceiverSetup, slow_raw_input_doesnt_cause_frameskipping) |
404 | { // Regression test for LP: #1372300 |
405 | - using namespace testing; |
406 | using namespace std::chrono; |
407 | using namespace std::literals::chrono_literals; |
408 | |
409 | - auto t = 0ns; |
410 | - |
411 | - std::unique_ptr<MirEvent> ev; |
412 | - bool handler_called{false}; |
413 | + std::atomic_int handler_called{0}; |
414 | |
415 | mircva::InputReceiver receiver{channel.client_fd(), |
416 | std::make_shared<mircv::XKBMapper>(), |
417 | - [&ev, &handler_called](MirEvent* event) |
418 | + [&handler_called](MirEvent*) |
419 | { |
420 | - ev.reset(new MirEvent(*event)); |
421 | - handler_called = true; |
422 | + ++handler_called; |
423 | }, |
424 | - std::make_shared<mircv::NullInputReceiverReport>(), |
425 | - [&t](int) |
426 | - { |
427 | - return t; |
428 | - }}; |
429 | + std::make_shared<mircv::NullInputReceiverReport>()}; |
430 | TestingInputProducer producer(channel.server_fd()); |
431 | |
432 | nanoseconds const one_frame = duration_cast<nanoseconds>(1s) / 59; |
433 | |
434 | - producer.produce_a_pointer_event(123, 456, t); |
435 | - producer.produce_a_key_event(); |
436 | + producer.produce_a_pointer_event(123, 456, 0ns); |
437 | + producer.produce_a_pointer_event(234, 567, one_frame); |
438 | flush_channels(); |
439 | |
440 | - // Key events don't get resampled. Will be reported first. |
441 | EXPECT_TRUE(mt::fd_becomes_readable(receiver.watch_fd(), next_event_timeout)); |
442 | receiver.dispatch(md::FdEvent::readable); |
443 | - EXPECT_TRUE(handler_called); |
444 | - ASSERT_EQ(mir_input_event_type_key, ev->to_input()->input_type()); |
445 | + EXPECT_EQ(1, handler_called); |
446 | |
447 | - t += 2 * one_frame; // Account for the slower 59Hz event rate |
448 | - // The motion is still too new. Won't be reported yet, but is batched. |
449 | - // and since batching is locked to the event rate android_input_receiver |
450 | - // will wake up in one frame.. |
451 | EXPECT_TRUE(mt::fd_becomes_readable(receiver.watch_fd(), 2 * one_frame)); |
452 | receiver.dispatch(md::FdEvent::readable); |
453 | - |
454 | - EXPECT_TRUE(handler_called); |
455 | - EXPECT_EQ(mir_input_event_type_touch, ev->to_input()->input_type()); |
456 | + EXPECT_EQ(2, handler_called); |
457 | } |
458 | |
459 | TEST_F(AndroidInputReceiverSetup, finish_signalled_after_handler) |
460 | @@ -287,62 +275,6 @@ |
461 | EXPECT_TRUE(producer.must_receive_handled_signal()); |
462 | } |
463 | |
464 | -TEST_F(AndroidInputReceiverSetup, rendering_does_not_lag_behind_input) |
465 | -{ |
466 | - using namespace testing; |
467 | - using namespace std::chrono; |
468 | - using namespace std::literals::chrono_literals; |
469 | - |
470 | - std::chrono::nanoseconds t; |
471 | - |
472 | - int frames_triggered = 0; |
473 | - |
474 | - mircva::InputReceiver receiver{channel.client_fd(), |
475 | - std::make_shared<mircv::XKBMapper>(), |
476 | - [&frames_triggered](MirEvent*) |
477 | - { |
478 | - ++frames_triggered; |
479 | - }, |
480 | - std::make_shared<mircv::NullInputReceiverReport>(), |
481 | - [&t](int) |
482 | - { |
483 | - return t; |
484 | - }}; |
485 | - TestingInputProducer producer(channel.server_fd()); |
486 | - |
487 | - std::chrono::nanoseconds const device_sample_interval = duration_cast<nanoseconds>(1s) / 250; |
488 | - std::chrono::nanoseconds const frame_interval = duration_cast<nanoseconds>(1s) / 60; |
489 | - std::chrono::nanoseconds const gesture_duration = 1s; |
490 | - |
491 | - std::chrono::nanoseconds last_produced = 0ns; |
492 | - |
493 | - for (t = 0ns; t < gesture_duration; t += 1ms) |
494 | - { |
495 | - if (!t.count() || t >= (last_produced + device_sample_interval)) |
496 | - { |
497 | - last_produced = t; |
498 | - float a = t.count() * M_PI / 1000000.0f; |
499 | - float x = 500.0f * sinf(a); |
500 | - float y = 1000.0f * cosf(a); |
501 | - producer.produce_a_pointer_event(x, y, t); |
502 | - flush_channels(); |
503 | - } |
504 | - |
505 | - if (mt::fd_is_readable(receiver.watch_fd())) |
506 | - { |
507 | - receiver.dispatch(md::FdEvent::readable); |
508 | - } |
509 | - } |
510 | - |
511 | - // If the rendering time resulting from the gesture is longer than the |
512 | - // gesture itself then that's laggy... |
513 | - std::chrono::nanoseconds render_duration = frame_interval * frames_triggered; |
514 | - EXPECT_THAT(render_duration, Le(gesture_duration)); |
515 | - |
516 | - int average_lag_milliseconds = (render_duration - gesture_duration) / (frames_triggered * 1ms); |
517 | - EXPECT_THAT(average_lag_milliseconds, Le(1)); |
518 | -} |
519 | - |
520 | TEST_F(AndroidInputReceiverSetup, input_comes_in_phase_with_rendering) |
521 | { |
522 | using namespace testing; |
FAILED: Continuous integration, rev:3910 /mir-jenkins. ubuntu. com/job/ mir-ci/ 2569/ /mir-jenkins. ubuntu. com/job/ build-mir/ 3347/console /mir-jenkins. ubuntu. com/job/ build-0- fetch/3414 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 3406 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial+ overlay/ 3406 /mir-jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= yakkety/ 3406 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 3376 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= clang,platform= mesa,release= yakkety/ 3376/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3376 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3376/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= amd64,compiler= gcc,platform= mesa,release= yakkety/ 3376/console /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 3376 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= cross-armhf, compiler= gcc,platform= android, release= vivid+overlay/ 3376/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 3376 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= android, release= vivid+overlay/ 3376/artifact/ output/ *zip*/output. zip /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3376 /mir-jenkins. ubuntu. com/job/ build-2- binpkg- mir/arch= i386,compiler= gcc,platform= mesa,release= xenial+ overlay/ 3376/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /mir-jenkins. ubuntu. com/job/ mir-ci/ 2569/rebuild
https:/