Merge into development-branch : fix-1546324 : Code : Mir

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration	2016-02-17	Approve on 2016-02-29
Alexandros Frantzis (community)			Approve on 2016-02-26
Cemil Azizoglu (community)		2016-02-17	Approve on 2016-02-24
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2016-02-24
Review via email: mp+286315@code.launchpad.net

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-02-17:

#

Linux input also defines BTN_BACK and BTN_FORWARD, how are those mapped in X11? I have no mouse with such a button - but I found one with SIDE And EXTRA.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

#

FAILED: Continuous integration, rev:3320
http://jenkins.qa.ubuntu.com/job/mir-ci/6314/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5955
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4862
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5911/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/638/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/638/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5908/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-17: Posted in a previous version of this proposal

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/325/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-17:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/326/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

#

FAILED: Continuous integration, rev:3321
http://jenkins.qa.ubuntu.com/job/mir-ci/6317/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5958
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4865
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5914/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/641/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5911/console

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-17:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/330/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-17:

#

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

review: Approve (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-17:

#

20 +MirPointerButtons to_button_state(int button)

Should be named to_mir_button(int button)
--------------------------------------------

487 + auto const up = 4, down = 5, left = 6, right = 7;

Better to use Button4 for 4, Button5 for 5 as those are standard. For 6 and 7, my understanding is that not every left/right button may be reported as 6 & 7 (though, I'm okay leaving this as you've implemented it). Is this also your understanding? That can also be said for up & down I guess.

--------------------------------------------

114 +void mix::XInputDevice::pointer_motion(std::chrono::nanoseconds event_time, mir::geometry::Point const& pos, mir::geometry::Displacement scroll)
115 +{
116 + std::cout << "pos " << pos.x.as_float() << std::endl;
117 + auto const movement = pos - pointer_pos;
118 + pointer_pos = pos;
119 + sink->handle_input(
120 + *builder->pointer_event(
121 + event_time,
122 + mir_pointer_action_motion,
123 + button_state,
124 + scroll.dx.as_float(),
125 + scroll.dy.as_float(),
126 + movement.dx.as_float(),
127 + movement.dy.as_float()
128 + )
129 + );
130 +}

button_state is not set to the proper value here. Unfortunately, this function is called at two different times; i) during scrolling, which doesn't require this setting, and ii) mouse motion, which does. So you may have to clone this function (pointer_scroll()?).

Note also that for (ii) you can't simply call to_button_state() function as it has to be extracted from, not 'button' field of the Xevent struct, but from the 'state' field. (You can look at the existing code for that case).

--------------------------------------------

Superluous lines
34 +
60 +
111 +

--------------------------------------------

Hmm, you're right implementing it this way, I think.

77 + button_state |= to_button_state(button);
96 + button_state = button_state & ~(to_button_state(button));

Though can you change the latter to
button_state &= ~(to_button_state(button));
for consistency?

20	+MirPointerButtons to_button_state(int button)

Should be named to_mir_button(int button)
--------------------------------------------

487	+                    auto const up = 4, down = 5, left = 6, right = 7;

Better to use Button4 for 4, Button5 for 5 as those are standard. For 6 and 7, my understanding is that not every left/right button may be reported as 6 & 7 (though, I'm okay leaving this as you've implemented it). Is this also your understanding? That can also be said for up & down I guess.

--------------------------------------------

114	+void mix::XInputDevice::pointer_motion(std::chrono::nanoseconds event_time, mir::geometry::Point const& pos, mir::geometry::Displacement scroll)
115	+{
116	+    std::cout << "pos " << pos.x.as_float() << std::endl;
117	+    auto const movement = pos - pointer_pos;
118	+    pointer_pos = pos;
119	+    sink->handle_input(
120	+        *builder->pointer_event(
121	+            event_time,
122	+            mir_pointer_action_motion,
123	+            button_state,
124	+            scroll.dx.as_float(),
125	+            scroll.dy.as_float(),
126	+            movement.dx.as_float(),
127	+            movement.dy.as_float()
128	+            )
129	+        );
130	+}

button_state is not set to the proper value here. Unfortunately, this function is called at two different times; i) during scrolling, which doesn't require this setting, and ii) mouse motion, which does. So you may have to clone this function (pointer_scroll()?).

Note also that for (ii) you can't simply call to_button_state() function as it has to be extracted from, not 'button' field of the Xevent struct, but from the 'state' field. (You can look at the existing code for that case).

--------------------------------------------

Superluous lines
34	+
60	+
111	+

--------------------------------------------

Hmm, you're right implementing it this way, I think.

77	+    button_state |= to_button_state(button);
96	+    button_state = button_state & ~(to_button_state(button));

Though can you change the latter to 
             button_state &= ~(to_button_state(button));
for consistency?

review: Needs Fixing

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-02-22:

#

