Mir

Merge lp:~mir-team/mir/unify-pointer-button into lp:mir

Proposed by Robert Carr
Status: Merged
Approved by: Robert Carr
Approved revision: no longer in the source branch.
Merged at revision: 2589
Proposed branch: lp:~mir-team/mir/unify-pointer-button
Merge into: lp:mir
Prerequisite: lp:~andreas-pokorny/mir/connect-cursor-listener-to-input-device-hub
Diff against target: 778 lines (+190/-114)
25 files modified
include/client/mir/events/event_builders.h (+1/-2)
include/client/mir_toolkit/events/input/pointer_event.h (+15/-5)
src/client/events/event_builders.cpp (+19/-30)
src/client/input/android/android_input_lexicon.cpp (+1/-1)
src/client/input/android/event_conversion_helpers.cpp (+34/-0)
src/client/input/input_event.cpp (+11/-5)
src/client/symbols.map (+3/-0)
src/include/common/mir/events/event_private.h (+1/-9)
src/include/common/mir/input/android/event_conversion_helpers.h (+3/-1)
src/server/input/android/android_input_dispatcher.cpp (+1/-1)
src/server/input/android/input_sender.cpp (+1/-1)
src/server/input/android/input_translator.cpp (+1/-1)
tests/acceptance-tests/test_client_input.cpp (+43/-0)
tests/acceptance-tests/test_surface_modifications.cpp (+2/-4)
tests/acceptance-tests/test_surface_placement.cpp (+1/-1)
tests/include/mir_test/event_matchers.h (+26/-25)
tests/mir_test_framework/fake_input_device_impl.cpp (+3/-3)
tests/mir_test_framework/fake_input_device_impl.h (+1/-3)
tests/unit-tests/input/android/test_android_input_dispatcher.cpp (+5/-4)
tests/unit-tests/input/android/test_android_input_lexicon.cpp (+2/-5)
tests/unit-tests/input/android/test_input_translator.cpp (+2/-3)
tests/unit-tests/input/test_default_input_device_hub.cpp (+3/-3)
tests/unit-tests/input/test_event_builders.cpp (+2/-4)
tests/unit-tests/input/test_input_event.cpp (+8/-2)
tests/unit-tests/scene/test_abstract_shell.cpp (+1/-1)
To merge this branch: bzr merge lp:~mir-team/mir/unify-pointer-button
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alberto Aguirre (community) Approve
Alan Griffiths Approve
Daniel van Vugt Pending
Alexandros Frantzis Pending
Andreas Pokorny Pending
Kevin DuBois Pending
Review via email: mp+259148@code.launchpad.net

This proposal supersedes a proposal from 2015-05-05.

Commit message

Unify the pointer button enumerations.

Description of the change

Unify the pointer button enums. The new style event buttons are not using a mask so as to allow for extensibility so there is some churn in the change of the internal representation.

Resubmit to depend on connect-cursor-listener-to-input-device-hub to enhanced testing

This branch is part of the event cleaning pipeline:

Pluck-low-hanging-event-fruit: Remove unused event members.
Unify-event-modifiers: Consolidate modifier enums (https://code.launchpad.net/~mir-team/mir/unify-event-modifiers/+merge/258218)
Unify-keyboard-actions: Consolidate keyboard action enums and key repeat representation (https://code.launchpad.net/~mir-team/mir/unify-keyboard-actions/+merge/258288)
Remaining-nsec-removal: Remove usage of nsecs_t in event_private.h https://code.launchpad.net/~mir-team/mir/remaining-nsec-removal/+merge/258292
unify-pointer-button: Consolidate pointer button enums
https://code.launchpad.net/~mir-team/mir/unify-pointer-button

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

> The new style event buttons are not using a mask so as to allow for extensibility so there
> is some churn in the change of the internal representation.

How is having a mask reduce extensibility? Do you mean running out of bits?

190+ bool button_state[mir_pointer_button_forward + 1];

This will break if we add a new member to the enum (although we will probably catch the issue at compile time). In any case, it's best to introduce a mir_pointer_button_count (or similar) value, and actually make it a policy to have such a member in all suitable enums.

Needs info/fixing

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

Added mir_pointer_button_Count...good call...tbh I dont like this solution that much but I'm concerned about the extensibility with flags (e.g. running out of bits). In particular what if devices have buttons only available through some sort of mir_input_device_list_buttons_ API (e.g. they don't correspond to forward/back...)? ON the other hand I guess this device wouldn't be a pointer...maybe flags are fine...I'd like to get another opinion! RAOF?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Seems self-consistent.

But I'm a little concerned that other APIs treat buttons as bitmasks (e.g. "143+ android_state |= AMOTION_EVENT_BUTTON_PRIMARY;"). Apart from not matching expectations are we really sure we know better?

review: Needs Information
Revision history for this message
Robert Carr (robertcarr) wrote : Posted in a previous version of this proposal

I've thought through my concerns that related to nto storing buttons as a flag. Primarly I was worried about exotic devices and allocating button indices for them and potentially running out of flag values. I suppose if anything though such devices would not be pointers and only available throuhg the raw input API so I'm not concerned. Updated branch to use flag...required more client ABI break :)

Revision history for this message
Kevin DuBois (kdub) wrote : Posted in a previous version of this proposal

nonblocking comment:
The bitmask seems okay for now, plenty of unused bits left.

needs-info
77+ mev.button_state[mir_pointer_button_tertiary] = true;

Since this appears private now, could we have something more like:
mev.button_state[mir_pointer_button_tertiary] = pressed;
It took a bit of reading of the struct to figure out what was going on in the present form.

