Merge lp:~kdub/mir/nested-db-link-to-streams into lp:mir

[Kevin DuBois]

nested: add a currently-unused way to link the nested MirBufferStream abstraction to a mgn::DisplayBuffer, and have the buffer associate incoming renderables with linked streams to see if an optimized render is possible.

nested: add a currently-unused way to link the nested MirBufferStream abstraction to a mgn::DisplayBuffer, and have the buffer associate incoming renderables with linked streams to see if an optimized render is possible.

note: mgn::HostStream will have to expand a bit, depending on how that RFC branch shapes up. mgn::DisplayBuffer::post_renderables_if_optimizable() will then use those positioning functions to arrange and commit the scene to the host in sync.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2484
http://jenkins.qa.ubuntu.com/job/mir-ci/3521/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2053
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/2052
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2003
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1518
        deb: http://jenkins.qa.ubuntu.com/job/mir-vivid-amd64-ci/1518/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2003
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2003/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/4936
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/19615

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

281+TEST_F(NestedDisplayBufferTest, post_optimized_success)

"post_optimized_success" is not a very clear test name. It is worth considering the "given/when/then" rule. (E.g. http://martinfowler.com/bliki/GivenWhenThen.html)

given_an_optimizable_stream_when_post_if_optimizable_then_result_is_success

Or more succinctly:

given_an_optimizable_stream_post_if_optimizable_succeeds

[Alan Griffiths (alan-griffiths)]

Actually, I misunderstood the test:

when_linked_with_a_stream_post_if_optimizable_succeeds

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

39+ //validate that we have a host-allocated stream for every renderable we're given
40+ for(auto const& renderable : renderables)
41+ {
42+ auto buffer = renderable->buffer();
43+ auto stream_it = std::find_if(streams.begin(), streams.end(),
44+ [&buffer](HostStream* stream){ return buffer_is_current_in_stream(*buffer, stream); });
45+ //if we have any stream that needs to be rendered but we don't have a stream associated
46+ //we can't passthrough the render
47+ if (stream_it == streams.end())
48+ return false;
49+ }
50+
51+ //TODO: work out how to submit changes synchronously to the client API
52+ for(auto const& renderable : renderables)
53+ {
54+ //TODO: arrange the surfaces via the api according to how RenderableList is structured
55+ auto buffer = renderable->buffer();
56+ auto stream_it = std::find_if(streams.begin(), streams.end(),
57+ [&buffer](HostStream* stream){ return buffer_is_current_in_stream(*buffer, stream); });
58+ (*stream_it)->swap();59+ }

I've not thought it through, but this code has the ugliness I associate with choosing the wrong data structure (or algorithm).

[Kevin DuBois (kdub)]

> I've not thought it through, but this code has the ugliness I associate with
> choosing the wrong data structure (or algorithm).

Fair enough. At a high level though, the linkage between the BufferQueue and the DisplayBuffer is needed. The BufferQueue has to get its buffers from MirBufferStream, and the DisplayBuffer has to sort and arrange those streams to paint the desired picture.

It does become nicer if the client isn't so locked into its choices of buffers to use (currently the client has 1 buffer it can use). If the client could allocate more than one buffer, it could simply use those as the buffers in BufferQueue, and mgn::DisplayBuffer could figure out which which buffer to set as the 'active one' when it goes to post, as well as arrange appropriately.

Maybe I'll explore that route a bit more.

Unmerged revisions

2484. By Kevin DuBois on 2015-04-14

merge mir

2483. By Kevin DuBois on 2015-04-14

cleanups

2482. By Kevin DuBois on 2015-04-13

missing files

2481. By Kevin DuBois on 2015-04-13

test to pass

2480. By Kevin DuBois on 2015-04-13

another test

2479. By Kevin DuBois on 2015-04-13

test to compile

2478. By Kevin DuBois on 2015-04-13

add tests for nested posting for nested db

=== modified file 'src/server/graphics/nested/display_buffer.cpp'
--- src/server/graphics/nested/display_buffer.cpp	2015-04-10 01:59:08 +0000
+++ src/server/graphics/nested/display_buffer.cpp	2015-04-14 17:12:08 +0000
@@ -19,12 +19,15 @@
 #include "display_buffer.h"
 
 #include "host_connection.h"
+#include "host_stream.h"
 #include "mir/input/input_dispatcher.h"
 #include "mir/graphics/pixel_format_utils.h"
+#include "mir/graphics/buffer.h"
 #include "mir/events/event_private.h"
 
 #include <boost/throw_exception.hpp>
 #include <stdexcept>
+#include <algorithm>
 
 namespace mg = mir::graphics;
 namespace mgn = mir::graphics::nested;
@@ -69,9 +72,41 @@
     eglSwapBuffers(egl_display, egl_surface);
 }
 
