Skip to content

Commit

Permalink
Fix stuck-frame-after-mode-switch bug (#3673)
Browse files Browse the repository at this point in the history
  • Loading branch information
Saviq authored Nov 19, 2024
2 parents a2da926 + 9b1935a commit e72bacc
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 1 deletion.
37 changes: 36 additions & 1 deletion src/server/compositor/multi_monitor_arbiter.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,43 @@ namespace mir
namespace graphics { class Buffer; }
namespace compositor
{
class Schedule;

/**
* Coordinate surface buffer consumption across different outputs
*
* When multiple compositors have a view on the same surface, they will
* request buffers from that surface at their own pace, gated by either
* the display VSync (for compositors driving physical outputs), by
* client consumption (for compositors for screencopy), or some other
* mechanism.
*
* These timings will (almost always) not align, so if each compositor's
* request advanced the surface buffer we would request frames at some
* multiple of the frequencies involved.
*
* As an additional constraint, it must always be possible for a compositor
* to acquire a buffer.
*
* To avoid this, the MultiMonitorArbiter stores up to two buffers -
* a current buffer and a next buffer - and tracks which compositor has
* seen the current buffer. The next buffer is only advanced when a
* compositor requests a buffer *and* that compositor has seen the current buffer.
*
* This results in the buffer queue being advanced at only the rate of the fastest
* compositor.
*
* Unfortunately, because this doesn't have any feedback into any other
* component, we can get into a state where there *is* a new buffer available
* for a surface, but the *Compositor* isn't going to request a buffer until
* something else changes. This is most easy to trigger around mode switches,
* where we destroy all Compositors and then create new ones which then,
* definitionally, haven't seen the current buffer.
*
* This system will need to be significantly overhauled in future. A variety
* of Wayland extensions we wish to support - notably wp_presentation and
* wp_fifo - expect that a surface's buffer queue is synchronised to a single
* output.
*/
class MultiMonitorArbiter : public std::enable_shared_from_this<MultiMonitorArbiter>
{
public:
Expand Down
18 changes: 18 additions & 0 deletions src/server/compositor/multi_threaded_compositor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/

#include "multi_threaded_compositor.h"
#include "mir/compositor/scene_element.h"
#include "mir/graphics/display.h"
#include "mir/graphics/display_sink.h"
#include "mir/compositor/display_buffer_compositor.h"
Expand Down Expand Up @@ -117,6 +118,23 @@ class CompositingFunctor

try
{
/* Ensure we get a fresh frame by consuming any existing stale frame
*
* This ensures each Compositor is treated as displaying the
* “current frame”, so the first time through the composition
* loop below we will pull the most recently submitted frame from
* each surface (either the “current” frame, or the ”next” frame).
*/
for (auto const& [_, compositor] : compositors)
{
auto elements = scene->scene_elements_for(compositor.get());
for (auto const& element : elements)
{
// We need to actually access the buffer in order for it to be marked in use.
element->renderable()->buffer();
}
}

while (running)
{
/* Wait until compositing has been scheduled or we are stopped */
Expand Down

0 comments on commit e72bacc

Please sign in to comment.