Merge lp:~alan-griffiths/mir/event-timestamps into lp:mir

[Alan Griffiths]

client API: provide access to timestamps from the various input event types

client API: provide access to timestamps from the various input event types

[Robert Carr (robertcarr)]

+1

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2699
http://jenkins.qa.ubuntu.com/job/mir-ci/4190/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2982
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/499/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2930/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/346/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2930/console

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

review: Needs Fixing (continuous-integration)

[Andreas Pokorny (andreas-pokorny)]

*Discussion Needed*
The input_event 'method name' is mir_input_event_get_time. The derived structures do not use '_get_' for getters. But why not stick with time instead of timestamp? That would mean 'mir_keyboard_event_time'.

Libinput has a similar thing with base and specific events and there the solution is to provide a libinput_{specific_event}_get_base_event function.

review: Needs Information

[Alexandros Frantzis (afrantzis)]

> The input_event 'method name' is mir_input_event_get_time. The derived structures
> do not use '_get_' for getters

Yes, this is an unfortunate inconsistency.

> Libinput has a similar thing with base and specific events and there the solution is to
> provide a libinput_{specific_event}_get_base_event function.

+1 for this solution. In addition to the event time, there is also the device id that's a common property of all input events. Having a way to get the base input event from any derived input event would elegantly allow access to both these properties (and any new ones we may add in the future).

review: Needs Information

[Alan Griffiths (alan-griffiths)]

> *Discussion Needed*
> The input_event 'method name' is mir_input_event_get_time. The derived
> structures do not use '_get_' for getters. But why not stick with time instead
> of timestamp? That would mean 'mir_keyboard_event_time'.
>
> Libinput has a similar thing with base and specific events and there the
> solution is to provide a libinput_{specific_event}_get_base_event function.

I considered that, but felt the code I wanted to write (in qtmir) would be clearer with this approach.

Vis:

    auto const timestamp = mir_keyboard_event_timestamp(kev);

as opposed to:

    auto const input_event = mir_keyboard_event_get_base_event(kev);
    auto const timestamp = mir_input_event_get_time(input_event);

or, more consistently:

    auto const input_event = mir_keyboard_event_input_event(kev);
    auto const timestamp = mir_input_event_get_time(input_event);

But as Alexandros mentions this suggestion would avoid writing 3*n (trivially different) functions where 3 would suffice.

In any case I have a mild preference for this approach but will conform to the majority view.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2701
http://jenkins.qa.ubuntu.com/job/mir-ci/4197/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2991
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/508
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2939
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/353
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/353/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2939
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2939/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5758
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21473

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

review: Approve (continuous-integration)

=== modified file 'include/client/mir_toolkit/events/input/keyboard_event.h'
--- include/client/mir_toolkit/events/input/keyboard_event.h	2015-04-28 07:54:10 +0000
+++ include/client/mir_toolkit/events/input/keyboard_event.h	2015-06-25 09:33:08 +0000
@@ -90,6 +90,14 @@
  */
 MirInputEventModifiers mir_keyboard_event_modifiers(MirKeyboardEvent const* event);
 
+/*
+ * Retrieve the time at which a keyboard event occurred.
+ *
+ * \param [in] event The keyboard event
+ * \return           A timestamp in nanoseconds-since-epoch
+ */
+int64_t mir_keyboard_event_timestamp(MirKeyboardEvent const* event);
+
 #ifdef __cplusplus
 }
 /**@}*/

=== modified file 'include/client/mir_toolkit/events/input/pointer_event.h'
--- include/client/mir_toolkit/events/input/pointer_event.h	2015-06-17 05:20:42 +0000
+++ include/client/mir_toolkit/events/input/pointer_event.h	2015-06-25 09:33:08 +0000
@@ -122,6 +122,14 @@
 float mir_pointer_event_axis_value(MirPointerEvent const* event, 
     MirPointerAxis axis);
 
+/*
+ * Retrieve the time at which a pointer event occurred.
+ *
+ * \param [in] event The pointer event
+ * \return           A timestamp in nanoseconds-since-epoch
+ */
+int64_t mir_pointer_event_timestamp(MirPointerEvent const* event);
+
 #ifdef __cplusplus
 }
 /**@}*/

=== modified file 'include/client/mir_toolkit/events/input/touch_event.h'
--- include/client/mir_toolkit/events/input/touch_event.h	2015-04-28 07:54:10 +0000
+++ include/client/mir_toolkit/events/input/touch_event.h	2015-06-25 09:33:08 +0000
@@ -141,6 +141,14 @@
 float mir_touch_event_axis_value(MirTouchEvent const* event, 
     size_t touch_index, MirTouchAxis axis);
 
+/*
+ * Retrieve the time at which a touch event occurred.
+ *
+ * \param [in] event The touch event
+ * \return           A timestamp in nanoseconds-since-epoch
+ */
+int64_t mir_touch_event_timestamp(MirTouchEvent const* event);
+
 #ifdef __cplusplus
 }
 /**@}*/

=== modified file 'src/client/input/input_event.cpp'
--- src/client/input/input_event.cpp	2015-06-17 05:20:42 +0000
+++ src/client/input/input_event.cpp	2015-06-25 09:33:08 +0000
@@ -187,6 +187,24 @@
     }
 }
 
