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

[Robert Carr]

Add definition for Motion and Key event actions.

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

[Kevin DuBois (kdub)]

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

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:590
http://jenkins.qa.ubuntu.com/job/mir-ci/472/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/502
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/385

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

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

[Robert Carr (robertcarr)]

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.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:591
http://jenkins.qa.ubuntu.com/job/mir-ci/483/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/519
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/402
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/488
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/488/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/236

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

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

+1 on using a typedef, otherwise good.

review: Approve

[Daniel van Vugt (vanvugt)]

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

=== 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 @@
 {
     (void) surface;
     (void) context;
-    if (ev->key.key_code == XKB_KEY_q)
+    if (ev->key.key_code == XKB_KEY_q && ev->key.action == mir_key_action_up)
         running = 0;
 }
 

=== 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 @@
     mir_event_type_motion
 } MirEventType;
 
+enum {
+    mir_key_action_down = 0,
+    mir_key_action_up = 1,
+    mir_key_action_multiple = 2
+};
+
+enum {
+    mir_motion_action_down = 0,
+    mir_motion_action_up = 1,
+    mir_motion_action_move = 2,
+    mir_motion_action_cancel = 3,
+    mir_motion_action_outside = 4,
+    mir_motion_action_pointer_down = 5,
+    mir_motion_action_pointer_up = 6,
+    mir_motion_action_hover_move = 7,
+    mir_motion_action_scroll = 8,
+    mir_motion_action_hover_enter = 9,
+    mir_motion_action_hover_exit = 10
+};
+
 typedef struct
 {
     MirEventType type;

=== 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 @@
     if (ev.type != mir_event_type_key)
         return;
     
-    if (ev.key.action == 1)
+    if (ev.key.action == mir_key_action_up)
         ev.key.key_code = xkb_mapper->release_and_map_key(ev.key.scan_code);
     else 
         ev.key.key_code = xkb_mapper->press_and_map_key(ev.key.scan_code);

=== 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 @@
     static void handle_input(MirSurface* /* surface */, MirEvent const* ev, void* context)
     {
         auto client = static_cast<InputReceivingClient *>(context);
-        if (ev->key.action == 0)
+        if (ev->key.action == mir_key_action_down)
         {
             client->handler->handle_key_down(ev);
             client->event_received[client->events_received].wake_up_everyone();

=== 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 @@
     // Some default values
     // device_id must be > 0 or input publisher will reject
     static const int32_t filler_device_id = 1;
-    // AMOTION_EVENT_ACTION_MOVE is necessary to engage batching behavior
-    static const int32_t motion_event_action_flags = AMOTION_EVENT_ACTION_MOVE;
+    // event_action_move is necessary to engage batching behavior
+    static const int32_t motion_event_action_flags = mir_motion_action_move;
     // We have to have at least 1 pointer or the publisher will fail to marshal a motion event
     static const int32_t default_pointer_count = 1;
 };