Merge lp:~alan-griffiths/mir/nested-sessions-dont-post-buffers-until-something-happens into lp:mir

[Alan Griffiths]

compositor: Don't automatically paint the framebuffer when compositing starts. (This is really only appropriate for a host session.)

compositor: Don't automatically paint the framebuffer when compositing starts. (This is really only appropriate for a host session.)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1445
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/971/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1096
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1094
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/685
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/703
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/703/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/708
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/708/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/686
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/686/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/654
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4437

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

299 + compositor.start();
300 + std::this_thread::sleep_for(std::chrono::milliseconds(100));

I think the time limit is too small for our whimsical CI. Perhaps a loop checking the condition every 20ms for 5 seconds? (5s is probably too much, but it doesn't matter since we will pay the price only in case of a failure, which should hopefully be never).

302 + // Verify we're still at zero frames
303 + EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update));

Wrong comment?

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1448
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/977/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1109
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/694
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/709
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/709/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/714
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/714/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/695
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/695/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/662
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4456

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

review: Approve (continuous-integration)

[Alberto Aguirre (albaguirre)]

Looks good.

review: Approve

[Alexandros Frantzis (afrantzis)]

OK.

review: Approve

[kevin gunn (kgunn72)]

prereq branch merged
https://code.launchpad.net/~alan-griffiths/mir/add-some-intelligence-to-scheduling-compositing/+merge/208633

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-autolanding/446/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1137/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1135/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/720/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-autolanding/351/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-autolanding/352/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/721/console

review: Needs Fixing (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Trivial merge conflicts resolved.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1449
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/993/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-trusty-i386-build/1138
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-trusty-amd64-build/1136
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-trusty-touch/721
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/725
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-amd64-ci/725/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/730
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-trusty-armhf-ci/730/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/722
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-trusty-armhf/722/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/684
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/4498

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

review: Approve (continuous-integration)

=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp	2014-03-04 10:10:23 +0000
+++ examples/render_surfaces.cpp	2014-03-05 17:19:32 +0000
@@ -17,6 +17,7 @@
  */
 
 #include "mir/compositor/display_buffer_compositor_factory.h"
+#include "mir/server_status_listener.h"
 #include "mir/compositor/display_buffer_compositor.h"
 #include "mir/options/default_configuration.h"
 #include "mir/graphics/graphic_buffer_allocator.h"
@@ -86,6 +87,7 @@
 
 namespace
 {
+std::atomic<bool> created{false};
 bool input_is_on = false;
 std::weak_ptr<mg::Cursor> cursor;
 static const uint32_t bg_color = 0x00000000;
@@ -326,6 +328,31 @@
     }
     ///\internal [RenderResourcesBufferInitializer_tag]
 
+    // Unless the compositor starts before we create the surfaces it won't respond to
+    // the change notification that causes.
+    std::shared_ptr<mir::ServerStatusListener> the_server_status_listener()
+    {
+        struct ServerStatusListener : mir::ServerStatusListener
+        {
+            ServerStatusListener(std::function<void()> create_surfaces, std::shared_ptr<mir::ServerStatusListener> wrapped) :
+                create_surfaces(create_surfaces), wrapped(wrapped) {}
+
+            virtual void paused() override { wrapped->paused(); }
+            virtual void resumed() override { wrapped->resumed(); }
+            virtual void started() override { wrapped->started(); create_surfaces(); create_surfaces = []{}; }
+
+            std::function<void()> create_surfaces;
+            std::shared_ptr<mir::ServerStatusListener> const wrapped;
+        };
+
+        return server_status_listener(
+            [this]()
+            {
+                auto wrapped = ServerConfiguration::the_server_status_listener();
+                return std::make_shared<ServerStatusListener>([this] { create_surfaces(); }, wrapped);
+            });
+    }
+
     ///\internal [RenderSurfacesDisplayBufferCompositor_tag]
     // Decorate the DefaultDisplayBufferCompositor in order to move surfaces.
     std::shared_ptr<mc::DisplayBufferCompositorFactory> the_display_buffer_compositor_factory() override
@@ -344,6 +371,7 @@
 
             bool composite()
             {
+                while (!created) std::this_thread::yield();
                 animate_cursor();
                 stop_watch.stop();
                 if (stop_watch.elapsed_seconds_since_last_restart() >= 1)
@@ -460,6 +488,8 @@
                     2.0f * M_PI * cos(i));
             ++i;
         }
+
+        created = true;
     }
 
     bool input_is_on()
@@ -493,8 +523,6 @@
 
     mir::run_mir(conf, [&](mir::DisplayServer&)
     {
-        conf.create_surfaces();
-
         cursor = conf.the_cursor();
 
         input_is_on = conf.input_is_on();

=== modified file 'src/server/compositor/default_configuration.cpp'
--- src/server/compositor/default_configuration.cpp	2014-02-28 13:51:43 +0000
+++ src/server/compositor/default_configuration.cpp	2014-03-05 17:19:32 +0000
@@ -25,6 +25,7 @@
 #include "compositing_screencast.h"
 
 #include "mir/frontend/screencast.h"
+#include "mir/options/configuration.h"
 
 #include <boost/throw_exception.hpp>
 
@@ -59,10 +60,12 @@
     return compositor(
         [this]()
         {
-            return std::make_shared<mc::MultiThreadedCompositor>(the_display(),
-                                                                 the_scene(),
-                                                                 the_display_buffer_compositor_factory(),
-                                                                 the_compositor_report());
+            return std::make_shared<mc::MultiThreadedCompositor>(
+                the_display(),
+                the_scene(),
+                the_display_buffer_compositor_factory(),
+                the_compositor_report(),
+                !the_options()->is_set(options::host_socket_opt));
         });
 }
 

=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp	2014-03-04 12:16:12 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp	2014-03-05 17:19:32 +0000
@@ -147,12 +147,14 @@
     std::shared_ptr<mg::Display> const& display,
     std::shared_ptr<mc::Scene> const& scene,
     std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,
-    std::shared_ptr<CompositorReport> const& compositor_report)
+    std::shared_ptr<CompositorReport> const& compositor_report,
+    bool compose_on_start)
     : display{display},
       scene{scene},
       display_buffer_compositor_factory{db_compositor_factory},
       report{compositor_report},
-      started{false}
+      started{false},
+      compose_on_start{compose_on_start}
 {
 }
 
@@ -161,6 +163,14 @@
     stop();
 }
 
+void mc::MultiThreadedCompositor::schedule_compositing()
+{
+    std::unique_lock<std::mutex> lk(started_guard);
+    report->scheduled();
+    for (auto& f : thread_functors)
+        f->schedule_compositing();
+}
+
 void mc::MultiThreadedCompositor::start()
 {
     std::unique_lock<std::mutex> lk(started_guard);
@@ -184,16 +194,18 @@
     /* Recomposite whenever the scene changes */
     scene->set_change_callback([this]()
     {
-        report->scheduled();
-        for (auto& f : thread_functors)
-            f->schedule_compositing();
+        schedule_compositing();
     });
 
-    /* First render */
-    for (auto& f : thread_functors)
-        f->schedule_compositing();
-
     started = true;
+
+    /* Optional first render */
+    if (compose_on_start)
+    {
+        lk.unlock();
+        schedule_compositing();
+    }
+
 }
 
 void mc::MultiThreadedCompositor::stop()
@@ -204,7 +216,9 @@
         return;
     }
 
+    lk.unlock();
     scene->set_change_callback([]{});
+    lk.lock();
 
     for (auto& f : thread_functors)
         f->stop();
@@ -218,4 +232,8 @@
     report->stopped();
 
     started = false;
+
+    // If the compositor is restarted we've likely got clients blocked
+    // so we will need to schedule compositing immediately
+    compose_on_start = true;
 }

=== modified file 'src/server/compositor/multi_threaded_compositor.h'
--- src/server/compositor/multi_threaded_compositor.h	2014-03-05 07:21:46 +0000
+++ src/server/compositor/multi_threaded_compositor.h	2014-03-05 17:19:32 +0000
@@ -46,7 +46,8 @@
     MultiThreadedCompositor(std::shared_ptr<graphics::Display> const& display,
                             std::shared_ptr<Scene> const& scene,
                             std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,
-                            std::shared_ptr<CompositorReport> const& compositor_report);
+                            std::shared_ptr<CompositorReport> const& compositor_report,
+                            bool compose_on_start);
     ~MultiThreadedCompositor();
 
     void start();
