Mir

Merge lp:~alan-griffiths/mir/nested-sessions-dont-post-buffers-until-something-happens into lp:mir

Proposed by Alan Griffiths
Status: Merged
Approved by: Alan Griffiths
Approved revision: no longer in the source branch.
Merged at revision: 1456
Proposed branch: lp:~alan-griffiths/mir/nested-sessions-dont-post-buffers-until-something-happens
Merge into: lp:mir
Prerequisite: lp:~alan-griffiths/mir/add-some-intelligence-to-scheduling-compositing
Diff against target: 344 lines (+135/-23)
5 files modified
examples/render_surfaces.cpp (+30/-2)
src/server/compositor/default_configuration.cpp (+7/-4)
src/server/compositor/multi_threaded_compositor.cpp (+27/-9)
src/server/compositor/multi_threaded_compositor.h (+5/-1)
tests/unit-tests/compositor/test_multi_threaded_compositor.cpp (+66/-7)
To merge this branch: bzr merge lp:~alan-griffiths/mir/nested-sessions-dont-post-buffers-until-something-happens
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Alexandros Frantzis (community) Approve
Alberto Aguirre (community) Approve
Review via email: mp+209107@code.launchpad.net

Commit message

compositor: Don't automatically paint the framebuffer when compositing starts. (This is really only appropriate for a host session.)

Description of the change

compositor: Don't automatically paint the framebuffer when compositing starts. (This is really only appropriate for a host session.)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

299 + compositor.start();
300 + std::this_thread::sleep_for(std::chrono::milliseconds(100));

I think the time limit is too small for our whimsical CI. Perhaps a loop checking the condition every 20ms for 5 seconds? (5s is probably too much, but it doesn't matter since we will pay the price only in case of a failure, which should hopefully be never).

302 + // Verify we're still at zero frames
303 + EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update));

Wrong comment?

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

Looks good.

review: Approve
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

OK.

review: Approve
Revision history for this message
kevin gunn (kgunn72) wrote :
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