+int64_t mir_pointer_event_timestamp(MirPointerEvent const* event)
+{
+    auto const& old_ev = old_mev_from_new(event);
+    return old_ev.event_time.count();
+}
+
+int64_t mir_keyboard_event_timestamp(MirKeyboardEvent const* event)
+{
+    auto const& old_ev = old_kev_from_new(event);
+    return old_ev.event_time.count();
+}
+
+int64_t mir_touch_event_timestamp(MirTouchEvent const* event)
+{
+    auto const& old_ev = old_mev_from_new(event);
+    return old_ev.event_time.count();
+}
+
 /* Key event accessors */
 
 MirKeyboardEvent const* mir_input_event_get_keyboard_event(MirInputEvent const* ev)

=== modified file 'src/client/symbols.map'
--- src/client/symbols.map	2015-06-17 14:47:11 +0000
+++ src/client/symbols.map	2015-06-25 09:33:08 +0000
@@ -73,6 +73,7 @@
     mir_keyboard_event_key_code;
     mir_keyboard_event_modifiers;
     mir_keyboard_event_scan_code;
+    mir_keyboard_event_timestamp;
     mir_keymap_event_get_rules;
     mir_omnidirectional_resize_cursor_name;
     mir_open_hand_cursor_name;
@@ -91,6 +92,7 @@
     mir_pointer_event_buttons;
     mir_pointer_event_button_state;
     mir_pointer_event_modifiers;
+    mir_pointer_event_timestamp;
     mir_pointing_hand_cursor_name;
     mir_prompt_session_error_message;
     mir_prompt_session_event_get_state;
@@ -152,6 +154,7 @@
     mir_touch_event_id;
     mir_touch_event_modifiers;
     mir_touch_event_point_count;
+    mir_touch_event_timestamp;
     mir_touch_event_tooltype;
     mir_vertical_resize_cursor_name;
     mir_vsplit_resize_cursor_name;

=== modified file 'tests/unit-tests/input/test_input_event.cpp'
--- tests/unit-tests/input/test_input_event.cpp	2015-06-17 05:20:42 +0000
+++ tests/unit-tests/input/test_input_event.cpp	2015-06-25 09:33:08 +0000
@@ -21,6 +21,8 @@
 #include "mir/events/event_private.h"
 #include "mir_toolkit/events/input/input_event.h"
 
+using namespace testing;
+
 // See: https://bugs.launchpad.net/mir/+bug/1311699
 #define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
 #define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;
@@ -127,6 +129,22 @@
         mir_event_get_input_event(&old_ev)));
 }
 
+TEST(KeyInputEventProperties, timestamp_taken_from_old_style_event)
+{
+    std::chrono::nanoseconds event_time_1{79}, event_time_2{83};
+    auto old_ev = a_key_ev();
+
+    for (auto expected : {event_time_1, event_time_2})
+    {
+        old_ev.key.event_time = expected;
+
+        auto const input_event = mir_event_get_input_event(&old_ev);
+        auto const keyboard_event = mir_input_event_get_keyboard_event(input_event);
+
+        EXPECT_THAT(mir_keyboard_event_timestamp(keyboard_event), Eq(expected.count()));
+    }
+}
+
 TEST(KeyInputEventProperties, up_and_down_actions_copied_from_old_style_event)
 {
     auto old_ev = a_key_ev();
@@ -157,6 +175,22 @@
     EXPECT_EQ(modifiers, mir_keyboard_event_modifiers(new_kev));
 }
 
+TEST(TouchEventProperties, timestamp_taken_from_old_style_event)
+{
+    std::chrono::nanoseconds event_time_1{79}, event_time_2{83};
+    auto old_ev = a_motion_ev(AINPUT_SOURCE_TOUCHSCREEN);
+
+    for (auto expected : {event_time_1, event_time_2})
+    {
+        old_ev.motion.event_time = expected;
+
+        auto const input_event = mir_event_get_input_event(&old_ev);
+        auto const touch_event = mir_input_event_get_touch_event(input_event);
+
+        EXPECT_THAT(mir_touch_event_timestamp(touch_event), Eq(expected.count()));
+    }
+}
+
 TEST(TouchEventProperties, touch_count_taken_from_pointer_count)
 {
     unsigned const pointer_count = 3;
@@ -270,6 +304,22 @@
 
 /* Pointer event property accessors */
 
+TEST(PointerInputEventProperties, timestamp_taken_from_old_style_event)
+{
+    std::chrono::nanoseconds event_time_1{79}, event_time_2{83};
+    auto old_ev = a_motion_ev(AINPUT_SOURCE_MOUSE);
+
+    for (auto expected : {event_time_1, event_time_2})
+    {
+        old_ev.motion.event_time = expected;
+
+        auto const input_event = mir_event_get_input_event(&old_ev);
+        auto const pointer_event = mir_input_event_get_pointer_event(input_event);
+
+        EXPECT_THAT(mir_pointer_event_timestamp(pointer_event), Eq(expected.count()));
+    }
+}
+
 TEST(PointerInputEventProperties, modifiers_taken_from_old_style_ev)
 {
     MirInputEventModifiers modifiers = mir_input_event_modifier_shift;