From 0a82b37e249701114e78529862626ef8ffb9e0c1 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 18 Nov 2024 12:47:47 +1100 Subject: [PATCH 1/3] Drop unused forward-declaration of vestigial interface --- src/server/compositor/multi_monitor_arbiter.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/server/compositor/multi_monitor_arbiter.h b/src/server/compositor/multi_monitor_arbiter.h index c251475450b..0bf80c0ebe1 100644 --- a/src/server/compositor/multi_monitor_arbiter.h +++ b/src/server/compositor/multi_monitor_arbiter.h @@ -31,7 +31,6 @@ namespace mir namespace graphics { class Buffer; } namespace compositor { -class Schedule; class MultiMonitorArbiter : public std::enable_shared_from_this { From 3f05ae749b6d5a82d6ed190fc4f239e00c45fff2 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 18 Nov 2024 17:42:41 +1100 Subject: [PATCH 2/3] MIRENG-742: Fix stuck frame after mode switch. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we perform a mode switch Mir first tears down all the `Compositor`s, does the switch, and then creates a new set of `Compositor`s, one for each output. But, in the buffer queue code, we track which `Compositor`s have accessed the “current” buffer of a surface, and only advance to the most recently submitted buffer when the `Compositor` had already seen the current one. After the mode switch, *all* the `Compositor`s are new, so *none* of them will have seen the “current” buffer, and so will not receive the most recently submitted client buffer until something else triggers a new composite pass. The buffer queue code is going to need to change significantly in the forseeable future to accomodate the requirements of some Wayland extensions, so work around this problem by having each `Compositor` thread mark itself as a user of the “current” buffer on construction, so the first composition pass will receive the most recent buffer. --- .../compositor/multi_threaded_compositor.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/server/compositor/multi_threaded_compositor.cpp b/src/server/compositor/multi_threaded_compositor.cpp index 7c7d5960c19..22fd64bacb4 100644 --- a/src/server/compositor/multi_threaded_compositor.cpp +++ b/src/server/compositor/multi_threaded_compositor.cpp @@ -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" @@ -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 */ From 9b1935a46b16c95931f501e20821365ff478bf16 Mon Sep 17 00:00:00 2001 From: Christopher James Halse Rogers Date: Mon, 18 Nov 2024 17:43:04 +1100 Subject: [PATCH 3/3] Document `MultiMonitorArbiter` a bit --- src/server/compositor/multi_monitor_arbiter.h | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/server/compositor/multi_monitor_arbiter.h b/src/server/compositor/multi_monitor_arbiter.h index 0bf80c0ebe1..89b6f0e6640 100644 --- a/src/server/compositor/multi_monitor_arbiter.h +++ b/src/server/compositor/multi_monitor_arbiter.h @@ -32,6 +32,42 @@ namespace graphics { class Buffer; } namespace compositor { +/** + * 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 { public: