Mir

Merge lp:~raof/mir/further-buffer-plumbing-cleanup into lp:mir

Proposed by Chris Halse Rogers on 2017-07-07
Status: Merged
Approved by: Alan Griffiths on 2017-08-16
Approved revision: 4222
Merged at revision: 4229
Proposed branch: lp:~raof/mir/further-buffer-plumbing-cleanup
Merge into: lp:mir
Prerequisite: lp:~raof/mir/no-ipc-on-compositor-threads
Diff against target: 1034 lines (+171/-490)
11 files modified
src/server/compositor/CMakeLists.txt (+0/-1)
src/server/compositor/buffer_acquisition.h (+17/-2)
src/server/compositor/multi_monitor_arbiter.cpp (+11/-66)
src/server/compositor/multi_monitor_arbiter.h (+1/-16)
src/server/compositor/stream.cpp (+2/-4)
src/server/compositor/temporary_buffers.cpp (+0/-81)
src/server/compositor/temporary_buffers.h (+0/-74)
tests/integration-tests/test_buffer_scheduling.cpp (+31/-7)
tests/unit-tests/compositor/CMakeLists.txt (+0/-1)
tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp (+109/-95)
tests/unit-tests/compositor/test_temporary_buffers.cpp (+0/-143)
To merge this branch: bzr merge lp:~raof/mir/further-buffer-plumbing-cleanup
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Approve on 2017-08-16
Alan Griffiths Approve on 2017-07-21
Brandon Schaefer (community) 2017-07-07 Approve on 2017-07-20
Review via email: mp+326987@code.launchpad.net

Commit message

Remove even more manual reference counting of mg::Buffer-s.

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:4219
https://mir-jenkins.ubuntu.com/job/mir-ci/3479/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4752/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4910
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4899
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4899
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4899
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4789/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4789/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4789/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4789/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4789/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4789/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4789/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4789
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4789/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3479/rebuild

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4220
https://mir-jenkins.ubuntu.com/job/mir-ci/3483/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4758
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4916
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4905
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4905
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4905
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4795/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4795
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4795/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3483/rebuild

review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

I dont have the full context on the reason for temp buffers, but this clean up looks very nice.

review: Approve
Chris Halse Rogers (raof) wrote :

The temp buffers were there to wrap
compositor_acquire/compositor_release and
snapshot_acquire/snapshot_release, mainly in order to drive the
buffer-release IPC.

Since we now drive the buffer-release IPC based on when the buffer
submitted to the queue hits refcount 0, we no longer need the extra
wrapper.

Alan Griffiths (alan-griffiths) wrote :

I think this is OK, but from past experience this is a fragile piece of functionality.

Are we completely sure that the change from "onscreen_buffers" to "current_buffer" accommodates multi-monitor?

review: Needs Information
Chris Halse Rogers (raof) wrote :

Yes.

There's always a current_buffer to give out to a compositor, and it is advanced at the same times that a new buffer was pushed to the front of onscreen_buffers.

We needed onscreen_buffers in order to reference-count the buffers we'd handed out to ensure they were returned to the client only when unused. Since we now do that directly in the shared_ptr<>, onscreen_buffers is no longer necessary.

You can see that we only ever push onto the front of onscreen_buffers and only ever access the buffer associated with the front of onscreen_buffers.

In addition to this theoretical analysis, it correctly runs lots of clients on my 3-head AMD/NVIDIA hybrid system, including clients that span heads (and, indeed, GPUs), and also appears correct in clone mode.

Alan Griffiths (alan-griffiths) wrote :

> In addition to this theoretical analysis, it correctly runs lots of clients on
> my 3-head AMD/NVIDIA hybrid system, including clients that span heads (and,
> indeed, GPUs), and also appears correct in clone mode.

Thanks, I couldn't try that as I'm away from my desktop.

review: Approve
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4221
https://mir-jenkins.ubuntu.com/job/mir-ci/3502/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4791
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4966
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4955
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4955
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4955
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4828/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4828
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4828/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3502/rebuild

review: Approve (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
Unapproved changes made after approval.
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1389/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4872
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1514/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4909/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1389/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4872
    FAILURE: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1514/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1515/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/5085
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/5074
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4909/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4909
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4909/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Mir CI Bot (mir-ci-bot) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/server/compositor/CMakeLists.txt'
2--- src/server/compositor/CMakeLists.txt 2017-05-22 23:26:24 +0000
3+++ src/server/compositor/CMakeLists.txt 2017-08-08 04:14:04 +0000
4@@ -9,7 +9,6 @@
5
6 default_display_buffer_compositor.cpp
7 default_display_buffer_compositor_factory.cpp
8- temporary_buffers.cpp
9 buffer_stream_factory.cpp
10 multi_threaded_compositor.cpp
11 occlusion.cpp
12
13=== modified file 'src/server/compositor/buffer_acquisition.h'
14--- src/server/compositor/buffer_acquisition.h 2017-07-28 17:00:43 +0000
15+++ src/server/compositor/buffer_acquisition.h 2017-08-08 04:14:04 +0000
16@@ -36,6 +36,11 @@
17 /**
18 * Acquire the next buffer that's ready to display/composite.
19 *
20+ * \note The returned buffer is considered in-use until the its
21+ * use-count reaches 0. In-use buffers will not be returned
22+ * to the client, so for best performance it is important to
23+ * release the returned buffer as soon as possible.
24+ *
25 * \param [in] user_id A unique identifier of who is going to use the
26 * buffer, to ensure that separate users representing
27 * separate monitors who need the same frame will get
28@@ -46,9 +51,19 @@
29 */
30 virtual std::shared_ptr<graphics::Buffer>
31 compositor_acquire(void const* user_id) = 0;
32- virtual void compositor_release(std::shared_ptr<graphics::Buffer> const&) = 0;
33+
34+ /**
35+ * Acquire the most recently displayed buffer.
36+ *
37+ * In contrast with compositor_acquire() this does not consume a client
38+ * buffer.
39+ *
40+ * Like compositor_acquire(), you should release your reference to the
41+ * returned buffer as soon as possible.
42+ *
43+ * \return A shared reference to the most recent visible client buffer.
44+ */
45 virtual std::shared_ptr<graphics::Buffer> snapshot_acquire() = 0;
46- virtual void snapshot_release(std::shared_ptr<graphics::Buffer> const&) = 0;
47 virtual ~BufferAcquisition() = default;
48
49 protected:
50
51=== modified file 'src/server/compositor/multi_monitor_arbiter.cpp'
52--- src/server/compositor/multi_monitor_arbiter.cpp 2017-07-28 17:00:43 +0000
53+++ src/server/compositor/multi_monitor_arbiter.cpp 2017-08-08 04:14:04 +0000
54@@ -36,95 +36,40 @@
55
56 mc::MultiMonitorArbiter::~MultiMonitorArbiter()
57 {
58- std::lock_guard<decltype(mutex)> lk(mutex);
59- for(auto it = onscreen_buffers.begin(); it != onscreen_buffers.end(); it++)
60- {
61- if (it->use_count == 0)
62- it->buffer.reset();
63- }
64-
65 }
66
67 std::shared_ptr<mg::Buffer> mc::MultiMonitorArbiter::compositor_acquire(compositor::CompositorID id)
68 {
69 std::lock_guard<decltype(mutex)> lk(mutex);
70
71- if (onscreen_buffers.empty() && !schedule->num_scheduled())
72+ if (!current_buffer && !schedule->num_scheduled())
73 BOOST_THROW_EXCEPTION(std::logic_error("no buffer to give to compositor"));
74
75- if (current_buffer_users.find(id) != current_buffer_users.end() || onscreen_buffers.empty())
76+ if (current_buffer_users.find(id) != current_buffer_users.end() || !current_buffer)
77 {
78 if (schedule->num_scheduled())
79- onscreen_buffers.emplace_front(schedule->next_buffer(), 0);
80+ current_buffer = schedule->next_buffer();
81 current_buffer_users.clear();
82 }
83 current_buffer_users.insert(id);
84
85- auto& last_entry = onscreen_buffers.front();
86- last_entry.use_count++;
87- auto last_entry_buffer = last_entry.buffer;
88- clean_onscreen_buffers(lk);
89- return last_entry_buffer;
90-}
91-
92-void mc::MultiMonitorArbiter::compositor_release(std::shared_ptr<mg::Buffer> const& buffer)
93-{
94- std::lock_guard<decltype(mutex)> lk(mutex);
95-
96- decrease_refcount_for(buffer->id(), lk);
97-
98- if (onscreen_buffers.begin()->buffer != buffer)
99- clean_onscreen_buffers(lk);
100-}
101-
102-void mc::MultiMonitorArbiter::decrease_refcount_for(mg::BufferID id, std::lock_guard<std::mutex> const&)
103-{
104- auto it = std::find_if(onscreen_buffers.begin(), onscreen_buffers.end(),
105- [&id](ScheduleEntry const& s) { return s.buffer->id() == id; });
106- if (it == onscreen_buffers.end())
107- BOOST_THROW_EXCEPTION(std::logic_error("buffer not scheduled"));
108- it->use_count--;
109-}
110-
111-void mc::MultiMonitorArbiter::clean_onscreen_buffers(std::lock_guard<std::mutex> const&)
112-{
113- for(auto it = onscreen_buffers.begin(); it != onscreen_buffers.end();)
114- {
115- if ((it->use_count == 0) &&
116- (it != onscreen_buffers.begin() || schedule->num_scheduled())) //ensure monitors always have a buffer
117- {
118- it = onscreen_buffers.erase(it);
119- }
120- else
121- {
122- it++;
123- }
124- }
125+ return current_buffer;
126 }
127
128 std::shared_ptr<mg::Buffer> mc::MultiMonitorArbiter::snapshot_acquire()
129 {
130 std::lock_guard<decltype(mutex)> lk(mutex);
131
132- if (onscreen_buffers.empty() && !schedule->num_scheduled())
133+ if (!current_buffer && !schedule->num_scheduled())
134 BOOST_THROW_EXCEPTION(std::logic_error("no buffer to give to snapshotter"));
135
136- if (onscreen_buffers.empty())
137+ if (!current_buffer)
138 {
139 if (schedule->num_scheduled())
140- onscreen_buffers.emplace_front(schedule->next_buffer(), 0);
141+ current_buffer = schedule->next_buffer();
142 }
143
144- auto& last_entry = onscreen_buffers.front();
145- last_entry.use_count++;
146- return last_entry.buffer;
147-}
148-
149-void mc::MultiMonitorArbiter::snapshot_release(std::shared_ptr<mg::Buffer> const& buffer)
150-{
151- std::lock_guard<decltype(mutex)> lk(mutex);
152- decrease_refcount_for(buffer->id(), lk);
153- clean_onscreen_buffers(lk);
154+ return current_buffer;
155 }
156
157 void mc::MultiMonitorArbiter::set_schedule(std::shared_ptr<Schedule> const& new_schedule)
158@@ -137,13 +82,13 @@
159 {
160 std::lock_guard<decltype(mutex)> lk(mutex);
161 return schedule->num_scheduled() ||
162- ((current_buffer_users.find(id) == current_buffer_users.end()) && !onscreen_buffers.empty());
163+ ((current_buffer_users.find(id) == current_buffer_users.end()) && current_buffer);
164 }
165
166 bool mc::MultiMonitorArbiter::has_buffer()
167 {
168 std::lock_guard<decltype(mutex)> lk(mutex);
169- return !onscreen_buffers.empty();
170+ return static_cast<bool>(current_buffer);
171 }
172
173 void mc::MultiMonitorArbiter::advance_schedule()
174@@ -151,7 +96,7 @@
175 std::lock_guard<decltype(mutex)> lk(mutex);
176 if (schedule->num_scheduled())
177 {
178- onscreen_buffers.emplace_front(schedule->next_buffer(), 0);
179+ current_buffer = schedule->next_buffer();
180 current_buffer_users.clear();
181 }
182 }
183
184=== modified file 'src/server/compositor/multi_monitor_arbiter.h'
185--- src/server/compositor/multi_monitor_arbiter.h 2017-07-28 17:00:43 +0000
186+++ src/server/compositor/multi_monitor_arbiter.h 2017-08-08 04:14:04 +0000
187@@ -42,30 +42,15 @@
188 ~MultiMonitorArbiter();
189
190 std::shared_ptr<graphics::Buffer> compositor_acquire(compositor::CompositorID id) override;
191- void compositor_release(std::shared_ptr<graphics::Buffer> const&) override;
192 std::shared_ptr<graphics::Buffer> snapshot_acquire() override;
193- void snapshot_release(std::shared_ptr<graphics::Buffer> const&) override;
194 void set_schedule(std::shared_ptr<Schedule> const& schedule);
195 bool buffer_ready_for(compositor::CompositorID id);
196 bool has_buffer();
197 void advance_schedule();
198
199 private:
200- void decrease_refcount_for(graphics::BufferID id, std::lock_guard<std::mutex> const&);
201- void clean_onscreen_buffers(std::lock_guard<std::mutex> const&);
202-
203 std::mutex mutable mutex;
204- struct ScheduleEntry
205- {
206- ScheduleEntry(std::shared_ptr<graphics::Buffer> const& buffer, unsigned int use_count) :
207- buffer(buffer),
208- use_count(use_count)
209- {
210- }
211- std::shared_ptr<graphics::Buffer> buffer;
212- unsigned int use_count;
213- };
214- std::deque<ScheduleEntry> onscreen_buffers;
215+ std::shared_ptr<graphics::Buffer> current_buffer;
216 std::set<compositor::CompositorID> current_buffer_users;
217 std::shared_ptr<Schedule> schedule;
218 };
219
220=== modified file 'src/server/compositor/stream.cpp'
221--- src/server/compositor/stream.cpp 2017-08-08 04:14:03 +0000
222+++ src/server/compositor/stream.cpp 2017-08-08 04:14:04 +0000
223@@ -20,7 +20,6 @@
224 #include "stream.h"
225 #include "queueing_schedule.h"
226 #include "dropping_schedule.h"
227-#include "temporary_buffers.h"
228 #include "mir/graphics/buffer.h"
229 #include <boost/throw_exception.hpp>
230
231@@ -73,8 +72,7 @@
232 void mc::Stream::with_most_recent_buffer_do(std::function<void(mg::Buffer&)> const& fn)
233 {
234 std::lock_guard<decltype(mutex)> lk(mutex);
235- TemporarySnapshotBuffer buffer(arbiter);
236- fn(buffer);
237+ fn(*arbiter->snapshot_acquire());
238 }
239
240 MirPixelFormat mc::Stream::pixel_format() const
241@@ -96,7 +94,7 @@
242
243 std::shared_ptr<mg::Buffer> mc::Stream::lock_compositor_buffer(void const* id)
244 {
245- return std::make_shared<mc::TemporaryCompositorBuffer>(arbiter, id);
246+ return arbiter->compositor_acquire(id);
247 }
248
249 geom::Size mc::Stream::stream_size()
250
251=== removed file 'src/server/compositor/temporary_buffers.cpp'
252--- src/server/compositor/temporary_buffers.cpp 2017-07-28 17:00:43 +0000
253+++ src/server/compositor/temporary_buffers.cpp 1970-01-01 00:00:00 +0000
254@@ -1,81 +0,0 @@
255-/*
256- * Copyright © 2012 Canonical Ltd.
257- *
258- * This program is free software: you can redistribute it and/or modify it
259- * under the terms of the GNU General Public License version 2 or 3,
260- * as published by the Free Software Foundation.
261- *
262- * This program is distributed in the hope that it will be useful,
263- * but WITHOUT ANY WARRANTY; without even the implied warranty of
264- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
265- * GNU General Public License for more details.
266- *
267- * You should have received a copy of the GNU General Public License
268- * along with this program. If not, see <http://www.gnu.org/licenses/>.
269- *
270- * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
271- */
272-
273-#include "buffer_acquisition.h"
274-#include "temporary_buffers.h"
275-
276-#include <boost/throw_exception.hpp>
277-#include <stdexcept>
278-
279-namespace mc=mir::compositor;
280-namespace mg=mir::graphics;
281-namespace geom=mir::geometry;
282-
283-mc::TemporaryBuffer::TemporaryBuffer(std::shared_ptr<mg::Buffer> const& real_buffer)
284- : buffer(real_buffer)
285-{
286-}
287-
288-mc::TemporaryCompositorBuffer::TemporaryCompositorBuffer(
289- std::shared_ptr<BufferAcquisition> const& acquisition, void const* user_id)
290- : TemporaryBuffer(acquisition->compositor_acquire(user_id)),
291- acquisition(acquisition)
292-{
293-}
294-
295-mc::TemporaryCompositorBuffer::~TemporaryCompositorBuffer()
296-{
297- acquisition->compositor_release(buffer);
298-}
299-
300-mc::TemporarySnapshotBuffer::TemporarySnapshotBuffer(
301- std::shared_ptr<BufferAcquisition> const& acquisition)
302- : TemporaryBuffer(acquisition->snapshot_acquire()),
303- acquisition(acquisition)
304-{
305-}
306-
307-mc::TemporarySnapshotBuffer::~TemporarySnapshotBuffer()
308-{
309- acquisition->snapshot_release(buffer);
310-}
311-
312-geom::Size mc::TemporaryBuffer::size() const
313-{
314- return buffer->size();
315-}
316-
317-MirPixelFormat mc::TemporaryBuffer::pixel_format() const
318-{
319- return buffer->pixel_format();
320-}
321-
322-mg::BufferID mc::TemporaryBuffer::id() const
323-{
324- return buffer->id();
325-}
326-
327-std::shared_ptr<mg::NativeBuffer> mc::TemporaryBuffer::native_buffer_handle() const
328-{
329- return buffer->native_buffer_handle();
330-}
331-
332-mg::NativeBufferBase* mc::TemporaryBuffer::native_buffer_base()
333-{
334- return buffer->native_buffer_base();
335-}
336
337=== removed file 'src/server/compositor/temporary_buffers.h'
338--- src/server/compositor/temporary_buffers.h 2017-07-28 17:00:43 +0000
339+++ src/server/compositor/temporary_buffers.h 1970-01-01 00:00:00 +0000
340@@ -1,74 +0,0 @@
341-/*
342- * Copyright © 2012 Canonical Ltd.
343- *
344- * This program is free software: you can redistribute it and/or modify it
345- * under the terms of the GNU General Public License version 2 or 3,
346- * as published by the Free Software Foundation.
347- *
348- * This program is distributed in the hope that it will be useful,
349- * but WITHOUT ANY WARRANTY; without even the implied warranty of
350- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
351- * GNU General Public License for more details.
352- *
353- * You should have received a copy of the GNU General Public License
354- * along with this program. If not, see <http://www.gnu.org/licenses/>.
355- *
356- * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
357- */
358-
359-#ifndef MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_
360-#define MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_
361-
362-#include "mir/graphics/buffer.h"
363-#include "mir/graphics/buffer_id.h"
364-
365-namespace mg = mir::graphics;
366-
367-namespace mir
368-{
369-namespace compositor
370-{
371-
372-class BufferAcquisition;
373-class BackBufferStrategy;
374-
375-class TemporaryBuffer : public mg::Buffer
376-{
377-public:
378- geometry::Size size() const override;
379- MirPixelFormat pixel_format() const override;
380- mg::BufferID id() const override;
381- std::shared_ptr<mg::NativeBuffer> native_buffer_handle() const override;
382- graphics::NativeBufferBase* native_buffer_base() override;
383-
384-protected:
385- explicit TemporaryBuffer(std::shared_ptr<mg::Buffer> const& real_buffer);
386- std::shared_ptr<mg::Buffer> const buffer;
387-};
388-
389-class TemporaryCompositorBuffer : public TemporaryBuffer
390-{
391-public:
392- explicit TemporaryCompositorBuffer(
393- std::shared_ptr<BufferAcquisition> const& acquisition, void const* user_id);
394- ~TemporaryCompositorBuffer();
395-
396-private:
397- std::shared_ptr<BufferAcquisition> const acquisition;
398-};
399-
400-class TemporarySnapshotBuffer : public TemporaryBuffer
401-{
402-public:
403- explicit TemporarySnapshotBuffer(
404- std::shared_ptr<BufferAcquisition> const& acquisition);
405- ~TemporarySnapshotBuffer();
406-
407-private:
408- std::shared_ptr<BufferAcquisition> const acquisition;
409-};
410-
411-}
412-}
413-
414-#endif /* MIR_COMPOSITOR_TEMPORARY_BUFFERS_H_ */
415
416=== modified file 'tests/integration-tests/test_buffer_scheduling.cpp'
417--- tests/integration-tests/test_buffer_scheduling.cpp 2017-07-28 17:00:43 +0000
418+++ tests/integration-tests/test_buffer_scheduling.cpp 2017-08-08 04:14:04 +0000
419@@ -24,7 +24,6 @@
420 #include "src/client/protobuf_to_native_buffer.h"
421 #include "src/client/connection_surface_map.h"
422 #include "src/server/compositor/stream.h"
423-#include "src/server/compositor/temporary_buffers.h"
424 #include "mir/test/doubles/stub_client_buffer_factory.h"
425 #include "mir/test/doubles/mock_client_buffer_factory.h"
426 #include "mir/test/doubles/stub_buffer_allocator.h"
427@@ -454,22 +453,49 @@
428 return never_blocks;
429 }
430
431-class AutoSendBuffer : public mc::TemporaryBuffer
432+class AutoSendBuffer : public mg::Buffer
433 {
434 public:
435 AutoSendBuffer(
436 std::shared_ptr<mg::Buffer> const& wrapped,
437 std::shared_ptr<mf::BufferSink> const& sink)
438- : TemporaryBuffer(wrapped),
439+ : wrapped{wrapped},
440 sink{sink}
441 {
442 }
443
444 ~AutoSendBuffer()
445 {
446- sink->update_buffer(*buffer);
447- }
448+ sink->update_buffer(*wrapped);
449+ }
450+
451+ std::shared_ptr<mg::NativeBuffer> native_buffer_handle() const override
452+ {
453+ return wrapped->native_buffer_handle();
454+ }
455+
456+ mg::BufferID id() const override
457+ {
458+ return wrapped->id();
459+ }
460+
461+ geom::Size size() const override
462+ {
463+ return wrapped->size();
464+ }
465+
466+ MirPixelFormat pixel_format() const override
467+ {
468+ return wrapped->pixel_format();
469+ }
470+
471+ mg::NativeBufferBase *native_buffer_base() override
472+ {
473+ return wrapped->native_buffer_base();
474+ }
475+
476 private:
477+ std::shared_ptr<mg::Buffer> const wrapped;
478 std::shared_ptr<mf::BufferSink> const sink;
479 };
480
481@@ -1175,7 +1201,6 @@
482 producer->produce();
483 a = consumer->consume_resource();
484 b = consumer->consume_resource();
485- EXPECT_THAT(a, Ne(b));
486 }
487 a.reset();
488 b.reset();
489@@ -1214,7 +1239,6 @@
490 {
491 first = consumer->consume_resource();
492 second = consumer->consume_resource();
493- EXPECT_THAT(first, Ne(second));
494 producer->produce();
495 }
496
497
498=== modified file 'tests/unit-tests/compositor/CMakeLists.txt'
499--- tests/unit-tests/compositor/CMakeLists.txt 2017-07-04 04:06:59 +0000
500+++ tests/unit-tests/compositor/CMakeLists.txt 2017-08-08 04:14:04 +0000
501@@ -1,7 +1,6 @@
502 list(APPEND UNIT_TEST_SOURCES
503 ${CMAKE_CURRENT_SOURCE_DIR}/test_default_display_buffer_compositor.cpp
504 ${CMAKE_CURRENT_SOURCE_DIR}/test_stream.cpp
505- ${CMAKE_CURRENT_SOURCE_DIR}/test_temporary_buffers.cpp
506 ${CMAKE_CURRENT_SOURCE_DIR}/test_multi_threaded_compositor.cpp
507 ${CMAKE_CURRENT_SOURCE_DIR}/test_occlusion.cpp
508 ${CMAKE_CURRENT_SOURCE_DIR}/test_screencast_display_buffer.cpp
509
510=== modified file 'tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp'
511--- tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp 2017-08-08 04:14:03 +0000
512+++ tests/unit-tests/compositor/test_multi_monitor_arbiter.cpp 2017-08-08 04:14:04 +0000
513@@ -21,7 +21,6 @@
514 #include "mir/test/doubles/stub_buffer_allocator.h"
515 #include "src/server/compositor/multi_monitor_arbiter.h"
516 #include "src/server/compositor/schedule.h"
517-#include "src/server/compositor/temporary_buffers.h"
518
519 #include <gtest/gtest.h>
520 using namespace testing;
521@@ -83,13 +82,13 @@
522 std::shared_ptr<mg::Buffer> const& buffer,
523 std::shared_ptr<bool> const& destroyed)
524 {
525- class DestructionNotifyingBuffer : public mc::TemporaryBuffer
526+ class DestructionNotifyingBuffer : public mg::Buffer
527 {
528 public:
529 DestructionNotifyingBuffer(
530 std::shared_ptr<mg::Buffer> const& buffer,
531 std::shared_ptr<bool> const& destroyed)
532- : TemporaryBuffer(buffer),
533+ : wrapped{buffer},
534 destroyed{destroyed}
535 {
536 }
537@@ -98,7 +97,34 @@
538 {
539 *destroyed = true;
540 }
541+
542+ std::shared_ptr<mg::NativeBuffer> native_buffer_handle() const override
543+ {
544+ return wrapped->native_buffer_handle();
545+ }
546+
547+ mg::BufferID id() const override
548+ {
549+ return wrapped->id();
550+ }
551+
552+ mir::geometry::Size size() const override
553+ {
554+ return wrapped->size();
555+ }
556+
557+ MirPixelFormat pixel_format() const override
558+ {
559+ return wrapped->pixel_format();
560+ }
561+
562+ mg::NativeBufferBase *native_buffer_base() override
563+ {
564+ return wrapped->native_buffer_base();
565+ }
566+
567 private:
568+ std::shared_ptr<mg::Buffer> const wrapped;
569 std::shared_ptr<bool> const destroyed;
570 };
571
572@@ -133,7 +159,6 @@
573
574 auto cbuffer = arbiter.compositor_acquire(this);
575 schedule.set_schedule({buffers[1]});
576- arbiter.compositor_release(cbuffer);
577 cbuffer.reset();
578 // We need to acquire a new buffer - the current one is on-screen, so can't be sent back.
579 arbiter.compositor_acquire(this);
580@@ -182,22 +207,17 @@
581 {
582 schedule.set_schedule({buffers[0],buffers[1],buffers[2],buffers[3],buffers[4]});
583
584- auto cbuffer1 = arbiter.compositor_acquire(this);
585- arbiter.compositor_release(cbuffer1);
586- auto cbuffer2 = arbiter.compositor_acquire(this);
587- arbiter.compositor_release(cbuffer2);
588- auto cbuffer3 = arbiter.compositor_acquire(this);
589- arbiter.compositor_release(cbuffer3);
590- auto cbuffer4 = arbiter.compositor_acquire(this);
591- arbiter.compositor_release(cbuffer4);
592- auto cbuffer5 = arbiter.compositor_acquire(this);
593- arbiter.compositor_release(cbuffer5);
594+ auto id1 = arbiter.compositor_acquire(this)->id();
595+ auto id2 = arbiter.compositor_acquire(this)->id();
596+ auto id3 = arbiter.compositor_acquire(this)->id();
597+ auto id4 = arbiter.compositor_acquire(this)->id();
598+ auto iddqd = arbiter.compositor_acquire(this)->id();
599
600- EXPECT_THAT(cbuffer1, IsSameBufferAs(buffers[0]));
601- EXPECT_THAT(cbuffer2, IsSameBufferAs(buffers[1]));
602- EXPECT_THAT(cbuffer3, IsSameBufferAs(buffers[2]));
603- EXPECT_THAT(cbuffer4, IsSameBufferAs(buffers[3]));
604- EXPECT_THAT(cbuffer5, IsSameBufferAs(buffers[4]));
605+ EXPECT_THAT(id1, Eq(buffers[0]->id()));
606+ EXPECT_THAT(id2, Eq(buffers[1]->id()));
607+ EXPECT_THAT(id3, Eq(buffers[2]->id()));
608+ EXPECT_THAT(id4, Eq(buffers[3]->id()));
609+ EXPECT_THAT(iddqd, Eq(buffers[4]->id()));
610 }
611
612 TEST_F(MultiMonitorArbiter, compositor_consumes_all_buffers_when_operating_as_a_bypassed_buffer_would)
613@@ -206,20 +226,28 @@
614
615 auto cbuffer1 = arbiter.compositor_acquire(this);
616 auto cbuffer2 = arbiter.compositor_acquire(this);
617- arbiter.compositor_release(cbuffer1);
618+ auto id1 = cbuffer1->id();
619+ cbuffer1.reset();
620+
621 auto cbuffer3 = arbiter.compositor_acquire(this);
622- arbiter.compositor_release(cbuffer2);
623+ auto id2 = cbuffer2->id();
624+ cbuffer2.reset();
625+
626 auto cbuffer4 = arbiter.compositor_acquire(this);
627- arbiter.compositor_release(cbuffer3);
628+ auto id3 = cbuffer3->id();
629+ cbuffer3.reset();
630+
631 auto cbuffer5 = arbiter.compositor_acquire(this);
632- arbiter.compositor_release(cbuffer4);
633- arbiter.compositor_release(cbuffer5);
634+ auto id4 = cbuffer4->id();
635+ cbuffer4.reset();
636+ auto id5 = cbuffer5->id();
637+ cbuffer5.reset();
638
639- EXPECT_THAT(cbuffer1, IsSameBufferAs(buffers[0]));
640- EXPECT_THAT(cbuffer2, IsSameBufferAs(buffers[1]));
641- EXPECT_THAT(cbuffer3, IsSameBufferAs(buffers[2]));
642- EXPECT_THAT(cbuffer4, IsSameBufferAs(buffers[3]));
643- EXPECT_THAT(cbuffer5, IsSameBufferAs(buffers[4]));
644+ EXPECT_THAT(id1, Eq(buffers[0]->id()));
645+ EXPECT_THAT(id2, Eq(buffers[1]->id()));
646+ EXPECT_THAT(id3, Eq(buffers[2]->id()));
647+ EXPECT_THAT(id4, Eq(buffers[3]->id()));
648+ EXPECT_THAT(id5, Eq(buffers[4]->id()));
649 }
650
651 TEST_F(MultiMonitorArbiter, multimonitor_compositor_buffer_syncs_to_fastest_with_more_queueing)
652@@ -287,8 +315,7 @@
653 auto cbuffer1 = arbiter.compositor_acquire(this);
654 auto cbuffer2 = arbiter.compositor_acquire(&that);
655 auto sbuffer1 = arbiter.snapshot_acquire();
656- arbiter.snapshot_release(sbuffer1);
657- arbiter.compositor_release(cbuffer2);
658+ cbuffer2.reset();
659 cbuffer2 = arbiter.compositor_acquire(&that);
660
661 auto sbuffer2 = arbiter.snapshot_acquire();
662@@ -302,17 +329,15 @@
663 auto that = 4;
664 auto a_few_times = 5u;
665 auto cbuffer1 = arbiter.compositor_acquire(this);
666- std::vector<std::shared_ptr<mg::Buffer>> snapshot_buffers(a_few_times);
667+ std::vector<mg::BufferID> snapshot_buffers(a_few_times);
668 for(auto i = 0u; i < a_few_times; i++)
669 {
670- auto b = arbiter.snapshot_acquire();
671- arbiter.snapshot_release(b);
672- snapshot_buffers[i] = b;
673+ snapshot_buffers[i] = arbiter.snapshot_acquire()->id();
674 }
675 auto cbuffer2 = arbiter.compositor_acquire(&that);
676
677 EXPECT_THAT(cbuffer1, IsSameBufferAs(cbuffer2));
678- EXPECT_THAT(snapshot_buffers, Each(IsSameBufferAs(cbuffer1)));
679+ EXPECT_THAT(snapshot_buffers, Each(Eq(cbuffer1->id())));
680 }
681
682 TEST_F(MultiMonitorArbiter, no_buffers_available_throws_on_snapshot)
683@@ -333,14 +358,12 @@
684 });
685 auto cbuffer1 = arbiter.compositor_acquire(this);
686 auto sbuffer1 = arbiter.snapshot_acquire();
687- arbiter.compositor_release(cbuffer1);
688 cbuffer1.reset();
689
690 // Acquire a new buffer so first one is no longer onscreen.
691 arbiter.compositor_acquire(this);
692
693 EXPECT_FALSE(*buffer_released);
694- arbiter.snapshot_release(sbuffer1);
695 sbuffer1.reset();
696 EXPECT_TRUE(*buffer_released);
697 }
698@@ -361,9 +384,7 @@
699 EXPECT_THAT(cbuffer1, IsSameBufferAs(cbuffer2));
700
701 auto cbuffer3 = arbiter.compositor_acquire(&comp_id1);
702- arbiter.compositor_release(cbuffer2);
703 EXPECT_FALSE(*buffer_released);
704- arbiter.compositor_release(cbuffer1);
705 cbuffer1.reset();
706 cbuffer2.reset();
707 EXPECT_TRUE(*buffer_released);
708@@ -375,15 +396,13 @@
709 int comp_id2{0};
710 schedule.set_schedule({buffers[0],buffers[1]});
711
712- auto cbuffer1 = arbiter.compositor_acquire(&comp_id1); //buffer[0]
713- arbiter.compositor_release(cbuffer1);
714- auto cbuffer2 = arbiter.compositor_acquire(&comp_id2); //buffer[0]
715- arbiter.compositor_release(cbuffer2);
716+ auto id1 = arbiter.compositor_acquire(&comp_id1)->id(); //buffer[0]
717+ auto id2 = arbiter.compositor_acquire(&comp_id2)->id(); //buffer[0]
718
719 auto cbuffer3 = arbiter.compositor_acquire(&comp_id1); //buffer[1]
720
721- EXPECT_THAT(cbuffer1, IsSameBufferAs(cbuffer2));
722- EXPECT_THAT(cbuffer1, IsSameBufferAs(buffers[0]));
723+ EXPECT_THAT(id1, Eq(id2));
724+ EXPECT_THAT(id1, Eq(buffers[0]->id()));
725 EXPECT_THAT(cbuffer3, IsSameBufferAs(buffers[1]));
726 }
727
728@@ -407,23 +426,16 @@
729 });
730
731 auto b1 = arbiter.compositor_acquire(&comp_id1);
732- arbiter.compositor_release(b1);
733+ b1.reset();
734 auto b2 = arbiter.compositor_acquire(&comp_id1);
735- arbiter.compositor_release(b2);
736+ b2.reset();
737 auto b3 = arbiter.compositor_acquire(&comp_id1);
738 auto b5 = arbiter.compositor_acquire(&comp_id2);
739- arbiter.compositor_release(b3);
740+ b3.reset();
741 auto b4 = arbiter.compositor_acquire(&comp_id1);
742- arbiter.compositor_release(b5);
743- arbiter.compositor_release(b4);
744+ b5.reset();
745+ b4.reset();
746 auto b6 = arbiter.compositor_acquire(&comp_id1);
747- arbiter.compositor_release(b6);
748-
749- b1.reset();
750- b2.reset();
751- b3.reset();
752- b4.reset();
753- b5.reset();
754 b6.reset();
755
756 EXPECT_THAT(buffer_released, Each(Pointee(true)));
757@@ -441,13 +453,12 @@
758 auto b1 = arbiter.compositor_acquire(&comp_id1);
759 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id1));
760 EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
761- arbiter.compositor_release(b1);
762+ b1.reset();
763
764 auto b2 = arbiter.compositor_acquire(&comp_id2);
765 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id1));
766 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id2));
767- arbiter.compositor_release(b2);
768-}
769+}
770
771 TEST_F(MultiMonitorArbiter, other_compositor_ready_status_advances_with_fastest_compositor)
772 {
773@@ -458,23 +469,19 @@
774 EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id1));
775 EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
776
777- auto b1 = arbiter.compositor_acquire(&comp_id1);
778- arbiter.compositor_release(b1);
779- EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id1));
780- EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
781-
782- b1 = arbiter.compositor_acquire(&comp_id1);
783- arbiter.compositor_release(b1);
784- EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id1));
785- EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
786-
787- b1 = arbiter.compositor_acquire(&comp_id1);
788- arbiter.compositor_release(b1);
789+ arbiter.compositor_acquire(&comp_id1);
790+ EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id1));
791+ EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
792+
793+ arbiter.compositor_acquire(&comp_id1);
794+ EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id1));
795+ EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
796+
797+ arbiter.compositor_acquire(&comp_id1);
798 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id1));
799 EXPECT_TRUE(arbiter.buffer_ready_for(&comp_id2));
800
801- b1 = arbiter.compositor_acquire(&comp_id2);
802- arbiter.compositor_release(b1);
803+ arbiter.compositor_acquire(&comp_id2);
804 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id1));
805 EXPECT_FALSE(arbiter.buffer_ready_for(&comp_id2));
806 }
807@@ -497,8 +504,6 @@
808 auto b2 = arbiter.compositor_acquire(&comp_id1);
809 EXPECT_THAT(b1, IsSameBufferAs(buffers[0]));
810 EXPECT_THAT(b2, IsSameBufferAs(buffers[1]));
811- arbiter.compositor_release(b1);
812- arbiter.compositor_release(b2);
813 b1.reset();
814 b2.reset();
815
816@@ -512,20 +517,26 @@
817 schedule.set_schedule({buffers[0], buffers[1], buffers[0], buffers[1]});
818
819 auto b1 = arbiter.compositor_acquire(&comp_id1);
820+ auto id1 = b1->id();
821 auto b2 = arbiter.compositor_acquire(&comp_id1);
822- arbiter.compositor_release(b1);
823+ auto id2 = b2->id();
824+
825+ b1.reset();
826
827 auto b3 = arbiter.compositor_acquire(&comp_id2);
828+ auto id3 = b3->id();
829 auto b4 = arbiter.compositor_acquire(&comp_id2);
830- arbiter.compositor_release(b3);
831-
832- arbiter.compositor_release(b2);
833- arbiter.compositor_release(b4);
834-
835- EXPECT_THAT(b1, IsSameBufferAs(buffers[0]));
836- EXPECT_THAT(b2, IsSameBufferAs(buffers[1]));
837- EXPECT_THAT(b3, IsSameBufferAs(buffers[1]));
838- EXPECT_THAT(b4, IsSameBufferAs(buffers[0]));
839+ auto id4 = b4->id();
840+
841+ b3.reset();
842+
843+ b2.reset();
844+ b4.reset();
845+
846+ EXPECT_THAT(id1, Eq(buffers[0]->id()));
847+ EXPECT_THAT(id2, Eq(buffers[1]->id()));
848+ EXPECT_THAT(id3, Eq(buffers[1]->id()));
849+ EXPECT_THAT(id4, Eq(buffers[0]->id()));
850
851 }
852
853@@ -540,22 +551,25 @@
854 buffers[0], buffers[1], buffers[2]});
855
856 auto b1 = arbiter.compositor_acquire(&comp_id1);
857+ auto id1 = b1->id();
858 auto b2 = arbiter.compositor_acquire(&comp_id2);
859- arbiter.compositor_release(b1); //send nothing
860+ auto id2 = b2->id();
861+ b1.reset(); // Send nothing
862
863 auto b3 = arbiter.compositor_acquire(&comp_id1);
864- arbiter.compositor_release(b3); //send nothing
865+ auto id3 = b3->id();
866+ b3.reset(); // Send nothing
867
868 auto b4 = arbiter.compositor_acquire(&comp_id2);
869- arbiter.compositor_release(b2); //send 0
870+ auto id4 = b4->id();
871+ b2.reset(); // Send 0
872
873 auto b5 = arbiter.compositor_acquire(&comp_id1);
874- arbiter.compositor_release(b5); //send nothing
875
876- EXPECT_THAT(b1, IsSameBufferAs(buffers[0]));
877- EXPECT_THAT(b2, IsSameBufferAs(buffers[0]));
878- EXPECT_THAT(b3, IsSameBufferAs(buffers[1]));
879- EXPECT_THAT(b4, IsSameBufferAs(buffers[1]));
880+ EXPECT_THAT(id1, Eq(buffers[0]->id()));
881+ EXPECT_THAT(id2, Eq(buffers[0]->id()));
882+ EXPECT_THAT(id3, Eq(buffers[1]->id()));
883+ EXPECT_THAT(id4, Eq(buffers[1]->id()));
884 EXPECT_THAT(b5, IsSameBufferAs(buffers[2]));
885 }
886
887
888=== removed file 'tests/unit-tests/compositor/test_temporary_buffers.cpp'
889--- tests/unit-tests/compositor/test_temporary_buffers.cpp 2017-07-28 17:00:43 +0000
890+++ tests/unit-tests/compositor/test_temporary_buffers.cpp 1970-01-01 00:00:00 +0000
891@@ -1,143 +0,0 @@
892-/*
893- * Copyright © 2012 Canonical Ltd.
894- *
895- * This program is free software: you can redistribute it and/or modify
896- * it under the terms of the GNU General Public License version 2 or 3 as
897- * published by the Free Software Foundation.
898- *
899- * This program is distributed in the hope that it will be useful,
900- * but WITHOUT ANY WARRANTY; without even the implied warranty of
901- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
902- * GNU General Public License for more details.
903- *
904- * You should have received a copy of the GNU General Public License
905- * along with this program. If not, see <http://www.gnu.org/licenses/>.
906- *
907- * Authored by: Kevin DuBois <kevin.dubois@canonical.com>
908- */
909-
910-#include "src/server/compositor/temporary_buffers.h"
911-#include "src/server/compositor/buffer_acquisition.h"
912-#include "mir/test/doubles/mock_buffer.h"
913-#include "mir/test/doubles/stub_buffer.h"
914-#include <gtest/gtest.h>
915-#include <stdexcept>
916-
917-namespace mtd=mir::test::doubles;
918-namespace mg = mir::graphics;
919-namespace mc=mir::compositor;
920-namespace geom=mir::geometry;
921-
922-namespace
923-{
924-class TemporaryTestBuffer : public mc::TemporaryBuffer
925-{
926-public:
927- TemporaryTestBuffer(const std::shared_ptr<mg::Buffer>& buf)
928- : TemporaryBuffer(buf)
929- {
930- }
931-};
932-
933-struct MockBufferAcquisition : mc::BufferAcquisition
934-{
935- MOCK_METHOD1(compositor_acquire, std::shared_ptr<mg::Buffer>(void const*));
936- MOCK_METHOD1(compositor_release, void(std::shared_ptr<mg::Buffer> const&));
937- MOCK_METHOD0(snapshot_acquire, std::shared_ptr<mg::Buffer>());
938- MOCK_METHOD1(snapshot_release, void(std::shared_ptr<mg::Buffer> const&));
939-};
940-
941-class TemporaryBuffersTest : public ::testing::Test
942-{
943-public:
944- TemporaryBuffersTest()
945- : buffer_size{1024, 768},
946- buffer_stride{1024},
947- buffer_pixel_format{mir_pixel_format_abgr_8888},
948- mock_buffer{std::make_shared<testing::NiceMock<mtd::MockBuffer>>(
949- buffer_size, buffer_stride, buffer_pixel_format)},
950- mock_acquisition{std::make_shared<testing::NiceMock<MockBufferAcquisition>>()}
951- {
952- using namespace testing;
953- ON_CALL(*mock_acquisition, compositor_acquire(_))
954- .WillByDefault(Return(mock_buffer));
955- }
956-
957- geom::Size const buffer_size;
958- geom::Stride const buffer_stride;
959- MirPixelFormat const buffer_pixel_format;
960- std::shared_ptr<mtd::MockBuffer> const mock_buffer;
961- std::shared_ptr<MockBufferAcquisition> mock_acquisition;
962-};
963-}
964-
965-TEST_F(TemporaryBuffersTest, compositor_buffer_acquires_and_releases)
966-{
967- using namespace testing;
968- EXPECT_CALL(*mock_acquisition, compositor_acquire(_))
969- .WillOnce(Return(mock_buffer));
970- EXPECT_CALL(*mock_acquisition, compositor_release(_))
971- .Times(1);
972-
973- mc::TemporaryCompositorBuffer proxy_buffer(mock_acquisition, 0);
974-}
975-
976-TEST_F(TemporaryBuffersTest, snapshot_buffer_acquires_and_releases)
977-{
978- using namespace testing;
979- EXPECT_CALL(*mock_acquisition, snapshot_acquire())
980- .WillOnce(Return(mock_buffer));
981- EXPECT_CALL(*mock_acquisition, snapshot_release(_))
982- .Times(1);
983-
984- mc::TemporarySnapshotBuffer proxy_buffer(mock_acquisition);
985-}
986-
987-TEST_F(TemporaryBuffersTest, base_test_size)
988-{
989- TemporaryTestBuffer proxy_buffer(mock_buffer);
990- EXPECT_CALL(*mock_buffer, size())
991- .Times(1);
992-
993- geom::Size size;
994- size = proxy_buffer.size();
995- EXPECT_EQ(buffer_size, size);
996-}
997-
998-TEST_F(TemporaryBuffersTest, base_test_pixel_format)
999-{
1000- TemporaryTestBuffer proxy_buffer(mock_buffer);
1001- EXPECT_CALL(*mock_buffer, pixel_format())
1002- .Times(1);
1003-
1004- MirPixelFormat pixel_format;
1005- pixel_format = proxy_buffer.pixel_format();
1006- EXPECT_EQ(buffer_pixel_format, pixel_format);
1007-}
1008-
1009-TEST_F(TemporaryBuffersTest, base_test_id)
1010-{
1011- TemporaryTestBuffer proxy_buffer(mock_buffer);
1012- EXPECT_CALL(*mock_buffer, id())
1013- .Times(1);
1014-
1015- proxy_buffer.id();
1016-}
1017-
1018-TEST_F(TemporaryBuffersTest, base_test_native_buffer_handle)
1019-{
1020- TemporaryTestBuffer proxy_buffer(mock_buffer);
1021- EXPECT_CALL(*mock_buffer, native_buffer_handle())
1022- .Times(1);
1023-
1024- proxy_buffer.native_buffer_handle();
1025-}
1026-
1027-TEST_F(TemporaryBuffersTest, forwards_native_buffer_base_to_wrapped_buffer)
1028-{
1029- TemporaryTestBuffer proxy_buffer(mock_buffer);
1030- EXPECT_CALL(*mock_buffer, native_buffer_base())
1031- .Times(1);
1032-
1033- proxy_buffer.native_buffer_base();
1034-}

Subscribers

People subscribed via source and target branches