diff --git a/src/include/server/mir/compositor/display_buffer_compositor.h b/src/include/server/mir/compositor/display_buffer_compositor.h index 670751f40d5..a66a8d5a1a9 100644 --- a/src/include/server/mir/compositor/display_buffer_compositor.h +++ b/src/include/server/mir/compositor/display_buffer_compositor.h @@ -29,7 +29,8 @@ class DisplayBufferCompositor public: virtual ~DisplayBufferCompositor() = default; - virtual void composite(SceneElementSequence&& scene_sequence) = 0; + /// Returns true if any compositing happened, otherwise false. + virtual bool composite(SceneElementSequence&& scene_sequence) = 0; protected: DisplayBufferCompositor() = default; diff --git a/src/server/compositor/default_display_buffer_compositor.cpp b/src/server/compositor/default_display_buffer_compositor.cpp index 6d0cb241b52..b36b70bcee2 100644 --- a/src/server/compositor/default_display_buffer_compositor.cpp +++ b/src/server/compositor/default_display_buffer_compositor.cpp @@ -42,8 +42,12 @@ mc::DefaultDisplayBufferCompositor::DefaultDisplayBufferCompositor( { } -void mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& scene_elements) +bool mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& scene_elements) { + if (scene_elements.size() == 0 && !completed_first_render) + return false; + + completed_first_render = true; report->began_frame(this); auto const& view_area = display_buffer.view_area(); @@ -134,4 +138,5 @@ void mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& sc } report->finished_frame(this); + return true; } diff --git a/src/server/compositor/default_display_buffer_compositor.h b/src/server/compositor/default_display_buffer_compositor.h index 4bf13a3ca3b..2dc4fe21b37 100644 --- a/src/server/compositor/default_display_buffer_compositor.h +++ b/src/server/compositor/default_display_buffer_compositor.h @@ -49,13 +49,14 @@ class DefaultDisplayBufferCompositor : public DisplayBufferCompositor std::shared_ptr const& renderer, std::shared_ptr const& report); - void composite(SceneElementSequence&& scene_sequence) override; + bool composite(SceneElementSequence&& scene_sequence) override; private: graphics::DisplayBuffer& display_buffer; std::shared_ptr const renderer; std::unique_ptr const fb_adaptor; std::shared_ptr const report; + bool completed_first_render = false; }; } diff --git a/src/server/compositor/multi_threaded_compositor.cpp b/src/server/compositor/multi_threaded_compositor.cpp index 8ced4e4465c..40a941c2555 100644 --- a/src/server/compositor/multi_threaded_compositor.cpp +++ b/src/server/compositor/multi_threaded_compositor.cpp @@ -141,12 +141,17 @@ class CompositingFunctor not_posted_yet = false; lock.unlock(); + bool needs_post = false; for (auto& tuple : compositors) { auto& compositor = std::get<1>(tuple); - compositor->composite(scene->scene_elements_for(compositor.get())); + if (compositor->composite(scene->scene_elements_for(compositor.get()))) + needs_post = true; } - group.post(); + + // We can skip the post if none of the compositors ended up compositing + if (needs_post) + group.post(); /* * "Predictive bypass" optimization: If the last frame was diff --git a/tests/include/mir/test/doubles/null_display_buffer_compositor_factory.h b/tests/include/mir/test/doubles/null_display_buffer_compositor_factory.h index edd54213c4b..a13231c9d49 100644 --- a/tests/include/mir/test/doubles/null_display_buffer_compositor_factory.h +++ b/tests/include/mir/test/doubles/null_display_buffer_compositor_factory.h @@ -37,11 +37,12 @@ class NullDisplayBufferCompositorFactory : public compositor::DisplayBufferCompo { struct NullDisplayBufferCompositor : compositor::DisplayBufferCompositor { - void composite(compositor::SceneElementSequence&&) + bool composite(compositor::SceneElementSequence&&) override { // yield() is needed to ensure reasonable runtime under // valgrind for some tests std::this_thread::yield(); + return true; } }; diff --git a/tests/integration-tests/test_surface_stack_with_compositor.cpp b/tests/integration-tests/test_surface_stack_with_compositor.cpp index 9818bc3dbac..cedef6f3b41 100644 --- a/tests/integration-tests/test_surface_stack_with_compositor.cpp +++ b/tests/integration-tests/test_surface_stack_with_compositor.cpp @@ -164,7 +164,19 @@ struct SurfaceStackCompositor : public Test streams, std::shared_ptr(), null_scene_report)}, - stub_buffer(std::make_shared()) + stub_buffer(std::make_shared()), + other_stream(std::make_shared(geom::Size{ 1, 1 }, mir_pixel_format_abgr_8888 )), + other_streams({ { other_stream, {0,0}, {} } }), + other_stub_surface{std::make_shared( + nullptr /* session */, + mw::Weak{}, + std::string("other_stub"), + geom::Rectangle{{10,0},{1,1}}, + mir_pointer_unconfined, + other_streams, + std::shared_ptr(), + null_scene_report)}, + other_stub_buffer(std::make_shared()) { ON_CALL(*mock_buffer_stream, lock_compositor_buffer(_)) .WillByDefault(Return(mt::fake_shared(*stub_buffer))); @@ -179,6 +191,10 @@ struct SurfaceStackCompositor : public Test std::list const streams; std::shared_ptr stub_surface; std::shared_ptr stub_buffer; + std::shared_ptr other_stream; + std::list const other_streams; + std::shared_ptr other_stub_surface; + std::shared_ptr other_stub_buffer; CountingDisplaySyncGroup stub_primary_db; CountingDisplaySyncGroup stub_secondary_db; StubDisplay stub_display{stub_primary_db, stub_secondary_db}; @@ -196,8 +212,11 @@ std::chrono::milliseconds const default_delay{-1}; } -TEST_F(SurfaceStackCompositor, composes_on_start_if_told_to_in_constructor) +TEST_F(SurfaceStackCompositor, composes_on_start_if_told_to_in_constructor_when_stack_has_at_least_one_surface) { + streams.front().stream->submit_buffer(stub_buffer); + stack.add_surface(stub_surface, mi::InputReceptionMode::normal); + mc::MultiThreadedCompositor mt_compositor( mt::fake_shared(stub_display), mt::fake_shared(stack), @@ -210,6 +229,20 @@ TEST_F(SurfaceStackCompositor, composes_on_start_if_told_to_in_constructor) EXPECT_TRUE(stub_secondary_db.has_posted_at_least(1, timeout)); } +TEST_F(SurfaceStackCompositor, does_not_compose_on_start_if_told_to_in_constructor_but_has_no_surfaces) +{ + mc::MultiThreadedCompositor mt_compositor( + mt::fake_shared(stub_display), + mt::fake_shared(stack), + mt::fake_shared(dbc_factory), + mt::fake_shared(stub_display_listener), + null_comp_report, default_delay, true); + mt_compositor.start(); + + EXPECT_TRUE(stub_primary_db.has_posted_at_least(0, timeout)); + EXPECT_TRUE(stub_secondary_db.has_posted_at_least(0, timeout)); +} + TEST_F(SurfaceStackCompositor, does_not_composes_on_start_if_told_not_to_in_constructor) { mc::MultiThreadedCompositor mt_compositor( @@ -344,6 +377,9 @@ TEST_F(SurfaceStackCompositor, removing_a_surface_triggers_composition) streams.front().stream->submit_buffer(stub_buffer); stack.add_surface(stub_surface, mi::InputReceptionMode::normal); + other_streams.front().stream->submit_buffer(other_stub_buffer); + stack.add_surface(other_stub_surface, mi::InputReceptionMode::normal); + mc::MultiThreadedCompositor mt_compositor( mt::fake_shared(stub_display), mt::fake_shared(stack), diff --git a/tests/mir_test_framework/headless_display_buffer_compositor_factory.cpp b/tests/mir_test_framework/headless_display_buffer_compositor_factory.cpp index 93c92f18967..a41c285006b 100644 --- a/tests/mir_test_framework/headless_display_buffer_compositor_factory.cpp +++ b/tests/mir_test_framework/headless_display_buffer_compositor_factory.cpp @@ -93,7 +93,7 @@ mtf::HeadlessDisplayBufferCompositorFactory::create_compositor_for(mg::DisplayBu return renderables; } - void composite(mir::compositor::SceneElementSequence&& seq) override + bool composite(mir::compositor::SceneElementSequence&& seq) override { auto renderlist = filter(seq, db.view_area()); @@ -113,6 +113,7 @@ mtf::HeadlessDisplayBufferCompositorFactory::create_compositor_for(mg::DisplayBu } output->commit(); + return true; } mg::DisplayBuffer& db; std::unique_ptr const output; diff --git a/tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp b/tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp index 5e937aa3517..a1fd4cbc409 100644 --- a/tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp +++ b/tests/unit-tests/compositor/test_default_display_buffer_compositor.cpp @@ -103,54 +103,50 @@ struct DefaultDisplayBufferCompositor : public testing::Test }; } -TEST_F(DefaultDisplayBufferCompositor, render) +TEST_F(DefaultDisplayBufferCompositor, composite_returns_false_when_scene_elements_are_empty_and_this_is_first_composite) { using namespace testing; - EXPECT_CALL(mock_renderer, render(IsEmpty())) - .Times(1); - EXPECT_CALL(display_buffer, view_area()) - .Times(AtLeast(1)); mc::DefaultDisplayBufferCompositor compositor( display_buffer, gl_provider, mt::fake_shared(mock_renderer), mr::null_compositor_report()); - compositor.composite(make_scene_elements({})); + EXPECT_FALSE(compositor.composite(make_scene_elements({}))); } -TEST_F(DefaultDisplayBufferCompositor, optimization_skips_composition) +TEST_F(DefaultDisplayBufferCompositor, composite_returns_true_when_scene_elements_is_not_empty_and_this_is_first_composite) { using namespace testing; - auto report = std::make_shared(); - Sequence seq; - EXPECT_CALL(*report, began_frame(_)) - .InSequence(seq); - EXPECT_CALL(display_buffer, overlay(_)) - .InSequence(seq) - .WillOnce(Return(true)); - EXPECT_CALL(*report, renderables_in_frame(_,_)) - .InSequence(seq); - EXPECT_CALL(mock_renderer, suspend()) - .InSequence(seq); - EXPECT_CALL(*report, rendered_frame(_)) - .Times(0); - EXPECT_CALL(*report, finished_frame(_)) - .InSequence(seq); + mc::DefaultDisplayBufferCompositor compositor( + display_buffer, + gl_provider, + mt::fake_shared(mock_renderer), + mr::null_compositor_report()); + EXPECT_TRUE(compositor.composite(make_scene_elements({big}))); +} - EXPECT_CALL(mock_renderer, render(_)) - .Times(0); +TEST_F(DefaultDisplayBufferCompositor, renderer_is_suspended_when_we_have_composited_twice_in_a_row_with_the_same_elements) +{ + using namespace testing; mc::DefaultDisplayBufferCompositor compositor( display_buffer, gl_provider, mt::fake_shared(mock_renderer), - report); + mr::null_compositor_report()); + compositor.composite(make_scene_elements({big})); + compositor.composite(make_scene_elements({})); + + EXPECT_CALL(display_buffer, overlay(_)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_renderer, suspend()) + .Times(1); compositor.composite(make_scene_elements({})); } -TEST_F(DefaultDisplayBufferCompositor, rendering_reports_everything) +TEST_F(DefaultDisplayBufferCompositor, all_expected_reports_are_received_during_composite) { using namespace testing; auto report = std::make_shared(); @@ -158,9 +154,6 @@ TEST_F(DefaultDisplayBufferCompositor, rendering_reports_everything) Sequence seq; EXPECT_CALL(*report, began_frame(_)) .InSequence(seq); - EXPECT_CALL(display_buffer, overlay(_)) - .InSequence(seq) - .WillOnce(Return(false)); EXPECT_CALL(*report, renderables_in_frame(_,_)) .InSequence(seq); EXPECT_CALL(*report, rendered_frame(_)) @@ -168,31 +161,19 @@ TEST_F(DefaultDisplayBufferCompositor, rendering_reports_everything) EXPECT_CALL(*report, finished_frame(_)) .InSequence(seq); - EXPECT_CALL(mock_renderer, render(_)) - .Times(1); - mc::DefaultDisplayBufferCompositor compositor( display_buffer, gl_provider, mt::fake_shared(mock_renderer), report); - compositor.composite(make_scene_elements({})); + compositor.composite(make_scene_elements({big})); } -TEST_F(DefaultDisplayBufferCompositor, calls_renderer_in_sequence) +TEST_F(DefaultDisplayBufferCompositor, elements_provided_to_composite_are_rendered_in_order) { using namespace testing; - Sequence render_seq; - EXPECT_CALL(mock_renderer, suspend()) - .Times(0); - EXPECT_CALL(display_buffer, transformation()) - .WillOnce(Return(no_transformation)); - EXPECT_CALL(mock_renderer, set_output_transform(no_transformation)) - .InSequence(render_seq); - EXPECT_CALL(mock_renderer, set_viewport(screen)) - .InSequence(render_seq); EXPECT_CALL(mock_renderer, render(ContainerEq(mg::RenderableList{big, small}))) - .InSequence(render_seq); + .Times(1); mc::DefaultDisplayBufferCompositor compositor( display_buffer, @@ -206,7 +187,7 @@ TEST_F(DefaultDisplayBufferCompositor, calls_renderer_in_sequence) })); } -TEST_F(DefaultDisplayBufferCompositor, rotates_viewport) +TEST_F(DefaultDisplayBufferCompositor, viewport_is_rotated_when_display_buffer_view_area_is_rotated) { // Regression test for LP: #1643488 using namespace testing; @@ -220,77 +201,44 @@ TEST_F(DefaultDisplayBufferCompositor, rotates_viewport) ON_CALL(display_buffer, view_area()) .WillByDefault(Return(rotated_screen)); + mc::DefaultDisplayBufferCompositor compositor( + display_buffer, + gl_provider, + mt::fake_shared(mock_renderer), + mr::null_compositor_report()); + Sequence render_seq; - EXPECT_CALL(mock_renderer, suspend()) - .Times(0); EXPECT_CALL(display_buffer, transformation()) .WillOnce(Return(rotate_left)); EXPECT_CALL(mock_renderer, set_output_transform(rotate_left)) .InSequence(render_seq); EXPECT_CALL(mock_renderer, set_viewport(rotated_screen)) .InSequence(render_seq); - EXPECT_CALL(mock_renderer, render(ContainerEq(mg::RenderableList{big, small}))) - .InSequence(render_seq); - - mc::DefaultDisplayBufferCompositor compositor( - display_buffer, - gl_provider, - mt::fake_shared(mock_renderer), - mr::null_compositor_report()); - compositor.composite(make_scene_elements({ - big, - small - })); + compositor.composite(make_scene_elements({big})); } -TEST_F(DefaultDisplayBufferCompositor, optimization_toggles_seamlessly) +TEST_F(DefaultDisplayBufferCompositor, renderer_is_suspended_when_we_have_composited_many_times_in_a_row_with_the_same_elements) { using namespace testing; - ON_CALL(display_buffer, view_area()) - .WillByDefault(Return(screen)); - ON_CALL(display_buffer, transformation()) - .WillByDefault(Return(no_transformation)); - - Sequence seq; - EXPECT_CALL(display_buffer, overlay(_)) - .InSequence(seq) - .WillOnce(Return(false)); - - EXPECT_CALL(display_buffer, transformation()) - .InSequence(seq); - EXPECT_CALL(mock_renderer, set_output_transform(no_transformation)) - .InSequence(seq); - EXPECT_CALL(mock_renderer, set_viewport(screen)) - .InSequence(seq); - EXPECT_CALL(mock_renderer, render(IsEmpty())) - .InSequence(seq); - - EXPECT_CALL(display_buffer, overlay(_)) - .InSequence(seq) - .WillOnce(Return(true)); - //we should be testing that post_buffer is called, not just that - //we check the bits on the compositor buffer - EXPECT_CALL(display_buffer, overlay(_)) - .InSequence(seq) - .WillOnce(Return(false)); - EXPECT_CALL(display_buffer, transformation()) - .InSequence(seq); - EXPECT_CALL(mock_renderer, set_output_transform(no_transformation)) - .InSequence(seq); - EXPECT_CALL(mock_renderer, set_viewport(screen)) - .InSequence(seq); - EXPECT_CALL(mock_renderer, render(IsEmpty())) - .InSequence(seq); - mc::DefaultDisplayBufferCompositor compositor( display_buffer, gl_provider, mt::fake_shared(mock_renderer), mr::null_compositor_report()); + compositor.composite(make_scene_elements({big})); compositor.composite(make_scene_elements({})); + + EXPECT_CALL(display_buffer, overlay(_)) + .WillRepeatedly(Return(true)); + EXPECT_CALL(mock_renderer, suspend()) + .Times(1); + compositor.composite(make_scene_elements({})); + + EXPECT_CALL(mock_renderer, suspend()) + .Times(1); compositor.composite(make_scene_elements({})); fullscreen->set_buffer({}); // Avoid GMock complaining about false leaks @@ -299,12 +247,6 @@ TEST_F(DefaultDisplayBufferCompositor, optimization_toggles_seamlessly) TEST_F(DefaultDisplayBufferCompositor, occluded_surfaces_are_not_rendered) { using namespace testing; - EXPECT_CALL(display_buffer, view_area()) - .WillRepeatedly(Return(screen)); - EXPECT_CALL(display_buffer, transformation()) - .WillOnce(Return(no_transformation)); - EXPECT_CALL(display_buffer, overlay(_)) - .WillRepeatedly(Return(false)); auto window0 = std::make_shared(geom::Rectangle{{99,99},{2,2}}); auto window1 = std::make_shared(geom::Rectangle{{10,10},{20,20}}); diff --git a/tests/unit-tests/compositor/test_multi_threaded_compositor.cpp b/tests/unit-tests/compositor/test_multi_threaded_compositor.cpp index 8cf3fcf5903..8bac89819bb 100644 --- a/tests/unit-tests/compositor/test_multi_threaded_compositor.cpp +++ b/tests/unit-tests/compositor/test_multi_threaded_compositor.cpp @@ -159,11 +159,12 @@ class RecordingDisplayBufferCompositor : public mc::DisplayBufferCompositor { } - void composite(mc::SceneElementSequence&&) + bool composite(mc::SceneElementSequence&&) { mark_render_buffer(); /* Reduce run-time under valgrind */ std::this_thread::yield(); + return true; } private: @@ -275,11 +276,12 @@ class SurfaceUpdatingDisplayBufferCompositor : public mc::DisplayBufferComposito { } - void composite(mc::SceneElementSequence&&) override + bool composite(mc::SceneElementSequence&&) override { fake_surface_update(); /* Reduce run-time under valgrind */ std::this_thread::yield(); + return true; } private: