From aae4778b4e455630af1a084f0f235babf2d05a01 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Wed, 25 Jun 2025 13:46:15 -0600 Subject: [PATCH] gpui: Add more flushing of x11 requests (#33407) Flushes should happen after sending messages to X11 when effects should be applied quickly. This is not needed for requests that return replies since it automatically flushes in that case. Release Notes: - N/A --- crates/gpui/src/platform/linux/x11/client.rs | 26 ++++++++------ crates/gpui/src/platform/linux/x11/window.rs | 36 +++++++++++--------- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/crates/gpui/src/platform/linux/x11/client.rs b/crates/gpui/src/platform/linux/x11/client.rs index dddff8566102be44b63680999be7bed30439e7a5..f0ad8b8cf416498f3a1719180f3d6cc7327dceef 100644 --- a/crates/gpui/src/platform/linux/x11/client.rs +++ b/crates/gpui/src/platform/linux/x11/client.rs @@ -1,4 +1,4 @@ -use crate::Capslock; +use crate::{Capslock, xcb_flush}; use core::str; use std::{ cell::RefCell, @@ -378,6 +378,7 @@ impl X11Client { xcb_connection .xkb_use_extension(XKB_X11_MIN_MAJOR_XKB_VERSION, XKB_X11_MIN_MINOR_XKB_VERSION), )?; + assert!(xkb.supported); let events = xkb::EventType::STATE_NOTIFY | xkb::EventType::MAP_NOTIFY @@ -401,7 +402,6 @@ impl X11Client { &xkb::SelectEventsAux::new(), ), )?; - assert!(xkb.supported); let xkb_context = xkbc::Context::new(xkbc::CONTEXT_NO_FLAGS); let xkb_device_id = xkbc::x11::get_core_keyboard_device_id(&xcb_connection); @@ -484,6 +484,8 @@ impl X11Client { }) .map_err(|err| anyhow!("Failed to initialize XDP event source: {err:?}"))?; + xcb_flush(&xcb_connection); + Ok(X11Client(Rc::new(RefCell::new(X11ClientState { modifiers: Modifiers::default(), capslock: Capslock::default(), @@ -1523,6 +1525,7 @@ impl LinuxClient for X11Client { ), ) .log_err(); + xcb_flush(&state.xcb_connection); let window_ref = WindowRef { window: window.0.clone(), @@ -1554,19 +1557,18 @@ impl LinuxClient for X11Client { }; state.cursor_styles.insert(focused_window, style); - state - .xcb_connection - .change_window_attributes( + check_reply( + || "Failed to set cursor style", + state.xcb_connection.change_window_attributes( focused_window, &ChangeWindowAttributesAux { cursor: Some(cursor), ..Default::default() }, - ) - .anyhow() - .and_then(|cookie| cookie.check().anyhow()) - .context("X11: Failed to set cursor style") - .log_err(); + ), + ) + .log_err(); + state.xcb_connection.flush().log_err(); } fn open_uri(&self, uri: &str) { @@ -2087,6 +2089,7 @@ fn xdnd_send_finished( xcb_connection.send_event(false, target, EventMask::default(), message), ) .log_err(); + xcb_connection.flush().log_err(); } fn xdnd_send_status( @@ -2109,6 +2112,7 @@ fn xdnd_send_status( xcb_connection.send_event(false, target, EventMask::default(), message), ) .log_err(); + xcb_connection.flush().log_err(); } /// Recomputes `pointer_device_states` by querying all pointer devices. @@ -2262,6 +2266,6 @@ fn create_invisible_cursor( connection.free_pixmap(empty_pixmap)?; - connection.flush()?; + xcb_flush(connection); Ok(cursor) } diff --git a/crates/gpui/src/platform/linux/x11/window.rs b/crates/gpui/src/platform/linux/x11/window.rs index 2b6028f1ccc2e258f80e33f533fcf5bbf69f881a..673c04a3e5edfcdbb4efd508bffd50b0c50891c5 100644 --- a/crates/gpui/src/platform/linux/x11/window.rs +++ b/crates/gpui/src/platform/linux/x11/window.rs @@ -320,6 +320,13 @@ impl rwh::HasDisplayHandle for X11Window { } } +pub(crate) fn xcb_flush(xcb: &XCBConnection) { + xcb.flush() + .map_err(handle_connection_error) + .context("X11 flush failed") + .log_err(); +} + pub(crate) fn check_reply( failure_context: F, result: Result, ConnectionError>, @@ -597,7 +604,7 @@ impl X11WindowState { ), )?; - xcb.flush()?; + xcb_flush(&xcb); let renderer = { let raw_window = RawWindow { @@ -657,7 +664,7 @@ impl X11WindowState { || "X11 DestroyWindow failed while cleaning it up after setup failure.", xcb.destroy_window(x_window), )?; - xcb.flush()?; + xcb_flush(&xcb); } setup_result @@ -685,7 +692,7 @@ impl Drop for X11WindowHandle { || "X11 DestroyWindow failed while dropping X11WindowHandle.", self.xcb.destroy_window(self.id), )?; - self.xcb.flush()?; + xcb_flush(&self.xcb); anyhow::Ok(()) }) .log_err(); @@ -704,7 +711,7 @@ impl Drop for X11Window { || "X11 DestroyWindow failure.", self.0.xcb.destroy_window(self.0.x_window), )?; - self.0.xcb.flush()?; + xcb_flush(&self.0.xcb); anyhow::Ok(()) }) @@ -799,7 +806,9 @@ impl X11Window { xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY, message, ), - ) + )?; + xcb_flush(&self.0.xcb); + Ok(()) } fn get_root_position( @@ -852,15 +861,8 @@ impl X11Window { ), )?; - self.flush() - } - - fn flush(&self) -> anyhow::Result<()> { - self.0 - .xcb - .flush() - .map_err(handle_connection_error) - .context("X11 flush failed") + xcb_flush(&self.0.xcb); + Ok(()) } } @@ -1198,7 +1200,7 @@ impl PlatformWindow for X11Window { ), ) .log_err(); - self.flush().log_err(); + xcb_flush(&self.0.xcb); } fn scale_factor(&self) -> f32 { @@ -1289,7 +1291,7 @@ impl PlatformWindow for X11Window { xproto::Time::CURRENT_TIME, ) .log_err(); - self.flush().log_err(); + xcb_flush(&self.0.xcb); } fn is_active(&self) -> bool { @@ -1324,7 +1326,7 @@ impl PlatformWindow for X11Window { ), ) .log_err(); - self.flush().log_err(); + xcb_flush(&self.0.xcb); } fn set_app_id(&mut self, app_id: &str) {