linux/x11: handle XIM events sync to reduce lag (#12840)

Thorsten Ball created 

This helps with the problem of keyboard input feeling laggy when the
event loop is under load.

What would previously happen is:

- N events from X11 arrive
- N events get forwarded to XIM
- N events are handled in N iterations of the event loop (sadly, yes: we
only seem to be getting back one `ClientMessage` per poll from XCB
connection)
- Each event is pushed into the channel
- N event loop iterations are needed to get the events off the channel
and handle them

With this change, we get rid of the last 2 steps: instead of pushing the
event onto a channel, we store it on the XIM handler itself, and then
work it off synchronously.

Usually one shouldn't block the event loop, but I think in this case -
user input! - it's better to handle the events directly instead of
re-enqueuing them again in a channel, where they can accumulate and need
multiple iterations of the loop to be worked off.

This does *not* fix the problem of input feeling choppy/slower when the
system is under load, but it makes the behavior now feel exactly the
same as when XIM is disabled.

I also think the code is easier to understand since it's more
straightforward.

Release Notes:

- N/A

Change summary

crates/gpui/src/platform/linux/x11/client.rs      | 54 ++++++++--------
crates/gpui/src/platform/linux/x11/xim_handler.rs | 37 ++++-------
2 files changed, 42 insertions(+), 49 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/x11/client.rs 🔗 

@@ -5,7 +5,7 @@ use std::rc::{Rc, Weak};
 use std::time::{Duration, Instant};
 
 use calloop::generic::{FdWrapper, Generic};
-use calloop::{channel, EventLoop, LoopHandle, RegistrationToken};
+use calloop::{EventLoop, LoopHandle, RegistrationToken};
 
 use collections::HashMap;
 use copypasta::x11_clipboard::{Clipboard, Primary, X11ClipboardContext};
@@ -282,11 +282,9 @@ impl X11Client {
 
         let xcb_connection = Rc::new(xcb_connection);
 
-        let (xim_tx, xim_rx) = channel::channel::<XimCallbackEvent>();
-
         let ximc = X11rbClient::init(Rc::clone(&xcb_connection), x_root_index, None).ok();
         let xim_handler = if ximc.is_some() {
-            Some(XimHandler::new(xim_tx))
+            Some(XimHandler::new())
         } else {
             None
         };
@@ -311,10 +309,12 @@ impl X11Client {
                                 client.handle_event(event);
                                 continue;
                             }
+
                             let mut ximc = state.ximc.take().unwrap();
                             let mut xim_handler = state.xim_handler.take().unwrap();
                             let xim_connected = xim_handler.connected;
                             drop(state);
+
                             let xim_filtered = match ximc.filter_event(&event, &mut xim_handler) {
                                 Ok(handled) => handled,
                                 Err(err) => {
@@ -322,46 +322,34 @@ impl X11Client {
                                     false
                                 }
                             };
+                            let xim_callback_event = xim_handler.last_callback_event.take();
+
                             let mut state = client.0.borrow_mut();
                             state.ximc = Some(ximc);
                             state.xim_handler = Some(xim_handler);
                             drop(state);
+
+                            if let Some(event) = xim_callback_event {
+                                client.handle_xim_callback_event(event);
+                            }
+
                             if xim_filtered {
                                 continue;
                             }
+
                             if xim_connected {
                                 client.xim_handle_event(event);
                             } else {
                                 client.handle_event(event);
                             }
                         }
+
                         Ok(calloop::PostAction::Continue)
                     }
                 },
             )
             .expect("Failed to initialize x11 event source");
