From 4e8fc9e91281adf98c9bc6aa3b39c84a21669300 Mon Sep 17 00:00:00 2001 From: jtnunley Date: Sun, 28 May 2023 16:15:40 -0700 Subject: [PATCH 1/3] Replace some parts of the Xlib impl w/ x11rb --- Cargo.toml | 4 +- src/platform_impl/linux/mod.rs | 6 +- src/platform_impl/linux/x11/atoms.rs | 110 ++ src/platform_impl/linux/x11/dnd.rs | 142 +-- .../linux/x11/event_processor.rs | 138 ++- src/platform_impl/linux/x11/ime/context.rs | 4 +- .../linux/x11/ime/input_method.rs | 24 +- src/platform_impl/linux/x11/mod.rs | 206 +++- src/platform_impl/linux/x11/monitor.rs | 14 +- src/platform_impl/linux/x11/util/atom.rs | 70 -- .../linux/x11/util/client_msg.rs | 59 +- src/platform_impl/linux/x11/util/cursor.rs | 21 +- src/platform_impl/linux/x11/util/format.rs | 55 - src/platform_impl/linux/x11/util/geometry.rs | 162 +-- src/platform_impl/linux/x11/util/hint.rs | 243 +--- src/platform_impl/linux/x11/util/input.rs | 100 +- src/platform_impl/linux/x11/util/memory.rs | 17 - src/platform_impl/linux/x11/util/mod.rs | 37 +- src/platform_impl/linux/x11/util/modifiers.rs | 4 +- src/platform_impl/linux/x11/util/randr.rs | 15 +- .../linux/x11/util/window_property.rs | 250 ++-- src/platform_impl/linux/x11/util/wm.rs | 53 +- src/platform_impl/linux/x11/window.rs | 1037 ++++++++++------- src/platform_impl/linux/x11/xdisplay.rs | 106 +- 24 files changed, 1517 insertions(+), 1360 deletions(-) create mode 100644 src/platform_impl/linux/x11/atoms.rs delete mode 100644 src/platform_impl/linux/x11/util/atom.rs delete mode 100644 src/platform_impl/linux/x11/util/format.rs diff --git a/Cargo.toml b/Cargo.toml index 46b3fcf74f..5c5b94720b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,7 +36,7 @@ rustdoc-args = ["--cfg", "docsrs"] [features] default = ["x11", "wayland", "wayland-dlopen", "wayland-csd-adwaita"] -x11 = ["x11-dl", "percent-encoding", "xkbcommon-dl/x11"] +x11 = ["x11-dl", "bytemuck", "percent-encoding", "xkbcommon-dl/x11", "x11rb"] wayland = ["wayland-client", "wayland-backend", "wayland-protocols", "sctk", "fnv", "memmap2"] wayland-dlopen = ["wayland-backend/dlopen"] wayland-csd-adwaita = ["sctk-adwaita", "sctk-adwaita/ab_glyph"] @@ -113,6 +113,7 @@ features = [ ] [target.'cfg(all(unix, not(any(target_os = "redox", target_family = "wasm", target_os = "android", target_os = "ios", target_os = "macos"))))'.dependencies] +bytemuck = { version = "1.13.1", default-features = false, optional = true } libc = "0.2.64" percent-encoding = { version = "2.0", optional = true } fnv = { version = "1.0.3", optional = true } @@ -123,6 +124,7 @@ wayland-backend = { version = "0.1.0", default_features = false, features = ["cl wayland-protocols = { version = "0.30.0", features = [ "staging"], optional = true } calloop = "0.10.5" x11-dl = { version = "2.18.5", optional = true } +x11rb = { version = "0.12.0", default-features = false, features = ["allow-unsafe-code", "dl-libxcb", "xinput", "xkb"], optional = true } xkbcommon-dl = "0.4.0" memmap2 = { version = "0.5.0", optional = true } diff --git a/src/platform_impl/linux/mod.rs b/src/platform_impl/linux/mod.rs index 24cf135ce3..d82721616f 100644 --- a/src/platform_impl/linux/mod.rs +++ b/src/platform_impl/linux/mod.rs @@ -23,7 +23,7 @@ use smol_str::SmolStr; #[cfg(x11_platform)] pub use self::x11::XNotSupported; #[cfg(x11_platform)] -use self::x11::{ffi::XVisualInfo, util::WindowType as XWindowType, XConnection, XError}; +use self::x11::{ffi::XVisualInfo, util::WindowType as XWindowType, X11Error, XConnection, XError}; #[cfg(x11_platform)] use crate::platform::x11::XlibErrorHook; use crate::{ @@ -124,7 +124,7 @@ pub(crate) static X11_BACKEND: Lazy, XNotSupported #[derive(Debug, Clone)] pub enum OsError { #[cfg(x11_platform)] - XError(XError), + XError(Arc), #[cfg(x11_platform)] XMisc(&'static str), #[cfg(wayland_platform)] @@ -135,7 +135,7 @@ impl fmt::Display for OsError { fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { match *self { #[cfg(x11_platform)] - OsError::XError(ref e) => _f.pad(&e.description), + OsError::XError(ref e) => fmt::Display::fmt(e, _f), #[cfg(x11_platform)] OsError::XMisc(e) => _f.pad(e), #[cfg(wayland_platform)] diff --git a/src/platform_impl/linux/x11/atoms.rs b/src/platform_impl/linux/x11/atoms.rs new file mode 100644 index 0000000000..662a0e619b --- /dev/null +++ b/src/platform_impl/linux/x11/atoms.rs @@ -0,0 +1,110 @@ +//! Collects every atom used by the platform implementation. + +use core::ops::Index; + +macro_rules! atom_manager { + ($($name:ident $(:$lit:literal)?),*) => { + x11rb::atom_manager! { + /// The atoms used by `winit` + pub(crate) Atoms: AtomsCookie { + $($name $(:$lit)?,)* + } + } + + /// Indices into the `Atoms` struct. + #[derive(Copy, Clone, Debug)] + #[allow(non_camel_case_types)] + pub(crate) enum AtomName { + $($name,)* + } + + impl AtomName { + pub(crate) fn atom_from( + self, + atoms: &Atoms + ) -> &x11rb::protocol::xproto::Atom { + match self { + $(AtomName::$name => &atoms.$name,)* + } + } + } + }; +} + +atom_manager! { + // General Use Atoms + CARD32, + UTF8_STRING, + WM_CHANGE_STATE, + WM_CLIENT_MACHINE, + WM_DELETE_WINDOW, + WM_PROTOCOLS, + WM_STATE, + XIM_SERVERS, + + // Assorted ICCCM Atoms + _NET_WM_ICON, + _NET_WM_MOVERESIZE, + _NET_WM_NAME, + _NET_WM_PID, + _NET_WM_PING, + _NET_WM_STATE, + _NET_WM_STATE_ABOVE, + _NET_WM_STATE_BELOW, + _NET_WM_STATE_FULLSCREEN, + _NET_WM_STATE_HIDDEN, + _NET_WM_STATE_MAXIMIZED_HORZ, + _NET_WM_STATE_MAXIMIZED_VERT, + _NET_WM_WINDOW_TYPE, + + // WM window types. + _NET_WM_WINDOW_TYPE_DESKTOP, + _NET_WM_WINDOW_TYPE_DOCK, + _NET_WM_WINDOW_TYPE_TOOLBAR, + _NET_WM_WINDOW_TYPE_MENU, + _NET_WM_WINDOW_TYPE_UTILITY, + _NET_WM_WINDOW_TYPE_SPLASH, + _NET_WM_WINDOW_TYPE_DIALOG, + _NET_WM_WINDOW_TYPE_DROPDOWN_MENU, + _NET_WM_WINDOW_TYPE_POPUP_MENU, + _NET_WM_WINDOW_TYPE_TOOLTIP, + _NET_WM_WINDOW_TYPE_NOTIFICATION, + _NET_WM_WINDOW_TYPE_COMBO, + _NET_WM_WINDOW_TYPE_DND, + _NET_WM_WINDOW_TYPE_NORMAL, + + // Drag-N-Drop Atoms + XdndAware, + XdndEnter, + XdndLeave, + XdndDrop, + XdndPosition, + XdndStatus, + XdndActionPrivate, + XdndSelection, + XdndFinished, + XdndTypeList, + TextUriList: b"text/uri-list", + None: b"None", + + // Miscellaneous Atoms + _GTK_THEME_VARIANT, + _MOTIF_WM_HINTS, + _NET_ACTIVE_WINDOW, + _NET_CLIENT_LIST, + _NET_FRAME_EXTENTS, + _NET_SUPPORTED, + _NET_SUPPORTING_WM_CHECK +} + +impl Index for Atoms { + type Output = x11rb::protocol::xproto::Atom; + + fn index(&self, index: AtomName) -> &Self::Output { + index.atom_from(self) + } +} + +pub(crate) use AtomName::*; +// Make sure `None` is still defined. +pub(crate) use core::option::Option::None; diff --git a/src/platform_impl/linux/x11/dnd.rs b/src/platform_impl/linux/x11/dnd.rs index 198494e184..618a4cde9d 100644 --- a/src/platform_impl/linux/x11/dnd.rs +++ b/src/platform_impl/linux/x11/dnd.rs @@ -7,55 +7,12 @@ use std::{ }; use percent_encoding::percent_decode; +use x11rb::protocol::xproto::{self, ConnectionExt}; -use super::{ffi, util, XConnection, XError}; - -#[derive(Debug)] -pub(crate) struct DndAtoms { - pub enter: ffi::Atom, - pub leave: ffi::Atom, - pub drop: ffi::Atom, - pub position: ffi::Atom, - pub status: ffi::Atom, - pub action_private: ffi::Atom, - pub selection: ffi::Atom, - pub finished: ffi::Atom, - pub type_list: ffi::Atom, - pub uri_list: ffi::Atom, - pub none: ffi::Atom, -} - -impl DndAtoms { - pub fn new(xconn: &Arc) -> Result { - let names = [ - b"XdndEnter\0".as_ptr() as *mut c_char, - b"XdndLeave\0".as_ptr() as *mut c_char, - b"XdndDrop\0".as_ptr() as *mut c_char, - b"XdndPosition\0".as_ptr() as *mut c_char, - b"XdndStatus\0".as_ptr() as *mut c_char, - b"XdndActionPrivate\0".as_ptr() as *mut c_char, - b"XdndSelection\0".as_ptr() as *mut c_char, - b"XdndFinished\0".as_ptr() as *mut c_char, - b"XdndTypeList\0".as_ptr() as *mut c_char, - b"text/uri-list\0".as_ptr() as *mut c_char, - b"None\0".as_ptr() as *mut c_char, - ]; - let atoms = unsafe { xconn.get_atoms(&names) }?; - Ok(DndAtoms { - enter: atoms[0], - leave: atoms[1], - drop: atoms[2], - position: atoms[3], - status: atoms[4], - action_private: atoms[5], - selection: atoms[6], - finished: atoms[7], - type_list: atoms[8], - uri_list: atoms[9], - none: atoms[10], - }) - } -} +use super::{ + atoms::{AtomName::None as DndNone, *}, + util, CookieResultExt, X11Error, XConnection, +}; #[derive(Debug, Clone, Copy)] pub enum DndState { @@ -86,22 +43,19 @@ impl From for DndDataParseError { pub(crate) struct Dnd { xconn: Arc, - pub atoms: DndAtoms, // Populated by XdndEnter event handler pub version: Option, - pub type_list: Option>, + pub type_list: Option>, // Populated by XdndPosition event handler - pub source_window: Option, + pub source_window: Option, // Populated by SelectionNotify event handler (triggered by XdndPosition event handler) pub result: Option, DndDataParseError>>, } impl Dnd { - pub fn new(xconn: Arc) -> Result { - let atoms = DndAtoms::new(&xconn)?; + pub fn new(xconn: Arc) -> Result { Ok(Dnd { xconn, - atoms, version: None, type_list: None, source_window: None, @@ -118,71 +72,85 @@ impl Dnd { pub unsafe fn send_status( &self, - this_window: c_ulong, - target_window: c_ulong, + this_window: xproto::Window, + target_window: xproto::Window, state: DndState, - ) -> Result<(), XError> { + ) -> Result<(), X11Error> { + let atoms = self.xconn.atoms(); let (accepted, action) = match state { - DndState::Accepted => (1, self.atoms.action_private as c_long), - DndState::Rejected => (0, self.atoms.none as c_long), + DndState::Accepted => (1, atoms[XdndActionPrivate]), + DndState::Rejected => (0, atoms[DndNone]), }; self.xconn .send_client_msg( target_window, target_window, - self.atoms.status, + atoms[XdndStatus] as _, None, - [this_window as c_long, accepted, 0, 0, action], - ) - .flush() + [this_window, accepted, 0, 0, action as _], + )? + .ignore_error(); + + Ok(()) } pub unsafe fn send_finished( &self, - this_window: c_ulong, - target_window: c_ulong, + this_window: xproto::Window, + target_window: xproto::Window, state: DndState, - ) -> Result<(), XError> { + ) -> Result<(), X11Error> { + let atoms = self.xconn.atoms(); let (accepted, action) = match state { - DndState::Accepted => (1, self.atoms.action_private as c_long), - DndState::Rejected => (0, self.atoms.none as c_long), + DndState::Accepted => (1, atoms[XdndActionPrivate]), + DndState::Rejected => (0, atoms[DndNone]), }; self.xconn .send_client_msg( target_window, target_window, - self.atoms.finished, + atoms[XdndFinished] as _, None, - [this_window as c_long, accepted, action, 0, 0], - ) - .flush() + [this_window, accepted, action as _, 0, 0], + )? + .ignore_error(); + + Ok(()) } pub unsafe fn get_type_list( &self, - source_window: c_ulong, - ) -> Result, util::GetPropertyError> { - self.xconn - .get_property(source_window, self.atoms.type_list, ffi::XA_ATOM) + source_window: xproto::Window, + ) -> Result, util::GetPropertyError> { + let atoms = self.xconn.atoms(); + self.xconn.get_property( + source_window, + atoms[XdndTypeList], + xproto::Atom::from(xproto::AtomEnum::ATOM), + ) } - pub unsafe fn convert_selection(&self, window: c_ulong, time: c_ulong) { - (self.xconn.xlib.XConvertSelection)( - self.xconn.display, - self.atoms.selection, - self.atoms.uri_list, - self.atoms.selection, - window, - time, - ); + pub unsafe fn convert_selection(&self, window: xproto::Window, time: xproto::Timestamp) { + let atoms = self.xconn.atoms(); + self.xconn + .xcb_connection() + .convert_selection( + window, + atoms[XdndSelection], + atoms[TextUriList], + atoms[XdndSelection], + time, + ) + .expect_then_ignore_error("Failed to send XdndSelection event") } pub unsafe fn read_data( &self, - window: c_ulong, + window: xproto::Window, ) -> Result, util::GetPropertyError> { + let atoms = self.xconn.atoms(); self.xconn - .get_property(window, self.atoms.selection, self.atoms.uri_list) + .get_property(window, atoms[XdndSelection], atoms[TextUriList]) } pub fn parse_data(&self, data: &mut [c_uchar]) -> Result, DndDataParseError> { diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index 053ccc09a8..a75a310742 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -2,9 +2,13 @@ use std::{cell::RefCell, collections::HashMap, rc::Rc, slice, sync::Arc}; use libc::{c_char, c_int, c_long, c_ulong}; +use x11rb::protocol::xproto::{self, ConnectionExt as _}; +use x11rb::x11_utils::Serialize; + use super::{ - ffi, get_xtarget, mkdid, mkwid, monitor, util, Device, DeviceId, DeviceInfo, Dnd, DndState, - GenericEventCookie, ImeReceiver, ScrollOrientation, UnownedWindow, WindowId, XExtension, + atoms::*, ffi, get_xtarget, mkdid, mkwid, monitor, util, CookieResultExt, Device, DeviceId, + DeviceInfo, Dnd, DndState, GenericEventCookie, ImeReceiver, ScrollOrientation, UnownedWindow, + WindowId, XExtension, }; use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventReceiver, ImeRequest}; @@ -38,7 +42,7 @@ pub(super) struct EventProcessor { pub(super) held_key_press: Option, pub(super) first_touch: Option, // Currently focused window belonging to this process - pub(super) active_window: Option, + pub(super) active_window: Option, pub(super) is_composing: bool, } @@ -53,7 +57,7 @@ impl EventProcessor { } } - fn with_window(&self, window_id: ffi::Window, callback: F) -> Option + fn with_window(&self, window_id: xproto::Window, callback: F) -> Option where F: Fn(&Arc) -> Ret, { @@ -77,7 +81,7 @@ impl EventProcessor { result } - fn window_exists(&self, window_id: ffi::Window) -> bool { + fn window_exists(&self, window_id: xproto::Window) -> bool { self.with_window(window_id, |_| ()).is_some() } @@ -120,6 +124,7 @@ impl EventProcessor { F: FnMut(Event<'_, T>), { let wt = get_xtarget(&self.target); + let atoms = wt.x_connection().atoms(); // XFilterEvent tells us when an event has been discarded by the input method. // Specifically, this involves all of the KeyPress events in compose/pre-edit sequences, // along with an extra copy of the KeyRelease events. This also prevents backspace and @@ -140,42 +145,57 @@ impl EventProcessor { ffi::ClientMessage => { let client_msg: &ffi::XClientMessageEvent = xev.as_ref(); - let window = client_msg.window; + let window = client_msg.window as xproto::Window; let window_id = mkwid(window); - if client_msg.data.get_long(0) as ffi::Atom == wt.wm_delete_window { + if client_msg.data.get_long(0) as xproto::Atom == wt.wm_delete_window { callback(Event::WindowEvent { window_id, event: WindowEvent::CloseRequested, }); - } else if client_msg.data.get_long(0) as ffi::Atom == wt.net_wm_ping { + } else if client_msg.data.get_long(0) as xproto::Atom == wt.net_wm_ping { let response_msg: &mut ffi::XClientMessageEvent = xev.as_mut(); - response_msg.window = wt.root; + let client_msg = xproto::ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, + format: response_msg.format as _, + sequence: response_msg.serial as _, + window: wt.root, + type_: response_msg.message_type as _, + data: xproto::ClientMessageData::from({ + let [a, b, c, d, e]: [c_long; 5] = + response_msg.data.as_longs().try_into().unwrap(); + [a as u32, b as u32, c as u32, d as u32, e as u32] + }), + }; + wt.xconn + .xcb_connection() .send_event( + false, wt.root, - Some(ffi::SubstructureNotifyMask | ffi::SubstructureRedirectMask), - *response_msg, + xproto::EventMask::SUBSTRUCTURE_NOTIFY + | xproto::EventMask::SUBSTRUCTURE_REDIRECT, + client_msg.serialize(), ) - .queue(); - } else if client_msg.message_type == self.dnd.atoms.enter { - let source_window = client_msg.data.get_long(0) as c_ulong; + .expect_then_ignore_error("Failed to send `ClientMessage` event."); + } else if client_msg.message_type == atoms[XdndEnter] as c_ulong { + let source_window = client_msg.data.get_long(0) as xproto::Window; let flags = client_msg.data.get_long(1); let version = flags >> 24; self.dnd.version = Some(version); let has_more_types = flags - (flags & (c_long::max_value() - 1)) == 1; if !has_more_types { let type_list = vec![ - client_msg.data.get_long(2) as c_ulong, - client_msg.data.get_long(3) as c_ulong, - client_msg.data.get_long(4) as c_ulong, + client_msg.data.get_long(2) as xproto::Atom, + client_msg.data.get_long(3) as xproto::Atom, + client_msg.data.get_long(4) as xproto::Atom, ]; self.dnd.type_list = Some(type_list); } else if let Ok(more_types) = unsafe { self.dnd.get_type_list(source_window) } { self.dnd.type_list = Some(more_types); } - } else if client_msg.message_type == self.dnd.atoms.position { + } else if client_msg.message_type == atoms[XdndPosition] as c_ulong { // This event occurs every time the mouse moves while a file's being dragged // over our window. We emit HoveredFile in response; while the macOS backend // does that upon a drag entering, XDND doesn't have access to the actual drop @@ -184,7 +204,7 @@ impl EventProcessor { // supply position updates with `HoveredFile` or another event, implementing // that here would be trivial. - let source_window = client_msg.data.get_long(0) as c_ulong; + let source_window = client_msg.data.get_long(0) as xproto::Window; // Equivalent to `(x << shift) | y` // where `shift = mem::size_of::() * 8` @@ -202,7 +222,7 @@ impl EventProcessor { //let action = client_msg.data.get_long(4); let accepted = if let Some(ref type_list) = self.dnd.type_list { - type_list.contains(&self.dnd.atoms.uri_list) + type_list.contains(&atoms[TextUriList]) } else { false }; @@ -212,10 +232,10 @@ impl EventProcessor { unsafe { if self.dnd.result.is_none() { let time = if version >= 1 { - client_msg.data.get_long(3) as c_ulong + client_msg.data.get_long(3) as xproto::Timestamp } else { // In version 0, time isn't specified - ffi::CurrentTime + x11rb::CURRENT_TIME }; // This results in the `SelectionNotify` event below self.dnd.convert_selection(window, time); @@ -232,7 +252,7 @@ impl EventProcessor { } self.dnd.reset(); } - } else if client_msg.message_type == self.dnd.atoms.drop { + } else if client_msg.message_type == atoms[XdndDrop] as c_ulong { let (source_window, state) = if let Some(source_window) = self.dnd.source_window { if let Some(Ok(ref path_list)) = self.dnd.result { @@ -247,7 +267,7 @@ impl EventProcessor { } else { // `source_window` won't be part of our DND state if we already rejected the drop in our // `XdndPosition` handler. - let source_window = client_msg.data.get_long(0) as c_ulong; + let source_window = client_msg.data.get_long(0) as xproto::Window; (source_window, DndState::Rejected) }; unsafe { @@ -256,7 +276,7 @@ impl EventProcessor { .expect("Failed to send `XdndFinished` message."); } self.dnd.reset(); - } else if client_msg.message_type == self.dnd.atoms.leave { + } else if client_msg.message_type == atoms[XdndLeave] as c_ulong { self.dnd.reset(); callback(Event::WindowEvent { window_id, @@ -268,10 +288,10 @@ impl EventProcessor { ffi::SelectionNotify => { let xsel: &ffi::XSelectionEvent = xev.as_ref(); - let window = xsel.requestor; + let window = xsel.requestor as xproto::Window; let window_id = mkwid(window); - if xsel.property == self.dnd.atoms.selection { + if xsel.property == atoms[XdndSelection] as c_ulong { let mut result = None; // This is where we receive data from drag and drop @@ -294,7 +314,7 @@ impl EventProcessor { ffi::ConfigureNotify => { let xev: &ffi::XConfigureEvent = xev.as_ref(); - let xwindow = xev.window; + let xwindow = xev.window as xproto::Window; let window_id = mkwid(xwindow); if let Some(window) = self.with_window(xwindow, Arc::clone) { @@ -469,13 +489,13 @@ impl EventProcessor { // effect is that we waste some time trying to query unsupported properties. wt.xconn.update_cached_wm_info(wt.root); - self.with_window(xev.window, |window| { + self.with_window(xev.window as xproto::Window, |window| { window.invalidate_cached_frame_extents(); }); } ffi::MapNotify => { let xev: &ffi::XMapEvent = xev.as_ref(); - let window = xev.window; + let window = xev.window as xproto::Window; let window_id = mkwid(window); // XXX re-issue the focus state when mapping the window. @@ -494,7 +514,7 @@ impl EventProcessor { ffi::DestroyNotify => { let xev: &ffi::XDestroyWindowEvent = xev.as_ref(); - let window = xev.window; + let window = xev.window as xproto::Window; let window_id = mkwid(window); // In the event that the window's been destroyed without being dropped first, we @@ -505,7 +525,7 @@ impl EventProcessor { // context here instead of when dropping the window. wt.ime .borrow_mut() - .remove_context(window) + .remove_context(window as ffi::Window) .expect("Failed to destroy input context"); callback(Event::WindowEvent { @@ -516,7 +536,7 @@ impl EventProcessor { ffi::VisibilityNotify => { let xev: &ffi::XVisibilityEvent = xev.as_ref(); - let xwindow = xev.window; + let xwindow = xev.window as xproto::Window; callback(Event::WindowEvent { window_id: mkwid(xwindow), event: WindowEvent::Occluded(xev.state == ffi::VisibilityFullyObscured), @@ -532,7 +552,7 @@ impl EventProcessor { // Multiple Expose events may be received for subareas of a window. // We issue `RedrawRequested` only for the last event of such a series. if xev.count == 0 { - let window = xev.window; + let window = xev.window as xproto::Window; let window_id = mkwid(window); callback(Event::RedrawRequested(window_id)); @@ -548,7 +568,7 @@ impl EventProcessor { }; let window_id = mkwid(window); - let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD); + let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD.into()); let keycode = xkev.keycode as _; @@ -597,7 +617,7 @@ impl EventProcessor { is_synthetic: false, }, }); - } else if let Some(ic) = wt.ime.borrow().get_context(window) { + } else if let Some(ic) = wt.ime.borrow().get_context(window as ffi::Window) { let written = wt.xconn.lookup_utf8(ic, xkev); if !written.is_empty() { let event = Event::WindowEvent { @@ -642,7 +662,7 @@ impl EventProcessor { match xev.evtype { ffi::XI_ButtonPress | ffi::XI_ButtonRelease => { let xev: &ffi::XIDeviceEvent = unsafe { &*(xev.data as *const _) }; - let window_id = mkwid(xev.event); + let window_id = mkwid(xev.event as xproto::Window); let device_id = mkdid(xev.deviceid); if (xev.flags & ffi::XIPointerEmulated) != 0 { // Deliver multi-touch events instead of emulated mouse events. @@ -732,10 +752,11 @@ impl EventProcessor { ffi::XI_Motion => { let xev: &ffi::XIDeviceEvent = unsafe { &*(xev.data as *const _) }; let device_id = mkdid(xev.deviceid); - let window_id = mkwid(xev.event); + let window = xev.event as xproto::Window; + let window_id = mkwid(window); let new_cursor_pos = (xev.event_x, xev.event_y); - let cursor_moved = self.with_window(xev.event, |window| { + let cursor_moved = self.with_window(window, |window| { let mut shared_state_lock = window.shared_state_lock(); util::maybe_change(&mut shared_state_lock.cursor_pos, new_cursor_pos) }); @@ -817,7 +838,8 @@ impl EventProcessor { ffi::XI_Enter => { let xev: &ffi::XIEnterEvent = unsafe { &*(xev.data as *const _) }; - let window_id = mkwid(xev.event); + let window = xev.event as xproto::Window; + let window_id = mkwid(window); let device_id = mkdid(xev.deviceid); if let Some(all_info) = DeviceInfo::get(&wt.xconn, ffi::XIAllDevices) { @@ -838,7 +860,7 @@ impl EventProcessor { } } - if self.window_exists(xev.event) { + if self.window_exists(window) { callback(Event::WindowEvent { window_id, event: CursorEntered { device_id }, @@ -857,13 +879,14 @@ impl EventProcessor { } ffi::XI_Leave => { let xev: &ffi::XILeaveEvent = unsafe { &*(xev.data as *const _) }; + let window = xev.event as xproto::Window; // Leave, FocusIn, and FocusOut can be received by a window that's already // been destroyed, which the user presumably doesn't want to deal with. - let window_closed = !self.window_exists(xev.event); + let window_closed = !self.window_exists(window); if !window_closed { callback(Event::WindowEvent { - window_id: mkwid(xev.event), + window_id: mkwid(window), event: CursorLeft { device_id: mkdid(xev.deviceid), }, @@ -872,21 +895,22 @@ impl EventProcessor { } ffi::XI_FocusIn => { let xev: &ffi::XIFocusInEvent = unsafe { &*(xev.data as *const _) }; + let window = xev.event as xproto::Window; wt.ime .borrow_mut() .focus(xev.event) .expect("Failed to focus input context"); - if self.active_window != Some(xev.event) { - self.active_window = Some(xev.event); + if self.active_window != Some(window) { + self.active_window = Some(window); wt.update_listen_device_events(true); - let window_id = mkwid(xev.event); + let window_id = mkwid(window); let position = PhysicalPosition::new(xev.event_x, xev.event_y); - if let Some(window) = self.with_window(xev.event, Arc::clone) { + if let Some(window) = self.with_window(window, Arc::clone) { window.shared_state_lock().has_focus = true; } @@ -933,7 +957,8 @@ impl EventProcessor { } ffi::XI_FocusOut => { let xev: &ffi::XIFocusOutEvent = unsafe { &*(xev.data as *const _) }; - if !self.window_exists(xev.event) { + let window = xev.event as xproto::Window; + if !self.window_exists(window) { return; } @@ -942,8 +967,8 @@ impl EventProcessor { .unfocus(xev.event) .expect("Failed to unfocus input context"); - if self.active_window.take() == Some(xev.event) { - let window_id = mkwid(xev.event); + if self.active_window.take() == Some(window) { + let window_id = mkwid(window); wt.update_listen_device_events(false); @@ -966,7 +991,7 @@ impl EventProcessor { ), }); - if let Some(window) = self.with_window(xev.event, Arc::clone) { + if let Some(window) = self.with_window(window, Arc::clone) { window.shared_state_lock().has_focus = false; } @@ -979,14 +1004,15 @@ impl EventProcessor { ffi::XI_TouchBegin | ffi::XI_TouchUpdate | ffi::XI_TouchEnd => { let xev: &ffi::XIDeviceEvent = unsafe { &*(xev.data as *const _) }; - let window_id = mkwid(xev.event); + let window = xev.event as xproto::Window; + let window_id = mkwid(window); let phase = match xev.evtype { ffi::XI_TouchBegin => TouchPhase::Started, ffi::XI_TouchUpdate => TouchPhase::Moved, ffi::XI_TouchEnd => TouchPhase::Ended, _ => unreachable!(), }; - if self.window_exists(xev.event) { + if self.window_exists(window) { let id = xev.detail as u64; let location = PhysicalPosition::new(xev.event_x, xev.event_y); @@ -997,7 +1023,7 @@ impl EventProcessor { callback(Event::WindowEvent { window_id, event: WindowEvent::CursorMoved { - device_id: mkdid(util::VIRTUAL_CORE_POINTER), + device_id: mkdid(util::VIRTUAL_CORE_POINTER.into()), position: location.cast(), }, }); @@ -1260,7 +1286,7 @@ impl EventProcessor { } let (window, event) = match self.ime_event_receiver.try_recv() { - Ok((window, event)) => (window, event), + Ok((window, event)) => (window as xproto::Window, event), Err(_) => return, }; @@ -1313,7 +1339,7 @@ impl EventProcessor { ) where F: FnMut(Event<'_, T>), { - let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD); + let device_id = mkdid(util::VIRTUAL_CORE_KEYBOARD.into()); // Update modifiers state and emit key events based on which keys are currently pressed. for keycode in wt diff --git a/src/platform_impl/linux/x11/ime/context.rs b/src/platform_impl/linux/x11/ime/context.rs index ae3ff127f2..96c0ffb8f9 100644 --- a/src/platform_impl/linux/x11/ime/context.rs +++ b/src/platform_impl/linux/x11/ime/context.rs @@ -274,7 +274,7 @@ impl ImeContext { client_data: ffi::XPointer, ) -> Option { let preedit_callbacks = PreeditCallbacks::new(client_data); - let preedit_attr = util::XSmartPointer::new( + let preedit_attr = util::memory::XSmartPointer::new( xconn, (xconn.xlib.XVaCreateNestedList)( 0, @@ -354,7 +354,7 @@ impl ImeContext { self.ic_spot = ffi::XPoint { x, y }; unsafe { - let preedit_attr = util::XSmartPointer::new( + let preedit_attr = util::memory::XSmartPointer::new( xconn, (xconn.xlib.XVaCreateNestedList)( 0, diff --git a/src/platform_impl/linux/x11/ime/input_method.rs b/src/platform_impl/linux/x11/ime/input_method.rs index 0231d669eb..454fd5e9ff 100644 --- a/src/platform_impl/linux/x11/ime/input_method.rs +++ b/src/platform_impl/linux/x11/ime/input_method.rs @@ -7,9 +7,9 @@ use std::{ sync::{Arc, Mutex}, }; +use super::{super::atoms::*, ffi, util, XConnection, XError}; use once_cell::sync::Lazy; - -use super::{ffi, util, XConnection, XError}; +use x11rb::protocol::xproto; static GLOBAL_LOCK: Lazy> = Lazy::new(Default::default); @@ -162,6 +162,12 @@ enum GetXimServersError { InvalidUtf8(IntoStringError), } +impl From for GetXimServersError { + fn from(error: util::GetPropertyError) -> Self { + GetXimServersError::GetPropertyError(error) + } +} + // The root window has a property named XIM_SERVERS, which contains a list of atoms represeting // the availabile XIM servers. For instance, if you're using ibus, it would contain an atom named // "@server=ibus". It's possible for this property to contain multiple atoms, though presumably @@ -169,13 +175,21 @@ enum GetXimServersError { // modifiers, since we don't want a user who's looking at logs to ask "am I supposed to set // XMODIFIERS to `@server=ibus`?!?" unsafe fn get_xim_servers(xconn: &Arc) -> Result, GetXimServersError> { - let servers_atom = xconn.get_atom_unchecked(b"XIM_SERVERS\0"); + let atoms = xconn.atoms(); + let servers_atom = atoms[XIM_SERVERS]; let root = (xconn.xlib.XDefaultRootWindow)(xconn.display); let mut atoms: Vec = xconn - .get_property(root, servers_atom, ffi::XA_ATOM) - .map_err(GetXimServersError::GetPropertyError)?; + .get_property::( + root as xproto::Window, + servers_atom, + xproto::Atom::from(xproto::AtomEnum::ATOM), + ) + .map_err(GetXimServersError::GetPropertyError)? + .into_iter() + .map(ffi::Atom::from) + .collect::>(); let mut names: Vec<*const c_char> = Vec::with_capacity(atoms.len()); (xconn.xlib.XGetAtomNames)( diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index 10bec837cd..ab6c7580d0 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -1,5 +1,6 @@ #![cfg(x11_platform)] +mod atoms; mod dnd; mod event_processor; pub mod ffi; @@ -25,10 +26,13 @@ use std::{ cell::{Cell, RefCell}, collections::{HashMap, HashSet, VecDeque}, ffi::CStr, + fmt, mem::{self, MaybeUninit}, ops::Deref, - os::raw::*, - os::unix::io::RawFd, + os::{ + raw::*, + unix::io::{AsRawFd, RawFd}, + }, ptr, rc::Rc, slice, @@ -37,8 +41,20 @@ use std::{ }; use libc::{self, setlocale, LC_CTYPE}; + +use atoms::*; use raw_window_handle::{RawDisplayHandle, XlibDisplayHandle}; +use x11rb::protocol::{ + xinput, + xproto::{self, ConnectionExt}, +}; +use x11rb::x11_utils::X11Error as LogicalError; +use x11rb::{ + errors::{ConnectError, ConnectionError, IdsExhausted, ReplyError}, + xcb_ffi::ReplyOrIdError, +}; + use self::{ dnd::{Dnd, DndState}, event_processor::EventProcessor, @@ -60,10 +76,10 @@ type X11Source = Generic; pub struct EventLoopWindowTarget { xconn: Arc, - wm_delete_window: ffi::Atom, - net_wm_ping: ffi::Atom, + wm_delete_window: xproto::Atom, + net_wm_ping: xproto::Atom, ime_sender: ImeSender, - root: ffi::Window, + root: xproto::Window, ime: RefCell, windows: RefCell>>, redraw_sender: Sender, @@ -106,11 +122,11 @@ impl Clone for EventLoopProxy { impl EventLoop { pub(crate) fn new(xconn: Arc) -> EventLoop { - let root = unsafe { (xconn.xlib.XDefaultRootWindow)(xconn.display) }; - - let wm_delete_window = unsafe { xconn.get_atom_unchecked(b"WM_DELETE_WINDOW\0") }; + let root = xconn.default_root().root; + let atoms = xconn.atoms(); - let net_wm_ping = unsafe { xconn.get_atom_unchecked(b"_NET_WM_PING\0") }; + let wm_delete_window = atoms[WM_DELETE_WINDOW]; + let net_wm_ping = atoms[_NET_WM_PING]; let dnd = Dnd::new(Arc::clone(&xconn)) .expect("Failed to call XInternAtoms when initializing drag and drop"); @@ -149,7 +165,7 @@ impl EventLoop { }); let randr_event_offset = xconn - .select_xrandr_input(root) + .select_xrandr_input(root as ffi::Window) .expect("Failed to query XRandR extension"); let xi2ext = unsafe { @@ -223,7 +239,11 @@ impl EventLoop { let handle = event_loop.handle(); // Create the X11 event dispatcher. - let source = X11Source::new(xconn.x11_fd, calloop::Interest::READ, calloop::Mode::Level); + let source = X11Source::new( + xconn.xcb_connection().as_raw_fd(), + calloop::Interest::READ, + calloop::Mode::Level, + ); handle .insert_source(source, |_, _, _| Ok(calloop::PostAction::Continue)) .expect("Failed to register the X11 event dispatcher"); @@ -254,8 +274,7 @@ impl EventLoop { .expect("Failed to register the redraw event channel with the event loop"); let kb_state = - KbdState::from_x11_xkb(unsafe { (xconn.xlib_xcb.XGetXCBConnection)(xconn.display) }) - .unwrap(); + KbdState::from_x11_xkb(xconn.xcb_connection().get_raw_xcb_connection()).unwrap(); let window_target = EventLoopWindowTarget { ime, @@ -299,8 +318,13 @@ impl EventLoop { // (The request buffer is flushed during `init_device`) get_xtarget(&target) .xconn - .select_xinput_events(root, ffi::XIAllDevices, ffi::XI_HierarchyChangedMask) - .queue(); + .select_xinput_events( + root, + ffi::XIAllDevices as _, + x11rb::protocol::xinput::XIEventMask::HIERARCHY, + ) + .expect("Failed to register for XInput2 device hotplug events") + .ignore_error(); get_xtarget(&target) .xconn @@ -308,8 +332,7 @@ impl EventLoop { 0x100, // Use the "core keyboard device" ffi::XkbNewKeyboardNotifyMask | ffi::XkbStateNotifyMask, ) - .unwrap() - .queue(); + .unwrap(); event_processor.init_device(ffi::XIAllDevices); @@ -592,25 +615,25 @@ impl EventLoopWindowTarget { let device_events = self.device_events.get() == DeviceEvents::Always || (focus && self.device_events.get() == DeviceEvents::WhenFocused); - let mut mask = 0; + let mut mask = xinput::XIEventMask::from(0u32); if device_events { - mask = ffi::XI_RawMotionMask - | ffi::XI_RawButtonPressMask - | ffi::XI_RawButtonReleaseMask - | ffi::XI_RawKeyPressMask - | ffi::XI_RawKeyReleaseMask; + mask = xinput::XIEventMask::RAW_MOTION + | xinput::XIEventMask::RAW_BUTTON_PRESS + | xinput::XIEventMask::RAW_BUTTON_RELEASE + | xinput::XIEventMask::RAW_KEY_PRESS + | xinput::XIEventMask::RAW_KEY_RELEASE; } self.xconn - .select_xinput_events(self.root, ffi::XIAllMasterDevices, mask) - .queue(); + .select_xinput_events(self.root, ffi::XIAllMasterDevices as _, mask) + .expect("Failed to update device event filter") + .ignore_error(); } pub fn raw_display_handle(&self) -> raw_window_handle::RawDisplayHandle { let mut display_handle = XlibDisplayHandle::empty(); display_handle.display = self.xconn.display as *mut _; - display_handle.screen = - unsafe { (self.xconn.xlib.XDefaultScreen)(self.xconn.display as *mut _) }; + display_handle.screen = self.xconn.default_screen_index() as c_int; RawDisplayHandle::Xlib(display_handle) } } @@ -702,14 +725,133 @@ impl Drop for Window { fn drop(&mut self) { let window = self.deref(); let xconn = &window.xconn; - unsafe { - (xconn.xlib.XDestroyWindow)(xconn.display, window.id().0 as ffi::Window); - // If the window was somehow already destroyed, we'll get a `BadWindow` error, which we don't care about. - let _ = xconn.check_errors(); + + if let Ok(c) = xconn + .xcb_connection() + .destroy_window(window.id().0 as xproto::Window) + { + c.ignore_error(); + } + } +} + +/// Generic sum error type for X11 errors. +#[derive(Debug)] +pub enum X11Error { + /// An error from the Xlib library. + Xlib(XError), + + /// An error that occurred while trying to connect to the X server. + Connect(ConnectError), + + /// An error that occurred over the connection medium. + Connection(ConnectionError), + + /// An error that occurred logically on the X11 end. + X11(LogicalError), + + /// The XID range has been exhausted. + XidsExhausted(IdsExhausted), + + /// Got `null` from an Xlib function without a reason. + UnexpectedNull(&'static str), +} + +impl fmt::Display for X11Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + X11Error::Xlib(e) => write!(f, "Xlib error: {}", e), + X11Error::Connect(e) => write!(f, "X11 connection error: {}", e), + X11Error::Connection(e) => write!(f, "X11 connection error: {}", e), + X11Error::XidsExhausted(e) => write!(f, "XID range exhausted: {}", e), + X11Error::X11(e) => write!(f, "X11 error: {:?}", e), + X11Error::UnexpectedNull(s) => write!(f, "Xlib function returned null: {}", s), + } + } +} + +impl std::error::Error for X11Error { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + X11Error::Xlib(e) => Some(e), + X11Error::Connect(e) => Some(e), + X11Error::Connection(e) => Some(e), + X11Error::XidsExhausted(e) => Some(e), + _ => None, + } + } +} + +impl From for X11Error { + fn from(e: XError) -> Self { + X11Error::Xlib(e) + } +} + +impl From for X11Error { + fn from(e: ConnectError) -> Self { + X11Error::Connect(e) + } +} + +impl From for X11Error { + fn from(e: ConnectionError) -> Self { + X11Error::Connection(e) + } +} + +impl From for X11Error { + fn from(e: LogicalError) -> Self { + X11Error::X11(e) + } +} + +impl From for X11Error { + fn from(value: ReplyError) -> Self { + match value { + ReplyError::ConnectionError(e) => e.into(), + ReplyError::X11Error(e) => e.into(), } } } +impl From for X11Error { + fn from(value: ime::ImeContextCreationError) -> Self { + match value { + ime::ImeContextCreationError::XError(e) => e.into(), + ime::ImeContextCreationError::Null => Self::UnexpectedNull("XOpenIM"), + } + } +} + +impl From for X11Error { + fn from(value: ReplyOrIdError) -> Self { + match value { + ReplyOrIdError::ConnectionError(e) => e.into(), + ReplyOrIdError::X11Error(e) => e.into(), + ReplyOrIdError::IdsExhausted => Self::XidsExhausted(IdsExhausted), + } + } +} + +/// The underlying x11rb connection that we are using. +type X11rbConnection = x11rb::xcb_ffi::XCBConnection; + +/// Type alias for a void cookie. +type VoidCookie<'a> = x11rb::cookie::VoidCookie<'a, X11rbConnection>; + +/// Extension trait for `Result`. +trait CookieResultExt { + /// Unwrap the send error and ignore the result. + fn expect_then_ignore_error(self, msg: &str); +} + +impl<'a, E: fmt::Debug> CookieResultExt for Result, E> { + fn expect_then_ignore_error(self, msg: &str) { + self.expect(msg).ignore_error() + } +} + /// XEvents of type GenericEvent store their actual data in an XGenericEventCookie data structure. This is a wrapper to /// extract the cookie from a GenericEvent XEvent and release the cookie data once it has been processed struct GenericEventCookie<'a> { @@ -745,7 +887,7 @@ struct XExtension { first_error_id: c_int, } -fn mkwid(w: ffi::Window) -> crate::window::WindowId { +fn mkwid(w: xproto::Window) -> crate::window::WindowId { crate::window::WindowId(crate::platform_impl::platform::WindowId(w as _)) } fn mkdid(w: c_int) -> crate::event::DeviceId { diff --git a/src/platform_impl/linux/x11/monitor.rs b/src/platform_impl/linux/x11/monitor.rs index 297e3debf5..0671626449 100644 --- a/src/platform_impl/linux/x11/monitor.rs +++ b/src/platform_impl/linux/x11/monitor.rs @@ -6,10 +6,10 @@ use once_cell::sync::Lazy; use super::{ ffi::{ - RRCrtc, RRCrtcChangeNotifyMask, RRMode, RROutputPropertyNotifyMask, + self, RRCrtc, RRCrtcChangeNotifyMask, RRMode, RROutputPropertyNotifyMask, RRScreenChangeNotifyMask, True, Window, XRRCrtcInfo, XRRModeInfo, XRRScreenResources, }, - util, XConnection, XError, + util, X11Error, XConnection, }; use crate::{ dpi::{PhysicalPosition, PhysicalSize}, @@ -241,13 +241,13 @@ impl XConnection { let mut minor = 0; (self.xrandr.XRRQueryVersion)(self.display, &mut major, &mut minor); - let root = (self.xlib.XDefaultRootWindow)(self.display); + let root = self.default_root().root; let resources = if (major == 1 && minor >= 3) || major > 1 { - (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root) + (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root as ffi::Window) } else { // WARNING: this function is supposedly very slow, on the order of hundreds of ms. // Upon failure, `resources` will be null. - (self.xrandr.XRRGetScreenResources)(self.display, root) + (self.xrandr.XRRGetScreenResources)(self.display, root as ffi::Window) }; if resources.is_null() { @@ -256,7 +256,7 @@ impl XConnection { let mut has_primary = false; - let primary = (self.xrandr.XRRGetOutputPrimary)(self.display, root); + let primary = (self.xrandr.XRRGetOutputPrimary)(self.display, root as ffi::Window); let mut available = Vec::with_capacity((*resources).ncrtc as usize); for crtc_index in 0..(*resources).ncrtc { @@ -311,7 +311,7 @@ impl XConnection { .unwrap_or_else(MonitorHandle::dummy) } - pub fn select_xrandr_input(&self, root: Window) -> Result { + pub fn select_xrandr_input(&self, root: Window) -> Result { let has_xrandr = unsafe { let mut major = 0; let mut minor = 0; diff --git a/src/platform_impl/linux/x11/util/atom.rs b/src/platform_impl/linux/x11/util/atom.rs deleted file mode 100644 index 55ebeb609c..0000000000 --- a/src/platform_impl/linux/x11/util/atom.rs +++ /dev/null @@ -1,70 +0,0 @@ -use std::{ - collections::HashMap, - ffi::{CStr, CString}, - fmt::Debug, - os::raw::*, - sync::Mutex, -}; - -use once_cell::sync::Lazy; - -use super::*; - -type AtomCache = HashMap; - -static ATOM_CACHE: Lazy> = Lazy::new(|| Mutex::new(HashMap::with_capacity(2048))); - -impl XConnection { - pub fn get_atom + Debug>(&self, name: T) -> ffi::Atom { - let name = name.as_ref(); - let mut atom_cache_lock = ATOM_CACHE.lock().unwrap(); - let cached_atom = (*atom_cache_lock).get(name).cloned(); - if let Some(atom) = cached_atom { - atom - } else { - let atom = unsafe { - (self.xlib.XInternAtom)(self.display, name.as_ptr() as *const c_char, ffi::False) - }; - if atom == 0 { - panic!( - "`XInternAtom` failed, which really shouldn't happen. Atom: {:?}, Error: {:#?}", - name, - self.check_errors(), - ); - } - /*println!( - "XInternAtom name:{:?} atom:{:?}", - name, - atom, - );*/ - (*atom_cache_lock).insert(name.to_owned(), atom); - atom - } - } - - pub unsafe fn get_atom_unchecked(&self, name: &[u8]) -> ffi::Atom { - debug_assert!(CStr::from_bytes_with_nul(name).is_ok()); - let name = CStr::from_bytes_with_nul_unchecked(name); - self.get_atom(name) - } - - // Note: this doesn't use caching, for the sake of simplicity. - // If you're dealing with this many atoms, you'll usually want to cache them locally anyway. - pub unsafe fn get_atoms(&self, names: &[*mut c_char]) -> Result, XError> { - let mut atoms = Vec::with_capacity(names.len()); - (self.xlib.XInternAtoms)( - self.display, - names.as_ptr() as *mut _, - names.len() as c_int, - ffi::False, - atoms.as_mut_ptr(), - ); - self.check_errors()?; - atoms.set_len(names.len()); - /*println!( - "XInternAtoms atoms:{:?}", - atoms, - );*/ - Ok(atoms) - } -} diff --git a/src/platform_impl/linux/x11/util/client_msg.rs b/src/platform_impl/linux/x11/util/client_msg.rs index 2f2b7edf9c..cf5517764d 100644 --- a/src/platform_impl/linux/x11/util/client_msg.rs +++ b/src/platform_impl/linux/x11/util/client_msg.rs @@ -1,46 +1,31 @@ use super::*; - -pub type ClientMsgPayload = [c_long; 5]; +use x11rb::x11_utils::Serialize; impl XConnection { - pub fn send_event>( - &self, - target_window: c_ulong, - event_mask: Option, - event: T, - ) -> Flusher<'_> { - let event_mask = event_mask.unwrap_or(ffi::NoEventMask); - unsafe { - (self.xlib.XSendEvent)( - self.display, - target_window, - ffi::False, - event_mask, - &mut event.into(), - ); - } - Flusher::new(self) - } - pub fn send_client_msg( &self, - window: c_ulong, // The window this is "about"; not necessarily this window - target_window: c_ulong, // The window we're sending to - message_type: ffi::Atom, - event_mask: Option, - data: ClientMsgPayload, - ) -> Flusher<'_> { - let event = ffi::XClientMessageEvent { - type_: ffi::ClientMessage, - display: self.display, + window: xproto::Window, // The window this is "about"; not necessarily this window + target_window: xproto::Window, // The window we're sending to + message_type: xproto::Atom, + event_mask: Option, + data: impl Into, + ) -> Result, X11Error> { + let event = xproto::ClientMessageEvent { + response_type: xproto::CLIENT_MESSAGE_EVENT, window, - message_type, - format: c_long::FORMAT as c_int, - data: unsafe { mem::transmute(data) }, - // These fields are ignored by `XSendEvent` - serial: 0, - send_event: 0, + format: 32, + data: data.into(), + sequence: 0, + type_: message_type, }; - self.send_event(target_window, event_mask, event) + + self.xcb_connection() + .send_event( + false, + target_window, + event_mask.unwrap_or(xproto::EventMask::NO_EVENT), + event.serialize(), + ) + .map_err(Into::into) } } diff --git a/src/platform_impl/linux/x11/util/cursor.rs b/src/platform_impl/linux/x11/util/cursor.rs index 1e70b864e0..5845511d07 100644 --- a/src/platform_impl/linux/x11/util/cursor.rs +++ b/src/platform_impl/linux/x11/util/cursor.rs @@ -1,11 +1,13 @@ use std::ffi::CString; +use x11rb::connection::Connection; + use crate::window::CursorIcon; use super::*; impl XConnection { - pub fn set_cursor_icon(&self, window: ffi::Window, cursor: Option) { + pub fn set_cursor_icon(&self, window: xproto::Window, cursor: Option) { let cursor = *self .cursor_cache .lock() @@ -13,7 +15,8 @@ impl XConnection { .entry(cursor) .or_insert_with(|| self.get_cursor(cursor)); - self.update_cursor(window, cursor); + self.update_cursor(window, cursor) + .expect("Failed to set cursor"); } fn create_empty_cursor(&self) -> ffi::Cursor { @@ -59,11 +62,15 @@ impl XConnection { } } - fn update_cursor(&self, window: ffi::Window, cursor: ffi::Cursor) { - unsafe { - (self.xlib.XDefineCursor)(self.display, window, cursor); + fn update_cursor(&self, window: xproto::Window, cursor: ffi::Cursor) -> Result<(), X11Error> { + self.xcb_connection() + .change_window_attributes( + window, + &xproto::ChangeWindowAttributesAux::new().cursor(cursor as xproto::Cursor), + )? + .ignore_error(); - self.flush_requests().expect("Failed to set the cursor"); - } + self.xcb_connection().flush()?; + Ok(()) } } diff --git a/src/platform_impl/linux/x11/util/format.rs b/src/platform_impl/linux/x11/util/format.rs deleted file mode 100644 index 997946b1ee..0000000000 --- a/src/platform_impl/linux/x11/util/format.rs +++ /dev/null @@ -1,55 +0,0 @@ -use std::{fmt::Debug, mem, os::raw::*}; - -// This isn't actually the number of the bits in the format. -// X11 does a match on this value to determine which type to call sizeof on. -// Thus, we use 32 for c_long, since 32 maps to c_long which maps to 64. -// ...if that sounds confusing, then you know why this enum is here. -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] -pub enum Format { - Char = 8, - Short = 16, - Long = 32, -} - -impl Format { - pub fn from_format(format: usize) -> Option { - match format { - 8 => Some(Format::Char), - 16 => Some(Format::Short), - 32 => Some(Format::Long), - _ => None, - } - } - - pub fn get_actual_size(&self) -> usize { - match self { - Format::Char => mem::size_of::(), - Format::Short => mem::size_of::(), - Format::Long => mem::size_of::(), - } - } -} - -pub trait Formattable: Debug + Clone + Copy + PartialEq + PartialOrd { - const FORMAT: Format; -} - -// You might be surprised by the absence of c_int, but not as surprised as X11 would be by the presence of it. -impl Formattable for c_schar { - const FORMAT: Format = Format::Char; -} -impl Formattable for c_uchar { - const FORMAT: Format = Format::Char; -} -impl Formattable for c_short { - const FORMAT: Format = Format::Short; -} -impl Formattable for c_ushort { - const FORMAT: Format = Format::Short; -} -impl Formattable for c_long { - const FORMAT: Format = Format::Long; -} -impl Formattable for c_ulong { - const FORMAT: Format = Format::Long; -} diff --git a/src/platform_impl/linux/x11/util/geometry.rs b/src/platform_impl/linux/x11/util/geometry.rs index 7afced804a..ada56c1a38 100644 --- a/src/platform_impl/linux/x11/util/geometry.rs +++ b/src/platform_impl/linux/x11/util/geometry.rs @@ -40,16 +40,9 @@ impl AaRect { } } -#[derive(Debug, Default)] -pub struct TranslatedCoords { - pub x_rel_root: c_int, - pub y_rel_root: c_int, - pub child: ffi::Window, -} - #[derive(Debug, Default)] pub struct Geometry { - pub root: ffi::Window, + pub root: xproto::Window, // If you want positions relative to the root window, use translate_coords. // Note that the overwhelming majority of window managers are reparenting WMs, thus the window // ID we get from window creation is for a nested window used as the window's client area. If @@ -69,14 +62,14 @@ pub struct Geometry { #[derive(Debug, Clone)] pub struct FrameExtents { - pub left: c_ulong, - pub right: c_ulong, - pub top: c_ulong, - pub bottom: c_ulong, + pub left: u32, + pub right: u32, + pub top: u32, + pub bottom: u32, } impl FrameExtents { - pub fn new(left: c_ulong, right: c_ulong, top: c_ulong, bottom: c_ulong) -> Self { + pub fn new(left: u32, right: u32, top: u32, bottom: u32) -> Self { FrameExtents { left, right, @@ -85,7 +78,7 @@ impl FrameExtents { } } - pub fn from_border(border: c_ulong) -> Self { + pub fn from_border(border: u32) -> Self { Self::new(border, border, border, border) } } @@ -144,52 +137,29 @@ impl XConnection { // This is adequate for inner_position pub fn translate_coords( &self, - window: ffi::Window, - root: ffi::Window, - ) -> Result { - let mut coords = TranslatedCoords::default(); - - unsafe { - (self.xlib.XTranslateCoordinates)( - self.display, - window, - root, - 0, - 0, - &mut coords.x_rel_root, - &mut coords.y_rel_root, - &mut coords.child, - ); - } - - self.check_errors()?; - Ok(coords) + window: xproto::Window, + root: xproto::Window, + ) -> Result { + self.xcb_connection() + .translate_coordinates(window, root, 0, 0)? + .reply() + .map_err(Into::into) } // This is adequate for inner_size - pub fn get_geometry(&self, window: ffi::Window) -> Result { - let mut geometry = Geometry::default(); - - let _status = unsafe { - (self.xlib.XGetGeometry)( - self.display, - window, - &mut geometry.root, - &mut geometry.x_rel_parent, - &mut geometry.y_rel_parent, - &mut geometry.width, - &mut geometry.height, - &mut geometry.border, - &mut geometry.depth, - ) - }; - - self.check_errors()?; - Ok(geometry) + pub fn get_geometry( + &self, + window: xproto::Window, + ) -> Result { + self.xcb_connection() + .get_geometry(window)? + .reply() + .map_err(Into::into) } - fn get_frame_extents(&self, window: ffi::Window) -> Option { - let extents_atom = unsafe { self.get_atom_unchecked(b"_NET_FRAME_EXTENTS\0") }; + fn get_frame_extents(&self, window: xproto::Window) -> Option { + let atoms = self.atoms(); + let extents_atom = atoms[_NET_FRAME_EXTENTS]; if !hint_is_supported(extents_atom) { return None; @@ -198,8 +168,12 @@ impl XConnection { // Of the WMs tested, xmonad, i3, dwm, IceWM (1.3.x and earlier), and blackbox don't // support this. As this is part of EWMH (Extended Window Manager Hints), it's likely to // be unsupported by many smaller WMs. - let extents: Option> = self - .get_property(window, extents_atom, ffi::XA_CARDINAL) + let extents: Option> = self + .get_property( + window, + extents_atom, + xproto::Atom::from(xproto::AtomEnum::CARDINAL), + ) .ok(); extents.and_then(|extents| { @@ -216,52 +190,35 @@ impl XConnection { }) } - pub fn is_top_level(&self, window: ffi::Window, root: ffi::Window) -> Option { - let client_list_atom = unsafe { self.get_atom_unchecked(b"_NET_CLIENT_LIST\0") }; + pub fn is_top_level(&self, window: xproto::Window, root: xproto::Window) -> Option { + let atoms = self.atoms(); + let client_list_atom = atoms[_NET_CLIENT_LIST]; if !hint_is_supported(client_list_atom) { return None; } - let client_list: Option> = self - .get_property(root, client_list_atom, ffi::XA_WINDOW) + let client_list: Option> = self + .get_property( + root, + client_list_atom, + xproto::Atom::from(xproto::AtomEnum::WINDOW), + ) .ok(); - client_list.map(|client_list| client_list.contains(&window)) + client_list.map(|client_list| client_list.contains(&(window as xproto::Window))) } - fn get_parent_window(&self, window: ffi::Window) -> Result { - let parent = unsafe { - let mut root = 0; - let mut parent = 0; - let mut children: *mut ffi::Window = ptr::null_mut(); - let mut nchildren = 0; - - // What's filled into `parent` if `window` is the root window? - let _status = (self.xlib.XQueryTree)( - self.display, - window, - &mut root, - &mut parent, - &mut children, - &mut nchildren, - ); - - // The list of children isn't used - if !children.is_null() { - (self.xlib.XFree)(children as *mut _); - } - - parent - }; - self.check_errors().map(|_| parent) + fn get_parent_window(&self, window: xproto::Window) -> Result { + let parent = self.xcb_connection().query_tree(window)?.reply()?.parent; + Ok(parent) } fn climb_hierarchy( &self, - window: ffi::Window, - root: ffi::Window, - ) -> Result { + window: xproto::Window, + root: xproto::Window, + ) -> Result { let mut outer_window = window; loop { let candidate = self.get_parent_window(outer_window)?; @@ -275,8 +232,8 @@ impl XConnection { pub fn get_frame_extents_heuristic( &self, - window: ffi::Window, - root: ffi::Window, + window: xproto::Window, + root: xproto::Window, ) -> FrameExtentsHeuristic { use self::FrameExtentsHeuristicPath::*; @@ -288,7 +245,7 @@ impl XConnection { let coords = self .translate_coords(window, root) .expect("Failed to translate window coordinates"); - (coords.y_rel_root, coords.child) + (coords.dst_y, coords.child) }; let (width, height, border) = { @@ -298,7 +255,7 @@ impl XConnection { ( inner_geometry.width, inner_geometry.height, - inner_geometry.border, + inner_geometry.border_width, ) }; @@ -353,7 +310,7 @@ impl XConnection { .get_geometry(outer_window) .expect("Failed to get outer window geometry"); ( - outer_geometry.y_rel_parent, + outer_geometry.y, outer_geometry.width, outer_geometry.height, ) @@ -361,21 +318,16 @@ impl XConnection { // Since we have the geometry of the outermost window and the geometry of the client // area, we can figure out what's in between. - let diff_x = outer_width.saturating_sub(width); - let diff_y = outer_height.saturating_sub(height); - let offset_y = inner_y_rel_root.saturating_sub(outer_y) as c_uint; + let diff_x = outer_width.saturating_sub(width) as u32; + let diff_y = outer_height.saturating_sub(height) as u32; + let offset_y = inner_y_rel_root.saturating_sub(outer_y) as u32; let left = diff_x / 2; let right = left; let top = offset_y; let bottom = diff_y.saturating_sub(offset_y); - let frame_extents = FrameExtents::new( - left as c_ulong, - right as c_ulong, - top as c_ulong, - bottom as c_ulong, - ); + let frame_extents = FrameExtents::new(left, right, top, bottom); FrameExtentsHeuristic { frame_extents, heuristic_path: UnsupportedNested, @@ -383,7 +335,7 @@ impl XConnection { } else { // This is the case for xmonad and dwm, AKA the only WMs tested that supplied a // border value. This is convenient, since we can use it to get an accurate frame. - let frame_extents = FrameExtents::from_border(border as c_ulong); + let frame_extents = FrameExtents::from_border(border.into()); FrameExtentsHeuristic { frame_extents, heuristic_path: UnsupportedBordered, diff --git a/src/platform_impl/linux/x11/util/hint.rs b/src/platform_impl/linux/x11/util/hint.rs index 96157e7052..968627e120 100644 --- a/src/platform_impl/linux/x11/util/hint.rs +++ b/src/platform_impl/linux/x11/util/hint.rs @@ -1,4 +1,3 @@ -use std::slice; use std::sync::Arc; use super::*; @@ -66,25 +65,27 @@ pub enum WindowType { } impl WindowType { - pub(crate) fn as_atom(&self, xconn: &Arc) -> ffi::Atom { + pub(crate) fn as_atom(&self, xconn: &Arc) -> xproto::Atom { use self::WindowType::*; - let atom_name: &[u8] = match *self { - Desktop => b"_NET_WM_WINDOW_TYPE_DESKTOP\0", - Dock => b"_NET_WM_WINDOW_TYPE_DOCK\0", - Toolbar => b"_NET_WM_WINDOW_TYPE_TOOLBAR\0", - Menu => b"_NET_WM_WINDOW_TYPE_MENU\0", - Utility => b"_NET_WM_WINDOW_TYPE_UTILITY\0", - Splash => b"_NET_WM_WINDOW_TYPE_SPLASH\0", - Dialog => b"_NET_WM_WINDOW_TYPE_DIALOG\0", - DropdownMenu => b"_NET_WM_WINDOW_TYPE_DROPDOWN_MENU\0", - PopupMenu => b"_NET_WM_WINDOW_TYPE_POPUP_MENU\0", - Tooltip => b"_NET_WM_WINDOW_TYPE_TOOLTIP\0", - Notification => b"_NET_WM_WINDOW_TYPE_NOTIFICATION\0", - Combo => b"_NET_WM_WINDOW_TYPE_COMBO\0", - Dnd => b"_NET_WM_WINDOW_TYPE_DND\0", - Normal => b"_NET_WM_WINDOW_TYPE_NORMAL\0", + let atom_name = match *self { + Desktop => _NET_WM_WINDOW_TYPE_DESKTOP, + Dock => _NET_WM_WINDOW_TYPE_DOCK, + Toolbar => _NET_WM_WINDOW_TYPE_TOOLBAR, + Menu => _NET_WM_WINDOW_TYPE_MENU, + Utility => _NET_WM_WINDOW_TYPE_UTILITY, + Splash => _NET_WM_WINDOW_TYPE_SPLASH, + Dialog => _NET_WM_WINDOW_TYPE_DIALOG, + DropdownMenu => _NET_WM_WINDOW_TYPE_DROPDOWN_MENU, + PopupMenu => _NET_WM_WINDOW_TYPE_POPUP_MENU, + Tooltip => _NET_WM_WINDOW_TYPE_TOOLTIP, + Notification => _NET_WM_WINDOW_TYPE_NOTIFICATION, + Combo => _NET_WM_WINDOW_TYPE_COMBO, + Dnd => _NET_WM_WINDOW_TYPE_DND, + Normal => _NET_WM_WINDOW_TYPE_NORMAL, }; - unsafe { xconn.get_atom_unchecked(atom_name) } + + let atoms = xconn.atoms(); + atoms[atom_name] } } @@ -92,30 +93,27 @@ pub struct MotifHints { hints: MwmHints, } -#[repr(C)] struct MwmHints { - flags: c_ulong, - functions: c_ulong, - decorations: c_ulong, - input_mode: c_long, - status: c_ulong, + flags: u32, + functions: u32, + decorations: u32, + input_mode: u32, + status: u32, } #[allow(dead_code)] mod mwm { - use libc::c_ulong; - // Motif WM hints are obsolete, but still widely supported. // https://stackoverflow.com/a/1909708 - pub const MWM_HINTS_FUNCTIONS: c_ulong = 1 << 0; - pub const MWM_HINTS_DECORATIONS: c_ulong = 1 << 1; - - pub const MWM_FUNC_ALL: c_ulong = 1 << 0; - pub const MWM_FUNC_RESIZE: c_ulong = 1 << 1; - pub const MWM_FUNC_MOVE: c_ulong = 1 << 2; - pub const MWM_FUNC_MINIMIZE: c_ulong = 1 << 3; - pub const MWM_FUNC_MAXIMIZE: c_ulong = 1 << 4; - pub const MWM_FUNC_CLOSE: c_ulong = 1 << 5; + pub const MWM_HINTS_FUNCTIONS: u32 = 1 << 0; + pub const MWM_HINTS_DECORATIONS: u32 = 1 << 1; + + pub const MWM_FUNC_ALL: u32 = 1 << 0; + pub const MWM_FUNC_RESIZE: u32 = 1 << 1; + pub const MWM_FUNC_MOVE: u32 = 1 << 2; + pub const MWM_FUNC_MINIMIZE: u32 = 1 << 3; + pub const MWM_FUNC_MAXIMIZE: u32 = 1 << 4; + pub const MWM_FUNC_CLOSE: u32 = 1 << 5; } impl MotifHints { @@ -133,7 +131,7 @@ impl MotifHints { pub fn set_decorations(&mut self, decorations: bool) { self.hints.flags |= mwm::MWM_HINTS_DECORATIONS; - self.hints.decorations = decorations as c_ulong; + self.hints.decorations = decorations as u32; } pub fn set_maximizable(&mut self, maximizable: bool) { @@ -144,7 +142,7 @@ impl MotifHints { } } - fn add_func(&mut self, func: c_ulong) { + fn add_func(&mut self, func: u32) { if self.hints.flags & mwm::MWM_HINTS_FUNCTIONS != 0 { if self.hints.functions & mwm::MWM_FUNC_ALL != 0 { self.hints.functions &= !func; @@ -154,7 +152,7 @@ impl MotifHints { } } - fn remove_func(&mut self, func: c_ulong) { + fn remove_func(&mut self, func: u32) { if self.hints.flags & mwm::MWM_HINTS_FUNCTIONS == 0 { self.hints.flags |= mwm::MWM_HINTS_FUNCTIONS; self.hints.functions = mwm::MWM_FUNC_ALL; @@ -174,170 +172,47 @@ impl Default for MotifHints { } } -impl MwmHints { - fn as_slice(&self) -> &[c_ulong] { - unsafe { slice::from_raw_parts(self as *const _ as *const c_ulong, 5) } - } -} - -pub(crate) struct NormalHints<'a> { - size_hints: XSmartPointer<'a, ffi::XSizeHints>, -} - -impl<'a> NormalHints<'a> { - pub fn new(xconn: &'a XConnection) -> Self { - NormalHints { - size_hints: xconn.alloc_size_hints(), - } - } - - pub fn get_resize_increments(&self) -> Option<(u32, u32)> { - has_flag(self.size_hints.flags, ffi::PResizeInc).then_some({ - ( - self.size_hints.width_inc as u32, - self.size_hints.height_inc as u32, - ) - }) - } - - pub fn set_position(&mut self, position: Option<(i32, i32)>) { - if let Some((x, y)) = position { - self.size_hints.flags |= ffi::PPosition; - self.size_hints.x = x as c_int; - self.size_hints.y = y as c_int; - } else { - self.size_hints.flags &= !ffi::PPosition; - } - } - - // WARNING: This hint is obsolete - pub fn set_size(&mut self, size: Option<(u32, u32)>) { - if let Some((width, height)) = size { - self.size_hints.flags |= ffi::PSize; - self.size_hints.width = width as c_int; - self.size_hints.height = height as c_int; - } else { - self.size_hints.flags &= !ffi::PSize; - } - } - - pub fn set_max_size(&mut self, max_size: Option<(u32, u32)>) { - if let Some((max_width, max_height)) = max_size { - self.size_hints.flags |= ffi::PMaxSize; - self.size_hints.max_width = max_width as c_int; - self.size_hints.max_height = max_height as c_int; - } else { - self.size_hints.flags &= !ffi::PMaxSize; - } - } - - pub fn set_min_size(&mut self, min_size: Option<(u32, u32)>) { - if let Some((min_width, min_height)) = min_size { - self.size_hints.flags |= ffi::PMinSize; - self.size_hints.min_width = min_width as c_int; - self.size_hints.min_height = min_height as c_int; - } else { - self.size_hints.flags &= !ffi::PMinSize; - } - } - - pub fn set_resize_increments(&mut self, resize_increments: Option<(u32, u32)>) { - if let Some((width_inc, height_inc)) = resize_increments { - self.size_hints.flags |= ffi::PResizeInc; - self.size_hints.width_inc = width_inc as c_int; - self.size_hints.height_inc = height_inc as c_int; - } else { - self.size_hints.flags &= !ffi::PResizeInc; - } - } - - pub fn set_base_size(&mut self, base_size: Option<(u32, u32)>) { - if let Some((base_width, base_height)) = base_size { - self.size_hints.flags |= ffi::PBaseSize; - self.size_hints.base_width = base_width as c_int; - self.size_hints.base_height = base_height as c_int; - } else { - self.size_hints.flags &= !ffi::PBaseSize; - } - } -} - impl XConnection { - pub fn get_wm_hints( - &self, - window: ffi::Window, - ) -> Result, XError> { - let wm_hints = unsafe { (self.xlib.XGetWMHints)(self.display, window) }; - self.check_errors()?; - let wm_hints = if wm_hints.is_null() { - self.alloc_wm_hints() - } else { - XSmartPointer::new(self, wm_hints).unwrap() - }; - Ok(wm_hints) - } - - pub fn set_wm_hints( - &self, - window: ffi::Window, - wm_hints: XSmartPointer<'_, ffi::XWMHints>, - ) -> Flusher<'_> { - unsafe { - (self.xlib.XSetWMHints)(self.display, window, wm_hints.ptr); - } - Flusher::new(self) - } - - pub fn get_normal_hints(&self, window: ffi::Window) -> Result, XError> { - let size_hints = self.alloc_size_hints(); - let mut supplied_by_user = MaybeUninit::uninit(); - unsafe { - (self.xlib.XGetWMNormalHints)( - self.display, - window, - size_hints.ptr, - supplied_by_user.as_mut_ptr(), - ); - } - self.check_errors().map(|_| NormalHints { size_hints }) - } - - pub fn set_normal_hints( - &self, - window: ffi::Window, - normal_hints: NormalHints<'_>, - ) -> Flusher<'_> { - unsafe { - (self.xlib.XSetWMNormalHints)(self.display, window, normal_hints.size_hints.ptr); - } - Flusher::new(self) - } - - pub fn get_motif_hints(&self, window: ffi::Window) -> MotifHints { - let motif_hints = unsafe { self.get_atom_unchecked(b"_MOTIF_WM_HINTS\0") }; + pub fn get_motif_hints(&self, window: xproto::Window) -> MotifHints { + let atoms = self.atoms(); + let motif_hints = atoms[_MOTIF_WM_HINTS]; let mut hints = MotifHints::new(); - if let Ok(props) = self.get_property::(window, motif_hints, motif_hints) { + if let Ok(props) = self.get_property::(window, motif_hints, motif_hints) { hints.hints.flags = props.first().cloned().unwrap_or(0); hints.hints.functions = props.get(1).cloned().unwrap_or(0); hints.hints.decorations = props.get(2).cloned().unwrap_or(0); - hints.hints.input_mode = props.get(3).cloned().unwrap_or(0) as c_long; + hints.hints.input_mode = props.get(3).cloned().unwrap_or(0); hints.hints.status = props.get(4).cloned().unwrap_or(0); } hints } - pub fn set_motif_hints(&self, window: ffi::Window, hints: &MotifHints) -> Flusher<'_> { - let motif_hints = unsafe { self.get_atom_unchecked(b"_MOTIF_WM_HINTS\0") }; + #[allow(clippy::unnecessary_cast)] + pub fn set_motif_hints( + &self, + window: xproto::Window, + hints: &MotifHints, + ) -> Result, X11Error> { + let atoms = self.atoms(); + let motif_hints = atoms[_MOTIF_WM_HINTS]; + + let hints_data: [u32; 5] = [ + hints.hints.flags as u32, + hints.hints.functions as u32, + hints.hints.decorations as u32, + hints.hints.input_mode as u32, + hints.hints.status as u32, + ]; self.change_property( window, motif_hints, motif_hints, - PropMode::Replace, - hints.hints.as_slice(), + xproto::PropMode::REPLACE, + &hints_data, ) } } diff --git a/src/platform_impl/linux/x11/util/input.rs b/src/platform_impl/linux/x11/util/input.rs index 41108839f8..08e69f8d5e 100644 --- a/src/platform_impl/linux/x11/util/input.rs +++ b/src/platform_impl/linux/x11/util/input.rs @@ -1,9 +1,10 @@ use std::{slice, str}; +use x11rb::protocol::xinput::{self, ConnectionExt as _}; use super::*; -pub const VIRTUAL_CORE_POINTER: c_int = 2; -pub const VIRTUAL_CORE_KEYBOARD: c_int = 3; +pub const VIRTUAL_CORE_POINTER: u16 = 2; +pub const VIRTUAL_CORE_KEYBOARD: u16 = 3; // A base buffer size of 1kB uses a negligible amount of RAM while preventing us from having to // re-allocate (and make another round-trip) in the *vast* majority of cases. @@ -13,8 +14,8 @@ const TEXT_BUFFER_SIZE: usize = 1024; // NOTE: Some of these fields are not used, but may be of use in the future. pub struct PointerState<'a> { xconn: &'a XConnection, - pub root: ffi::Window, - pub child: ffi::Window, + pub root: xproto::Window, + pub child: xproto::Window, pub root_x: c_double, pub root_y: c_double, pub win_x: c_double, @@ -38,82 +39,43 @@ impl<'a> Drop for PointerState<'a> { impl XConnection { pub fn select_xinput_events( &self, - window: c_ulong, - device_id: c_int, - mask: i32, - ) -> Flusher<'_> { - let mut event_mask = ffi::XIEventMask { - deviceid: device_id, - mask: &mask as *const _ as *mut c_uchar, - mask_len: mem::size_of_val(&mask) as c_int, - }; - unsafe { - (self.xinput2.XISelectEvents)( - self.display, + window: xproto::Window, + device_id: u16, + mask: xinput::XIEventMask, + ) -> Result, X11Error> { + self.xcb_connection() + .xinput_xi_select_events( window, - &mut event_mask as *mut ffi::XIEventMask, - 1, // number of masks to read from pointer above - ); - } - Flusher::new(self) + &[xinput::EventMask { + deviceid: device_id, + mask: vec![mask], + }], + ) + .map_err(Into::into) } - pub fn select_xkb_events(&self, device_id: c_uint, mask: c_ulong) -> Option> { - let status = unsafe { (self.xlib.XkbSelectEvents)(self.display, device_id, mask, mask) }; + pub fn select_xkb_events(&self, device_id: c_int, mask: c_ulong) -> Result { + let status = + unsafe { (self.xlib.XkbSelectEvents)(self.display, device_id as _, mask, mask) }; + if status == ffi::True { - Some(Flusher::new(self)) + self.flush_requests()?; + Ok(true) } else { error!("Could not select XKB events: The XKB extension is not initialized!"); - None + Ok(false) } } pub fn query_pointer( &self, - window: ffi::Window, - device_id: c_int, - ) -> Result, XError> { - unsafe { - let mut root = 0; - let mut child = 0; - let mut root_x = 0.0; - let mut root_y = 0.0; - let mut win_x = 0.0; - let mut win_y = 0.0; - let mut buttons = Default::default(); - let mut modifiers = Default::default(); - let mut group = Default::default(); - - let relative_to_window = (self.xinput2.XIQueryPointer)( - self.display, - device_id, - window, - &mut root, - &mut child, - &mut root_x, - &mut root_y, - &mut win_x, - &mut win_y, - &mut buttons, - &mut modifiers, - &mut group, - ) == ffi::True; - - self.check_errors()?; - - Ok(PointerState { - xconn: self, - root, - child, - root_x, - root_y, - win_x, - win_y, - buttons, - group, - relative_to_window, - }) - } + window: xproto::Window, + device_id: u16, + ) -> Result { + self.xcb_connection() + .xinput_xi_query_pointer(window, device_id)? + .reply() + .map_err(Into::into) } fn lookup_utf8_inner( diff --git a/src/platform_impl/linux/x11/util/memory.rs b/src/platform_impl/linux/x11/util/memory.rs index 181fc6a97f..d32eb8cebe 100644 --- a/src/platform_impl/linux/x11/util/memory.rs +++ b/src/platform_impl/linux/x11/util/memory.rs @@ -40,20 +40,3 @@ impl<'a, T> Drop for XSmartPointer<'a, T> { } } } - -impl XConnection { - pub fn alloc_class_hint(&self) -> XSmartPointer<'_, ffi::XClassHint> { - XSmartPointer::new(self, unsafe { (self.xlib.XAllocClassHint)() }) - .expect("`XAllocClassHint` returned null; out of memory") - } - - pub fn alloc_size_hints(&self) -> XSmartPointer<'_, ffi::XSizeHints> { - XSmartPointer::new(self, unsafe { (self.xlib.XAllocSizeHints)() }) - .expect("`XAllocSizeHints` returned null; out of memory") - } - - pub fn alloc_wm_hints(&self) -> XSmartPointer<'_, ffi::XWMHints> { - XSmartPointer::new(self, unsafe { (self.xlib.XAllocWMHints)() }) - .expect("`XAllocWMHints` returned null; out of memory") - } -} diff --git a/src/platform_impl/linux/x11/util/mod.rs b/src/platform_impl/linux/x11/util/mod.rs index ba7d7eaee2..851e65bc51 100644 --- a/src/platform_impl/linux/x11/util/mod.rs +++ b/src/platform_impl/linux/x11/util/mod.rs @@ -1,35 +1,30 @@ // Welcome to the util module, where we try to keep you from shooting yourself in the foot. // *results may vary -mod atom; mod client_msg; mod cursor; -mod format; mod geometry; mod hint; mod icon; mod input; pub mod keys; -mod memory; +pub(crate) mod memory; mod randr; mod window_property; mod wm; pub use self::{ - atom::*, client_msg::*, format::*, geometry::*, hint::*, icon::*, input::*, randr::*, - window_property::*, wm::*, + client_msg::*, geometry::*, hint::*, icon::*, input::*, randr::*, window_property::*, wm::*, }; -pub(crate) use self::memory::*; - use std::{ mem::{self, MaybeUninit}, ops::BitAnd, os::raw::*, - ptr, }; -use super::{ffi, XConnection, XError}; +use super::{atoms::*, ffi, VoidCookie, X11Error, XConnection, XError}; +use x11rb::protocol::xproto::{self, ConnectionExt as _}; pub fn maybe_change(field: &mut Option, value: T) -> bool { let wrapped = Some(value); @@ -48,30 +43,6 @@ where bitset & flag == flag } -#[must_use = "This request was made asynchronously, and is still in the output buffer. You must explicitly choose to either `.flush()` (empty the output buffer, sending the request now) or `.queue()` (wait to send the request, allowing you to continue to add more requests without additional round-trips). For more information, see the documentation for `util::flush_requests`."] -pub(crate) struct Flusher<'a> { - xconn: &'a XConnection, -} - -impl<'a> Flusher<'a> { - pub fn new(xconn: &'a XConnection) -> Self { - Flusher { xconn } - } - - // "I want this request sent now!" - pub fn flush(self) -> Result<(), XError> { - self.xconn.flush_requests() - } - - // "I want the response now too!" - pub fn sync(self) -> Result<(), XError> { - self.xconn.sync_with_server() - } - - // "I'm aware that this request hasn't been sent, and I'm okay with waiting." - pub fn queue(self) {} -} - impl XConnection { // This is impoartant, so pay attention! // Xlib has an output buffer, and tries to hide the async nature of X from you. diff --git a/src/platform_impl/linux/x11/util/modifiers.rs b/src/platform_impl/linux/x11/util/modifiers.rs index 7e317c1b24..bb157e44b4 100644 --- a/src/platform_impl/linux/x11/util/modifiers.rs +++ b/src/platform_impl/linux/x11/util/modifiers.rs @@ -48,8 +48,8 @@ impl ModifierKeymap { } pub fn reset_from_x_connection(&mut self, xconn: &XConnection) { - unsafe { - let keymap = (xconn.xlib.XGetModifierMapping)(xconn.display); + { + let keymap = xconn.xcb_connection().get_modifier_mapping().expect("get_modifier_mapping failed").reply().expect("get_modifier_mapping failed"); if keymap.is_null() { panic!("failed to allocate XModifierKeymap"); diff --git a/src/platform_impl/linux/x11/util/randr.rs b/src/platform_impl/linux/x11/util/randr.rs index a076157b14..765c9a00db 100644 --- a/src/platform_impl/linux/x11/util/randr.rs +++ b/src/platform_impl/linux/x11/util/randr.rs @@ -68,8 +68,7 @@ impl XConnection { return None; } - let screen = (self.xlib.XDefaultScreen)(self.display); - let bit_depth = (self.xlib.XDefaultDepth)(self.display, screen); + let bit_depth = self.default_root().root_depth; let output_modes = slice::from_raw_parts((*output_info).modes, (*output_info).nmode as usize); @@ -159,11 +158,11 @@ impl XConnection { let mut minor = 0; (self.xrandr.XRRQueryVersion)(self.display, &mut major, &mut minor); - let root = (self.xlib.XDefaultRootWindow)(self.display); + let root = self.default_root().root; let resources = if (major == 1 && minor >= 3) || major > 1 { - (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root) + (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root as ffi::Window) } else { - (self.xrandr.XRRGetScreenResources)(self.display, root) + (self.xrandr.XRRGetScreenResources)(self.display, root as ffi::Window) }; let crtc = (self.xrandr.XRRGetCrtcInfo)(self.display, resources, crtc_id); @@ -197,11 +196,11 @@ impl XConnection { let mut minor = 0; (self.xrandr.XRRQueryVersion)(self.display, &mut major, &mut minor); - let root = (self.xlib.XDefaultRootWindow)(self.display); + let root = self.default_root().root; let resources = if (major == 1 && minor >= 3) || major > 1 { - (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root) + (self.xrandr.XRRGetScreenResourcesCurrent)(self.display, root as ffi::Window) } else { - (self.xrandr.XRRGetScreenResources)(self.display, root) + (self.xrandr.XRRGetScreenResources)(self.display, root as ffi::Window) }; let crtc = (self.xrandr.XRRGetCrtcInfo)(self.display, resources, crtc_id); diff --git a/src/platform_impl/linux/x11/util/window_property.rs b/src/platform_impl/linux/x11/util/window_property.rs index e93d5bb072..2a2ed52347 100644 --- a/src/platform_impl/linux/x11/util/window_property.rs +++ b/src/platform_impl/linux/x11/util/window_property.rs @@ -1,18 +1,28 @@ use super::*; +use bytemuck::{NoUninit, Pod}; +use std::sync::Arc; -pub type Cardinal = c_long; -pub const CARDINAL_SIZE: usize = mem::size_of::(); +use x11rb::connection::Connection; +use x11rb::errors::ReplyError; + +pub type Cardinal = u32; +pub const CARDINAL_SIZE: usize = mem::size_of::(); #[derive(Debug, Clone)] pub enum GetPropertyError { - XError(XError), - TypeMismatch(ffi::Atom), + X11rbError(Arc), + TypeMismatch(xproto::Atom), FormatMismatch(c_int), - NothingAllocated, +} + +impl> From for GetPropertyError { + fn from(e: T) -> Self { + Self::X11rbError(Arc::new(e.into())) + } } impl GetPropertyError { - pub fn is_actual_property_type(&self, t: ffi::Atom) -> bool { + pub fn is_actual_property_type(&self, t: xproto::Atom) -> bool { if let GetPropertyError::TypeMismatch(actual_type) = *self { actual_type == t } else { @@ -23,120 +33,150 @@ impl GetPropertyError { // Number of 32-bit chunks to retrieve per iteration of get_property's inner loop. // To test if `get_property` works correctly, set this to 1. -const PROPERTY_BUFFER_SIZE: c_long = 1024; // 4k of RAM ought to be enough for anyone! - -#[derive(Debug)] -#[allow(dead_code)] -pub enum PropMode { - Replace = ffi::PropModeReplace as isize, - Prepend = ffi::PropModePrepend as isize, - Append = ffi::PropModeAppend as isize, -} +const PROPERTY_BUFFER_SIZE: u32 = 1024; // 4k of RAM ought to be enough for anyone! impl XConnection { - pub fn get_property( + pub fn get_property( &self, - window: c_ulong, - property: ffi::Atom, - property_type: ffi::Atom, + window: xproto::Window, + property: xproto::Atom, + property_type: xproto::Atom, ) -> Result, GetPropertyError> { - let mut data = Vec::new(); - let mut offset = 0; - - let mut done = false; - let mut actual_type = 0; - let mut actual_format = 0; - let mut quantity_returned = 0; - let mut bytes_after = 0; - let mut buf: *mut c_uchar = ptr::null_mut(); - - while !done { - unsafe { - (self.xlib.XGetWindowProperty)( - self.display, - window, - property, - // This offset is in terms of 32-bit chunks. - offset, - // This is the quantity of 32-bit chunks to receive at once. - PROPERTY_BUFFER_SIZE, - ffi::False, - property_type, - &mut actual_type, - &mut actual_format, - // This is the quantity of items we retrieved in our format, NOT of 32-bit chunks! - &mut quantity_returned, - // ...and this is a quantity of bytes. So, this function deals in 3 different units. - &mut bytes_after, - &mut buf, - ); - - if let Err(e) = self.check_errors() { - return Err(GetPropertyError::XError(e)); - } - - if actual_type != property_type { - return Err(GetPropertyError::TypeMismatch(actual_type)); - } - - let format_mismatch = Format::from_format(actual_format as _) != Some(T::FORMAT); - if format_mismatch { - return Err(GetPropertyError::FormatMismatch(actual_format)); - } - - if !buf.is_null() { - offset += PROPERTY_BUFFER_SIZE; - let new_data = - std::slice::from_raw_parts(buf as *mut T, quantity_returned as usize); - /*println!( - "XGetWindowProperty prop:{:?} fmt:{:02} len:{:02} off:{:02} out:{:02}, buf:{:?}", - property, - mem::size_of::() * 8, - data.len(), - offset, - quantity_returned, - new_data, - );*/ - data.extend_from_slice(new_data); - // Fun fact: XGetWindowProperty allocates one extra byte at the end. - (self.xlib.XFree)(buf as _); // Don't try to access new_data after this. - } else { - return Err(GetPropertyError::NothingAllocated); - } - - done = bytes_after == 0; + let mut iter = PropIterator::new(self.xcb_connection(), window, property, property_type); + let mut data = vec![]; + + loop { + if !iter.next_window(&mut data)? { + break; } } Ok(data) } - pub fn change_property<'a, T: Formattable>( + pub fn change_property<'a, T: NoUninit>( &'a self, - window: c_ulong, - property: ffi::Atom, - property_type: ffi::Atom, - mode: PropMode, + window: xproto::Window, + property: xproto::Atom, + property_type: xproto::Atom, + mode: xproto::PropMode, new_value: &[T], - ) -> Flusher<'a> { - debug_assert_eq!(mem::size_of::(), T::FORMAT.get_actual_size()); - unsafe { - (self.xlib.XChangeProperty)( - self.display, + ) -> Result, X11Error> { + assert!([1usize, 2, 4].contains(&mem::size_of::())); + self.xcb_connection() + .change_property( + mode, window, property, property_type, - T::FORMAT as c_int, - mode as c_int, - new_value.as_ptr() as *const c_uchar, - new_value.len() as c_int, - ); - } - /*println!( - "XChangeProperty prop:{:?} val:{:?}", + (mem::size_of::() * 8) as u8, + new_value + .len() + .try_into() + .expect("too many items for propery"), + bytemuck::cast_slice::(new_value), + ) + .map_err(Into::into) + } +} + +/// An iterator over the "windows" of the property that we are fetching. +struct PropIterator<'a, C: ?Sized, T> { + /// Handle to the connection. + conn: &'a C, + + /// The window that we're fetching the property from. + window: xproto::Window, + + /// The property that we're fetching. + property: xproto::Atom, + + /// The type of the property that we're fetching. + property_type: xproto::Atom, + + /// The offset of the next window, in 32-bit chunks. + offset: u32, + + /// The format of the type. + format: u8, + + /// Keep a reference to `T`. + _phantom: std::marker::PhantomData, +} + +impl<'a, C: Connection + ?Sized, T: Pod> PropIterator<'a, C, T> { + /// Create a new property iterator. + fn new( + conn: &'a C, + window: xproto::Window, + property: xproto::Atom, + property_type: xproto::Atom, + ) -> Self { + let format = match mem::size_of::() { + 1 => 8, + 2 => 16, + 4 => 32, + _ => unreachable!(), + }; + + Self { + conn, + window, property, - new_value, - );*/ - Flusher::new(self) + property_type, + offset: 0, + format, + _phantom: Default::default(), + } + } + + /// Get the next window and append it to `data`. + /// + /// Returns whether there are more windows to fetch. + fn next_window(&mut self, data: &mut Vec) -> Result { + // Send the request and wait for the reply. + let reply = self + .conn + .get_property( + false, + self.window, + self.property, + self.property_type, + self.offset, + PROPERTY_BUFFER_SIZE, + )? + .reply()?; + + // Make sure that the reply is of the correct type. + if reply.type_ != self.property_type { + return Err(GetPropertyError::TypeMismatch(reply.type_)); + } + + // Make sure that the reply is of the correct format. + if reply.format != self.format { + return Err(GetPropertyError::FormatMismatch(reply.format.into())); + } + + // Append the data to the output. + if mem::size_of::() == 1 && mem::align_of::() == 1 { + // We can just do a bytewise append. + data.extend_from_slice(bytemuck::cast_slice(&reply.value)); + } else { + // Rust's borrowing and types system makes this a bit tricky. + // + // We need to make sure that the data is properly aligned. Unfortunately the best + // safe way to do this is to copy the data to another buffer and then append. + // + // TODO(notgull): It may be worth it to use `unsafe` to copy directly from + // `reply.value` to `data`; check if this is faster. Use benchmarks! + let old_len = data.len(); + let added_len = reply.value.len() / mem::size_of::(); + data.resize(old_len + added_len, T::zeroed()); + bytemuck::cast_slice_mut::(&mut data[old_len..]).copy_from_slice(&reply.value); + } + + // Check `bytes_after` to see if there are more windows to fetch. + self.offset += PROPERTY_BUFFER_SIZE; + Ok(reply.bytes_after != 0) } } diff --git a/src/platform_impl/linux/x11/util/wm.rs b/src/platform_impl/linux/x11/util/wm.rs index 03890facc8..43a800dd73 100644 --- a/src/platform_impl/linux/x11/util/wm.rs +++ b/src/platform_impl/linux/x11/util/wm.rs @@ -16,11 +16,11 @@ pub const MOVERESIZE_LEFT: isize = 7; pub const MOVERESIZE_MOVE: isize = 8; // This info is global to the window manager. -static SUPPORTED_HINTS: Lazy>> = +static SUPPORTED_HINTS: Lazy>> = Lazy::new(|| Mutex::new(Vec::with_capacity(0))); static WM_NAME: Lazy>> = Lazy::new(|| Mutex::new(None)); -pub fn hint_is_supported(hint: ffi::Atom) -> bool { +pub fn hint_is_supported(hint: xproto::Atom) -> bool { (*SUPPORTED_HINTS.lock().unwrap()).contains(&hint) } @@ -33,20 +33,27 @@ pub fn wm_name_is_one_of(names: &[&str]) -> bool { } impl XConnection { - pub fn update_cached_wm_info(&self, root: ffi::Window) { + pub fn update_cached_wm_info(&self, root: xproto::Window) { *SUPPORTED_HINTS.lock().unwrap() = self.get_supported_hints(root); *WM_NAME.lock().unwrap() = self.get_wm_name(root); } - fn get_supported_hints(&self, root: ffi::Window) -> Vec { - let supported_atom = unsafe { self.get_atom_unchecked(b"_NET_SUPPORTED\0") }; - self.get_property(root, supported_atom, ffi::XA_ATOM) - .unwrap_or_else(|_| Vec::with_capacity(0)) + fn get_supported_hints(&self, root: xproto::Window) -> Vec { + let atoms = self.atoms(); + let supported_atom = atoms[_NET_SUPPORTED]; + self.get_property( + root, + supported_atom, + xproto::Atom::from(xproto::AtomEnum::ATOM), + ) + .unwrap_or_else(|_| Vec::with_capacity(0)) } - fn get_wm_name(&self, root: ffi::Window) -> Option { - let check_atom = unsafe { self.get_atom_unchecked(b"_NET_SUPPORTING_WM_CHECK\0") }; - let wm_name_atom = unsafe { self.get_atom_unchecked(b"_NET_WM_NAME\0") }; + #[allow(clippy::useless_conversion)] + fn get_wm_name(&self, root: xproto::Window) -> Option { + let atoms = self.atoms(); + let check_atom = atoms[_NET_SUPPORTING_WM_CHECK]; + let wm_name_atom = atoms[_NET_WM_NAME]; // Mutter/Muffin/Budgie doesn't have _NET_SUPPORTING_WM_CHECK in its _NET_SUPPORTED, despite // it working and being supported. This has been reported upstream, but due to the @@ -70,7 +77,11 @@ impl XConnection { // Querying this property on the root window will give us the ID of a child window created by // the WM. let root_window_wm_check = { - let result = self.get_property(root, check_atom, ffi::XA_WINDOW); + let result = self.get_property::( + root, + check_atom, + xproto::Atom::from(xproto::AtomEnum::WINDOW), + ); let wm_check = result.ok().and_then(|wm_check| wm_check.first().cloned()); @@ -80,7 +91,11 @@ impl XConnection { // Querying the same property on the child window we were given, we should get this child // window's ID again. let child_window_wm_check = { - let result = self.get_property(root_window_wm_check, check_atom, ffi::XA_WINDOW); + let result = self.get_property::( + root_window_wm_check.into(), + check_atom, + xproto::Atom::from(xproto::AtomEnum::WINDOW), + ); let wm_check = result.ok().and_then(|wm_check| wm_check.first().cloned()); @@ -94,9 +109,11 @@ impl XConnection { // All of that work gives us a window ID that we can get the WM name from. let wm_name = { - let utf8_string_atom = unsafe { self.get_atom_unchecked(b"UTF8_STRING\0") }; + let atoms = self.atoms(); + let utf8_string_atom = atoms[UTF8_STRING]; - let result = self.get_property(root_window_wm_check, wm_name_atom, utf8_string_atom); + let result = + self.get_property(root_window_wm_check.into(), wm_name_atom, utf8_string_atom); // IceWM requires this. IceWM was also the only WM tested that returns a null-terminated // string. For more fun trivia, IceWM is also unique in including version and uname @@ -105,13 +122,17 @@ impl XConnection { // The unofficial 1.4 fork of IceWM still includes the extra details, but properly // returns a UTF8 string that isn't null-terminated. let no_utf8 = if let Err(ref err) = result { - err.is_actual_property_type(ffi::XA_STRING) + err.is_actual_property_type(xproto::Atom::from(xproto::AtomEnum::STRING)) } else { false }; if no_utf8 { - self.get_property(root_window_wm_check, wm_name_atom, ffi::XA_STRING) + self.get_property( + root_window_wm_check.into(), + wm_name_atom, + xproto::Atom::from(xproto::AtomEnum::STRING), + ) } else { result } diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index c361c5ea9d..87bd290d6a 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -1,7 +1,7 @@ use std::{ cmp, env, ffi::CString, - mem::{self, replace, MaybeUninit}, + mem::{replace, MaybeUninit}, os::raw::*, path::Path, ptr, slice, @@ -10,13 +10,21 @@ use std::{ use libc; use raw_window_handle::{RawDisplayHandle, RawWindowHandle, XlibDisplayHandle, XlibWindowHandle}; -use x11_dl::xlib::{TrueColor, XID}; +use x11rb::{ + connection::Connection, + properties::WmHintsState, + protocol::xproto::{self, ConnectionExt as _}, +}; +use x11rb::{ + properties::{WmHints, WmSizeHints, WmSizeHintsSpecification}, + protocol::xinput, +}; use crate::{ dpi::{PhysicalPosition, PhysicalSize, Position, Size}, error::{ExternalError, NotSupportedError, OsError as RootOsError}, platform_impl::{ - x11::{ime::ImeContextCreationError, MonitorHandle as X11MonitorHandle}, + x11::{atoms::*, MonitorHandle as X11MonitorHandle, X11Error}, Fullscreen, MonitorHandle as PlatformMonitorHandle, OsError, PlatformSpecificWindowBuilderAttributes, VideoMode as PlatformVideoMode, }, @@ -27,7 +35,8 @@ use crate::{ }; use super::{ - ffi, util, EventLoopWindowTarget, ImeRequest, ImeSender, WindowId, XConnection, XError, + ffi, util, CookieResultExt, EventLoopWindowTarget, ImeRequest, ImeSender, VoidCookie, WindowId, + XConnection, }; use calloop::channel::Sender; @@ -105,8 +114,8 @@ unsafe impl Sync for UnownedWindow {} pub(crate) struct UnownedWindow { pub(crate) xconn: Arc, // never changes - xwindow: ffi::Window, // never changes - root: ffi::Window, // never changes + xwindow: xproto::Window, // never changes + root: xproto::Window, // never changes screen_id: i32, // never changes cursor: Mutex, cursor_grabbed_mode: Mutex, @@ -118,15 +127,26 @@ pub(crate) struct UnownedWindow { } impl UnownedWindow { + #[allow(clippy::unnecessary_cast)] pub(crate) fn new( event_loop: &EventLoopWindowTarget, window_attrs: WindowAttributes, pl_attribs: PlatformSpecificWindowBuilderAttributes, ) -> Result { + macro_rules! leap { + ($e:expr) => { + match $e { + Ok(x) => x, + Err(err) => return Err(os_error!(OsError::XError(X11Error::from(err).into()))), + } + }; + } + let xconn = &event_loop.xconn; + let atoms = xconn.atoms(); let root = match window_attrs.parent_window { - Some(RawWindowHandle::Xlib(handle)) => handle.window, - Some(RawWindowHandle::Xcb(handle)) => handle.window as XID, + Some(RawWindowHandle::Xlib(handle)) => handle.window as xproto::Window, + Some(RawWindowHandle::Xcb(handle)) => handle.window, Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"), None => event_loop.root, }; @@ -192,97 +212,103 @@ impl UnownedWindow { let screen_id = match pl_attribs.screen_id { Some(id) => id, - None => unsafe { (xconn.xlib.XDefaultScreen)(xconn.display) }, + None => xconn.default_screen_index() as c_int, }; // creating let (visual, depth, require_colormap) = match pl_attribs.visual_infos { - Some(vi) => (vi.visual, vi.depth, false), + Some(vi) => (vi.visualid as _, vi.depth as _, false), None if window_attrs.transparent => { - // Find a suitable visual - let mut vinfo = MaybeUninit::uninit(); - let vinfo_initialized = unsafe { - (xconn.xlib.XMatchVisualInfo)( - xconn.display, - screen_id, - 32, - TrueColor, - vinfo.as_mut_ptr(), - ) != 0 - }; - if vinfo_initialized { - let vinfo = unsafe { vinfo.assume_init() }; - (vinfo.visual, vinfo.depth, true) - } else { - debug!("Could not set transparency, because XMatchVisualInfo returned zero for the required parameters"); - ( - ffi::CopyFromParent as *mut ffi::Visual, - ffi::CopyFromParent, - false, - ) - } + // Find a suitable visual, true color with 32 bits of depth. + xconn.xcb_connection().setup().roots + .iter() + .flat_map(|root| &root.allowed_depths) + .filter(|depth| depth.depth == 32) + .flat_map(|depth| depth.visuals.iter().map(move |visual| (visual, depth.depth))) + .find(|(visual, _)| { + visual.class == xproto::VisualClass::TRUE_COLOR + }) + .map_or_else(|| { + debug!("Could not set transparency, because XMatchVisualInfo returned zero for the required parameters"); + (x11rb::COPY_FROM_PARENT as _, x11rb::COPY_FROM_PARENT as _, false) + }, |(visual, depth)| (visual.visual_id, depth, true)) } _ => ( - ffi::CopyFromParent as *mut ffi::Visual, - ffi::CopyFromParent, + x11rb::COPY_FROM_PARENT as _, + x11rb::COPY_FROM_PARENT as _, false, ), }; - let mut set_win_attr = { - let mut swa: ffi::XSetWindowAttributes = unsafe { mem::zeroed() }; - swa.colormap = if let Some(vi) = pl_attribs.visual_infos { - unsafe { - let visual = vi.visual; - (xconn.xlib.XCreateColormap)(xconn.display, root, visual, ffi::AllocNone) - } - } else if require_colormap { - unsafe { (xconn.xlib.XCreateColormap)(xconn.display, root, visual, ffi::AllocNone) } - } else { - 0 + let window_attributes = { + use xproto::EventMask; + + let mut aux = xproto::CreateWindowAux::new(); + let event_mask = EventMask::EXPOSURE + | EventMask::STRUCTURE_NOTIFY + | EventMask::VISIBILITY_CHANGE + | EventMask::KEY_PRESS + | EventMask::KEY_RELEASE + | EventMask::KEYMAP_STATE + | EventMask::BUTTON_PRESS + | EventMask::BUTTON_RELEASE + | EventMask::POINTER_MOTION; + + aux = aux.event_mask(event_mask).border_pixel(0); + + if pl_attribs.override_redirect { + aux = aux.override_redirect(true as u32); + } + + // Add a colormap if needed. + let colormap_visual = match pl_attribs.visual_infos { + Some(vi) => Some(vi.visualid as u32), + None if require_colormap => Some(visual), + _ => None, }; - swa.event_mask = ffi::ExposureMask - | ffi::StructureNotifyMask - | ffi::VisibilityChangeMask - | ffi::KeyPressMask - | ffi::KeyReleaseMask - | ffi::KeymapStateMask - | ffi::ButtonPressMask - | ffi::ButtonReleaseMask - | ffi::PointerMotionMask; - swa.border_pixel = 0; - swa.override_redirect = pl_attribs.override_redirect as c_int; - swa - }; - let mut window_attributes = ffi::CWBorderPixel | ffi::CWColormap | ffi::CWEventMask; + if let Some(visual) = colormap_visual { + let colormap = leap!(xconn.xcb_connection().generate_id()); + leap!(xconn.xcb_connection().create_colormap( + xproto::ColormapAlloc::NONE, + colormap, + root, + visual, + )); + aux = aux.colormap(colormap); + } else { + aux = aux.colormap(0); + } - if pl_attribs.override_redirect { - window_attributes |= ffi::CWOverrideRedirect; - } + aux + }; // finally creating the window - let xwindow = unsafe { - (xconn.xlib.XCreateWindow)( - xconn.display, + let xwindow = { + let (x, y) = position.map_or((0, 0), Into::into); + let wid = leap!(xconn.xcb_connection().generate_id()); + let result = xconn.xcb_connection().create_window( + depth, + wid, root, - position.map_or(0, |p: PhysicalPosition| p.x as c_int), - position.map_or(0, |p: PhysicalPosition| p.y as c_int), - dimensions.0 as c_uint, - dimensions.1 as c_uint, + x, + y, + dimensions.0.try_into().unwrap(), + dimensions.1.try_into().unwrap(), 0, - depth, - ffi::InputOutput as c_uint, + xproto::WindowClass::INPUT_OUTPUT, visual, - window_attributes, - &mut set_win_attr, - ) + &window_attributes, + ); + leap!(leap!(result).check()); + + wid }; #[allow(clippy::mutex_atomic)] let mut window = UnownedWindow { xconn: Arc::clone(xconn), - xwindow, + xwindow: xwindow as xproto::Window, root, screen_id, cursor: Default::default(), @@ -296,72 +322,64 @@ impl UnownedWindow { // Title must be set before mapping. Some tiling window managers (i.e. i3) use the window // title to determine placement/etc., so doing this after mapping would cause the WM to // act on the wrong title state. - window.set_title_inner(&window_attrs.title).queue(); - window - .set_decorations_inner(window_attrs.decorations) - .queue(); + leap!(window.set_title_inner(&window_attrs.title)).ignore_error(); + leap!(window.set_decorations_inner(window_attrs.decorations)).ignore_error(); if let Some(theme) = window_attrs.preferred_theme { - window.set_theme_inner(Some(theme)).queue(); + leap!(window.set_theme_inner(Some(theme))).ignore_error(); } { // Enable drag and drop (TODO: extend API to make this toggleable) - unsafe { - let dnd_aware_atom = xconn.get_atom_unchecked(b"XdndAware\0"); - let version = &[5 as c_ulong]; // Latest version; hasn't changed since 2002 - xconn.change_property( + { + let dnd_aware_atom = atoms[XdndAware]; + let version = &[5u32]; // Latest version; hasn't changed since 2002 + leap!(xconn.change_property( window.xwindow, dnd_aware_atom, - ffi::XA_ATOM, - util::PropMode::Replace, + u32::from(xproto::AtomEnum::ATOM), + xproto::PropMode::REPLACE, version, - ) + )) + .ignore_error(); } - .queue(); // WM_CLASS must be set *before* mapping the window, as per ICCCM! { let (class, instance) = if let Some(name) = pl_attribs.name { - let instance = CString::new(name.instance.as_str()) - .expect("`WM_CLASS` instance contained null byte"); - let class = CString::new(name.general.as_str()) - .expect("`WM_CLASS` class contained null byte"); - (instance, class) + (name.instance, name.general) } else { - let class = env::args() + let class = env::args_os() .next() .as_ref() // Default to the name of the binary (via argv[0]) .and_then(|path| Path::new(path).file_name()) .and_then(|bin_name| bin_name.to_str()) .map(|bin_name| bin_name.to_owned()) - .or_else(|| Some(window_attrs.title.clone())) - .and_then(|string| CString::new(string.as_str()).ok()) - .expect("Default `WM_CLASS` class contained null byte"); + .unwrap_or_else(|| window_attrs.title.clone()); // This environment variable is extraordinarily unlikely to actually be used... let instance = env::var("RESOURCE_NAME") .ok() - .and_then(|instance| CString::new(instance.as_str()).ok()) - .or_else(|| Some(class.clone())) - .expect("Default `WM_CLASS` instance contained null byte"); + .unwrap_or_else(|| class.clone()); (instance, class) }; - let mut class_hint = xconn.alloc_class_hint(); - class_hint.res_name = class.as_ptr() as *mut c_char; - class_hint.res_class = instance.as_ptr() as *mut c_char; - - unsafe { - (xconn.xlib.XSetClassHint)(xconn.display, window.xwindow, class_hint.ptr); - } //.queue(); + let class = format!("{instance}\0{class}\0"); + leap!(xconn.change_property( + window.xwindow, + xproto::Atom::from(xproto::AtomEnum::WM_CLASS), + xproto::Atom::from(xproto::AtomEnum::STRING), + xproto::PropMode::REPLACE, + class.as_bytes(), + )) + .ignore_error(); } - if let Some(flusher) = window.set_pid() { - flusher.queue() + if let Some(flusher) = leap!(window.set_pid()) { + flusher.ignore_error() } - window.set_window_types(pl_attribs.x11_window_types).queue(); + leap!(window.set_window_types(pl_attribs.x11_window_types)).ignore_error(); // set size hints { @@ -387,45 +405,62 @@ impl UnownedWindow { shared_state.resize_increments = window_attrs.resize_increments; shared_state.base_size = pl_attribs.base_size; - let mut normal_hints = util::NormalHints::new(xconn); - normal_hints.set_position(position.map(|PhysicalPosition { x, y }| (x, y))); - normal_hints.set_size(Some(dimensions)); - normal_hints.set_min_size(min_inner_size.map(Into::into)); - normal_hints.set_max_size(max_inner_size.map(Into::into)); - normal_hints.set_resize_increments( - window_attrs + let normal_hints = WmSizeHints { + position: position.map(|PhysicalPosition { x, y }| { + (WmSizeHintsSpecification::UserSpecified, x, y) + }), + size: Some(( + WmSizeHintsSpecification::UserSpecified, + dimensions.0 as i32, + dimensions.1 as i32, + )), + max_size: max_inner_size.map(Into::into), + min_size: min_inner_size.map(Into::into), + size_increment: window_attrs .resize_increments - .map(|size| size.to_physical::(scale_factor).into()), - ); - normal_hints.set_base_size( - pl_attribs + .map(|size| size.to_physical::(scale_factor).into()), + base_size: pl_attribs .base_size - .map(|size| size.to_physical::(scale_factor).into()), - ); - xconn.set_normal_hints(window.xwindow, normal_hints).queue(); + .map(|size| size.to_physical::(scale_factor).into()), + aspect: None, + win_gravity: None, + }; + leap!(leap!(normal_hints.set( + xconn.xcb_connection(), + window.xwindow as xproto::Window, + xproto::AtomEnum::WM_NORMAL_HINTS, + )) + .check()) } // Set window icons if let Some(icon) = window_attrs.window_icon { - window.set_icon_inner(icon).queue(); + leap!(window.set_icon_inner(icon)).ignore_error(); } // Opt into handling window close - unsafe { - (xconn.xlib.XSetWMProtocols)( - xconn.display, - window.xwindow, - &[event_loop.wm_delete_window, event_loop.net_wm_ping] as *const ffi::Atom - as *mut ffi::Atom, - 2, - ); - } //.queue(); + let result = xconn.xcb_connection().change_property( + xproto::PropMode::REPLACE, + window.xwindow, + atoms[WM_PROTOCOLS], + xproto::AtomEnum::ATOM, + 32, + 2, + bytemuck::cast_slice::(&[ + atoms[WM_DELETE_WINDOW], + atoms[_NET_WM_PING], + ]), + ); + leap!(result).ignore_error(); // Set visibility (map window) if window_attrs.visible { - unsafe { - (xconn.xlib.XMapRaised)(xconn.display, window.xwindow); - } //.queue(); + leap!(xconn.xcb_connection().map_window(window.xwindow)).ignore_error(); + leap!(xconn.xcb_connection().configure_window( + xwindow, + &xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::ABOVE) + )) + .ignore_error(); } // Attempt to make keyboard input repeat detectable @@ -444,45 +479,37 @@ impl UnownedWindow { } // Select XInput2 events - let mask = ffi::XI_MotionMask - | ffi::XI_ButtonPressMask - | ffi::XI_ButtonReleaseMask - | ffi::XI_EnterMask - | ffi::XI_LeaveMask - | ffi::XI_FocusInMask - | ffi::XI_FocusOutMask - | ffi::XI_TouchBeginMask - | ffi::XI_TouchUpdateMask - | ffi::XI_TouchEndMask; - xconn - .select_xinput_events(window.xwindow, ffi::XIAllMasterDevices, mask) - .queue(); + let mask = xinput::XIEventMask::MOTION + | xinput::XIEventMask::BUTTON_PRESS + | xinput::XIEventMask::BUTTON_RELEASE + | xinput::XIEventMask::ENTER + | xinput::XIEventMask::LEAVE + | xinput::XIEventMask::FOCUS_IN + | xinput::XIEventMask::FOCUS_OUT + | xinput::XIEventMask::TOUCH_BEGIN + | xinput::XIEventMask::TOUCH_UPDATE + | xinput::XIEventMask::TOUCH_END; + leap!(xconn.select_xinput_events(window.xwindow, ffi::XIAllMasterDevices as u16, mask)) + .ignore_error(); { let result = event_loop .ime .borrow_mut() - .create_context(window.xwindow, false); - if let Err(err) = result { - let e = match err { - ImeContextCreationError::XError(err) => OsError::XError(err), - ImeContextCreationError::Null => { - OsError::XMisc("IME Context creation failed") - } - }; - return Err(os_error!(e)); - } + .create_context(window.xwindow as ffi::Window, false); + leap!(result); } // These properties must be set after mapping if window_attrs.maximized { - window.set_maximized_inner(window_attrs.maximized).queue(); + leap!(window.set_maximized_inner(window_attrs.maximized)).ignore_error(); } if window_attrs.fullscreen.is_some() { if let Some(flusher) = - window.set_fullscreen_inner(window_attrs.fullscreen.clone().map(Into::into)) + leap!(window + .set_fullscreen_inner(window_attrs.fullscreen.clone().map(Into::into))) { - flusher.queue() + flusher.ignore_error() } if let Some(PhysicalPosition { x, y }) = position { @@ -492,25 +519,23 @@ impl UnownedWindow { } } - window - .set_window_level_inner(window_attrs.window_level) - .queue(); + leap!(window.set_window_level_inner(window_attrs.window_level)).ignore_error(); } // We never want to give the user a broken window, since by then, it's too late to handle. - xconn - .sync_with_server() - .map(|_| window) - .map_err(|x_err| os_error!(OsError::XError(x_err))) + let window = leap!(xconn.sync_with_server().map(|_| window)); + + Ok(window) } pub(super) fn shared_state_lock(&self) -> MutexGuard<'_, SharedState> { self.shared_state.lock().unwrap() } - fn set_pid(&self) -> Option> { - let pid_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_PID\0") }; - let client_machine_atom = unsafe { self.xconn.get_atom_unchecked(b"WM_CLIENT_MACHINE\0") }; + fn set_pid(&self) -> Result>, X11Error> { + let atoms = self.xconn.atoms(); + let pid_atom = atoms[_NET_WM_PID]; + let client_machine_atom = atoms[WM_CLIENT_MACHINE]; unsafe { // 64 would suffice for Linux, but 256 will be enough everywhere (as per SUSv2). For instance, this is // the limit defined by OpenBSD. @@ -521,7 +546,7 @@ impl UnownedWindow { MaybeUninit::uninit().assume_init(); let status = libc::gethostname(buffer.as_mut_ptr() as *mut c_char, buffer.len()); if status != 0 { - return None; + return Ok(None); } ptr::write(buffer[MAXHOSTNAMELEN - 1].as_mut_ptr() as *mut u8, b'\0'); // a little extra safety let hostname_length = libc::strlen(buffer.as_ptr() as *const c_char); @@ -532,24 +557,28 @@ impl UnownedWindow { .change_property( self.xwindow, pid_atom, - ffi::XA_CARDINAL, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::CARDINAL), + xproto::PropMode::REPLACE, &[libc::getpid() as util::Cardinal], - ) - .queue(); + )? + .ignore_error(); let flusher = self.xconn.change_property( self.xwindow, client_machine_atom, - ffi::XA_STRING, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::STRING), + xproto::PropMode::REPLACE, &hostname[0..hostname_length], ); - Some(flusher) + flusher.map(Some) } } - fn set_window_types(&self, window_types: Vec) -> util::Flusher<'_> { - let hint_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_WINDOW_TYPE\0") }; + fn set_window_types( + &self, + window_types: Vec, + ) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let hint_atom = atoms[_NET_WM_WINDOW_TYPE]; let atoms: Vec<_> = window_types .iter() .map(|t| t.as_atom(&self.xconn)) @@ -558,15 +587,16 @@ impl UnownedWindow { self.xconn.change_property( self.xwindow, hint_atom, - ffi::XA_ATOM, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::ATOM), + xproto::PropMode::REPLACE, &atoms, ) } - pub fn set_theme_inner(&self, theme: Option) -> util::Flusher<'_> { - let hint_atom = unsafe { self.xconn.get_atom_unchecked(b"_GTK_THEME_VARIANT\0") }; - let utf8_atom = unsafe { self.xconn.get_atom_unchecked(b"UTF8_STRING\0") }; + pub fn set_theme_inner(&self, theme: Option) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let hint_atom = atoms[_GTK_THEME_VARIANT]; + let utf8_atom = atoms[UTF8_STRING]; let variant = match theme { Some(Theme::Dark) => "dark", Some(Theme::Light) => "light", @@ -577,7 +607,7 @@ impl UnownedWindow { self.xwindow, hint_atom, utf8_atom, - util::PropMode::Replace, + xproto::PropMode::REPLACE, variant.as_bytes(), ) } @@ -585,23 +615,28 @@ impl UnownedWindow { #[inline] pub fn set_theme(&self, theme: Option) { self.set_theme_inner(theme) - .flush() .expect("Failed to change window theme") + .ignore_error(); + + self.xconn + .flush_requests() + .expect("Failed to change window theme"); } fn set_netwm( &self, operation: util::StateOperation, - properties: (c_long, c_long, c_long, c_long), - ) -> util::Flusher<'_> { - let state_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_STATE\0") }; + properties: (u32, u32, u32, u32), + ) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let state_atom = atoms[_NET_WM_STATE]; self.xconn.send_client_msg( self.xwindow, self.root, state_atom, - Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask), + Some(xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY), [ - operation as c_long, + operation as u32, properties.0, properties.1, properties.2, @@ -610,42 +645,45 @@ impl UnownedWindow { ) } - fn set_fullscreen_hint(&self, fullscreen: bool) -> util::Flusher<'_> { - let fullscreen_atom = - unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_STATE_FULLSCREEN\0") }; - let flusher = self.set_netwm(fullscreen.into(), (fullscreen_atom as c_long, 0, 0, 0)); + fn set_fullscreen_hint(&self, fullscreen: bool) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let fullscreen_atom = atoms[_NET_WM_STATE_FULLSCREEN]; + let flusher = self.set_netwm(fullscreen.into(), (fullscreen_atom, 0, 0, 0)); if fullscreen { // Ensure that the fullscreen window receives input focus to prevent // locking up the user's display. - unsafe { - (self.xconn.xlib.XSetInputFocus)( - self.xconn.display, + self.xconn + .xcb_connection() + .set_input_focus( + xproto::InputFocus::PARENT, self.xwindow, - ffi::RevertToParent, - ffi::CurrentTime, - ); - } + x11rb::CURRENT_TIME, + )? + .ignore_error(); } flusher } - fn set_fullscreen_inner(&self, fullscreen: Option) -> Option> { + fn set_fullscreen_inner( + &self, + fullscreen: Option, + ) -> Result>, X11Error> { let mut shared_state_lock = self.shared_state_lock(); match shared_state_lock.visibility { // Setting fullscreen on a window that is not visible will generate an error. Visibility::No | Visibility::YesWait => { shared_state_lock.desired_fullscreen = Some(fullscreen); - return None; + return Ok(None); } Visibility::Yes => (), } let old_fullscreen = shared_state_lock.fullscreen.clone(); if old_fullscreen == fullscreen { - return None; + return Ok(None); } shared_state_lock.fullscreen = fullscreen.clone(); @@ -682,9 +720,11 @@ impl UnownedWindow { let mut shared_state_lock = self.shared_state_lock(); if let Some(position) = shared_state_lock.restore_position.take() { drop(shared_state_lock); - self.set_position_inner(position.0, position.1).queue(); + self.set_position_inner(position.0, position.1) + .expect("Failed to restore window position") + .ignore_error(); } - Some(flusher) + flusher.map(Some) } Some(fullscreen) => { let (video_mode, monitor) = match fullscreen { @@ -701,7 +741,7 @@ impl UnownedWindow { // Don't set fullscreen on an invalid dummy monitor handle if monitor.is_dummy() { - return None; + return Ok(None); } if let Some(video_mode) = video_mode { @@ -739,8 +779,8 @@ impl UnownedWindow { self.shared_state_lock().restore_position = Some(window_position); let monitor_origin: (i32, i32) = monitor.position().into(); self.set_position_inner(monitor_origin.0, monitor_origin.1) - .queue(); - Some(self.set_fullscreen_hint(true)) + .expect_then_ignore_error("Failed to set window position"); + self.set_fullscreen_hint(true).map(Some) } } } @@ -757,9 +797,12 @@ impl UnownedWindow { #[inline] pub(crate) fn set_fullscreen(&self, fullscreen: Option) { - if let Some(flusher) = self.set_fullscreen_inner(fullscreen) { + if let Some(flusher) = self + .set_fullscreen_inner(fullscreen) + .expect("Failed to change window fullscreen state") + { flusher - .sync() + .check() .expect("Failed to change window fullscreen state"); self.invalidate_cached_frame_extents(); } @@ -770,9 +813,11 @@ impl UnownedWindow { let mut shared_state = self.shared_state_lock(); match shared_state.visibility { - Visibility::No => unsafe { - (self.xconn.xlib.XUnmapWindow)(self.xconn.display, self.xwindow); - }, + Visibility::No => self + .xconn + .xcb_connection() + .unmap_window(self.xwindow) + .expect_then_ignore_error("Failed to unmap window"), Visibility::Yes => (), Visibility::YesWait => { shared_state.visibility = Visibility::Yes; @@ -800,125 +845,139 @@ impl UnownedWindow { #[inline] pub fn is_minimized(&self) -> Option { - let state_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_STATE\0") }; - let state = self - .xconn - .get_property(self.xwindow, state_atom, ffi::XA_ATOM); - let hidden_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_STATE_HIDDEN\0") }; + let atoms = self.xconn.atoms(); + let state_atom = atoms[_NET_WM_STATE]; + let state = self.xconn.get_property( + self.xwindow, + state_atom, + xproto::Atom::from(xproto::AtomEnum::ATOM), + ); + let hidden_atom = atoms[_NET_WM_STATE_HIDDEN]; Some(match state { - Ok(atoms) => atoms.iter().any(|atom: &ffi::Atom| *atom == hidden_atom), + Ok(atoms) => atoms + .iter() + .any(|atom: &xproto::Atom| *atom as xproto::Atom == hidden_atom), _ => false, }) } - fn set_minimized_inner(&self, minimized: bool) -> util::Flusher<'_> { - unsafe { - if minimized { - let screen = (self.xconn.xlib.XDefaultScreen)(self.xconn.display); + fn set_minimized_inner(&self, minimized: bool) -> Result, X11Error> { + let atoms = self.xconn.atoms(); - (self.xconn.xlib.XIconifyWindow)(self.xconn.display, self.xwindow, screen); + if minimized { + let root_window = self.xconn.default_root().root; - util::Flusher::new(&self.xconn) - } else { - let atom = self.xconn.get_atom_unchecked(b"_NET_ACTIVE_WINDOW\0"); - - self.xconn.send_client_msg( - self.xwindow, - self.root, - atom, - Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask), - [1, ffi::CurrentTime as c_long, 0, 0, 0], - ) - } + self.xconn.send_client_msg( + self.xwindow, + root_window, + atoms[WM_CHANGE_STATE], + Some( + xproto::EventMask::SUBSTRUCTURE_REDIRECT + | xproto::EventMask::SUBSTRUCTURE_NOTIFY, + ), + [WmHintsState::Iconic as u32, 0, 0, 0, 0], + ) + } else { + self.xconn.send_client_msg( + self.xwindow, + self.root, + atoms[_NET_ACTIVE_WINDOW], + Some( + xproto::EventMask::SUBSTRUCTURE_REDIRECT + | xproto::EventMask::SUBSTRUCTURE_NOTIFY, + ), + [1, x11rb::CURRENT_TIME, 0, 0, 0], + ) } } #[inline] pub fn set_minimized(&self, minimized: bool) { self.set_minimized_inner(minimized) - .flush() + .expect_then_ignore_error("Failed to change window minimization"); + + self.xconn + .flush_requests() .expect("Failed to change window minimization"); } #[inline] pub fn is_maximized(&self) -> bool { - let state_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_STATE\0") }; - let state = self - .xconn - .get_property(self.xwindow, state_atom, ffi::XA_ATOM); - let horz_atom = unsafe { - self.xconn - .get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_HORZ\0") - }; - let vert_atom = unsafe { - self.xconn - .get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_VERT\0") - }; + let atoms = self.xconn.atoms(); + let state_atom = atoms[_NET_WM_STATE]; + let state = self.xconn.get_property( + self.xwindow, + state_atom, + xproto::Atom::from(xproto::AtomEnum::ATOM), + ); + let horz_atom = atoms[_NET_WM_STATE_MAXIMIZED_HORZ]; + let vert_atom = atoms[_NET_WM_STATE_MAXIMIZED_VERT]; match state { Ok(atoms) => { - let horz_maximized = atoms.iter().any(|atom: &ffi::Atom| *atom == horz_atom); - let vert_maximized = atoms.iter().any(|atom: &ffi::Atom| *atom == vert_atom); + let horz_maximized = atoms.iter().any(|atom: &xproto::Atom| *atom == horz_atom); + let vert_maximized = atoms.iter().any(|atom: &xproto::Atom| *atom == vert_atom); horz_maximized && vert_maximized } _ => false, } } - fn set_maximized_inner(&self, maximized: bool) -> util::Flusher<'_> { - let horz_atom = unsafe { - self.xconn - .get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_HORZ\0") - }; - let vert_atom = unsafe { - self.xconn - .get_atom_unchecked(b"_NET_WM_STATE_MAXIMIZED_VERT\0") - }; - self.set_netwm( - maximized.into(), - (horz_atom as c_long, vert_atom as c_long, 0, 0), - ) + fn set_maximized_inner(&self, maximized: bool) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let horz_atom = atoms[_NET_WM_STATE_MAXIMIZED_HORZ]; + let vert_atom = atoms[_NET_WM_STATE_MAXIMIZED_VERT]; + + self.set_netwm(maximized.into(), (horz_atom, vert_atom, 0, 0)) } #[inline] pub fn set_maximized(&self, maximized: bool) { self.set_maximized_inner(maximized) - .flush() + .expect("Failed to change window maximization") + .ignore_error(); + self.xconn + .flush_requests() .expect("Failed to change window maximization"); self.invalidate_cached_frame_extents(); } - fn set_title_inner(&self, title: &str) -> util::Flusher<'_> { - let wm_name_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_NAME\0") }; - let utf8_atom = unsafe { self.xconn.get_atom_unchecked(b"UTF8_STRING\0") }; + fn set_title_inner(&self, title: &str) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let title = CString::new(title).expect("Window title contained null byte"); - unsafe { - (self.xconn.xlib.XStoreName)( - self.xconn.display, - self.xwindow, - title.as_ptr() as *const c_char, - ); - self.xconn.change_property( + self.xconn + .change_property( self.xwindow, - wm_name_atom, - utf8_atom, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::WM_NAME), + xproto::Atom::from(xproto::AtomEnum::STRING), + xproto::PropMode::REPLACE, title.as_bytes(), - ) - } + )? + .ignore_error(); + self.xconn.change_property( + self.xwindow, + atoms[_NET_WM_NAME], + atoms[UTF8_STRING], + xproto::PropMode::REPLACE, + title.as_bytes(), + ) } #[inline] pub fn set_title(&self, title: &str) { self.set_title_inner(title) - .flush() + .expect_then_ignore_error("Failed to set window title"); + + self.xconn + .flush_requests() .expect("Failed to set window title"); } #[inline] pub fn set_transparent(&self, _transparent: bool) {} - fn set_decorations_inner(&self, decorations: bool) -> util::Flusher<'_> { + fn set_decorations_inner(&self, decorations: bool) -> Result, X11Error> { self.shared_state_lock().is_decorated = decorations; let mut hints = self.xconn.get_motif_hints(self.xwindow); @@ -930,7 +989,10 @@ impl UnownedWindow { #[inline] pub fn set_decorations(&self, decorations: bool) { self.set_decorations_inner(decorations) - .flush() + .expect("Failed to set decoration state") + .ignore_error(); + self.xconn + .flush_requests() .expect("Failed to set decoration state"); self.invalidate_cached_frame_extents(); } @@ -940,7 +1002,7 @@ impl UnownedWindow { self.shared_state_lock().is_decorated } - fn set_maximizable_inner(&self, maximizable: bool) -> util::Flusher<'_> { + fn set_maximizable_inner(&self, maximizable: bool) -> Result, X11Error> { let mut hints = self.xconn.get_motif_hints(self.xwindow); hints.set_maximizable(maximizable); @@ -948,47 +1010,50 @@ impl UnownedWindow { self.xconn.set_motif_hints(self.xwindow, &hints) } - fn toggle_atom(&self, atom_bytes: &[u8], enable: bool) -> util::Flusher<'_> { - let atom = unsafe { self.xconn.get_atom_unchecked(atom_bytes) }; - self.set_netwm(enable.into(), (atom as c_long, 0, 0, 0)) + fn toggle_atom(&self, atom_name: AtomName, enable: bool) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let atom = atoms[atom_name]; + self.set_netwm(enable.into(), (atom, 0, 0, 0)) } - fn set_window_level_inner(&self, level: WindowLevel) -> util::Flusher<'_> { - self.toggle_atom(b"_NET_WM_STATE_ABOVE\0", level == WindowLevel::AlwaysOnTop) - .queue(); - self.toggle_atom( - b"_NET_WM_STATE_BELOW\0", - level == WindowLevel::AlwaysOnBottom, - ) + fn set_window_level_inner(&self, level: WindowLevel) -> Result, X11Error> { + self.toggle_atom(_NET_WM_STATE_ABOVE, level == WindowLevel::AlwaysOnTop)? + .ignore_error(); + self.toggle_atom(_NET_WM_STATE_BELOW, level == WindowLevel::AlwaysOnBottom) } #[inline] pub fn set_window_level(&self, level: WindowLevel) { self.set_window_level_inner(level) - .flush() + .expect("Failed to set window-level state") + .ignore_error(); + self.xconn + .flush_requests() .expect("Failed to set window-level state"); } - fn set_icon_inner(&self, icon: Icon) -> util::Flusher<'_> { - let icon_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_ICON\0") }; + fn set_icon_inner(&self, icon: Icon) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let icon_atom = atoms[_NET_WM_ICON]; let data = icon.to_cardinals(); self.xconn.change_property( self.xwindow, icon_atom, - ffi::XA_CARDINAL, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::CARDINAL), + xproto::PropMode::REPLACE, data.as_slice(), ) } - fn unset_icon_inner(&self) -> util::Flusher<'_> { - let icon_atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_ICON\0") }; + fn unset_icon_inner(&self) -> Result, X11Error> { + let atoms = self.xconn.atoms(); + let icon_atom = atoms[_NET_WM_ICON]; let empty_data: [util::Cardinal; 0] = []; self.xconn.change_property( self.xwindow, icon_atom, - ffi::XA_CARDINAL, - util::PropMode::Replace, + xproto::Atom::from(xproto::AtomEnum::CARDINAL), + xproto::PropMode::REPLACE, &empty_data, ) } @@ -999,8 +1064,10 @@ impl UnownedWindow { Some(icon) => self.set_icon_inner(icon), None => self.unset_icon_inner(), } - .flush() - .expect("Failed to set icons"); + .expect("Failed to set icons") + .ignore_error(); + + self.xconn.flush_requests().expect("Failed to set icons"); } #[inline] @@ -1015,17 +1082,29 @@ impl UnownedWindow { } if visible { - unsafe { - (self.xconn.xlib.XMapRaised)(self.xconn.display, self.xwindow); - } + self.xconn + .xcb_connection() + .map_window(self.xwindow) + .expect("Failed to call `xcb_map_window`") + .ignore_error(); + self.xconn + .xcb_connection() + .configure_window( + self.xwindow, + &xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::ABOVE), + ) + .expect("Failed to call `xcb_configure_window`") + .ignore_error(); self.xconn .flush_requests() .expect("Failed to call XMapRaised"); shared_state.visibility = Visibility::YesWait; } else { - unsafe { - (self.xconn.xlib.XUnmapWindow)(self.xconn.display, self.xwindow); - } + self.xconn + .xcb_connection() + .unmap_window(self.xwindow) + .expect("Failed to call `xcb_unmap_window`") + .ignore_error(); self.xconn .flush_requests() .expect("Failed to call XUnmapWindow"); @@ -1077,7 +1156,7 @@ impl UnownedWindow { // is BadWindow, and if the window handle is bad we have bigger problems. self.xconn .translate_coords(self.xwindow, self.root) - .map(|coords| (coords.x_rel_root, coords.y_rel_root)) + .map(|coords| (coords.dst_x.into(), coords.dst_y.into())) .unwrap() } @@ -1086,7 +1165,11 @@ impl UnownedWindow { Ok(self.inner_position_physical().into()) } - pub(crate) fn set_position_inner(&self, mut x: i32, mut y: i32) -> util::Flusher<'_> { + pub(crate) fn set_position_inner( + &self, + mut x: i32, + mut y: i32, + ) -> Result, X11Error> { // There are a few WMs that set client area position rather than window position, so // we'll translate for consistency. if util::wm_name_is_one_of(&["Enlightenment", "FVWM"]) { @@ -1099,16 +1182,17 @@ impl UnownedWindow { return self.set_position_inner(x, y); } } - unsafe { - (self.xconn.xlib.XMoveWindow)(self.xconn.display, self.xwindow, x as c_int, y as c_int); - } - util::Flusher::new(&self.xconn) + + self.xconn + .xcb_connection() + .configure_window(self.xwindow, &xproto::ConfigureWindowAux::new().x(x).y(y)) + .map_err(Into::into) } pub(crate) fn set_position_physical(&self, x: i32, y: i32) { self.set_position_inner(x, y) - .flush() - .expect("Failed to call `XMoveWindow`"); + .expect("Failed to call `XMoveWindow`") + .ignore_error(); } #[inline] @@ -1122,7 +1206,7 @@ impl UnownedWindow { // is BadWindow, and if the window handle is bad we have bigger problems. self.xconn .get_geometry(self.xwindow) - .map(|geo| (geo.width, geo.height)) + .map(|geo| (geo.width.into(), geo.height.into())) .unwrap() } @@ -1144,16 +1228,19 @@ impl UnownedWindow { } pub(crate) fn request_inner_size_physical(&self, width: u32, height: u32) { - unsafe { - (self.xconn.xlib.XResizeWindow)( - self.xconn.display, + self.xconn + .xcb_connection() + .configure_window( self.xwindow, - width as c_uint, - height as c_uint, - ); - self.xconn.flush_requests() - } - .expect("Failed to call `XResizeWindow`"); + &xproto::ConfigureWindowAux::new() + .width(width) + .height(height), + ) + .expect("Failed to call `xcb_configure_window`") + .ignore_error(); + self.xconn + .flush_requests() + .expect("Failed to call XResizeWindow"); } #[inline] @@ -1162,30 +1249,42 @@ impl UnownedWindow { let size = size.to_physical::(scale_factor).into(); if !self.shared_state_lock().is_resizable { self.update_normal_hints(|normal_hints| { - normal_hints.set_min_size(Some(size)); - normal_hints.set_max_size(Some(size)); + normal_hints.min_size = Some(size); + normal_hints.max_size = Some(size); }) .expect("Failed to call `XSetWMNormalHints`"); } - self.request_inner_size_physical(size.0, size.1); + self.request_inner_size_physical(size.0 as u32, size.1 as u32); None } - fn update_normal_hints(&self, callback: F) -> Result<(), XError> + fn update_normal_hints(&self, callback: F) -> Result<(), X11Error> where - F: FnOnce(&mut util::NormalHints<'_>), + F: FnOnce(&mut WmSizeHints), { - let mut normal_hints = self.xconn.get_normal_hints(self.xwindow)?; + let mut normal_hints = WmSizeHints::get( + self.xconn.xcb_connection(), + self.xwindow as xproto::Window, + xproto::AtomEnum::WM_NORMAL_HINTS, + )? + .reply()?; callback(&mut normal_hints); - self.xconn - .set_normal_hints(self.xwindow, normal_hints) - .flush() + normal_hints + .set( + self.xconn.xcb_connection(), + self.xwindow as xproto::Window, + xproto::AtomEnum::WM_NORMAL_HINTS, + )? + .ignore_error(); + Ok(()) } pub(crate) fn set_min_inner_size_physical(&self, dimensions: Option<(u32, u32)>) { - self.update_normal_hints(|normal_hints| normal_hints.set_min_size(dimensions)) - .expect("Failed to call `XSetWMNormalHints`"); + self.update_normal_hints(|normal_hints| { + normal_hints.min_size = dimensions.map(|(w, h)| (w as i32, h as i32)) + }) + .expect("Failed to call `XSetWMNormalHints`"); } #[inline] @@ -1197,8 +1296,10 @@ impl UnownedWindow { } pub(crate) fn set_max_inner_size_physical(&self, dimensions: Option<(u32, u32)>) { - self.update_normal_hints(|normal_hints| normal_hints.set_max_size(dimensions)) - .expect("Failed to call `XSetWMNormalHints`"); + self.update_normal_hints(|normal_hints| { + normal_hints.max_size = dimensions.map(|(w, h)| (w as i32, h as i32)) + }) + .expect("Failed to call `XSetWMNormalHints`"); } #[inline] @@ -1211,19 +1312,23 @@ impl UnownedWindow { #[inline] pub fn resize_increments(&self) -> Option> { - self.xconn - .get_normal_hints(self.xwindow) - .ok() - .and_then(|hints| hints.get_resize_increments()) - .map(Into::into) + WmSizeHints::get( + self.xconn.xcb_connection(), + self.xwindow as xproto::Window, + xproto::AtomEnum::WM_NORMAL_HINTS, + ) + .ok() + .and_then(|cookie| cookie.reply().ok()) + .and_then(|hints| hints.size_increment) + .map(|(width, height)| (width as u32, height as u32).into()) } #[inline] pub fn set_resize_increments(&self, increments: Option) { self.shared_state_lock().resize_increments = increments; let physical_increments = - increments.map(|increments| increments.to_physical::(self.scale_factor()).into()); - self.update_normal_hints(|hints| hints.set_resize_increments(physical_increments)) + increments.map(|increments| increments.to_physical::(self.scale_factor()).into()); + self.update_normal_hints(|hints| hints.size_increment = physical_increments) .expect("Failed to call `XSetWMNormalHints`"); } @@ -1238,15 +1343,16 @@ impl UnownedWindow { let scale_factor = new_scale_factor / old_scale_factor; self.update_normal_hints(|normal_hints| { let dpi_adjuster = - |size: Size| -> (u32, u32) { size.to_physical::(new_scale_factor).into() }; + |size: Size| -> (i32, i32) { size.to_physical::(new_scale_factor).into() }; let max_size = shared_state.max_inner_size.map(dpi_adjuster); let min_size = shared_state.min_inner_size.map(dpi_adjuster); let resize_increments = shared_state.resize_increments.map(dpi_adjuster); let base_size = shared_state.base_size.map(dpi_adjuster); - normal_hints.set_max_size(max_size); - normal_hints.set_min_size(min_size); - normal_hints.set_resize_increments(resize_increments); - normal_hints.set_base_size(base_size); + + normal_hints.max_size = max_size; + normal_hints.min_size = min_size; + normal_hints.size_increment = resize_increments; + normal_hints.base_size = base_size; }) .expect("Failed to update normal hints"); @@ -1277,18 +1383,20 @@ impl UnownedWindow { }; self.shared_state_lock().is_resizable = resizable; - self.set_maximizable_inner(resizable).queue(); + self.set_maximizable_inner(resizable) + .expect("Failed to call `XSetWMNormalHints`") + .ignore_error(); let scale_factor = self.scale_factor(); let min_inner_size = min_size - .map(|size| size.to_physical::(scale_factor)) + .map(|size| size.to_physical::(scale_factor)) .map(Into::into); let max_inner_size = max_size - .map(|size| size.to_physical::(scale_factor)) + .map(|size| size.to_physical::(scale_factor)) .map(Into::into); self.update_normal_hints(|normal_hints| { - normal_hints.set_min_size(min_inner_size); - normal_hints.set_max_size(max_inner_size); + normal_hints.min_size = min_inner_size; + normal_hints.max_size = max_inner_size; }) .expect("Failed to call `XSetWMNormalHints`"); } @@ -1318,12 +1426,12 @@ impl UnownedWindow { #[inline] pub fn xlib_window(&self) -> c_ulong { - self.xwindow + self.xwindow as ffi::Window } #[inline] pub fn xcb_connection(&self) -> *mut c_void { - unsafe { (self.xconn.xlib_xcb.XGetXCBConnection)(self.xconn.display) as *mut _ } + self.xconn.xcb_connection().get_raw_xcb_connection() } #[inline] @@ -1342,54 +1450,62 @@ impl UnownedWindow { return Ok(()); } - unsafe { + { // We ungrab before grabbing to prevent passive grabs from causing `AlreadyGrabbed`. // Therefore, this is common to both codepaths. - (self.xconn.xlib.XUngrabPointer)(self.xconn.display, ffi::CurrentTime); + self.xconn + .xcb_connection() + .ungrab_pointer(x11rb::CURRENT_TIME) + .expect("Failed to call `xcb_ungrab_pointer`") + .ignore_error(); } let result = match mode { - CursorGrabMode::None => self - .xconn - .flush_requests() - .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err)))), + CursorGrabMode::None => self.xconn.flush_requests().map_err(|err| { + ExternalError::Os(os_error!(OsError::XError(X11Error::Xlib(err).into()))) + }), CursorGrabMode::Confined => { - let result = unsafe { - (self.xconn.xlib.XGrabPointer)( - self.xconn.display, - self.xwindow, - ffi::True, - (ffi::ButtonPressMask - | ffi::ButtonReleaseMask - | ffi::EnterWindowMask - | ffi::LeaveWindowMask - | ffi::PointerMotionMask - | ffi::PointerMotionHintMask - | ffi::Button1MotionMask - | ffi::Button2MotionMask - | ffi::Button3MotionMask - | ffi::Button4MotionMask - | ffi::Button5MotionMask - | ffi::ButtonMotionMask - | ffi::KeymapStateMask) as c_uint, - ffi::GrabModeAsync, - ffi::GrabModeAsync, - self.xwindow, - 0, - ffi::CurrentTime, - ) + let result = { + self.xconn + .xcb_connection() + .grab_pointer( + true as _, + self.xwindow, + xproto::EventMask::BUTTON_PRESS + | xproto::EventMask::BUTTON_RELEASE + | xproto::EventMask::ENTER_WINDOW + | xproto::EventMask::LEAVE_WINDOW + | xproto::EventMask::POINTER_MOTION + | xproto::EventMask::POINTER_MOTION_HINT + | xproto::EventMask::BUTTON1_MOTION + | xproto::EventMask::BUTTON2_MOTION + | xproto::EventMask::BUTTON3_MOTION + | xproto::EventMask::BUTTON4_MOTION + | xproto::EventMask::BUTTON5_MOTION + | xproto::EventMask::KEYMAP_STATE, + xproto::GrabMode::ASYNC, + xproto::GrabMode::ASYNC, + self.xwindow, + 0u32, + x11rb::CURRENT_TIME, + ) + .expect("Failed to call `grab_pointer`") + .reply() + .expect("Failed to receive reply from `grab_pointer`") }; - match result { - ffi::GrabSuccess => Ok(()), - ffi::AlreadyGrabbed => { + match result.status { + xproto::GrabStatus::SUCCESS => Ok(()), + xproto::GrabStatus::ALREADY_GRABBED => { Err("Cursor could not be confined: already confined by another client") } - ffi::GrabInvalidTime => Err("Cursor could not be confined: invalid time"), - ffi::GrabNotViewable => { + xproto::GrabStatus::INVALID_TIME => { + Err("Cursor could not be confined: invalid time") + } + xproto::GrabStatus::NOT_VIEWABLE => { Err("Cursor could not be confined: confine location not viewable") } - ffi::GrabFrozen => { + xproto::GrabStatus::FROZEN => { Err("Cursor could not be confined: frozen by another client") } _ => unreachable!(), @@ -1431,11 +1547,16 @@ impl UnownedWindow { } pub fn set_cursor_position_physical(&self, x: i32, y: i32) -> Result<(), ExternalError> { - unsafe { - (self.xconn.xlib.XWarpPointer)(self.xconn.display, 0, self.xwindow, 0, 0, 0, 0, x, y); + { self.xconn - .flush_requests() - .map_err(|e| ExternalError::Os(os_error!(OsError::XError(e)))) + .xcb_connection() + .warp_pointer(x11rb::NONE, self.xwindow, 0, 0, 0, 0, x as _, y as _) + .map_err(|e| { + ExternalError::Os(os_error!(OsError::XError(X11Error::from(e).into()))) + })?; + self.xconn.flush_requests().map_err(|e| { + ExternalError::Os(os_error!(OsError::XError(X11Error::Xlib(e).into()))) + }) } } @@ -1474,21 +1595,26 @@ impl UnownedWindow { let pointer = self .xconn .query_pointer(self.xwindow, util::VIRTUAL_CORE_POINTER) - .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err))))?; + .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err.into()))))?; let window = self.inner_position().map_err(ExternalError::NotSupported)?; - let message = unsafe { self.xconn.get_atom_unchecked(b"_NET_WM_MOVERESIZE\0") }; + let atoms = self.xconn.atoms(); + let message = atoms[_NET_WM_MOVERESIZE]; // we can't use `set_cursor_grab(false)` here because it doesn't run `XUngrabPointer` // if the cursor isn't currently grabbed let mut grabbed_lock = self.cursor_grabbed_mode.lock().unwrap(); - unsafe { - (self.xconn.xlib.XUngrabPointer)(self.xconn.display, ffi::CurrentTime); - } self.xconn - .flush_requests() - .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err))))?; + .xcb_connection() + .ungrab_pointer(x11rb::CURRENT_TIME) + .map_err(|err| { + ExternalError::Os(os_error!(OsError::XError(X11Error::from(err).into()))) + })? + .ignore_error(); + self.xconn.flush_requests().map_err(|err| { + ExternalError::Os(os_error!(OsError::XError(X11Error::Xlib(err).into()))) + })?; *grabbed_lock = CursorGrabMode::None; // we keep the lock until we are done @@ -1497,27 +1623,33 @@ impl UnownedWindow { self.xwindow, self.root, message, - Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask), + Some( + xproto::EventMask::SUBSTRUCTURE_REDIRECT + | xproto::EventMask::SUBSTRUCTURE_NOTIFY, + ), [ - (window.x as c_long + pointer.win_x as c_long), - (window.y as c_long + pointer.win_y as c_long), + (window.x as u32 + pointer.win_x as u32), + (window.y as u32 + pointer.win_y as u32), action.try_into().unwrap(), - ffi::Button1 as c_long, + 1, // Button 1 1, ], ) - .flush() - .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err)))) + .map_err(|err| ExternalError::Os(os_error!(OsError::XError(err.into()))))?; + + self.xconn.flush_requests().map_err(|err| { + ExternalError::Os(os_error!(OsError::XError(X11Error::Xlib(err).into()))) + }) } #[inline] pub fn set_ime_cursor_area(&self, spot: Position, _size: Size) { let (x, y) = spot.to_physical::(self.scale_factor()).into(); - let _ = self - .ime_sender - .lock() - .unwrap() - .send(ImeRequest::Position(self.xwindow, x, y)); + let _ = self.ime_sender.lock().unwrap().send(ImeRequest::Position( + self.xwindow as ffi::Window, + x, + y, + )); } #[inline] @@ -1526,7 +1658,7 @@ impl UnownedWindow { .ime_sender .lock() .unwrap() - .send(ImeRequest::Allow(self.xwindow, allowed)); + .send(ImeRequest::Allow(self.xwindow as ffi::Window, allowed)); } #[inline] @@ -1534,8 +1666,9 @@ impl UnownedWindow { #[inline] pub fn focus_window(&self) { - let state_atom = unsafe { self.xconn.get_atom_unchecked(b"WM_STATE\0") }; - let state_type_atom = unsafe { self.xconn.get_atom_unchecked(b"CARD32\0") }; + let atoms = self.xconn.atoms(); + let state_atom = atoms[WM_STATE]; + let state_type_atom = atoms[CARD32]; let is_minimized = if let Ok(state) = self.xconn .get_property(self.xwindow, state_atom, state_type_atom) @@ -1550,15 +1683,21 @@ impl UnownedWindow { }; if is_visible && !is_minimized { - let atom = unsafe { self.xconn.get_atom_unchecked(b"_NET_ACTIVE_WINDOW\0") }; - let flusher = self.xconn.send_client_msg( - self.xwindow, - self.root, - atom, - Some(ffi::SubstructureRedirectMask | ffi::SubstructureNotifyMask), - [1, ffi::CurrentTime as c_long, 0, 0, 0], - ); - if let Err(e) = flusher.flush() { + let atom = atoms[_NET_ACTIVE_WINDOW]; + self.xconn + .send_client_msg( + self.xwindow, + self.root, + atom, + Some( + xproto::EventMask::SUBSTRUCTURE_REDIRECT + | xproto::EventMask::SUBSTRUCTURE_NOTIFY, + ), + [1, x11rb::CURRENT_TIME, 0, 0, 0], + ) + .expect("Failed to send client message") + .ignore_error(); + if let Err(e) = self.xconn.flush_requests() { log::error!( "`flush` returned an error when focusing the window. Error was: {}", e @@ -1569,19 +1708,17 @@ impl UnownedWindow { #[inline] pub fn request_user_attention(&self, request_type: Option) { - let mut wm_hints = self - .xconn - .get_wm_hints(self.xwindow) - .expect("`XGetWMHints` failed"); - if request_type.is_some() { - wm_hints.flags |= ffi::XUrgencyHint; - } else { - wm_hints.flags &= !ffi::XUrgencyHint; - } - self.xconn - .set_wm_hints(self.xwindow, wm_hints) - .flush() - .expect("Failed to set urgency hint"); + let mut wm_hints = + WmHints::get(self.xconn.xcb_connection(), self.xwindow as xproto::Window) + .ok() + .and_then(|cookie| cookie.reply().ok()) + .unwrap_or_default(); + + wm_hints.urgent = request_type.is_some(); + wm_hints + .set(self.xconn.xcb_connection(), self.xwindow as xproto::Window) + .expect("Failed to set WM hints") + .ignore_error(); } #[inline] diff --git a/src/platform_impl/linux/x11/xdisplay.rs b/src/platform_impl/linux/x11/xdisplay.rs index e3d7f4ffdc..3848bdec6c 100644 --- a/src/platform_impl/linux/x11/xdisplay.rs +++ b/src/platform_impl/linux/x11/xdisplay.rs @@ -1,8 +1,14 @@ -use std::{collections::HashMap, error::Error, fmt, os::raw::c_int, ptr, sync::Mutex}; +use std::{ + collections::HashMap, + error::Error, + fmt, ptr, + sync::{Arc, Mutex}, +}; use crate::window::CursorIcon; -use super::ffi; +use super::{atoms::Atoms, ffi}; +use x11rb::{connection::Connection, protocol::xproto, xcb_ffi::XCBConnection}; /// A connection to an X server. pub(crate) struct XConnection { @@ -11,9 +17,21 @@ pub(crate) struct XConnection { pub xrandr: ffi::Xrandr_2_2_0, pub xcursor: ffi::Xcursor, pub xinput2: ffi::XInput2, - pub xlib_xcb: ffi::Xlib_xcb, pub display: *mut ffi::Display, - pub x11_fd: c_int, + /// The manager for the XCB connection. + /// + /// The `Option` ensures that we can drop it before we close the `Display`. + xcb: Option, + + /// The atoms used by `winit`. + /// + /// This is a large structure, so I've elected to Box it to make accessing the fields of + /// this struct easier. Feel free to unbox it if you like kicking puppies. + atoms: Box, + + /// The index of the default screen. + default_screen: usize, + pub latest_error: Mutex>, pub cursor_cache: Mutex, ffi::Cursor>>, } @@ -45,17 +63,38 @@ impl XConnection { display }; - // Get X11 socket file descriptor - let fd = unsafe { (xlib.XConnectionNumber)(display) }; + // Open the x11rb XCB connection. + let xcb = { + // Get a pointer to the underlying XCB connection + let xcb_connection = + unsafe { (xlib_xcb.XGetXCBConnection)(display as *mut ffi::Display) }; + assert!(!xcb_connection.is_null()); + + // Wrap the XCB connection in an x11rb XCB connection + let conn = + unsafe { XCBConnection::from_raw_xcb_connection(xcb_connection.cast(), false) }; + + conn.map_err(|e| XNotSupported::XcbConversionError(Arc::new(WrapConnectError(e))))? + }; + + // Get the default screen. + let default_screen = unsafe { (xlib.XDefaultScreen)(display) } as usize; + + // Fetch the atoms. + let atoms = Atoms::new(&xcb) + .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))? + .reply() + .map_err(|e| XNotSupported::XcbConversionError(Arc::new(e)))?; Ok(XConnection { xlib, xrandr, xcursor, xinput2, - xlib_xcb, display, - x11_fd: fd, + xcb: Some(xcb), + atoms: Box::new(atoms), + default_screen, latest_error: Mutex::new(None), cursor_cache: Default::default(), }) @@ -71,6 +110,32 @@ impl XConnection { Ok(()) } } + + /// Get the underlying XCB connection. + #[inline] + pub fn xcb_connection(&self) -> &XCBConnection { + self.xcb + .as_ref() + .expect("xcb_connection somehow called after drop?") + } + + /// Get the list of atoms. + #[inline] + pub fn atoms(&self) -> &Atoms { + &self.atoms + } + + /// Get the index of the default screen. + #[inline] + pub fn default_screen_index(&self) -> usize { + self.default_screen + } + + /// Get the default screen. + #[inline] + pub fn default_root(&self) -> &xproto::Screen { + &self.xcb_connection().setup().roots[self.default_screen] + } } impl fmt::Debug for XConnection { @@ -82,6 +147,7 @@ impl fmt::Debug for XConnection { impl Drop for XConnection { #[inline] fn drop(&mut self) { + self.xcb = None; unsafe { (self.xlib.XCloseDisplay)(self.display) }; } } @@ -112,8 +178,12 @@ impl fmt::Display for XError { pub enum XNotSupported { /// Failed to load one or several shared libraries. LibraryOpenError(ffi::OpenError), + /// Connecting to the X server with `XOpenDisplay` failed. - XOpenDisplayFailed, // TODO: add better message + XOpenDisplayFailed, // TODO: add better message. + + /// We encountered an error while converting the connection to XCB. + XcbConversionError(Arc), } impl From for XNotSupported { @@ -128,6 +198,7 @@ impl XNotSupported { match self { XNotSupported::LibraryOpenError(_) => "Failed to load one of xlib's shared libraries", XNotSupported::XOpenDisplayFailed => "Failed to open connection to X server", + XNotSupported::XcbConversionError(_) => "Failed to convert Xlib connection to XCB", } } } @@ -137,6 +208,7 @@ impl Error for XNotSupported { fn source(&self) -> Option<&(dyn Error + 'static)> { match *self { XNotSupported::LibraryOpenError(ref err) => Some(err), + XNotSupported::XcbConversionError(ref err) => Some(&**err), _ => None, } } @@ -147,3 +219,19 @@ impl fmt::Display for XNotSupported { formatter.write_str(self.description()) } } + +/// A newtype wrapper around a `ConnectError` that can't be accessed by downstream libraries. +/// +/// Without this, `x11rb` would become a public dependency. +#[derive(Debug)] +struct WrapConnectError(x11rb::rust_connection::ConnectError); + +impl fmt::Display for WrapConnectError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&self.0, f) + } +} + +impl Error for WrapConnectError { + // We can't implement `source()` here or otherwise risk exposing `x11rb`. +} From 431edcbbc306590b117f4c7912ebd83f57d1ac0f Mon Sep 17 00:00:00 2001 From: John Nunley Date: Fri, 30 Jun 2023 18:36:16 -0700 Subject: [PATCH 2/3] Remaining review comments --- src/platform_impl/linux/x11/mod.rs | 6 +-- src/platform_impl/linux/x11/window.rs | 57 ++++++++++----------------- 2 files changed, 22 insertions(+), 41 deletions(-) diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs index ab6c7580d0..e1d6575669 100644 --- a/src/platform_impl/linux/x11/mod.rs +++ b/src/platform_impl/linux/x11/mod.rs @@ -323,8 +323,7 @@ impl EventLoop { ffi::XIAllDevices as _, x11rb::protocol::xinput::XIEventMask::HIERARCHY, ) - .expect("Failed to register for XInput2 device hotplug events") - .ignore_error(); + .expect_then_ignore_error("Failed to register for XInput2 device hotplug events"); get_xtarget(&target) .xconn @@ -626,8 +625,7 @@ impl EventLoopWindowTarget { self.xconn .select_xinput_events(self.root, ffi::XIAllMasterDevices as _, mask) - .expect("Failed to update device event filter") - .ignore_error(); + .expect_then_ignore_error("Failed to update device event filter"); } pub fn raw_display_handle(&self) -> raw_window_handle::RawDisplayHandle { diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index 87bd290d6a..23a1f116d2 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -721,8 +721,7 @@ impl UnownedWindow { if let Some(position) = shared_state_lock.restore_position.take() { drop(shared_state_lock); self.set_position_inner(position.0, position.1) - .expect("Failed to restore window position") - .ignore_error(); + .expect_then_ignore_error("Failed to restore window position"); } flusher.map(Some) } @@ -934,8 +933,7 @@ impl UnownedWindow { #[inline] pub fn set_maximized(&self, maximized: bool) { self.set_maximized_inner(maximized) - .expect("Failed to change window maximization") - .ignore_error(); + .expect_then_ignore_error("Failed to change window maximization"); self.xconn .flush_requests() .expect("Failed to change window maximization"); @@ -989,8 +987,7 @@ impl UnownedWindow { #[inline] pub fn set_decorations(&self, decorations: bool) { self.set_decorations_inner(decorations) - .expect("Failed to set decoration state") - .ignore_error(); + .expect_then_ignore_error("Failed to set decoration state"); self.xconn .flush_requests() .expect("Failed to set decoration state"); @@ -1025,8 +1022,7 @@ impl UnownedWindow { #[inline] pub fn set_window_level(&self, level: WindowLevel) { self.set_window_level_inner(level) - .expect("Failed to set window-level state") - .ignore_error(); + .expect_then_ignore_error("Failed to set window-level state"); self.xconn .flush_requests() .expect("Failed to set window-level state"); @@ -1064,8 +1060,7 @@ impl UnownedWindow { Some(icon) => self.set_icon_inner(icon), None => self.unset_icon_inner(), } - .expect("Failed to set icons") - .ignore_error(); + .expect_then_ignore_error("Failed to set icons"); self.xconn.flush_requests().expect("Failed to set icons"); } @@ -1085,16 +1080,14 @@ impl UnownedWindow { self.xconn .xcb_connection() .map_window(self.xwindow) - .expect("Failed to call `xcb_map_window`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `xcb_map_window`"); self.xconn .xcb_connection() .configure_window( self.xwindow, &xproto::ConfigureWindowAux::new().stack_mode(xproto::StackMode::ABOVE), ) - .expect("Failed to call `xcb_configure_window`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `xcb_configure_window`"); self.xconn .flush_requests() .expect("Failed to call XMapRaised"); @@ -1103,8 +1096,7 @@ impl UnownedWindow { self.xconn .xcb_connection() .unmap_window(self.xwindow) - .expect("Failed to call `xcb_unmap_window`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `xcb_unmap_window`"); self.xconn .flush_requests() .expect("Failed to call XUnmapWindow"); @@ -1191,8 +1183,7 @@ impl UnownedWindow { pub(crate) fn set_position_physical(&self, x: i32, y: i32) { self.set_position_inner(x, y) - .expect("Failed to call `XMoveWindow`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `XMoveWindow`"); } #[inline] @@ -1236,8 +1227,7 @@ impl UnownedWindow { .width(width) .height(height), ) - .expect("Failed to call `xcb_configure_window`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `xcb_configure_window`"); self.xconn .flush_requests() .expect("Failed to call XResizeWindow"); @@ -1384,8 +1374,7 @@ impl UnownedWindow { self.shared_state_lock().is_resizable = resizable; self.set_maximizable_inner(resizable) - .expect("Failed to call `XSetWMNormalHints`") - .ignore_error(); + .expect_then_ignore_error("Failed to call `XSetWMNormalHints`"); let scale_factor = self.scale_factor(); let min_inner_size = min_size @@ -1450,15 +1439,12 @@ impl UnownedWindow { return Ok(()); } - { - // We ungrab before grabbing to prevent passive grabs from causing `AlreadyGrabbed`. - // Therefore, this is common to both codepaths. - self.xconn - .xcb_connection() - .ungrab_pointer(x11rb::CURRENT_TIME) - .expect("Failed to call `xcb_ungrab_pointer`") - .ignore_error(); - } + // We ungrab before grabbing to prevent passive grabs from causing `AlreadyGrabbed`. + // Therefore, this is common to both codepaths. + self.xconn + .xcb_connection() + .ungrab_pointer(x11rb::CURRENT_TIME) + .expect_then_ignore_error("Failed to call `xcb_ungrab_pointer`"); let result = match mode { CursorGrabMode::None => self.xconn.flush_requests().map_err(|err| { @@ -1683,20 +1669,18 @@ impl UnownedWindow { }; if is_visible && !is_minimized { - let atom = atoms[_NET_ACTIVE_WINDOW]; self.xconn .send_client_msg( self.xwindow, self.root, - atom, + atoms[_NET_ACTIVE_WINDOW], Some( xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY, ), [1, x11rb::CURRENT_TIME, 0, 0, 0], ) - .expect("Failed to send client message") - .ignore_error(); + .expect_then_ignore_error("Failed to send client message"); if let Err(e) = self.xconn.flush_requests() { log::error!( "`flush` returned an error when focusing the window. Error was: {}", @@ -1717,8 +1701,7 @@ impl UnownedWindow { wm_hints.urgent = request_type.is_some(); wm_hints .set(self.xconn.xcb_connection(), self.xwindow as xproto::Window) - .expect("Failed to set WM hints") - .ignore_error(); + .expect_then_ignore_error("Failed to set WM hints"); } #[inline] From 961e2daadbf6590defc2e031d2eca681efd67ddd Mon Sep 17 00:00:00 2001 From: John Nunley Date: Sun, 2 Jul 2023 11:17:28 -0700 Subject: [PATCH 3/3] Clamp dimension values instead of truncating --- src/platform_impl/linux/x11/window.rs | 141 +++++++++++++++----------- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/src/platform_impl/linux/x11/window.rs b/src/platform_impl/linux/x11/window.rs index 23a1f116d2..b7f6215a61 100644 --- a/src/platform_impl/linux/x11/window.rs +++ b/src/platform_impl/linux/x11/window.rs @@ -381,58 +381,56 @@ impl UnownedWindow { leap!(window.set_window_types(pl_attribs.x11_window_types)).ignore_error(); - // set size hints - { - let mut min_inner_size = window_attrs - .min_inner_size - .map(|size| size.to_physical::(scale_factor)); - let mut max_inner_size = window_attrs - .max_inner_size - .map(|size| size.to_physical::(scale_factor)); - - if !window_attrs.resizable { - if util::wm_name_is_one_of(&["Xfwm4"]) { - warn!("To avoid a WM bug, disabling resizing has no effect on Xfwm4"); - } else { - max_inner_size = Some(dimensions.into()); - min_inner_size = Some(dimensions.into()); - } + // Set size hints. + let mut min_inner_size = window_attrs + .min_inner_size + .map(|size| size.to_physical::(scale_factor)); + let mut max_inner_size = window_attrs + .max_inner_size + .map(|size| size.to_physical::(scale_factor)); + + if !window_attrs.resizable { + if util::wm_name_is_one_of(&["Xfwm4"]) { + warn!("To avoid a WM bug, disabling resizing has no effect on Xfwm4"); + } else { + max_inner_size = Some(dimensions.into()); + min_inner_size = Some(dimensions.into()); } - - let shared_state = window.shared_state.get_mut().unwrap(); - shared_state.min_inner_size = min_inner_size.map(Into::into); - shared_state.max_inner_size = max_inner_size.map(Into::into); - shared_state.resize_increments = window_attrs.resize_increments; - shared_state.base_size = pl_attribs.base_size; - - let normal_hints = WmSizeHints { - position: position.map(|PhysicalPosition { x, y }| { - (WmSizeHintsSpecification::UserSpecified, x, y) - }), - size: Some(( - WmSizeHintsSpecification::UserSpecified, - dimensions.0 as i32, - dimensions.1 as i32, - )), - max_size: max_inner_size.map(Into::into), - min_size: min_inner_size.map(Into::into), - size_increment: window_attrs - .resize_increments - .map(|size| size.to_physical::(scale_factor).into()), - base_size: pl_attribs - .base_size - .map(|size| size.to_physical::(scale_factor).into()), - aspect: None, - win_gravity: None, - }; - leap!(leap!(normal_hints.set( - xconn.xcb_connection(), - window.xwindow as xproto::Window, - xproto::AtomEnum::WM_NORMAL_HINTS, - )) - .check()) } + let shared_state = window.shared_state.get_mut().unwrap(); + shared_state.min_inner_size = min_inner_size.map(Into::into); + shared_state.max_inner_size = max_inner_size.map(Into::into); + shared_state.resize_increments = window_attrs.resize_increments; + shared_state.base_size = pl_attribs.base_size; + + let normal_hints = WmSizeHints { + position: position.map(|PhysicalPosition { x, y }| { + (WmSizeHintsSpecification::UserSpecified, x, y) + }), + size: Some(( + WmSizeHintsSpecification::UserSpecified, + cast_dimension_to_hint(dimensions.0), + cast_dimension_to_hint(dimensions.1), + )), + max_size: max_inner_size.map(cast_physical_size_to_hint), + min_size: min_inner_size.map(cast_physical_size_to_hint), + size_increment: window_attrs + .resize_increments + .map(|size| cast_size_to_hint(size, scale_factor)), + base_size: pl_attribs + .base_size + .map(|size| cast_size_to_hint(size, scale_factor)), + aspect: None, + win_gravity: None, + }; + leap!(leap!(normal_hints.set( + xconn.xcb_connection(), + window.xwindow as xproto::Window, + xproto::AtomEnum::WM_NORMAL_HINTS, + )) + .check()); + // Set window icons if let Some(icon) = window_attrs.window_icon { leap!(window.set_icon_inner(icon)).ignore_error(); @@ -1167,8 +1165,8 @@ impl UnownedWindow { if util::wm_name_is_one_of(&["Enlightenment", "FVWM"]) { let extents = self.shared_state_lock().frame_extents.clone(); if let Some(extents) = extents { - x += extents.frame_extents.left as i32; - y += extents.frame_extents.top as i32; + x += cast_dimension_to_hint(extents.frame_extents.left); + y += cast_dimension_to_hint(extents.frame_extents.top); } else { self.update_cached_frame_extents(); return self.set_position_inner(x, y); @@ -1272,7 +1270,8 @@ impl UnownedWindow { pub(crate) fn set_min_inner_size_physical(&self, dimensions: Option<(u32, u32)>) { self.update_normal_hints(|normal_hints| { - normal_hints.min_size = dimensions.map(|(w, h)| (w as i32, h as i32)) + normal_hints.min_size = + dimensions.map(|(w, h)| (cast_dimension_to_hint(w), cast_dimension_to_hint(h))) }) .expect("Failed to call `XSetWMNormalHints`"); } @@ -1287,7 +1286,8 @@ impl UnownedWindow { pub(crate) fn set_max_inner_size_physical(&self, dimensions: Option<(u32, u32)>) { self.update_normal_hints(|normal_hints| { - normal_hints.max_size = dimensions.map(|(w, h)| (w as i32, h as i32)) + normal_hints.max_size = + dimensions.map(|(w, h)| (cast_dimension_to_hint(w), cast_dimension_to_hint(h))) }) .expect("Failed to call `XSetWMNormalHints`"); } @@ -1317,7 +1317,7 @@ impl UnownedWindow { pub fn set_resize_increments(&self, increments: Option) { self.shared_state_lock().resize_increments = increments; let physical_increments = - increments.map(|increments| increments.to_physical::(self.scale_factor()).into()); + increments.map(|increments| cast_size_to_hint(increments, self.scale_factor())); self.update_normal_hints(|hints| hints.size_increment = physical_increments) .expect("Failed to call `XSetWMNormalHints`"); } @@ -1332,8 +1332,7 @@ impl UnownedWindow { ) -> (u32, u32) { let scale_factor = new_scale_factor / old_scale_factor; self.update_normal_hints(|normal_hints| { - let dpi_adjuster = - |size: Size| -> (i32, i32) { size.to_physical::(new_scale_factor).into() }; + let dpi_adjuster = |size: Size| -> (i32, i32) { cast_size_to_hint(size, scale_factor) }; let max_size = shared_state.max_inner_size.map(dpi_adjuster); let min_size = shared_state.min_inner_size.map(dpi_adjuster); let resize_increments = shared_state.resize_increments.map(dpi_adjuster); @@ -1377,12 +1376,8 @@ impl UnownedWindow { .expect_then_ignore_error("Failed to call `XSetWMNormalHints`"); let scale_factor = self.scale_factor(); - let min_inner_size = min_size - .map(|size| size.to_physical::(scale_factor)) - .map(Into::into); - let max_inner_size = max_size - .map(|size| size.to_physical::(scale_factor)) - .map(Into::into); + let min_inner_size = min_size.map(|size| cast_size_to_hint(size, scale_factor)); + let max_inner_size = max_size.map(|size| cast_size_to_hint(size, scale_factor)); self.update_normal_hints(|normal_hints| { normal_hints.min_size = min_inner_size; normal_hints.max_size = max_inner_size; @@ -1745,3 +1740,25 @@ impl UnownedWindow { String::new() } } + +/// Cast a dimension value into a hinted dimension for `WmSizeHints`, clamping if too large. +fn cast_dimension_to_hint(val: u32) -> i32 { + val.try_into().unwrap_or(i32::MAX) +} + +/// Use the above strategy to cast a physical size into a hinted size. +fn cast_physical_size_to_hint(size: PhysicalSize) -> (i32, i32) { + let PhysicalSize { width, height } = size; + ( + cast_dimension_to_hint(width), + cast_dimension_to_hint(height), + ) +} + +/// Use the above strategy to cast a size into a hinted size. +fn cast_size_to_hint(size: Size, scale_factor: f64) -> (i32, i32) { + match size { + Size::Physical(size) => cast_physical_size_to_hint(size), + Size::Logical(size) => size.to_physical::(scale_factor).into(), + } +}