-bool mgn::detail::DisplayBuffer::post_renderables_if_optimizable(RenderableList const&)
-{
-    return false;
+namespace
+{
+bool buffer_is_current_in_stream(mg::Buffer& buffer, mgn::HostStream* stream)
+{
+    return buffer.id() == stream->current_buffer().lock()->id(); 
+}
+}
+
+bool mgn::detail::DisplayBuffer::post_renderables_if_optimizable(RenderableList const& renderables)
+{
+    std::unique_lock<decltype(stream_mutex)> lk(stream_mutex);
+
+    //validate that we have a host-allocated stream for every renderable we're given
+    for(auto const& renderable : renderables)
+    {
+        auto buffer = renderable->buffer();
+        auto stream_it = std::find_if(streams.begin(), streams.end(),
+            [&buffer](HostStream* stream){ return buffer_is_current_in_stream(*buffer, stream); });
+        //if we have any stream that needs to be rendered but we don't have a stream associated
+        //we can't passthrough the render
+        if (stream_it == streams.end())
+            return false;
+    }
+
+    //TODO: work out how to submit changes synchronously to the client API
+    for(auto const& renderable : renderables)
+    {
+        //TODO: arrange the surfaces via the api according to how RenderableList is structured
+        auto buffer = renderable->buffer();
+        auto stream_it = std::find_if(streams.begin(), streams.end(),
+            [&buffer](HostStream* stream){ return buffer_is_current_in_stream(*buffer, stream); });
+        (*stream_it)->swap();
+    }
+
+    return true;
 }
 
 MirOrientation mgn::detail::DisplayBuffer::orientation() const
@@ -119,3 +154,17 @@
         dispatcher->dispatch(event);
     }
 }
+
+void mgn::detail::DisplayBuffer::link_with_stream(HostStream* stream)
+{
+    std::unique_lock<decltype(stream_mutex)> lk(stream_mutex);
+    streams.insert(stream);
+}
+
+void mgn::detail::DisplayBuffer::unlink_from_stream(HostStream* stream)
+{
+    std::unique_lock<decltype(stream_mutex)> lk(stream_mutex);
+    auto it = streams.find(stream);
+    if (it != streams.end())
+        streams.erase(it);
+}

=== modified file 'src/server/graphics/nested/display_buffer.h'
--- src/server/graphics/nested/display_buffer.h	2015-04-08 03:02:37 +0000
+++ src/server/graphics/nested/display_buffer.h	2015-04-14 17:12:08 +0000
@@ -24,6 +24,8 @@
 #include "host_surface.h"
 
 #include <EGL/egl.h>
