Merge lp:~andreas-pokorny/mir/use-input-channel-to-send-input into lp:mir

[Andreas Pokorny]

Add send_event to InputChannel - to be able to dispatch user input events to the client surfaces.

Adds a method to the InputChannel to send MirEvents to clients. This is necessary to implement a custom InputDispatcher outside of mir - hence without access to the adopted android input stack.

[Kevin DuBois (kdub)]

send_event(uint32_t seq, MirEvent)
as someone calling send_event, how do I arrange seq; is a particular sequence needed? Would it be better to have AndroidInputChannel manage the sequence if a particular ordering is required?

review: Needs Information

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1545
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1315/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1580
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1578
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/1153
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1047
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/1047/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1052
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/1052/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1154
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/1154/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1069
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5972

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/1315/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

24 + virtual void send_event(uint32_t seq, MirEvent const& event) const = 0;

Like Kevin says: What is the significance of seq? Why do I need it to send an event?

~~~~

77 +void react_to_result(droidinput::status_t result)

This seems a bad name. Vis:

142 + react_to_result(publisher.publishMotionEvent(seq, ...));

"check_result" is slightly better, "check" possibly better still.

~~~~

90 + channel = new droidinput::InputChannel(droidinput::String8("TODO: Name"), s_fd);

"TODO: Name" is a distracting choice. What is going on here?

review: Needs Fixing

[Andreas Pokorny (andreas-pokorny)]

Kevin:
> send_event(uint32_t seq, MirEvent)
> as someone calling send_event, how do I arrange seq;
> is a particular sequence needed? Would it be better to have
> AndroidInputChannel manage the sequence if a particular ordering is required?
Alan:
> 24 + virtual void send_event(uint32_t seq, MirEvent const& event) const =
> 0;
>
> Like Kevin says: What is the significance of seq? Why do I need it to send an
> event?

I believe down - motion - up sequences of events need to have the same sequence number. So I suspect AndroidInputChannel could keep track of press / release event occurrences. The sequence number must be unique for some time, as the client has to respond using the sequence number, as a form of ACK to the server.

> ~~~~
>
> 77 +void react_to_result(droidinput::status_t result)
>
> This seems a bad name. Vis:
>
> 142 + react_to_result(publisher.publishMotionEvent(seq, ...));
>
> "check_result" is slightly better, "check" possibly better still.

ack