-        handle
-            .insert_source(xim_rx, {
-                move |chan_event, _, client| match chan_event {
-                    channel::Event::Msg(xim_event) => {
-                        match xim_event {
-                            XimCallbackEvent::XimXEvent(event) => {
-                                client.handle_event(event);
-                            }
-                            XimCallbackEvent::XimCommitEvent(window, text) => {
-                                client.xim_handle_commit(window, text);
-                            }
-                            XimCallbackEvent::XimPreeditEvent(window, text) => {
-                                client.xim_handle_preedit(window, text);
-                            }
-                        };
-                    }
-                    channel::Event::Closed => {
-                        log::error!("XIM Event Sender dropped")
-                    }
-                }
-            })
-            .expect("Failed to initialize XIM event source");
+
         handle
             .insert_source(XDPEventSource::new(&common.background_executor), {
                 move |event, _, client| match event {
@@ -801,6 +789,20 @@ impl X11Client {
         Some(())
     }
 
+    fn handle_xim_callback_event(&self, event: XimCallbackEvent) {
+        match event {
+            XimCallbackEvent::XimXEvent(event) => {
+                self.handle_event(event);
+            }
+            XimCallbackEvent::XimCommitEvent(window, text) => {
+                self.xim_handle_commit(window, text);
+            }
+            XimCallbackEvent::XimPreeditEvent(window, text) => {
+                self.xim_handle_preedit(window, text);
+            }
+        };
+    }
+
     fn xim_handle_event(&self, event: Event) -> Option<()> {
         match event {
             Event::KeyPress(event) | Event::KeyRelease(event) => {

crates/gpui/src/platform/linux/x11/xim_handler.rs 🔗 

@@ -1,7 +1,5 @@
 use std::default::Default;
 
-use calloop::channel;
-
 use x11rb::protocol::{xproto, Event};
 use xim::{AHashMap, AttributeName, Client, ClientError, ClientHandler, InputStyle};
 
@@ -14,19 +12,19 @@ pub enum XimCallbackEvent {
 pub struct XimHandler {
     pub im_id: u16,
     pub ic_id: u16,
-    pub xim_tx: channel::Sender<XimCallbackEvent>,
     pub connected: bool,
     pub window: xproto::Window,
+    pub last_callback_event: Option<XimCallbackEvent>,
 }
 
 impl XimHandler {
-    pub fn new(xim_tx: channel::Sender<XimCallbackEvent>) -> Self {
+    pub fn new() -> Self {
         Self {
             im_id: Default::default(),
             ic_id: Default::default(),
-            xim_tx,
             connected: false,
             window: Default::default(),
+            last_callback_event: None,
         }
     }
 }
@@ -80,12 +78,10 @@ impl<C: Client<XEvent = xproto::KeyPressEvent>> ClientHandler<C> for XimHandler
         _input_context_id: u16,
         text: &str,
     ) -> Result<(), ClientError> {
-        self.xim_tx
-            .send(XimCallbackEvent::XimCommitEvent(
-                self.window,
-                String::from(text),
-            ))
-            .ok();
+        self.last_callback_event = Some(XimCallbackEvent::XimCommitEvent(
+            self.window,
+            String::from(text),
+        ));
         Ok(())
     }
 
@@ -99,14 +95,11 @@ impl<C: Client<XEvent = xproto::KeyPressEvent>> ClientHandler<C> for XimHandler
     ) -> Result<(), ClientError> {
         match xev.response_type {
             x11rb::protocol::xproto::KEY_PRESS_EVENT => {
-                self.xim_tx
-                    .send(XimCallbackEvent::XimXEvent(Event::KeyPress(xev)))
-                    .ok();
+                self.last_callback_event = Some(XimCallbackEvent::XimXEvent(Event::KeyPress(xev)));
             }
             x11rb::protocol::xproto::KEY_RELEASE_EVENT => {
-                self.xim_tx
-                    .send(XimCallbackEvent::XimXEvent(Event::KeyRelease(xev)))
-                    .ok();
+                self.last_callback_event =
+                    Some(XimCallbackEvent::XimXEvent(Event::KeyRelease(xev)));
             }
             _ => {}
         }
@@ -145,12 +138,10 @@ impl<C: Client<XEvent = xproto::KeyPressEvent>> ClientHandler<C> for XimHandler
         // XIMPrimary, XIMHighlight, XIMSecondary, XIMTertiary are not specified,
         // but interchangeable as above
         // Currently there's no way to support these.
-        self.xim_tx
-            .send(XimCallbackEvent::XimPreeditEvent(
-                self.window,
-                String::from(preedit_string),
-            ))
-            .ok();
+        self.last_callback_event = Some(XimCallbackEvent::XimPreeditEvent(
+            self.window,
+            String::from(preedit_string),
+        ));
         Ok(())
     }
 }