Merge lp:~alan-griffiths/mir/tidy-up-nested-class-responsibilities into lp:~mir-team/mir/trunk

[Alan Griffiths]

graphics: tidy up the roles and responsibilities of nested classes

graphics: tidy up the roles and responsibilities of nested classes

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1047
http://jenkins.qa.ubuntu.com/job/mir-ci/1428/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1928
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1813
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/666
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/666/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Eleni Maria Stea (hikiko)]

nice you moved this piece of code, looks good.

review: Approve

=== modified file 'src/server/graphics/nested/CMakeLists.txt'
--- src/server/graphics/nested/CMakeLists.txt	2013-08-30 09:01:48 +0000
+++ src/server/graphics/nested/CMakeLists.txt	2013-08-30 17:00:40 +0000
@@ -7,6 +7,7 @@
 
   nested_display.cpp
   nested_display_configuration.cpp
+  nested_output.cpp
   nested_platform.cpp
   host_connection.cpp
 )

=== modified file 'src/server/graphics/nested/nested_display.cpp'
--- src/server/graphics/nested/nested_display.cpp	2013-08-29 03:48:16 +0000
+++ src/server/graphics/nested/nested_display.cpp	2013-08-30 17:00:40 +0000
@@ -18,70 +18,21 @@
 
 #include "nested_display.h"
 #include "nested_display_configuration.h"
+#include "nested_output.h"
 #include "mir_api_wrappers.h"
 
 #include "mir/geometry/rectangle.h"
 #include "mir/graphics/gl_context.h"
+#include "mir/graphics/nested/host_connection.h"
 
 #include <boost/throw_exception.hpp>
 #include <stdexcept>
-#include <atomic>
 
 namespace mg = mir::graphics;
 namespace mgn = mir::graphics::nested;
 namespace mgnw = mir::graphics::nested::mir_api_wrappers;
 namespace geom = mir::geometry;
 
-namespace
-{
-EGLint const egl_attribs[] = {
-    EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
-    EGL_RED_SIZE, 8,
-    EGL_GREEN_SIZE, 8,
-    EGL_BLUE_SIZE, 8,
-    EGL_ALPHA_SIZE, 8,
-    EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
-    EGL_NONE
-};
-
-EGLint const egl_context_attribs[] = {
-    EGL_CONTEXT_CLIENT_VERSION, 2,
-    EGL_NONE
-};
-}
-
-
-mgn::detail::MirSurfaceHandle::MirSurfaceHandle(MirConnection* connection, DisplayConfigurationOutput const& output)
-{
-    auto const& egl_display_mode = output.modes[output.current_mode_index];
-    auto const egl_display_format = output.pixel_formats[output.current_format_index];
-
-    MirSurfaceParameters const request_params =
-        {
-            "Mir nested display",
-            egl_display_mode.size.width.as_int(),
-            egl_display_mode.size.height.as_int(),
-            MirPixelFormat(egl_display_format),
-            mir_buffer_usage_hardware,
-            static_cast<uint32_t>(output.id.as_value())
-        };
-
-    mir_surface = mir_connection_create_surface_sync(connection, &request_params);
-
-    if (!mir_surface_is_valid(mir_surface))
-        BOOST_THROW_EXCEPTION(std::runtime_error(mir_surface_get_error_message(mir_surface)));
-}
-
-mgn::detail::MirSurfaceHandle::~MirSurfaceHandle() noexcept
-{
-    mir_surface_release_sync(mir_surface);
-}
-
-namespace
-{
-std::atomic<int> display_handles{-1};
-}
-
 mgn::detail::EGLDisplayHandle::EGLDisplayHandle(MirConnection* connection)
 {
     auto const native_display = (EGLNativeDisplayType) mir_connection_get_egl_native_display(connection);
@@ -91,8 +42,6 @@
     egl_display = eglGetDisplay(native_display);
     if (egl_display == EGL_NO_DISPLAY)
         BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to fetch EGL display."));
-
-    display_handles.fetch_add(1);
 }
 
 void mgn::detail::EGLDisplayHandle::initialize() const
