Merge lp:~kdub/mir/dropping-schedule into lp:mir

[Kevin DuBois]

compositor: add a yet unused mc::Schedule implementation for egl-swapinterval-0 scenarios.

compositor: add a yet unused mc::Schedule implementation for egl-swapinterval-0 scenarios.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2750
http://jenkins.qa.ubuntu.com/job/mir-ci/4367/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3227
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/80
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/752
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3175
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/516
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/516/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3175
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3175/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5977
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21991

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

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

Nits:

51 + std::unique_lock<decltype(mutex)> lk(mutex);

Could use std::lock_guard<>

219 +TEST_F(DroppingSchedule, queues_drops_excess_buffers)

"queues_drops_..." ?

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2753
http://jenkins.qa.ubuntu.com/job/mir-ci/4381/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3244
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/97
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/769
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3192
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/530
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/530/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3192
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3192/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5993
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22020

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

What is the cancel() call in there for? It introduces a race with anything_scheduled() (it's not possible for a consumer to call next_buffer() without possibly generating a logic_error), and I don't think it does what we want it to do.

If I remember our previous discussions about this correctly, the objective of cancel() is to replace a stale back-buffer after a long period of inactivity (for example, display off). But for swapinterval 0 this is easy - submit a new buffer and you replace the stale back-buffer.

And for swap interval != 1 you need to discard the entire queue; this makes QueuingShedule vulnerable to the same race as DroppingSchedule.

I think this means that cancel() should actually be replace_queue_with(buffer).

review: Needs Fixing

[Kevin DuBois (kdub)]

The objective is a bit different. Its a client-facing lever that can be pushed to discard a buffer that has been submitted. Hopefully it'll help service ANativeWindow::cancelBuffer() as well.

With swapinterval0 with 2 buffers, if one buffer is used by the compositor, and one has the last-produced content, the client has a problem getting another buffer to produce immediately. The most straightforward choices are to allocate another buffer to use, or to cancel the last-produced content and use that again.

Allocating another buffer solves this problem, (as submitting one buffer would send back the other one), but if we want to limit swapinterval0's space, cancel can help out. Cancel is not something that is crucial to the first round of integration.

[Kevin DuBois (kdub)]

removed cancel(), as its not something thats needed at first, nor does it even have the ipc around at present.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2754
http://jenkins.qa.ubuntu.com/job/mir-ci/4395/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3269
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/794
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3217/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/544
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/544/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3217
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3217/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6016/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22058

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2754
http://jenkins.qa.ubuntu.com/job/mir-ci/4404/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3282
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/135/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/807
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3230
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/553
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/553/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3230
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3230/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6030
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22087

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2755
http://jenkins.qa.ubuntu.com/job/mir-ci/4408/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3288
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/141/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/813
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3236/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/557
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/557/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3236
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3236/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6035/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22099

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

review: Needs Fixing (continuous-integration)

[Chris Halse Rogers (raof)]

Seems a reasonable transition step.

review: Approve

[Cemil Azizoglu (cemil-azizoglu)]

ok

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1698/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3303/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/156/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/828/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3251/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/251/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3251/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/1702/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3309/console
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/162/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/834/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3257/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-autolanding/255/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3257/console

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

review: Approve (continuous-integration)

=== modified file 'src/server/compositor/CMakeLists.txt'
--- src/server/compositor/CMakeLists.txt	2015-07-23 19:14:27 +0000
+++ src/server/compositor/CMakeLists.txt	2015-07-27 11:30:52 +0000
@@ -19,6 +19,7 @@
   recently_used_cache.cpp
   multi_monitor_arbiter.cpp
   buffer_map.cpp
+  dropping_schedule.cpp
   queueing_schedule.cpp
 )
 

=== added file 'src/server/compositor/dropping_schedule.cpp'
--- src/server/compositor/dropping_schedule.cpp	1970-01-01 00:00:00 +0000
+++ src/server/compositor/dropping_schedule.cpp	2015-07-27 11:30:52 +0000
@@ -0,0 +1,55 @@
+/*
+ * Copyright © 2015 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "dropping_schedule.h"
+#include "mir/frontend/client_buffers.h"
+#include "mir/graphics/buffer.h"
+
+#include <boost/throw_exception.hpp>
+namespace mf = mir::frontend;
+namespace mg = mir::graphics;
+namespace mc = mir::compositor;
+
+mc::DroppingSchedule::DroppingSchedule(std::shared_ptr<mf::ClientBuffers> const& client_buffers) :
+    sender(client_buffers)
+{
+}
+
+void mc::DroppingSchedule::schedule(std::shared_ptr<mg::Buffer> const& buffer)
+{
+    std::lock_guard<decltype(mutex)> lk(mutex);
+    if ((the_only_buffer != buffer) && the_only_buffer)
+        sender->send_buffer(the_only_buffer->id());
+    the_only_buffer = buffer;
+}
+
+bool mc::DroppingSchedule::anything_scheduled()
+{
+    std::lock_guard<decltype(mutex)> lk(mutex);
+    return static_cast<bool>(the_only_buffer);
+}
+
+std::shared_ptr<mg::Buffer> mc::DroppingSchedule::next_buffer()
+{
+    std::lock_guard<decltype(mutex)> lk(mutex);
+    if (!the_only_buffer)
+        BOOST_THROW_EXCEPTION(std::logic_error("no buffer scheduled"));
+    auto buffer = the_only_buffer;
+    the_only_buffer = nullptr;
+    return buffer;
+}

=== added file 'src/server/compositor/dropping_schedule.h'
--- src/server/compositor/dropping_schedule.h	1970-01-01 00:00:00 +0000
+++ src/server/compositor/dropping_schedule.h	2015-07-27 11:30:52 +0000
@@ -0,0 +1,45 @@
+/*
+ * Copyright © 2015 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: Kevin DuBois <[e-mail]>
+ */
+
+#ifndef MIR_COMPOSITOR_DROPPING_SCHEDULE_H_
+#define MIR_COMPOSITOR_DROPPING_SCHEDULE_H_
+#include <memory>
+#include <mutex>
+
+namespace mir
+{
+namespace graphics { class Buffer; }
+namespace frontend { class ClientBuffers; }
+namespace compositor
+{
+class DroppingSchedule
+{
+public:
+    DroppingSchedule(std::shared_ptr<frontend::ClientBuffers> const&);
+    void schedule(std::shared_ptr<graphics::Buffer> const& buffer);
+    bool anything_scheduled();
+    std::shared_ptr<graphics::Buffer> next_buffer();
+
+private:
+    std::mutex mutable mutex;
+    std::shared_ptr<frontend::ClientBuffers> const sender;
+    std::shared_ptr<graphics::Buffer> the_only_buffer;
+};
+}
+}
+#endif /* MIR_COMPOSITOR_DROPPING_SCHEDULE_H_ */

