Mir

Merge lp:~mir-team/mir/introduce-mir-event-2.0 into lp:mir

introduce-mir-event-2.0
Merge into development-branch

Proposed by Robert Carr on 2014-11-21

Status:

Merged

Approved by:

Robert Carr on 2014-12-08

Approved revision:

no longer in the source branch.

Merged at revision:

2129

Proposed branch:

lp:~mir-team/mir/introduce-mir-event-2.0

Merge into:

lp:mir

Diff against target:

1174 lines (+963/-22)

14 files modified

client-ABI-sha1sums (+4/-1)
common-ABI-sha1sums (+4/-1)
examples/fingerpaint.c (+23/-16)
include/common/mir_toolkit/event.h (+22/-1)
include/common/mir_toolkit/input/input_event.h (+101/-0)
include/common/mir_toolkit/input/key_input_event.h (+113/-0)
include/common/mir_toolkit/input/touch_input_event.h (+141/-0)
platform-ABI-sha1sums (+4/-1)
server-ABI-sha1sums (+4/-1)
src/common/input/CMakeLists.txt (+1/-0)
src/common/input/input_event.cpp (+330/-0)
src/common/symbols.map (+1/-1)
tests/unit-tests/input/CMakeLists.txt (+1/-0)
tests/unit-tests/input/test_input_event.cpp (+214/-0)

To merge this branch:

bzr merge lp:~mir-team/mir/introduce-mir-event-2.0

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Chris Halse Rogers		2014-11-21	Approve on 2014-12-08
Alberto Aguirre (community)			Needs Information on 2014-12-05
PS Jenkins bot (community)	continuous-integration		Approve on 2014-12-03
Alexandros Frantzis (community)			Approve on 2014-12-02
Andreas Pokorny (community)			Approve on 2014-12-02
Cemil Azizoglu (community)			Approve on 2014-12-01
Review via email: mp+242443@code.launchpad.net

Commit message

Introduce a transition path to MirEvent 2.0, henceforth MirInputEvent.

Direct access to MirEvent::type is deprecated, and a new method mir_event_get_type is encouraged. For what was mir_event_type_key/motion a new type mir_event_type_input will be returned. Client code may call mir_event_get_input_event(...) to retreive a "MirInputEvent*" a.k.a. MirEvent 2.0.

Description of the change

Introduce an API/ABI preserving transition path to MirEvent 2.0, henceforth MirInputEvent.

The MirInputEvent API is of course incomplete, but I believe what is in this proposal has at least these two qualities:
1. Contains everything needed by current down streams.
2. Is ABI/API compatible with the 'final form' of MirInputEvent.

I'd like to transition as follows:
1. Iterate on names, documentation, etc, then land this branch.
2. Port downstream and internal MirEvent usage
3. Remove MirMotionEvent and MirKeyEvent

A quick guide to the branch contents:
include/common/mir_toolkit/input/* : New InputEvent API
tests/unit-tests/input/test_input_event.cpp : Test of translations from MirEvent to MirInputEvent, I would say the only surprising one involves the pointer action masking.
src/common/input/input_event.cpp: Impl of API
examples/fingerpaint.cpp: Port fingerpaint to new API
include/mir_toolkit/event.h: Add pointer action masks (ala https://bugs.launchpad.net/mir/+bug/1311699) to support the MirInputEvent translation.
src/common/symbols.map : Export the C symbols

Thanks!
Robert

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-21:

FAILED: Continuous integration, rev:2080
http://jenkins.qa.ubuntu.com/job/mir-ci/2192/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/252
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/252/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/226/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/181/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/226/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-11-21:

179 +MirInputEvent* mir_event_get_input_event(MirEvent* ev);

Const? Why is the MirEvent and MirInputEvent mutable?

---

229 +typedef struct _MirInputEvent MirInputEvent;
344 +typedef struct _MirKeyInputEvent MirKeyInputEvent;
460 +typedef struct _MirTouchInputEvent MirTouchInputEvent;

We've traditionally not used the typedef struct _Foo Foo idiom. Any particular reason to not make this “typedef struct Mir*InputEvent Mir*InputEvent”?

---

271 +MirInputEventTime mir_input_event_get_event_time(MirInputEvent const* ev);

Is there any particular reason to typedef int64_t to MirInputEventTime here? We're documenting it's nanoseconds-since-the-epoch, and this is not really useful if the type is opaque.

---

467 +typedef int64_t MirTouchInputEventTouchId;

We'll probably not have a full 2^63 touch ids at any point; int32_t maybe? :)

---

479 + /* This touch point was cancelled, an down should have been
480 + received but an up will never come. */
481 + mir_touch_input_event_action_cancel = 3,

Comment seems a bit off, and not just the an/down mismatch :)
Should mention that this is opt-in behaviour, “a down should have been received” is a bit wishy-washy.

“This touch point was cancelled. Any actions taken in response to this touch should be undone. No further events will be delivered for this touch point.

A cancelled event will only be sent to a client that has explicitly enabled cancellation, via mir_input_break_me_harder()”

Maybe?

---

482 + /* This touch point is unchanged but is reported as part of
483 + a full state update. */
484 + mir_touch_input_event_action_none = 4

Why do we have this? Is there an ability to request a full-state update?

---

518 +// Touch is made with a mouse through touch emulation
519 + mir_touch_input_tool_type_mouse = 3,

Are we going to do this?

---

549 +MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index);

Hm. Would it be nice for clients if instead of get_touch_id(..., index) and get_*(..., index) we just had.
get_*(MirTouchInputEvent const* event, MirTouchInputEventTouchId id)?

That would seem be a more natural API to me, but we'd need toolkit input as to whether or not that's better.

---

655 + default:
656 + return MirInputEventType();

I'm a fan of abort() when faced with egregious client API abuse. Likewise for the DeviceId just afterwards.

---

761 +MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index)

Oooh, ugh. Maybe we need an actual MirTouchInputEvent to pass in here, so that we can maintain the state that lets us implement this properly?

179	+MirInputEvent* mir_event_get_input_event(MirEvent* ev);

Const? Why is the MirEvent and MirInputEvent mutable?

---

229	+typedef struct _MirInputEvent MirInputEvent;
344	+typedef struct _MirKeyInputEvent MirKeyInputEvent;
460	+typedef struct _MirTouchInputEvent MirTouchInputEvent;

We've traditionally not used the typedef struct _Foo Foo idiom. Any particular reason to not make this “typedef struct Mir*InputEvent Mir*InputEvent”?

---

271	+MirInputEventTime mir_input_event_get_event_time(MirInputEvent const* ev);

Is there any particular reason to typedef int64_t to MirInputEventTime here? We're documenting it's nanoseconds-since-the-epoch, and this is not really useful if the type is opaque.

---

467	+typedef int64_t MirTouchInputEventTouchId;

We'll probably not have a full 2^63 touch ids at any point; int32_t maybe? :)

---

479	+    /* This touch point was cancelled, an down should have been
480	+       received but an up will never come. */
481	+    mir_touch_input_event_action_cancel = 3,

Comment seems a bit off, and not just the an/down mismatch :)
Should mention that this is opt-in behaviour, “a down should have been received” is a bit wishy-washy.

“This touch point was cancelled. Any actions taken in response to this touch should be undone. No further events will be delivered for this touch point.

A cancelled event will only be sent to a client that has explicitly enabled cancellation, via mir_input_break_me_harder()”

Maybe?

---

482	+    /* This touch point is unchanged but is reported as part of
483	+       a full state update. */
484	+    mir_touch_input_event_action_none = 4

Why do we have this? Is there an ability to request a full-state update?

---

518	+// Touch is made with a mouse through touch emulation
519	+    mir_touch_input_tool_type_mouse = 3,

Are we going to do this?

---

549	+MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index);

Hm. Would it be nice for clients if instead of get_touch_id(..., index) and get_*(..., index) we just had.
get_*(MirTouchInputEvent const* event, MirTouchInputEventTouchId id)?

That would seem be a more natural API to me, but we'd need toolkit input as to whether or not that's better.

---

655	+    default:
656	+        return MirInputEventType();

I'm a fan of abort() when faced with egregious client API abuse. Likewise for the DeviceId just afterwards.

---

761	+MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index)

Oooh, ugh. Maybe we need an actual MirTouchInputEvent to pass in here, so that we can maintain the state that lets us implement this properly?

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-21:

FAILED: Continuous integration, rev:2081
http://jenkins.qa.ubuntu.com/job/mir-ci/2193/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/254
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/254/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/228/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/182/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/228/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-21:

FAILED: Continuous integration, rev:2083
http://jenkins.qa.ubuntu.com/job/mir-ci/2195/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/257
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/257/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/231/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/184/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/231/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-11-21:

10 +da39a3ee5e6b4b0d3255bfef95601890afd80709 include/common/mir_toolkit/input/event.h~

A script gone awry?

130 +#define MIR_EVENT_ACTION_MASK 0xff
131 +#define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
132 +#define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;

Given that we propose a new API that hides all the details, and these weren't previously expose, do we need to expose these now?

365 + * Retreive
... and other function docs

Typo "Retrieve"

677 + assert(mir_event_get_type(ev) == mir_event_type_input);

Since all users of our library would use the release builds, these asserts would just protect us from problematic tests and example code, which I don't think is a significant gain. We should decide (at an API-wide level) how to react in such cases: abort, ignore, return null/error object (and if so what to do when this null/error value is passed to other functions).

Needs fixing/information

review: Needs Fixing

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2014-11-21:

165 +// and the mir_input_event* family of function.s
"functions."
-----------
other spelling errors : search for all instances of "Retreive", "assosciated", "occured"
-----------
233 + * \param [in] event The input event
"The event"
-----------
241 + * \param [in] event The event
"The input event"
-----------
Inconsistencies in terms of arg, or return type, e.g. sometimes we refer to a structure, as in "MirInputEvent", other times "the input event"
-----------
269 +MirKeyInputEvent const* mir_input_event_get_key_input_event(MirInputEvent const* ev);
Better if this was in include/common/mir_toolkit/input/key_input_event.h?
-----------
278 +MirTouchInputEvent const* mir_input_event_get_touch_input_event(MirInputEvent const* ev);
Better if this was in include/common/mir_toolkit/input/touch_input_event.h?
-----------
460 + /* This touch point was cancelled, an down should have been
"a down"
-----------
536 + * \param [in] touch_index The touch index. Must be less than touch_count.
"touch_count-1"
-----------
521 +MirTouchInputEventTouchId mir_touch_input_event_get_touch_id(MirTouchInputEvent const* event, size_t touch_index);
530 +MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index);
Lines too long, wanna break like others?
-----------
570 +da39a3ee5e6b4b0d3255bfef95601890afd80709 include/common/mir_toolkit/input/event.h~
accidental commit (there are others)
-----------
130 +#define MIR_EVENT_ACTION_MASK 0xff
131 +#define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
132 +#define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;

Better to move these to src/common/input/input_event.cpp, as they are only used there.
-----------
Too hungry to continue. :-)

review: Needs Fixing

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-11-21:

Hi guys! Thanks. Will reply to reviews 1 by 1

>> 179 +MirInputEvent* mir_event_get_input_event(MirEvent* ev);
>>
>> Const? Why is the MirEvent and MirInputEvent mutable?

Fixed!

>> We've traditionally not used the typedef struct _Foo Foo idiom. Any particular reason to not make >> this “typedef struct Mir*InputEvent Mir*InputEvent”?

Old habit

>> Is there any particular reason to typedef int64_t to MirInputEventTime here? We're documenting
>> it's nanoseconds-since-the-epoch, and this is not really useful if the type is opaque.

I was kind of imagining mir_input_event_time_to_nsecs or something but there probably is no point...updated to use int64_t.

---

>> We'll probably not have a full 2^63 touch ids at any point; int32_t maybe? :)

Ubuntu: Linux for trillion fingered hindu gods.

...:p changed.

---

>> 479 + /* This touch point was cancelled, an down should have been
>> 480 + received but an up will never come. */
>> 481 + mir_touch_input_event_action_cancel = 3,
>> 482 + /* This touch point is unchanged but is reported as part of
>> 483 + a full state update. */
>> 484 + mir_touch_input_event_action_none = 4
>>
>> Why do we have this? Is there an ability to request a full-state update?
You are right cancel is wishy washy....I will remove it for now since there is no "mir_input_break_me_harder" to refer to. I've removed it for now since it can be added later. Likewise I've removed none for now as with the current impl change is always used. None was added because the API is always a full state update (mirroring the hardware frames).

>> 518 +// Touch is made with a mouse through touch emulation
>> 519 + mir_touch_input_tool_type_mouse = 3,
>>
>> Are we going to do this?

Probably not and we can add it later...removed.

---

>>655 + default:
>> 656 + return MirInputEventType();

>> I'm a fan of abort() when faced with egregious client API abuse. Likewise for the DeviceId just >> afterwards.

Not confident here we have a consensus so lets discuss on monday?

---

761 +MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index)

>> Oooh, ugh. Maybe we need an actual MirTouchInputEvent to pass in here, so that we can maintain
>> the state that lets us implement this properly?

I think the proposed implementation is correct. From android input only one pointer can go up or down per event, and this lets us always determine the correct touch to assign the up/down action to and rest get the change action.

Hi guys! Thanks. Will reply to reviews 1 by 1

>> 179 +MirInputEvent* mir_event_get_input_event(MirEvent* ev);
>> 
>> Const? Why is the MirEvent and MirInputEvent mutable?

Fixed!

>> We've traditionally not used the typedef struct _Foo Foo idiom. Any particular reason to not make >> this “typedef struct Mir*InputEvent Mir*InputEvent”?

Old habit

>> Is there any particular reason to typedef int64_t to MirInputEventTime here? We're documenting 
>> it's nanoseconds-since-the-epoch, and this is not really useful if the type is opaque.

I was kind of imagining mir_input_event_time_to_nsecs or something but there probably is no point...updated to use int64_t.

---

>> We'll probably not have a full 2^63 touch ids at any point; int32_t maybe? :)

Ubuntu: Linux for trillion fingered hindu gods.

...:p changed.

---

>> 518 +// Touch is made with a mouse through touch emulation
>> 519 + mir_touch_input_tool_type_mouse = 3,
>>
>> Are we going to do this?

Probably not and we can add it later...removed.

---

>>655 + default:
>> 656 + return MirInputEventType();

>> I'm a fan of abort() when faced with egregious client API abuse. Likewise for the DeviceId just >> afterwards.

Not confident here we have a consensus so lets discuss on monday?

---

761 +MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index)

>> Oooh, ugh. Maybe we need an actual MirTouchInputEvent to pass in here, so that we can maintain 
>> the state that lets us implement this properly?

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-11-21:

Alexandros:

>> A script gone awry?
Yes MP to fix the script incoming next.

>> 130 +#define MIR_EVENT_ACTION_MASK 0xff
>> 131 +#define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
>> 132 +#define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;
>>
>> Given that we propose a new API that hides all the details, and these weren't previously expose, >> do we need to expose these now?

Good point I guess.

>> Typo "Retrieve"

Whoops.

>> 677 + assert(mir_event_get_type(ev) == mir_event_type_input);

Lets discuss a strategy on monday

Needs fixing/information

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-21:

FAILED: Continuous integration, rev:2087
http://jenkins.qa.ubuntu.com/job/mir-ci/2213/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/282/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/282/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/256/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/202/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/256/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-24:

FAILED: Continuous integration, rev:2088
http://jenkins.qa.ubuntu.com/job/mir-ci/2219/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/295
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/295/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/269/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/208/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/269/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-24:

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

review: Approve (continuous-integration)

Revision history for this message

Daniel d'Andrada (dandrader) wrote on 2014-11-24:

I have shell rotation to get done ASAP, so I won't really have time to take a close look at it until then.

From my side, what I would like to ensure is that touch ids are kept unique for some time after they've ended (ie, you don't recycle IDs ASAP but keep incrementing the nextIdNumber until it loops over a certain high-enough upper bound). As long as you keep this, I'm happy. This is important for the touch point ownership and cancellation scheme I've in place in unity8.

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-11-25:

Hey daniel. The old touch ids are being copied over 1-1 so we shouldn't have any problem there.

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-11-25:

Did some self review and ended up with three changes:

