Merge lp:~robert-ancell/mir/alt-ctrl-backspace into lp:~mir-team/mir/trunk

[Robert Ancell]

Quit demo servers on alt-ctrl-backspace

[Robert Ancell (robert-ancell)]

It would have been nicer to put this into run_mir() so the code isn't copied everywhere. But since the filter needs to be in the configuration object it has to be defined in each application.

[Alexandros Frantzis (afrantzis)]

In https://code.launchpad.net/~afrantzis/mir/multimonitor-examples/+merge/178039 (not merged yet) I have added a base mir::examples::ServerConfiguration which all the demo servers and standalone programs use. We could put the filters there. In order to allow derived configuration classes to extend the filter list, we need to change the filter list interface to use a vector, or other manipulable object, instead of an initializer list.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:909
http://jenkins.qa.ubuntu.com/job/mir-ci/1170/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1540
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1425
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/408
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/408/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Robert Ancell (robert-ancell)]

> In https://code.launchpad.net/~afrantzis/mir/multimonitor-
> examples/+merge/178039 (not merged yet) I have added a base
> mir::examples::ServerConfiguration which all the demo servers and standalone
> programs use. We could put the filters there. In order to allow derived
> configuration classes to extend the filter list, we need to change the filter
> list interface to use a vector, or other manipulable object, instead of an
> initializer list.

Sounds good!

[Alan Griffiths (alan-griffiths)]

> > In https://code.launchpad.net/~afrantzis/mir/multimonitor-
> > examples/+merge/178039 (not merged yet) I have added a base
> > mir::examples::ServerConfiguration which all the demo servers and standalone
> > programs use. We could put the filters there.
...
> Sounds good!

The above has landed, so is this the plan?

It will address the duplication of code.

~~~~

18 +static mir::DisplayServer *server_ptr{nullptr};

I'm not a fan of mutable static data, and this seems unnecessary as quit_filter can be captured by the init lambda and could have set_server()/server_ptr members.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:910
http://jenkins.qa.ubuntu.com/job/mir-ci/1198/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1579
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1464
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/436
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/436/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> > > In https://code.launchpad.net/~afrantzis/mir/multimonitor-
> > > examples/+merge/178039 (not merged yet) I have added a base
> > > mir::examples::ServerConfiguration which all the demo servers and
> standalone
> > > programs use. We could put the filters there.
> ...
> > Sounds good!
>
> The above has landed, so is this the plan?
>
> It will address the duplication of code.

Yes, that my plan at least. I have started working on it, so assuming all goes well this MP will be obsoleted.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:911
http://jenkins.qa.ubuntu.com/job/mir-ci/1200/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1581
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1466
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/438
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/438/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I think we should only be concerned with examples/demo-shell/ right now (demo_server_shell)

