Merge lp:~alan-griffiths/mir/tweak-cursor-configuration into lp:mir

[Alan Griffiths]

server: replace Server::override_the_cursor() with Server::wrap_cursor()

server: replace Server::override_the_cursor() with Server::wrap_cursor()

It is necessary for servers to wrap rather than replace cursors because they need access to the underlying hardware or nested cursor.

There are some tests coming that need this API change, but I want this to be ready before we branch 0.17 to avoid publishing Server::override_the_cursor().

There's also a bit of code tidy up and documentation of why things are not done the "obvious" way (of implementing nested::Display::create_hardware_cursor().

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2997
http://jenkins.qa.ubuntu.com/job/mir-ci/5164/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4306
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3213
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4248/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/206/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1318
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1318/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/206/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4249
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4249/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6910/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23986
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/207/console

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

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

*
GLib:ERROR:/build/buildd/glib2.0-2.44.1/./glib/ghash.c:373:g_hash_table_lookup_node: assertion failed: (hash_table->ref_count > 0)
/bin/bash: line 1: 6398 Aborted mir_demo_server --test-client /usr/bin/mir_demo_client_basic
+ mir_rc=-1

Concerning, but appears unrelated.

[lp:~alan-griffiths/mir/tweak-cursor-configuration]

2998. By Alan Griffiths on 2015-10-06

merge lp:mir

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2998
http://jenkins.qa.ubuntu.com/job/mir-ci/5167/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/4311
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/3218
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/4253
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-wily-touch/211/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1321
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/1321/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-i386-ci/211/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4254
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/4254/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/6914
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/23991
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-wily-armhf/212/console

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

review: Approve (continuous-integration)

[Andreas Pokorny (andreas-pokorny)]

> *
> GLib:ERROR:/build/buildd/glib2.0-2.44.1/./glib/ghash.c:373:g_hash_table_lookup
> _node: assertion failed: (hash_table->ref_count > 0)
> /bin/bash: line 1: 6398 Aborted mir_demo_server --test-client
> /usr/bin/mir_demo_client_basic
> + mir_rc=-1
>
> Concerning, but appears unrelated.

Yes .. back to asio main loop? or should we try to build one with dispatchables?.. (and with a simpler alarm interface)

[Andreas Pokorny (andreas-pokorny)]

yes makes sense

review: Approve

[Kevin DuBois (kdub)]

lgtm

review: Approve

=== modified file 'include/server/mir/server.h'
--- include/server/mir/server.h	2015-09-30 07:19:48 +0000
+++ include/server/mir/server.h	2015-10-06 13:26:24 +0000
@@ -224,9 +224,6 @@
     /// Sets an override functor for creating the compositor.
     void override_the_compositor(Builder<compositor::Compositor> const& compositor_builder);
 
-    /// Sets an override functor for creating the cursor.
-    void override_the_cursor(Builder<graphics::Cursor> const& cursor_builder);
-
     /// Sets an override functor for creating the cursor images.
     void override_the_cursor_images(Builder<input::CursorImages> const& cursor_images_builder);
 
@@ -282,6 +279,9 @@
     /// Each of the wrap functions takes a wrapper functor of the same form
     template<typename T> using Wrapper = std::function<std::shared_ptr<T>(std::shared_ptr<T> const&)>;
 
+    /// Sets a wrapper functor for creating the cursor.
+    void wrap_cursor(Wrapper<graphics::Cursor> const& cursor_builder);
+
     /// Sets a wrapper functor for creating the cursor listener.
     void wrap_cursor_listener(Wrapper<input::CursorListener> const& wrapper);
 

=== modified file 'src/include/server/mir/default_server_configuration.h'
--- src/include/server/mir/default_server_configuration.h	2015-09-30 07:19:48 +0000
+++ src/include/server/mir/default_server_configuration.h	2015-10-06 13:26:24 +0000
@@ -220,6 +220,7 @@
      *  @{ */
     virtual std::shared_ptr<graphics::DisplayReport> the_display_report();
     virtual std::shared_ptr<graphics::Cursor> the_cursor();
+    virtual std::shared_ptr<graphics::Cursor> wrap_cursor(std::shared_ptr<graphics::Cursor> const& wrapped);
     virtual std::shared_ptr<graphics::CursorImage> the_default_cursor_image();
     virtual std::shared_ptr<input::CursorImages> the_cursor_images();
     virtual std::shared_ptr<graphics::DisplayConfigurationReport> the_display_configuration_report();

=== modified file 'src/server/graphics/default_configuration.cpp'
--- src/server/graphics/default_configuration.cpp	2015-10-05 08:19:39 +0000
+++ src/server/graphics/default_configuration.cpp	2015-10-06 13:26:24 +0000
@@ -168,31 +168,38 @@
     return cursor(
         [this]() -> std::shared_ptr<mg::Cursor>
         {
+            // In nested mode we delegate cursor presentation to the host
             if (the_options()->is_set(options::host_socket_opt))
-                return std::make_shared<mgn::Cursor>(the_host_connection(), the_default_cursor_image());
-            
-            // We try to create a hardware cursor, if this fails we use a software cursor
-            auto hardware_cursor = the_display()->create_hardware_cursor(the_default_cursor_image());
-            if (hardware_cursor)
+            {
+                mir::log_info("Using nested cursor");
+                return wrap_cursor(std::make_shared<mgn::Cursor>(the_host_connection(), the_default_cursor_image()));
+            }
+
+            // Otherwise we try to create a hardware cursor
+            if (auto hardware_cursor = the_display()->create_hardware_cursor(the_default_cursor_image()))
             {
                 mir::log_info("Using hardware cursor");
-                return hardware_cursor;
-            }
-            else
-            {
-                mir::log_info("Using software cursor");
-
-                auto const cursor = std::make_shared<mg::SoftwareCursor>(
-                    the_buffer_allocator(),
-                    the_input_scene());
-
-                cursor->show(*the_default_cursor_image());
-
-                return cursor;
-            }
+                return wrap_cursor(hardware_cursor);
+            }
+
+            // If other options fail we use a software cursor
+            mir::log_info("Using software cursor");
+            auto const cursor = wrap_cursor(std::make_shared<mg::SoftwareCursor>(
+                the_buffer_allocator(),
+                the_input_scene()));
+
+            cursor->show(*the_default_cursor_image());
+
+            return cursor;
         });
 }
 