1. Use abort in case of obvious programmer error, i.e. passing touch index > mir_touch_input_event_get_touch_count.
2. Remove mir_touch_input_tool_type_eraser, I think stylus/finger/unknown is the right level here and eraser is probably some sort of state on stylus events. If eraser, why not airbrush :p
3. A few comment tweaks.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-26:

FAILED: Continuous integration, rev:2092
http://jenkins.qa.ubuntu.com/job/mir-ci/2261/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/350
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/350/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/324/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/250/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/324/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-11-26:

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

review: Approve (continuous-integration)

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-12-01:

Looks good overall. Some remaining nits/questions:

325 + mir_key_input_event_modifier_alt = 0x02,
326 + mir_key_input_event_modifier_alt_left = 0x10,
327 + mir_key_input_event_modifier_alt_right = 0x20,

The bit flags associated with the modifiers seem somewhat arbitrary (which may be OK since they are just flags). I wonder, though, if we could arrange them in a way that made more sense? Are we following some kind of external mapping?

Formatting nits:

124 +// Type for new style input event. Will be returned from mir_event_get_type for old style
125 +// key and motion events.

446 +/* Axis representing the x coordinate for the touch */
447 + mir_touch_input_axis_x = 0,

... and others

Comments are usually indented at the same level as the symbol they refers to.

633 + case mir_event_type_key:
634 + case mir_event_type_motion:
... and others

'case' should be indented at the same level as 'switch' (according to our guide).

Needs fixing + information for the modifier bit flags.

review: Needs Fixing

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-12-01:

Fixed formatting nits.

Modifier flags are per android, compatibility is not required though...updated to use less question provoking values :)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2014-12-01:

Needs fixing :

216 +MirEventType mir_event_get_type(MirEvent const* ev);

This function is not input specific so it should be elsewhere (not in input_event.h). E.g. it 'd be odd to have to include input_event.h in your app if you were looking for, say, 'mir_event_type_prompt_session_state_change'.
_______________________

637 +MirEventType mir_event_get_type(MirEvent const* ev)

This one also looks out of place as it's not input specific, yet it's defined in input_event.cpp.
_______________________

Nits :

Do we want some masks for MirKeyInputEventModifiers. E.g. mir_key_input_event_modifier_alt_mask = mir_key_input_event_modifier_alt |mir_key_input_event_modifier_alt_left | mir_key_input_event_modifier_alt_right;

_______________________

old_modifiers_to_new could be made more efficient by using masks.

_________________________

883 + case mir_motion_tool_type_mouse:

Not sure why 'mouse' should return 'unknown'.

review: Needs Fixing

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-12-01:

Thanks :D

>> This function is not input specific so it should be elsewhere (not in input_event.h). E.g. it >> be odd to have to include input_event.h in your app if you were looking for, say,
>> 'mir_event_type_prompt_session_state_change'.

Indeed. Fixed.

>> 637 +MirEventType mir_event_get_type(MirEvent const* ev)

>> This one also looks out of place as it's not input specific, yet it's defined in
>> input_event.cpp.

Yes but I dont think its worth creating event.cpp until there is other stuff in there.

Nits :

>> Do we want some masks for MirKeyInputEventModifiers.

I don't think so, as I mentioned in standup, I think in the future we will move to using opaque modifier masks from strings anyway.
_________________________

>> 883 + case mir_motion_tool_type_mouse:
>> Not sure why 'mouse' should return 'unknown'.

Because mir_motion_tool_type_mouse does not have an equivalent in the MirEvent 2.0 world. Or rather at least, mouse is not a touch tool, and will generate a separate pointer event category (not defined in this mp).This at least doesn't regress pointer support anywhere though (though already its quite flakey...)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2014-12-01:

No more to add. LGTM.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-01:

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-02:

FAILED: Continuous integration, rev:2095
http://jenkins.qa.ubuntu.com/job/mir-ci/2306/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/403
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/403/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/377/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/297/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/377/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-12-02:

Weird failure retrying.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-02:

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

review: Approve (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2014-12-02:

Looks reasonable. +1 on sanitizing the touch tools..

review: Approve

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2014-12-02:

Looks good.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-12-03:

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

review: Approve (continuous-integration)

Revision history for this message

Kevin DuBois (kdub) wrote on 2014-12-03:

abort seems unfriendly, if there's an invalid argument. Other than that looks okay by me

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-12-03:

Thanks for looking :) I think I've placed aborts only where the client can (and is required to have) detected incorrect use of the API...e.g.

mir_event_get_type != mir_event_type_input but passed to mir_event_get_input_event

I think in cases like this it's always an error in client code and the abort is helpful to developers.

Revision history for this message

Alberto Aguirre (albaguirre) wrote on 2014-12-05:

Why are all apis under common? Is the server going to use those APIs?

review: Needs Information

Revision history for this message

Robert Carr (robertcarr) wrote on 2014-12-05:

Definitely. QtMir will use it as opposed to direct access to MirEvent. Likewise other shells could consume it via mi::InputFilter etc. It's not infeasible USC would use it to implement some keybindings, etc.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2014-12-08:

LGTM now.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'client-ABI-sha1sums'
 --- client-ABI-sha1sums	2014-11-27 03:48:20 +0000
 +++ client-ABI-sha1sums	2014-12-02 23:48:29 +0000
@@ -11,6 +11,9 @@
 a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
 c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
  fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
--bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
++d71dddf6c4275ee423d3e0fa204e3b77b4d87401  include/common/mir_toolkit/event.h
++49403c824f69fa1cc1f58bf5fe635639148765eb  include/common/mir_toolkit/input/input_event.h
++f531c6a49883884a6c694393d5614f9942fe2703  include/common/mir_toolkit/input/key_input_event.h
++fd6d86becfa5ffa367480fd2654180f6ffcbf059  include/common/mir_toolkit/input/touch_input_event.h
  4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 c17714a57db57d79b7b5e7df40a3568236  include/common/mir_toolkit/mir_native_buffer.h
 === modified file 'common-ABI-sha1sums'
 --- common-ABI-sha1sums	2014-11-27 03:48:20 +0000
 +++ common-ABI-sha1sums	2014-12-02 23:48:29 +0000
@@ -17,6 +17,9 @@
 a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
 c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
  fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
--bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
++d71dddf6c4275ee423d3e0fa204e3b77b4d87401  include/common/mir_toolkit/event.h
++49403c824f69fa1cc1f58bf5fe635639148765eb  include/common/mir_toolkit/input/input_event.h
++f531c6a49883884a6c694393d5614f9942fe2703  include/common/mir_toolkit/input/key_input_event.h
++fd6d86becfa5ffa367480fd2654180f6ffcbf059  include/common/mir_toolkit/input/touch_input_event.h
  4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 c17714a57db57d79b7b5e7df40a3568236  include/common/mir_toolkit/mir_native_buffer.h
 === modified file 'examples/fingerpaint.c'
 --- examples/fingerpaint.c	2014-11-24 02:16:00 +0000
 +++ examples/fingerpaint.c	2014-12-02 23:48:29 +0000
@@ -17,6 +17,8 @@
   */
  #include "mir_toolkit/mir_client_library.h"
++#include "mir_toolkit/input/input_event.h"
++
  #include <stdio.h>
  #include <signal.h>
  #include <stdint.h>
@@ -200,41 +202,46 @@
          {0x00, 0xff, 0x00, 0xff},
          {0x00, 0x00, 0xff, 0xff},
      };