The purpose of demo_server is to show what a minimal server looks like. So adding complexity there is probably not a good idea. I would like to see the changes to examples/*.{h,cpp} reverted then.

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

> I think we should only be concerned with examples/demo-shell/ right now
> (demo_server_shell)
>
> The purpose of demo_server is to show what a minimal server looks like. So
> adding complexity there is probably not a good idea. I would like to see the
> changes to examples/*.{h,cpp} reverted then.

Good point. This definitely blurs the line between what the Mir library should provide and the minimal client code required to use it.

I think it would be better for the library to provide a default and allow it to be overridden than for every client to need to provide something similar to this.

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

The plan was to add this to the mir::examples::ServerConfiguration (after changing the interface so that derived ServerConfigurations can add more filters), so that all example servers and standalone programs can benefit from this transparently.

Alternatively, as mentioned, we can add support for this in mir::DefaultServerConfiguration, depending on what we want the default policy to be. I am OK with both.

[Robert Ancell (robert-ancell)]

Now updated with Alexandros' changes so the filter can be shared between all the examples.

I don't think this should be in mir::DefaultServerConfiguration since we don't want a magic key combination to kill Mir. For "real" Mir servers you should VT switch and kill the server. I'll make a separate MP to add VT switching key combinations by default.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:912
http://jenkins.qa.ubuntu.com/job/mir-ci/1225/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1617/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1502
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/463/console

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

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Now mir_demo_server* all seem to exit immediately (without crashing!?), exit status zero.

Try merging lp:mir again?

review: Needs Fixing

[Alexandros Frantzis (afrantzis)]

212 + std::shared_ptr<QuitFilter> quit_filter;

The quit filter doesn't seem to get created...

Anyway, I will propose an alternative MP for this, removing the need for the set_server() calls. Stay tuned.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:913
http://jenkins.qa.ubuntu.com/job/mir-ci/1233/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1629
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1514
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/471
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/471/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Alternative MP at: https://code.launchpad.net/~afrantzis/mir/examples-quit-filter/+merge/178908

=== modified file 'examples/demo-inprocess-surface-client/CMakeLists.txt'
--- examples/demo-inprocess-surface-client/CMakeLists.txt	2013-05-22 14:21:16 +0000
+++ examples/demo-inprocess-surface-client/CMakeLists.txt	2013-08-07 07:20:37 +0000
@@ -4,6 +4,7 @@
   demo_inprocess_surface_client.cpp
   inprocess_egl_client.cpp
   example_egl_helper.cpp
+  ../server_configuration.cpp
 )
 
 target_link_libraries(mir_demo_standalone_inprocess_egl

=== modified file 'examples/demo-inprocess-surface-client/demo_inprocess_surface_client.cpp'
--- examples/demo-inprocess-surface-client/demo_inprocess_surface_client.cpp	2013-05-13 20:40:37 +0000
+++ examples/demo-inprocess-surface-client/demo_inprocess_surface_client.cpp	2013-08-07 07:20:37 +0000
@@ -17,11 +17,11 @@
  */
 
 #include "inprocess_egl_client.h"
+#include "../server_configuration.h"
 
 #include "mir/run_mir.h"
-#include "mir/default_server_configuration.h"
-
 #include "mir/report_exception.h"
+
 #include <iostream>
 
 namespace me = mir::examples;