@@ -133,62 +82,16 @@
 
 mgn::detail::EGLDisplayHandle::~EGLDisplayHandle() noexcept
 {
-    if (!display_handles.fetch_add(-1)) eglTerminate(egl_display);
-}
-
-mgn::detail::NestedOutput::NestedOutput(MirConnection* connection, DisplayConfigurationOutput const& output) :
-    mir_surface(connection, output),
-    egl_display{connection},
-    egl_config{(egl_display.initialize(), egl_display.choose_config(egl_attribs))},
-    egl_context{egl_display, eglCreateContext(egl_display, egl_config, EGL_NO_CONTEXT, egl_context_attribs)},
-    area{output.top_left, output.modes[output.current_mode_index].size},
-    egl_surface{EGL_NO_SURFACE}
-{
-}
-
-geom::Rectangle mgn::detail::NestedOutput::view_area() const
-{
-    return area;
-}
-
-void mgn::detail::NestedOutput::make_current()
-{
-    egl_surface = egl_display.egl_surface(egl_config, mir_surface);
-
-    if (eglMakeCurrent(egl_display, egl_surface, egl_surface, egl_context) != EGL_TRUE)
-        BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to update EGL surface.\n"));
-}
-
-void mgn::detail::NestedOutput::release_current()
-{
-    eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
-    eglDestroySurface(egl_display, egl_surface);
-    egl_surface = EGL_NO_SURFACE;
-}
-
-void mgn::detail::NestedOutput::post_update()
-{
-    mir_surface_swap_buffers_sync(mir_surface);
-}
-
-bool mgn::detail::NestedOutput::can_bypass() const
-{
-    // TODO we really should return "true" - but we need to support bypass properly then
-    return false;
-}
-
-
-mgn::detail::NestedOutput::~NestedOutput() noexcept
-{
-    if (egl_surface != EGL_NO_SURFACE)
-        eglDestroySurface(egl_display, egl_surface);
-}
-
-mgn::NestedDisplay::NestedDisplay(MirConnection* connection, std::shared_ptr<mg::DisplayReport> const& display_report) :
+    eglTerminate(egl_display);
+}
+
+mgn::NestedDisplay::NestedDisplay(std::shared_ptr<HostConnection> const& connection, std::shared_ptr<mg::DisplayReport> const& display_report) :
     connection{connection},
     display_report{display_report},
+    egl_display{*connection},
     outputs{}
 {
+    egl_display.initialize();
     configure(*configuration());
 }
 
@@ -204,7 +107,7 @@
 
 std::shared_ptr<mg::DisplayConfiguration> mgn::NestedDisplay::configuration()
 {
-    return std::make_shared<NestedDisplayConfiguration>(mir_connection_create_display_config(connection));
+    return std::make_shared<NestedDisplayConfiguration>(mir_connection_create_display_config(*connection));
 }
 
 void mgn::NestedDisplay::configure(mg::DisplayConfiguration const& configuration)
@@ -219,7 +122,27 @@
         {
             if (output.used)
             {
-                result[output.id] = std::make_shared<mgn::detail::NestedOutput>(connection, output);
+                geometry::Rectangle const area{output.top_left, output.modes[output.current_mode_index].size};
+
+                auto const& egl_display_mode = output.modes[output.current_mode_index];
+                auto const egl_display_format = output.pixel_formats[output.current_format_index];
+
+                MirSurfaceParameters const request_params =
+                    {
+                        "Mir nested display",
+                        egl_display_mode.size.width.as_int(),
+                        egl_display_mode.size.height.as_int(),
+                        MirPixelFormat(egl_display_format),
+                        mir_buffer_usage_hardware,
+                        static_cast<uint32_t>(output.id.as_value())
+                    };
+
+                auto const mir_surface = mir_connection_create_surface_sync(*connection, &request_params);
+
+                if (!mir_surface_is_valid(mir_surface))
+                    BOOST_THROW_EXCEPTION(std::runtime_error(mir_surface_get_error_message(mir_surface)));
+
+                result[output.id] = std::make_shared<mgn::detail::NestedOutput>(egl_display, mir_surface, area);
             }
         });
 