>
> 20 +MirPointerButtons to_button_state(int button)
>
> Should be named to_mir_button(int button)
> --------------------------------------------
>
> 487 + auto const up = 4, down = 5, left = 6, right = 7;
>
> Better to use Button4 for 4, Button5 for 5 as those are standard. For 6 and 7,
> my understanding is that not every left/right button may be reported as 6 & 7
> (though, I'm okay leaving this as you've implemented it). Is this also your
> understanding? That can also be said for up & down I guess.

Yeah not entirely sure - but Gtk seems to be sure about that and defines those constants
in its X11 backend. At least my mouse has about 10 buttons - but no horizontal scrolling
wheels and the additional buttons begin with 9 ...

> --------------------------------------------
>
> 114 +void mix::XInputDevice::pointer_motion(std::chrono::nanoseconds
> event_time, mir::geometry::Point const& pos, mir::geometry::Displacement
> scroll)
> 115 +{
> 116 + std::cout << "pos " << pos.x.as_float() << std::endl;
> 117 + auto const movement = pos - pointer_pos;
> 118 + pointer_pos = pos;
> 119 + sink->handle_input(
> 120 + *builder->pointer_event(
> 121 + event_time,
> 122 + mir_pointer_action_motion,
> 123 + button_state,
> 124 + scroll.dx.as_float(),
> 125 + scroll.dy.as_float(),
> 126 + movement.dx.as_float(),
> 127 + movement.dy.as_float()
> 128 + )
> 129 + );
> 130 +}
>
> button_state is not set to the proper value here. Unfortunately, this function
> is called at two different times; i) during scrolling, which doesn't require
> this setting, and ii) mouse motion, which does. So you may have to clone this
> function (pointer_scroll()?).
>
> Note also that for (ii) you can't simply call to_button_state() function as it
> has to be extracted from, not 'button' field of the Xevent struct, but from
> the 'state' field. (You can look at the existing code for that case).

Ack I should pull it from the most recent event. And remove the iostream mess..

> --------------------------------------------
>
> Superluous lines
> 34 +
> 60 +
> 111 +
>
> --------------------------------------------
>
> Hmm, you're right implementing it this way, I think.
>
> 77 + button_state |= to_button_state(button);
> 96 + button_state = button_state & ~(to_button_state(button));
>
> Though can you change the latter to
> button_state &= ~(to_button_state(button));
> for consistency?

sure..

> 
> 20      +MirPointerButtons to_button_state(int button)
> 
> Should be named to_mir_button(int button)
> --------------------------------------------
> 
> 487     +                    auto const up = 4, down = 5, left = 6, right = 7;
> 
> Better to use Button4 for 4, Button5 for 5 as those are standard. For 6 and 7,
> my understanding is that not every left/right button may be reported as 6 & 7
> (though, I'm okay leaving this as you've implemented it). Is this also your
> understanding? That can also be said for up & down I guess.

Yeah not entirely sure - but Gtk seems to be sure about that and defines those constants
in its X11 backend. At least my mouse has about 10 buttons - but no horizontal scrolling
wheels and the additional buttons begin with 9 ...

> --------------------------------------------
> 
> 114     +void mix::XInputDevice::pointer_motion(std::chrono::nanoseconds
> event_time, mir::geometry::Point const& pos, mir::geometry::Displacement
> scroll)
> 115     +{
> 116     +    std::cout << "pos " << pos.x.as_float() << std::endl;
> 117     +    auto const movement = pos - pointer_pos;
> 118     +    pointer_pos = pos;
> 119     +    sink->handle_input(
> 120     +        *builder->pointer_event(
> 121     +            event_time,
> 122     +            mir_pointer_action_motion,
> 123     +            button_state,
> 124     +            scroll.dx.as_float(),
> 125     +            scroll.dy.as_float(),
> 126     +            movement.dx.as_float(),
> 127     +            movement.dy.as_float()
> 128     +            )
> 129     +        );
> 130     +}
> 
> button_state is not set to the proper value here. Unfortunately, this function
> is called at two different times; i) during scrolling, which doesn't require
> this setting, and ii) mouse motion, which does. So you may have to clone this
> function (pointer_scroll()?).
> 
> Note also that for (ii) you can't simply call to_button_state() function as it
> has to be extracted from, not 'button' field of the Xevent struct, but from
> the 'state' field. (You can look at the existing code for that case).

Ack I should pull it from the most recent event. And remove the iostream mess..

> --------------------------------------------
> 
> Superluous lines
> 34      +
> 60      +
> 111     +
> 
> --------------------------------------------
> 
> Hmm, you're right implementing it this way, I think.
> 
> 77      +    button_state |= to_button_state(button);
> 96      +    button_state = button_state & ~(to_button_state(button));
> 
> Though can you change the latter to
>              button_state &= ~(to_button_state(button));
> for consistency?

sure..

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-22:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/350/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-22:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-22:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/355/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-22:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-22:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/356/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-22:

#

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

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-22:

#

