Mir

Merge lp:~vanvugt/mir/timerless-multimonitor-frame-sync into lp:~mir-team/mir/trunk

  1. timerless-multimonitor-frame-sync
  2. Merge into trunk
Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: no longer in the source branch.
Merged at revision: 1040
Proposed branch: lp:~vanvugt/mir/timerless-multimonitor-frame-sync
Merge into: lp:~mir-team/mir/trunk
Prerequisite: lp:~vanvugt/mir/bypass
Diff against target: 1180 lines (+176/-173)
33 files modified
include/server/mir/compositor/buffer_stream_surfaces.h (+2/-1)
include/server/mir/compositor/renderer.h (+1/-1)
include/server/mir/surfaces/buffer_stream.h (+2/-1)
include/test/mir_test_doubles/mock_buffer_bundle.h (+1/-1)
include/test/mir_test_doubles/mock_buffer_stream.h (+2/-1)
include/test/mir_test_doubles/mock_surface_renderer.h (+1/-1)
include/test/mir_test_doubles/stub_buffer_stream.h (+1/-1)
src/server/compositor/buffer_bundle.h (+2/-1)
src/server/compositor/buffer_stream_surfaces.cpp (+4/-2)
src/server/compositor/default_display_buffer_compositor.cpp (+28/-3)
src/server/compositor/default_display_buffer_compositor.h (+2/-0)
src/server/compositor/gl_renderer.cpp (+4/-2)
src/server/compositor/gl_renderer.h (+3/-1)
src/server/compositor/switching_bundle.cpp (+5/-7)
src/server/compositor/switching_bundle.h (+3/-6)
src/server/compositor/temporary_buffers.cpp (+2/-2)
src/server/compositor/temporary_buffers.h (+1/-1)
src/server/surfaces/surface.cpp (+1/-1)
tests/acceptance-tests/test_server_shutdown.cpp (+1/-1)
tests/integration-tests/compositor/test_buffer_stream.cpp (+15/-21)
tests/integration-tests/compositor/test_swapping_swappers.cpp (+4/-2)
tests/integration-tests/shell/test_session.cpp (+2/-2)
tests/integration-tests/test_surface_first_frame_sync.cpp (+1/-1)
tests/integration-tests/test_swapinterval.cpp (+1/-1)
tests/mir_test_framework/testing_server_options.cpp (+2/-2)
tests/unit-tests/compositor/test_buffer_stream.cpp (+4/-4)
tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp (+8/-8)
tests/unit-tests/compositor/test_gl_renderer.cpp (+1/-1)
tests/unit-tests/compositor/test_rendering_operator.cpp (+1/-1)
tests/unit-tests/compositor/test_switching_bundle.cpp (+65/-91)
tests/unit-tests/compositor/test_temporary_buffers.cpp (+3/-3)
tests/unit-tests/surfaces/test_surface.cpp (+1/-1)
tests/unit-tests/surfaces/test_surface_stack.cpp (+2/-1)
To merge this branch: bzr merge lp:~vanvugt/mir/timerless-multimonitor-frame-sync
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Robert Ancell Approve
Alan Griffiths Approve
Review via email: mp+182840@code.launchpad.net

This proposal supersedes a proposal from 2013-08-26.

Commit message

Dramatically improved multi-monitor frame synchronization, using a global
frame count equivalent to the highest refresh rate of all monitors. This is
much more reliable than the old logic which was based on timers.

This fixes LP: #1210478

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:948
http://jenkins.qa.ubuntu.com/job/mir-ci/1392/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1865
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1750
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/630
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/630/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Chris Halse Rogers (raof) wrote : Posted in a previous version of this proposal

Mostly looks good to me, only nitpick:

160 + local_frame_count <= 0) // Wrap around, unlikely, but handle it...

I'm not a huge fan of testing unsigned values for <= 0. Maybe make it == 0?

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I prefer to keep "<= 0". It's extra protection against regressions if anyone changes it to signed in future.

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I have what looks like a test setup to reproduce LP: #1216472 now, and it looks like it is not fixed by this :(

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I like the approach, but some small concerns:

159 + if (global_frame_count.load() < local_frame_count ||
160 + local_frame_count <= 0) // Wrap around, unlikely, but handle it...
161 + {
162 + global_frame_count.store(local_frame_count);
163 + }
164 + else
165 + {
166 + local_frame_count = global_frame_count.load();
167 + }

As there is no synchronization between the first load and the store I don't think the logic is mathematically correct. Could we improve on this by using compare_exchange? (Not that I think there's much chance of a problem in practice.)

~~~~

134 +std::atomic_ulong mc::DefaultDisplayBufferCompositor::global_frame_count(0);

There's no reason for this to be "published" in a header: it could be a namespace scope variable (in anonymous namespace) rather than a static class member.

~~~~

I don't have a better name but "frameno" doesn't really indicate what the parameter is for.

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Alan, I was thinking the same. Just haven't got to it yet.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:950
http://jenkins.qa.ubuntu.com/job/mir-ci/1399/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1876
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1761
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/637
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/637/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Alternatively, see:
https://code.launchpad.net/~vanvugt/mir/timerless-multimonitor-frame-sync-without-bypass/+merge/182540
which is not blocked by bypass.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal

PASSED: Continuous integration, rev:952
http://jenkins.qa.ubuntu.com/job/mir-ci/1418/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1908
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/656
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/656/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Review: Needs Fixing

162 + if (global_frame_count < local_frame_count || local_frame_count <= 0)
163 + global_frame_count = local_frame_count;
164 + else
165 + local_frame_count = global_frame_count;
166 + }

Start with:

thread 1: local_frame_count = MAX_UNIT
thread 2: local_frame_count = MAX_UNIT-1

thread 1 runs through this code leaving:

global_frame_count == (t1)local_frame_count == 0

Then thread 2 runs through it leaving:

global_frame_count == (t2)local_frame_count == MAX_UNIT

When the result needed by the algorithm is:

global_frame_count == (t2)local_frame_count == 0

Things do sort themselves out and the error is mostly harmless (and only occurs occasionally) but it could be improved upon. Vis:

162 + if (global_frame_count < local_frame_count || local_frame_count <= 0)
          if (global_frame_count - local_frame_count > UNIT_MAX/2)

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I'm still not entirely happy with the naming - frameno, global_frame_count and local_frame_count are not really what their names suggest. They are neither identifiers nor counts but actually sequence numbers.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

137 +bool wrapped_greater_or_equal(unsigned long a, unsigned long b)

It probably results in the same code, but "inline" would hint at an optimization.

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:955
http://jenkins.qa.ubuntu.com/job/mir-ci/1421/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1912
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1797
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/659
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/659/artifact/work/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Looks good

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/646/
Executed test runs:
    None: http://s-jenkins:8080/job/generic-cleanup-mbs/1924/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1922
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1807
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-autolanding/301/console

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/compositor/buffer_stream_surfaces.h'
2--- include/server/mir/compositor/buffer_stream_surfaces.h 2013-08-09 01:37:53 +0000
3+++ include/server/mir/compositor/buffer_stream_surfaces.h 2013-08-29 09:11:55 +0000
4@@ -41,7 +41,8 @@
5
6 std::shared_ptr<graphics::Buffer> secure_client_buffer();
7
8- std::shared_ptr<graphics::Buffer> lock_compositor_buffer() override;
9+ std::shared_ptr<graphics::Buffer>
10+ lock_compositor_buffer(unsigned long frameno) override;
11 std::shared_ptr<graphics::Buffer> lock_snapshot_buffer() override;
12
13 geometry::PixelFormat get_stream_pixel_format();
14
15=== modified file 'include/server/mir/compositor/renderer.h'
16--- include/server/mir/compositor/renderer.h 2013-08-09 01:37:53 +0000
17+++ include/server/mir/compositor/renderer.h 2013-08-29 09:11:55 +0000
18@@ -37,7 +37,7 @@
19 public:
20 virtual ~Renderer() = default;
21
22- virtual void clear() = 0;
23+ virtual void clear(unsigned long frameno) = 0;
24 virtual void render(std::function<void(std::shared_ptr<void> const&)> save_resource,
25 CompositingCriteria const& info, surfaces::BufferStream& stream) = 0;
26
27
28=== modified file 'include/server/mir/surfaces/buffer_stream.h'
29--- include/server/mir/surfaces/buffer_stream.h 2013-08-09 01:37:53 +0000
30+++ include/server/mir/surfaces/buffer_stream.h 2013-08-29 09:11:55 +0000
31@@ -42,7 +42,8 @@
32 virtual ~BufferStream() {/* TODO: make nothrow */}
33
34 virtual std::shared_ptr<graphics::Buffer> secure_client_buffer() = 0;
35- virtual std::shared_ptr<graphics::Buffer> lock_compositor_buffer() = 0;
36+ virtual std::shared_ptr<graphics::Buffer>
37+ lock_compositor_buffer(unsigned long frameno) = 0;
38 virtual std::shared_ptr<graphics::Buffer> lock_snapshot_buffer() = 0;
39 virtual geometry::PixelFormat get_stream_pixel_format() = 0;
40 virtual geometry::Size stream_size() = 0;
41
42=== modified file 'include/test/mir_test_doubles/mock_buffer_bundle.h'
43--- include/test/mir_test_doubles/mock_buffer_bundle.h 2013-08-09 01:37:53 +0000
44+++ include/test/mir_test_doubles/mock_buffer_bundle.h 2013-08-29 09:11:55 +0000
45@@ -39,7 +39,7 @@
46
47 MOCK_METHOD0(client_acquire, std::shared_ptr<graphics::Buffer>());
48 MOCK_METHOD1(client_release, void(std::shared_ptr<graphics::Buffer> const&));
49- MOCK_METHOD0(compositor_acquire, std::shared_ptr<graphics::Buffer>());
50+ MOCK_METHOD1(compositor_acquire, std::shared_ptr<graphics::Buffer>(unsigned long));
51 MOCK_METHOD1(compositor_release, void(std::shared_ptr<graphics::Buffer> const&));
52 MOCK_METHOD0(snapshot_acquire, std::shared_ptr<graphics::Buffer>());
53 MOCK_METHOD1(snapshot_release, void(std::shared_ptr<graphics::Buffer> const&));
54
55=== modified file 'include/test/mir_test_doubles/mock_buffer_stream.h'
56--- include/test/mir_test_doubles/mock_buffer_stream.h 2013-08-09 01:37:53 +0000
57+++ include/test/mir_test_doubles/mock_buffer_stream.h 2013-08-29 09:11:55 +0000
58@@ -32,7 +32,8 @@
59 struct MockBufferStream : public surfaces::BufferStream
60 {
61 MOCK_METHOD0(secure_client_buffer, std::shared_ptr<graphics::Buffer>());
62- MOCK_METHOD0(lock_compositor_buffer, std::shared_ptr<graphics::Buffer>());
63+ MOCK_METHOD1(lock_compositor_buffer,
64+ std::shared_ptr<graphics::Buffer>(unsigned long));
65 MOCK_METHOD0(lock_snapshot_buffer, std::shared_ptr<graphics::Buffer>());
66
67 MOCK_METHOD0(get_stream_pixel_format, geometry::PixelFormat());
68
69=== modified file 'include/test/mir_test_doubles/mock_surface_renderer.h'
70--- include/test/mir_test_doubles/mock_surface_renderer.h 2013-08-09 01:37:53 +0000
71+++ include/test/mir_test_doubles/mock_surface_renderer.h 2013-08-29 09:11:55 +0000
72@@ -33,7 +33,7 @@
73 {
74 MOCK_METHOD3(render, void(
75 std::function<void(std::shared_ptr<void> const&)>, compositor::CompositingCriteria const&, surfaces::BufferStream&));
76- MOCK_METHOD0(clear, void());
77+ MOCK_METHOD1(clear, void(unsigned long));
78
79 ~MockSurfaceRenderer() noexcept {}
80 };
81
82=== modified file 'include/test/mir_test_doubles/stub_buffer_stream.h'
83--- include/test/mir_test_doubles/stub_buffer_stream.h 2013-08-09 01:37:53 +0000
84+++ include/test/mir_test_doubles/stub_buffer_stream.h 2013-08-29 09:11:55 +0000
85@@ -42,7 +42,7 @@
86 return stub_client_buffer;
87 }
88
89- std::shared_ptr<graphics::Buffer> lock_compositor_buffer()
90+ std::shared_ptr<graphics::Buffer> lock_compositor_buffer(unsigned long)
91 {
92 return stub_compositor_buffer;
93 }
94
95=== modified file 'src/server/compositor/buffer_bundle.h'
96--- src/server/compositor/buffer_bundle.h 2013-08-09 01:37:53 +0000
97+++ src/server/compositor/buffer_bundle.h 2013-08-29 09:11:55 +0000
98@@ -36,7 +36,8 @@
99 virtual ~BufferBundle() noexcept {}
100 virtual std::shared_ptr<graphics::Buffer> client_acquire() = 0;
101 virtual void client_release(std::shared_ptr<graphics::Buffer> const&) = 0;
102- virtual std::shared_ptr<graphics::Buffer> compositor_acquire() = 0;
103+ virtual std::shared_ptr<graphics::Buffer>
104+ compositor_acquire(unsigned long frameno) = 0;
105 virtual void compositor_release(std::shared_ptr<graphics::Buffer> const&) = 0;
106 virtual std::shared_ptr<graphics::Buffer> snapshot_acquire() = 0;
107 virtual void snapshot_release(std::shared_ptr<graphics::Buffer> const&) = 0;
108
109=== modified file 'src/server/compositor/buffer_stream_surfaces.cpp'
110--- src/server/compositor/buffer_stream_surfaces.cpp 2013-08-09 01:37:53 +0000
111+++ src/server/compositor/buffer_stream_surfaces.cpp 2013-08-29 09:11:55 +0000
112@@ -37,9 +37,11 @@
113 force_requests_to_complete();
114 }
115
116-std::shared_ptr<mg::Buffer> mc::BufferStreamSurfaces::lock_compositor_buffer()
117+std::shared_ptr<mg::Buffer> mc::BufferStreamSurfaces::lock_compositor_buffer(
118+ unsigned long frameno)
119 {
120- return std::make_shared<mc::TemporaryCompositorBuffer>(buffer_bundle);
121+ return std::make_shared<mc::TemporaryCompositorBuffer>(
122+ buffer_bundle, frameno);
123 }
124
125 std::shared_ptr<mg::Buffer> mc::BufferStreamSurfaces::lock_snapshot_buffer()
126
127=== modified file 'src/server/compositor/default_display_buffer_compositor.cpp'
128--- src/server/compositor/default_display_buffer_compositor.cpp 2013-08-26 03:21:56 +0000
129+++ src/server/compositor/default_display_buffer_compositor.cpp 2013-08-29 09:11:55 +0000
130@@ -49,6 +49,14 @@
131 mir::geometry::Rectangle const& enclosing_region;
132 };
133
134+std::mutex global_frameno_lock;
135+unsigned long global_frameno = 0;
136+
137+bool wrapped_greater_or_equal(unsigned long a, unsigned long b)
138+{
139+ return (a - b) < (~0UL / 2UL);
140+}
141+
142 }
143
144 mc::DefaultDisplayBufferCompositor::DefaultDisplayBufferCompositor(
145@@ -59,16 +67,32 @@
146 : mc::BasicDisplayBufferCompositor{display_buffer},
147 scene{scene},
148 renderer{renderer},
149- overlay_renderer{overlay_renderer}
150+ overlay_renderer{overlay_renderer},
151+ local_frameno{global_frameno}
152 {
153 }
154
155+
156 void mc::DefaultDisplayBufferCompositor::composite()
157 {
158 static bool got_bypass_env = false;
159 static bool bypass_env = true;
160 bool bypassed = false;
161
162+ /*
163+ * Increment frame counts for each tick of the fastest instance of
164+ * DefaultDisplayBufferCompositor. This means for the fastest refresh
165+ * rate of all attached outputs.
166+ */
167+ local_frameno++;
168+ {
169+ std::lock_guard<std::mutex> lock(global_frameno_lock);
170+ if (wrapped_greater_or_equal(local_frameno, global_frameno))
171+ global_frameno = local_frameno;
172+ else
173+ local_frameno = global_frameno;
174+ }
175+
176 if (!got_bypass_env)
177 {
178 const char *env = getenv("MIR_BYPASS");
179@@ -91,7 +115,8 @@
180 if (filter.fullscreen_on_top())
181 {
182 auto bypass_buf =
183- match.topmost_fullscreen()->lock_compositor_buffer();
184+ match.topmost_fullscreen()->lock_compositor_buffer(
185+ local_frameno);
186
187 if (bypass_buf->can_bypass())
188 {
189@@ -110,7 +135,7 @@
190 mir::geometry::Rectangle const& view_area,
191 std::function<void(std::shared_ptr<void> const&)> save_resource)
192 {
193- renderer->clear();
194+ renderer->clear(local_frameno);
195
196 mc::RenderingOperator applicator(*renderer, save_resource);
197 FilterForVisibleSceneInRegion selector(view_area);
198
199=== modified file 'src/server/compositor/default_display_buffer_compositor.h'
200--- src/server/compositor/default_display_buffer_compositor.h 2013-07-25 04:18:04 +0000
201+++ src/server/compositor/default_display_buffer_compositor.h 2013-08-29 09:11:55 +0000
202@@ -53,6 +53,8 @@
203 std::shared_ptr<Scene> const scene;
204 std::shared_ptr<Renderer> const renderer;
205 std::shared_ptr<OverlayRenderer> const overlay_renderer;
206+
207+ unsigned long local_frameno;
208 };
209
210 }
211
212=== modified file 'src/server/compositor/gl_renderer.cpp'
213--- src/server/compositor/gl_renderer.cpp 2013-08-23 01:44:53 +0000
214+++ src/server/compositor/gl_renderer.cpp 2013-08-29 09:11:55 +0000
215@@ -235,6 +235,7 @@
216 }
217
218 mc::GLRenderer::GLRenderer(geom::Rectangle const& display_area)
219+ : frameno{0}
220 {
221 resources.setup(display_area);
222 }
223@@ -265,7 +266,7 @@
224 /* Use the renderable's texture */
225 glBindTexture(GL_TEXTURE_2D, resources.texture);
226
227- auto region_resource = stream.lock_compositor_buffer();
228+ auto region_resource = stream.lock_compositor_buffer(frameno);
229 region_resource->bind_to_texture();
230 save_resource(region_resource);
231
232@@ -277,8 +278,9 @@
233 glDisableVertexAttribArray(resources.position_attr_loc);
234 }
235
236-void mc::GLRenderer::clear()
237+void mc::GLRenderer::clear(unsigned long frame)
238 {
239+ frameno = frame;
240 glClear(GL_COLOR_BUFFER_BIT);
241 }
242
243
244=== modified file 'src/server/compositor/gl_renderer.h'
245--- src/server/compositor/gl_renderer.h 2013-08-09 01:37:53 +0000
246+++ src/server/compositor/gl_renderer.h 2013-08-29 09:11:55 +0000
247@@ -36,7 +36,7 @@
248 /* From renderer */
249 void render(std::function<void(std::shared_ptr<void> const&)> save_resource,
250 CompositingCriteria const& info, surfaces::BufferStream& stream);
251- void clear();
252+ void clear(unsigned long frameno) override;
253
254 ~GLRenderer() noexcept {}
255
256@@ -60,6 +60,8 @@
257 };
258
259 Resources resources;
260+
261+ unsigned long frameno;
262 };
263
264 }
265
266=== modified file 'src/server/compositor/switching_bundle.cpp'
267--- src/server/compositor/switching_bundle.cpp 2013-08-09 01:37:53 +0000
268+++ src/server/compositor/switching_bundle.cpp 2013-08-29 09:11:55 +0000
269@@ -76,6 +76,7 @@
270 first_ready{0}, nready{0},
271 first_client{0}, nclients{0},
272 snapshot{-1}, nsnapshotters{0},
273+ last_consumed{0},
274 overlapping_compositors{false},
275 framedropping{false}, force_drop{0}
276 {
277@@ -85,8 +86,6 @@
278 "nbuffers betwee 1 and " +
279 std::to_string(MAX_NBUFFERS)));
280 }
281-
282- last_consumed = now() - std::chrono::seconds(1);
283 }
284
285 int mc::SwitchingBundle::nfree() const
286@@ -259,15 +258,14 @@
287 cond.notify_all();
288 }
289
290-std::shared_ptr<mg::Buffer> mc::SwitchingBundle::compositor_acquire()
291+std::shared_ptr<mg::Buffer> mc::SwitchingBundle::compositor_acquire(
292+ unsigned long frameno)
293 {
294 std::unique_lock<std::mutex> lock(guard);
295 int compositor;
296
297- auto t = now();
298-
299 // Multi-monitor acquires close to each other get the same frame:
300- bool same_frame = (t - last_consumed) < std::chrono::milliseconds(10);
301+ bool same_frame = (frameno == last_consumed);
302
303 int avail = nfree();
304 bool can_recycle = ncompositors || avail;
305@@ -296,7 +294,7 @@
306 first_ready = next(first_ready);
307 nready--;
308 ncompositors++;
309- last_consumed = t;
310+ last_consumed = frameno;
311 }
312
313 overlapping_compositors = (ncompositors > 1);
314
315=== modified file 'src/server/compositor/switching_bundle.h'
316--- src/server/compositor/switching_bundle.h 2013-08-09 01:37:53 +0000
317+++ src/server/compositor/switching_bundle.h 2013-08-29 09:11:55 +0000
318@@ -24,7 +24,6 @@
319 #include <condition_variable>
320 #include <mutex>
321 #include <memory>
322-#include <chrono>
323
324 namespace mir
325 {
326@@ -47,7 +46,8 @@
327
328 std::shared_ptr<graphics::Buffer> client_acquire();
329 void client_release(std::shared_ptr<graphics::Buffer> const&);
330- std::shared_ptr<graphics::Buffer> compositor_acquire();
331+ std::shared_ptr<graphics::Buffer>
332+ compositor_acquire(unsigned long frameno) override;
333 void compositor_release(std::shared_ptr<graphics::Buffer> const& released_buffer);
334 std::shared_ptr<graphics::Buffer> snapshot_acquire();
335 void snapshot_release(std::shared_ptr<graphics::Buffer> const& released_buffer);
336@@ -89,10 +89,7 @@
337 std::mutex guard;
338 std::condition_variable cond;
339
340- typedef std::chrono::high_resolution_clock::time_point time_point;
341- static time_point now()
342- { return std::chrono::high_resolution_clock::now(); }
343- time_point last_consumed;
344+ unsigned long last_consumed;
345
346 bool overlapping_compositors;
347
348
349=== modified file 'src/server/compositor/temporary_buffers.cpp'
350--- src/server/compositor/temporary_buffers.cpp 2013-08-13 07:56:50 +0000
351+++ src/server/compositor/temporary_buffers.cpp 2013-08-29 09:11:55 +0000
352@@ -41,8 +41,8 @@
353 }
354
355 mc::TemporaryCompositorBuffer::TemporaryCompositorBuffer(
356- std::shared_ptr<BufferBundle> const& bun)
357- : TemporaryBuffer(bun->compositor_acquire()),
358+ std::shared_ptr<BufferBundle> const& bun, unsigned long frameno)
359+ : TemporaryBuffer(bun->compositor_acquire(frameno)),
360 bundle(bun)
361 {
362 }
363
364=== modified file 'src/server/compositor/temporary_buffers.h'
365--- src/server/compositor/temporary_buffers.h 2013-08-13 07:56:50 +0000
366+++ src/server/compositor/temporary_buffers.h 2013-08-29 09:11:55 +0000
367@@ -62,7 +62,7 @@
368 {
369 public:
370 explicit TemporaryCompositorBuffer(
371- std::shared_ptr<BufferBundle> const& bun);
372+ std::shared_ptr<BufferBundle> const& bun, unsigned long frameno);
373 ~TemporaryCompositorBuffer();
374
375 private:
376
377=== modified file 'src/server/surfaces/surface.cpp'
378--- src/server/surfaces/surface.cpp 2013-08-09 01:37:53 +0000
379+++ src/server/surfaces/surface.cpp 2013-08-29 09:11:55 +0000
380@@ -125,7 +125,7 @@
381
382 std::shared_ptr<mg::Buffer> ms::Surface::compositor_buffer() const
383 {
384- return surface_buffer_stream->lock_compositor_buffer();
385+ return surface_buffer_stream->lock_compositor_buffer(0);
386 }
387
388 std::shared_ptr<mg::Buffer> ms::Surface::snapshot_buffer() const
389
390=== modified file 'tests/acceptance-tests/test_server_shutdown.cpp'
391--- tests/acceptance-tests/test_server_shutdown.cpp 2013-08-26 03:00:33 +0000
392+++ tests/acceptance-tests/test_server_shutdown.cpp 2013-08-29 09:11:55 +0000
393@@ -58,7 +58,7 @@
394 std::this_thread::yield();
395 }
396
397- void clear() {}
398+ void clear(unsigned long) override {}
399 };
400
401
402
403=== modified file 'tests/integration-tests/compositor/test_buffer_stream.cpp'
404--- tests/integration-tests/compositor/test_buffer_stream.cpp 2013-08-09 01:37:53 +0000
405+++ tests/integration-tests/compositor/test_buffer_stream.cpp 2013-08-29 09:11:55 +0000
406@@ -64,11 +64,6 @@
407 mc::BufferStreamSurfaces buffer_stream;
408 };
409
410-void sleep_one_frame()
411-{
412- std::this_thread::sleep_for(std::chrono::milliseconds(16));
413-}
414-
415 }
416
417 TEST_F(BufferStreamTest, gives_same_back_buffer_until_more_available)
418@@ -77,8 +72,8 @@
419 auto client1_id = client1->id();
420 client1.reset();
421
422- auto comp1 = buffer_stream.lock_compositor_buffer();
423- auto comp2 = buffer_stream.lock_compositor_buffer();
424+ auto comp1 = buffer_stream.lock_compositor_buffer(1);
425+ auto comp2 = buffer_stream.lock_compositor_buffer(1);
426
427 EXPECT_EQ(comp1->id(), comp2->id());
428 EXPECT_EQ(comp1->id(), client1_id);
429@@ -86,8 +81,7 @@
430 comp1.reset();
431
432 buffer_stream.secure_client_buffer().reset();
433- sleep_one_frame();
434- auto comp3 = buffer_stream.lock_compositor_buffer();
435+ auto comp3 = buffer_stream.lock_compositor_buffer(2);
436
437 EXPECT_NE(client1_id, comp3->id());
438
439@@ -95,7 +89,7 @@
440 auto comp3_id = comp3->id();
441 comp3.reset();
442
443- auto comp4 = buffer_stream.lock_compositor_buffer();
444+ auto comp4 = buffer_stream.lock_compositor_buffer(2);
445 EXPECT_EQ(comp3_id, comp4->id());
446 }
447
448@@ -104,13 +98,13 @@
449 for (int i = 0; i < nbuffers - 1; i++)
450 buffer_stream.secure_client_buffer().reset();
451
452- auto first_monitor = buffer_stream.lock_compositor_buffer();
453+ auto first_monitor = buffer_stream.lock_compositor_buffer(1);
454 auto first_compositor_id = first_monitor->id();
455 first_monitor.reset();
456
457 for (int m = 0; m < 10; m++)
458 {
459- auto monitor = buffer_stream.lock_compositor_buffer();
460+ auto monitor = buffer_stream.lock_compositor_buffer(1);
461 ASSERT_EQ(first_compositor_id, monitor->id());
462 }
463 }
464@@ -120,12 +114,10 @@
465 if (nbuffers > 1)
466 {
467 buffer_stream.secure_client_buffer().reset();
468- auto comp1 = buffer_stream.lock_compositor_buffer();
469-
470- sleep_one_frame();
471+ auto comp1 = buffer_stream.lock_compositor_buffer(1);
472
473 buffer_stream.secure_client_buffer().reset();
474- auto comp2 = buffer_stream.lock_compositor_buffer();
475+ auto comp2 = buffer_stream.lock_compositor_buffer(2);
476
477 EXPECT_NE(comp1->id(), comp2->id());
478
479@@ -139,14 +131,14 @@
480 buffer_stream.secure_client_buffer().reset();
481 auto client1 = buffer_stream.secure_client_buffer();
482
483- auto comp1 = buffer_stream.lock_compositor_buffer();
484- auto comp2 = buffer_stream.lock_compositor_buffer();
485+ auto comp1 = buffer_stream.lock_compositor_buffer(123);
486+ auto comp2 = buffer_stream.lock_compositor_buffer(123);
487
488 EXPECT_EQ(comp1->id(), comp2->id());
489
490 comp1.reset();
491
492- auto comp3 = buffer_stream.lock_compositor_buffer();
493+ auto comp3 = buffer_stream.lock_compositor_buffer(123);
494
495 EXPECT_EQ(comp2->id(), comp3->id());
496 }
497@@ -167,13 +159,15 @@
498 void compositor_loop(ms::BufferStream &stream,
499 std::atomic<bool> &done)
500 {
501+ unsigned long count = 0;
502+
503 while (!done.load())
504 {
505- auto comp1 = stream.lock_compositor_buffer();
506+ auto comp1 = stream.lock_compositor_buffer(++count);
507 ASSERT_NE(nullptr, comp1);
508
509 // Also stress test getting a second compositor buffer before yielding
510- auto comp2 = stream.lock_compositor_buffer();
511+ auto comp2 = stream.lock_compositor_buffer(count);
512 ASSERT_NE(nullptr, comp2);
513
514 std::this_thread::yield();
515
516=== modified file 'tests/integration-tests/compositor/test_swapping_swappers.cpp'
517--- tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-08-09 01:37:53 +0000
518+++ tests/integration-tests/compositor/test_swapping_swappers.cpp 2013-08-29 09:11:55 +0000
519@@ -72,9 +72,10 @@
520 auto g = std::async(std::launch::async,
521 [this]
522 {
523+ unsigned long count = 0;
524 while(!client_thread_done)
525 {
526- auto b = switching_bundle->compositor_acquire();
527+ auto b = switching_bundle->compositor_acquire(++count);
528 std::this_thread::yield();
529 switching_bundle->compositor_release(b);
530 }
531@@ -116,9 +117,10 @@
532 auto g = std::async(std::launch::async,
533 [this]
534 {
535+ unsigned long count = 0;
536 while(!client_thread_done)
537 {
538- auto b = switching_bundle->compositor_acquire();
539+ auto b = switching_bundle->compositor_acquire(++count);
540 std::this_thread::yield();
541 switching_bundle->compositor_release(b);
542 }
543
544=== modified file 'tests/integration-tests/shell/test_session.cpp'
545--- tests/integration-tests/shell/test_session.cpp 2013-08-12 01:44:15 +0000
546+++ tests/integration-tests/shell/test_session.cpp 2013-08-29 09:11:55 +0000
547@@ -95,11 +95,11 @@
548 {
549 struct StubRenderer : public mc::Renderer
550 {
551- void clear() {}
552+ void clear(unsigned long) override {}
553 void render(std::function<void(std::shared_ptr<void> const&)>,
554 mc::CompositingCriteria const&, mir::surfaces::BufferStream& stream)
555 {
556- stream.lock_compositor_buffer();
557+ stream.lock_compositor_buffer(0);
558 }
559
560 void ensure_no_live_buffers_bound() {}
561
562=== modified file 'tests/integration-tests/test_surface_first_frame_sync.cpp'
563--- tests/integration-tests/test_surface_first_frame_sync.cpp 2013-08-26 03:00:33 +0000
564+++ tests/integration-tests/test_surface_first_frame_sync.cpp 2013-08-29 09:11:55 +0000
565@@ -96,7 +96,7 @@
566 {
567 }
568
569- void clear() {}
570+ void clear(unsigned long) override {}
571
572 void render(std::function<void(std::shared_ptr<void> const&)>, mc::CompositingCriteria const&, ms::BufferStream&)
573 {
574
575=== modified file 'tests/integration-tests/test_swapinterval.cpp'
576--- tests/integration-tests/test_swapinterval.cpp 2013-08-26 03:00:33 +0000
577+++ tests/integration-tests/test_swapinterval.cpp 2013-08-29 09:11:55 +0000
578@@ -57,7 +57,7 @@
579 }
580
581 std::shared_ptr<mg::Buffer> secure_client_buffer() { return std::make_shared<mtd::StubBuffer>(); }
582- std::shared_ptr<mg::Buffer> lock_compositor_buffer() { return std::make_shared<mtd::StubBuffer>(); }
583+ std::shared_ptr<mg::Buffer> lock_compositor_buffer(unsigned long) { return std::make_shared<mtd::StubBuffer>(); }
584 std::shared_ptr<mg::Buffer> lock_snapshot_buffer() { return std::make_shared<mtd::StubBuffer>(); }
585 geom::PixelFormat get_stream_pixel_format() { return geom::PixelFormat::abgr_8888; }
586 geom::Size stream_size() { return geom::Size{}; }
587
588=== modified file 'tests/mir_test_framework/testing_server_options.cpp'
589--- tests/mir_test_framework/testing_server_options.cpp 2013-08-27 10:19:54 +0000
590+++ tests/mir_test_framework/testing_server_options.cpp 2013-08-29 09:11:55 +0000
591@@ -200,10 +200,10 @@
592 mc::CompositingCriteria const&, ms::BufferStream& stream)
593 {
594 // Need to acquire the texture to cycle buffers
595- stream.lock_compositor_buffer();
596+ stream.lock_compositor_buffer(0);
597 }
598
599- void clear() {}
600+ void clear(unsigned long) override {}
601 };
602
603 class StubRendererFactory : public mc::RendererFactory
604
605=== modified file 'tests/unit-tests/compositor/test_buffer_stream.cpp'
606--- tests/unit-tests/compositor/test_buffer_stream.cpp 2013-08-09 01:37:53 +0000
607+++ tests/unit-tests/compositor/test_buffer_stream.cpp 2013-08-29 09:11:55 +0000
608@@ -95,7 +95,7 @@
609 {
610 using namespace testing;
611
612- EXPECT_CALL(*mock_bundle, compositor_acquire())
613+ EXPECT_CALL(*mock_bundle, compositor_acquire(_))
614 .Times(1)
615 .WillOnce(Return(mock_buffer));
616 EXPECT_CALL(*mock_bundle, compositor_release(_))
617@@ -103,14 +103,14 @@
618
619 mc::BufferStreamSurfaces buffer_stream(mock_bundle);
620
621- buffer_stream.lock_compositor_buffer();
622+ buffer_stream.lock_compositor_buffer(0);
623 }
624
625 TEST_F(BufferStreamTest, get_buffer_for_compositor_can_lock)
626 {
627 using namespace testing;
628
629- EXPECT_CALL(*mock_bundle, compositor_acquire())
630+ EXPECT_CALL(*mock_bundle, compositor_acquire(_))
631 .Times(1)
632 .WillOnce(Return(mock_buffer));
633 EXPECT_CALL(*mock_bundle, compositor_release(_))
634@@ -118,7 +118,7 @@
635
636 mc::BufferStreamSurfaces buffer_stream(mock_bundle);
637
638- buffer_stream.lock_compositor_buffer();
639+ buffer_stream.lock_compositor_buffer(0);
640 }
641
642 TEST_F(BufferStreamTest, get_buffer_for_client_releases_resources)
643
644=== modified file 'tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp'
645--- tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2013-08-26 03:00:33 +0000
646+++ tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp 2013-08-29 09:11:55 +0000
647@@ -97,7 +97,7 @@
648 {
649 }
650
651- void clear() { renderer->clear(); }
652+ void clear(unsigned long f) override { renderer->clear(f); }
653 void render(std::function<void(std::shared_ptr<void> const&)> save_resource,
654 mc::CompositingCriteria const& info, mir::surfaces::BufferStream& stream)
655 {
656@@ -254,7 +254,7 @@
657 auto compositor_buffer = std::make_shared<mtd::MockBuffer>();
658 EXPECT_CALL(*compositor_buffer, can_bypass())
659 .WillOnce(Return(true));
660- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
661+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
662 .WillOnce(Return(compositor_buffer));
663
664 mc::DefaultDisplayBufferCompositorFactory factory(
665@@ -303,7 +303,7 @@
666 auto compositor_buffer = std::make_shared<mtd::MockBuffer>();
667 EXPECT_CALL(*compositor_buffer, can_bypass())
668 .Times(0);
669- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
670+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
671 .Times(0);
672
673 mc::DefaultDisplayBufferCompositorFactory factory(
674@@ -352,7 +352,7 @@
675 auto compositor_buffer = std::make_shared<mtd::MockBuffer>();
676 EXPECT_CALL(*compositor_buffer, can_bypass())
677 .Times(0);
678- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
679+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
680 .Times(0);
681
682 mc::DefaultDisplayBufferCompositorFactory factory(
683@@ -399,7 +399,7 @@
684 FakeScene scene(renderable_vec);
685
686 auto compositor_buffer = std::make_shared<mtd::MockBuffer>();
687- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
688+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
689 .WillOnce(Return(compositor_buffer));
690 EXPECT_CALL(*compositor_buffer, can_bypass())
691 .WillRepeatedly(Return(false));
692@@ -450,7 +450,7 @@
693 auto compositor_buffer = std::make_shared<mtd::MockBuffer>();
694 EXPECT_CALL(*compositor_buffer, can_bypass())
695 .Times(0);
696- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
697+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
698 .Times(0);
699
700 mc::DefaultDisplayBufferCompositorFactory factory(
701@@ -476,7 +476,7 @@
702 .Times(0);
703 EXPECT_CALL(renderer_factory.mock_renderer, render(_,Ref(fullscreen),_))
704 .Times(0);
705- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
706+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
707 .WillOnce(Return(compositor_buffer));
708 EXPECT_CALL(*compositor_buffer, can_bypass())
709 .WillOnce(Return(true));
710@@ -494,7 +494,7 @@
711 .Times(1);
712 EXPECT_CALL(renderer_factory.mock_renderer, render(_,Ref(fullscreen),_))
713 .Times(0);
714- EXPECT_CALL(scene.stub_stream, lock_compositor_buffer())
715+ EXPECT_CALL(scene.stub_stream, lock_compositor_buffer(_))
716 .Times(0);
717 EXPECT_CALL(*compositor_buffer, can_bypass())
718 .Times(0);
719
720=== modified file 'tests/unit-tests/compositor/test_gl_renderer.cpp'
721--- tests/unit-tests/compositor/test_gl_renderer.cpp 2013-08-09 01:37:53 +0000
722+++ tests/unit-tests/compositor/test_gl_renderer.cpp 2013-08-29 09:11:55 +0000
723@@ -319,7 +319,7 @@
724
725 EXPECT_CALL(mock_gl, glBindTexture(GL_TEXTURE_2D, stub_texture));
726
727- EXPECT_CALL(stream, lock_compositor_buffer())
728+ EXPECT_CALL(stream, lock_compositor_buffer(_))
729 .Times(1)
730 .WillOnce(Return(mt::fake_shared(gr)));
731 EXPECT_CALL(gr, bind_to_texture());
732
733=== modified file 'tests/unit-tests/compositor/test_rendering_operator.cpp'
734--- tests/unit-tests/compositor/test_rendering_operator.cpp 2013-08-09 01:37:53 +0000
735+++ tests/unit-tests/compositor/test_rendering_operator.cpp 2013-08-29 09:11:55 +0000
736@@ -46,7 +46,7 @@
737 {
738 }
739
740- void clear() {}
741+ void clear(unsigned long) override {}
742
743 void render(std::function<void(std::shared_ptr<void> const&)> save_resource,
744 mc::CompositingCriteria const&, ms::BufferStream&)
745
746=== modified file 'tests/unit-tests/compositor/test_switching_bundle.cpp'
747--- tests/unit-tests/compositor/test_switching_bundle.cpp 2013-08-09 01:37:53 +0000
748+++ tests/unit-tests/compositor/test_switching_bundle.cpp 2013-08-29 09:11:55 +0000
749@@ -102,19 +102,12 @@
750
751 namespace
752 {
753- const int frame_rate = 60;
754- const std::chrono::microseconds frame_time(1000000 / frame_rate);
755-
756- void sleep_one_frame()
757- {
758- std::this_thread::sleep_for(frame_time);
759- }
760-
761 void composite_thread(mc::SwitchingBundle &bundle,
762+ unsigned long &frameno,
763 mg::BufferID &composited)
764 {
765- sleep_one_frame();
766- auto buffer = bundle.compositor_acquire();
767+ frameno++;
768+ auto buffer = bundle.compositor_acquire(frameno);
769 composited = buffer->id();
770 bundle.compositor_release(buffer);
771 }
772@@ -126,6 +119,7 @@
773 {
774 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
775 mg::BufferID prev_id, prev_prev_id;
776+ unsigned long frameno = 0;
777
778 ASSERT_FALSE(bundle.framedropping_allowed());
779
780@@ -137,6 +131,7 @@
781
782 std::thread compositor(composite_thread,
783 std::ref(bundle),
784+ std::ref(frameno),
785 std::ref(composited_id));
786
787 compositor.join();
788@@ -148,7 +143,7 @@
789 prev_prev_id = prev_id;
790 prev_id = composited_id;
791
792- auto second_monitor = bundle.compositor_acquire();
793+ auto second_monitor = bundle.compositor_acquire(frameno);
794 ASSERT_EQ(composited_id, second_monitor->id());
795 bundle.compositor_release(second_monitor);
796 }
797@@ -160,6 +155,7 @@
798 for (int nbuffers = 2; nbuffers <= 5; nbuffers++)
799 {
800 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
801+ unsigned int frameno = 0;
802
803 bundle.allow_framedropping(true);
804 mg::BufferID last_client_id;
805@@ -176,11 +172,12 @@
806 // Flush the pipeline of previously ready buffers
807 for (int k = 0; k < nbuffers-1; k++)
808 {
809- bundle.compositor_release(bundle.compositor_acquire());
810- sleep_one_frame();
811+ frameno++;
812+ bundle.compositor_release(bundle.compositor_acquire(frameno));
813 }
814
815- auto compositor = bundle.compositor_acquire();
816+ frameno++;
817+ auto compositor = bundle.compositor_acquire(frameno);
818 ASSERT_EQ(last_client_id, compositor->id());
819 bundle.compositor_release(compositor);
820 }
821@@ -200,7 +197,7 @@
822 bundle.client_release(client2);
823 client2.reset();
824
825- auto compositor = bundle.compositor_acquire();
826+ auto compositor = bundle.compositor_acquire(1);
827 EXPECT_EQ(client1_id, compositor->id());
828 bundle.compositor_release(compositor);
829 }
830@@ -231,6 +228,7 @@
831 for (int nbuffers = 1; nbuffers <= 5; nbuffers++)
832 {
833 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
834+ unsigned long frameno = 0;
835
836 auto client = bundle.client_acquire();
837 auto client_id = client->id();
838@@ -238,7 +236,8 @@
839
840 for (int monitor = 0; monitor < 10; monitor++)
841 {
842- auto compositor = bundle.compositor_acquire();
843+ frameno++;
844+ auto compositor = bundle.compositor_acquire(frameno);
845 ASSERT_EQ(client_id, compositor->id());
846 bundle.compositor_release(compositor);
847 }
848@@ -250,13 +249,14 @@
849 for (int nbuffers = 1; nbuffers <= 5; nbuffers++)
850 {
851 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
852+ unsigned long frameno = 0;
853 const int N = 100;
854
855 bundle.force_requests_to_complete();
856
857 std::shared_ptr<mg::Buffer> buf[N];
858 for (int i = 0; i < N; i++)
859- buf[i] = bundle.compositor_acquire();
860+ buf[i] = bundle.compositor_acquire(++frameno);
861
862 for (int i = 0; i < N; i++)
863 bundle.compositor_release(buf[i]);
864@@ -268,6 +268,7 @@
865 for (int nbuffers = 1; nbuffers <= 5; nbuffers++)
866 {
867 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
868+ unsigned long frameno = 1;
869
870 mg::BufferID client_id;
871
872@@ -282,12 +283,12 @@
873
874 for (int monitor_id = 0; monitor_id < 10; monitor_id++)
875 {
876- auto compositor = bundle.compositor_acquire();
877+ auto compositor = bundle.compositor_acquire(frameno);
878 ASSERT_EQ(client_id, compositor->id());
879 bundle.compositor_release(compositor);
880 }
881
882- sleep_one_frame();
883+ frameno++;
884 }
885 }
886 }
887@@ -297,6 +298,7 @@
888 for (int nbuffers = 2; nbuffers <= 5; nbuffers++)
889 {
890 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
891+ unsigned long frameno = 0;
892
893 auto client = bundle.client_acquire();
894
895@@ -306,7 +308,7 @@
896 );
897 bundle.client_release(client);
898
899- auto compositor1 = bundle.compositor_acquire();
900+ auto compositor1 = bundle.compositor_acquire(++frameno);
901 bundle.compositor_release(compositor1);
902 EXPECT_THROW(
903 bundle.compositor_release(compositor1),
904@@ -321,15 +323,16 @@
905 for (int nbuffers = 2; nbuffers <= 5; nbuffers++)
906 {
907 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
908+ unsigned long frameno = 1;
909
910 std::shared_ptr<mg::Buffer> compositor[2];
911
912 bundle.client_release(bundle.client_acquire());
913- compositor[0] = bundle.compositor_acquire();
914+ compositor[0] = bundle.compositor_acquire(frameno);
915
916- sleep_one_frame();
917+ frameno++;
918 bundle.client_release(bundle.client_acquire());
919- compositor[1] = bundle.compositor_acquire();
920+ compositor[1] = bundle.compositor_acquire(frameno);
921
922 for (int i = 0; i < 20; i++)
923 {
924@@ -340,8 +343,8 @@
925 int oldest = i & 1;
926 bundle.compositor_release(compositor[oldest]);
927 bundle.client_release(bundle.client_acquire());
928- sleep_one_frame();
929- compositor[oldest] = bundle.compositor_acquire();
930+ frameno++;
931+ compositor[oldest] = bundle.compositor_acquire(frameno);
932 }
933
934 bundle.compositor_release(compositor[0]);
935@@ -355,7 +358,7 @@
936 {
937 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
938
939- auto compositor = bundle.compositor_acquire();
940+ auto compositor = bundle.compositor_acquire(1);
941 auto snapshot = bundle.snapshot_acquire();
942 EXPECT_EQ(snapshot->id(), compositor->id());
943 bundle.compositor_release(compositor);
944@@ -385,7 +388,7 @@
945 {
946 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
947
948- auto compositor = bundle.compositor_acquire();
949+ auto compositor = bundle.compositor_acquire(1);
950
951 EXPECT_THROW(
952 bundle.snapshot_release(compositor),
953@@ -418,9 +421,10 @@
954 void compositor_thread(mc::SwitchingBundle &bundle,
955 std::atomic<bool> &done)
956 {
957+ unsigned long frameno = 0;
958 while (!done)
959 {
960- bundle.compositor_release(bundle.compositor_acquire());
961+ bundle.compositor_release(bundle.compositor_acquire(++frameno));
962 std::this_thread::yield();
963 }
964 }
965@@ -510,6 +514,7 @@
966 for (int nbuffers = 1; nbuffers <= 5; nbuffers++)
967 {
968 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
969+ unsigned long frameno = 0;
970
971 bundle.allow_framedropping(false);
972
973@@ -520,9 +525,9 @@
974
975 for (int frame = 0; frame < nframes; frame++)
976 {
977- sleep_one_frame();
978+ frameno++;
979
980- auto compositor = bundle.compositor_acquire();
981+ auto compositor = bundle.compositor_acquire(frameno);
982 auto compositor_id = compositor->id();
983 bundle.compositor_release(compositor);
984
985@@ -542,15 +547,16 @@
986 for (int nbuffers = 3; nbuffers <= 5; nbuffers++)
987 {
988 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
989+ unsigned long frameno = 1;
990
991 std::shared_ptr<mg::Buffer> compositor[2];
992
993 bundle.client_release(bundle.client_acquire());
994- compositor[0] = bundle.compositor_acquire();
995+ compositor[0] = bundle.compositor_acquire(frameno);
996
997- sleep_one_frame();
998+ frameno++;
999 bundle.client_release(bundle.client_acquire());
1000- compositor[1] = bundle.compositor_acquire();
1001+ compositor[1] = bundle.compositor_acquire(frameno);
1002
1003 for (int i = 0; i < 20; i++)
1004 {
1005@@ -566,8 +572,8 @@
1006 int oldest = i & 1;
1007 bundle.compositor_release(compositor[oldest]);
1008
1009- sleep_one_frame();
1010- compositor[oldest] = bundle.compositor_acquire();
1011+ frameno++;
1012+ compositor[oldest] = bundle.compositor_acquire(frameno);
1013 }
1014
1015 bundle.compositor_release(compositor[0]);
1016@@ -649,85 +655,53 @@
1017 namespace
1018 {
1019 void realtime_compositor_thread(mc::SwitchingBundle &bundle,
1020+ unsigned long frames,
1021 std::atomic<bool> &done)
1022 {
1023- while (!done)
1024+ std::this_thread::sleep_for(std::chrono::milliseconds(1000));
1025+
1026+ for (unsigned long frame = 0; frame < frames; frame++)
1027 {
1028- auto buf = bundle.compositor_acquire();
1029- sleep_one_frame();
1030+ std::this_thread::sleep_for(std::chrono::milliseconds(100));
1031+
1032+ auto buf = bundle.compositor_acquire(frame);
1033 bundle.compositor_release(buf);
1034 }
1035+ done.store(true);
1036 }
1037 }
1038
1039 TEST_F(SwitchingBundleTest, client_framerate_matches_compositor)
1040 {
1041- const int expected =
1042- std::chrono::duration_cast<std::chrono::microseconds>(frame_time)
1043- .count();
1044-
1045- int nframes = 0;
1046- int nhiccups = 0;
1047-
1048 for (int nbuffers = 2; nbuffers <= 3; nbuffers++)
1049 {
1050 mc::SwitchingBundle bundle(nbuffers, allocator, basic_properties);
1051+ unsigned long client_frames = 0;
1052+ const unsigned long compose_frames = 20;
1053
1054 bundle.allow_framedropping(false);
1055
1056- std::atomic<bool> done;
1057- done = false;
1058+ std::atomic<bool> done(false);
1059
1060 std::thread monitor1(realtime_compositor_thread,
1061- std::ref(bundle), std::ref(done));
1062- std::thread monitor2(realtime_compositor_thread,
1063- std::ref(bundle), std::ref(done));
1064- std::thread monitor3(realtime_compositor_thread,
1065- std::ref(bundle), std::ref(done));
1066-
1067- for (int attempt = 0; attempt < 2; attempt++)
1068+ std::ref(bundle),
1069+ compose_frames,
1070+ std::ref(done));
1071+
1072+ bundle.client_release(bundle.client_acquire());
1073+
1074+ while (!done.load())
1075 {
1076- // Initial queue filling is not throttled...
1077- for (int f = 0; f < nbuffers + 1; f++)
1078- bundle.client_release(bundle.client_acquire());
1079-
1080- auto prev = std::chrono::high_resolution_clock::now();
1081-
1082- for (int second = 0; second < 2; second++)
1083- {
1084- for (int frame = 0; frame < frame_rate; frame++)
1085- {
1086- bundle.client_release(bundle.client_acquire());
1087-
1088- auto t = std::chrono::high_resolution_clock::now();
1089- auto d = t - prev;
1090- prev = t;
1091-
1092- int measured_frame_time_usec =
1093- std::chrono::duration_cast<std::chrono::microseconds>(d)
1094- .count();
1095-
1096- nframes++;
1097- if (expected * 0.7f > measured_frame_time_usec ||
1098- expected * 1.3f < measured_frame_time_usec)
1099- {
1100- nhiccups++;
1101- }
1102- }
1103- }
1104-
1105- // Simulate a pause/resume like VT switching
1106- bundle.force_requests_to_complete();
1107+ bundle.client_release(bundle.client_acquire());
1108+ client_frames++;
1109+ std::this_thread::yield();
1110 }
1111
1112- done = true;
1113-
1114 monitor1.join();
1115- monitor2.join();
1116- monitor3.join();
1117
1118- int hiccups_percent = nhiccups * 100 / nframes;
1119- ASSERT_LT(hiccups_percent, 10);
1120+ // Roughly compose_frames == client_frames within 50%
1121+ ASSERT_GT(client_frames, compose_frames / 2);
1122+ ASSERT_LT(client_frames, compose_frames * 3 / 2);
1123 }
1124 }
1125
1126
1127=== modified file 'tests/unit-tests/compositor/test_temporary_buffers.cpp'
1128--- tests/unit-tests/compositor/test_temporary_buffers.cpp 2013-08-09 01:37:53 +0000
1129+++ tests/unit-tests/compositor/test_temporary_buffers.cpp 2013-08-29 09:11:55 +0000
1130@@ -54,7 +54,7 @@
1131
1132 ON_CALL(*mock_bundle, client_acquire())
1133 .WillByDefault(Return(mock_buffer));
1134- ON_CALL(*mock_bundle, compositor_acquire())
1135+ ON_CALL(*mock_bundle, compositor_acquire(_))
1136 .WillByDefault(Return(mock_buffer));
1137 }
1138
1139@@ -92,12 +92,12 @@
1140 TEST_F(TemporaryBuffersTest, compositor_buffer_acquires_and_releases)
1141 {
1142 using namespace testing;
1143- EXPECT_CALL(*mock_bundle, compositor_acquire())
1144+ EXPECT_CALL(*mock_bundle, compositor_acquire(_))
1145 .WillOnce(Return(mock_buffer));
1146 EXPECT_CALL(*mock_bundle, compositor_release(_))
1147 .Times(1);
1148
1149- mc::TemporaryCompositorBuffer proxy_buffer(mock_bundle);
1150+ mc::TemporaryCompositorBuffer proxy_buffer(mock_bundle, 0);
1151 }
1152
1153 TEST_F(TemporaryBuffersTest, snapshot_buffer_acquires_and_releases)
1154
1155=== modified file 'tests/unit-tests/surfaces/test_surface.cpp'
1156--- tests/unit-tests/surfaces/test_surface.cpp 2013-08-09 01:37:53 +0000
1157+++ tests/unit-tests/surfaces/test_surface.cpp 2013-08-29 09:11:55 +0000
1158@@ -308,7 +308,7 @@
1159 ms::Surface surf(mock_basic_state, mock_buffer_stream, std::shared_ptr<mi::InputChannel>());
1160 auto buffer_resource = std::make_shared<mtd::StubBuffer>();
1161
1162- EXPECT_CALL(*mock_buffer_stream, lock_compositor_buffer())
1163+ EXPECT_CALL(*mock_buffer_stream, lock_compositor_buffer(_))
1164 .Times(AtLeast(1))
1165 .WillOnce(Return(buffer_resource));
1166
1167
1168=== modified file 'tests/unit-tests/surfaces/test_surface_stack.cpp'
1169--- tests/unit-tests/surfaces/test_surface_stack.cpp 2013-08-26 03:00:33 +0000
1170+++ tests/unit-tests/surfaces/test_surface_stack.cpp 2013-08-29 09:11:55 +0000
1171@@ -69,7 +69,8 @@
1172 public:
1173 virtual std::shared_ptr<mg::Buffer> client_acquire() { return std::shared_ptr<mg::Buffer>(); }
1174 virtual void client_release(std::shared_ptr<mg::Buffer> const&) {}
1175- virtual std::shared_ptr<mg::Buffer> compositor_acquire(){ return std::shared_ptr<mg::Buffer>(); };
1176+ virtual std::shared_ptr<mg::Buffer> compositor_acquire(unsigned long)
1177+ { return std::shared_ptr<mg::Buffer>(); };
1178 virtual void compositor_release(std::shared_ptr<mg::Buffer> const&){}
1179 virtual std::shared_ptr<mg::Buffer> snapshot_acquire(){ return std::shared_ptr<mg::Buffer>(); };
1180 virtual void snapshot_release(std::shared_ptr<mg::Buffer> const&){}

Subscribers

People subscribed via source and target branches