Merge pull request godotengine#101454 from Riteo/fifo_ftw

Wayland: Handle `fifo_v1` and clean up suspension logic

Author: Repiteo

platform/linuxbsd/wayland/display_server_wayland.cpp

@@ -822,8 +822,9 @@ void DisplayServerWayland::show_window(WindowID p_window_id) {
 		}
 #endif
 
-		// NOTE: The public window-handling methods might depend on this flag being
-		// set. Ensure to not make any of these calls before this assignment.
+		// NOTE: Some public window-handling methods might depend on this flag being
+		// set. Make sure the method you're calling does not depend on it before this
+		// assignment.
 		wd.visible = true;
 
 		// Actually try to apply the window's mode now that it's visible.
@@ -1349,7 +1350,7 @@ void DisplayServerWayland::window_set_vsync_mode(DisplayServer::VSyncMode p_vsyn
 	if (rendering_context) {
 		rendering_context->window_set_vsync_mode(p_window_id, p_vsync_mode);
 
-		wd.emulate_vsync = (rendering_context->window_get_vsync_mode(p_window_id) == DisplayServer::VSYNC_ENABLED);
+		wd.emulate_vsync = (!wayland_thread.is_fifo_available() && rendering_context->window_get_vsync_mode(p_window_id) == DisplayServer::VSYNC_ENABLED);
 
 		if (wd.emulate_vsync) {
 			print_verbose("VSYNC: manually throttling frames using MAILBOX.");
@@ -1362,6 +1363,8 @@ void DisplayServerWayland::window_set_vsync_mode(DisplayServer::VSyncMode p_vsyn
 	if (egl_manager) {
 		egl_manager->set_use_vsync(p_vsync_mode != DisplayServer::VSYNC_DISABLED);
 
+		// NOTE: Mesa's EGL implementation does not seem to make use of fifo_v1 so
+		// we'll have to always emulate V-Sync.
 		wd.emulate_vsync = egl_manager->is_using_vsync();
 
 		if (wd.emulate_vsync) {
@@ -1542,38 +1545,14 @@ bool DisplayServerWayland::color_picker(const Callable &p_callback) {
 }
 
 void DisplayServerWayland::try_suspend() {
-	bool must_emulate = false;
-
-	for (KeyValue<DisplayServer::WindowID, WindowData> &pair : windows) {
-		if (pair.value.emulate_vsync) {
-			must_emulate = true;
-			break;
-		}
-	}
-
 	// Due to various reasons, we manually handle display synchronization by
 	// waiting for a frame event (request to draw) or, if available, the actual
 	// window's suspend status. When a window is suspended, we can avoid drawing
 	// altogether, either because the compositor told us that we don't need to or
 	// because the pace of the frame events became unreliable.
-	if (must_emulate) {
-		bool frame = wayland_thread.wait_frame_suspend_ms(WAYLAND_MAX_FRAME_TIME_US / 1000);
-		if (!frame) {
-			suspend_state = SuspendState::TIMEOUT;
-		}
-	}
-
-	// If we suspended by capability, we'll know with this check. We must do this
-	// after `wait_frame_suspend_ms` as it progressively dispatches the event queue
-	// during the "timeout".
-	if (wayland_thread.is_suspended()) {
-		suspend_state = SuspendState::CAPABILITY;
-	}
-
-	if (suspend_state == SuspendState::TIMEOUT) {
-		DEBUG_LOG_WAYLAND("Suspending. Reason: timeout.");
-	} else if (suspend_state == SuspendState::CAPABILITY) {
-		DEBUG_LOG_WAYLAND("Suspending. Reason: capability.");
+	bool frame = wayland_thread.wait_frame_suspend_ms(WAYLAND_MAX_FRAME_TIME_US / 1000);
+	if (!frame) {
+		suspend_state = SuspendState::TIMEOUT;
 	}
 }
 
@@ -1724,51 +1703,75 @@ void DisplayServerWayland::process_events() {
 
 	wayland_thread.keyboard_echo_keys();
 
-	if (suspend_state == SuspendState::NONE) {
-		// Due to the way legacy suspension works, we have to treat low processor
-		// usage mode very differently than the regular one.
-		if (OS::get_singleton()->is_in_low_processor_usage_mode()) {
-			// NOTE: We must avoid committing a surface if we expect a new frame, as we
-			// might otherwise commit some inconsistent data (e.g. buffer scale). Note
-			// that if a new frame is expected it's going to be committed by the renderer
-			// soon anyways.
-			if (!RenderingServer::get_singleton()->has_changed()) {
-				// We _can't_ commit in a different thread (such as in the frame callback
-				// itself) because we would risk to step on the renderer's feet, which would
-				// cause subtle but severe issues, such as crashes on setups with explicit
-				// sync. This isn't normally a problem, as the renderer commits at every
-				// frame (which is what we need for atomic surface updates anyways), but in
-				// low processor usage mode that expectation is broken. When it's on, our
-				// frame rate stops being constant. This also reflects in the frame
-				// information we use for legacy suspension. In order to avoid issues, let's
-				// manually commit all surfaces, so that we can get fresh frame data.
-				wayland_thread.commit_surfaces();
-				try_suspend();
+	switch (suspend_state) {
+		case SuspendState::NONE: {
+			bool emulate_vsync = false;
+			for (KeyValue<DisplayServer::WindowID, WindowData> &pair : windows) {
+				if (pair.value.emulate_vsync) {
+					emulate_vsync = true;
+					break;
+				}
 			}
-		} else {
-			try_suspend();
-		}
-	} else {
-		if (suspend_state == SuspendState::CAPABILITY) {
-			// If we suspended by capability we can assume that it will be reset when
-			// the compositor wants us to repaint.
-			if (!wayland_thread.is_suspended()) {
-				suspend_state = SuspendState::NONE;
-				DEBUG_LOG_WAYLAND("Unsuspending from capability.");
+
+			if (emulate_vsync) {
+				// Due to the way legacy suspension works, we have to treat low processor
+				// usage mode very differently than the regular one.
+				if (OS::get_singleton()->is_in_low_processor_usage_mode()) {
+					// NOTE: We must avoid committing a surface if we expect a new frame, as we
+					// might otherwise commit some inconsistent data (e.g. buffer scale). Note
+					// that if a new frame is expected it's going to be committed by the renderer
+					// soon anyways.
+					if (!RenderingServer::get_singleton()->has_changed()) {
+						// We _can't_ commit in a different thread (such as in the frame callback
+						// itself) because we would risk to step on the renderer's feet, which would
+						// cause subtle but severe issues, such as crashes on setups with explicit
+						// sync. This isn't normally a problem, as the renderer commits at every
+						// frame (which is what we need for atomic surface updates anyways), but in
+						// low processor usage mode that expectation is broken. When it's on, our
+						// frame rate stops being constant. This also reflects in the frame
+						// information we use for legacy suspension. In order to avoid issues, let's
+						// manually commit all surfaces, so that we can get fresh frame data.
+						wayland_thread.commit_surfaces();
+						try_suspend();
+					}
+				} else {
+					try_suspend();
+				}
+			}
+
+			if (wayland_thread.is_suspended()) {
+				suspend_state = SuspendState::CAPABILITY;
 			}
-		} else if (suspend_state == SuspendState::TIMEOUT) {
+
+			if (suspend_state == SuspendState::TIMEOUT) {
+				DEBUG_LOG_WAYLAND("Suspending. Reason: timeout.");
+			} else if (suspend_state == SuspendState::CAPABILITY) {
+				DEBUG_LOG_WAYLAND("Suspending. Reason: capability.");
+			}
+		} break;
+
+		case SuspendState::TIMEOUT: {
 			// Certain compositors might not report the "suspended" wm_capability flag.
 			// Because of this we'll wake up at the next frame event, indicating the
 			// desire for the compositor to let us repaint.
 			if (wayland_thread.get_reset_frame()) {
 				suspend_state = SuspendState::NONE;
 				DEBUG_LOG_WAYLAND("Unsuspending from timeout.");
 			}
-		}
 
-		// Since we're not rendering, nothing is committing the windows'
-		// surfaces. We have to do it ourselves.
-		wayland_thread.commit_surfaces();
+			// Since we're not rendering, nothing is committing the windows'
+			// surfaces. We have to do it ourselves.
+			wayland_thread.commit_surfaces();
+		} break;
+
+		case SuspendState::CAPABILITY: {
+			// If we suspended by capability we can assume that it will be reset when
+			// the compositor wants us to repaint.
+			if (!wayland_thread.is_suspended()) {
+				suspend_state = SuspendState::NONE;
+				DEBUG_LOG_WAYLAND("Unsuspending from capability.");
+			}
+		} break;
 	}
 
 #ifdef DBUS_ENABLED

platform/linuxbsd/wayland/wayland_thread.cpp

@@ -60,6 +60,10 @@
 #define DEBUG_LOG_WAYLAND_THREAD(...)
 #endif
 
+// Since we're never going to use this interface directly, it's not worth
+// generating the whole deal.
+#define FIFO_INTERFACE_NAME "wp_fifo_manager_v1"
+
 // Read the content pointed by fd into a Vector<uint8_t>.
 Vector<uint8_t> WaylandThread::_read_fd(int fd) {
 	// This is pretty much an arbitrary size.
@@ -640,6 +644,10 @@ void WaylandThread::_wl_registry_on_global(void *data, struct wl_registry *wl_re
 
 		return;
 	}
+
+	if (strcmp(interface, FIFO_INTERFACE_NAME) == 0) {
+		registry->wp_fifo_manager_name = name;
+	}
 }
 
 void WaylandThread::_wl_registry_on_global_remove(void *data, struct wl_registry *wl_registry, uint32_t name) {
@@ -1028,6 +1036,10 @@ void WaylandThread::_wl_registry_on_global_remove(void *data, struct wl_registry
 			it = it->next();
 		}
 	}
+
+	if (name == registry->wp_fifo_manager_name) {
+		registry->wp_fifo_manager_name = 0;
+	}
 }
 
 void WaylandThread::_wl_surface_on_enter(void *data, struct wl_surface *wl_surface, struct wl_output *wl_output) {
@@ -4275,6 +4287,10 @@ Error WaylandThread::init() {
 	}
 #endif // DBUS_ENABLED
 
+	if (!registry.wp_fifo_manager_name) {
+		WARN_PRINT("FIFO protocol not found! Frame pacing will be degraded.");
+	}
+
 	// Wait for seat capabilities.
 	wl_display_roundtrip(wl_display);
 
@@ -4777,6 +4793,10 @@ bool WaylandThread::window_is_suspended(DisplayServer::WindowID p_window_id) con
 	return windows[p_window_id].suspended;
 }
 
+bool WaylandThread::is_fifo_available() const {
+	return registry.wp_fifo_manager_name != 0;
+}
+
 bool WaylandThread::is_suspended() const {
 	for (const KeyValue<DisplayServer::WindowID, WindowState> &E : windows) {
 		if (!E.value.suspended) {

platform/linuxbsd/wayland/wayland_thread.h

@@ -216,6 +216,10 @@ class WaylandThread {
 
 		struct zwp_text_input_manager_v3 *wp_text_input_manager = nullptr;
 		uint32_t wp_text_input_manager_name = 0;
+
+		// We're really not meant to use this one directly but we still need to know
+		// whether it's available.
+		uint32_t wp_fifo_manager_name = 0;
 	};
 
 	// General Wayland-specific states. Shouldn't be accessed directly.
@@ -1068,6 +1072,7 @@ class WaylandThread {
 	void set_frame();
 	bool get_reset_frame();
 	bool wait_frame_suspend_ms(int p_timeout);
+	bool is_fifo_available() const;
 
 	uint64_t window_get_last_frame_time(DisplayServer::WindowID p_window_id) const;
 	bool window_is_suspended(DisplayServer::WindowID p_window_id) const;