Mir

Merge lp:~vanvugt/mir/fix-1476201 into lp:mir

Proposed by Daniel van Vugt
Status: Merged
Approved by: Daniel van Vugt
Approved revision: 2863
Merged at revision: 3254
Proposed branch: lp:~vanvugt/mir/fix-1476201
Merge into: lp:mir
Prerequisite: lp:~vanvugt/mir/better-scaling-test-2
Diff against target: 245 lines (+142/-9)
3 files modified
src/server/compositor/buffer_queue.cpp (+24/-4)
src/server/compositor/buffer_queue.h (+1/-0)
tests/unit-tests/compositor/test_buffer_queue.cpp (+117/-5)
To merge this branch: bzr merge lp:~vanvugt/mir/fix-1476201
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Mir CI Bot continuous-integration Approve
Alan Griffiths Approve
Mir development team Pending
Review via email: mp+269849@code.launchpad.net

Commit message

Enhance BufferQueue::buffers_ready_for_compositor() again, this time
to support the usage pattern found in QtMir so that it too can have
working detection of slow clients (to help us decide when to scale
to triple instead of double buffers). (LP: #1476201)

We could have changed QtMir instead, but I decided it's a perfectly
valid usage pattern and we should support both, making Mir more
robust in its ability to support third-party compositors.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

What are ghost frames?

review: Needs Information
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

At least two of those sizeable comments in the code explain it. If you can think of a better noun, please suggest...

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:2838
https://mir-jenkins.ubuntu.com/job/mir-ci/21/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/21/console

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

review: Approve (continuous-integration)
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

This one has been around for a while. Alan, are you happy with the name "ghost frames" or did you have a suggestion?

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

> This one has been around for a while. Alan, are you happy with the name "ghost
> frames" or did you have a suggestion?

I don't know what "ghost frames" are, so I have no suggestion. The comments leave me no wiser (and, if possible, I'd prefer the code to be readable without parsing "large comments").

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

"Ghost frame" terminology now removed.

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

PASSED: Continuous integration, rev:2857
http://jenkins.qa.ubuntu.com/job/mir-ci/5982/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5491
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4398
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5447
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/260
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/306
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/306/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/306
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/306/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5444
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5444/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7929
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26565
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/256
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/256/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/114
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26568

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