review: Needs Information
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Andreas Pokorny (andreas-pokorny) wrote : Posted in a previous version of this proposal

ok looks good apart from the merge conflict

Revision history for this message
Andreas Pokorny (andreas-pokorny) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

(0) MirPointerButton: Definitely a client ABI break now, so we must remember to bump to libmirclient9 ASAP (Alan is working on it but blocked by CI annoyances).

(1) I guess this isn't well tested yet because there's a logic error (should be "&="):
365 - buttons.erase(remove(begin(buttons), end(buttons), button));
366 + buttons |= ~button;

review: Needs Fixing
Revision history for this message
Alexandros Frantzis (afrantzis) wrote : Posted in a previous version of this proposal

Besides Daniel's "Needs Fixing", looks good.

Nit:

239-

Why remove the line?

review: Approve
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Agree with Daniel about test coverage and Alexandros about whitespace.

review: Needs Fixing
Revision history for this message
Robert Carr (robertcarr) wrote :

Good catch that the fake input device wasn't properly tested. Real devices worked though! Merged connect-cursor-listener-to-input-device-hub in order to write a test to make this possible.

Added said test (see diff: presses and releases all buttons)

Discovered another issue: If we omit mir_motion_action_down (gesture start) for a second button down instead of mir_motion_action_pointer_down the "conflicting action" codepath in android::InputDispatcher::findFocusedWindowTargetsLocked and the event gets converted to a motion event. Corrected this with some refactoring to the event builders.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

=== modified file '3rd_party/android-input/android/frameworks/base/services/input/InputDispatcher.cpp'

Are the changes to this file anything to do with merging the pointer types?

review: Needs Information
Revision history for this message
Robert Carr (robertcarr) wrote :

Removed extra changes

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

229 + auto const& old_mev = old_mev_from_new(pev);
230 + return old_mev.buttons;

There still seems little point in naming the reference. (C.f. return old_mev_from_new(pev).buttons;)

review: Approve
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

LGTM

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