++
++    MirEventType event_type = mir_event_get_type(event);
--    if (event->type == mir_event_type_motion)
++    if (event_type == mir_event_type_input)
+     {
          static size_t base_color = 0;
          static size_t max_fingers = 0;
          static float max_pressure = 1.0f;
++
++        MirInputEvent const* input_event = mir_event_get_input_event(event);
++        if (mir_input_event_get_type(input_event) != mir_input_event_type_touch)
++            return;
++        MirTouchInputEvent const* tev = mir_input_event_get_touch_input_event(input_event);
++        unsigned touch_count = mir_touch_input_event_get_touch_count(tev);
--        // FIXME: https://bugs.launchpad.net/mir/+bug/1311699
--        MirMotionAction masked_action = event->motion.action & ~0xff00;
--
--        if (masked_action == mir_motion_action_up)
++        if (touch_count == 1 && mir_touch_input_event_get_touch_action(tev, 0) == mir_touch_input_event_action_up)
+         {
              base_color = (base_color + max_fingers) %
                           (sizeof(color)/sizeof(color[0]));
              max_fingers = 0;
+         }
--
--        if (masked_action == mir_motion_action_move ||
--            masked_action == mir_motion_action_down)
++        else
+         {
              size_t p;
--            if (event->motion.pointer_count > max_fingers)
--                max_fingers = event->motion.pointer_count;
++            if (touch_count > max_fingers)
++                max_fingers = touch_count;
--            for (p = 0; p < event->motion.pointer_count; p++)
++            for (p = 0; p < touch_count; p++)
+             {
--                int x = event->motion.pointer_coordinates[p].x;
--                int y = event->motion.pointer_coordinates[p].y;
--                int radius = event->motion.pointer_coordinates[p].size * 50.0f
++                int x = mir_touch_input_event_get_touch_axis_value(tev, p, mir_touch_input_axis_x);
++                int y = mir_touch_input_event_get_touch_axis_value(tev, p, mir_touch_input_axis_y);
++                float size = mir_touch_input_event_get_touch_axis_value(tev, p, mir_touch_input_axis_size);
++                float pressure = mir_touch_input_event_get_touch_axis_value(tev, p, mir_touch_input_axis_pressure);
++
++                int radius = size * 50.0f
                               + 1.0f;
                  size_t c = (base_color + p) %
                             (sizeof(color)/sizeof(color[0]));
                  Color tone = color[c];
--                float pressure = event->motion.pointer_coordinates[p].pressure;
                  if (pressure > max_pressure)
                      max_pressure = pressure;
@@ -247,7 +254,7 @@
              redraw(surface, canvas);
+         }
+     }
--    else if (event->type == mir_event_type_resize)
++    else if (event_type == mir_event_type_resize)
+     {
          /* FIXME: https://bugs.launchpad.net/mir/+bug/1194384
           * mir_event_type_resize will arrive in a different thread to that of
 === modified file 'include/common/mir_toolkit/event.h'
 --- include/common/mir_toolkit/event.h	2014-11-27 03:48:20 +0000
 +++ include/common/mir_toolkit/event.h	2014-12-02 23:48:29 +0000
@@ -43,7 +43,10 @@
      mir_event_type_resize,
      mir_event_type_prompt_session_state_change,
      mir_event_type_orientation,
--    mir_event_type_close_surface
++    mir_event_type_close_surface,
++    /* Type for new style input event will be returned from mir_event_get_type
++       when old style event type was mir_event_type_key or mir_event_type_motion */
++    mir_event_type_input
  } MirEventType;
  typedef enum {
@@ -121,6 +124,9 @@
     mir_motion_tool_type_eraser  = 4
  } MirMotionToolType;
++// DEPRECATED
++// Direct access to MirKeyEvent is deprecated. Please use mir_event_get_input_event
++// and the mir_input_event* family of functions.
  typedef struct
+ {
      MirEventType type;
@@ -135,6 +141,7 @@
      int32_t scan_code;
      int32_t repeat_count;
      nsecs_t down_time;
++
      nsecs_t event_time;
      int is_system_key;
  } MirKeyEvent;
@@ -157,6 +164,9 @@
      int unused3;
  } MirMotionPointer;
++// DEPRECATED
++// Direct access to MirMotionEvent is deprecated. Please use mir_event_get_input_event
++// and the mir_input_event* family of functions.
  typedef struct
+ {
      MirEventType type;
@@ -234,6 +244,7 @@
  typedef union
+ {
++    // Direct access to the type member is deprecated. Instead use mir_event_get_type.
      MirEventType    type;
      MirKeyEvent     key;
      MirMotionEvent  motion;
@@ -244,6 +255,16 @@
      MirCloseSurfaceEvent   close_surface;
  } MirEvent;
++/*
++ * Retrieves the type of a MirEvent. Now preferred over direct access to ev->type.
++ * In particular ev->type will never be mir_event_type_input and mir_event_get_type
++ * is the only way to ensure mir_event_get_input_event will succeed.
++ *
++ * \param [in] event The event
++ * \return           The event type
++ */
++MirEventType mir_event_get_type(MirEvent const* ev);
++
  #ifdef __cplusplus
+ }
  /**@}*/
 === added directory 'include/common/mir_toolkit/input'
 === added file 'include/common/mir_toolkit/input/input_event.h'
 --- include/common/mir_toolkit/input/input_event.h	1970-01-01 00:00:00 +0000
 +++ include/common/mir_toolkit/input/input_event.h	2014-12-02 23:48:29 +0000
@@ -0,0 +1,101 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Robert Carr <robert.carr@canonical.com>
++ */
++
++#ifndef MIR_TOOLKIT_INPUT_EVENT_H_
++#define MIR_TOOLKIT_INPUT_EVENT_H_
++
++#include "mir_toolkit/event.h"
++#include "mir_toolkit/input/key_input_event.h"
++#include "mir_toolkit/input/touch_input_event.h"
++
++#include <stdint.h>
++
++#ifdef __cplusplus
++/**
++ * \addtogroup mir_toolkit
++ * @{
++ */
++extern "C" {
++#endif
++
++typedef struct MirInputEvent MirInputEvent;
++
++typedef int64_t MirInputDeviceId;
++
++typedef enum {
++    mir_input_event_type_key = 0,
++    mir_input_event_type_touch = 1
++} MirInputEventType;
++
++/*
++ * Retrieve the MirInputEvent associated with a MirEvent of
++ * type mir_event_type_input.
++ *
++ * \param [in] event The event
++ * \return           The associated MirInputEvent
++ */
++MirInputEvent const* mir_event_get_input_event(MirEvent const* ev);
++
++/*
++ * Retrieves the device id responsible for generating an input event.
++ *
++ * \param [in] event The input event
++ * \return           The id of the generating device
++ */
++MirInputDeviceId mir_input_event_get_device_id(MirInputEvent const* ev);
++
++/*
++ * Retrieve the time at which an input event occured.
++ *
++ * \param [in] event The input event
++ * \return           A timestamp in nanoseconds-since-epoch
++ */
++int64_t mir_input_event_get_event_time(MirInputEvent const* ev);
++
++/*
++ * Retrieve the type of an input event (e.g. key, touch...)
++ *
++ * \param [in] event The input event
++ * \return           The input event type
++ */
++MirInputEventType mir_input_event_get_type(MirInputEvent const* ev);
++
++/*
++ * Retrieve the MirKeyInputEvent associated with a given input event.
++ *
++ * \param[in] event The input event
++ * \return          The MirKeyInputEvent or NULL if event type is not
++ *                  mir_input_event_type_key
++ */
++MirKeyInputEvent const* mir_input_event_get_key_input_event(MirInputEvent const* ev);
++
++/*
++ * Retrieve the MirTouchInputEvent associated with a given input event.
++ *
++ * \param[in] event The input event
++ * \return          The MirTouchInputEvent or NULL if event type is not
++ *                  mir_input_event_type_touch
++ */
++MirTouchInputEvent const* mir_input_event_get_touch_input_event(MirInputEvent const* ev);
++
++#ifdef __cplusplus
++}
++/**@}*/
++#endif
++
++#endif // MIR_TOOLKIT_INPUT_EVENT_H_
 === added file 'include/common/mir_toolkit/input/key_input_event.h'
 --- include/common/mir_toolkit/input/key_input_event.h	1970-01-01 00:00:00 +0000
 +++ include/common/mir_toolkit/input/key_input_event.h	2014-12-02 23:48:29 +0000
