From 98a8f8e57c99ce783f45930901226349eaf66060 Mon Sep 17 00:00:00 2001 From: Joshua Ashton Date: Wed, 27 Mar 2024 18:33:50 +0000 Subject: [PATCH] video: Only prefer Wayland if fifo-v1 and commit-timing-v1 are available Wayland has a myriad of unresolved problems regarding surface suspension blocking forever in QueuePresent/SwapBuffers when occludedand the FIFO (vsync) implementation being fundamentally broken leading to reduced GPU-bound performance and 'barcoding' frametimes due to swapchain starvation. There are two protocols used to solve these two problems together -- fifo-v1 and commit-timing-v1, which implement the commit queue on the compositor side, and a timestamp that frames are intended to be displayed for/discarded respectfully. To avoid severe performance regressions for developers targeting SDL3, only pick Wayland as the default backend when these two protocols are supported -- otherwise fallback to X11/XWayland. We do this by having two VideoBootStraps, one which is tests the preferred case, "wayland_preferred" (ie. if fifo-v1 + commit-timing-v1 are available init time), and the fallback, which is just "wayland", the same name as before, which does no such tests. Thus, forcing with SDL_VIDEO_DRIVER=wayland will go onto the fallback option, and pick Wayland always, as usual, so there is no behaviour change. In the case that X11/XWayland is not available (ie. no DISPLAY), we will still fallback to using Wayland without these protocols available. Signed-off-by: Joshua Ashton --- src/video/SDL_sysvideo.h | 3 +- src/video/SDL_video.c | 5 +- src/video/wayland/SDL_waylandvideo.c | 82 +++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 5 deletions(-) diff --git a/src/video/SDL_sysvideo.h b/src/video/SDL_sysvideo.h index 762ed27457a14..4d777a571df34 100644 --- a/src/video/SDL_sysvideo.h +++ b/src/video/SDL_sysvideo.h @@ -478,7 +478,8 @@ extern VideoBootStrap KMSDRM_bootstrap; extern VideoBootStrap KMSDRM_LEGACY_bootstrap; extern VideoBootStrap DUMMY_bootstrap; extern VideoBootStrap DUMMY_evdev_bootstrap; -extern VideoBootStrap Wayland_bootstrap; +extern VideoBootStrap Wayland_preferred_bootstrap; +extern VideoBootStrap Wayland_fallback_bootstrap; extern VideoBootStrap VIVANTE_bootstrap; extern VideoBootStrap Emscripten_bootstrap; extern VideoBootStrap OFFSCREEN_bootstrap; diff --git a/src/video/SDL_video.c b/src/video/SDL_video.c index 1ce0528c18ec3..e01e5853cfa4a 100644 --- a/src/video/SDL_video.c +++ b/src/video/SDL_video.c @@ -70,11 +70,14 @@ static VideoBootStrap *bootstrap[] = { &COCOA_bootstrap, #endif #ifdef SDL_VIDEO_DRIVER_WAYLAND - &Wayland_bootstrap, + &Wayland_preferred_bootstrap, #endif #ifdef SDL_VIDEO_DRIVER_X11 &X11_bootstrap, #endif +#ifdef SDL_VIDEO_DRIVER_WAYLAND + &Wayland_fallback_bootstrap, +#endif #ifdef SDL_VIDEO_DRIVER_VIVANTE &VIVANTE_bootstrap, #endif diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c index 4b372efecd6da..13bdbc105cd3a 100644 --- a/src/video/wayland/SDL_waylandvideo.c +++ b/src/video/wayland/SDL_waylandvideo.c @@ -65,6 +65,7 @@ #include #endif +#define WAYLANDVID_PREFERRED_DRIVER_NAME "wayland_preferred" #define WAYLANDVID_DRIVER_NAME "wayland" /* Clamp certain core protocol versions on older versions of libwayland. */ @@ -369,7 +370,54 @@ static void Wayland_DeleteDevice(SDL_VideoDevice *device) SDL_WAYLAND_UnloadSymbols(); } -static SDL_VideoDevice *Wayland_CreateDevice(void) +typedef struct +{ + SDL_bool has_fifo_v1; + SDL_bool has_commit_timing_v1; +} SDL_WaylandPreferredData; + +static void wayland_preferred_check_handle_global(void *data, struct wl_registry *registry, uint32_t id, + const char *interface, uint32_t version) +{ + SDL_WaylandPreferredData *d = data; + + if (SDL_strcmp(interface, "wp_fifo_manager_v1") == 0) { + d->has_fifo_v1 = SDL_TRUE; + } else if (SDL_strcmp(interface, "wp_commit_timing_manager_v1") == 0) { + d->has_commit_timing_v1 = SDL_TRUE; + } +} + +static void wayland_preferred_check_remove_global(void *data, struct wl_registry *registry, uint32_t id) +{ + /* No need to do anything here. */ +} + +static const struct wl_registry_listener preferred_registry_listener = { + wayland_preferred_check_handle_global, + wayland_preferred_check_remove_global +}; + +static SDL_bool Wayland_IsPreferred(struct wl_display *display) +{ + struct wl_registry *registry = wl_display_get_registry(display); + SDL_WaylandPreferredData preferred_data = { 0 }; + + if (!registry) { + SDL_SetError("Failed to get the Wayland registry"); + return SDL_FALSE; + } + + wl_registry_add_listener(registry, &preferred_registry_listener, &preferred_data); + + WAYLAND_wl_display_roundtrip(display); + + wl_registry_destroy(registry); + + return preferred_data.has_fifo_v1 && preferred_data.has_commit_timing_v1; +} + +static SDL_VideoDevice *Wayland_CreateDevice(SDL_bool fallback) { SDL_VideoDevice *device; SDL_VideoData *data; @@ -398,6 +446,18 @@ static SDL_VideoDevice *Wayland_CreateDevice(void) } } + /* + * If we are checking for preferred Wayland, then let's query for + * fifo-v1 and commit-timing-v1's existance so we don't regress + * GPU-bound performance and frame-pacing by default due to + * swapchain starvation. + */ + if (!fallback && !Wayland_IsPreferred(display)) { + WAYLAND_wl_display_disconnect(display); + SDL_WAYLAND_UnloadSymbols(); + return NULL; + } + data = SDL_calloc(1, sizeof(*data)); if (!data) { WAYLAND_wl_display_disconnect(display); @@ -523,9 +583,25 @@ static SDL_VideoDevice *Wayland_CreateDevice(void) return device; } -VideoBootStrap Wayland_bootstrap = { +static SDL_VideoDevice *Wayland_Preferred_CreateDevice(void) +{ + return Wayland_CreateDevice(SDL_FALSE); +} + +static SDL_VideoDevice *Wayland_Fallback_CreateDevice(void) +{ + return Wayland_CreateDevice(SDL_TRUE); +} + +VideoBootStrap Wayland_preferred_bootstrap = { + WAYLANDVID_PREFERRED_DRIVER_NAME, "SDL Wayland video driver", + Wayland_Preferred_CreateDevice, + Wayland_ShowMessageBox +}; + +VideoBootStrap Wayland_fallback_bootstrap = { WAYLANDVID_DRIVER_NAME, "SDL Wayland video driver", - Wayland_CreateDevice, + Wayland_Fallback_CreateDevice, Wayland_ShowMessageBox };