From 9b4e88f83c342020916331409f545892b0411b18 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Thu, 22 Aug 2024 19:25:20 -0700 Subject: [PATCH] m: Replace libxcursor with custom cursor code Another one bites the dust. This replaces the code dependent on libxcursor with equivalent code written using x11rb, featuring its special "cursor" module. cc #3198 Signed-off-by: John Nunley --- Cargo.toml | 1 + src/platform_impl/linux/x11/ffi.rs | 1 - src/platform_impl/linux/x11/mod.rs | 6 + src/platform_impl/linux/x11/util/cursor.rs | 232 +++++++++++++-------- src/platform_impl/linux/x11/window.rs | 22 +- src/platform_impl/linux/x11/xdisplay.rs | 35 +++- 6 files changed, 201 insertions(+), 96 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index da027b479d..ec463a3e03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -253,6 +253,7 @@ wayland-protocols-plasma = { version = "0.3.2", features = [ x11-dl = { version = "2.19.1", optional = true } x11rb = { version = "0.13.0", default-features = false, features = [ "allow-unsafe-code", + "cursor", "dl-libxcb", "randr", "resource_manager", diff --git a/src/platform_impl/linux/x11/ffi.rs b/src/platform_impl/linux/x11/ffi.rs index 57bd78e95d..6895f19238 100644 --- a/src/platform_impl/linux/x11/ffi.rs +++ b/src/platform_impl/linux/x11/ffi.rs @@ -1,5 +1,4 @@ pub use x11_dl::error::OpenError; -pub use x11_dl::xcursor::*; pub use x11_dl::xinput2::*; pub use x11_dl::xlib::*; pub use x11_dl::xlib_xcb::*; diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index 04cee98625..30634421cd 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -838,6 +838,9 @@ pub enum X11Error { /// Failed to get property. GetProperty(util::GetPropertyError), + + /// Could not find an ARGB32 pict format. + NoArgb32Format, } impl fmt::Display for X11Error { @@ -862,6 +865,9 @@ impl fmt::Display for X11Error { X11Error::XsettingsParse(err) => { write!(f, "Failed to parse xsettings: {:?}", err) }, + X11Error::NoArgb32Format => { + f.write_str("winit only supports X11 displays with ARGB32 picture formats") + }, } } } diff --git a/src/platform_impl/linux/x11/util/cursor.rs b/src/platform_impl/linux/x11/util/cursor.rs index 169748d14d..21d078d178 100644 --- a/src/platform_impl/linux/x11/util/cursor.rs +++ b/src/platform_impl/linux/x11/util/cursor.rs @@ -1,9 +1,11 @@ -use std::ffi::CString; +use std::collections::hash_map::Entry; use std::hash::{Hash, Hasher}; +use std::iter; use std::sync::Arc; -use std::{iter, slice}; use x11rb::connection::Connection; +use x11rb::protocol::render::{self, ConnectionExt as _}; +use x11rb::protocol::xproto; use crate::platform_impl::PlatformCustomCursorSource; use crate::window::CursorIcon; @@ -12,81 +14,148 @@ use super::super::ActiveEventLoop; use super::*; impl XConnection { - pub fn set_cursor_icon(&self, window: xproto::Window, cursor: Option) { - let cursor = *self - .cursor_cache - .lock() - .unwrap() - .entry(cursor) - .or_insert_with(|| self.get_cursor(cursor)); - - self.update_cursor(window, cursor).expect("Failed to set cursor"); - } + pub fn set_cursor_icon( + &self, + window: xproto::Window, + cursor: Option, + ) -> Result<(), X11Error> { + let cursor = { + let mut cache = self.cursor_cache.lock().unwrap_or_else(|e| e.into_inner()); + + match cache.entry(cursor) { + Entry::Occupied(o) => *o.get(), + Entry::Vacant(v) => *v.insert(self.get_cursor(cursor)?), + } + }; - pub(crate) fn set_custom_cursor(&self, window: xproto::Window, cursor: &CustomCursor) { - self.update_cursor(window, cursor.inner.cursor).expect("Failed to set cursor"); + self.update_cursor(window, cursor) } - fn create_empty_cursor(&self) -> ffi::Cursor { - let data = 0; - let pixmap = unsafe { - let screen = (self.xlib.XDefaultScreen)(self.display); - let window = (self.xlib.XRootWindow)(self.display, screen); - (self.xlib.XCreateBitmapFromData)(self.display, window, &data, 1, 1) - }; - - if pixmap == 0 { - panic!("failed to allocate pixmap for cursor"); - } + pub(crate) fn set_custom_cursor( + &self, + window: xproto::Window, + cursor: &CustomCursor, + ) -> Result<(), X11Error> { + self.update_cursor(window, cursor.inner.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 = MaybeUninit::uninit(); - let cursor = (self.xlib.XCreatePixmapCursor)( - self.display, - pixmap, - pixmap, - dummy_color.as_mut_ptr(), - dummy_color.as_mut_ptr(), + /// Create a cursor from an image. + fn create_cursor_from_image( + &self, + width: u16, + height: u16, + hotspot_x: u16, + hotspot_y: u16, + image: &[u8], + ) -> Result { + // Create a pixmap for the default root window. + let root = self.default_root().root; + let pixmap = + xproto::PixmapWrapper::create_pixmap(self.xcb_connection(), 32, root, width, height)?; + + // Create a GC to draw with. + let gc = xproto::GcontextWrapper::create_gc( + self.xcb_connection(), + pixmap.pixmap(), + &Default::default(), + )?; + + // Draw the data into it. + self.xcb_connection() + .put_image( + xproto::ImageFormat::Z_PIXMAP, + pixmap.pixmap(), + gc.gcontext(), + width, + height, + 0, 0, 0, - ); - (self.xlib.XFreePixmap)(self.display, pixmap); + 32, + image, + )? + .ignore_error(); + drop(gc); + + // Create the XRender picture. + let picture = render::PictureWrapper::create_picture( + self.xcb_connection(), + pixmap.pixmap(), + self.find_argb32_format()?, + &Default::default(), + )?; + drop(pixmap); + + // Create the cursor. + let cursor = self.xcb_connection().generate_id()?; + self.xcb_connection() + .render_create_cursor(cursor, picture.picture(), hotspot_x, hotspot_y)? + .check()?; + + Ok(cursor) + } - cursor + /// Find the render format that corresponds to ARGB32. + fn find_argb32_format(&self) -> Result { + macro_rules! direct { + ($format:expr, $shift_name:ident, $mask_name:ident, $shift:expr) => {{ + ($format).direct.$shift_name == $shift && ($format).direct.$mask_name == 0xff + }}; } + + self.render_formats() + .formats + .iter() + .find(|format| { + format.type_ == render::PictType::DIRECT + && format.depth == 32 + && direct!(format, red_shift, red_mask, 16) + && direct!(format, green_shift, green_mask, 8) + && direct!(format, blue_shift, blue_mask, 0) + && direct!(format, alpha_shift, alpha_mask, 24) + }) + .ok_or(X11Error::NoArgb32Format) + .map(|format| format.id) + } + + fn create_empty_cursor(&self) -> Result { + self.create_cursor_from_image(1, 1, 0, 0, &[0, 0, 0, 0]) } - fn get_cursor(&self, cursor: Option) -> ffi::Cursor { + fn get_cursor(&self, cursor: Option) -> Result { let cursor = match cursor { Some(cursor) => cursor, None => return self.create_empty_cursor(), }; - let mut xcursor = 0; + let database = self.database(); + let handle = x11rb::cursor::Handle::new( + self.xcb_connection(), + self.default_screen_index(), + &database, + )? + .reply()?; + + let mut last_error = None; for &name in iter::once(&cursor.name()).chain(cursor.alt_names().iter()) { - let name = CString::new(name).unwrap(); - xcursor = unsafe { - (self.xcursor.XcursorLibraryLoadCursor)( - self.display, - name.as_ptr() as *const c_char, - ) - }; - - if xcursor != 0 { - break; + match handle.load_cursor(self.xcb_connection(), name) { + Ok(cursor) => return Ok(cursor), + Err(err) => last_error = Some(err.into()), } } - xcursor + Err(last_error.unwrap()) } - fn update_cursor(&self, window: xproto::Window, cursor: ffi::Cursor) -> Result<(), X11Error> { + fn update_cursor( + &self, + window: xproto::Window, + cursor: xproto::Cursor, + ) -> Result<(), X11Error> { self.xcb_connection() .change_window_attributes( window, - &xproto::ChangeWindowAttributesAux::new().cursor(cursor as xproto::Cursor), + &xproto::ChangeWindowAttributesAux::new().cursor(cursor), )? .ignore_error(); @@ -123,47 +192,44 @@ impl Eq for CustomCursor {} impl CustomCursor { pub(crate) fn new( event_loop: &ActiveEventLoop, - cursor: PlatformCustomCursorSource, - ) -> CustomCursor { - unsafe { - let ximage = (event_loop.xconn.xcursor.XcursorImageCreate)( - cursor.0.width as i32, - cursor.0.height as i32, - ); - if ximage.is_null() { - panic!("failed to allocate cursor image"); - } - (*ximage).xhot = cursor.0.hotspot_x as u32; - (*ximage).yhot = cursor.0.hotspot_y as u32; - (*ximage).delay = 0; - - let dst = slice::from_raw_parts_mut((*ximage).pixels, cursor.0.rgba.len() / 4); - for (dst, chunk) in dst.iter_mut().zip(cursor.0.rgba.chunks_exact(4)) { - *dst = (chunk[0] as u32) << 16 - | (chunk[1] as u32) << 8 - | (chunk[2] as u32) - | (chunk[3] as u32) << 24; + mut cursor: PlatformCustomCursorSource, + ) -> Result { + // Reverse RGBA order to BGRA. + cursor.0.rgba.chunks_mut(4).for_each(|chunk| { + let chunk: &mut [u8; 4] = chunk.try_into().unwrap(); + chunk[0..3].reverse(); + + // Byteswap if we need to. + if event_loop.xconn.needs_endian_swap() { + let value = u32::from_ne_bytes(*chunk).swap_bytes(); + *chunk = value.to_ne_bytes(); } - - let cursor = - (event_loop.xconn.xcursor.XcursorImageLoadCursor)(event_loop.xconn.display, ximage); - (event_loop.xconn.xcursor.XcursorImageDestroy)(ximage); - Self { inner: Arc::new(CustomCursorInner { xconn: event_loop.xconn.clone(), cursor }) } - } + }); + + let cursor = event_loop + .xconn + .create_cursor_from_image( + cursor.0.width, + cursor.0.height, + cursor.0.hotspot_x, + cursor.0.hotspot_y, + &cursor.0.rgba, + ) + .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err.into()))))?; + + Ok(Self { inner: Arc::new(CustomCursorInner { xconn: event_loop.xconn.clone(), cursor }) }) } } #[derive(Debug)] struct CustomCursorInner { xconn: Arc, - cursor: ffi::Cursor, + cursor: xproto::Cursor, } impl Drop for CustomCursorInner { fn drop(&mut self) { - unsafe { - (self.xconn.xlib.XFreeCursor)(self.xconn.display, self.cursor); - } + self.xconn.xcb_connection().free_cursor(self.cursor).map(|r| r.ignore_error()).ok(); } } diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index 8c2c349633..234c013de2 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -1466,13 +1466,17 @@ impl UnownedWindow { #[allow(clippy::mutex_atomic)] if SelectedCursor::Named(icon) != old_cursor && *self.cursor_visible.lock().unwrap() { - self.xconn.set_cursor_icon(self.xwindow, Some(icon)); + if let Err(err) = self.xconn.set_cursor_icon(self.xwindow, Some(icon)) { + tracing::error!("failed to set cursor icon: {err}"); + } } }, Cursor::Custom(RootCustomCursor { inner: PlatformCustomCursor::X(cursor) }) => { #[allow(clippy::mutex_atomic)] if *self.cursor_visible.lock().unwrap() { - self.xconn.set_custom_cursor(self.xwindow, &cursor); + if let Err(err) = self.xconn.set_custom_cursor(self.xwindow, &cursor) { + tracing::error!("failed to set window icon: {err}"); + } } *self.selected_cursor.lock().unwrap() = SelectedCursor::Custom(cursor); @@ -1573,16 +1577,18 @@ impl UnownedWindow { if visible { Some((*self.selected_cursor.lock().unwrap()).clone()) } else { None }; *visible_lock = visible; drop(visible_lock); - match cursor { + let result = match cursor { Some(SelectedCursor::Custom(cursor)) => { - self.xconn.set_custom_cursor(self.xwindow, &cursor); + self.xconn.set_custom_cursor(self.xwindow, &cursor) }, Some(SelectedCursor::Named(cursor)) => { - self.xconn.set_cursor_icon(self.xwindow, Some(cursor)); - }, - None => { - self.xconn.set_cursor_icon(self.xwindow, None); + self.xconn.set_cursor_icon(self.xwindow, Some(cursor)) }, + None => self.xconn.set_cursor_icon(self.xwindow, None), + }; + + if let Err(err) = result { + tracing::error!("failed to set cursor icon: {err}"); } } diff --git a/src/platform_impl/linux/x11/xdisplay.rs b/src/platform_impl/linux/x11/xdisplay.rs index 0a56f290a4..f26de0db1e 100644 --- a/src/platform_impl/linux/x11/xdisplay.rs +++ b/src/platform_impl/linux/x11/xdisplay.rs @@ -11,6 +11,7 @@ use super::ffi; use super::monitor::MonitorHandle; use x11rb::connection::Connection; use x11rb::protocol::randr::ConnectionExt as _; +use x11rb::protocol::render; use x11rb::protocol::xproto::{self, ConnectionExt}; use x11rb::resource_manager; use x11rb::xcb_ffi::XCBConnection; @@ -18,7 +19,6 @@ use x11rb::xcb_ffi::XCBConnection; /// A connection to an X server. pub struct XConnection { pub xlib: ffi::Xlib, - pub xcursor: ffi::Xcursor, // TODO(notgull): I'd like to remove this, but apparently Xlib and Xinput2 are tied together // for some reason. @@ -55,8 +55,11 @@ pub struct XConnection { /// Atom for the XSettings screen. xsettings_screen: Option, + /// XRender format information. + render_formats: render::QueryPictFormatsReply, + pub latest_error: Mutex>, - pub cursor_cache: Mutex, ffi::Cursor>>, + pub cursor_cache: Mutex, xproto::Cursor>>, } unsafe impl Send for XConnection {} @@ -69,7 +72,6 @@ impl XConnection { pub fn new(error_handler: XErrorHandler) -> Result { // opening the libraries let xlib = ffi::Xlib::open()?; - let xcursor = ffi::Xcursor::open()?; let xlib_xcb = ffi::Xlib_xcb::open()?; let xinput2 = ffi::XInput2::open()?; @@ -118,15 +120,22 @@ impl XConnection { tracing::warn!("error setting XSETTINGS; Xft options won't reload automatically") } + // Start getting the XRender formats. + let formats_cookie = render::query_pict_formats(&xcb) + .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; + // Fetch atoms. let atoms = Atoms::new(&xcb) .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? .reply() .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; + // Finish getting everything else. + let formats = + formats_cookie.reply().map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; + Ok(XConnection { xlib, - xcursor, xinput2, display, xcb: Some(xcb), @@ -138,6 +147,7 @@ impl XConnection { database: RwLock::new(database), cursor_cache: Default::default(), randr_version: (randr_version.major_version, randr_version.minor_version), + render_formats: formats, xsettings_screen, }) } @@ -257,6 +267,23 @@ impl XConnection { pub fn xsettings_screen(&self) -> Option { self.xsettings_screen } + + /// Get the data containing our rendering formats. + #[inline] + pub fn render_formats(&self) -> &render::QueryPictFormatsReply { + &self.render_formats + } + + /// Do we need to do an endian swap? + #[inline] + pub fn needs_endian_swap(&self) -> bool { + #[cfg(target_endian = "big")] + let endian = xproto::ImageOrder::MSB_FIRST; + #[cfg(not(target_endian = "big"))] + let endian = xproto::ImageOrder::LSB_FIRST; + + self.xcb_connection().setup().image_byte_order != endian + } } impl fmt::Debug for XConnection {