> ~~~~
>
> 90 + channel = new droidinput::InputChannel(droidinput::String8("TODO:
> Name"), s_fd);
>
> "TODO: Name" is a distracting choice. What is going on here?

just moved the code - I believe we could use the surface name here instead. fixing that.

[Robert Carr (robertcarr)]

We talked a little about seq_id on IRC:

< anpok> dandrader: i would keep the current mir::input::InputDispatcher rather stable wrt further refactorings, but InputChannel .. I have to change rather sooner..
18:52 < anpok> or the thing to dispatch the input too
18:52 < dandrader> anpok, what's wrong with InputChannel?
18:53 < racarr__> one thing is send_event(seq_id, MirEvent) is weird because the caller cant know how to use seq_id really so its weird as a public interface
18:53 < anpok> it has to be non null :) and should not collide with other input sequences
18:54 < anpok> and nobody reads the ACK signals that come back from the input reader..
18:54 < racarr__> indeed but they will
18:54 < anpok> I only discovered that part today
18:54 < dandrader> racarr__, true
18:54 < racarr__> and if it has to be non null and not collide and thats all
18:54 < racarr__> then it should just be hidden inside the object

[Robert Carr (robertcarr)]

79 (missing spaces), 143, 166 (wrong indentation), 310 (trailing line):
whitespace.

Definitely use surface named for TODO: Name now...that was an artifact of interface inadequacies long past...sorry for never coming and fixing my own todo ><. Only used for logging (which is redundantly named elsewhere iirc) anyway

Given seq_id cleanup as part of dispatcher split this is basically good to go to unblock qt compositor I think

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

That's too low level. It's the android::InputDispatcher responsibility to shepherd events through android::InputChannels.

What is needed is something like a mir::input::Surface::consume_event(MirEvent) that, under
the hood, calls eventually android::InputDispatcher::notifyMotion(event,
surface->input_channel). Doing exactly what
MirSurfaceItem::dispatchTouchEventToMirInputChannel from lp:~unity-team/+junk
/qpa-mirserver does.

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

So in the end we will have to go all the way and refactor android::InputDispatcher already now. :(

Making it only shepherd events through android::InputChannels (exactly like its stripped-down version existing in lp:~unity-team/+junk/qpa-mirserver) and leaving the responsibility of selecting which input channel gets the input event coming out of the android::InputReader to another entity (in the qt compositor case, the QQuickWindow dispatch logic that selects the qml item that will receive incoming input) that receives all input coming out of android::InputReader and calls mir::input::Surface::consume_event on the surface it decides that should receive the event, if any.

[Daniel d'Andrada (dandrader)]

> So in the end we will have to go all the way and refactor
> android::InputDispatcher already now. :(
>
> Making it only shepherd events through android::InputChannels (exactly like
> its stripped-down version existing in lp:~unity-team/+junk/qpa-mirserver) and
> leaving the responsibility of selecting which input channel gets the input
> event coming out of the android::InputReader to another entity (in the qt
> compositor case, the QQuickWindow dispatch logic that selects the qml item
> that will receive incoming input) that receives all input coming out of
> android::InputReader and calls mir::input::Surface::consume_event on the
> surface it decides that should receive the event, if any.

Then, that stripped down android::InputDispatcher should probably be renamed to something else to avoid confusion since the selection of the recipient of incoming input events is what a event/input dispatcher is supposed to do.

Unmerged revisions

1545. By Andreas Pokorny on 2014-04-11

Add send_event to InputChannel - for sending events to the client fd.

=== modified file 'include/server/mir/input/input_channel.h'
--- include/server/mir/input/input_channel.h	2013-08-28 03:41:48 +0000
+++ include/server/mir/input/input_channel.h	2014-04-11 16:00:27 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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,
@@ -19,6 +19,8 @@
 #ifndef MIR_INPUT_INPUT_CHANNEL_H_
 #define MIR_INPUT_INPUT_CHANNEL_H_
 
+#include "mir_toolkit/event.h"
+
 namespace mir
 {
 namespace input
@@ -32,6 +34,7 @@
 
     virtual int client_fd() const = 0;
     virtual int server_fd() const = 0;
+    virtual void send_event(uint32_t seq, MirEvent const& event) const = 0;
 
 protected:
     InputChannel() = default;

=== modified file 'include/test/mir_test_doubles/stub_input_channel.h'
--- include/test/mir_test_doubles/stub_input_channel.h	2013-08-28 03:41:48 +0000
+++ include/test/mir_test_doubles/stub_input_channel.h	2014-04-11 16:00:27 +0000
@@ -48,6 +48,9 @@
     {
         return input_fd;
     }
+    void send_event(uint32_t /*seq*/, MirEvent const& /*event*/) const override
+    {
+    }
     int input_fd;
 };
 

=== modified file 'src/server/input/android/android_input_channel.cpp'
--- src/server/input/android/android_input_channel.cpp	2014-03-06 06:05:17 +0000
+++ src/server/input/android/android_input_channel.cpp	2014-04-11 16:00:27 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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,
@@ -14,21 +14,44 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Robert Carr <[e-mail]>
+ *              Andreas Pokorny <[e-mail]>
  */
 
 #include "android_input_channel.h"
 
 #include <androidfw/InputTransport.h>
+#include <std/Errors.h>
+
+#include <boost/exception/errinfo_errno.hpp>
+#include <boost/throw_exception.hpp>
+
+#include <cmath>
 
 #include <unistd.h>
 
 namespace mia = mir::input::android;
 namespace droidinput = android;
 
+namespace
+{
+void react_to_result(droidinput::status_t result)
+{
+    if (result!=droidinput::OK)
+    {
+        BOOST_THROW_EXCEPTION(boost::enable_error_info(std::runtime_error("Failure sending input event ")) << boost::errinfo_errno(result));
+    }
+}
+}
+
 mia::AndroidInputChannel::AndroidInputChannel()
 {
-
     droidinput::InputChannel::openInputFdPair(s_fd, c_fd);
+    channel = new droidinput::InputChannel(droidinput::String8("TODO: Name"), s_fd);
+}
+
+droidinput::sp<droidinput::InputChannel> mia::AndroidInputChannel::get_android_channel() const
+{
+    return channel;
 }
 
 mia::AndroidInputChannel::~AndroidInputChannel()
@@ -46,3 +69,78 @@
 {
     return s_fd;
 }
+
+void mia::AndroidInputChannel::send_event(uint32_t seq, MirEvent const& event) const
+{
+    switch(event.type)
+    {
+    case mir_event_type_key:
+        send_key_event(seq, event.key);
+        break;
+    case mir_event_type_motion:
+        send_motion_event(seq, event.motion);
+        break;
+    default:
+        break;
+    }
+}
+
+void mia::AndroidInputChannel::send_motion_event(uint32_t seq, MirMotionEvent const& event) const
+{
+    droidinput::PointerCoords coords[MIR_INPUT_EVENT_MAX_POINTER_COUNT];
+    droidinput::PointerProperties properties[MIR_INPUT_EVENT_MAX_POINTER_COUNT];
+    for (size_t i = 0; i < event.pointer_count; i++)
+    {
+        // TODO this assumes that: x == raw_x + x_offset;
+        // here x,y is used instead of the raw co-ordinates and offset is set to zero
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_X, event.pointer_coordinates[i].x);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_Y, event.pointer_coordinates[i].y);
+
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_TOUCH_MAJOR, event.pointer_coordinates[i].touch_major);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_TOUCH_MINOR, event.pointer_coordinates[i].touch_minor);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_SIZE, event.pointer_coordinates[i].size);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_PRESSURE, event.pointer_coordinates[i].pressure);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_ORIENTATION, event.pointer_coordinates[i].orientation);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_VSCROLL, event.pointer_coordinates[i].vscroll);
+        coords[i].setAxisValue(AMOTION_EVENT_AXIS_HSCROLL, event.pointer_coordinates[i].hscroll);
+        properties[i].toolType = event.pointer_coordinates[i].tool_type;
+    }
+
+    droidinput::InputPublisher publisher(channel);
+
+    react_to_result(publisher.publishMotionEvent(seq,
+                                                 event.device_id,
+                                                 event.source_id,
+                                                 event.action,
+                                                 static_cast<int32_t>(event.flags),
+                                                 event.edge_flags,
+                                                 static_cast<int32_t>(event.modifiers),
+                                                 static_cast<int32_t>(event.button_state),
+                                                 0.0f,  // event.x_offset,
+                                                 0.0f,  // event.y_offset,
+                                                 event.x_precision,
+                                                 event.y_precision,
+                                                 event.down_time,
+                                                 event.event_time,
+                                                 event.pointer_count,
+                                                 properties,
+                                                 coords));
+}
+
+void mia::AndroidInputChannel::send_key_event(uint32_t seq, MirKeyEvent const& event) const
+{
+    droidinput::InputPublisher publisher(channel);
+
+    react_to_result(publisher.publishKeyEvent(seq,
+                                              event.device_id,
+                                              event.source_id,
+                                              event.action,
+                                              event.flags,
+                                              event.key_code,
+                                              event.scan_code,
+                                              event.modifiers,
+                                              event.repeat_count,
+                                              event.down_time,
+                                              event.event_time));
+}
+

