wayland: simplify render loop

This is actually a very nice simplification that should have been
thought of years ago (sue me). In a nutshell, the story with the
wayland code is that the frame callback and swap buffer behavior doesn't
fit very well with mpv's rendering loop. It's been refactored/changed
quite a few times over the years and works well enough but things could
be better. The current iteration works with an external swapchain to
check if we have frame callback before deciding whether or not to
render. This logic was implemented in both egl and vulkan.

This does have its warts however. There's some hidden state detection
logic which works but is kind of ugly. Since wayland doesn't allow
clients to know if they are actually visible (questionable but
whatever), you can just reasonably assume that if a bunch of callbacks
are missed in a row, you're probably not visible. That's fine, but it is
indeed less than ideal since the threshold is basically entirely
arbitrary and mpv does do a few wasteful renders before it decides that
the window is actually hidden.

The biggest urk in the vo_wayland_wait_frame is the use of
wl_display_roundtrip. Wayland developers would probably be offended by
the way mpv abuses that function, but essentially it was a way to have
semi-blocking behavior needed for display-resample to work. Since the
swap interval must be 0 on wayland (otherwise it will block the entire
player's rendering loop), we need some other way to wait on vsync. The
idea here was to dispatch and poll a bunch of wayland events, wait (with
a timeout) until we get frame callback, and then wait for the compositor
to process it. That pretty much perfectly waits on vsync and lets us
keep all the good timings and all that jazz that we want for mpv. The
problem is that wl_display_roundtrip is conceptually a bad function. It
can internally call wl_display_dispatch which in certain instances,
empty event queue, will block forever. Now strictly speaking, this
probably will never, ever happen (once I was able to to trigger it by
hardcoding an error into a compositor), but ideally
vo_wayland_wait_frame should never infinitely block and stall the
player. Unfortunately, removing that function always lead to problems
with timings and unsteady vsync intervals so it survived many refactors.

Until now, of course. In wayland, the ideal is to never do wasteful
rendering (i.e. don't render if the window isn't visible). Instead of
wrestling around with hidden states and possible missed vblanks, let's
rearrange the wayland rendering logic so we only ever draw a frame when
the frame callback is returned to use (within a reasonable timeout to
avoid blocking forever).

This slight rearrangement of the wait allows for several simplifications
to be made. Namely, wl_display_roundtrip stops being needed. Instead, we
can rely entirely on totally nonblocking calls (dispatch_pending, flush,
and so on). We still need to poll the fd here to actually get the frame
callback event from the compositor, but there's no longer any reason to
do extra waiting. As soon as we get the callback, we immediately draw.
This works quite well and has stable vsync (display-resample and audio).
Additionally, all of the logic about hidden states is no longer needed.
If vo_wayland_wait_frame times out, it's okay to assume immediately that
the window is not visible and skip rendering.

Unfortunately, there's one limitation on this new approach. It will only
work correctly if the compositor implements presentation time. That
means a reduced version of the old way still has to be carried around in
vo_wayland_wait_frame. So if the compositor has no presentation time,
then we are forced to use wl_display_roundtrip and juggle some funny
assumptions about whether or not the window is hidden or not. Plasma is
the only real notable compositor without presentation time at this stage
so perhaps this "legacy" mechanism could be removed in the future.

5 files changed, 36 insertions, 49 deletions

diff --git a/video/out/opengl/context_wayland.c b/video/out/opengl/context_wayland.c
index 05c2b6c436..27a9aab32a 100644
--- a/video/out/opengl/context_wayland.c
+++ b/video/out/opengl/context_wayland.c
@@ -63,15 +63,12 @@ static bool wayland_egl_start_frame(struct ra_swapchain *sw, struct ra_fbo *out_
 {
     struct ra_ctx *ctx = sw->ctx;
     struct vo_wayland_state *wl = ctx->vo->wl;
+    bool render = true;
 
-    bool render = !wl->hidden || wl->opts->disable_vsync;
-
-    if (wl->frame_wait && wl->presentation)
-        vo_wayland_sync_clear(wl);
-
-    if (render)
-        wl->frame_wait = true;
+    if (!wl->opts->disable_vsync)
+        render = vo_wayland_wait_frame(wl);
 
+    wl->frame_wait = true;
     return render ? ra_gl_ctx_start_frame(sw, out_fbo) : false;
 }
 
@@ -83,16 +80,13 @@ static void wayland_egl_swap_buffers(struct ra_swapchain *sw)
 
     eglSwapBuffers(p->egl_display, p->egl_surface);
 
-    if (!wl->opts->disable_vsync)
-        vo_wayland_wait_frame(wl);
-
     if (wl->presentation)
         wayland_sync_swap(wl);
 }
 
 static const struct ra_swapchain_fns wayland_egl_swapchain = {
-    .start_frame   = wayland_egl_start_frame,
-    .swap_buffers  = wayland_egl_swap_buffers,
+    .start_frame  = wayland_egl_start_frame,
+    .swap_buffers = wayland_egl_swap_buffers,
 };
 
 static void wayland_egl_get_vsync(struct ra_ctx *ctx, struct vo_vsync_info *info)
diff --git a/video/out/vo_wlshm.c b/video/out/vo_wlshm.c
index a71f9507fc..508da4261f 100644
--- a/video/out/vo_wlshm.c
+++ b/video/out/vo_wlshm.c
@@ -218,11 +218,14 @@ static void draw_image(struct vo *vo, struct mp_image *src)
     struct priv *p = vo->priv;
     struct vo_wayland_state *wl = vo->wl;
     struct buffer *buf;
+    bool callback = true;
 
-    if (wl->hidden)
-        return;
+    if (!wl->opts->disable_vsync)
+        callback = vo_wayland_wait_frame(wl);
 
     wl->frame_wait = true;
+    if (!callback)
+        return;
 
     buf = p->free_buffers;
     if (buf) {
@@ -274,9 +277,6 @@ static void flip_page(struct vo *vo)
                       mp_rect_h(wl->geometry));
     wl_surface_commit(wl->surface);
 
-    if (!wl->opts->disable_vsync)
-        vo_wayland_wait_frame(wl);
-
     if (wl->presentation)
         wayland_sync_swap(wl);
 }
diff --git a/video/out/vulkan/context_wayland.c b/video/out/vulkan/context_wayland.c
index 8782af5527..d36ab89ed4 100644
--- a/video/out/vulkan/context_wayland.c
+++ b/video/out/vulkan/context_wayland.c
@@ -32,25 +32,18 @@ struct priv {
 static bool wayland_vk_start_frame(struct ra_ctx *ctx)
 {
     struct vo_wayland_state *wl = ctx->vo->wl;
+    bool render = true;
 
-    bool render = !wl->hidden || wl->opts->disable_vsync;
-
-    if (wl->frame_wait && wl->presentation)
-        vo_wayland_sync_clear(wl);
-
-    if (render)
-        wl->frame_wait = true;
+    if (!wl->opts->disable_vsync)
+        render = vo_wayland_wait_frame(wl);
 
+    wl->frame_wait = true;
     return render;
 }
 
 static void wayland_vk_swap_buffers(struct ra_ctx *ctx)
 {
     struct vo_wayland_state *wl = ctx->vo->wl;
-
-    if (!wl->opts->disable_vsync)
-        vo_wayland_wait_frame(wl);
-
     if (wl->presentation)
         wayland_sync_swap(wl);
 }
diff --git a/video/out/wayland_common.c b/video/out/wayland_common.c
index 1902475ead..6075b9a34b 100644
--- a/video/out/wayland_common.c
+++ b/video/out/wayland_common.c
@@ -1796,13 +1796,6 @@ void vo_wayland_set_opaque_region(struct vo_wayland_state *wl, int alpha)
     }
 }
 
-void vo_wayland_sync_clear(struct vo_wayland_state *wl)
-{
-    struct vo_wayland_sync sync = {0, 0, 0, 0};
-    for (int i = 0; i < wl->sync_size; ++i)
-        wl->sync[i] = sync;
-}
-
 void vo_wayland_sync_shift(struct vo_wayland_state *wl)
 {
     for (int i = wl->sync_size - 1; i > 0; --i) {
@@ -1871,7 +1864,7 @@ void vo_wayland_wakeup(struct vo *vo)
     (void)write(wl->wakeup_pipe[1], &(char){0}, 1);
 }
 
-void vo_wayland_wait_frame(struct vo_wayland_state *wl)
+bool vo_wayland_wait_frame(struct vo_wayland_state *wl)
 {
     int64_t vblank_time = 0;
     struct pollfd fds[1] = {
@@ -1922,19 +1915,28 @@ void vo_wayland_wait_frame(struct vo_wayland_state *wl)
         wl_display_dispatch_pending(wl->display);
     }
 
-    if (!wl->hidden && wl->frame_wait) {
-        wl->timeout_count += 1;
-        if (wl->timeout_count > ((1 / (double)vblank_time) * 1e6))
-            wl->hidden = true;
-    }
+    /* If the compositor does not have presentatiom time, we cannot be sure
+     * that this wait is accurate. Do some crap with wl_display_roundtrip
+     * and randomly assume that if timeouts > refresh rate, the window is
+     * hidden. This is neccesary otherwise we may mistakeningly skip rendering.*/
+    if (!wl->presentation) {
+        if (wl_display_get_error(wl->display) == 0)
+            wl_display_roundtrip(wl->display);
+        if (wl->frame_wait) {
+            if (wl->timeout_count > ((1 / (double)vblank_time) * 1e6)) {
+                return false;
+            } else {
+                wl->timeout_count += 1;
+                return true;
+            }
+        } else {
+            wl->timeout_count = 0;
+            return true;
+        }
 
-    if (!wl->frame_wait) {
-        wl->timeout_count = 0;
-        wl->hidden = false;
     }
 
-    if (wl_display_get_error(wl->display) == 0)
-        wl_display_roundtrip(wl->display);
+    return !wl->frame_wait;
 }
 
 void vo_wayland_wait_events(struct vo *vo, int64_t until_time_us)
diff --git a/video/out/wayland_common.h b/video/out/wayland_common.h
index 0414ac6352..948859e79a 100644
--- a/video/out/wayland_common.h
+++ b/video/out/wayland_common.h
@@ -79,7 +79,6 @@ struct vo_wayland_state {
     bool activated;
     bool has_keyboard_input;
     bool focused;
-    bool hidden;
     int timeout_count;
     int wakeup_pipe[2];
     int pending_vo_events;
@@ -153,9 +152,8 @@ int last_available_sync(struct vo_wayland_state *wl);
 void vo_wayland_uninit(struct vo *vo);
 void vo_wayland_wakeup(struct vo *vo);
 void vo_wayland_wait_events(struct vo *vo, int64_t until_time_us);
-void vo_wayland_wait_frame(struct vo_wayland_state *wl);
+bool vo_wayland_wait_frame(struct vo_wayland_state *wl);
 void vo_wayland_set_opaque_region(struct vo_wayland_state *wl, int alpha);
-void vo_wayland_sync_clear(struct vo_wayland_state *wl);
 void wayland_sync_swap(struct vo_wayland_state *wl);
 void vo_wayland_sync_shift(struct vo_wayland_state *wl);
 void queue_new_sync(struct vo_wayland_state *wl);