Mir

Merge lp:~robertcarr/mir/add-event-action-defines into lp:~mir-team/mir/trunk

Proposed by Robert Carr
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 625
Proposed branch: lp:~robertcarr/mir/add-event-action-defines
Merge into: lp:~mir-team/mir/trunk
Diff against target: 84 lines (+25/-5)
5 files modified
examples/eglapp.c (+1/-1)
include/shared/mir_toolkit/event.h (+20/-0)
src/client/input/android_input_receiver.cpp (+1/-1)
tests/acceptance-tests/test_client_input.cpp (+1/-1)
tests/unit-tests/client/input/test_android_input_receiver.cpp (+2/-2)
To merge this branch: bzr merge lp:~robertcarr/mir/add-event-action-defines
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Robert Ancell Approve
PS Jenkins bot (community) continuous-integration Approve
Kevin DuBois (community) Approve
Review via email: mp+160224@code.launchpad.net

Commit message

Add definition for Motion and Key event actions.

Description of the change

Add definitions for event action defines so toolkit porters (i.e. bschaefer working working on porting SDL :)) do not have to guess.

To post a comment you must log in.
Revision history for this message
Kevin DuBois (kdub) wrote :

well, seems more orderly, I'll +1.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

1. Missed a spot :)
src/client/input/android_input_receiver.cpp: if (ev.key.action == 1)

2. Trying to avoid identifiers getting too long, can we change:
   mir_key_event_action_*
   mir_motion_event_action_*
to
   mir_key_action_*
   mir_motion_action_*
?

3. Can we use typedefs instead of anonymous enums? That would allow us to change:
   int32_t action;
to:
   MirKeyAction action;
   MirMotionAction action;
etc.

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

Thanks for review!

>> 1. Missed a spot :)
>> src/client/input/android_input_receiver.cpp: if (ev.key.action == 1)

Good catch. Fixed

>>2. Trying to avoid identifiers getting too long, can we change:
>> mir_key_event_action_*
>> mir_motion_event_action_*
>>to
>> mir_key_action_*
>> mir_motion_action_*
>>?

Yeah!

>> 3. Can we use typedefs instead of anonymous enums? That would allow us to change:
>> int32_t action;
>> to:
>> MirKeyAction action;
>> MirMotionAction action;
>> etc.

I don't have a strong opinion. As it stands though each field is chosen to be the same as the platform-api event struct, there is a bit of a mess there (currently they are out of sync requiring expensive marshalling in the platform-API). Anyway the moral is that I'd like to discuss redesigning and solidifying the event struct next week and delay any changes until then.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

+1 on using a typedef, otherwise good.

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

Looks good.

The typedefs can come gradually later. It's an evolution.

Also, I don't understand what this is: "mir_key_action_multiple", but don't really care.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'examples/eglapp.c'
--- examples/eglapp.c 2013-04-22 08:20:31 +0000
+++ examples/eglapp.c 2013-04-23 17:17:30 +0000
@@ -90,7 +90,7 @@
90{90{
91 (void) surface;91 (void) surface;
92 (void) context;92 (void) context;
93 if (ev->key.key_code == XKB_KEY_q)93 if (ev->key.key_code == XKB_KEY_q && ev->key.action == mir_key_action_up)
94 running = 0;94 running = 0;
95}95}
9696
9797
=== modified file 'include/shared/mir_toolkit/event.h'
--- include/shared/mir_toolkit/event.h 2013-04-11 22:47:09 +0000
+++ include/shared/mir_toolkit/event.h 2013-04-23 17:17:30 +0000
@@ -40,6 +40,26 @@
40 mir_event_type_motion40 mir_event_type_motion
41} MirEventType;41} MirEventType;
4242
43enum {
44 mir_key_action_down = 0,
45 mir_key_action_up = 1,
46 mir_key_action_multiple = 2
47};
48
49enum {
50 mir_motion_action_down = 0,
51 mir_motion_action_up = 1,
52 mir_motion_action_move = 2,
53 mir_motion_action_cancel = 3,
54 mir_motion_action_outside = 4,
55 mir_motion_action_pointer_down = 5,
56 mir_motion_action_pointer_up = 6,
57 mir_motion_action_hover_move = 7,
58 mir_motion_action_scroll = 8,
59 mir_motion_action_hover_enter = 9,
60 mir_motion_action_hover_exit = 10
61};
62
43typedef struct63typedef struct
44{64{
45 MirEventType type;65 MirEventType type;
4666
=== modified file 'src/client/input/android_input_receiver.cpp'
--- src/client/input/android_input_receiver.cpp 2013-04-16 09:27:00 +0000
+++ src/client/input/android_input_receiver.cpp 2013-04-23 17:17:30 +0000
@@ -66,7 +66,7 @@
66 if (ev.type != mir_event_type_key)66 if (ev.type != mir_event_type_key)
67 return;67 return;
68 68
69 if (ev.key.action == 1)69 if (ev.key.action == mir_key_action_up)
70 ev.key.key_code = xkb_mapper->release_and_map_key(ev.key.scan_code);70 ev.key.key_code = xkb_mapper->release_and_map_key(ev.key.scan_code);
71 else 71 else
72 ev.key.key_code = xkb_mapper->press_and_map_key(ev.key.scan_code);72 ev.key.key_code = xkb_mapper->press_and_map_key(ev.key.scan_code);
7373
=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp 2013-04-18 17:22:34 +0000
+++ tests/acceptance-tests/test_client_input.cpp 2013-04-23 17:17:30 +0000
@@ -200,7 +200,7 @@
200 static void handle_input(MirSurface* /* surface */, MirEvent const* ev, void* context)200 static void handle_input(MirSurface* /* surface */, MirEvent const* ev, void* context)
201 {201 {
202 auto client = static_cast<InputReceivingClient *>(context);202 auto client = static_cast<InputReceivingClient *>(context);
203 if (ev->key.action == 0)203 if (ev->key.action == mir_key_action_down)
204 {204 {
205 client->handler->handle_key_down(ev);205 client->handler->handle_key_down(ev);
206 client->event_received[client->events_received].wake_up_everyone();206 client->event_received[client->events_received].wake_up_everyone();
207207
=== modified file 'tests/unit-tests/client/input/test_android_input_receiver.cpp'
--- tests/unit-tests/client/input/test_android_input_receiver.cpp 2013-04-11 19:40:14 +0000
+++ tests/unit-tests/client/input/test_android_input_receiver.cpp 2013-04-23 17:17:30 +0000
@@ -104,8 +104,8 @@
104 // Some default values104 // Some default values
105 // device_id must be > 0 or input publisher will reject105 // device_id must be > 0 or input publisher will reject
106 static const int32_t filler_device_id = 1;106 static const int32_t filler_device_id = 1;
107 // AMOTION_EVENT_ACTION_MOVE is necessary to engage batching behavior107 // event_action_move is necessary to engage batching behavior
108 static const int32_t motion_event_action_flags = AMOTION_EVENT_ACTION_MOVE;108 static const int32_t motion_event_action_flags = mir_motion_action_move;
109 // We have to have at least 1 pointer or the publisher will fail to marshal a motion event109 // We have to have at least 1 pointer or the publisher will fail to marshal a motion event
110 static const int32_t default_pointer_count = 1;110 static const int32_t default_pointer_count = 1;
111};111};

Subscribers

People subscribed via source and target branches