review: Approve (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

It would be nice to have some automated performance testing for the scenario this is intended to support. Especially as this optimization is likely to be bypassed by the "new buffer semantics" work in progress elsewhere.

review: Approve
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

The automated performance testing (other than the unit tests below) is ready and waiting. The ClientLatency acceptance tests will start failing (unless updated) when/if dynamic queue scaling is re-enabled, as is documented in the ClientLatency test.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:2857
https://mir-jenkins.ubuntu.com/job/mir-ci/36/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/36/console

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

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

Possibly not even related to this branch, given queue scaling is still disabled:

12: /tmp/buildd/mir-0.19.0bzr2858pkg0xenial158/tests/acceptance-tests/test_latency.cpp:245: Failure
12: Value of: observed_latency
12: Expected: is < 3.4
12: Actual: 3.40404 (of type float)

Try again.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:2859
https://mir-jenkins.ubuntu.com/job/mir-ci/49/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/49/console

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

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

PASSED: Continuous integration, rev:2859
http://jenkins.qa.ubuntu.com/job/mir-ci/6014/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5530
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4437
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5486
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/275
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/338
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/338/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/338
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/338/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5483
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5483/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7954
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26638
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/271
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/271/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/129
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26640

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

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

PASSED: Continuous integration, rev:2860
http://jenkins.qa.ubuntu.com/job/mir-ci/6043/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5565
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4472
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5521
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/292/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/367
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/367/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/367
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/367/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5518
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5518/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/7981
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26739
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/288
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/288/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/145/console
    None: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26751/console

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Continuous integration, rev:2860
https://mir-jenkins.ubuntu.com/job/mir-ci/81/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/81/console

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

review: Needs Fixing (continuous-integration)
lp:~vanvugt/mir/fix-1476201 updated
2861. By Daniel van Vugt

Merge latest trunk

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:2861
https://mir-jenkins.ubuntu.com/job/mir-ci/82/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/82/console

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

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
lp:~vanvugt/mir/fix-1476201 updated
2862. By Daniel van Vugt

Merge latest trunk

2863. By Daniel van Vugt

No change, just ping Jenkins.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:2863
https://mir-jenkins.ubuntu.com/job/mir-ci/90/
Executed test runs:
    None: https://mir-jenkins.ubuntu.com/job/generic-update-mp/91/console

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

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

PASSED: Continuous integration, rev:2861
http://jenkins.qa.ubuntu.com/job/mir-ci/6058/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5589
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4496
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5545
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/299
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/382
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/382/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/382
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/382/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5542
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5542/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8001
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26787
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/295
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/295/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/151
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26797

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

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

PASSED: Continuous integration, rev:2863
http://jenkins.qa.ubuntu.com/job/mir-ci/6065/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/5597
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-vivid-amd64-build/4504
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/5553
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-xenial-touch/303
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/389
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-amd64-ci/389/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/389
        deb: http://jenkins.qa.ubuntu.com/job/mir-xenial-i386-ci/389/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5550
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/5550/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-touch/8006
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26807
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/299
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-xenial-armhf/299/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-xenial-touch/155
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/26811

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

review: Approve (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 'src/server/compositor/buffer_queue.cpp'
2--- src/server/compositor/buffer_queue.cpp 2015-10-20 03:30:00 +0000
3+++ src/server/compositor/buffer_queue.cpp 2016-01-19 21:00:22 +0000
4@@ -139,6 +139,7 @@
5 : nbuffers{nbuffers},
6 frame_deadlines_threshold{-1}, // Disable scaling by default
7 frame_deadlines_met{0},
8+ scheduled_extra_frames{0},
9 frame_dropping_enabled{false},
10 current_compositor_buffer_valid{false},
11 the_properties{props},
12@@ -266,6 +267,14 @@
13
14 auto const buffer = pop(buffers_owned_by_client);
15 ready_to_composite_queue.push_back(buffer);
16+
17+ /*
18+ * Timerless client performance detection:
19+ * Over-schedule the compositor so that it can detect if the client
20+ * is falling behind. Otherwise the compositor itself goes to sleep
21+ * and wouldn't be able to tell a slow client from a fast one.
22+ */
23+ scheduled_extra_frames = frame_deadlines_threshold - 1;
24 }
25
26 std::shared_ptr<mg::Buffer>
27@@ -276,6 +285,15 @@
28 bool use_current_buffer = false;
29 if (is_a_current_buffer_user(user_id)) // Primary/fastest display
30 {
31+ /*
32+ * Yes I know different compositor user_ids will get different
33+ * results with this but that's OK, and actually more efficient.
34+ * We only need to overschedule one display at most for the
35+ * slow client detection to work.
36+ */
37+ if (scheduled_extra_frames > 0)
38+ --scheduled_extra_frames;
39+
40 if (ready_to_composite_queue.empty())
41 frame_deadlines_met = 0;
42 else if (frame_deadlines_met < frame_deadlines_threshold)
43@@ -454,21 +472,21 @@
44 }
45
46 /*
47- * Intentionally schedule one more frame than we need, and for good
48+ * Intentionally schedule more frames than we need, and for good
49 * reason... We can only accurately detect frame_deadlines_met in
50 * compositor_acquire if compositor_acquire is still waking up at full
51 * frame rate even with a slow client. This is crucial to scaling the
52 * queue performance dynamically in "client_ahead_of_compositor".
53 * But don't be concerned; very little is lost by over-scheduling. Under
54 * normal smooth rendering conditions all frames are used (not wasted).
55- * And under sluggish client rendering conditions the extra frame has a
56+ * And under sluggish client rendering conditions the extra frames have a
57 * critical role in providing a sample point in which we detect if the
58 * client is keeping up. Only when the compositor changes from active to
59 * idle is the extra frame wasted. Sounds like a reasonable price to pay
60 * for dynamic performance monitoring.
61 */
62- if (count && frame_deadlines_threshold >= 0)
63- ++count;
64+ if (frame_deadlines_threshold >= 0 && scheduled_extra_frames > 0)
65+ count += scheduled_extra_frames;
66
67 return count;
68 }
69@@ -660,6 +678,8 @@
70 std::unique_lock<decltype(guard)> lock{guard};
71 release(buffer, std::move(lock));
72 }
73+
74+ scheduled_extra_frames = 0;
75 }
76
77 void mc::BufferQueue::drop_client_requests()
78
79=== modified file 'src/server/compositor/buffer_queue.h'
80--- src/server/compositor/buffer_queue.h 2015-09-17 20:32:32 +0000
81+++ src/server/compositor/buffer_queue.h 2016-01-19 21:00:22 +0000
82@@ -109,6 +109,7 @@
83 int nbuffers;
84 int frame_deadlines_threshold;
85 int frame_deadlines_met;
86+ int scheduled_extra_frames;
87 bool frame_dropping_enabled;
88 bool current_compositor_buffer_valid;
89 graphics::BufferProperties the_properties;
90
91=== modified file 'tests/unit-tests/compositor/test_buffer_queue.cpp'
92--- tests/unit-tests/compositor/test_buffer_queue.cpp 2015-09-17 20:32:32 +0000
93+++ tests/unit-tests/compositor/test_buffer_queue.cpp 2016-01-19 21:00:22 +0000
94@@ -1405,6 +1405,33 @@
95 q.compositor_release(c);
96 }
97
98+TEST_P(WithTwoOrMoreBuffers, buffers_ready_count_tapers_off)
99+{ // Another test related to QtMir's style of doing things (LP: #1476201)
100+ mc::BufferQueue q{nbuffers, allocator, basic_properties, policy_factory};
101+
102+ q.set_scaling_delay(3);
103+
104+ ASSERT_THAT(q.buffers_ready_for_compositor(this), Eq(0));
105+
106+ // Produce one frame
107+ q.client_release(client_acquire_sync(q));
108+
109+ // Ensure we're told multiple frames are ready to facilitate the
110+ // compositor detecting a slow client that misses the second one.
111+ ASSERT_THAT(q.buffers_ready_for_compositor(this), Ge(2));
112+
113+ // Consume one frame
114+ q.compositor_release(q.compositor_acquire(this));
115+
116+ // Finally verify the count is tapering off instead of dropping off
117+ ASSERT_THAT(q.buffers_ready_for_compositor(this), Ge(1));
118+
119+ for (int flush = 0; flush < q.scaling_delay(); ++flush)
120+ q.compositor_release(q.compositor_acquire(this));
121+
122+ ASSERT_THAT(q.buffers_ready_for_compositor(this), Eq(0));
123+}
124+
125 TEST_P(WithTwoOrMoreBuffers, buffers_ready_eventually_reaches_zero)
126 {
127 mc::BufferQueue q{nbuffers, allocator, basic_properties, policy_factory};
128@@ -1425,11 +1452,12 @@
129 {
130 ASSERT_NE(0, q.buffers_ready_for_compositor(&monitor[m]));
131
132- // Double consume to account for the +1 that
133+ // Extra consume to account for the additional frames that
134 // buffers_ready_for_compositor adds to do dynamic performance
135 // detection.
136- q.compositor_release(q.compositor_acquire(&monitor[m]));
137- q.compositor_release(q.compositor_acquire(&monitor[m]));
138+ int const nflush = (q.scaling_delay() > 0) ? q.scaling_delay() : 1;
139+ for (int flush = 0; flush < nflush; ++flush)
140+ q.compositor_release(q.compositor_acquire(&monitor[m]));
141
142 ASSERT_EQ(0, q.buffers_ready_for_compositor(&monitor[m]));
143 }
144@@ -1711,6 +1739,91 @@
145 EXPECT_THAT(buffers_acquired.size(), Eq(2));
146 }
147
148+TEST_P(WithThreeOrMoreBuffers, queue_size_scales_up_without_accumulator)
149+{ // A regression test similar to above but designed to mimic QtMir
150+ // for LP: #1476201.
151+
152+ q.allow_framedropping(false);
153+ std::unordered_set<mg::Buffer *> buffers_acquired;
154+
155+ int const delay = 3;
156+ q.set_scaling_delay(delay);
157+
158+ int const nframes = 100;
159+
160+ std::shared_ptr<AcquireWaitHandle> client;
161+
162+ for (int frame = 0; frame < nframes; ++frame)
163+ {
164+ do
165+ {
166+ if (!client)
167+ client = client_acquire_async(q);
168+ if (client->has_acquired_buffer())
169+ {
170+ if (frame > delay)
171+ buffers_acquired.insert(client->buffer());
172+ client->release_buffer();
173+ client.reset();
174+ }
175+ } while (!client);
176+
177+ q.compositor_release(q.compositor_acquire(nullptr));
178+ }
179+ // Expect double-buffers for fast clients
180+ EXPECT_THAT(buffers_acquired.size(), Eq(2));
181+
182+ // Now check what happens if the client becomes slow...
183+ buffers_acquired.clear();
184+ for (int frame = 0; frame < nframes;)
185+ {
186+ do
187+ {
188+ if (!client)
189+ client = client_acquire_async(q);
190+ if (client->has_acquired_buffer())
191+ {
192+ if (frame > delay)
193+ buffers_acquired.insert(client->buffer());
194+ client->release_buffer();
195+ client.reset();
196+ }
197+ } while (!client);
198+
199+ // Mimic QtMir in that it tests buffers ready as a boolean and does
200+ // not keep its own accumulator in the compositor:
201+ while (q.buffers_ready_for_compositor(nullptr))
202+ {
203+ q.compositor_release(q.compositor_acquire(nullptr));
204+ ++frame;
205+ }
206+ }
207+ // Expect at least triple buffers for sluggish clients
208+ EXPECT_THAT(buffers_acquired.size(), Ge(3));
209+
210+ // And what happens if the client becomes fast again?...
211+ buffers_acquired.clear();
212+ for (int frame = 0; frame < nframes; ++frame)
213+ {
214+ do
215+ {
216+ if (!client)
217+ client = client_acquire_async(q);
218+ if (client->has_acquired_buffer())
219+ {
220+ if (frame > delay)
221+ buffers_acquired.insert(client->buffer());
222+ client->release_buffer();
223+ client.reset();
224+ }
225+ } while (!client);
226+
227+ q.compositor_release(q.compositor_acquire(nullptr));
228+ }
229+ // Expect double-buffers for fast clients
230+ EXPECT_THAT(buffers_acquired.size(), Eq(2));
231+}
232+
233 TEST_P(WithThreeOrMoreBuffers, greedy_compositors_need_triple_buffers)
234 {
235 /*
236@@ -1755,8 +1868,7 @@
237 ASSERT_EQ(0, q.buffers_ready_for_compositor(this));
238 q.client_release(client_acquire_sync(q));
239
240- // Detecting a slow client requires scheduling at least one extra
241- // frame...
242+ // Detecting a slow client requires scheduling extra frames
243 int nready = q.buffers_ready_for_compositor(this);
244 ASSERT_THAT(nready, Ge(2));
245 for (int i = 0; i < nready; ++i)

Subscribers

People subscribed via source and target branches