@@ -40,11 +40,12 @@
 try
 {
     ///\internal [main_tag]
-    mir::DefaultServerConfiguration config(argc, argv);
+    me::ServerConfiguration config(argc, argv);
 
     std::shared_ptr<me::InprocessEGLClient> client;
-    mir::run_mir(config, [&config, &client](mir::DisplayServer&)
+    mir::run_mir(config, [&config, &client](mir::DisplayServer& display_server)
     {
+        config.set_server(display_server);
         client = std::make_shared<me::InprocessEGLClient>(
               config.the_main_loop(),
               config.the_graphics_platform(),

=== modified file 'examples/demo-shell/demo_shell.cpp'
--- examples/demo-shell/demo_shell.cpp	2013-08-06 10:39:53 +0000
+++ examples/demo-shell/demo_shell.cpp	2013-08-07 07:20:37 +0000
@@ -92,8 +92,10 @@
     auto wm = std::make_shared<me::WindowManager>();
     me::DemoServerConfiguration config(argc, argv, {wm});
 
-    mir::run_mir(config, [&config, &wm](mir::DisplayServer&)
+    mir::run_mir(config, [&config, &wm](mir::DisplayServer& display_server)
         {
+            config.set_server(display_server);
+
             // We use this strange two stage initialization to avoid a circular dependency between the EventFilters
             // and the SessionStore
             wm->set_focus_controller(config.the_focus_controller());

=== modified file 'examples/demo_input_filter.cpp'
--- examples/demo_input_filter.cpp	2013-08-06 10:39:53 +0000
+++ examples/demo_input_filter.cpp	2013-08-07 07:20:37 +0000
@@ -74,7 +74,7 @@
 {
     DemoServerConfiguration config(argc, argv);
 
-    mir::run_mir(config, [](mir::DisplayServer&) {/* empty init */});
+    mir::run_mir(config, [&](mir::DisplayServer& display_server) { config.set_server(display_server); });
     return 0;
 }
 catch (mir::AbnormalExit const& error)

=== modified file 'examples/demo_server.cpp'
--- examples/demo_server.cpp	2013-08-01 11:05:45 +0000
+++ examples/demo_server.cpp	2013-08-07 07:20:37 +0000
@@ -17,6 +17,7 @@
  */
 
 #include "mir/run_mir.h"
+#include "mir/display_server.h"
 #include "mir/report_exception.h"
 #include "server_configuration.h"
 
@@ -27,7 +28,7 @@
 {
     mir::examples::ServerConfiguration config(argc, argv);
 
-    run_mir(config, [](mir::DisplayServer&) {/* empty init */});
+    run_mir(config, [&](mir::DisplayServer& display_server) { config.set_server(display_server); });
     return 0;
 }
 catch (...)

=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp	2013-08-01 11:05:45 +0000
+++ examples/render_surfaces.cpp	2013-08-07 07:20:37 +0000
@@ -464,8 +464,10 @@
     ///\internal [main_tag]
     RenderSurfacesServerConfiguration conf{argc, argv};
 
-    mir::run_mir(conf, [&](mir::DisplayServer&)
+    mir::run_mir(conf, [&](mir::DisplayServer& display_server)
     {
+        conf.set_server(display_server);
+
         conf.create_surfaces();
 
         cursor = conf.the_cursor();

=== modified file 'examples/server_configuration.cpp'
--- examples/server_configuration.cpp	2013-08-01 11:05:45 +0000
+++ examples/server_configuration.cpp	2013-08-07 07:20:37 +0000
@@ -17,13 +17,17 @@
  */
 
 #include "server_configuration.h"
+#include "mir/display_server.h"
 #include "mir/graphics/display_configuration_policy.h"
 #include "mir/graphics/display_configuration.h"
+#include "mir/input/composite_event_filter.h"
 
 #include <string>
+#include <linux/input.h>
 
 namespace me = mir::examples;
 namespace mg = mir::graphics;
+namespace mi = mir::input;
 namespace geom = mir::geometry;
 
 namespace
@@ -88,6 +92,21 @@
 
 }
 
+bool me::QuitFilter::handle(MirEvent const& event)
+{
+    if (event.type == mir_event_type_key &&
+        event.key.action == mir_key_action_down &&
+        (event.key.modifiers & mir_key_modifier_alt) &&
+        (event.key.modifiers & mir_key_modifier_ctrl) &&
+        event.key.scan_code == KEY_BACKSPACE)
+    {
+        server_ptr->stop();
+        return true;
+    }
+
+    return false;
+}
+
 me::ServerConfiguration::ServerConfiguration(int argc, char const** argv)
     : DefaultServerConfiguration(argc, argv)
 {
@@ -114,3 +133,12 @@
                 return DefaultServerConfiguration::the_display_configuration_policy();
         });
 }
+
+std::shared_ptr<mi::CompositeEventFilter>
+me::ServerConfiguration::the_composite_event_filter()
+{
+    auto composite_filter = mir::DefaultServerConfiguration::the_composite_event_filter();
+    composite_filter->prepend(quit_filter);
+
+    return composite_filter;
+}

=== modified file 'examples/server_configuration.h'
--- examples/server_configuration.h	2013-08-01 11:05:45 +0000
+++ examples/server_configuration.h	2013-08-07 07:20:37 +0000
@@ -20,18 +20,43 @@
 #define MIR_EXAMPLES_SERVER_CONFIGURATION_H_
 
 #include "mir/default_server_configuration.h"
+#include "mir/input/event_filter.h"
 
 namespace mir
 {
+class DisplayServer;
+
 namespace examples
 {
+class QuitFilter : public mir::input::EventFilter
+{
+public:
+    bool handle(MirEvent const& event) override;
+
+    void set_server(DisplayServer& display_server)
+    {
+        server_ptr = &display_server;
+    }
+
+private:
+    mir::DisplayServer *server_ptr;
+};
 
 class ServerConfiguration : public DefaultServerConfiguration
 {
 public:
     ServerConfiguration(int argc, char const** argv);
 
+    void set_server(DisplayServer& display_server)
+    {
+        quit_filter->set_server(display_server);
+    }
+
     std::shared_ptr<graphics::DisplayConfigurationPolicy> the_display_configuration_policy() override;
+    std::shared_ptr<input::CompositeEventFilter> the_composite_event_filter() override;
+
+private:
+    std::shared_ptr<QuitFilter> quit_filter;
 };
 
 }