+#include <mutex>
+#include <set>
 
 namespace mir
 {
@@ -32,6 +34,7 @@
 namespace nested
 {
 class HostSurface;
+class HostStream;
 
 namespace detail
 {
@@ -57,6 +60,9 @@
 
     bool post_renderables_if_optimizable(RenderableList const& renderlist) override;
 
+    void link_with_stream(HostStream*);
+    void unlink_from_stream(HostStream*);
+
     DisplayBuffer(DisplayBuffer const&) = delete;
     DisplayBuffer operator=(DisplayBuffer const&) = delete;
 private:
@@ -69,6 +75,9 @@
     std::shared_ptr<input::InputDispatcher> const dispatcher;
     EGLSurfaceHandle const egl_surface;
 
+    std::mutex stream_mutex;
+    std::set<HostStream*> streams;
+
     static void event_thunk(MirSurface* surface, MirEvent const* event, void* context);
     void mir_event(MirEvent const& event);
 };

=== added file 'src/server/graphics/nested/host_stream.h'
--- src/server/graphics/nested/host_stream.h	1970-01-01 00:00:00 +0000
+++ src/server/graphics/nested/host_stream.h	2015-04-14 17:12:08 +0000
@@ -0,0 +1,47 @@
+/*
+ * 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_GRAPHICS_NESTED_HOST_STREAM_H_
+#define MIR_GRAPHICS_NESTED_HOST_STREAM_H_
+
+#include <memory>
+
+namespace mir
+{
+namespace graphics
+{
+class Buffer;
+namespace nested
+{
+
+class HostStream
+{
+public:
+    virtual ~HostStream() = default;
+    virtual std::weak_ptr<Buffer> current_buffer() const = 0;
+    virtual void swap() = 0;
+protected:
+    HostStream() = default;
+    HostStream(HostStream const&) = delete;
+    HostStream& operator=(HostStream const&) = delete;
+};
+
+}
+}
+}
+#endif // MIR_GRAPHICS_NESTED_HOST_STREAM_H_

=== added file 'tests/include/mir_test_doubles/mock_host_stream.h'
--- tests/include/mir_test_doubles/mock_host_stream.h	1970-01-01 00:00:00 +0000
+++ tests/include/mir_test_doubles/mock_host_stream.h	2015-04-14 17:12:08 +0000
@@ -0,0 +1,43 @@
+/*
+ * 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_TEST_DOUBLES_MOCK_HOST_STREAM_H_
+#define MIR_TEST_DOUBLES_MOCK_HOST_STREAM_H_
+
+#include "src/server/graphics/nested/host_stream.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace mir
+{
+namespace test
+{
+namespace doubles
+{
+
+struct MockHostStream : graphics::nested::HostStream
+{
+    MOCK_CONST_METHOD0(current_buffer, std::weak_ptr<graphics::Buffer>());
+    MOCK_METHOD0(swap, void());
+};
+
+}
+}
+}
+#endif /* MIR_TEST_DOUBLES_MOCK_HOST_STREAM_H_ */

=== modified file 'tests/unit-tests/graphics/nested/test_nested_display_buffer.cpp'
--- tests/unit-tests/graphics/nested/test_nested_display_buffer.cpp	2015-04-08 03:02:37 +0000
+++ tests/unit-tests/graphics/nested/test_nested_display_buffer.cpp	2015-04-14 17:12:08 +0000
@@ -21,7 +21,10 @@
 #include "src/server/input/null_input_dispatcher.h"
 
 #include "mir_test_doubles/mock_egl.h"
+#include "mir_test_doubles/mock_host_stream.h"
 #include "mir_test_doubles/stub_gl_config.h"
+#include "mir_test_doubles/stub_buffer.h"
+#include "mir_test_doubles/stub_renderable.h"
 #include "mir_test/fake_shared.h"
 
 #include <gtest/gtest.h>
@@ -47,24 +50,28 @@
         : egl_disp_handle{mock_egl.fake_egl_display,
                           mt::fake_shared(stub_gl_config)},
           default_rect{{100,100}, {800,600}}
-    {}
+    {
+        ON_CALL(mock_stream, current_buffer())
+            .WillByDefault(testing::Return(stub_buffer));
+    }
     testing::NiceMock<mtd::MockEGL> mock_egl;
     mtd::StubGLConfig stub_gl_config;
     NullHostSurface null_host_surface;
     mi::NullInputDispatcher null_input_dispatcher;
     mgnd::EGLDisplayHandle egl_disp_handle;
     geom::Rectangle const default_rect;
+    mgnd::DisplayBuffer db{
+        egl_disp_handle,
+        mt::fake_shared(null_host_surface),
+        default_rect,
+        mt::fake_shared(null_input_dispatcher),
+        mir_pixel_format_abgr_8888};
+    testing::NiceMock<mtd::MockHostStream> mock_stream;
+    std::shared_ptr<mtd::StubBuffer> stub_buffer{std::make_shared<mtd::StubBuffer>()};
 };
 
 TEST_F(NestedDisplayBufferTest, alpha_enabled_pixel_format_enables_destination_alpha)
 {
-    mgnd::DisplayBuffer db{
-        egl_disp_handle,
-        mt::fake_shared(null_host_surface),
-        default_rect,
-        mt::fake_shared(null_input_dispatcher),
-        mir_pixel_format_abgr_8888};
-
     EXPECT_TRUE(db.uses_alpha());
 }
 
@@ -79,3 +86,34 @@
 
     EXPECT_FALSE(db.uses_alpha());
 }
+
+TEST_F(NestedDisplayBufferTest, post_optimized_success)
+{
+    EXPECT_CALL(mock_stream, swap());
+    db.link_with_stream(&mock_stream);
+
+    auto renderable_from_stream = std::make_shared<mtd::StubRenderable>(stub_buffer);
+    EXPECT_TRUE(db.post_renderables_if_optimizable({renderable_from_stream}));
+    db.unlink_from_stream(&mock_stream);
+}
+
+TEST_F(NestedDisplayBufferTest, unlinking_causes_rejections)
+{
+    db.link_with_stream(&mock_stream);
+
+    auto renderable_from_stream = std::make_shared<mtd::StubRenderable>(
+        stub_buffer);
+    db.post_renderables_if_optimizable({renderable_from_stream});
+    db.unlink_from_stream(&mock_stream);
+
+    EXPECT_FALSE(db.post_renderables_if_optimizable({renderable_from_stream}));
+}
+
+TEST_F(NestedDisplayBufferTest, post_optimized_rejection_because_of_unknown_buffer)
+{
+    db.link_with_stream(&mock_stream);
+    
+    auto nonstream_renderable = std::make_shared<mtd::StubRenderable>();
+    EXPECT_FALSE(db.post_renderables_if_optimizable({nonstream_renderable}));
+    db.unlink_from_stream(&mock_stream);
+}