Merge pull request #3086 from MirServer/feature/not_compositing_right…

…_away

Support for not compositing until at least one renderable has been sent to the compositor

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;

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;
 }

src/server/compositor/default_display_buffer_compositor.h

@@ -49,13 +49,14 @@ class DefaultDisplayBufferCompositor : public DisplayBufferCompositor
         std::shared_ptr<renderer::Renderer> const& renderer,
         std::shared_ptr<compositor::CompositorReport> const& report);
 
-    void composite(SceneElementSequence&& scene_sequence) override;
+    bool composite(SceneElementSequence&& scene_sequence) override;
 
 private:
     graphics::DisplayBuffer& display_buffer;
     std::shared_ptr<renderer::Renderer> const renderer;
     std::unique_ptr<graphics::RenderingProvider::FramebufferProvider> const fb_adaptor;
     std::shared_ptr<compositor::CompositorReport> const report;
+    bool completed_first_render = false;
 };
 
 }

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

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;
             }
         };
 

tests/integration-tests/test_surface_stack_with_compositor.cpp

@@ -164,7 +164,19 @@ struct SurfaceStackCompositor : public Test
             streams,
             std::shared_ptr<mg::CursorImage>(),
             null_scene_report)},
-        stub_buffer(std::make_shared<mtd::StubBuffer>())
+        stub_buffer(std::make_shared<mtd::StubBuffer>()),
+        other_stream(std::make_shared<mc::Stream>(geom::Size{ 1, 1 }, mir_pixel_format_abgr_8888 )),
+        other_streams({ { other_stream, {0,0}, {} } }),
+        other_stub_surface{std::make_shared<ms::BasicSurface>(
+            nullptr /* session */,
+            mw::Weak<mf::WlSurface>{},
+            std::string("other_stub"),
+            geom::Rectangle{{10,0},{1,1}},
+            mir_pointer_unconfined,
+            other_streams,
+            std::shared_ptr<mg::CursorImage>(),
+            null_scene_report)},
+        other_stub_buffer(std::make_shared<mtd::StubBuffer>())
     {
         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<ms::StreamInfo> const streams;
     std::shared_ptr<ms::BasicSurface> stub_surface;
     std::shared_ptr<mg::Buffer> stub_buffer;
+    std::shared_ptr<mc::Stream> other_stream;
+    std::list<ms::StreamInfo> const other_streams;
+    std::shared_ptr<ms::BasicSurface> other_stub_surface;
+    std::shared_ptr<mg::Buffer> 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),

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<mg::gl::OutputSurface> const output;