@@ -63,6 +64,9 @@
 
     std::mutex started_guard;
     bool started;
+    bool compose_on_start;
+
+    void schedule_compositing();
 };
 
 }

=== modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
--- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-03-05 07:21:46 +0000
+++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-03-05 17:19:32 +0000
@@ -324,7 +324,7 @@
     auto display = std::make_shared<StubDisplay>(nbuffers);
     auto scene = std::make_shared<StubScene>();
     auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
-    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
 
     compositor.start();
 
@@ -348,7 +348,8 @@
     auto mock_report = std::make_shared<mtd::MockCompositorReport>();
     mc::MultiThreadedCompositor compositor{display, scene,
                                            db_compositor_factory,
-                                           mock_report};
+                                           mock_report,
+                                           true};
 
     EXPECT_CALL(*mock_report, started())
         .Times(1);
@@ -363,7 +364,7 @@
     EXPECT_CALL(*mock_report, added_display(_,_,_,_,_))
         .Times(1);
     EXPECT_CALL(*mock_report, scheduled())
-        .Times(1);
+        .Times(2);
 
     display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
     {
@@ -398,7 +399,7 @@
     auto display = std::make_shared<StubDisplay>(nbuffers);
     auto scene = std::make_shared<StubScene>();
     auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
-    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
 
     // Verify we're actually starting at zero frames
     EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
@@ -449,6 +450,64 @@
     compositor.stop();
 }
 
+TEST(MultiThreadedCompositor, when_no_initial_composite_is_needed_there_is_none)
+{
+    using namespace testing;
+
+    unsigned int const nbuffers = 3;
+
+    auto display = std::make_shared<StubDisplay>(nbuffers);
+    auto scene = std::make_shared<StubScene>();
+    auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
+
+    // Verify we're actually starting at zero frames
+    ASSERT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
+
+    compositor.start();
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // Verify we're still at zero frames
+    EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
+
+    compositor.stop();
+}
+
+TEST(MultiThreadedCompositor, when_no_initial_composite_is_needed_we_still_composite_on_restart)
+{
+    using namespace testing;
+
+    unsigned int const nbuffers = 3;
+
+    auto display = std::make_shared<StubDisplay>(nbuffers);
+    auto scene = std::make_shared<StubScene>();
+    auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
+
+    compositor.start();
+
+    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+    // Verify we're actually starting at zero frames
+    ASSERT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
+
+    compositor.stop();
+    compositor.start();
+
+    for (int countdown = 100;
+        countdown != 0 &&
+        !db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update);
+        --countdown)
+    {
+        std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    }
+
+    // Verify we composited the expected frame
+    EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update));
+
+    compositor.stop();
+}
+
 TEST(MultiThreadedCompositor, surface_update_from_render_doesnt_deadlock)
 {
     using namespace testing;
@@ -458,7 +517,7 @@
     auto display = std::make_shared<StubDisplay>(nbuffers);
     auto scene = std::make_shared<StubScene>();
     auto db_compositor_factory = std::make_shared<SurfaceUpdatingDisplayBufferCompositorFactory>(scene);
-    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
 
     compositor.start();
 
@@ -477,7 +536,7 @@
     auto display = std::make_shared<StubDisplayWithMockBuffers>(nbuffers);
     auto scene = std::make_shared<StubScene>();
     auto db_compositor_factory = std::make_shared<NullDisplayBufferCompositorFactory>();
-    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};
+    mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
 
     display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
     {
@@ -505,7 +564,7 @@
     EXPECT_CALL(*mock_scene, set_change_callback(testing::_))
         .Times(2);
 
-    mc::MultiThreadedCompositor compositor{display, mock_scene, db_compositor_factory, mock_report};
+    mc::MultiThreadedCompositor compositor{display, mock_scene, db_compositor_factory, mock_report, true};
 
     compositor.start();
     compositor.start();