+std::shared_ptr<mg::Cursor>
+mir::DefaultServerConfiguration::wrap_cursor(std::shared_ptr<mg::Cursor> const& wrapped)
+{
+    return wrapped;
+}
+
 auto mir::DefaultServerConfiguration::the_host_connection()
 -> std::shared_ptr<graphics::nested::HostConnection>
 {

=== modified file 'src/server/graphics/nested/display.cpp'
--- src/server/graphics/nested/display.cpp	2015-06-24 08:48:36 +0000
+++ src/server/graphics/nested/display.cpp	2015-10-06 13:26:24 +0000
@@ -273,20 +273,18 @@
 
 void mgn::Display::pause()
 {
-    // TODO Do we "own" the cursor or does the host mir?
-    // If we "own" the cursor then we need to hide it
+    // No need to do anything
 }
 
 void mgn::Display::resume()
 {
-    // TODO Do we "own" the cursor or does the host mir?
-    // TODO If we "own" the cursor then we need to restore it
+    // No need to do anything
 }
 
-auto mgn::Display::create_hardware_cursor(std::shared_ptr<mg::CursorImage> const& /* initial image */)->std::shared_ptr<Cursor>
+auto mgn::Display::create_hardware_cursor(std::shared_ptr<mg::CursorImage> const& /*initial_image*/) -> std::shared_ptr<mg::Cursor>
 {
-    // TODO Do we "own" the cursor or does the host mir?
-    return std::shared_ptr<Cursor>();
+    BOOST_THROW_EXCEPTION(std::logic_error("Initialization loop: we already need the Cursor when creating the Display"));
+    // So we can't do this: return std::make_shared<Cursor>(connection, initial_image);
 }
 
 std::unique_ptr<mg::GLContext> mgn::Display::create_gl_context()

=== modified file 'src/server/server.cpp'
--- src/server/server.cpp	2015-10-06 04:45:25 +0000
+++ src/server/server.cpp	2015-10-06 13:26:24 +0000
@@ -43,6 +43,7 @@
 namespace mo = mir::options;
 
 #define FOREACH_WRAPPER(MACRO)\
+    MACRO(cursor)\
     MACRO(cursor_listener)\
     MACRO(display_buffer_compositor_factory)\
     MACRO(display_configuration_policy)\
@@ -50,7 +51,6 @@
 
 #define FOREACH_OVERRIDE(MACRO)\
     MACRO(compositor)\
-    MACRO(cursor)\
     MACRO(cursor_images)\
     MACRO(display_buffer_compositor_factory)\
     MACRO(display_configuration_report)\

=== modified file 'src/server/symbols.map'
--- src/server/symbols.map	2015-09-30 07:19:48 +0000
+++ src/server/symbols.map	2015-10-06 13:26:24 +0000
@@ -145,7 +145,7 @@
     mir::Server::open_prompt_socket*;
     mir::Server::override_the_application_not_responding_detector*;
     mir::Server::override_the_compositor*;
-    mir::Server::override_the_cursor*;
+    mir::Server::override_the_cursor_images*;
     mir::Server::override_the_display_buffer_compositor_factory*;
     mir::Server::override_the_display_configuration_report*;
     mir::Server::override_the_gl_config*;
@@ -199,6 +199,7 @@
     mir::Server::the_surface_coordinator*;
     mir::Server::the_surface_factory*;
     mir::Server::the_touch_visualizer*;
+    mir::Server::wrap_cursor*;
     mir::Server::wrap_cursor_listener*;
     mir::Server::wrap_display_buffer_compositor_factory*;
     mir::Server::wrap_display_configuration_policy*;
@@ -684,6 +685,7 @@
     mir::DefaultServerConfiguration::the_surface_stack_model*;
     mir::DefaultServerConfiguration::the_touch_visualizer*;
     mir::DefaultServerConfiguration::the_window_manager_builder*;
+    mir::DefaultServerConfiguration::wrap_cursor*;
     mir::DefaultServerConfiguration::wrap_cursor_listener*;
     mir::DefaultServerConfiguration::wrap_display_buffer_compositor_factory*;
     mir::DefaultServerConfiguration::wrap_display_configuration_policy*;

=== modified file 'tests/acceptance-tests/test_nested_mir.cpp'
--- tests/acceptance-tests/test_nested_mir.cpp	2015-10-05 08:59:41 +0000
+++ tests/acceptance-tests/test_nested_mir.cpp	2015-10-06 13:26:24 +0000
@@ -159,7 +159,7 @@
         server.override_the_display_configuration_report([this]
             { return the_mock_display_configuration_report(); });
 
-        server.override_the_cursor([this] { return the_mock_cursor(); });
+        server.wrap_cursor([this](std::shared_ptr<mg::Cursor> const&) { return the_mock_cursor(); });
 
         mtf::HeadlessInProcessServer::SetUp();
     }
@@ -495,7 +495,6 @@
     mir_surface_apply_spec(surface, spec);
     mir_surface_spec_release(spec);
 
-    nested_mir.server.the_cursor()->move_to({489, 9});
     server.the_cursor_listener()->cursor_moved_to(489, 9);
 
     auto stream = mir_connection_create_buffer_stream_sync(

=== modified file 'tests/acceptance-tests/throwback/test_client_cursor_api.cpp'
--- tests/acceptance-tests/throwback/test_client_cursor_api.cpp	2015-10-02 21:04:21 +0000
+++ tests/acceptance-tests/throwback/test_client_cursor_api.cpp	2015-10-06 13:26:24 +0000
@@ -227,7 +227,7 @@
         mock_egl.provide_egl_extensions();
         mock_egl.provide_stub_platform_buffer_swapping();
 
-        server.override_the_cursor([this]() { return mt::fake_shared(cursor); });
+        server.wrap_cursor([this](std::shared_ptr<mg::Cursor> const&) { return mt::fake_shared(cursor); });
 
         server.override_the_cursor_images([]() { return std::make_shared<NamedCursorImages>(); });