@@ -229,7 +152,7 @@
     auto const& conf = dynamic_cast<NestedDisplayConfiguration const&>(configuration);
 
     outputs.swap(result);
-    mir_connection_apply_display_config(connection, conf);
+    mir_connection_apply_display_config(*connection, conf);
 }
 
 namespace
@@ -245,7 +168,7 @@
         DisplayConfigurationChangeHandler const& conf_change_handler)
 {
     mir_connection_set_display_config_change_callback(
-        connection,
+        *connection,
         &display_config_callback_thunk,
         &(my_conf_change_handler = conf_change_handler));
 }
@@ -281,10 +204,10 @@
     class NestedGLContext : public mg::GLContext
     {
     public:
-        NestedGLContext(MirConnection* connection) :
-            egl_display{connection},
-            egl_config{(egl_display.initialize(), egl_display.choose_config(egl_attribs))},
-            egl_context{egl_display, eglCreateContext(egl_display, egl_config, EGL_NO_CONTEXT, egl_context_attribs)}
+        NestedGLContext(detail::EGLDisplayHandle const& egl_display) :
+            egl_display{egl_display},
+            egl_config{egl_display.choose_config(detail::egl_attribs)},
+            egl_context{egl_display, eglCreateContext(egl_display, egl_config, EGL_NO_CONTEXT, detail::egl_context_attribs)}
         {
         }
 
@@ -299,10 +222,10 @@
         }
 
     private:
-        detail::EGLDisplayHandle const egl_display;
+        EGLDisplay const egl_display;
         EGLConfig const egl_config;
         EGLContextStore const egl_context;
     };
 
-    return std::unique_ptr<mg::GLContext>{new NestedGLContext(connection)};
+    return std::unique_ptr<mg::GLContext>{new NestedGLContext(egl_display)};
 }

=== modified file 'src/server/graphics/nested/nested_display.h'
--- src/server/graphics/nested/nested_display.h	2013-08-29 03:48:16 +0000
+++ src/server/graphics/nested/nested_display.h	2013-08-30 17:00:40 +0000
@@ -24,7 +24,7 @@
 #include "mir/graphics/display_configuration.h"
 #include "mir/graphics/egl_resources.h"
 
-#include "mir_toolkit/mir_client_library.h"
+#include "mir_toolkit/client_types.h"
 
 #include <EGL/egl.h>
 
