Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Oct 26, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -134,4 +138,5 @@ void mc::DefaultDisplayBufferCompositor::composite(mc::SceneElementSequence&& sc
}

report->finished_frame(this);
return true;
}
3 changes: 2 additions & 1 deletion src/server/compositor/default_display_buffer_compositor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

}
Expand Down
9 changes: 7 additions & 2 deletions src/server/compositor/multi_threaded_compositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
};

Expand Down
40 changes: 38 additions & 2 deletions tests/integration-tests/test_surface_stack_with_compositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand All @@ -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};
Expand All @@ -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),
Expand All @@ -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(
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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;
Expand Down
Loading
Loading