Merge lp:~raof/mir/further-buffer-plumbing-cleanup into lp:mir
- further-buffer-plumbing-cleanup
- Merge into development-branch
| Status: | Merged |
|---|---|
| Approved by: | Alan Griffiths |
| Approved revision: | no longer in the source branch. |
| 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 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Mir CI Bot | continuous-integration | Approve | |
| Alan Griffiths | Approve | ||
| Brandon Schaefer (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Remove even more manual reference counting of mg::Buffer-s.
Description of the change
| Mir CI Bot (mir-ci-bot) wrote : | # |
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:4220
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Brandon Schaefer (brandontschaefer) wrote : | # |
I dont have the full context on the reason for temp buffers, but this clean up looks very nice.
| Chris Halse Rogers (raof) wrote : | # |
The temp buffers were there to wrap
compositor_
snapshot_
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?
| 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.
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:4221
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Autolanding.
Unapproved changes made after approval.
https:/
Executed test runs:
SUCCESS: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
https:/
Executed test runs:
SUCCESS: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
| Mir CI Bot (mir-ci-bot) : | # |
Preview Diff
| 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 | -} |
FAILED: Continuous integration, rev:4219
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/