@@ -45,21 +45,6 @@
 {
 namespace detail
 {
-class MirSurfaceHandle
-{
-public:
-    explicit MirSurfaceHandle(MirConnection* connection, DisplayConfigurationOutput const& output);
-    ~MirSurfaceHandle() noexcept;
-
-    operator MirSurface*() const { return mir_surface; }
-
-private:
-    MirSurface* mir_surface;
-
-    MirSurfaceHandle(MirSurfaceHandle const&) = delete;
-    MirSurfaceHandle operator=(MirSurfaceHandle const&) = delete;
-};
-
 class EGLDisplayHandle
 {
 public:
@@ -79,38 +64,19 @@
     EGLDisplayHandle operator=(EGLDisplayHandle const&) = delete;
 };
 
-class NestedOutput : public DisplayBuffer
-{
-public:
-    NestedOutput(MirConnection* connection, DisplayConfigurationOutput const& output);
-    ~NestedOutput() noexcept;
-
-    geometry::Rectangle view_area() const override;
-    void make_current() override;
-    void release_current() override;
-    void post_update() override;
-    virtual bool can_bypass() const;
-
-    NestedOutput(NestedOutput const&) = delete;
-    NestedOutput operator=(NestedOutput const&) = delete;
-private:
-    detail::MirSurfaceHandle const mir_surface;
-    detail::EGLDisplayHandle const egl_display;
-
-    EGLConfig const egl_config;
-    EGLContextStore const egl_context;
-
-    geometry::Rectangle const area;
-    EGLSurface egl_surface;
-};
-
+class NestedOutput;
 }
 
+class HostConnection;
+
 class NestedDisplay : public Display
 {
 public:
-    NestedDisplay(MirConnection* connection, std::shared_ptr<DisplayReport>const& display_report);
-    virtual ~NestedDisplay() noexcept;
+    NestedDisplay(
+        std::shared_ptr<HostConnection> const& connection,
+        std::shared_ptr<DisplayReport> const& display_report);
+
+    ~NestedDisplay() noexcept;
 
     void for_each_display_buffer(std::function<void(DisplayBuffer&)>const& f) override;
 
@@ -133,8 +99,10 @@
     std::unique_ptr<graphics::GLContext> create_gl_context() override;
 
 private:
-    MirConnection* const connection;
+    std::shared_ptr<HostConnection> const connection;
     std::shared_ptr<DisplayReport> const display_report;
+    detail::EGLDisplayHandle const egl_display;
+
     std::unordered_map<DisplayConfigurationOutputId, std::shared_ptr<detail::NestedOutput>> outputs;
     DisplayConfigurationChangeHandler my_conf_change_handler;
 };

=== added file 'src/server/graphics/nested/nested_output.cpp'
--- src/server/graphics/nested/nested_output.cpp	1970-01-01 00:00:00 +0000
+++ src/server/graphics/nested/nested_output.cpp	2013-08-30 17:00:40 +0000
@@ -0,0 +1,105 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alan Griffiths <[e-mail]>
+ */
+
+#include "nested_output.h"
+
+#include "mir_toolkit/mir_client_library.h"
+
+#include <boost/throw_exception.hpp>
+#include <stdexcept>
+
+namespace mgn = mir::graphics::nested;
+namespace geom = mir::geometry;
+
+mgn::detail::MirSurfaceHandle::MirSurfaceHandle(MirSurface* mir_surface) :
+    mir_surface(mir_surface)
+{
+}
+
+mgn::detail::MirSurfaceHandle::~MirSurfaceHandle() noexcept
+{
+    mir_surface_release_sync(mir_surface);
+}
+
+EGLint const mgn::detail::egl_attribs[] = {
+    EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
+    EGL_RED_SIZE, 8,
+    EGL_GREEN_SIZE, 8,
+    EGL_BLUE_SIZE, 8,
+    EGL_ALPHA_SIZE, 8,
+    EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
+    EGL_NONE
+};
+
+EGLint const mgn::detail::egl_context_attribs[] = {
+    EGL_CONTEXT_CLIENT_VERSION, 2,
+    EGL_NONE
+};
+
+mgn::detail::NestedOutput::NestedOutput(
+    EGLDisplayHandle const& egl_display,
+    MirSurface* mir_surface,
+    geometry::Rectangle const& area) :
+    egl_display(egl_display),
+    mir_surface{mir_surface},
+    egl_config{egl_display.choose_config(egl_attribs)},
+    egl_context{egl_display, eglCreateContext(egl_display, egl_config, EGL_NO_CONTEXT, egl_context_attribs)},
+    area{area.top_left, area.size},
+    egl_surface{EGL_NO_SURFACE}
+{
+}
+
+geom::Rectangle mgn::detail::NestedOutput::view_area() const
+{
+    return area;
+}
+
+void mgn::detail::NestedOutput::make_current()
+{
+    egl_surface = egl_display.egl_surface(egl_config, mir_surface);
+
+    if (eglMakeCurrent(egl_display, egl_surface, egl_surface, egl_context) != EGL_TRUE)
+        BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to update EGL surface.\n"));
+}
+
+void mgn::detail::NestedOutput::release_current()
+{
+    eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, EGL_NO_CONTEXT);
+    eglDestroySurface(egl_display, egl_surface);
+    egl_surface = EGL_NO_SURFACE;
+}
+
+void mgn::detail::NestedOutput::post_update()
+{
+    mir_surface_swap_buffers_sync(mir_surface);
+}
+
+bool mgn::detail::NestedOutput::can_bypass() const
+{
+    // TODO we really should return "true" - but we need to support bypass properly then
+    return false;
+}
+
+
+mgn::detail::NestedOutput::~NestedOutput() noexcept
+{
+    if (egl_surface != EGL_NO_SURFACE)
+        eglDestroySurface(egl_display, egl_surface);
+}
+
+