=== modified file 'tests/unit-tests/compositor/CMakeLists.txt'
--- tests/unit-tests/compositor/CMakeLists.txt	2015-07-23 19:14:27 +0000
+++ tests/unit-tests/compositor/CMakeLists.txt	2015-07-27 11:30:52 +0000
@@ -13,6 +13,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_timeout_frame_dropping_policy.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_multi_monitor_arbiter.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_client_buffers.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_dropping_schedule.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_queueing_schedule.cpp
 )
 

=== added file 'tests/unit-tests/compositor/test_dropping_schedule.cpp'
--- tests/unit-tests/compositor/test_dropping_schedule.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/compositor/test_dropping_schedule.cpp	2015-07-27 11:30:52 +0000
@@ -0,0 +1,100 @@
+/*
+ * Copyright © 2015 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir/frontend/client_buffers.h"
+#include "src/server/compositor/dropping_schedule.h"
+#include "mir/test/doubles/stub_buffer.h"
+#include "mir/test/fake_shared.h"
+#include <gtest/gtest.h>
+
+using namespace testing;
+namespace mtd = mir::test::doubles;
+namespace mt = mir::test;
+namespace mg = mir::graphics;
+namespace mc = mir::compositor;
+namespace mf = mir::frontend;
+namespace
+{
+
+struct MockBufferMap : mf::ClientBuffers
+{
+    MOCK_METHOD1(add_buffer, void(mg::BufferProperties const&));
+    MOCK_METHOD1(remove_buffer, void(mg::BufferID id));
+    MOCK_METHOD1(send_buffer, void(mg::BufferID id));
+    MOCK_METHOD1(at, std::shared_ptr<mg::Buffer>&(mg::BufferID));
+    std::shared_ptr<mg::Buffer>& operator[](mg::BufferID id) { return at(id); }
+};
+
+struct DroppingSchedule : Test
+{
+    DroppingSchedule()
+    {
+        for(auto i = 0u; i < num_buffers; i++)
+            buffers.emplace_back(std::make_shared<mtd::StubBuffer>());
+    }
+    unsigned int const num_buffers{5};
+    std::vector<std::shared_ptr<mg::Buffer>> buffers;
+
+    MockBufferMap mock_client_buffers;
+    mc::DroppingSchedule schedule{mt::fake_shared(mock_client_buffers)};
+    std::vector<std::shared_ptr<mg::Buffer>> drain_queue()
+    {
+        std::vector<std::shared_ptr<mg::Buffer>> scheduled_buffers;
+        while(schedule.anything_scheduled())
+            scheduled_buffers.emplace_back(schedule.next_buffer());
+        return scheduled_buffers;
+    }
+};
+}
+
+TEST_F(DroppingSchedule, throws_if_no_buffers)
+{
+    EXPECT_FALSE(schedule.anything_scheduled());
+    EXPECT_THROW({
+        schedule.next_buffer();
+    }, std::logic_error);
+}
+
+TEST_F(DroppingSchedule, drops_excess_buffers)
+{
+    InSequence seq;
+    EXPECT_CALL(mock_client_buffers, send_buffer(buffers[0]->id()));
+    EXPECT_CALL(mock_client_buffers, send_buffer(buffers[1]->id()));
+    EXPECT_CALL(mock_client_buffers, send_buffer(buffers[2]->id()));
+    EXPECT_CALL(mock_client_buffers, send_buffer(buffers[3]->id()));
+ 
+    for(auto i = 0u; i < num_buffers; i++)
+        schedule.schedule(buffers[i]);
+
+    auto queue = drain_queue();
+    ASSERT_THAT(queue, SizeIs(1));
+    EXPECT_THAT(queue[0]->id(), Eq(buffers[4]->id()));
+}
+
+TEST_F(DroppingSchedule, queueing_same_buffer_many_times_doesnt_drop)
+{
+    EXPECT_CALL(mock_client_buffers, send_buffer(_)).Times(0);
+ 
+    schedule.schedule(buffers[2]);
+    schedule.schedule(buffers[2]);
+    schedule.schedule(buffers[2]);
+
+    auto queue = drain_queue();
+    ASSERT_THAT(queue, SizeIs(1));
+    EXPECT_THAT(queue[0]->id(), Eq(buffers[2]->id()));
+}