@@ -0,0 +1,113 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Robert Carr <robert.carr@canonical.com>
++ */
++
++#ifndef MIR_TOOLKIT_KEY_INPUT_EVENT_H_
++#define MIR_TOOLKIT_KEY_INPUT_EVENT_H_
++
++#include <xkbcommon/xkbcommon.h>
++
++#ifdef __cplusplus
++/**
++ * \addtogroup mir_toolkit
++ * @{
++ */
++extern "C" {
++#endif
++
++/**
++ * An event type describing a change in keyboard state
++ */
++typedef struct MirKeyInputEvent MirKeyInputEvent;
++
++/**
++ * Possible actions for changing key state
++ */
++typedef enum {
++    /* A key has gone down */
++    mir_key_input_event_action_up,
++    /* A key has come up */
++    mir_key_input_event_action_down,
++    /* System policy has triggered a key repeat on a key
++       which was already down */
++    mir_key_input_event_action_repeat
++} MirKeyInputEventAction;
++
++/**
++ * Description of key modifier state.
++ */
++typedef enum {
++    mir_key_input_event_modifier_none        = 1 << 0,
++    mir_key_input_event_modifier_alt         = 1 << 1,
++    mir_key_input_event_modifier_alt_left    = 1 << 2,
++    mir_key_input_event_modifier_alt_right   = 1 << 3,
++    mir_key_input_event_modifier_shift       = 1 << 4,
++    mir_key_input_event_modifier_shift_left  = 1 << 5,
++    mir_key_input_event_modifier_shift_right = 1 << 6,
++    mir_key_input_event_modifier_sym         = 1 << 7,
++    mir_key_input_event_modifier_function    = 1 << 8,
++    mir_key_input_event_modifier_ctrl        = 1 << 9,
++    mir_key_input_event_modifier_ctrl_left   = 1 << 10,
++    mir_key_input_event_modifier_ctrl_right  = 1 << 11,
++    mir_key_input_event_modifier_meta        = 1 << 12,
++    mir_key_input_event_modifier_meta_left   = 1 << 13,
++    mir_key_input_event_modifier_meta_right  = 1 << 14,
++    mir_key_input_event_modifier_caps_lock   = 1 << 15,
++    mir_key_input_event_modifier_num_lock    = 1 << 16,
++    mir_key_input_event_modifier_scroll_lock = 1 << 17
++} MirKeyInputEventModifiers;
++
++/**
++ * Retrieve the action which triggered a given key event.
++ *
++ *  \param [in] event The key event
++ *  \return           The associated action
++ */
++MirKeyInputEventAction mir_key_input_event_get_action(MirKeyInputEvent const* event);
++
++/**
++ * Retrieve the xkb mapped keycode associated with the key acted on.. May
++ * be interpreted as per <xkbcommon/xkb-keysyms.h>
++ *
++ *   \param [in] event The key event
++ *   \return           The xkb_keysym
++ */
++xkb_keysym_t mir_key_input_event_get_key_code(MirKeyInputEvent const* event);
++
++/**
++ * Retrieve the raw hardware scan code associated with the key acted on. May
++ * be interpreted as per <linux/input.h>
++ *
++ *   \param [in] event The key event
++ *   \return           The scancode
++ */
++int mir_key_input_event_get_scan_code(MirKeyInputEvent const* event);
++
++/**
++ * Retrieve the modifier keys pressed when the key action occured.
++ *
++ *   \param [in] event The key event
++ *   \return           The modifier mask
++ */
++MirKeyInputEventModifiers mir_key_input_event_get_modifiers(MirKeyInputEvent const* event);
++
++#ifdef __cplusplus
++}
++/**@}*/
++#endif
++
++#endif /* MIR_TOOLKIT_KEY_INPUT_EVENT_H_ */
 === added file 'include/common/mir_toolkit/input/touch_input_event.h'
 --- include/common/mir_toolkit/input/touch_input_event.h	1970-01-01 00:00:00 +0000
 +++ include/common/mir_toolkit/input/touch_input_event.h	2014-12-02 23:48:29 +0000
@@ -0,0 +1,141 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Robert Carr <robert.carr@canonical.com>
++ */
++
++#ifndef MIR_TOOLKIT_TOUCH_INPUT_EVENT_H_
++#define MIR_TOOLKIT_TOUCH_INPUT_EVENT_H_
++
++#ifdef __cplusplus
++/**
++ * \addtogroup mir_toolkit
++ * @{
++ */
++extern "C" {
++#endif
++
++/**
++ * An event type describing a change in touch device state.
++ */
++typedef struct MirTouchInputEvent MirTouchInputEvent;
++
++/**
++ * An identifier for a touch-point. TouchId's are unique per-gesture.
++ * That is to say, once a touch has gone down at time T, no other touch will
++ * use that touch's ID until all touches at time T have come up.
++ */
++typedef int32_t MirTouchInputEventTouchId;
++
++/**
++ * Possible per touch actions for state changing
++ */
++typedef enum {
++    /* This touch point is going up */
++    mir_touch_input_event_action_up = 0,
++    /* This touch point is going down */
++    mir_touch_input_event_action_down = 1,
++    /* Axis values have changed on this touch point */
++    mir_touch_input_event_action_change = 2
++} MirTouchInputEventTouchAction;
++
++/**
++ * Identifiers for touch axis
++ */
++typedef enum {
++/* Axis representing the x coordinate for the touch */
++    mir_touch_input_axis_x = 0,
++/* Axis representing the y coordinate for the touch */
++    mir_touch_input_axis_y = 1,
++/* Axis representing pressure of the touch */
++    mir_touch_input_axis_pressure = 2,
++/* Axis representing the length of the major axis of an ellipse
++   centered at the touch point */
++    mir_touch_input_axis_touch_major = 3,
++/* Axis representing the length of the minor axis of an ellipse
++   centered at the touch point */
++    mir_touch_input_axis_touch_minor = 4,
++/* Axis representing the diameter of a circle centered on the touch
++   point */
++    mir_touch_input_axis_size = 5
++} MirTouchInputEventTouchAxis;
++
++/**
++ * Identifiers for per-touch tool types
++ */
++typedef enum {
++// Tool type could not be determined
++    mir_touch_input_tool_type_unknown = 0,
++// Touch is made with a finger
++    mir_touch_input_tool_type_finger = 1,
++// Touch is made with a stylus
++    mir_touch_input_tool_type_stylus = 2
++} MirTouchInputEventTouchTooltype;
++
++/**
++ * Retrieve the number of touches reported for a given touch event. Each touch
++ * is said to be index in the event and may be accessed by index 0, 1, ... , (touch_count - 1)
++ *
++ *   \param [in] event The touch event
++ *   \return           The number of touches
++ */
++unsigned int mir_touch_input_event_get_touch_count(MirTouchInputEvent const* event);
++
++/**
++ * Retrieve the TouchID for a touch at given index.
++ *
++ *  \param [in] event       The touch event
++ *  \param [in] touch_index The touch index. Must be less than (touch_count - 1).
++ *  \return                 ID of the touch at index
++ */
++MirTouchInputEventTouchId mir_touch_input_event_get_touch_id(MirTouchInputEvent const* event, size_t touch_index);
++
++/**
++ * Retrieve the action which occured for a touch at given index.
++ *
++ *  \param [in] event       The touch event
++ *  \param [in] touch_index The touch index. Must be less than (touch_count - 1).
++ *  \return                 Action performed for the touch at index.
++ */
++MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index);
++
++/**
++ * Retrieve the tooltype for touch at given index.
++ *
++ *  \param [in] event       The touch event
++ *  \param [in] touch_index The touch index. Must be less than (touch_count - 1).
++ *  \return                 Tooltype used for the touch at index
++ */
++MirTouchInputEventTouchTooltype mir_touch_input_event_get_touch_tooltype(MirTouchInputEvent const* event,
++    size_t touch_index);
++
++
++/**
++ * Retrieve the axis value for a given axis on an indexed touch.
++ *
++ *  \param [in] event       The touch event
++ *  \param [in] touch_index The touch index. Must be less than (touch_count - 1).
++ *  \param [in] axis        The axis to retreive a value from
++ *  \return                 The value of the given axis
++ */
++float mir_touch_input_event_get_touch_axis_value(MirTouchInputEvent const* event,
++    size_t touch_index, MirTouchInputEventTouchAxis axis);
++
++#ifdef __cplusplus
++}
++/**@}*/
++#endif
++
++#endif /* MIR_TOOLKIT_TOUCH_INPUT_EVENT_H_ */
 === modified file 'platform-ABI-sha1sums'
 --- platform-ABI-sha1sums	2014-11-27 03:48:20 +0000
 +++ platform-ABI-sha1sums	2014-12-02 23:48:29 +0000
@@ -17,7 +17,10 @@
 a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
 c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
  fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
--bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
++d71dddf6c4275ee423d3e0fa204e3b77b4d87401  include/common/mir_toolkit/event.h
++49403c824f69fa1cc1f58bf5fe635639148765eb  include/common/mir_toolkit/input/input_event.h
++f531c6a49883884a6c694393d5614f9942fe2703  include/common/mir_toolkit/input/key_input_event.h
++fd6d86becfa5ffa367480fd2654180f6ffcbf059  include/common/mir_toolkit/input/touch_input_event.h
  4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 c17714a57db57d79b7b5e7df40a3568236  include/common/mir_toolkit/mir_native_buffer.h
 e609c0fed0d566ddbaaa8ac2d7cd5c06dd09a  include/platform/mir/abnormal_exit.h
 === modified file 'server-ABI-sha1sums'
 --- server-ABI-sha1sums	2014-11-27 03:48:20 +0000
 +++ server-ABI-sha1sums	2014-12-02 23:48:29 +0000
@@ -17,7 +17,10 @@
 a7fe3a382061f2752c0236cc971e7e68f93f  include/common/mir_toolkit/client_types.h
 c0674d9d882c1845550847357f6a5de5af66  include/common/mir_toolkit/common.h
  fce4c1a9e0d037244f7e9e96ea2d8eaab4fc404c  include/common/mir_toolkit/cursors.h
--bccde9cdbc2f0d07e186c2950ef88d3a7d5f0d3b  include/common/mir_toolkit/event.h
++d71dddf6c4275ee423d3e0fa204e3b77b4d87401  include/common/mir_toolkit/event.h
++49403c824f69fa1cc1f58bf5fe635639148765eb  include/common/mir_toolkit/input/input_event.h
++f531c6a49883884a6c694393d5614f9942fe2703  include/common/mir_toolkit/input/key_input_event.h
++fd6d86becfa5ffa367480fd2654180f6ffcbf059  include/common/mir_toolkit/input/touch_input_event.h
  4975998aa1056ed0d39dcc538127453e516ad8e9  include/common/mir_toolkit/mesa/native_display.h
 c17714a57db57d79b7b5e7df40a3568236  include/common/mir_toolkit/mir_native_buffer.h
 e609c0fed0d566ddbaaa8ac2d7cd5c06dd09a  include/platform/mir/abnormal_exit.h
 === modified file 'src/common/input/CMakeLists.txt'
 --- src/common/input/CMakeLists.txt	2014-11-24 02:16:00 +0000
 +++ src/common/input/CMakeLists.txt	2014-12-02 23:48:29 +0000
