Merge lp:~andreas-pokorny/mir/use-input-channel-to-send-input into lp:mir
- use-input-channel-to-send-input
- Merge into development-branch
Status: | Rejected |
---|---|
Rejected by: | Andreas Pokorny |
Proposed branch: | lp:~andreas-pokorny/mir/use-input-channel-to-send-input |
Merge into: | lp:mir |
Diff against target: |
378 lines (+181/-18) 9 files modified
include/server/mir/input/input_channel.h (+4/-1) include/test/mir_test_doubles/stub_input_channel.h (+3/-0) src/server/input/android/android_input_channel.cpp (+100/-2) src/server/input/android/android_input_channel.h (+10/-3) src/server/input/android/android_input_window_handle.cpp (+11/-3) tests/acceptance-tests/test_client_input.cpp (+39/-1) tests/mir_test_framework/stubbed_server_configuration.cpp (+12/-8) tests/unit-tests/input/android/test_android_input_window_handle.cpp (+1/-0) tests/unit-tests/scene/test_surface.cpp (+1/-0) |
To merge this branch: | bzr merge lp:~andreas-pokorny/mir/use-input-channel-to-send-input |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel d'Andrada (community) | Needs Fixing | ||
Robert Carr (community) | Needs Fixing | ||
Alan Griffiths | Needs Fixing | ||
PS Jenkins bot (community) | continuous-integration | Approve | |
Kevin DuBois (community) | Needs Information | ||
Review via email: mp+215450@code.launchpad.net |
Commit message
Add send_event to InputChannel - to be able to dispatch user input events to the client surfaces.
Description of the change
Adds a method to the InputChannel to send MirEvents to clients. This is necessary to implement a custom InputDispatcher outside of mir - hence without access to the adopted android input stack.
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1545
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Alan Griffiths (alan-griffiths) wrote : | # |
24 + virtual void send_event(uint32_t seq, MirEvent const& event) const = 0;
Like Kevin says: What is the significance of seq? Why do I need it to send an event?
~~~~
77 +void react_to_
This seems a bad name. Vis:
142 + react_to_
"check_result" is slightly better, "check" possibly better still.
~~~~
90 + channel = new droidinput:
"TODO: Name" is a distracting choice. What is going on here?
Andreas Pokorny (andreas-pokorny) wrote : | # |
Kevin:
> send_event(uint32_t seq, MirEvent)
> as someone calling send_event, how do I arrange seq;
> is a particular sequence needed? Would it be better to have
> AndroidInputChannel manage the sequence if a particular ordering is required?
Alan:
> 24 + virtual void send_event(uint32_t seq, MirEvent const& event) const =
> 0;
>
> Like Kevin says: What is the significance of seq? Why do I need it to send an
> event?
I believe down - motion - up sequences of events need to have the same sequence number. So I suspect AndroidInputChannel could keep track of press / release event occurrences. The sequence number must be unique for some time, as the client has to respond using the sequence number, as a form of ACK to the server.
> ~~~~
>
> 77 +void react_to_
>
> This seems a bad name. Vis:
>
> 142 + react_to_
>
> "check_result" is slightly better, "check" possibly better still.
ack
> ~~~~
>
> 90 + channel = new droidinput:
> Name"), s_fd);
>
> "TODO: Name" is a distracting choice. What is going on here?
just moved the code - I believe we could use the surface name here instead. fixing that.
Robert Carr (robertcarr) wrote : | # |
We talked a little about seq_id on IRC:
< anpok> dandrader: i would keep the current mir::input:
18:52 < anpok> or the thing to dispatch the input too
18:52 < dandrader> anpok, what's wrong with InputChannel?
18:53 < racarr__> one thing is send_event(seq_id, MirEvent) is weird because the caller cant know how to use seq_id really so its weird as a public interface
18:53 < anpok> it has to be non null :) and should not collide with other input sequences
18:54 < anpok> and nobody reads the ACK signals that come back from the input reader..
18:54 < racarr__> indeed but they will
18:54 < anpok> I only discovered that part today
18:54 < dandrader> racarr__, true
18:54 < racarr__> and if it has to be non null and not collide and thats all
18:54 < racarr__> then it should just be hidden inside the object
Robert Carr (robertcarr) wrote : | # |
79 (missing spaces), 143, 166 (wrong indentation), 310 (trailing line):
whitespace.
Definitely use surface named for TODO: Name now...that was an artifact of interface inadequacies long past...sorry for never coming and fixing my own todo ><. Only used for logging (which is redundantly named elsewhere iirc) anyway
Given seq_id cleanup as part of dispatcher split this is basically good to go to unblock qt compositor I think
Daniel d'Andrada (dandrader) wrote : | # |
That's too low level. It's the android:
What is needed is something like a mir::input:
the hood, calls eventually android:
surface-
MirSurfaceItem:
/qpa-mirserver does.
Daniel d'Andrada (dandrader) wrote : | # |
So in the end we will have to go all the way and refactor android:
Making it only shepherd events through android:
Daniel d'Andrada (dandrader) wrote : | # |
> So in the end we will have to go all the way and refactor
> android:
>
> Making it only shepherd events through android:
> its stripped-down version existing in lp:~unity-team/+junk/qpa-mirserver) and
> leaving the responsibility of selecting which input channel gets the input
> event coming out of the android:
> compositor case, the QQuickWindow dispatch logic that selects the qml item
> that will receive incoming input) that receives all input coming out of
> android:
> surface it decides that should receive the event, if any.
Then, that stripped down android:
Unmerged revisions
- 1545. By Andreas Pokorny
-
Add send_event to InputChannel - for sending events to the client fd.
Preview Diff
1 | === modified file 'include/server/mir/input/input_channel.h' |
2 | --- include/server/mir/input/input_channel.h 2013-08-28 03:41:48 +0000 |
3 | +++ include/server/mir/input/input_channel.h 2014-04-11 16:00:27 +0000 |
4 | @@ -1,5 +1,5 @@ |
5 | /* |
6 | - * Copyright © 2013 Canonical Ltd. |
7 | + * Copyright © 2013-2014 Canonical Ltd. |
8 | * |
9 | * This program is free software: you can redistribute it and/or modify it |
10 | * under the terms of the GNU General Public License version 3, |
11 | @@ -19,6 +19,8 @@ |
12 | #ifndef MIR_INPUT_INPUT_CHANNEL_H_ |
13 | #define MIR_INPUT_INPUT_CHANNEL_H_ |
14 | |
15 | +#include "mir_toolkit/event.h" |
16 | + |
17 | namespace mir |
18 | { |
19 | namespace input |
20 | @@ -32,6 +34,7 @@ |
21 | |
22 | virtual int client_fd() const = 0; |
23 | virtual int server_fd() const = 0; |
24 | + virtual void send_event(uint32_t seq, MirEvent const& event) const = 0; |
25 | |
26 | protected: |
27 | InputChannel() = default; |
28 | |
29 | === modified file 'include/test/mir_test_doubles/stub_input_channel.h' |
30 | --- include/test/mir_test_doubles/stub_input_channel.h 2013-08-28 03:41:48 +0000 |
31 | +++ include/test/mir_test_doubles/stub_input_channel.h 2014-04-11 16:00:27 +0000 |
32 | @@ -48,6 +48,9 @@ |
33 | { |
34 | return input_fd; |
35 | } |
36 | + void send_event(uint32_t /*seq*/, MirEvent const& /*event*/) const override |
37 | + { |
38 | + } |
39 | int input_fd; |
40 | }; |
41 | |
42 | |
43 | === modified file 'src/server/input/android/android_input_channel.cpp' |
44 | --- src/server/input/android/android_input_channel.cpp 2014-03-06 06:05:17 +0000 |
45 | +++ src/server/input/android/android_input_channel.cpp 2014-04-11 16:00:27 +0000 |
46 | @@ -1,5 +1,5 @@ |
47 | /* |
48 | - * Copyright © 2013 Canonical Ltd. |
49 | + * Copyright © 2013-2014 Canonical Ltd. |
50 | * |
51 | * This program is free software: you can redistribute it and/or modify it |
52 | * under the terms of the GNU General Public License version 3, |
53 | @@ -14,21 +14,44 @@ |
54 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
55 | * |
56 | * Authored by: Robert Carr <robert.carr@canonical.com> |
57 | + * Andreas Pokorny <andreas.pokorny@canonical.com> |
58 | */ |
59 | |
60 | #include "android_input_channel.h" |
61 | |
62 | #include <androidfw/InputTransport.h> |
63 | +#include <std/Errors.h> |
64 | + |
65 | +#include <boost/exception/errinfo_errno.hpp> |
66 | +#include <boost/throw_exception.hpp> |
67 | + |
68 | +#include <cmath> |
69 | |
70 | #include <unistd.h> |
71 | |
72 | namespace mia = mir::input::android; |
73 | namespace droidinput = android; |
74 | |
75 | +namespace |
76 | +{ |
77 | +void react_to_result(droidinput::status_t result) |
78 | +{ |
79 | + if (result!=droidinput::OK) |
80 | + { |
81 | + BOOST_THROW_EXCEPTION(boost::enable_error_info(std::runtime_error("Failure sending input event ")) << boost::errinfo_errno(result)); |
82 | + } |
83 | +} |
84 | +} |
85 | + |
86 | mia::AndroidInputChannel::AndroidInputChannel() |
87 | { |
88 | - |
89 | droidinput::InputChannel::openInputFdPair(s_fd, c_fd); |
90 | + channel = new droidinput::InputChannel(droidinput::String8("TODO: Name"), s_fd); |
91 | +} |
92 | + |
93 | +droidinput::sp<droidinput::InputChannel> mia::AndroidInputChannel::get_android_channel() const |
94 | +{ |
95 | + return channel; |
96 | } |
97 | |
98 | mia::AndroidInputChannel::~AndroidInputChannel() |
99 | @@ -46,3 +69,78 @@ |
100 | { |
101 | return s_fd; |
102 | } |
103 | + |
104 | +void mia::AndroidInputChannel::send_event(uint32_t seq, MirEvent const& event) const |
105 | +{ |
106 | + switch(event.type) |
107 | + { |
108 | + case mir_event_type_key: |
109 | + send_key_event(seq, event.key); |
110 | + break; |
111 | + case mir_event_type_motion: |
112 | + send_motion_event(seq, event.motion); |
113 | + break; |
114 | + default: |
115 | + break; |
116 | + } |
117 | +} |
118 | + |
119 | +void mia::AndroidInputChannel::send_motion_event(uint32_t seq, MirMotionEvent const& event) const |
120 | +{ |
121 | + droidinput::PointerCoords coords[MIR_INPUT_EVENT_MAX_POINTER_COUNT]; |
122 | + droidinput::PointerProperties properties[MIR_INPUT_EVENT_MAX_POINTER_COUNT]; |
123 | + for (size_t i = 0; i < event.pointer_count; i++) |
124 | + { |
125 | + // TODO this assumes that: x == raw_x + x_offset; |
126 | + // here x,y is used instead of the raw co-ordinates and offset is set to zero |
127 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_X, event.pointer_coordinates[i].x); |
128 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_Y, event.pointer_coordinates[i].y); |
129 | + |
130 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_TOUCH_MAJOR, event.pointer_coordinates[i].touch_major); |
131 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_TOUCH_MINOR, event.pointer_coordinates[i].touch_minor); |
132 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_SIZE, event.pointer_coordinates[i].size); |
133 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_PRESSURE, event.pointer_coordinates[i].pressure); |
134 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_ORIENTATION, event.pointer_coordinates[i].orientation); |
135 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_VSCROLL, event.pointer_coordinates[i].vscroll); |
136 | + coords[i].setAxisValue(AMOTION_EVENT_AXIS_HSCROLL, event.pointer_coordinates[i].hscroll); |
137 | + properties[i].toolType = event.pointer_coordinates[i].tool_type; |
138 | + } |
139 | + |
140 | + droidinput::InputPublisher publisher(channel); |
141 | + |
142 | + react_to_result(publisher.publishMotionEvent(seq, |
143 | + event.device_id, |
144 | + event.source_id, |
145 | + event.action, |
146 | + static_cast<int32_t>(event.flags), |
147 | + event.edge_flags, |
148 | + static_cast<int32_t>(event.modifiers), |
149 | + static_cast<int32_t>(event.button_state), |
150 | + 0.0f, // event.x_offset, |
151 | + 0.0f, // event.y_offset, |
152 | + event.x_precision, |
153 | + event.y_precision, |
154 | + event.down_time, |
155 | + event.event_time, |
156 | + event.pointer_count, |
157 | + properties, |
158 | + coords)); |
159 | +} |
160 | + |
161 | +void mia::AndroidInputChannel::send_key_event(uint32_t seq, MirKeyEvent const& event) const |
162 | +{ |
163 | + droidinput::InputPublisher publisher(channel); |
164 | + |
165 | + react_to_result(publisher.publishKeyEvent(seq, |
166 | + event.device_id, |
167 | + event.source_id, |
168 | + event.action, |
169 | + event.flags, |
170 | + event.key_code, |
171 | + event.scan_code, |
172 | + event.modifiers, |
173 | + event.repeat_count, |
174 | + event.down_time, |
175 | + event.event_time)); |
176 | +} |
177 | + |
178 | |
179 | === modified file 'src/server/input/android/android_input_channel.h' |
180 | --- src/server/input/android/android_input_channel.h 2013-08-28 03:41:48 +0000 |
181 | +++ src/server/input/android/android_input_channel.h 2014-04-11 16:00:27 +0000 |
182 | @@ -1,5 +1,5 @@ |
183 | /* |
184 | - * Copyright © 2013 Canonical Ltd. |
185 | + * Copyright © 2013-2014 Canonical Ltd. |
186 | * |
187 | * This program is free software: you can redistribute it and/or modify it |
188 | * under the terms of the GNU General Public License version 3, |
189 | @@ -14,6 +14,7 @@ |
190 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
191 | * |
192 | * Authored by: Robert Carr <robert.carr@canonical.com> |
193 | + * Andreas Pokorny <andreas.pokorny@canonical.com> |
194 | */ |
195 | |
196 | #ifndef MIR_INPUT_ANDROID_INPUT_CHANNEL_H_ |
197 | @@ -22,6 +23,7 @@ |
198 | #include "mir/input/input_channel.h" |
199 | |
200 | #include <utils/StrongPointer.h> |
201 | +#include <androidfw/InputTransport.h> |
202 | |
203 | namespace android |
204 | { |
205 | @@ -43,15 +45,20 @@ |
206 | explicit AndroidInputChannel(); |
207 | virtual ~AndroidInputChannel(); |
208 | |
209 | - int client_fd() const; |
210 | - int server_fd() const; |
211 | + int client_fd() const override; |
212 | + int server_fd() const override; |
213 | + void send_event(uint32_t seq, MirEvent const& event) const override; |
214 | |
215 | + droidinput::sp<droidinput::InputChannel> get_android_channel() const; |
216 | protected: |
217 | AndroidInputChannel(AndroidInputChannel const&) = delete; |
218 | AndroidInputChannel& operator=(AndroidInputChannel const&) = delete; |
219 | |
220 | private: |
221 | int s_fd, c_fd; |
222 | + droidinput::sp<droidinput::InputChannel> channel; |
223 | + void send_key_event(uint32_t seq, MirKeyEvent const& event) const; |
224 | + void send_motion_event(uint32_t seq, MirMotionEvent const& event) const; |
225 | }; |
226 | |
227 | } |
228 | |
229 | === modified file 'src/server/input/android/android_input_window_handle.cpp' |
230 | --- src/server/input/android/android_input_window_handle.cpp 2014-03-06 06:05:17 +0000 |
231 | +++ src/server/input/android/android_input_window_handle.cpp 2014-04-11 16:00:27 +0000 |
232 | @@ -18,6 +18,7 @@ |
233 | |
234 | #include "android_input_window_handle.h" |
235 | #include "android_input_application_handle.h" |
236 | +#include "android_input_channel.h" |
237 | |
238 | #include "mir/input/input_channel.h" |
239 | #include "mir/input/surface.h" |
240 | @@ -64,9 +65,16 @@ |
241 | { |
242 | mInfo = new WindowInfo(surface); |
243 | |
244 | - // TODO: How can we avoid recreating the InputChannel which the InputChannelFactory has already created? |
245 | - mInfo->inputChannel = new droidinput::InputChannel(droidinput::String8("TODO: Name"), |
246 | - input_channel->server_fd()); |
247 | + std::shared_ptr<AndroidInputChannel> droidchannel = std::dynamic_pointer_cast<AndroidInputChannel>(input_channel); |
248 | + if (droidchannel) |
249 | + { |
250 | + mInfo->inputChannel = droidchannel->get_android_channel(); |
251 | + } |
252 | + else |
253 | + { |
254 | + mInfo->inputChannel = new droidinput::InputChannel(droidinput::String8("TODO: Name"), |
255 | + input_channel->server_fd()); |
256 | + } |
257 | } |
258 | |
259 | auto surface_size = surface->size(); |
260 | |
261 | === modified file 'tests/acceptance-tests/test_client_input.cpp' |
262 | --- tests/acceptance-tests/test_client_input.cpp 2014-04-04 14:42:55 +0000 |
263 | +++ tests/acceptance-tests/test_client_input.cpp 2014-04-11 16:00:27 +0000 |
264 | @@ -1,5 +1,5 @@ |
265 | /* |
266 | - * Copyright © 2013 Canonical Ltd. |
267 | + * Copyright © 2013-2014 Canonical Ltd. |
268 | * |
269 | * This program is free software: you can redistribute it and/or modify |
270 | * it under the terms of the GNU General Public License version 3 as |
271 | @@ -14,11 +14,13 @@ |
272 | * along with this program. If not, see <http://www.gnu.org/licenses/>. |
273 | * |
274 | * Authored by: Robert Carr <robert.carr@canonical.com> |
275 | + * Andreas Pokorny <andreas.pokorny@canonical.com> |
276 | */ |
277 | |
278 | #include "mir/graphics/display.h" |
279 | #include "mir/shell/surface_creation_parameters.h" |
280 | #include "mir/shell/placement_strategy.h" |
281 | +#include "mir/input/input_channel.h" |
282 | #include "mir/scene/surface_coordinator.h" |
283 | #include "mir/scene/surface.h" |
284 | #include "src/server/scene/session_container.h" |
285 | @@ -613,3 +615,39 @@ |
286 | |
287 | launch_client_process(*client); |
288 | } |
289 | + |
290 | +TEST_F(TestClientInput, send_mir_events_with_input_channel) |
291 | +{ |
292 | + using namespace ::testing; |
293 | + |
294 | + static std::string const test_client_name = "1"; |
295 | + |
296 | + mtf::CrossProcessSync fence; |
297 | + |
298 | + MirEvent key_event; |
299 | + memset(&key_event,0,sizeof(key_event)); |
300 | + key_event.type = mir_event_type_key; |
301 | + key_event.key.action= mir_key_action_down; |
302 | + |
303 | + auto server_config = make_event_producing_server(fence, 1, |
304 | + [&](mtf::InputTestingServerConfiguration& server) |
305 | + { |
306 | + server.the_session_container()->for_each([&](std::shared_ptr<ms::Session> const& session) -> void |
307 | + { |
308 | + session->default_surface()->input_channel()->send_event(1,key_event); |
309 | + }); |
310 | + |
311 | + }); |
312 | + launch_server_process(*server_config); |
313 | + |
314 | + auto client_config = make_event_expecting_client(fence, |
315 | + [&](MockHandler& handler, mt::WaitCondition& events_received) |
316 | + { |
317 | + using namespace ::testing; |
318 | + InSequence seq; |
319 | + EXPECT_CALL(handler, handle_input(mt::KeyDownEvent())).Times(1) |
320 | + .WillOnce(mt::WakeUp(&events_received)); |
321 | + |
322 | + }); |
323 | + launch_client_process(*client_config); |
324 | +} |
325 | |
326 | === modified file 'tests/mir_test_framework/stubbed_server_configuration.cpp' |
327 | --- tests/mir_test_framework/stubbed_server_configuration.cpp 2014-03-27 09:46:09 +0000 |
328 | +++ tests/mir_test_framework/stubbed_server_configuration.cpp 2014-04-11 16:00:27 +0000 |
329 | @@ -217,14 +217,18 @@ |
330 | |
331 | struct StubInputChannel : public mi::InputChannel |
332 | { |
333 | - int client_fd() const |
334 | - { |
335 | - return 0; |
336 | - } |
337 | - |
338 | - int server_fd() const |
339 | - { |
340 | - return 0; |
341 | + int client_fd() const override |
342 | + { |
343 | + return 0; |
344 | + } |
345 | + |
346 | + int server_fd() const override |
347 | + { |
348 | + return 0; |
349 | + } |
350 | + |
351 | + void send_event(uint32_t /*seq*/, MirEvent const& /*event*/) const override |
352 | + { |
353 | } |
354 | }; |
355 | |
356 | |
357 | === modified file 'tests/unit-tests/input/android/test_android_input_window_handle.cpp' |
358 | --- tests/unit-tests/input/android/test_android_input_window_handle.cpp 2014-03-26 14:20:14 +0000 |
359 | +++ tests/unit-tests/input/android/test_android_input_window_handle.cpp 2014-04-11 16:00:27 +0000 |
360 | @@ -52,6 +52,7 @@ |
361 | { |
362 | MOCK_CONST_METHOD0(client_fd, int()); |
363 | MOCK_CONST_METHOD0(server_fd, int()); |
364 | + MOCK_CONST_METHOD2(send_event,void(uint32_t /*seq*/, MirEvent const& /*event*/)); |
365 | }; |
366 | |
367 | } |
368 | |
369 | === modified file 'tests/unit-tests/scene/test_surface.cpp' |
370 | --- tests/unit-tests/scene/test_surface.cpp 2014-03-31 16:51:06 +0000 |
371 | +++ tests/unit-tests/scene/test_surface.cpp 2014-04-11 16:00:27 +0000 |
372 | @@ -52,6 +52,7 @@ |
373 | { |
374 | MOCK_CONST_METHOD0(server_fd, int()); |
375 | MOCK_CONST_METHOD0(client_fd, int()); |
376 | + MOCK_CONST_METHOD2(send_event,void(uint32_t /*seq*/, MirEvent const& /*event*/)); |
377 | }; |
378 | } |
379 |
send_event(uint32_t seq, MirEvent)
as someone calling send_event, how do I arrange seq; is a particular sequence needed? Would it be better to have AndroidInputChannel manage the sequence if a particular ordering is required?