Merge lp:~vanvugt/mir/judder into lp:mir

[Daniel van Vugt]

Fix frame skipping/stealing/beating regressions (LP: #1308843) by ensuring
that fake frames don't occur if real frames are being composited.

This also modifies minimum buffer consumption to drop to 10Hz if compositors
block, ensuring clients always get their frames consumed in a definite time.
And it also happens to solve LP: #1308844.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1585
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1758/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/375
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/376
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/373/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/277
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/277/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/276
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/276/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1037
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1037/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1602/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/7850

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

review: Needs Fixing (continuous-integration)

[lp:~vanvugt/mir/judder]

1586. By Daniel van Vugt on 2014-05-29

Merge latest development-branch

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1586
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1768/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/389
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/390
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/388/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/287
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/287/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/286
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/286/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1079/console

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

review: Needs Fixing (continuous-integration)

[lp:~vanvugt/mir/judder]

1587. By Daniel van Vugt on 2014-05-30

Merge latest development-branch

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1587
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/1780/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/407
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/408
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/406/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/299
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/299/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/298
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/298/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1122/console

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

review: Needs Fixing (continuous-integration)

Unmerged revisions

1587. By Daniel van Vugt on 2014-05-30

Merge latest development-branch

1586. By Daniel van Vugt on 2014-05-29

Merge latest development-branch

1585. By Daniel van Vugt on 2014-05-28

Adjust sleep to yield 10Hz client wakeups

1584. By Daniel van Vugt on 2014-05-28

Tidy up

1583. By Daniel van Vugt on 2014-05-28

Fix missing brace, FTBFS

1582. By Daniel van Vugt on 2014-05-28

Merge latest development-branch

1581. By Daniel van Vugt on 2014-05-27

Clean up

1580. By Daniel van Vugt on 2014-05-27

Remember to set the continuation flag correctly... even on frames when
we're not consuming frames.

1579. By Daniel van Vugt on 2014-05-27

Remember to keep fake compositing as required

1578. By Daniel van Vugt on 2014-05-27

Remove more tabs

=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp	2014-05-29 15:51:54 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp	2014-05-30 08:03:29 +0000
@@ -32,6 +32,7 @@
 #include <condition_variable>
 #include <cstdint>
 #include <chrono>
+#include <atomic>
 
 namespace mc = mir::compositor;
 namespace mg = mir::graphics;
@@ -86,6 +87,8 @@
     mg::DisplayBuffer& buffer;
 };
 
+std::atomic<int> real_frames(0);
+
 class CompositingFunctor
 {
 public:
@@ -180,7 +183,12 @@
                               r.top_left.x.as_int(), r.top_left.y.as_int(),
                               report_id);
 
-        run_compositing_loop([&] { return display_buffer_compositor->composite();});
+        run_compositing_loop([&]
+        {
+            bool ret = display_buffer_compositor->composite();
+            real_frames.fetch_add(1);
+            return ret;
+        });
     }
     catch (...)
     {
@@ -207,19 +215,31 @@
         run_compositing_loop(
             [this]
             {
-                std::vector<std::shared_ptr<mg::Buffer>> saved_resources;
-
+                int old_real_frames = real_frames;
+
+                // Emulate a moderately slow fake display.
+                // Note that due to feedback from consuming a buffer,
+                // this will trigger alternation between consuming a real
+                // and fake frame. So sleeping 50ms here actually results
+                // in a client frame rate of 10Hz.
+
+                std::this_thread::sleep_for(std::chrono::milliseconds(50));
+                bool consumed = real_frames != old_real_frames;
+
+                int more = 0;
                 auto const& renderables = scene->renderable_list_for(this);
-
                 for (auto const& r : renderables)
                 {
-                    if (r->buffers_ready_for_compositor() > 0)
-                        saved_resources.push_back(r->buffer());
+                    int ready = r->buffers_ready_for_compositor();
+                    more += ready;
+
+                    // Now only composite a fake frame if no real display has
+                    // composited during the sleep.
+                    if (ready && !consumed)
+                        (void)r->buffer();
                 }
 
-                wait_until_next_fake_vsync();
-
-                return false;
+                return more > 0;
             });
     }
     catch (...)
