gpui: Add more flushing of x11 requests (#33407)

Michael Sloan created 5 months ago

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

Change summary

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(-)

Detailed changes

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)
 }

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<E, F, C>(
     failure_context: F,
     result: Result<VoidCookie<'_, C>, 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) {