Trivial merge conflicts resolved.

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp 2014-03-04 10:10:23 +0000
+++ examples/render_surfaces.cpp 2014-03-05 17:19:32 +0000
@@ -17,6 +17,7 @@
17 */17 */
1818
19#include "mir/compositor/display_buffer_compositor_factory.h"19#include "mir/compositor/display_buffer_compositor_factory.h"
20#include "mir/server_status_listener.h"
20#include "mir/compositor/display_buffer_compositor.h"21#include "mir/compositor/display_buffer_compositor.h"
21#include "mir/options/default_configuration.h"22#include "mir/options/default_configuration.h"
22#include "mir/graphics/graphic_buffer_allocator.h"23#include "mir/graphics/graphic_buffer_allocator.h"
@@ -86,6 +87,7 @@
8687
87namespace88namespace
88{89{
90std::atomic<bool> created{false};
89bool input_is_on = false;91bool input_is_on = false;
90std::weak_ptr<mg::Cursor> cursor;92std::weak_ptr<mg::Cursor> cursor;
91static const uint32_t bg_color = 0x00000000;93static const uint32_t bg_color = 0x00000000;
@@ -326,6 +328,31 @@
326 }328 }
327 ///\internal [RenderResourcesBufferInitializer_tag]329 ///\internal [RenderResourcesBufferInitializer_tag]
328330
331 // Unless the compositor starts before we create the surfaces it won't respond to
332 // the change notification that causes.
333 std::shared_ptr<mir::ServerStatusListener> the_server_status_listener()
334 {
335 struct ServerStatusListener : mir::ServerStatusListener
336 {
337 ServerStatusListener(std::function<void()> create_surfaces, std::shared_ptr<mir::ServerStatusListener> wrapped) :
338 create_surfaces(create_surfaces), wrapped(wrapped) {}
339
340 virtual void paused() override { wrapped->paused(); }
341 virtual void resumed() override { wrapped->resumed(); }
342 virtual void started() override { wrapped->started(); create_surfaces(); create_surfaces = []{}; }
343
344 std::function<void()> create_surfaces;
345 std::shared_ptr<mir::ServerStatusListener> const wrapped;
346 };
347
348 return server_status_listener(
349 [this]()
350 {
351 auto wrapped = ServerConfiguration::the_server_status_listener();
352 return std::make_shared<ServerStatusListener>([this] { create_surfaces(); }, wrapped);
353 });
354 }
355
329 ///\internal [RenderSurfacesDisplayBufferCompositor_tag]356 ///\internal [RenderSurfacesDisplayBufferCompositor_tag]
330 // Decorate the DefaultDisplayBufferCompositor in order to move surfaces.357 // Decorate the DefaultDisplayBufferCompositor in order to move surfaces.
331 std::shared_ptr<mc::DisplayBufferCompositorFactory> the_display_buffer_compositor_factory() override358 std::shared_ptr<mc::DisplayBufferCompositorFactory> the_display_buffer_compositor_factory() override
@@ -344,6 +371,7 @@
344371
345 bool composite()372 bool composite()
346 {373 {
374 while (!created) std::this_thread::yield();
347 animate_cursor();375 animate_cursor();
348 stop_watch.stop();376 stop_watch.stop();
349 if (stop_watch.elapsed_seconds_since_last_restart() >= 1)377 if (stop_watch.elapsed_seconds_since_last_restart() >= 1)
@@ -460,6 +488,8 @@
460 2.0f * M_PI * cos(i));488 2.0f * M_PI * cos(i));
461 ++i;489 ++i;
462 }490 }
491
492 created = true;
463 }493 }
464494
465 bool input_is_on()495 bool input_is_on()
@@ -493,8 +523,6 @@
493523
494 mir::run_mir(conf, [&](mir::DisplayServer&)524 mir::run_mir(conf, [&](mir::DisplayServer&)
495 {525 {
496 conf.create_surfaces();
497
498 cursor = conf.the_cursor();526 cursor = conf.the_cursor();
499527
500 input_is_on = conf.input_is_on();528 input_is_on = conf.input_is_on();
501529
=== modified file 'src/server/compositor/default_configuration.cpp'
--- src/server/compositor/default_configuration.cpp 2014-02-28 13:51:43 +0000
+++ src/server/compositor/default_configuration.cpp 2014-03-05 17:19:32 +0000
@@ -25,6 +25,7 @@
25#include "compositing_screencast.h"25#include "compositing_screencast.h"
2626
27#include "mir/frontend/screencast.h"27#include "mir/frontend/screencast.h"
28#include "mir/options/configuration.h"
2829
29#include <boost/throw_exception.hpp>30#include <boost/throw_exception.hpp>
3031
@@ -59,10 +60,12 @@
59 return compositor(60 return compositor(
60 [this]()61 [this]()
61 {62 {
62 return std::make_shared<mc::MultiThreadedCompositor>(the_display(),63 return std::make_shared<mc::MultiThreadedCompositor>(
63 the_scene(),64 the_display(),
64 the_display_buffer_compositor_factory(),65 the_scene(),
65 the_compositor_report());66 the_display_buffer_compositor_factory(),
67 the_compositor_report(),
68 !the_options()->is_set(options::host_socket_opt));
66 });69 });
67}70}
6871
6972
=== modified file 'src/server/compositor/multi_threaded_compositor.cpp'
--- src/server/compositor/multi_threaded_compositor.cpp 2014-03-04 12:16:12 +0000
+++ src/server/compositor/multi_threaded_compositor.cpp 2014-03-05 17:19:32 +0000
@@ -147,12 +147,14 @@
147 std::shared_ptr<mg::Display> const& display,147 std::shared_ptr<mg::Display> const& display,
148 std::shared_ptr<mc::Scene> const& scene,148 std::shared_ptr<mc::Scene> const& scene,
149 std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,149 std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,
150 std::shared_ptr<CompositorReport> const& compositor_report)150 std::shared_ptr<CompositorReport> const& compositor_report,
151 bool compose_on_start)
151 : display{display},152 : display{display},
152 scene{scene},153 scene{scene},
153 display_buffer_compositor_factory{db_compositor_factory},154 display_buffer_compositor_factory{db_compositor_factory},
154 report{compositor_report},155 report{compositor_report},
155 started{false}156 started{false},
157 compose_on_start{compose_on_start}
156{158{
157}159}
158160
@@ -161,6 +163,14 @@
161 stop();163 stop();
162}164}
163165
166void mc::MultiThreadedCompositor::schedule_compositing()
167{
168 std::unique_lock<std::mutex> lk(started_guard);
169 report->scheduled();
170 for (auto& f : thread_functors)
171 f->schedule_compositing();
172}
173
164void mc::MultiThreadedCompositor::start()174void mc::MultiThreadedCompositor::start()
165{175{
166 std::unique_lock<std::mutex> lk(started_guard);176 std::unique_lock<std::mutex> lk(started_guard);
@@ -184,16 +194,18 @@
184 /* Recomposite whenever the scene changes */194 /* Recomposite whenever the scene changes */
185 scene->set_change_callback([this]()195 scene->set_change_callback([this]()
186 {196 {
187 report->scheduled();197 schedule_compositing();
188 for (auto& f : thread_functors)
189 f->schedule_compositing();
190 });198 });
191199
192 /* First render */
193 for (auto& f : thread_functors)
194 f->schedule_compositing();
195
196 started = true;200 started = true;
201
202 /* Optional first render */
203 if (compose_on_start)
204 {
205 lk.unlock();
206 schedule_compositing();
207 }
208
197}209}
198210
199void mc::MultiThreadedCompositor::stop()211void mc::MultiThreadedCompositor::stop()
@@ -204,7 +216,9 @@
204 return;216 return;
205 }217 }
206218
219 lk.unlock();
207 scene->set_change_callback([]{});220 scene->set_change_callback([]{});
221 lk.lock();
208222
209 for (auto& f : thread_functors)223 for (auto& f : thread_functors)
210 f->stop();224 f->stop();
@@ -218,4 +232,8 @@
218 report->stopped();232 report->stopped();
219233
220 started = false;234 started = false;
235
236 // If the compositor is restarted we've likely got clients blocked
237 // so we will need to schedule compositing immediately
238 compose_on_start = true;
221}239}
222240
=== modified file 'src/server/compositor/multi_threaded_compositor.h'
--- src/server/compositor/multi_threaded_compositor.h 2014-03-05 07:21:46 +0000
+++ src/server/compositor/multi_threaded_compositor.h 2014-03-05 17:19:32 +0000
@@ -46,7 +46,8 @@
46 MultiThreadedCompositor(std::shared_ptr<graphics::Display> const& display,46 MultiThreadedCompositor(std::shared_ptr<graphics::Display> const& display,
47 std::shared_ptr<Scene> const& scene,47 std::shared_ptr<Scene> const& scene,
48 std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,48 std::shared_ptr<DisplayBufferCompositorFactory> const& db_compositor_factory,
49 std::shared_ptr<CompositorReport> const& compositor_report);49 std::shared_ptr<CompositorReport> const& compositor_report,
50 bool compose_on_start);
50 ~MultiThreadedCompositor();51 ~MultiThreadedCompositor();
5152
52 void start();53 void start();
@@ -63,6 +64,9 @@
6364
64 std::mutex started_guard;65 std::mutex started_guard;
65 bool started;66 bool started;
67 bool compose_on_start;
68
69 void schedule_compositing();
66};70};
6771
68}72}
6973
=== modified file 'tests/unit-tests/compositor/test_multi_threaded_compositor.cpp'
--- tests/unit-tests/compositor/test_multi_threaded_compositor.cpp 2014-03-05 07:21:46 +0000
+++ tests/unit-tests/compositor/test_multi_threaded_compositor.cpp 2014-03-05 17:19:32 +0000
@@ -324,7 +324,7 @@
324 auto display = std::make_shared<StubDisplay>(nbuffers);324 auto display = std::make_shared<StubDisplay>(nbuffers);
325 auto scene = std::make_shared<StubScene>();325 auto scene = std::make_shared<StubScene>();
326 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();326 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
327 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};327 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
328328
329 compositor.start();329 compositor.start();
330330
@@ -348,7 +348,8 @@
348 auto mock_report = std::make_shared<mtd::MockCompositorReport>();348 auto mock_report = std::make_shared<mtd::MockCompositorReport>();
349 mc::MultiThreadedCompositor compositor{display, scene,349 mc::MultiThreadedCompositor compositor{display, scene,
350 db_compositor_factory,350 db_compositor_factory,
351 mock_report};351 mock_report,
352 true};
352353
353 EXPECT_CALL(*mock_report, started())354 EXPECT_CALL(*mock_report, started())
354 .Times(1);355 .Times(1);
@@ -363,7 +364,7 @@
363 EXPECT_CALL(*mock_report, added_display(_,_,_,_,_))364 EXPECT_CALL(*mock_report, added_display(_,_,_,_,_))
364 .Times(1);365 .Times(1);
365 EXPECT_CALL(*mock_report, scheduled())366 EXPECT_CALL(*mock_report, scheduled())
366 .Times(1);367 .Times(2);
367368
368 display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)369 display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
369 {370 {
@@ -398,7 +399,7 @@
398 auto display = std::make_shared<StubDisplay>(nbuffers);399 auto display = std::make_shared<StubDisplay>(nbuffers);
399 auto scene = std::make_shared<StubScene>();400 auto scene = std::make_shared<StubScene>();
400 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();401 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
401 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};402 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
402403
403 // Verify we're actually starting at zero frames404 // Verify we're actually starting at zero frames
404 EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));405 EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
@@ -449,6 +450,64 @@
449 compositor.stop();450 compositor.stop();
450}451}
451452
453TEST(MultiThreadedCompositor, when_no_initial_composite_is_needed_there_is_none)
454{
455 using namespace testing;
456
457 unsigned int const nbuffers = 3;
458
459 auto display = std::make_shared<StubDisplay>(nbuffers);
460 auto scene = std::make_shared<StubScene>();
461 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
462 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
463
464 // Verify we're actually starting at zero frames
465 ASSERT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
466
467 compositor.start();
468 std::this_thread::sleep_for(std::chrono::milliseconds(100));
469
470 // Verify we're still at zero frames
471 EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
472
473 compositor.stop();
474}
475
476TEST(MultiThreadedCompositor, when_no_initial_composite_is_needed_we_still_composite_on_restart)
477{
478 using namespace testing;
479
480 unsigned int const nbuffers = 3;
481
482 auto display = std::make_shared<StubDisplay>(nbuffers);
483 auto scene = std::make_shared<StubScene>();
484 auto db_compositor_factory = std::make_shared<RecordingDisplayBufferCompositorFactory>();
485 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, false};
486
487 compositor.start();
488
489 std::this_thread::sleep_for(std::chrono::milliseconds(100));
490
491 // Verify we're actually starting at zero frames
492 ASSERT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, 0, 0));
493
494 compositor.stop();
495 compositor.start();
496
497 for (int countdown = 100;
498 countdown != 0 &&
499 !db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update);
500 --countdown)
501 {
502 std::this_thread::sleep_for(std::chrono::milliseconds(10));
503 }
504
505 // Verify we composited the expected frame
506 EXPECT_TRUE(db_compositor_factory->check_record_count_for_each_buffer(nbuffers, composites_per_update, composites_per_update));
507
508 compositor.stop();
509}
510
452TEST(MultiThreadedCompositor, surface_update_from_render_doesnt_deadlock)511TEST(MultiThreadedCompositor, surface_update_from_render_doesnt_deadlock)
453{512{
454 using namespace testing;513 using namespace testing;
@@ -458,7 +517,7 @@
458 auto display = std::make_shared<StubDisplay>(nbuffers);517 auto display = std::make_shared<StubDisplay>(nbuffers);
459 auto scene = std::make_shared<StubScene>();518 auto scene = std::make_shared<StubScene>();
460 auto db_compositor_factory = std::make_shared<SurfaceUpdatingDisplayBufferCompositorFactory>(scene);519 auto db_compositor_factory = std::make_shared<SurfaceUpdatingDisplayBufferCompositorFactory>(scene);
461 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};520 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
462521
463 compositor.start();522 compositor.start();
464523
@@ -477,7 +536,7 @@
477 auto display = std::make_shared<StubDisplayWithMockBuffers>(nbuffers);536 auto display = std::make_shared<StubDisplayWithMockBuffers>(nbuffers);
478 auto scene = std::make_shared<StubScene>();537 auto scene = std::make_shared<StubScene>();
479 auto db_compositor_factory = std::make_shared<NullDisplayBufferCompositorFactory>();538 auto db_compositor_factory = std::make_shared<NullDisplayBufferCompositorFactory>();
480 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report};539 mc::MultiThreadedCompositor compositor{display, scene, db_compositor_factory, null_report, true};
481540
482 display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)541 display->for_each_mock_buffer([](mtd::MockDisplayBuffer& mock_buf)
483 {542 {
@@ -505,7 +564,7 @@
505 EXPECT_CALL(*mock_scene, set_change_callback(testing::_))564 EXPECT_CALL(*mock_scene, set_change_callback(testing::_))
506 .Times(2);565 .Times(2);
507566
508 mc::MultiThreadedCompositor compositor{display, mock_scene, db_compositor_factory, mock_report};567 mc::MultiThreadedCompositor compositor{display, mock_scene, db_compositor_factory, mock_report, true};
509568
510 compositor.start();569 compositor.start();
511 compositor.start();570 compositor.start();

Subscribers

People subscribed via source and target branches