@@ -27,6 +27,7 @@
  add_library(mirsharedinput OBJECT
    ${ANDROID_SHARED_INPUT_SOURCES}
    udev_wrapper.cpp
++  input_event.cpp
+ )
  list(APPEND MIR_COMMON_SOURCES
 === added file 'src/common/input/input_event.cpp'
 --- src/common/input/input_event.cpp	1970-01-01 00:00:00 +0000
 +++ src/common/input/input_event.cpp	2014-12-02 23:48:29 +0000
@@ -0,0 +1,330 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify it
++ * under the terms of the GNU Lesser General Public License version 3,
++ * as published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU Lesser General Public License for more details.
++ *
++ * You should have received a copy of the GNU Lesser General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Robert Carr <robert.carr@canonical.com>
++ */
++
++#include "mir_toolkit/input/input_event.h"
++
++#include <assert.h>
++#include <stdlib.h>
++
++// See: https://bugs.launchpad.net/mir/+bug/1311699
++#define MIR_EVENT_ACTION_MASK 0xff
++#define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
++#define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;
++
++namespace
++{
++MirEvent const* old_ev_from_new(MirInputEvent const* ev)
++{
++    return reinterpret_cast<MirEvent const*>(ev);
++}
++MirKeyEvent const& old_kev_from_new(MirKeyInputEvent const* ev)
++{
++    auto old_ev = reinterpret_cast<MirEvent const*>(ev);
++    if (old_ev->type != mir_event_type_key)
++        abort();
++    return old_ev->key;
++}
++MirMotionEvent const& old_mev_from_new(MirTouchInputEvent const* ev)
++{
++    auto old_ev = reinterpret_cast<MirEvent const*>(ev);
++    if (old_ev->type != mir_event_type_motion)
++        abort();
++    return old_ev->motion;
++}
++}
++
++MirEventType mir_event_get_type(MirEvent const* ev)
++{
++    switch (ev->type)
++    {
++    case mir_event_type_key:
++    case mir_event_type_motion:
++        return mir_event_type_input;
++    default:
++        return ev->type;
++    }
++}
++
++MirInputEvent const* mir_event_get_input_event(MirEvent  const* ev)
++{
++    if(mir_event_get_type(ev) != mir_event_type_input)
++        abort();
++
++    return reinterpret_cast<MirInputEvent const*>(ev);
++}
++
++MirInputEventType mir_input_event_get_type(MirInputEvent const* ev)
++{
++    auto old_ev = old_ev_from_new(ev);
++    assert(old_ev->type == mir_event_type_key || old_ev->type == mir_event_type_motion);
++
++    switch (old_ev->type)
++    {
++    case mir_event_type_key:
++        return mir_input_event_type_key;
++    case mir_event_type_motion:
++        return mir_input_event_type_touch;
++    default:
++        abort();
++    }
++}
++
++MirInputDeviceId mir_input_event_get_device_id(MirInputEvent const* ev)
++{
++    auto old_ev = old_ev_from_new(ev);
++    assert(mir_event_get_type(old_ev) == mir_event_type_input);
++
++    switch (old_ev->type)
++    {
++    case mir_event_type_motion:
++        return old_ev->motion.device_id;
++    case mir_event_type_key:
++        return old_ev->key.device_id;
++    default:
++        abort();
++    }
++}
++
++int64_t mir_input_event_get_event_time(MirInputEvent const* ev)
++{
++    auto old_ev = old_ev_from_new(ev);
++    assert(mir_event_get_type(old_ev) == mir_event_type_input);
++
++    switch (old_ev->type)
++    {
++    case mir_event_type_motion:
++        return old_ev->motion.event_time;
++    case mir_event_type_key:
++        return old_ev->key.event_time;
++    default:
++        abort();
++    }
++}
++
++/* Key event accessors */
++
++MirKeyInputEvent const* mir_input_event_get_key_input_event(MirInputEvent const* ev)
++{
++    if (mir_input_event_get_type(ev) != mir_input_event_type_key)
++        abort();
++
++    return reinterpret_cast<MirKeyInputEvent const*>(ev);
++}
++
++MirKeyInputEventAction mir_key_input_event_get_action(MirKeyInputEvent const* kev)
++{
++    auto const& old_kev = old_kev_from_new(kev);
++
++    switch (old_kev.action)
++    {
++    case mir_key_action_down:
++        if (old_kev.repeat_count != 0)
++            return mir_key_input_event_action_repeat;
++        else
++            return mir_key_input_event_action_down;
++    case mir_key_action_up:
++        return mir_key_input_event_action_up;
++    default:
++        // TODO:? This means we got key_action_multiple which I dont think is
++        // actually emitted yet (and never will be as in the future it would fall under text
++        // event in the new model).
++        return mir_key_input_event_action_down;
++    }
++}
++
++xkb_keysym_t mir_key_input_event_get_key_code(MirKeyInputEvent const* kev)
++{
++    auto const& old_kev = old_kev_from_new(kev);
++    return old_kev.key_code;
++}
++
++int mir_key_input_event_get_scan_code(MirKeyInputEvent const* kev)
++{
++    auto const& old_kev = old_kev_from_new(kev);
++    return old_kev.scan_code;
++}
++
++namespace
++{
++MirKeyInputEventModifiers old_modifiers_to_new(MirKeyModifier old_modifier)
++{
++    unsigned modifier = 0;
++
++    if (old_modifier & mir_key_modifier_none)
++        modifier |= mir_key_input_event_modifier_none;
++    if (old_modifier & mir_key_modifier_alt)
++        modifier |= mir_key_input_event_modifier_alt;
++    if (old_modifier & mir_key_modifier_alt_left)
++        modifier |= mir_key_input_event_modifier_alt_left;
++    if (old_modifier & mir_key_modifier_alt_right)
++        modifier |= mir_key_input_event_modifier_alt_right;
++    if (old_modifier & mir_key_modifier_shift)
++        modifier |= mir_key_input_event_modifier_shift;
++    if (old_modifier & mir_key_modifier_shift_left)
++        modifier |= mir_key_input_event_modifier_shift_left;
++    if (old_modifier & mir_key_modifier_shift_right)
++        modifier |= mir_key_input_event_modifier_shift_right;
++    if (old_modifier & mir_key_modifier_sym)
++        modifier |= mir_key_input_event_modifier_sym;
++    if (old_modifier & mir_key_modifier_function)
++        modifier |= mir_key_input_event_modifier_function;
++    if (old_modifier & mir_key_modifier_ctrl)
++        modifier |= mir_key_input_event_modifier_ctrl;
++    if (old_modifier & mir_key_modifier_ctrl_left)
++        modifier |= mir_key_input_event_modifier_ctrl_left;
++    if (old_modifier & mir_key_modifier_ctrl_right)
++        modifier |= mir_key_input_event_modifier_ctrl_right;
++    if (old_modifier & mir_key_modifier_meta)
++        modifier |= mir_key_input_event_modifier_meta;
++    if (old_modifier & mir_key_modifier_meta_left)
++        modifier |= mir_key_input_event_modifier_meta_left;
++    if (old_modifier & mir_key_modifier_meta_right)
++        modifier |= mir_key_input_event_modifier_meta_right;
++    if (old_modifier & mir_key_modifier_caps_lock)
++        modifier |= mir_key_input_event_modifier_caps_lock;
++    if (old_modifier & mir_key_modifier_num_lock)
++        modifier |= mir_key_input_event_modifier_num_lock;
++    if (old_modifier & mir_key_modifier_scroll_lock)
++        modifier |= mir_key_input_event_modifier_scroll_lock;
++    return static_cast<MirKeyInputEventModifiers>(modifier);
++}
++}
++MirKeyInputEventModifiers mir_key_input_event_get_modifiers(MirKeyInputEvent const* kev)
++{    auto const& old_kev = old_kev_from_new(kev);
++    return old_modifiers_to_new(static_cast<MirKeyModifier>(old_kev.modifiers));
++}
++/* Touch event accessors */
++
++MirTouchInputEvent const* mir_input_event_get_touch_input_event(MirInputEvent const* ev)
++{
++    if(mir_input_event_get_type(ev) != mir_input_event_type_touch)
++        abort();
++
++    return reinterpret_cast<MirTouchInputEvent const*>(ev);
++}
++
++unsigned int mir_touch_input_event_get_touch_count(MirTouchInputEvent const* event)
++{
++    auto const& old_mev = reinterpret_cast<MirEvent const*>(event)->motion;
++    return old_mev.pointer_count;
++}
++
++MirTouchInputEventTouchId mir_touch_input_event_get_touch_id(MirTouchInputEvent const* event, size_t touch_index)
++{
++    auto const& old_mev = old_mev_from_new(event);
++
++    if (touch_index >= old_mev.pointer_count)
++        abort();
++
++    return old_mev.pointer_coordinates[touch_index].id;
++}
++
++MirTouchInputEventTouchAction mir_touch_input_event_get_touch_action(MirTouchInputEvent const* event, size_t touch_index)
++{
++    auto const& old_mev = old_mev_from_new(event);
++
++    if(touch_index > old_mev.pointer_count)
++        abort();
++
++    auto masked_action = old_mev.action & MIR_EVENT_ACTION_MASK;
++    size_t masked_index = (old_mev.action & MIR_EVENT_ACTION_POINTER_INDEX_MASK) >> MIR_EVENT_ACTION_POINTER_INDEX_SHIFT;
++
++    switch (masked_action)
++    {
++    // For the next two cases we could assert pc=1...because a gesture must
++    // be starting or ending.
++    case mir_motion_action_down:
++        return mir_touch_input_event_action_down;
++    case mir_motion_action_up:
++        return mir_touch_input_event_action_up;
++    // We can't tell which touches have actually moved without tracking state
++    // so we report all touchpoints as changed.
++    case mir_motion_action_move:
++    case mir_motion_action_hover_move:
++        return mir_touch_input_event_action_change;
++    // All touch points are handled at once so we don't know the index
++    case mir_motion_action_pointer_down:
++        if (touch_index == masked_index)
++            return mir_touch_input_event_action_down;
++        else
++            return mir_touch_input_event_action_change;
++    case mir_motion_action_pointer_up:
++        if (touch_index == masked_index)
++            return mir_touch_input_event_action_up;
++        else
++            return mir_touch_input_event_action_change;
++    // TODO: How to deal with these?
++    case mir_motion_action_cancel:
++    case mir_motion_action_outside:
++    case mir_motion_action_scroll:
++    case mir_motion_action_hover_enter:
++    case mir_motion_action_hover_exit:
++    default:
++        return mir_touch_input_event_action_change;
++    }
++}
++
++MirTouchInputEventTouchTooltype mir_touch_input_event_get_touch_tooltype(MirTouchInputEvent const* event,
++    size_t touch_index)
++{
++    auto const& old_mev = old_mev_from_new(event);
++
++    if(touch_index > old_mev.pointer_count)
++        abort();
++
++    switch (old_mev.pointer_coordinates[touch_index].tool_type)
++    {
++    case mir_motion_tool_type_finger:
++        return mir_touch_input_tool_type_finger;
++    case mir_motion_tool_type_stylus:
++    case mir_motion_tool_type_eraser:
++        return mir_touch_input_tool_type_stylus;
++    case mir_motion_tool_type_mouse:
++    case mir_motion_tool_type_unknown:
++    default:
++        return mir_touch_input_tool_type_unknown;
++    }
++}
++
++float mir_touch_input_event_get_touch_axis_value(MirTouchInputEvent const* event,
++    size_t touch_index, MirTouchInputEventTouchAxis axis)
++{
++    auto const& old_mev = old_mev_from_new(event);
++
++    if(touch_index > old_mev.pointer_count)
++        abort();
++
++    auto const& old_pc = old_mev.pointer_coordinates[touch_index];
++    switch (axis)
++    {
++    case mir_touch_input_axis_x:
++        return old_pc.x;
++    case mir_touch_input_axis_y:
++        return old_pc.y;
++    case mir_touch_input_axis_pressure:
++        return old_pc.pressure;
++    case mir_touch_input_axis_touch_major:
++        return old_pc.touch_major;
++    case mir_touch_input_axis_touch_minor:
++        return old_pc.touch_minor;
++    case mir_touch_input_axis_size:
++        return old_pc.size;
++    default:
++        return -1;
++    }
++}
 === modified file 'src/common/symbols.map'
 --- src/common/symbols.map	2014-11-24 02:16:00 +0000
 +++ src/common/symbols.map	2014-12-02 23:48:29 +0000
@@ -1,5 +1,5 @@
  MIR_COMMON_3 {
-- global:
++ global: mir_*;
    extern "C++" {
  # The following symbols come from running a script over the generated docs. Vis:
  # ../tools/process_doxygen_xml.py doc/xml/*.xml | grep "^mircommon public" | sed "s/mircommon public: /    /" | sort
 === modified file 'tests/unit-tests/input/CMakeLists.txt'
 --- tests/unit-tests/input/CMakeLists.txt	2014-11-24 02:16:00 +0000
 +++ tests/unit-tests/input/CMakeLists.txt	2014-12-02 23:48:29 +0000
@@ -6,6 +6,7 @@
    ${CMAKE_CURRENT_SOURCE_DIR}/test_cursor_controller.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_xcursor_loader.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/test_touchspot_controller.cpp
++  ${CMAKE_CURRENT_SOURCE_DIR}/test_input_event.cpp
+ )
  set(
 === added file 'tests/unit-tests/input/test_input_event.cpp'
 --- tests/unit-tests/input/test_input_event.cpp	1970-01-01 00:00:00 +0000
 +++ tests/unit-tests/input/test_input_event.cpp	2014-12-02 23:48:29 +0000
@@ -0,0 +1,214 @@
++/*
++ * Copyright © 2014 Canonical Ltd.
++ *
++ * This program is free software: you can redistribute it and/or modify
++ * it under the terms of the GNU General Public License version 3 as
++ * published by the Free Software Foundation.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
++ *
++ * Authored by: Robert Carr <robert.carr@canonical.com>
++ */
++
++#include <gtest/gtest.h>
++
++#include "mir_toolkit/event.h"
++#include "mir_toolkit/input/input_event.h"
++
++// See: https://bugs.launchpad.net/mir/+bug/1311699
++#define MIR_EVENT_ACTION_POINTER_INDEX_MASK 0xff00
++#define MIR_EVENT_ACTION_POINTER_INDEX_SHIFT 8;
++
++TEST(InputEvent, old_style_key_and_motion_events_are_input_events)
++{
++    MirEvent key_ev;
++    key_ev.type = mir_event_type_key;
++
++    EXPECT_EQ(mir_event_type_input, mir_event_get_type(&key_ev));
++    EXPECT_EQ(mir_input_event_type_key, mir_input_event_get_type(mir_event_get_input_event(&key_ev)));
++
++    MirEvent motion_ev;
++    motion_ev.type = mir_event_type_motion;
++
++    EXPECT_EQ(mir_event_type_input, mir_event_get_type(&motion_ev));
++    EXPECT_NE(nullptr, mir_event_get_input_event(&motion_ev));
++}
++
++// MirInputEvent properties common to all events.
++
++TEST(CommonInputEventProperties, device_id_taken_from_old_style_event)
++{
++    int64_t dev_id_1 = 17, dev_id_2 = 23;
++    MirEvent old_ev;
++
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.device_id = dev_id_1;
++    EXPECT_EQ(dev_id_1, mir_input_event_get_device_id(
++        mir_event_get_input_event(&old_ev)));
++
++    old_ev.type = mir_event_type_key;
++    old_ev.motion.device_id = dev_id_2;
++    EXPECT_EQ(dev_id_2, mir_input_event_get_device_id(
++        mir_event_get_input_event(&old_ev)));
++}
++
++TEST(CommonInputEventProperties, event_time_taken_from_old_style_event)
++{
++    int64_t event_time_1 = 79, event_time_2 = 83;
++    MirEvent old_ev;
++
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.event_time = event_time_1;
++    EXPECT_EQ(event_time_1, mir_input_event_get_event_time(
++        mir_event_get_input_event(&old_ev)));
++
++    old_ev.type = mir_event_type_key;
++    old_ev.key.event_time = event_time_2;
++    EXPECT_EQ(event_time_2, mir_input_event_get_event_time(
++        mir_event_get_input_event(&old_ev)));
++}
++
++TEST(KeyInputEventProperties, up_and_down_actions_copied_from_old_style_event)
++{
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_key;
++
++    old_ev.key.action = mir_key_action_down;
++    old_ev.key.repeat_count = 0;
++
++    auto new_kev = mir_input_event_get_key_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(mir_key_input_event_action_down, mir_key_input_event_get_action(new_kev));
++
++    old_ev.key.action = mir_key_action_up;
++    EXPECT_EQ(mir_key_input_event_action_up, mir_key_input_event_get_action(new_kev));
++}
++
++TEST(KeyInputEventProperties, repeat_action_produced_from_non_zero_repeat_count_in_old_style_event)
++{
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_key;
++    old_ev.key.action = mir_key_action_down;
++    old_ev.key.repeat_count = 1;
++
++    auto new_kev = mir_input_event_get_key_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(mir_key_input_event_action_repeat, mir_key_input_event_get_action(new_kev));
++}
++
++TEST(KeyInputEventProperties, keycode_scancode_and_modifiers_taken_from_old_style_event)
++{
++    xkb_keysym_t key_code = 171;
++    int scan_code = 31;
++    MirKeyModifier old_modifiers = mir_key_modifier_shift;
++
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_key;
++    old_ev.key.key_code = key_code;
++    old_ev.key.scan_code = scan_code;
++    old_ev.key.modifiers = old_modifiers;
++
++    auto new_kev = mir_input_event_get_key_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(key_code, mir_key_input_event_get_key_code(new_kev));
++    EXPECT_EQ(scan_code, mir_key_input_event_get_scan_code(new_kev));
++    EXPECT_EQ(mir_key_input_event_modifier_shift, mir_key_input_event_get_modifiers(new_kev));
++}
++
++TEST(TouchInputEventProperties, touch_count_taken_from_pointer_count)
++{
++    unsigned const pointer_count = 3;
++
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.action = mir_motion_action_down;
++    old_ev.motion.pointer_count = pointer_count;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(pointer_count, mir_touch_input_event_get_touch_count(tev));
++}
++
++TEST(TouchInputEventProperties, touch_id_comes_from_pointer_coordinates)
++{
++    unsigned const touch_id = 31;
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.action = mir_motion_action_down;
++    old_ev.motion.pointer_count = 1;
++    old_ev.motion.pointer_coordinates[0].id = touch_id;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(touch_id, mir_touch_input_event_get_touch_id(tev, 0));
++}
++
++// mir_motion_action_up/down represent the start of a gesture. pointers only go up/down one at a time
++TEST(TouchInputEventProperties, down_and_up_actions_are_taken_from_old_event)
++{
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.action = mir_motion_action_down;
++    old_ev.motion.pointer_count = 1;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(mir_touch_input_event_action_down, mir_touch_input_event_get_touch_action(tev, 0));
++}
++
++TEST(TouchInputEventProperties, pointer_up_down_applies_only_to_masked_action)
++{
++    int const masked_pointer_index = 1;
++
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.action = masked_pointer_index << MIR_EVENT_ACTION_POINTER_INDEX_SHIFT;
++    old_ev.motion.action = (old_ev.motion.action & MIR_EVENT_ACTION_POINTER_INDEX_MASK) | mir_motion_action_pointer_up;
++    old_ev.motion.pointer_count = 3;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(mir_touch_input_event_action_change, mir_touch_input_event_get_touch_action(tev, 0));
++    EXPECT_EQ(mir_touch_input_event_action_up, mir_touch_input_event_get_touch_action(tev, 1));
++    EXPECT_EQ(mir_touch_input_event_action_change, mir_touch_input_event_get_touch_action(tev, 2));
++}
++
++TEST(TouchInputEventProperties, tool_type_copied_from_old_pc)
++{
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++
++    auto& old_mev = old_ev.motion;
++    old_mev.pointer_count = 4;
++    old_mev.pointer_coordinates[0].tool_type = mir_motion_tool_type_unknown;
++    old_mev.pointer_coordinates[1].tool_type = mir_motion_tool_type_finger;
++    old_mev.pointer_coordinates[2].tool_type = mir_motion_tool_type_stylus;
++    old_mev.pointer_coordinates[3].tool_type = mir_motion_tool_type_mouse;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(mir_touch_input_tool_type_unknown, mir_touch_input_event_get_touch_tooltype(tev, 0));
++    EXPECT_EQ(mir_touch_input_tool_type_finger, mir_touch_input_event_get_touch_tooltype(tev, 1));
++    EXPECT_EQ(mir_touch_input_tool_type_stylus, mir_touch_input_event_get_touch_tooltype(tev, 2));
++}
++
++TEST(TouchInputEventProperties, axis_values_used_by_qtmir_copied)
++{
++    float x_value = 19, y_value = 23, touch_major = .3, touch_minor = .2, pressure = .9, size = 1111;
++    MirEvent old_ev;
++    old_ev.type = mir_event_type_motion;
++    old_ev.motion.pointer_count = 1;
++    auto &old_pc = old_ev.motion.pointer_coordinates[0];
++    old_pc.x = x_value;
++    old_pc.y = y_value;
++    old_pc.touch_major = touch_major;
++    old_pc.touch_minor = touch_minor;
++    old_pc.pressure = pressure;
++    old_pc.size = size;
++
++    auto tev = mir_input_event_get_touch_input_event(mir_event_get_input_event(&old_ev));
++    EXPECT_EQ(x_value, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_x));
++    EXPECT_EQ(y_value, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_y));
++    EXPECT_EQ(touch_major, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_touch_major));
++    EXPECT_EQ(touch_minor, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_touch_minor));
++    EXPECT_EQ(pressure, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_pressure));
++    EXPECT_EQ(size, mir_touch_input_event_get_touch_axis_value(tev, 0, mir_touch_input_axis_size));
++}