=== modified file 'src/server/input/android/android_input_channel.h'
--- src/server/input/android/android_input_channel.h	2013-08-28 03:41:48 +0000
+++ src/server/input/android/android_input_channel.h	2014-04-11 16:00:27 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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,
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Robert Carr <[e-mail]>
+ *              Andreas Pokorny <[e-mail]>
  */
 
 #ifndef MIR_INPUT_ANDROID_INPUT_CHANNEL_H_
@@ -22,6 +23,7 @@
 #include "mir/input/input_channel.h"
 
 #include <utils/StrongPointer.h>
+#include <androidfw/InputTransport.h>
 
 namespace android
 {
@@ -43,15 +45,20 @@
     explicit AndroidInputChannel();
     virtual ~AndroidInputChannel();
 
-    int client_fd() const;
-    int server_fd() const;
+    int client_fd() const override;
+    int server_fd() const override;
+    void send_event(uint32_t seq, MirEvent const& event) const override;
 
+    droidinput::sp<droidinput::InputChannel> get_android_channel() const;
 protected:
     AndroidInputChannel(AndroidInputChannel const&) = delete;
     AndroidInputChannel& operator=(AndroidInputChannel const&) = delete;
 
 private:
     int s_fd, c_fd;
+    droidinput::sp<droidinput::InputChannel> channel;
+    void send_key_event(uint32_t seq, MirKeyEvent const& event) const;
+    void send_motion_event(uint32_t seq, MirMotionEvent const& event) const;
 };
 
 }

=== modified file 'src/server/input/android/android_input_window_handle.cpp'
--- src/server/input/android/android_input_window_handle.cpp	2014-03-06 06:05:17 +0000
+++ src/server/input/android/android_input_window_handle.cpp	2014-04-11 16:00:27 +0000
@@ -18,6 +18,7 @@
 
 #include "android_input_window_handle.h"
 #include "android_input_application_handle.h"
+#include "android_input_channel.h"
 
 #include "mir/input/input_channel.h"
 #include "mir/input/surface.h"
