Merge lp:~alan-griffiths/unity-system-compositor/constructors-and-destructors-for-initialization-and-finalization into lp:unity-system-compositor

[Alan Griffiths]

Use a SystemCompositor constructor for initializing the system compositor and a destructor for finalizing it.

Use a SystemCompositor constructor for initializing the system compositor and a destructor for finalizing it.

NB the prerequisite needs Mir v0.1.1

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:100
http://s-jenkins:8080/job/unity-system-compositor-ci/59/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/unity-system-compositor-trusty-amd64-ci/11/console
    SUCCESS: http://s-jenkins:8080/job/unity-system-compositor-trusty-armhf-ci/11

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-system-compositor-ci/59/rebuild

review: Needs Fixing (continuous-integration)

[lp:~alan-griffiths/unity-system-compositor/constructors-and-destructors-for-initialization-and-finalization]

101. By Alan Griffiths on 2013-11-26

merge -c 96 lp:unity-system-compositor

102. By Alan Griffiths on 2013-11-26

merge -c 97 lp:unity-system-compositor

103. By Alan Griffiths on 2013-11-26

merge -c 98 lp:unity-system-compositor

104. By Alan Griffiths on 2013-11-26

merge -c 99 lp:unity-system-compositor

105. By Alan Griffiths on 2013-11-26

merge -c 100 lp:unity-system-compositor

106. By Alan Griffiths on 2013-11-26

merge -c 101 lp:unity-system-compositor

107. By Alan Griffiths on 2013-11-26

merge -c 102 lp:unity-system-compositor

108. By Alan Griffiths on 2013-11-26

merge -c 103 lp:unity-system-compositor

109. By Alan Griffiths on 2013-11-26

merge -c 104 lp:unity-system-compositor

110. By Alan Griffiths on 2013-11-26

merge -c 105 lp:unity-system-compositor

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:110
http://jenkins.qa.ubuntu.com/job/unity-system-compositor-ci/68/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-trusty-amd64-ci/20
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-system-compositor-trusty-armhf-ci/20

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-system-compositor-ci/68/rebuild

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Too many conflicts with current code

Unmerged revisions

110. By Alan Griffiths on 2013-11-26

merge -c 105 lp:unity-system-compositor

109. By Alan Griffiths on 2013-11-26

merge -c 104 lp:unity-system-compositor

108. By Alan Griffiths on 2013-11-26

merge -c 103 lp:unity-system-compositor

107. By Alan Griffiths on 2013-11-26

merge -c 102 lp:unity-system-compositor

106. By Alan Griffiths on 2013-11-26

merge -c 101 lp:unity-system-compositor

105. By Alan Griffiths on 2013-11-26

merge -c 100 lp:unity-system-compositor

104. By Alan Griffiths on 2013-11-26

merge -c 99 lp:unity-system-compositor

103. By Alan Griffiths on 2013-11-26

merge -c 98 lp:unity-system-compositor

102. By Alan Griffiths on 2013-11-26

merge -c 97 lp:unity-system-compositor

101. By Alan Griffiths on 2013-11-26

merge -c 96 lp:unity-system-compositor

=== modified file 'src/dbus_screen.cpp'
--- src/dbus_screen.cpp	2013-10-31 19:16:23 +0000
+++ src/dbus_screen.cpp	2013-11-26 12:17:27 +0000
@@ -27,9 +27,10 @@
 namespace mg = mir::graphics;
 
 // Note: this class should be created only after when the Mir DisplayServer has started
-DBusScreen::DBusScreen(std::shared_ptr<mir::DefaultServerConfiguration> config, QObject *parent)
+DBusScreen::DBusScreen(mir::DefaultServerConfiguration& config, QObject *parent)
     : QObject(parent)
-    , config(config)
+    , display(config.the_display())
+
 {
     new DBusScreenAdaptor(this);
     QDBusConnection bus = QDBusConnection::systemBus();
@@ -55,7 +56,6 @@
         return false;
     }
 