FAILED: Continuous integration, rev:3325
http://jenkins.qa.ubuntu.com/job/mir-ci/6349/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/6001
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4908
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5957/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/673
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/673/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/673
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/673/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5954
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5954/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8328/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/27807

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-23:

#

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

review: Approve (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-23:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/364/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-23:

#

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

review: Approve (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-23:

#

37 +MirPointerButtons to_mir_button_state(int x_button_key_state)
38 +{
39 + // the state variable contains modifier and button state.
40 + MirPointerButtons button_state = x_button_key_state << 8;
41 + button_state = (button_state & ~(mir_pointer_button_secondary|mir_pointer_button_tertiary)) // second and middle button are swaped in X11
42 + | ((button_state & mir_pointer_button_secondary) >> 1)
43 + | ((button_state & mir_pointer_button_tertiary) << 1);
44 + return button_state;
45 +}

This is very hard to follow. It will also be difficult to change once other modifiers need to be handled here. Can we explicitly use 'ButtonNMask' as in the original code?

review: Needs Fixing

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-24:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/385/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2016-02-24:

#

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-02-24:

#

I'm happy

review: Approve

Revision history for this message

Alexandros Frantzis (afrantzis) wrote on 2016-02-26:

#

OK.

Nit/preexisting:

mix::XInputPlatform::process_input_event(s) begs to broken up into smaller functions.

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-02-26:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Andreas Pokorny (andreas-pokorny) wrote on 2016-02-27:

#

Hm NestedServer.display_orientation_changes_are_forwarded_to_host (5514 ms) failed..

Revision history for this message

Mir CI Bot (mir-ci-bot) on 2016-02-29:

#

review: Approve (continuous-integration)

Mir

Merge lp:~andreas-pokorny/mir/fix-1546324 into lp:mir

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'src/platforms/mesa/server/x11/input/input_device.cpp'
 --- src/platforms/mesa/server/x11/input/input_device.cpp	2016-01-29 08:18:22 +0000
 +++ src/platforms/mesa/server/x11/input/input_device.cpp	2016-02-24 09:00:08 +0000
@@ -22,10 +22,53 @@
  #include "mir/input/touchpad_settings.h"
  #include "mir/input/input_device_info.h"
  #include "mir/input/device_capability.h"
++#include "mir/input/event_builder.h"
++#include "mir/input/input_sink.h"
++
++#include <X11/Xlib.h>
++#include <iostream>
  namespace mi = mir::input;
++namespace geom = mir::geometry;
  namespace mix = mi::X;
++namespace
++{
++MirPointerButtons to_mir_button(int button)
++{
++    auto const button_side = 8;
++    auto const button_extra = 9;
++    if (button == Button1)
++        return mir_pointer_button_primary;
++    if (button == Button2)  // tertiary (middle) button is Button2 in X
++        return mir_pointer_button_tertiary;
++    if (button == Button3)
++        return mir_pointer_button_secondary;
++    if (button == button_side)
++        return mir_pointer_button_side;
++    if (button == button_extra)
++        return mir_pointer_button_extra;
++    return 0;
++}
++
++MirPointerButtons to_mir_button_state(int x_button_key_state)
++{
++    MirPointerButtons button_state = 0;
++    if (x_button_key_state & Button1Mask)
++        button_state |= mir_pointer_button_primary;
++    if (x_button_key_state & Button2Mask)
++        button_state |= mir_pointer_button_tertiary;
++    if (x_button_key_state & Button3Mask)
++        button_state |= mir_pointer_button_secondary;
++    if (x_button_key_state & Button4Mask)
++        button_state |= mir_pointer_button_back;
++    if (x_button_key_state & Button5Mask)
++        button_state |= mir_pointer_button_forward;
++    return button_state;
++}
++
++}
++
  mix::XInputDevice::XInputDevice(InputDeviceInfo const& device_info)
      : info(device_info)
+ {
@@ -45,6 +88,9 @@
  mi::InputDeviceInfo mix::XInputDevice::get_device_info()
+ {
++    // TODO Make use if X11-XInput2 to get raw device information
++    // and support other devices than just the unified pointer and
++    // keyboards.
      return info;
+ }
@@ -59,7 +105,6 @@
  void mix::XInputDevice::apply_settings(PointerSettings const&)
+ {
--    // TODO Make use if X11-XInput2
+ }
  mir::optional_value<mi::TouchpadSettings> mix::XInputDevice::get_touchpad_settings() const
@@ -74,3 +119,94 @@
  void mix::XInputDevice::apply_settings(TouchpadSettings const&)
+ {
+ }
++
++bool mix::XInputDevice::started() const
++{
++    return sink && builder;
++}
++
++void mix::XInputDevice::key_press(std::chrono::nanoseconds event_time, xkb_keysym_t key_sym, int32_t key_code)
++{
++    sink->handle_input(
++        *builder->key_event(
++            event_time,
++            mir_keyboard_action_down,
++            key_sym,
++            key_code
++            )
++        );
++
++}
++
++void mix::XInputDevice::key_release(std::chrono::nanoseconds event_time, xkb_keysym_t key_sym, int32_t key_code)
++{
++    sink->handle_input(
++        *builder->key_event(
++            event_time,
++            mir_keyboard_action_up,
++            key_sym,
++            key_code
++            )
++        );
++}
++
++void mix::XInputDevice::update_button_state(int button)
++{
++    button_state = to_mir_button_state(button);
++}
++
++void mix::XInputDevice::pointer_press(std::chrono::nanoseconds event_time, int button, mir::geometry::Point const& pos, mir::geometry::Displacement scroll)
++{
++    button_state |= to_mir_button(button);
++
++    auto const movement = pos - pointer_pos;
++    pointer_pos = pos;
++    sink->handle_input(
++        *builder->pointer_event(
++            event_time,
++            mir_pointer_action_button_down,
++            button_state,
++            scroll.dx.as_float(),
++            scroll.dy.as_float(),
++            movement.dx.as_float(),
++            movement.dy.as_float()
++            )
++        );
++}
++
++void mix::XInputDevice::pointer_release(std::chrono::nanoseconds event_time, int button, mir::geometry::Point const& pos, mir::geometry::Displacement scroll)
++{
++    button_state &= ~to_mir_button(button);
++
++    auto const movement = pos - pointer_pos;
++    pointer_pos = pos;
++    sink->handle_input(
++        *builder->pointer_event(
++            event_time,
++            mir_pointer_action_button_up,
++            button_state,
++            scroll.dx.as_float(),
++            scroll.dy.as_float(),
++            movement.dx.as_float(),
++            movement.dy.as_float()
++            )
++        );
++
++}
++
++void mix::XInputDevice::pointer_motion(std::chrono::nanoseconds event_time, mir::geometry::Point const& pos, mir::geometry::Displacement scroll)
++{
++    auto const movement = pos - pointer_pos;
++    pointer_pos = pos;
++    sink->handle_input(
++        *builder->pointer_event(
++            event_time,
++            mir_pointer_action_motion,
++            button_state,
++            scroll.dx.as_float(),
++            scroll.dy.as_float(),
++            movement.dx.as_float(),
++            movement.dy.as_float()
++            )
++        );
++}
 === modified file 'src/platforms/mesa/server/x11/input/input_device.h'
 --- src/platforms/mesa/server/x11/input/input_device.h	2016-01-29 08:18:22 +0000
 +++ src/platforms/mesa/server/x11/input/input_device.h	2016-02-24 09:00:08 +0000
@@ -21,9 +21,13 @@
  #include "mir/input/input_device.h"
  #include "mir/input/input_device_info.h"
++#include "mir_toolkit/event.h"
  #include "mir/geometry/point.h"
++#include "mir/geometry/displacement.h"
  #include "mir/optional_value.h"
++#include <chrono>
++
  namespace mir
+ {
  namespace input
@@ -47,10 +51,19 @@
      optional_value<TouchpadSettings> get_touchpad_settings() const override;
      void apply_settings(TouchpadSettings const& settings) override;
++    bool started() const;
++    void key_press(std::chrono::nanoseconds event_time, xkb_keysym_t key_sym, int32_t key_code);
++    void key_release(std::chrono::nanoseconds event_time, xkb_keysym_t key_sym, int32_t key_code);
++    void update_button_state(int button);
++    void pointer_press(std::chrono::nanoseconds event_time, int button, mir::geometry::Point const& pos, mir::geometry::Displacement scroll);
++    void pointer_release(std::chrono::nanoseconds event_time, int button, mir::geometry::Point const& pos, mir::geometry::Displacement scroll);
++    void pointer_motion(std::chrono::nanoseconds event_time, mir::geometry::Point const& pos, mir::geometry::Displacement scroll);
++
++private:
++    MirPointerButtons button_state{0};
      InputSink* sink{nullptr};
      EventBuilder* builder{nullptr};
      geometry::Point pointer_pos;
--private:
      InputDeviceInfo info;
  };
 === modified file 'src/platforms/mesa/server/x11/input/input_platform.cpp'
 --- src/platforms/mesa/server/x11/input/input_platform.cpp	2016-01-29 08:18:22 +0000
 +++ src/platforms/mesa/server/x11/input/input_platform.cpp	2016-02-24 09:00:08 +0000
@@ -21,8 +21,6 @@
  #include "mir/input/input_device_registry.h"
  #include "mir/input/input_device_info.h"
--#include "mir/input/event_builder.h"
--#include "mir/input/input_sink.h"
  #include "mir/dispatch/readable_fd.h"
  #define MIR_LOG_COMPONENT "x11-input"
@@ -43,6 +41,7 @@
  #define GRAB_KBD
  namespace mi = mir::input;
++namespace geom = mir::geometry;
  namespace md = mir::dispatch;
  namespace mix = mi::X;
@@ -81,233 +80,173 @@
  void mix::XInputPlatform::process_input_event()
+ {
--    // This code is based on :
--    // https://tronche.com/gui/x/xlib/events/keyboard-pointer/keyboard-pointer.html
--    XEvent xev;
--
--    XNextEvent(x11_connection.get(), &xev);
--
--    if (core_keyboard->sink || core_pointer->sink)
++    do
+     {
--        switch (xev.type)
++        // This code is based on :
++        // https://tronche.com/gui/x/xlib/events/keyboard-pointer/keyboard-pointer.html
++        XEvent xev;
++
++        XNextEvent(x11_connection.get(), &xev);
++
++        if (core_keyboard->started() && core_pointer->started())
+         {
++            switch (xev.type)
++            {
  #ifdef GRAB_KBD
--        case FocusIn:
--            {
--                auto const& xfiev = (XFocusInEvent&)xev;
--                XGrabKeyboard(xfiev.display, xfiev.window, True, GrabModeAsync, GrabModeAsync, CurrentTime);
--                break;
--            }
--
--        case FocusOut:
--            {
--                auto const& xfoev = (XFocusOutEvent&)xev;
--                XUngrabKeyboard(xfoev.display, CurrentTime);
--                break;
--            }
--#endif
--        case KeyPress:
--        case KeyRelease:
--            {
--                auto& xkev = (XKeyEvent&)xev;
--                static const int STRMAX = 32;
--                char str[STRMAX];
--                KeySym keysym;
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                mir::log_info("X11 key event :"
--                              " type=%s, serial=%u, send_event=%d, display=%p, window=%p,"
--                              " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
--                              " y_root=%d, state=0x%0X, keycode=%d, same_screen=%d",
--                              xkev.type == KeyPress ? "down" : "up", xkev.serial,
--                              xkev.send_event, xkev.display, xkev.window, xkev.root,
--                              xkev.subwindow, xkev.time, xkev.x, xkev.y, xkev.x_root,
--                              xkev.y_root, xkev.state, xkev.keycode, xkev.same_screen);
--                auto count =
--#endif
--                XLookupString(&xkev, str, STRMAX, &keysym, NULL);
--
--                auto event_time =
--                    std::chrono::duration_cast<std::chrono::nanoseconds>(
--                        std::chrono::milliseconds{xkev.time});
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                for (int i=0; i<count; i++)
--                    mir::log_info("buffer[%d]='%c'", i, str[i]);
--                mir::log_info("Mir key event : "
--                              "key_code=%d, scan_code=%d, modifiers=0x%0X, event_time=%" PRId64,
--                              keysym, xkev.keycode-8, modifiers, event_time);
--#endif
--                core_keyboard->sink->handle_input(
--                    *core_keyboard->builder->key_event(
--                        event_time,
--                        xkev.type == KeyPress ?
--                            mir_keyboard_action_down :
--                            mir_keyboard_action_up,
--                        keysym,
--                        xkev.keycode-8
--                        )
--                    );
--                break;
--            }
--
--        case ButtonPress:
--        case ButtonRelease:
--            {
--                auto const& xbev = (XButtonEvent&)xev;
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                mir::log_info("X11 button event :"
--                              " type=%s, serial=%u, send_event=%d, display=%p, window=%p,"
--                              " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
--                              " y_root=%d, state=0x%0X, button=%d, same_screen=%d",
--                              xbev.type == ButtonPress ? "down" : "up", xbev.serial,
--                              xbev.send_event, xbev.display, xbev.window, xbev.root,
--                              xbev.subwindow, xbev.time, xbev.x, xbev.y, xbev.x_root,
--                              xbev.y_root, xbev.state, xbev.button, xbev.same_screen);
--#endif
--                if ((xbev.type == ButtonRelease) &&
--                    ((xbev.button == Button4) || (xbev.button == Button5)))
--                {
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                    mir::log_info("Swallowed");
--#endif
--                    break;
--                }
--                auto event_time =
--                    std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds{xbev.time});
--
--                MirPointerButtons buttons_pressed = 0;
--                if (xbev.type == ButtonPress)
--                {
--                    if (xbev.button == Button1)
--                        buttons_pressed |= mir_pointer_button_primary;
--                    if (xbev.button == Button2) // tertiary (middle) button is Button2 in X
--                        buttons_pressed |= mir_pointer_button_tertiary;
--                    if (xbev.button == Button3)
--                        buttons_pressed |= mir_pointer_button_secondary;
--                    if (xbev.button == Button4)
--                        buttons_pressed |= mir_pointer_button_back;
--                    if (xbev.button == Button5)
--                        buttons_pressed |= mir_pointer_button_forward;
--                }
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                mir::log_info("Mir button event : x=%d, y=%d, "
--                              "buttons_pressed=0x%0X, event_time=%" PRId64,
--                              xbev.x, xbev.y, buttons_pressed, event_time);
--#endif
--
--                if ((xbev.button == Button4) || (xbev.button == Button5))
--                { // scroll event
--                    core_pointer->sink->handle_input(
--                        *core_pointer->builder->pointer_event(
--                            event_time,
--                            mir_pointer_action_motion,
--                            0,
--                            0.0f,
--                            (xbev.button == Button4) ? 1.0f : -1.0f,
--                            0.0f,
--                            0.0f
--                            )
--                        );
--                }
--                else
--                {
--                    auto const old_pointer_pos = core_pointer->pointer_pos;
--                    core_pointer->pointer_pos = geometry::Point{xbev.x, xbev.y};
--                    auto const movement = core_pointer->pointer_pos - old_pointer_pos;
--
--                    core_pointer->sink->handle_input(
--                        *core_pointer->builder->pointer_event(
--                            event_time,
--                            xbev.type == ButtonPress ?
--                                mir_pointer_action_button_down :
--                                mir_pointer_action_button_up,
--                            buttons_pressed,
--                            0.0f,
--                            0.0f,
--                            movement.dx.as_float(),
--                            movement.dy.as_float()
--                            )
--                        );
--                }
--                break;
--            }
--
--        case MotionNotify:
--            {
--                auto const& xmev = (XMotionEvent&)xev;
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                mir::log_info("X11 motion event :"
--                              " type=motion, serial=%u, send_event=%d, display=%p, window=%p,"
--                              " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
--                              " y_root=%d, state=0x%0X, is_hint=%s, same_screen=%d",
--                              xmev.serial, xmev.send_event, xmev.display, xmev.window,
--                              xmev.root, xmev.subwindow, xmev.time, xmev.x, xmev.y, xmev.x_root,
--                              xmev.y_root, xmev.state, xmev.is_hint == NotifyNormal ? "no" : "yes", xmev.same_screen);
--#endif
--
--                auto event_time =
--                    std::chrono::duration_cast<std::chrono::nanoseconds>(
--                        std::chrono::milliseconds{xmev.time});
--
--                MirPointerButtons buttons_pressed = 0;
--                if (xmev.state & Button1Mask)
--                    buttons_pressed |= mir_pointer_button_primary;
--                if (xmev.state & Button2Mask) // tertiary (middle) button is Button2 in X
--                    buttons_pressed |= mir_pointer_button_tertiary;
--                if (xmev.state & Button3Mask)
--                    buttons_pressed |= mir_pointer_button_secondary;
--                if (xmev.state & Button4Mask)
--                    buttons_pressed |= mir_pointer_button_back;
--                if (xmev.state & Button5Mask)
--                    buttons_pressed |= mir_pointer_button_forward;
--
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                mir::log_info("Mir pointer event : "
--                              "x=%d, y=%d, buttons_pressed=0x%0X, event_time=%" PRId64,
--                              xmev.x, xmev.y, buttons_pressed, event_time);
--#endif
--                core_pointer->sink->handle_input(
--                    *core_pointer->builder->pointer_event(
--                        event_time,
--                        mir_pointer_action_motion,
--                        buttons_pressed,
--                        0.0f,
--                        0.0f,
--                        0.0f,
--                        0.0f
--                        )
--                    );
--                break;
--            }
--
--        case ConfigureNotify:
--            {
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--                auto const& xcev = (XConfigureEvent&)xev;
--                mir::log_info("Window size : %dx%d", xcev.width, xcev.height);
--#endif
--                break;
--            }
--
--        case MappingNotify:
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--            mir::log_info("Keyboard mapping changed at server. Refreshing the cache.");
--#endif
--            XRefreshKeyboardMapping((XMappingEvent*)&xev);
--            break;
--
--        default:
--#ifdef MIR_ON_X11_INPUT_VERBOSE
--            mir::log_info("Uninteresting event : %08X", xev.type);
--#endif
--            break;
++            case FocusIn:
++                {
++                    auto const& xfiev = xev.xfocus;
++                    XGrabKeyboard(xfiev.display, xfiev.window, True, GrabModeAsync, GrabModeAsync, CurrentTime);
++                    break;
++                }
++
++            case FocusOut:
++                {
++                    auto const& xfoev = xev.xfocus;
++                    XUngrabKeyboard(xfoev.display, CurrentTime);
++                    break;
++                }
++#endif
++            case KeyPress:
++            case KeyRelease:
++                {
++                    auto & xkev = xev.xkey;
++                    static const int STRMAX = 32;
++                    char str[STRMAX];
++                    KeySym keysym;
++
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                    mir::log_info("X11 key event :"
++                                  " type=%s, serial=%u, send_event=%d, display=%p, window=%p,"
++                                  " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
++                                  " y_root=%d, state=0x%0X, keycode=%d, same_screen=%d",
++                                  xkev.type == KeyPress ? "down" : "up", xkev.serial,
++                                  xkev.send_event, xkev.display, xkev.window, xkev.root,
++                                  xkev.subwindow, xkev.time, xkev.x, xkev.y, xkev.x_root,
++                                  xkev.y_root, xkev.state, xkev.keycode, xkev.same_screen);
++                    auto count =
++#endif
++                        XLookupString(&xkev, str, STRMAX, &keysym, NULL);
++
++                    auto const event_time =
++                        std::chrono::duration_cast<std::chrono::nanoseconds>(
++                            std::chrono::milliseconds{xkev.time});
++
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                    for (int i=0; i<count; i++)
++                        mir::log_info("buffer[%d]='%c'", i, str[i]);
++                    mir::log_info("Mir key event : "
++                                  "key_code=%d, scan_code=%d, event_time=%" PRId64,
++                                  keysym, xkev.keycode-8, event_time);
++#endif
++
++                    if (xkev.type == KeyPress)
++                        core_keyboard->key_press(event_time, keysym, xkev.keycode - 8);
++                    else
++                        core_keyboard->key_release(event_time, keysym, xkev.keycode - 8);
++
++                    break;
++                }
++
++            case ButtonPress:
++            case ButtonRelease:
++                {
++                    auto const& xbev = xev.xbutton;
++                    auto const up = Button4, down = Button5, left = 6, right = 7;
++
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                    mir::log_info("X11 button event :"
++                                  " type=%s, serial=%u, send_event=%d, display=%p, window=%p,"
++                                  " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
++                                  " y_root=%d, state=0x%0X, button=%d, same_screen=%d",
++                                  xbev.type == ButtonPress ? "down" : "up", xbev.serial,
++                                  xbev.send_event, xbev.display, xbev.window, xbev.root,
++                                  xbev.subwindow, xbev.time, xbev.x, xbev.y, xbev.x_root,
++                                  xbev.y_root, xbev.state, xbev.button, xbev.same_screen);
++#endif
++                    if (xbev.type == ButtonRelease && xbev.button >= up && xbev.button <= right)
++                    {
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                        mir::log_info("Swallowed");
++#endif
++                        break;
++                    }
++                    auto const event_time =
++                        std::chrono::duration_cast<std::chrono::nanoseconds>(std::chrono::milliseconds{xbev.time});
++                    core_pointer->update_button_state(xbev.state);
++
++                    if (xbev.button >= up && xbev.button <= right)
++                    {  // scroll event
++                        core_pointer->pointer_motion(
++                            event_time,
++                            geom::Point{xbev.x, xbev.y},
++                            geom::Displacement{xbev.button == right ? 1 : xbev.button == left ? -1 : 0,
++                                               xbev.button == up ? 1 : xbev.button == down ? -1 : 0});
++                    }
++                    else
++                    {
++                        if (xbev.type == ButtonPress)
++                            core_pointer->pointer_press(
++                                event_time,
++                                xbev.button,
++                                geom::Point{xbev.x, xbev.y},
++                                geom::Displacement{0, 0});
++                        else
++                            core_pointer->pointer_release(
++                                event_time,
++                                xbev.button,
++                                geom::Point{xbev.x, xbev.y},
++                                geom::Displacement{0, 0});
++                    }
++                    break;
++                }
++
++            case MotionNotify:
++                {
++                    auto const& xmev = xev.xmotion;
++
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                    mir::log_info("X11 motion event :"
++                                  " type=motion, serial=%u, send_event=%d, display=%p, window=%p,"
++                                  " root=%p, subwindow=%p, time=%d, x=%d, y=%d, x_root=%d,"
++                                  " y_root=%d, state=0x%0X, is_hint=%s, same_screen=%d",
++                                  xmev.serial, xmev.send_event, xmev.display, xmev.window,
++                                  xmev.root, xmev.subwindow, xmev.time, xmev.x, xmev.y, xmev.x_root,
++                                  xmev.y_root, xmev.state, xmev.is_hint == NotifyNormal ? "no" : "yes", xmev.same_screen);
++#endif
++
++                    core_pointer->update_button_state(xmev.state);
++                    core_pointer->pointer_motion(
++                        std::chrono::milliseconds{xmev.time}, geom::Point{xmev.x, xmev.y}, geom::Displacement{0, 0});
++
++                    break;
++                }
++
++            case ConfigureNotify:
++                {
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                    auto const& xcev = xev.xconfigure;
++                    mir::log_info("Window size : %dx%d", xcev.width, xcev.height);
++#endif
++                    break;
++                }
++
++            case MappingNotify:
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                mir::log_info("Keyboard mapping changed at server. Refreshing the cache.");
++#endif
++                XRefreshKeyboardMapping(&(xev.xmapping));
++                break;
++
++            default:
++#ifdef MIR_ON_X11_INPUT_VERBOSE
++                mir::log_info("Uninteresting event : %08X", xev.type);
++#endif
++                break;
++            }
+         }
++        else
++            mir::log_error("input event received with no sink to handle it");
+     }
--    else
--        mir::log_error("input event received with no sink to handle it");
--
++    while(XPending(x11_connection.get()));
+ }
 === modified file 'tests/include/mir/test/doubles/mock_x11.h'
 --- tests/include/mir/test/doubles/mock_x11.h	2016-01-29 08:18:22 +0000
 +++ tests/include/mir/test/doubles/mock_x11.h	2016-02-24 09:00:08 +0000
@@ -46,6 +46,7 @@
      XEvent focus_in_event_return = { 0 };
      XEvent focus_out_event_return = { 0 };
      XEvent vscroll_event_return = { 0 };
++    XEvent motion_event_return = { 0 };
  };
  class MockX11
@@ -56,6 +57,7 @@
      MOCK_METHOD1(XOpenDisplay, Display*(const char*));
      MOCK_METHOD1(XCloseDisplay, int(Display*));
++    MOCK_METHOD1(XPending, int(Display*));
      MOCK_METHOD4(XGetVisualInfo, XVisualInfo*(Display*, long, XVisualInfo*, int*));
      MOCK_METHOD4(XCreateColormap, Colormap(Display*, Window, Visual*, int));
      /* Too long to mock, use wrapper instead.
 === modified file 'tests/mir_test_doubles/mock_x11.cpp'
 --- tests/mir_test_doubles/mock_x11.cpp	2016-01-29 08:18:22 +0000
 +++ tests/mir_test_doubles/mock_x11.cpp	2016-02-24 09:00:08 +0000
@@ -20,6 +20,8 @@
  #include "mir/test/doubles/mock_x11.h"
  #include <gtest/gtest.h>
++#include <cstring>
++
  namespace mtd=mir::test::doubles;
  namespace
@@ -31,6 +33,13 @@
      : display{reinterpret_cast<Display*>(0x12345678)},
        window{reinterpret_cast<Window>((long unsigned int)9876543210)}
+ {
++    std::memset(&keypress_event_return, 0, sizeof(XEvent));
++    std::memset(&button_release_event_return, 0, sizeof(XEvent));
++    std::memset(&expose_event_return, 0, sizeof(XEvent));
++    std::memset(&focus_in_event_return, 0, sizeof(XEvent));
++    std::memset(&focus_out_event_return, 0, sizeof(XEvent));
++    std::memset(&vscroll_event_return, 0, sizeof(XEvent));
++    std::memset(&motion_event_return, 0, sizeof(XEvent));
      visual_info.depth = 24;
      keypress_event_return.type = KeyPress;
      button_release_event_return.type = ButtonRelease;
@@ -40,6 +49,7 @@
      vscroll_event_return.type = ButtonPress;
      XButtonEvent& xbev = (XButtonEvent&)vscroll_event_return;
      xbev.button = Button4;
++    motion_event_return.type = MotionNotify;
+ }
  mtd::MockX11::MockX11()
@@ -171,3 +181,8 @@
+ {
      return global_mock->XSetWMHints(display, window, wmhints);
+ }
++
++int XPending(Display* display)
++{
++    return global_mock->XPending(display);
++}
 === modified file 'tests/unit-tests/input/test_x11_platform.cpp'
 --- tests/unit-tests/input/test_x11_platform.cpp	2016-01-29 08:18:22 +0000
 +++ tests/unit-tests/input/test_x11_platform.cpp	2016-02-24 09:00:08 +0000
@@ -174,6 +174,28 @@
      process_input_event();
+ }
++TEST_F(X11PlatformTest, motion_event_trigger_pointer_movement)
++{
++    prepare_event_processing();
++
++    InSequence seq;
++    mock_x11.fake_x11.motion_event_return.xmotion.x = 20;
++    mock_x11.fake_x11.motion_event_return.xmotion.y = 20;
++    EXPECT_CALL(mock_x11, XNextEvent(_,_))
++        .WillOnce(DoAll(SetArgPointee<1>(mock_x11.fake_x11.motion_event_return),
++                       Return(1)));
++    EXPECT_CALL(mock_pointer_sink, handle_input(mt::PointerEventWithDiff(20, 20)));
++    mock_x11.fake_x11.motion_event_return.xmotion.x = 40;
++    mock_x11.fake_x11.motion_event_return.xmotion.y = 25;
++    EXPECT_CALL(mock_x11, XNextEvent(_,_))
++        .WillOnce(DoAll(SetArgPointee<1>(mock_x11.fake_x11.motion_event_return),
++                       Return(1)));
++    EXPECT_CALL(mock_pointer_sink, handle_input(mt::PointerEventWithDiff(20, 5)));
++
++    process_input_event();
++    process_input_event();
++}
++
  TEST_F(X11PlatformTest, grabs_keyboard)
+ {
      prepare_event_processing();