@@ -64,9 +65,16 @@
     {
         mInfo = new WindowInfo(surface);
 
-        // TODO: How can we avoid recreating the InputChannel which the InputChannelFactory has already created?
-        mInfo->inputChannel = new droidinput::InputChannel(droidinput::String8("TODO: Name"),
-                                                           input_channel->server_fd());
+        std::shared_ptr<AndroidInputChannel> droidchannel = std::dynamic_pointer_cast<AndroidInputChannel>(input_channel);
+        if (droidchannel)
+        {
+            mInfo->inputChannel = droidchannel->get_android_channel();
+        }
+        else
+        {
+            mInfo->inputChannel = new droidinput::InputChannel(droidinput::String8("TODO: Name"),
+                                                               input_channel->server_fd());
+        }
     }
 
     auto surface_size = surface->size();

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2014-04-04 14:42:55 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2014-04-11 16:00:27 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright © 2013 Canonical Ltd.
+ * Copyright © 2013-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
@@ -14,11 +14,13 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by: Robert Carr <[e-mail]>
+ *              Andreas Pokorny <[e-mail]>
  */
 
 #include "mir/graphics/display.h"
 #include "mir/shell/surface_creation_parameters.h"
 #include "mir/shell/placement_strategy.h"
+#include "mir/input/input_channel.h"
 #include "mir/scene/surface_coordinator.h"
 #include "mir/scene/surface.h"
 #include "src/server/scene/session_container.h"
@@ -613,3 +615,39 @@
 
     launch_client_process(*client);
 }
+
+TEST_F(TestClientInput, send_mir_events_with_input_channel)
+{
+    using namespace ::testing;
+
+    static std::string const test_client_name = "1";
+
+    mtf::CrossProcessSync fence;
+
+    MirEvent key_event;
+    memset(&key_event,0,sizeof(key_event));
+    key_event.type = mir_event_type_key;
+    key_event.key.action= mir_key_action_down;
+
+    auto server_config = make_event_producing_server(fence, 1,
+         [&](mtf::InputTestingServerConfiguration& server)
+         {
+             server.the_session_container()->for_each([&](std::shared_ptr<ms::Session> const& session) -> void
+                {
+                    session->default_surface()->input_channel()->send_event(1,key_event);
+                });
+
+         });
+    launch_server_process(*server_config);
+
+    auto client_config = make_event_expecting_client(fence,
+         [&](MockHandler& handler, mt::WaitCondition& events_received)
+         {
+             using namespace ::testing;
+             InSequence seq;
+             EXPECT_CALL(handler, handle_input(mt::KeyDownEvent())).Times(1)
+                 .WillOnce(mt::WakeUp(&events_received));
+
+         });
+    launch_client_process(*client_config);
+}

=== modified file 'tests/mir_test_framework/stubbed_server_configuration.cpp'
--- tests/mir_test_framework/stubbed_server_configuration.cpp	2014-03-27 09:46:09 +0000
+++ tests/mir_test_framework/stubbed_server_configuration.cpp	2014-04-11 16:00:27 +0000
@@ -217,14 +217,18 @@
 
 struct StubInputChannel : public mi::InputChannel
 {
-    int client_fd() const
-    {
-        return 0;
-    }
-
-    int server_fd() const
-    {
-        return 0;
+    int client_fd() const override
+    {
+        return 0;
+    }
+
+    int server_fd() const override
+    {
+        return 0;
+    }
+
+    void send_event(uint32_t /*seq*/, MirEvent const& /*event*/) const override
+    {
     }
 };
 

=== modified file 'tests/unit-tests/input/android/test_android_input_window_handle.cpp'
--- tests/unit-tests/input/android/test_android_input_window_handle.cpp	2014-03-26 14:20:14 +0000
+++ tests/unit-tests/input/android/test_android_input_window_handle.cpp	2014-04-11 16:00:27 +0000
@@ -52,6 +52,7 @@
 {
     MOCK_CONST_METHOD0(client_fd, int());
     MOCK_CONST_METHOD0(server_fd, int());
+    MOCK_CONST_METHOD2(send_event,void(uint32_t /*seq*/, MirEvent const& /*event*/));
 };
 
 }

=== modified file 'tests/unit-tests/scene/test_surface.cpp'
--- tests/unit-tests/scene/test_surface.cpp	2014-03-31 16:51:06 +0000
+++ tests/unit-tests/scene/test_surface.cpp	2014-04-11 16:00:27 +0000
@@ -52,6 +52,7 @@
 {
     MOCK_CONST_METHOD0(server_fd, int());
     MOCK_CONST_METHOD0(client_fd, int());
+    MOCK_CONST_METHOD2(send_event,void(uint32_t /*seq*/, MirEvent const& /*event*/));
 };
 }