=== added file 'src/server/graphics/nested/nested_output.h'
--- src/server/graphics/nested/nested_output.h	1970-01-01 00:00:00 +0000
+++ src/server/graphics/nested/nested_output.h	2013-08-30 17:00:40 +0000
@@ -0,0 +1,83 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alan Griffiths <[e-mail]>
+ */
+
+#ifndef MIR_GRAPHICS_NESTED_DETAIL_NESTED_OUTPUT_H_
+#define MIR_GRAPHICS_NESTED_DETAIL_NESTED_OUTPUT_H_
+
+#include "nested_display.h"
+
+namespace mir
+{
+namespace graphics
+{
+namespace nested
+{
+namespace detail
+{
+class MirSurfaceHandle
+{
+public:
+    explicit MirSurfaceHandle(MirSurface* mir_surface);
+
+    ~MirSurfaceHandle() noexcept;
+
+    operator MirSurface*() const { return mir_surface; }
+
+private:
+    MirSurface* mir_surface;
+
+    MirSurfaceHandle(MirSurfaceHandle const&) = delete;
+    MirSurfaceHandle operator=(MirSurfaceHandle const&) = delete;
+};
+
+class NestedOutput : public DisplayBuffer
+{
+public:
+    NestedOutput(
+        EGLDisplayHandle const& egl_display,
+        MirSurface* mir_surface,
+        geometry::Rectangle const& area);
+
+    ~NestedOutput() noexcept;
+
+    geometry::Rectangle view_area() const override;
+    void make_current() override;
+    void release_current() override;
+    void post_update() override;
+    virtual bool can_bypass() const override;
+
+    NestedOutput(NestedOutput const&) = delete;
+    NestedOutput operator=(NestedOutput const&) = delete;
+private:
+    EGLDisplayHandle const& egl_display;
+    MirSurfaceHandle const mir_surface;
+    EGLConfig const egl_config;
+    EGLContextStore const egl_context;
+    geometry::Rectangle const area;
+
+    EGLSurface egl_surface;
+};
+
+extern EGLint const egl_attribs[];
+extern EGLint const egl_context_attribs[];
+}
+}
+}
+}
+
+#endif /* MIR_GRAPHICS_NESTED_DETAIL_NESTED_OUTPUT_H_ */

=== modified file 'src/server/graphics/nested/nested_platform.cpp'
--- src/server/graphics/nested/nested_platform.cpp	2013-08-30 09:01:48 +0000
+++ src/server/graphics/nested/nested_platform.cpp	2013-08-30 17:00:40 +0000
@@ -55,7 +55,7 @@
 
 std::shared_ptr<mg::Display> mgn::NestedPlatform::create_display(std::shared_ptr<mg::DisplayConfigurationPolicy> const& /*initial_conf_policy*/)
 {
-    return std::make_shared<mgn::NestedDisplay>(*connection, display_report);
+    return std::make_shared<mgn::NestedDisplay>(connection, display_report);
 }
 
 std::shared_ptr<mg::PlatformIPCPackage> mgn::NestedPlatform::get_ipc_package()