@@ -228,23 +248,6 @@
     }
 
 private:
-    void wait_until_next_fake_vsync()
-    {
-        using namespace std::chrono;
-        typedef duration<int64_t, std::ratio<1, 60>> vsync_periods;
-
-        /* Truncate to vsync periods */
-        auto const previous_vsync =
-            duration_cast<vsync_periods>(steady_clock::now().time_since_epoch());
-        /* Convert back to a timepoint */
-        auto const previous_vsync_tp =
-            time_point<steady_clock, vsync_periods>{previous_vsync};
-        /* Next vsync time point */
-        auto const next_vsync = previous_vsync_tp + vsync_periods(1);
-
-        std::this_thread::sleep_until(next_vsync);
-    }
-
     std::shared_ptr<mc::Scene> const scene;
 };
 

=== modified file 'tests/acceptance-tests/test_client_surface_swap_buffers.cpp'
--- tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-05-29 15:51:54 +0000
+++ tests/acceptance-tests/test_client_surface_swap_buffers.cpp	2014-05-30 08:03:29 +0000
@@ -21,7 +21,7 @@
 #include "mir_test_framework/stubbed_server_configuration.h"
 #include "mir_test_framework/in_process_server.h"
 #include "mir_test_framework/using_stub_client_platform.h"
-#include "mir_test_doubles/null_display_buffer_compositor_factory.h"
+#include "mir_test_doubles/null_display.h"
 #include "mir_test/spin_wait.h"
 
 #include <gtest/gtest.h>
@@ -37,10 +37,9 @@
 
 struct StubServerConfig : mtf::StubbedServerConfiguration
 {
-    auto the_display_buffer_compositor_factory()
-        -> std::shared_ptr<mc::DisplayBufferCompositorFactory> override
+    std::shared_ptr<mir::graphics::Display> the_display()
     {
-        return std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
+        return std::make_shared<mtd::NullDisplay>();
     }
 };
 

=== modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
--- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-05-29 15:51:54 +0000
+++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp	2014-05-30 08:03:29 +0000
@@ -150,6 +150,55 @@
     bool throw_on_add_observer_;
 };
 
+class StubDisplayBufferCompositor : public mc::DisplayBufferCompositor
+{
+public:
+    StubDisplayBufferCompositor(std::shared_ptr<mc::Scene> scene,
+                                std::chrono::milliseconds frame_time)
+        : scene{scene}, frame_time{frame_time}
+    {
+    }
+
+    bool composite() override
+    {
+        auto const& all = scene->renderable_list_for(this);
+        for (auto const& r : all)
+            r->buffer();  // Consume a frame
+
+        std::this_thread::sleep_for(frame_time);
+        return false;
+    }
+
+private:
+    std::shared_ptr<mc::Scene> const scene;
+    std::chrono::milliseconds const frame_time;
+};
+
+
+class StubDisplayBufferCompositorFactory :
+    public mc::DisplayBufferCompositorFactory
+{
+public:
+    StubDisplayBufferCompositorFactory(std::shared_ptr<mc::Scene> scene,
+                                       int hz)
+        : scene{scene},
+          frame_time{std::chrono::milliseconds(1000 / hz)}
+    {
+    }
+
+    std::unique_ptr<mc::DisplayBufferCompositor>
+        create_compositor_for(mg::DisplayBuffer&) override
+    {
+        std::unique_ptr<mc::DisplayBufferCompositor> ret(
+            new StubDisplayBufferCompositor(scene, frame_time));
+        return ret;
+    }
+
+private:
+    std::shared_ptr<mc::Scene> const scene;
+    std::chrono::milliseconds const frame_time;
+};
+
 class RecordingDisplayBufferCompositor : public mc::DisplayBufferCompositor
 {
 public:
@@ -323,7 +372,7 @@
 class BufferCountingRenderable : public mtd::StubRenderable
 {
 public:
-    BufferCountingRenderable(RenderableVisibility visibility)
+    BufferCountingRenderable(RenderableVisibility visibility = RenderableVisibility::visible)
         : buffers_requested_{0}, visibility{visibility}
     {
     }
@@ -627,12 +676,12 @@
 {
     using namespace testing;
 
-    unsigned int const nbuffers{2};
     auto renderable = std::make_shared<BufferCountingRenderable>(GetParam());
-    auto display = std::make_shared<StubDisplay>(nbuffers);
+
+    // Defining zero outputs is the simplest way to emulate what would
+    // happen if all your displays were blocked in swapping...
+    auto display = std::make_shared<StubDisplay>(0);
     auto stub_scene = std::make_shared<StubScene>(mg::RenderableList{renderable});
-    // We use NullDisplayBufferCompositors to simulate DisplayBufferCompositors
-    // not rendering a renderable.
     auto db_compositor_factory = std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
 
     mc::MultiThreadedCompositor compositor{
@@ -657,6 +706,86 @@
     compositor.stop();
 }
 
+TEST(MultiThreadedCompositor, never_steals_frames_from_real_displays)
+{
+    /*
+     * Verify dummy frames are consumed slower than any physical display would
+     * consume them, so that the two types of consumers don't race and don't
+     * visibly skip (LP: #1308843)
+     */
+    using namespace testing;
+
+    unsigned int const nbuffers{2};
+    auto renderable = std::make_shared<BufferCountingRenderable>();
+    auto display = std::make_shared<StubDisplay>(nbuffers);
+    auto stub_scene = std::make_shared<StubScene>(mg::RenderableList{renderable});
+    // We use NullDisplayBufferCompositors to simulate DisplayBufferCompositors
+    // not rendering a renderable.
+    auto db_compositor_factory = std::make_shared<mtd::NullDisplayBufferCompositorFactory>();
+
+    mc::MultiThreadedCompositor compositor{
+        display, stub_scene, db_compositor_factory, null_report, true};
+
+    compositor.start();
+
+    // Realistically I've only ever seen LCDs go as low as 40Hz. But some TV
+    // outputs will go as low as 24Hz (traditional movie frame rate). So we
+    // need to ensure the dummy consumer is slower than that...
+    int const min_real_refresh_rate = 20;
+
+    int const secs = 5;
+    auto const duration = std::chrono::seconds{secs};
+    mir::test::spin_wait_for_condition_or_timeout(
+        [&] { return renderable->buffers_requested() <= 1; },
+        duration);
+
+    auto const end = std::chrono::steady_clock::now() + duration;
+    while (std::chrono::steady_clock::now() < end)
+    {
+        stub_scene->emit_change_event();
+        std::this_thread::yield();
+    }
+
+    int const max_dummy_frames = min_real_refresh_rate * secs;
+    ASSERT_GT(max_dummy_frames, renderable->buffers_requested());
+
+    compositor.stop();
+}
+
+TEST(MultiThreadedCompositor, only_real_displays_limit_consumption_rate)
+{
+    using namespace testing;
+
+    auto display = std::make_shared<StubDisplay>(1);
+    auto renderable = std::make_shared<BufferCountingRenderable>();
+    auto stub_scene = std::make_shared<StubScene>(mg::RenderableList{renderable});
+
+    int const framerate = 75;   // Simulate a nice fast frame rate
+    auto db_compositor_factory =
+        std::make_shared<StubDisplayBufferCompositorFactory>(stub_scene,
+                                                             framerate);
+
+    mc::MultiThreadedCompositor compositor{
+        display, stub_scene, db_compositor_factory, null_report, false};
+
+    compositor.start();
+
+    int const secs = 5;
+    auto const duration = std::chrono::seconds{secs};
+    auto const end = std::chrono::steady_clock::now() + duration;
+    while (std::chrono::steady_clock::now() < end)
+    {
+        stub_scene->emit_change_event();
+        std::this_thread::yield();
+    }
+
+    int const expected_frames = framerate * secs;
+    ASSERT_GT(expected_frames * 1.2f, renderable->buffers_requested());
+    ASSERT_LT(expected_frames * 0.8f, renderable->buffers_requested());
+
+    compositor.stop();
+}
+
 INSTANTIATE_TEST_CASE_P(
     MultiThreadedCompositor, BufferConsumption,
     ::testing::Values(RenderableVisibility::hidden, RenderableVisibility::visible));