-    std::shared_ptr<mg::Display> display = config->the_display();
     std::shared_ptr<mg::DisplayConfiguration> displayConfig = display->configuration();
 
     displayConfig->for_each_output([&](const mg::DisplayConfigurationOutput displayConfigOutput) {

=== modified file 'src/dbus_screen.h'
--- src/dbus_screen.h	2013-10-31 19:16:23 +0000
+++ src/dbus_screen.h	2013-11-26 12:17:27 +0000
@@ -23,6 +23,7 @@
 namespace mir
 {
     class DefaultServerConfiguration;
+namespace graphics { class Display; }
 }
 
 class DBusScreen : public QObject
@@ -31,13 +32,13 @@
     Q_CLASSINFO("D-Bus Interface", "com.canonical.Unity.Screen")
 
 public:
-    explicit DBusScreen(std::shared_ptr<mir::DefaultServerConfiguration> config, QObject *parent = 0);
+    explicit DBusScreen(mir::DefaultServerConfiguration& config, QObject *parent = 0);
 
 public Q_SLOTS:
     bool setScreenPowerMode(const QString &mode);
 
 private:
-    std::shared_ptr<mir::DefaultServerConfiguration> config;
+    std::shared_ptr<mir::graphics::Display> const display;
 };
 
 #endif /* DBUS_SCREEN_H_ */

=== modified file 'src/main.cpp'
--- src/main.cpp	2013-10-31 22:00:19 +0000
+++ src/main.cpp	2013-11-26 12:17:27 +0000
@@ -20,11 +20,12 @@
 #include <mir/report_exception.h>
 #include <iostream>
 
-int main(int argc, char *argv[])
+int main(int argc, char const* argv[])
 try
 {
-    SystemCompositor system_compositor;
-    system_compositor.run(argc, argv);
+    SystemCompositor system_compositor(argc, argv);
+
+    system_compositor.run();
 
     return 0;
 }

=== modified file 'src/system_compositor.cpp'
--- src/system_compositor.cpp	2013-11-22 16:53:52 +0000
+++ src/system_compositor.cpp	2013-11-26 12:17:27 +0000
@@ -87,8 +87,8 @@
 class SystemCompositorServerConfiguration : public mir::DefaultServerConfiguration
 {
 public:
-    SystemCompositorServerConfiguration(SystemCompositor *compositor, int argc, char **argv)
-        : mir::DefaultServerConfiguration(argc, (char const **)argv), compositor{compositor}
+    SystemCompositorServerConfiguration(SystemCompositor *compositor, int argc, char const** argv)
+        : mir::DefaultServerConfiguration(argc, argv), compositor{compositor}
     {
         add_options()
             ("from-dm-fd", po::value<int>(),  "File descriptor of read end of pipe from display manager [int]")
@@ -108,11 +108,6 @@
         return the_options()->get("to-dm-fd", -1);
     }
 
-    bool show_version()
-    {
-        return the_options()->is_set ("version");
-    }
-
     std::string blacklist()
     {
         auto x = the_options()->get ("blacklist", "");
@@ -130,6 +125,11 @@
         setenv("MIR_SERVER_STANDALONE", "true", 0); // Default to standalone
         mir::DefaultServerConfiguration::parse_options(options_description, options);
         options.parse_file(options_description, "unity-system-compositor.conf");
+
+        if (options.is_set("version"))
+        {
+            throw mir::AbnormalExit (std::string("unity-system-compositor ") + USC_VERSION);
+        }
     }
 
     std::shared_ptr<mi::CursorListener> the_cursor_listener() override
@@ -227,50 +227,37 @@
     return true;
 }
 
-void SystemCompositor::run(int argc, char **argv)
-{
-    config = std::make_shared<SystemCompositorServerConfiguration>(this, argc, argv);
-  
-    if (config->show_version())
-    {
-        std::cerr << "unity-system-compositor " << USC_VERSION << std::endl;
-        return;
-    }
-
-    dm_connection = std::make_shared<DMConnection>(io_service, config->from_dm_fd(), config->to_dm_fd());
-
-    struct ScopeGuard
-    {
-        explicit ScopeGuard(boost::asio::io_service& io_service) : io_service(io_service) {}
-        ~ScopeGuard()
-        {
-            io_service.stop();
-            if (io_thread.joinable())
-                io_thread.join();
-            if (qt_thread.joinable())
-                qt_thread.join();
-        }
-
-        boost::asio::io_service& io_service;
-        std::thread io_thread;
-        std::thread qt_thread;
-    } guard(io_service);
-
-    mir::run_mir(*config, [&](mir::DisplayServer&)
-        {
-            auto vendor = (char *) glGetString(GL_VENDOR);
-            auto renderer = (char *) glGetString (GL_RENDERER);
-            auto version = (char *) glGetString (GL_VERSION);
-            std::cerr << "GL_VENDOR = " << vendor << std::endl;
-            std::cerr << "GL_RENDERER = " << renderer << std::endl;
-            std::cerr << "GL_VERSION = " << version << std::endl;
-
-            if (!check_blacklist(config->blacklist(), vendor, renderer, version))
-                throw mir::AbnormalExit ("Video driver is blacklisted, exiting");
-
-            shell = config->the_system_compositor_shell();
-            guard.io_thread = std::thread(&SystemCompositor::main, this);
-            guard.qt_thread = std::thread(&SystemCompositor::qt_main, this, argc, argv);
+SystemCompositor::SystemCompositor(int argc, char const** argv) :
+    argc(argc),
+    argv(argv),
+    config{new SystemCompositorServerConfiguration(this, argc, argv)},
+    shell{config->the_system_compositor_shell()},
+    dm_connection{std::make_shared<DMConnection>(io_service, config->from_dm_fd(), config->to_dm_fd())}
+{
+    auto vendor = (char *) glGetString(GL_VENDOR);
+    auto renderer = (char *) glGetString (GL_RENDERER);
+    auto version = (char *) glGetString (GL_VERSION);
+    std::cerr << "GL_VENDOR = " << vendor << std::endl;
+    std::cerr << "GL_RENDERER = " << renderer << std::endl;
+    std::cerr << "GL_VERSION = " << version << std::endl;
+
+    if (!check_blacklist(config->blacklist(), vendor, renderer, version))
+        throw mir::AbnormalExit ("Video driver is blacklisted, exiting");
+}
+
+SystemCompositor::~SystemCompositor() noexcept
+{
+    io_service.stop();
+    if (thread.joinable()) thread.join();
+    if (qt_thread.joinable()) qt_thread.join();
+}
+
+void SystemCompositor::run()
+{
+    mir::run_mir(*config, [this](mir::DisplayServer&)
+        {
+            thread = std::thread(&SystemCompositor::main, this);
+            qt_thread = std::thread(&SystemCompositor::qt_main, this);
         });
 }
 
@@ -294,6 +281,7 @@
 {
     std::cerr << "set_active_session" << std::endl;
 
+    // TODO the Mir Session hierarchy is odd - we shouldn't really need this cast.
     active_session = std::static_pointer_cast<mir::shell::Session>(shell->session_named(client_name));
 
     if (active_session)
@@ -327,9 +315,9 @@
     io_service.run();
 }
 
-void SystemCompositor::qt_main(int argc, char **argv)
+void SystemCompositor::qt_main()
 {
-    QCoreApplication app(argc, argv);
-    DBusScreen dbus_screen(config);
+    QCoreApplication app(argc, const_cast<char**>(argv));
+    DBusScreen dbus_screen(*config);
     app.exec();
 }

=== modified file 'src/system_compositor.h'
--- src/system_compositor.h	2013-11-22 16:53:52 +0000
+++ src/system_compositor.h	2013-11-26 12:17:27 +0000
@@ -21,6 +21,8 @@
 
 #include "dm_connection.h"
 
+#include <thread>
+
 namespace mir { namespace shell { class Session; } }
 
 class SystemCompositorShell;
@@ -29,21 +31,28 @@
 class SystemCompositor : public DMMessageHandler
 {
 public:
-    void run(int argc, char **argv);
+    SystemCompositor(int argc, char const** argv);
+    ~SystemCompositor() noexcept;
+    void run();
     void pause();
     void resume();
 
 private:
-    std::shared_ptr<SystemCompositorServerConfiguration> config;
-    std::shared_ptr<SystemCompositorShell> shell;
+    int argc;
+    char const** const argv;
+    std::unique_ptr<SystemCompositorServerConfiguration> const config;
+    std::shared_ptr<SystemCompositorShell> const shell;
+    std::shared_ptr<DMConnection> const dm_connection;
+
     boost::asio::io_service io_service;
-    std::shared_ptr<DMConnection> dm_connection;
+    std::thread thread;
+    std::thread qt_thread;
     std::shared_ptr<mir::shell::Session> active_session;
 
     void set_active_session(std::string client_name);
     void set_next_session(std::string client_name);
     void main();
-    void qt_main(int argc, char **argv);
+    void qt_main();
 };
 
 #endif /* SYSTEM_COMPOSITOR_H_ */