x11: refactor cursor logic

src/x11/cursor.rs

@@ -1,34 +1,65 @@
-use std::os::raw::{c_ulong, c_char};
-use std::collections::HashMap;
+use std::os::raw::c_char;
 
 use crate::MouseCursor;
 
-pub fn set_cursor(
-    xcb_connection: &mut crate::x11::XcbConnection,
-    window_id: u32,
-    cursor_cache: &mut HashMap<MouseCursor, c_ulong>,
-    mouse_cursor: MouseCursor,
-) {
-    let display = xcb_connection.conn.get_raw_dpy();
+fn create_empty_cursor(display: *mut x11::xlib::Display) -> Option<u32> {
+    let data = 0;
+    let pixmap = unsafe {
+        let screen = x11::xlib::XDefaultScreen(display);
+        let window = x11::xlib::XRootWindow(display, screen);
+        x11::xlib::XCreateBitmapFromData(display, window, &data, 1, 1)
+    };
 
-    let cursor = *cursor_cache
-                .entry(mouse_cursor)
-                .or_insert_with(|| get_cursor(display, mouse_cursor));
+    if pixmap == 0 {
+        return None;
+    }
 
     unsafe {
-        if cursor != 0 {
-            x11::xlib::XDefineCursor(display, window_id as c_ulong, cursor);
-        }
-        x11::xlib::XFlush(display);
+        // We don't care about this color, since it only fills bytes
+        // in the pixmap which are not 0 in the mask.
+        let mut color: x11::xlib::XColor = std::mem::zeroed();
+
+        let cursor = x11::xlib::XCreatePixmapCursor(
+            display,
+            pixmap,
+            pixmap,
+            &mut color as *mut _,
+            &mut color as *mut _,
+            0,
+            0,
+        );
+        x11::xlib::XFreePixmap(display, pixmap);
+
+        Some(cursor as u32)
     }
 }
 
-fn get_cursor(display: *mut x11::xlib::Display, cursor: MouseCursor) -> c_ulong {
+fn load_cursor(display: *mut x11::xlib::Display, name: &[u8]) -> Option<u32> {
+    let xcursor = unsafe {
+        x11::xcursor::XcursorLibraryLoadCursor(display, name.as_ptr() as *const c_char)
+    };
+
+    if xcursor == 0 {
+        None
+    } else {
+        Some(xcursor as u32)
+    }
+}
+
+fn load_first_existing_cursor(display: *mut x11::xlib::Display, names: &[&[u8]]) -> Option<u32> {
+    names.iter()
+        .map(|name| load_cursor(display, name))
+        .find(|xcursor| xcursor.is_some())
+        .unwrap_or(None)
+}
+
+pub(super) fn get_xcursor(display: *mut x11::xlib::Display, cursor: MouseCursor) -> u32 {
     let load = |name: &[u8]| load_cursor(display, name);
     let loadn = |names: &[&[u8]]| load_first_existing_cursor(display, names);
 
-    let mut cursor = match cursor {
-        MouseCursor::Default => load(b"left_ptr\0"),
+    let cursor = match cursor {
+        MouseCursor::Default => None, // catch this in the fallback case below
+
         MouseCursor::Hand => loadn(&[b"hand2\0", b"hand1\0"]),
         MouseCursor::HandGrabbing => loadn(&[b"closedhand\0", b"grabbing\0"]),
         MouseCursor::Help => load(b"question_arrow\0"),
@@ -70,56 +101,7 @@ fn get_cursor(display: *mut x11::xlib::Display, cursor: MouseCursor) -> c_ulong
         MouseCursor::RowResize => loadn(&[b"split_v\0", b"v_double_arrow\0"]),
     };
 
-    if cursor == 0 {
-        cursor = load(b"left_ptr\0")
-    }
-
     cursor
+        .or(load(b"left_ptr\0"))
+        .unwrap_or(0)
 }
-
-fn load_cursor(display: *mut x11::xlib::Display, name: &[u8]) -> c_ulong {
-    unsafe {
-        x11::xcursor::XcursorLibraryLoadCursor(display, name.as_ptr() as *const c_char)
-    }
-}
-
-fn load_first_existing_cursor(display: *mut x11::xlib::Display, names: &[&[u8]]) -> c_ulong {
-    for name in names.iter() {
-        let xcursor = load_cursor(display, name);
-        if xcursor != 0 {
-            return xcursor;
-        }
-    }
-    0
-}
-
-fn create_empty_cursor(display: *mut x11::xlib::Display,) -> c_ulong {
-    let data = 0;
-    let pixmap = unsafe {
-        let screen = x11::xlib::XDefaultScreen(display);
-        let window = x11::xlib::XRootWindow(display, screen);
-        x11::xlib::XCreateBitmapFromData(display, window, &data, 1, 1)
-    };
-
-    if pixmap == 0 {
-        panic!("failed to allocate pixmap for cursor");
-    }
-
-    unsafe {
-        // We don't care about this color, since it only fills bytes
-        // in the pixmap which are not 0 in the mask.
-        let mut dummy_color = maybe_uninit::MaybeUninit::uninit();
-        let cursor = x11::xlib::XCreatePixmapCursor(
-            display,
-            pixmap,
-            pixmap,
-            dummy_color.as_mut_ptr(),
-            dummy_color.as_mut_ptr(),
-            0,
-            0,
-        );
-        x11::xlib::XFreePixmap(display, pixmap);
-
-        cursor
-    }
-}

src/x11/window.rs

@@ -2,7 +2,6 @@ use std::os::raw::{c_ulong, c_void};
 use std::sync::mpsc;
 use std::time::*;
 use std::thread;
-use std::collections::HashMap;
 
 use raw_window_handle::{
     unix::XlibHandle,
@@ -21,7 +20,6 @@ pub struct Window {
     window_id: u32,
     window_info: WindowInfo,
     mouse_cursor: MouseCursor,
-    cursor_cache: HashMap<MouseCursor, c_ulong>,
 
     frame_interval: Duration,
     event_loop_running: bool,
@@ -159,7 +157,6 @@ impl Window {
             window_id,
             window_info,
             mouse_cursor: MouseCursor::default(),
-            cursor_cache: HashMap::new(),
 
             frame_interval: Duration::from_millis(15),
             event_loop_running: false,
@@ -180,16 +177,23 @@ impl Window {
     }
 
     pub fn set_mouse_cursor(&mut self, mouse_cursor: MouseCursor) {
-        if self.mouse_cursor != mouse_cursor {
-            crate::x11::cursor::set_cursor(
-                &mut self.xcb_connection,
+        if self.mouse_cursor == mouse_cursor {
+            return
+        }
+
+        let xid = self.xcb_connection.get_cursor_xid(mouse_cursor);
+
+        if xid != 0 {
+            xcb::change_window_attributes(
+                &self.xcb_connection.conn,
                 self.window_id,
-                &mut self.cursor_cache,
-                mouse_cursor
+                &[(xcb::CW_CURSOR, xid)]
             );
 
-            self.mouse_cursor = mouse_cursor;
+            self.xcb_connection.conn.flush();
         }
+
+        self.mouse_cursor = mouse_cursor;
     }
 
     #[inline]
@@ -418,4 +422,4 @@ fn mouse_id(id: u8) -> MouseButton {
         7 => MouseButton::Forward,
         id => MouseButton::Other(id),
     }
-}
+}

src/x11/xcb_connection.rs

@@ -1,7 +1,14 @@
 /// A very light abstraction around the XCB connection.
 ///
 /// Keeps track of the xcb connection itself and the xlib display ID that was used to connect.
+
 use std::ffi::{CStr, CString};
+use std::collections::HashMap;
+
+use crate::MouseCursor;
+
+use super::cursor;
+
 
 pub(crate) struct Atoms {
     pub wm_protocols: Option<u32>,
@@ -13,6 +20,8 @@ pub struct XcbConnection {
     pub xlib_display: i32,
 
     pub(crate) atoms: Atoms,
+
+    pub(super) cursor_cache: HashMap<MouseCursor, u32>,
 }
 
 macro_rules! intern_atoms {
@@ -46,6 +55,8 @@ impl XcbConnection {
                 wm_protocols,
                 wm_delete_window,
             },
+
+            cursor_cache: HashMap::new()
         })
     }
 
@@ -132,4 +143,13 @@ impl XcbConnection {
         self.get_scaling_xft()
             .or(self.get_scaling_screen_dimensions())
     }
+
+    #[inline]
+    pub fn get_cursor_xid(&mut self, cursor: MouseCursor) -> u32 {
+        let dpy = self.conn.get_raw_dpy();
+
+        *self.cursor_cache
+            .entry(cursor)
+            .or_insert_with(|| cursor::get_xcursor(dpy, cursor))
+    }
 }