(2) This could be simplified with a while loop with bitshift each time, but not a blocking issue:
90 +int count_buttons(MirPointerButtons buttons)

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/client/mir/events/event_builders.h'
2--- include/client/mir/events/event_builders.h 2015-05-13 06:23:25 +0000
3+++ include/client/mir/events/event_builders.h 2015-05-20 14:14:47 +0000
4@@ -25,7 +25,6 @@
5 #include "mir/frontend/surface_id.h"
6
7 #include <memory>
8-#include <vector>
9 #include <functional>
10 #include <chrono>
11
12@@ -63,7 +62,7 @@
13 // Pointer event
14 EventUPtr make_event(MirInputDeviceId device_id, std::chrono::nanoseconds timestamp,
15 MirInputEventModifiers modifiers, MirPointerAction action,
16- std::vector<MirPointerButton> const& buttons_pressed,
17+ MirPointerButtons buttons_pressed,
18 float x_axis_value, float y_axis_value,
19 float hscroll_value, float vscroll_value);
20 }
21
22=== modified file 'include/client/mir_toolkit/events/input/pointer_event.h'
23--- include/client/mir_toolkit/events/input/pointer_event.h 2015-03-31 02:35:42 +0000
24+++ include/client/mir_toolkit/events/input/pointer_event.h 2015-05-20 14:14:47 +0000
25@@ -68,12 +68,13 @@
26 * Identifiers for pointer buttons
27 */
28 typedef enum {
29- mir_pointer_button_primary = 1,
30- mir_pointer_button_secondary = 2,
31- mir_pointer_button_tertiary = 3,
32- mir_pointer_button_back = 4,
33- mir_pointer_button_forward = 5
34+ mir_pointer_button_primary = 1 << 0,
35+ mir_pointer_button_secondary = 1 << 1,
36+ mir_pointer_button_tertiary = 1 << 2,
37+ mir_pointer_button_back = 1 << 3,
38+ mir_pointer_button_forward = 1 << 4
39 } MirPointerButton;
40+typedef unsigned int MirPointerButtons;
41
42 /**
43 * Retrieve the modifier keys pressed when the pointer action occured.
44@@ -103,6 +104,15 @@
45 MirPointerButton button);
46
47 /**
48+ * Retreive the pointer button state as a masked set of values.
49+ *
50+ * \param [in] event The pointer event
51+ *
52+ * \return The button state
53+ */
54+MirPointerButtons mir_pointer_event_buttons(MirPointerEvent const* event);
55+
56+/**
57 * Retrieve the axis value reported by a given pointer event.
58 *
59 * \param [in] event The pointer event
60
61=== modified file 'src/client/events/event_builders.cpp'
62--- src/client/events/event_builders.cpp 2015-05-19 14:02:31 +0000
63+++ src/client/events/event_builders.cpp 2015-05-20 14:14:47 +0000
64@@ -231,29 +231,40 @@
65
66 namespace
67 {
68-MirMotionAction old_action_from_pointer_action(MirPointerAction action, MirMotionButton button_state)
69+MirMotionAction old_action_from_pointer_action(MirPointerAction action, int buttons_pressed)
70 {
71 switch (action)
72 {
73 case mir_pointer_action_button_up:
74- return mir_motion_action_up;
75+ return buttons_pressed == 0 ? mir_motion_action_up : mir_motion_action_pointer_up;
76 case mir_pointer_action_button_down:
77- return mir_motion_action_down;
78+ return buttons_pressed == 1 ? mir_motion_action_down : mir_motion_action_pointer_down;
79 case mir_pointer_action_enter:
80 return mir_motion_action_hover_enter;
81 case mir_pointer_action_leave:
82 return mir_motion_action_hover_exit;
83 case mir_pointer_action_motion:
84- return button_state ? mir_motion_action_move : mir_motion_action_hover_move;
85+ return buttons_pressed ? mir_motion_action_move : mir_motion_action_hover_move;
86 default:
87 BOOST_THROW_EXCEPTION(std::logic_error("Invalid pointer action"));
88 }
89 }
90+int count_buttons(MirPointerButtons buttons)
91+{
92+ int ret = 0;
93+ if (buttons & mir_pointer_button_primary) ret++;
94+ if (buttons & mir_pointer_button_secondary) ret++;
95+ if (buttons & mir_pointer_button_tertiary) ret++;
96+ if (buttons & mir_pointer_button_forward) ret++;
97+ if (buttons & mir_pointer_button_back) ret++;
98+
99+ return ret;
100+}
101 }
102
103 mir::EventUPtr mev::make_event(MirInputDeviceId device_id, std::chrono::nanoseconds timestamp,
104 MirInputEventModifiers modifiers, MirPointerAction action,
105- std::vector<MirPointerButton> const& buttons_pressed,
106+ MirPointerButtons buttons_pressed,
107 float x_axis_value, float y_axis_value,
108 float hscroll_value, float vscroll_value)
109 {
110@@ -266,31 +277,9 @@
111 mev.event_time = timestamp;
112 mev.modifiers = modifiers;
113 mev.source_id = AINPUT_SOURCE_MOUSE;
114-
115- int button_state = 0;
116- for (auto button : buttons_pressed)
117- {
118- switch (button)
119- {
120- case mir_pointer_button_primary:
121- button_state |= mir_motion_button_primary;
122- break;
123- case mir_pointer_button_secondary:
124- button_state |= mir_motion_button_secondary;
125- break;
126- case mir_pointer_button_tertiary:
127- button_state |= mir_motion_button_tertiary;
128- break;
129- case mir_pointer_button_back:
130- button_state |= mir_motion_button_back;
131- break;
132- case mir_pointer_button_forward:
133- button_state |= mir_motion_button_forward;
134- break;
135- }
136- }
137- mev.button_state = static_cast<MirMotionButton>(button_state);
138- mev.action = old_action_from_pointer_action(action, mev.button_state);
139+ mev.buttons = buttons_pressed;
140+
141+ mev.action = old_action_from_pointer_action(action, count_buttons(buttons_pressed));
142
143 mev.pointer_count = 1;
144 auto& pc = mev.pointer_coordinates[0];
145
146=== modified file 'src/client/input/android/android_input_lexicon.cpp'
147--- src/client/input/android/android_input_lexicon.cpp 2015-05-13 06:23:25 +0000
148+++ src/client/input/android/android_input_lexicon.cpp 2015-05-20 14:14:47 +0000
149@@ -50,7 +50,7 @@
150 mir_event.motion.source_id = android_event->getSource();
151 mir_event.motion.action = mev->getAction();
152 mir_event.motion.modifiers = mia::mir_modifiers_from_android(mev->getMetaState());
153- mir_event.motion.button_state = static_cast<MirMotionButton>(mev->getButtonState());
154+ mir_event.motion.buttons = mia::mir_pointer_buttons_from_android(mev->getButtonState());
155 mir_event.motion.event_time = mev->getEventTime();
156 mir_event.motion.pointer_count = mev->getPointerCount();
157 for(unsigned int i = 0; i < mev->getPointerCount(); i++)
158
159=== modified file 'src/client/input/android/event_conversion_helpers.cpp'
160--- src/client/input/android/event_conversion_helpers.cpp 2015-05-13 06:23:25 +0000
161+++ src/client/input/android/event_conversion_helpers.cpp 2015-05-20 14:14:47 +0000
162@@ -100,3 +100,37 @@
163 return AKEY_EVENT_ACTION_UP;
164 }
165 }
166+
167+MirPointerButtons mia::mir_pointer_buttons_from_android(int32_t android_state)
168+{
169+ MirPointerButtons buttons = 0;
170+
171+ if (android_state & AMOTION_EVENT_BUTTON_PRIMARY)
172+ buttons |= mir_pointer_button_primary;
173+ if (android_state & AMOTION_EVENT_BUTTON_SECONDARY)
174+ buttons |= mir_pointer_button_secondary;
175+ if (android_state & AMOTION_EVENT_BUTTON_TERTIARY)
176+ buttons |= mir_pointer_button_tertiary;
177+ if (android_state & AMOTION_EVENT_BUTTON_BACK)
178+ buttons |= mir_pointer_button_back;
179+ if (android_state & AMOTION_EVENT_BUTTON_FORWARD)
180+ buttons |= mir_pointer_button_forward;
181+
182+ return buttons;
183+}
184+
185+int32_t mia::android_pointer_buttons_from_mir(MirPointerButtons buttons)
186+{
187+ int32_t android_state = 0;
188+ if (buttons & mir_pointer_button_primary)
189+ android_state |= AMOTION_EVENT_BUTTON_PRIMARY;
190+ if (buttons & mir_pointer_button_secondary)
191+ android_state |= AMOTION_EVENT_BUTTON_SECONDARY;
192+ if (buttons & mir_pointer_button_tertiary)
193+ android_state |= AMOTION_EVENT_BUTTON_TERTIARY;
194+ if (buttons & mir_pointer_button_back)
195+ android_state |= AMOTION_EVENT_BUTTON_BACK;
196+ if (buttons & mir_pointer_button_forward)
197+ android_state |= AMOTION_EVENT_BUTTON_FORWARD;
198+ return android_state;
199+}
200
201=== modified file 'src/client/input/input_event.cpp'
202--- src/client/input/input_event.cpp 2015-05-13 06:23:25 +0000
203+++ src/client/input/input_event.cpp 2015-05-20 14:14:47 +0000
204@@ -424,20 +424,26 @@
205 switch (button)
206 {
207 case mir_pointer_button_primary:
208- return old_mev.button_state & mir_motion_button_primary;
209+ return old_mev.buttons & mir_pointer_button_primary;
210 case mir_pointer_button_secondary:
211- return old_mev.button_state & mir_motion_button_secondary;
212+ return old_mev.buttons & mir_pointer_button_secondary;
213 case mir_pointer_button_tertiary:
214- return old_mev.button_state & mir_motion_button_tertiary;
215+ return old_mev.buttons & mir_pointer_button_tertiary;
216 case mir_pointer_button_back:
217- return old_mev.button_state & mir_motion_button_back;
218+ return old_mev.buttons & mir_pointer_button_back;
219 case mir_pointer_button_forward:
220- return old_mev.button_state & mir_motion_button_forward;
221+ return old_mev.buttons & mir_pointer_button_forward;
222 default:
223 return false;
224 }
225 }
226
227+MirPointerButtons mir_pointer_event_buttons(MirPointerEvent const* pev)
228+{
229+ auto const& old_mev = old_mev_from_new(pev);
230+ return old_mev.buttons;
231+}
232+
233 float mir_pointer_event_axis_value(MirPointerEvent const* pev, MirPointerAxis axis)
234 {
235 auto const& old_mev = old_mev_from_new(pev);
236
237=== modified file 'src/client/symbols.map'
238--- src/client/symbols.map 2015-05-13 06:23:25 +0000
239+++ src/client/symbols.map 2015-05-20 14:14:47 +0000
240@@ -196,6 +196,7 @@
241 mir_buffer_stream_release;
242 mir_buffer_stream_release_sync;
243 mir_buffer_stream_is_valid;
244+ mir_pointer_event_buttons; # Move to 8.5
245 } MIR_CLIENT_8.3;
246
247 MIR_CLIENT_DETAIL_8 {
248@@ -208,6 +209,8 @@
249 mir::input::android::mir_modifiers_from_android*;
250 mir::input::android::mir_keyboard_action_from_android*;
251 mir::input::android::android_keyboard_action_from_mir*;
252+ mir::input::android::mir_pointer_buttons_from_android*;
253+ mir::input::android::android_pointer_buttons_from_mir*;
254 mir::client::DefaultConnectionConfiguration::DefaultConnectionConfiguration*;
255 mir::client::DefaultConnectionConfiguration::the_surface_map*;
256 mir::client::DefaultConnectionConfiguration::the_rpc_channel*;
257
258=== modified file 'src/include/common/mir/events/event_private.h'
259--- src/include/common/mir/events/event_private.h 2015-05-13 06:23:25 +0000
260+++ src/include/common/mir/events/event_private.h 2015-05-20 14:14:47 +0000
261@@ -60,14 +60,6 @@
262 } MirMotionAction;
263
264 typedef enum {
265- mir_motion_button_primary = 1 << 0,
266- mir_motion_button_secondary = 1 << 1,
267- mir_motion_button_tertiary = 1 << 2,
268- mir_motion_button_back = 1 << 3,
269- mir_motion_button_forward = 1 << 4
270-} MirMotionButton;
271-
272-typedef enum {
273 mir_motion_tool_type_unknown = 0,
274 mir_motion_tool_type_finger = 1,
275 mir_motion_tool_type_stylus = 2,
276@@ -125,7 +117,7 @@
277 int action;
278 MirInputEventModifiers modifiers;
279
280- MirMotionButton button_state;
281+ MirPointerButtons buttons;
282 std::chrono::nanoseconds event_time;
283
284 size_t pointer_count;
285
286=== modified file 'src/include/common/mir/input/android/event_conversion_helpers.h'
287--- src/include/common/mir/input/android/event_conversion_helpers.h 2015-05-13 06:23:25 +0000
288+++ src/include/common/mir/input/android/event_conversion_helpers.h 2015-05-20 14:14:47 +0000
289@@ -31,13 +31,15 @@
290 int32_t android_modifiers_from_mir(MirInputEventModifiers modifiers);
291
292 MirKeyboardAction mir_keyboard_action_from_android(int32_t android_action, int32_t repeat_count);
293-
294 // Mir differentiates between mir_keyboard_action_down
295 // and mir_keyboard_action_repeat whereas android encodes
296 // keyrepeats as AKEY_EVENT_ACTION_DOWN and a repeatCount of > 0
297 // Thus when converting from MirKeyboardAction to an android
298 // action we must also fetch a repeat count for the android event.
299 int32_t android_keyboard_action_from_mir(int32_t& repeat_count_out, MirKeyboardAction action);
300+
301+MirPointerButtons mir_pointer_buttons_from_android(int32_t android_state);
302+int32_t android_pointer_buttons_from_mir(MirPointerButtons buttons);
303 }
304 }
305 }
306
307=== modified file 'src/server/input/android/android_input_dispatcher.cpp'
308--- src/server/input/android/android_input_dispatcher.cpp 2015-05-13 06:23:25 +0000
309+++ src/server/input/android/android_input_dispatcher.cpp 2015-05-20 14:14:47 +0000
310@@ -102,7 +102,7 @@
311 event.motion.action,
312 0, /* flags */
313 mia::android_modifiers_from_mir(event.motion.modifiers),
314- event.motion.button_state,
315+ mia::android_pointer_buttons_from_mir(event.motion.buttons),
316 0, /* edge_flags */
317 event.motion.pointer_count,
318 pointer_properties.data(),
319
320=== modified file 'src/server/input/android/input_sender.cpp'
321--- src/server/input/android/input_sender.cpp 2015-05-13 06:23:25 +0000
322+++ src/server/input/android/input_sender.cpp 2015-05-20 14:14:47 +0000
323@@ -288,7 +288,7 @@
324 0, /* flags */
325 0, /* edge flags */
326 mia::android_modifiers_from_mir(event.modifiers),
327- static_cast<int32_t>(event.button_state),
328+ mia::android_pointer_buttons_from_mir(event.buttons),
329 0.0f, // event.x_offset,
330 0.0f, // event.y_offset,
331 0, 0, /* unused x/y precision */
332
333=== modified file 'src/server/input/android/input_translator.cpp'
334--- src/server/input/android/input_translator.cpp 2015-05-13 06:23:25 +0000
335+++ src/server/input/android/input_translator.cpp 2015-05-20 14:14:47 +0000
336@@ -147,7 +147,7 @@
337 mir_event.motion.source_id = args->source;
338 mir_event.motion.action = args->action;
339 mir_event.motion.modifiers = mia::mir_modifiers_from_android(args->metaState);
340- mir_event.motion.button_state = static_cast<MirMotionButton>(args->buttonState);
341+ mir_event.motion.buttons = mia::mir_pointer_buttons_from_android(args->buttonState);
342 mir_event.motion.event_time = args->eventTime;
343 mir_event.motion.pointer_count = args->pointerCount;
344 for(unsigned int i = 0; i < args->pointerCount; i++)
345
346=== modified file 'tests/acceptance-tests/test_client_input.cpp'
347--- tests/acceptance-tests/test_client_input.cpp 2015-05-19 14:02:31 +0000
348+++ tests/acceptance-tests/test_client_input.cpp 2015-05-20 14:14:47 +0000
349@@ -242,6 +242,49 @@
350 first_client.all_events_received.wait_for_at_most_seconds(10);
351 }
352
353+TEST_F(TestClientInput, clients_receive_many_button_events_inside_window)
354+{
355+ Client first_client(new_connection(), first);
356+ // The cursor starts at (0, 0).
357+
358+ InSequence seq;
359+ auto expect_buttons = [&](MirPointerButtons b) {
360+ EXPECT_CALL(first_client, handle_input(mt::ButtonsDown(0, 0, b)));
361+ };
362+
363+ MirPointerButtons buttons = mir_pointer_button_primary;
364+ expect_buttons(buttons);
365+ expect_buttons(buttons |= mir_pointer_button_secondary);
366+ expect_buttons(buttons |= mir_pointer_button_tertiary);
367+ expect_buttons(buttons |= mir_pointer_button_forward);
368+ expect_buttons(buttons |= mir_pointer_button_back);
369+ expect_buttons(buttons &= ~mir_pointer_button_back);
370+ expect_buttons(buttons &= ~mir_pointer_button_forward);
371+ expect_buttons(buttons &= ~mir_pointer_button_tertiary);
372+ expect_buttons(buttons &= ~mir_pointer_button_secondary);
373+ EXPECT_CALL(first_client, handle_input(mt::ButtonsDown(0, 0, 0))).WillOnce(
374+ mt::WakeUp(&first_client.all_events_received));
375+
376+ auto press_button = [&](int button) {
377+ fake_mouse->emit_event(mis::a_button_down_event().of_button(button).with_action(mis::EventAction::Down));
378+ };
379+ auto release_button = [&](int button) {
380+ fake_mouse->emit_event(mis::a_button_up_event().of_button(button).with_action(mis::EventAction::Up));
381+ };
382+ press_button(BTN_LEFT);
383+ press_button(BTN_RIGHT);
384+ press_button(BTN_MIDDLE);
385+ press_button(BTN_FORWARD);
386+ press_button(BTN_BACK);
387+ release_button(BTN_BACK);
388+ release_button(BTN_FORWARD);
389+ release_button(BTN_MIDDLE);
390+ release_button(BTN_RIGHT);
391+ release_button(BTN_LEFT);
392+
393+ first_client.all_events_received.wait_for_at_most_seconds(10);
394+}
395+
396 TEST_F(TestClientInput, multiple_clients_receive_pointer_inside_windows)
397 {
398 int const screen_width = screen_geometry.size.width.as_int();
399
400=== modified file 'tests/acceptance-tests/test_surface_modifications.cpp'
401--- tests/acceptance-tests/test_surface_modifications.cpp 2015-05-13 06:23:25 +0000
402+++ tests/acceptance-tests/test_surface_modifications.cpp 2015-05-20 14:14:47 +0000
403@@ -93,7 +93,6 @@
404 void generate_alt_click_at(Point const& click_position)
405 {
406 auto const modifiers = mir_input_event_modifier_alt;
407- std::vector<MirPointerButton> depressed_buttons{mir_pointer_button_tertiary};
408
409 auto const x_axis_value = click_position.x.as_float();
410 auto const y_axis_value = click_position.y.as_float();
411@@ -102,7 +101,7 @@
412 auto const action = mir_pointer_action_button_down;
413
414 auto const click_event = mev::make_event(device_id, timestamp, modifiers,
415- action, depressed_buttons, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
416+ action, mir_pointer_button_tertiary, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
417
418 server.the_shell()->handle(*click_event);
419 }
420@@ -110,7 +109,6 @@
421 void generate_alt_move_to(Point const& drag_position)
422 {
423 auto const modifiers = mir_input_event_modifier_alt;
424- std::vector<MirPointerButton> depressed_buttons{mir_pointer_button_tertiary};
425
426 auto const x_axis_value = drag_position.x.as_float();
427 auto const y_axis_value = drag_position.y.as_float();
428@@ -119,7 +117,7 @@
429 auto const action = mir_pointer_action_motion;
430
431 auto const drag_event = mev::make_event(device_id, timestamp, modifiers,
432- action, depressed_buttons, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
433+ action, mir_pointer_button_tertiary, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
434
435 server.the_shell()->handle(*drag_event);
436 }
437
438=== modified file 'tests/acceptance-tests/test_surface_placement.cpp'
439--- tests/acceptance-tests/test_surface_placement.cpp 2015-05-13 06:23:25 +0000
440+++ tests/acceptance-tests/test_surface_placement.cpp 2015-05-20 14:14:47 +0000
441@@ -121,7 +121,7 @@
442 MirInputDeviceId const device_id{7};
443
444 auto const modifiers = mir_input_event_modifier_none;
445- std::vector<MirPointerButton> depressed_buttons{mir_pointer_button_primary};
446+ auto const depressed_buttons = mir_pointer_button_primary;
447
448 auto const x_axis_value = click_position.x.as_float();
449 auto const y_axis_value = click_position.y.as_float();
450
451=== modified file 'tests/include/mir_test/event_matchers.h'
452--- tests/include/mir_test/event_matchers.h 2015-05-19 14:02:31 +0000
453+++ tests/include/mir_test/event_matchers.h 2015-05-20 14:14:47 +0000
454@@ -135,10 +135,9 @@
455 return false;
456
457 if(mir_keyboard_event_modifiers(kev) != modifiers)
458- {
459- printf("modifiers: %d vs expected %d \n", mir_keyboard_event_modifiers(kev), modifiers);
460+ {
461 return false;
462- }
463+ }
464
465 return true;
466 }
467@@ -229,36 +228,38 @@
468 return false;
469 }
470
471+inline bool button_event_matches(MirPointerEvent const* pev, float x, float y, MirPointerAction action, MirPointerButtons button_state,
472+ bool check_action = true)
473+{
474+ if (pev == nullptr)
475+ return false;
476+ if (check_action && mir_pointer_event_action(pev) != action)
477+ return false;
478+ if (mir_pointer_event_buttons(pev) != button_state)
479+ return false;
480+ if (mir_pointer_event_axis_value(pev, mir_pointer_axis_x) != x)
481+ return false;
482+ if (mir_pointer_event_axis_value(pev, mir_pointer_axis_y) != y)
483+ return false;
484+ return true;
485+}
486+
487 MATCHER_P2(ButtonDownEvent, x, y, "")
488 {
489 auto pev = maybe_pointer_event(to_address(arg));
490- if (pev == nullptr)
491- return false;
492- if (mir_pointer_event_action(pev) != mir_pointer_action_button_down)
493- return false;
494- if (mir_pointer_event_button_state(pev, mir_pointer_button_primary) == false)
495- return false;
496- if (mir_pointer_event_axis_value(pev, mir_pointer_axis_x) != x)
497- return false;
498- if (mir_pointer_event_axis_value(pev, mir_pointer_axis_y) != y)
499- return false;
500- return true;
501+ return button_event_matches(pev, x, y, mir_pointer_action_button_down, mir_pointer_button_primary);
502 }
503
504 MATCHER_P2(ButtonUpEvent, x, y, "")
505 {
506 auto pev = maybe_pointer_event(to_address(arg));
507- if (pev == nullptr)
508- return false;
509- if (mir_pointer_event_action(pev) != mir_pointer_action_button_up)
510- return false;
511- if (mir_pointer_event_button_state(pev, mir_pointer_button_primary) == true)
512- return false;
513- if (mir_pointer_event_axis_value(pev, mir_pointer_axis_x) != x)
514- return false;
515- if (mir_pointer_event_axis_value(pev, mir_pointer_axis_y) != y)
516- return false;
517- return true;
518+ return button_event_matches(pev, x, y, mir_pointer_action_button_up, 0);
519+}
520+
521+MATCHER_P3(ButtonsDown, x, y, buttons, "")
522+{
523+ auto pev = maybe_pointer_event(to_address(arg));
524+ return button_event_matches(pev, x, y, mir_pointer_action_button_down, buttons, false);
525 }
526
527 MATCHER_P2(TouchEvent, x, y, "")
528
529=== modified file 'tests/mir_test_framework/fake_input_device_impl.cpp'
530--- tests/mir_test_framework/fake_input_device_impl.cpp 2015-05-19 14:02:31 +0000
531+++ tests/mir_test_framework/fake_input_device_impl.cpp 2015-05-20 14:14:47 +0000
532@@ -144,7 +144,7 @@
533
534 mtf::FakeInputDeviceImpl::InputDevice::InputDevice(mi::InputDeviceInfo const& info,
535 std::shared_ptr<mir::dispatch::Dispatchable> const& dispatchable)
536- : info(info), queue{dispatchable}
537+ : info(info), queue{dispatchable}, buttons{0}
538 {
539 }
540
541@@ -197,12 +197,12 @@
542 {
543 if (action == synthesis::EventAction::Down)
544 {
545- buttons.push_back(button);
546+ buttons |= button;
547 return mir_pointer_action_button_down;
548 }
549 else
550 {
551- buttons.erase(remove(begin(buttons), end(buttons), button));
552+ buttons &= ~button;
553 return mir_pointer_action_button_up;
554 }
555 }
556
557=== modified file 'tests/mir_test_framework/fake_input_device_impl.h'
558--- tests/mir_test_framework/fake_input_device_impl.h 2015-05-19 14:02:31 +0000
559+++ tests/mir_test_framework/fake_input_device_impl.h 2015-05-20 14:14:47 +0000
560@@ -25,8 +25,6 @@
561 #include "mir/input/input_device_info.h"
562 #include "mir/geometry/point.h"
563
564-#include <vector>
565-
566 namespace mir
567 {
568 namespace dispatch
569@@ -76,7 +74,7 @@
570 std::shared_ptr<mir::dispatch::Dispatchable> const queue;
571 uint32_t modifiers{0};
572 mir::geometry::Point pos, scroll;
573- std::vector<MirPointerButton> buttons;
574+ MirPointerButtons buttons;
575 };
576 std::shared_ptr<mir::dispatch::ActionQueue> queue;
577 std::shared_ptr<InputDevice> device;
578
579=== modified file 'tests/unit-tests/input/android/test_android_input_dispatcher.cpp'
580--- tests/unit-tests/input/android/test_android_input_dispatcher.cpp 2015-05-13 06:23:25 +0000
581+++ tests/unit-tests/input/android/test_android_input_dispatcher.cpp 2015-05-20 14:14:47 +0000
582@@ -173,15 +173,16 @@
583 std::memset(expected_coords, 0, sizeof(expected_coords));
584 std::memset(expected_properties, 0, sizeof(expected_properties));
585 MirEvent event;
586+ std::memset(&event, 0, sizeof(event));
587+
588 event.type = mir_event_type_motion;
589 event.motion.pointer_count = 1;
590 event.motion.event_time = std::chrono::nanoseconds(2);
591 event.motion.device_id = 3;
592 event.motion.source_id = 4;
593 event.motion.action = mir_motion_action_scroll;
594- event.motion.modifiers = mir_input_event_modifier_shift,
595- event.motion.button_state =
596- static_cast<MirMotionButton>(mir_motion_button_forward | mir_motion_button_secondary);
597+ event.motion.modifiers = mir_input_event_modifier_shift;
598+ event.motion.buttons = mir_pointer_button_forward | mir_pointer_button_secondary;
599
600 auto & pointer = event.motion.pointer_coordinates[0];
601 pointer.id = 1;
602@@ -215,7 +216,7 @@
603 event.motion.action,
604 0, /* flags */
605 AMETA_SHIFT_ON,
606- event.motion.button_state,
607+ AMOTION_EVENT_BUTTON_FORWARD | AMOTION_EVENT_BUTTON_SECONDARY,
608 0, /* edge_flags */
609 event.motion.pointer_count,
610 expected_properties,
611
612=== modified file 'tests/unit-tests/input/android/test_android_input_lexicon.cpp'
613--- tests/unit-tests/input/android/test_android_input_lexicon.cpp 2015-05-13 06:23:25 +0000
614+++ tests/unit-tests/input/android/test_android_input_lexicon.cpp 2015-05-20 14:14:47 +0000
615@@ -78,7 +78,7 @@
616 const int32_t flags = 4;
617 const int32_t edge_flags = 5;
618 const int32_t meta_state = 6;
619- const int32_t button_state = 7;
620+ const int32_t button_state = 0;
621 const float x_offset = 8;
622 const float y_offset = 9;
623 const float x_precision = 10;
624@@ -134,7 +134,6 @@
625
626 auto mir_motion_ev = &mir_ev.motion;
627
628- EXPECT_EQ(mir_motion_ev->button_state, button_state);
629 EXPECT_EQ(mir_motion_ev->event_time, event_time);
630
631 EXPECT_EQ(mir_motion_ev->pointer_count, pointer_count);
632@@ -170,7 +169,6 @@
633 const int32_t flags = 4;
634 const int32_t edge_flags = 5;
635 const int32_t meta_state = 6;
636- const int32_t button_state = 7;
637 const float x_offset = 8;
638 const float y_offset = 9;
639 const float x_precision = 10;
640@@ -223,7 +221,7 @@
641 }
642
643 android_motion_ev->initialize(device_id, source_id, action, flags,
644- edge_flags, meta_state, button_state,
645+ edge_flags, meta_state, 0,
646 x_offset, y_offset, x_precision, y_precision,
647 down_time, event_time, pointer_count,
648 pointer_properties, pointer_coords);
649@@ -241,7 +239,6 @@
650
651 auto mir_motion_ev = &mir_ev.motion;
652
653- EXPECT_EQ(mir_motion_ev->button_state, button_state);
654 EXPECT_EQ(mir_motion_ev->event_time, event_time);
655 EXPECT_EQ(mir_motion_ev->pointer_count, pointer_count);
656
657
658=== modified file 'tests/unit-tests/input/android/test_input_translator.cpp'
659--- tests/unit-tests/input/android/test_input_translator.cpp 2015-05-13 06:23:25 +0000
660+++ tests/unit-tests/input/android/test_input_translator.cpp 2015-05-20 14:14:47 +0000
661@@ -266,8 +266,7 @@
662 expected.motion.source_id = 4;
663 expected.motion.action = mir_motion_action_scroll;
664 expected.motion.modifiers = 6;
665- expected.motion.button_state =
666- static_cast<MirMotionButton>(mir_motion_button_forward | mir_motion_button_secondary);
667+ expected.motion.buttons = mir_pointer_button_forward | mir_pointer_button_secondary;
668
669 auto & pointer = expected.motion.pointer_coordinates[0];
670 pointer.id = 1;
671@@ -303,7 +302,7 @@
672 expected.motion.action,
673 0, /* flags */
674 expected.motion.modifiers,
675- expected.motion.button_state,
676+ AMOTION_EVENT_BUTTON_FORWARD | AMOTION_EVENT_BUTTON_SECONDARY,
677 0, /* edge flags */
678 expected.motion.pointer_count,
679 properties,
680
681=== modified file 'tests/unit-tests/input/test_default_input_device_hub.cpp'
682--- tests/unit-tests/input/test_default_input_device_hub.cpp 2015-05-19 14:28:28 +0000
683+++ tests/unit-tests/input/test_default_input_device_hub.cpp 2015-05-20 14:14:47 +0000
684@@ -368,9 +368,9 @@
685 using namespace ::testing;
686
687 auto x = 12.2f, y = 14.3f;
688- std::vector<MirPointerButton> none_pressed;
689- auto event = mir::events::make_event(0, 0ns, mir_input_event_modifier_none, mir_pointer_action_motion, none_pressed,
690- x, y, 0.0f, 0.0f);
691+
692+ auto event = mir::events::make_event(0, 0ns, mir_input_event_modifier_none, mir_pointer_action_motion, 0,
693+ x, y, 0.0f, 0.0f);
694
695 EXPECT_CALL(mock_cursor_listener, cursor_moved_to(x, y)).Times(1);
696
697
698=== modified file 'tests/unit-tests/input/test_event_builders.cpp'
699--- tests/unit-tests/input/test_event_builders.cpp 2015-05-19 14:02:31 +0000
700+++ tests/unit-tests/input/test_event_builders.cpp 2015-05-20 14:14:47 +0000
701@@ -99,8 +99,7 @@
702 TEST_F(InputEventBuilder, makes_valid_pointer_event)
703 {
704 MirPointerAction action = mir_pointer_action_enter;
705- std::vector<MirPointerButton> depressed_buttons =
706- {mir_pointer_button_back, mir_pointer_button_tertiary};
707+ auto depressed_buttons = mir_pointer_button_back | mir_pointer_button_tertiary;
708 float x_axis_value = 3.9, y_axis_value = 7.4, hscroll_value = .9, vscroll_value = .3;
709 auto ev = mev::make_event(device_id, timestamp, modifiers,
710 action, depressed_buttons, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
711@@ -162,11 +161,10 @@
712
713 TEST_F(InputEventBuilder, map_to_hover_if_no_button_pressed)
714 {
715- std::vector<MirPointerButton> no_pressed_buttons;
716 float x_axis_value = 3.9, y_axis_value = 7.4, hscroll_value = .9, vscroll_value = .3;
717 MirPointerAction action = mir_pointer_action_motion;
718 auto ev = mev::make_event(device_id, timestamp, modifiers,
719- action, no_pressed_buttons, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
720+ action, 0, x_axis_value, y_axis_value, hscroll_value, vscroll_value);
721 auto e = ev.get();
722
723 auto ie = mir_event_get_input_event(e);
724
725=== modified file 'tests/unit-tests/input/test_input_event.cpp'
726--- tests/unit-tests/input/test_input_event.cpp 2015-05-13 06:23:25 +0000
727+++ tests/unit-tests/input/test_input_event.cpp 2015-05-20 14:14:47 +0000
728@@ -61,15 +61,21 @@
729 MirEvent a_key_ev()
730 {
731 MirEvent key_ev;
732+ memset(&key_ev, 0, sizeof(key_ev));
733+
734 key_ev.type = mir_event_type_key;
735+
736 return key_ev;
737 }
738
739 MirEvent a_motion_ev(int device_class = AINPUT_SOURCE_UNKNOWN)
740 {
741 MirEvent motion_ev;
742+ memset(&motion_ev, 0, sizeof(motion_ev));
743+
744 motion_ev.type = mir_event_type_motion;
745 motion_ev.motion.source_id = device_class;
746+
747 return motion_ev;
748 }
749
750@@ -350,13 +356,13 @@
751 {
752 auto old_ev = a_motion_ev(AINPUT_SOURCE_MOUSE);
753
754- old_ev.motion.button_state = mir_motion_button_primary;
755+ old_ev.motion.buttons = mir_pointer_button_primary;
756 auto pev = mir_input_event_get_pointer_event(mir_event_get_input_event(&old_ev));
757
758 EXPECT_TRUE(mir_pointer_event_button_state(pev, mir_pointer_button_primary));
759 EXPECT_FALSE(mir_pointer_event_button_state(pev, mir_pointer_button_secondary));
760
761- old_ev.motion.button_state = static_cast<MirMotionButton>(old_ev.motion.button_state | (mir_motion_button_secondary));
762+ old_ev.motion.buttons |= mir_pointer_button_secondary;
763
764 EXPECT_TRUE(mir_pointer_event_button_state(pev, mir_pointer_button_primary));
765 EXPECT_TRUE(mir_pointer_event_button_state(pev, mir_pointer_button_secondary));
766
767=== modified file 'tests/unit-tests/scene/test_abstract_shell.cpp'
768--- tests/unit-tests/scene/test_abstract_shell.cpp 2015-05-13 06:23:25 +0000
769+++ tests/unit-tests/scene/test_abstract_shell.cpp 2015-05-20 14:14:47 +0000
770@@ -283,7 +283,7 @@
771 {
772 MirInputEventModifiers const modifiers{mir_input_event_modifier_none};
773 MirPointerAction const action{mir_pointer_action_button_down};
774- std::vector<MirPointerButton> const buttons_pressed{mir_pointer_button_primary};
775+ auto const buttons_pressed = mir_pointer_button_primary;
776 float const x_axis_value{0.0};
777 float const y_axis_value{0.0};
778 float const hscroll_value{0.0};

Subscribers

People subscribed via source and target branches