X11: Internal refactoring and cleanups

src/context.rs

@@ -9,7 +9,7 @@ use raw_window_handle::{
 use std::fmt::Debug;
 use std::time::Duration;
 
-/// A handle to the window given to a [`WindowHandler`](crate::WindowHandler), which it can then
+/// A handle to the window given to a [`WindowHandler`](WindowHandler), which it can then
 /// use to perform various operations on the window itself.
 #[derive(Clone)]
 pub struct WindowContext {
@@ -31,7 +31,7 @@ impl WindowContext {
     ///
     /// This request is not immediate. When exactly it will be closed is platform-dependent.
     ///
-    /// However, it is guaranteed to only get closed after all [`WindowHandler`](crate::WindowHandler)
+    /// However, it is guaranteed to only get closed after all [`WindowHandler`](WindowHandler)
     /// methods are completed, and soon enough to be perceived as instantaneous by the user.
     pub fn request_close(&self) {
         self.inner.request_close();
@@ -88,11 +88,11 @@ impl WindowContext {
         PlatformHandle { inner: self.inner.platform_handle() }
     }
 
-    /// Returns the [`GlContext`](crate::gl::GlContext) associated to this window.
+    /// Returns the [`GlContext`](gl::GlContext) associated to this window.
     ///
     /// If the window was not created with a GL context, this will return [`None`].
     #[cfg(feature = "opengl")]
-    pub fn gl_context(&self) -> Option<crate::gl::GlContext> {
+    pub fn gl_context(&self) -> Option<gl::GlContext> {
         self.inner.gl_context()
     }
 
@@ -131,13 +131,13 @@ impl WindowContext {
 }
 
 impl HasWindowHandle for WindowContext {
-    fn window_handle(&self) -> core::result::Result<WindowHandle<'_>, HandleError> {
+    fn window_handle(&self) -> Result<WindowHandle<'_>, HandleError> {
         self.inner.window_handle().ok_or(HandleError::Unavailable)
     }
 }
 
 impl HasDisplayHandle for WindowContext {
-    fn display_handle(&self) -> core::result::Result<DisplayHandle<'_>, HandleError> {
+    fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
         Ok(self.inner.display_handle())
     }
 }
@@ -177,13 +177,13 @@ const _: () = {
 };
 
 impl HasWindowHandle for PlatformHandle {
-    fn window_handle(&self) -> core::result::Result<WindowHandle<'_>, HandleError> {
+    fn window_handle(&self) -> Result<WindowHandle<'_>, HandleError> {
         self.inner.window_handle().ok_or(HandleError::Unavailable)
     }
 }
 
 impl HasDisplayHandle for PlatformHandle {
-    fn display_handle(&self) -> core::result::Result<DisplayHandle<'_>, HandleError> {
+    fn display_handle(&self) -> Result<DisplayHandle<'_>, HandleError> {
         Ok(self.inner.display_handle())
     }
 }

src/gl.rs

@@ -1,3 +1,4 @@
+use crate::platform::gl::PlatformGlContext;
 use std::ffi::{c_void, CStr, CString};
 use std::marker::PhantomData;
 
@@ -54,13 +55,13 @@ pub enum Profile {
 
 #[derive(Clone)]
 pub struct GlContext {
-    inner: crate::platform::gl::GlContext,
+    inner: PlatformGlContext,
     // To make sure this is !Send, !Sync, and !UnwindSafe on all platforms
     phantom: PhantomData<(*mut (), &'static mut ())>,
 }
 
 impl GlContext {
-    pub(crate) fn new(context: crate::platform::gl::GlContext) -> GlContext {
+    pub(crate) fn new(context: PlatformGlContext) -> GlContext {
         GlContext { inner: context, phantom: PhantomData }
     }
 

src/platform/macos/gl.rs

@@ -51,16 +51,16 @@ impl Display for GlError {
 }
 
 #[derive(Clone)]
-pub struct GlContext {
+pub struct PlatformGlContext {
     pub(crate) view: Retained<NSOpenGLView>,
     context: Retained<NSOpenGLContext>,
     gl_bundle: CFRetained<CFBundle>,
 }
 
-impl GlContext {
+impl PlatformGlContext {
     pub(crate) fn create(
         parent_view: &NSView, config: GlConfig, marker: MainThreadMarker,
-    ) -> Result<GlContext> {
+    ) -> Result<PlatformGlContext> {
         let version = if config.version < (3, 2) && config.profile == Profile::Compatibility {
             NSOpenGLProfileVersionLegacy
         } else if config.version == (3, 2) && config.profile == Profile::Core {
@@ -133,7 +133,7 @@ impl GlContext {
         let gl_bundle = CFBundle::bundle_with_identifier(Some(&framework_name))
             .ok_or(GlError::OpenGlBundleNotFound)?;
 
-        Ok(GlContext { view, context, gl_bundle })
+        Ok(PlatformGlContext { view, context, gl_bundle })
     }
 
     pub unsafe fn make_current(&self) -> Result<()> {

src/platform/macos/view.rs

@@ -84,7 +84,7 @@ pub(crate) struct BaseviewView {
     timers: TimerManager,
 
     #[cfg(feature = "opengl")]
-    pub(crate) gl_context: OnceCell<super::gl::GlContext>,
+    pub(crate) gl_context: OnceCell<super::gl::PlatformGlContext>,
 }
 
 impl BaseviewView {
@@ -133,7 +133,8 @@ impl BaseviewView {
 
             #[cfg(feature = "opengl")]
             if let Some(gl_config) = init.settings.gl_config {
-                let gl_context = super::gl::GlContext::create(view.view, gl_config, view.mtm)?;
+                let gl_context =
+                    super::gl::PlatformGlContext::create(view.view, gl_config, view.mtm)?;
                 let Ok(()) = view.gl_context.set(gl_context) else { unreachable!() };
             }
 

src/platform/win/gl.rs

@@ -11,7 +11,7 @@ use crate::wrappers::win32::window::{
 };
 use crate::wrappers::win32::RawLibrary;
 
-pub type GlContext = Rc<GlContextInner>;
+pub type PlatformGlContext = Rc<GlContextInner>;
 
 pub struct GlContextInner {
     hdc: OwnDeviceContext,

src/platform/win/window_state.rs

@@ -35,7 +35,7 @@ pub(crate) struct WindowState {
     pub redraw_requested: Cell<bool>,
 
     #[cfg(feature = "opengl")]
-    pub gl_context: std::cell::OnceCell<super::gl::GlContext>,
+    pub gl_context: std::cell::OnceCell<super::gl::PlatformGlContext>,
 }
 
 impl WindowState {

src/platform/x11/cursor.rs

@@ -1,13 +1,66 @@
-use x11rb::connection::Connection;
-use x11rb::cursor::Handle as CursorHandle;
-use x11rb::protocol::xproto::{ConnectionExt as _, Cursor};
-use x11rb::xcb_ffi::XCBConnection;
-
-use crate::platform::*;
+use super::prelude::*;
 use crate::wrappers::xlib::XlibXcbConnection;
 use crate::MouseCursor;
+use std::cell::RefCell;
+use std::collections::hash_map::Entry;
+use std::collections::HashMap;
+use x11rb::protocol::xproto::ChangeWindowAttributesAux;
+
+pub struct CursorStateShared {
+    mouse_cursor: Cell<MouseCursor>,
+    cursor_cache: RefCell<HashMap<MouseCursor, u32>>,
+}
+
+impl CursorStateShared {
+    pub(crate) fn set_mouse_cursor(
+        &self, mouse_cursor: MouseCursor, window: &XcbWindow,
+    ) -> PlatformResult<()> {
+        if self.mouse_cursor.get() == mouse_cursor {
+            return Ok(());
+        }
+
+        let xid = self.get_cursor(mouse_cursor, window.connection())?;
+
+        if xid != 0 {
+            window
+                .connection()
+                .conn
+                .change_window_attributes(
+                    window.id().get(),
+                    &ChangeWindowAttributesAux::new().cursor(xid),
+                )?
+                .check()?;
+        }
+
+        self.mouse_cursor.set(mouse_cursor);
+
+        Ok(())
+    }
+
+    #[inline]
+    fn get_cursor(&self, cursor: MouseCursor, conn: &X11Connection) -> PlatformResult<Cursor> {
+        // PANIC: this function is the only point where we access the cache, and we never call
+        // external functions that may make a reentrant call to this function
+        let mut cursor_cache = self.cursor_cache.borrow_mut();
+
+        match cursor_cache.entry(cursor) {
+            Entry::Occupied(entry) => Ok(*entry.get()),
+            Entry::Vacant(entry) => {
+                let cursor = get_xcursor(conn, cursor)?;
+                entry.insert(cursor);
+                Ok(cursor)
+            }
+        }
+    }
+}
+
+impl CursorStateShared {
+    pub fn new() -> Self {
+        Self { mouse_cursor: MouseCursor::Default.into(), cursor_cache: HashMap::new().into() }
+    }
+}
 
-fn create_empty_cursor(conn: &XlibXcbConnection) -> Result<Cursor> {
+fn create_empty_cursor(conn: &XlibXcbConnection) -> PlatformResult<Cursor> {
     let cursor_id = conn.generate_id()?;
     let pixmap_id = conn.generate_id()?;
     let root_window = conn.default_screen().root;
@@ -18,22 +71,22 @@ fn create_empty_cursor(conn: &XlibXcbConnection) -> Result<Cursor> {
     Ok(cursor_id)
 }
 
-fn load_cursor(
-    conn: &XCBConnection, cursor_handle: &CursorHandle, name: &str,
-) -> Result<Option<Cursor>> {
-    let cursor = cursor_handle.load_cursor(conn, name)?;
+#[inline(never)]
+fn load_cursor(conn: &X11Connection, name: &str) -> PlatformResult<Option<Cursor>> {
+    let cursor = conn.resources.cursor_handle.load_cursor(&conn.conn as &XCBConnection, name)?;
     if cursor != x11rb::NONE {
         Ok(Some(cursor))
     } else {
         Ok(None)
     }
 }
 
+#[inline(never)]
 fn load_first_existing_cursor(
-    conn: &XCBConnection, cursor_handle: &CursorHandle, names: &[&str],
-) -> Result<Option<Cursor>> {
+    conn: &X11Connection, names: &[&str],
+) -> PlatformResult<Option<Cursor>> {
     for name in names {
-        let cursor = load_cursor(conn, cursor_handle, name)?;
+        let cursor = load_cursor(conn, name)?;
         if cursor.is_some() {
             return Ok(cursor);
         }
@@ -42,11 +95,9 @@ fn load_first_existing_cursor(
     Ok(None)
 }
 
-pub(crate) fn get_xcursor(
-    conn: &XlibXcbConnection, cursor_handle: &CursorHandle, cursor: MouseCursor,
-) -> Result<Cursor> {
-    let load = |name: &str| load_cursor(conn, cursor_handle, name);
-    let loadn = |names: &[&str]| load_first_existing_cursor(conn, cursor_handle, names);
+pub(crate) fn get_xcursor(conn: &X11Connection, cursor: MouseCursor) -> PlatformResult<Cursor> {
+    let load = |name: &str| load_cursor(conn, name);
+    let loadn = |names: &[&str]| load_first_existing_cursor(conn, names);
 
     let cursor = match cursor {
         MouseCursor::Default => None, // catch this in the fallback case below
@@ -55,7 +106,7 @@ pub(crate) fn get_xcursor(
         MouseCursor::HandGrabbing => loadn(&["closedhand", "grabbing"])?,
         MouseCursor::Help => load("question_arrow")?,
 
-        MouseCursor::Hidden => Some(create_empty_cursor(conn)?),
+        MouseCursor::Hidden => Some(create_empty_cursor(&conn.conn)?),
 
         MouseCursor::Text => loadn(&["text", "xterm"])?,
         MouseCursor::VerticalText => load("vertical-text")?,

src/platform/x11/drag_n_drop.rs

@@ -1,8 +1,8 @@
-use super::xcb_connection::{Atoms, GetPropertyError};
+use super::x11_connection::{Atoms, GetPropertyError};
 use super::*;
 use crate::dpi::PhysicalPosition;
-use crate::handler::WindowHandler;
 use crate::platform::x11::error::ReplyExt;
+use crate::platform::x11::handler::Handler;
 use crate::warn;
 use crate::{DropData, Event, MouseEvent};
 use core::result::Result;
@@ -95,7 +95,7 @@ pub(crate) enum DragNDropState {
 // Other errors (protocol errors, transfer errors) should be dealt with as gracefully as possible.
 impl DragNDropState {
     pub fn handle_enter_event(
-        &mut self, window: &WindowInner, handler: &dyn WindowHandler, event: &ClientMessageEvent,
+        &mut self, window: &WindowShared, handler: &Handler, event: &ClientMessageEvent,
     ) -> Result<(), ConnectionError> {
         let data = event.data.as_data32();
 
@@ -154,7 +154,7 @@ impl DragNDropState {
     }
 
     pub fn handle_position_event(
-        &mut self, window: &WindowInner, handler: &dyn WindowHandler, event: &ClientMessageEvent,
+        &mut self, window: &WindowShared, handler: &Handler, event: &ClientMessageEvent,
     ) -> Result<(), ConnectionError> {
         let event_data = event.data.as_data32();
 
@@ -260,7 +260,7 @@ impl DragNDropState {
         }
     }
 
-    pub fn handle_leave_event(&mut self, handler: &dyn WindowHandler, event: &ClientMessageEvent) {
+    pub fn handle_leave_event(&mut self, handler: &Handler, event: &ClientMessageEvent) {
         let data = event.data.as_data32();
         let event_source_window = data[0] as xproto::Window;
 
@@ -291,7 +291,7 @@ impl DragNDropState {
     }
 
     pub fn handle_drop_event(
-        &mut self, window: &WindowInner, handler: &dyn WindowHandler, event: &ClientMessageEvent,
+        &mut self, window: &WindowShared, handler: &Handler, event: &ClientMessageEvent,
     ) -> Result<(), ConnectionError> {
         let data = event.data.as_data32();
 
@@ -395,7 +395,7 @@ impl DragNDropState {
     }
 
     pub fn handle_selection_notify_event(
-        &mut self, window: &WindowInner, handler: &dyn WindowHandler, event: &SelectionNotifyEvent,
+        &mut self, window: &WindowShared, handler: &Handler, event: &SelectionNotifyEvent,
     ) -> Result<(), ConnectionError> {
         // Ignore the event if we weren't actually waiting for a selection notify event
         let WaitingForData {
@@ -488,7 +488,7 @@ impl DragNDropState {
 }
 
 fn send_status_rejected(
-    source_window: xproto::Window, window: &WindowInner,
+    source_window: xproto::Window, window: &WindowShared,
 ) -> Result<(), ConnectionError> {
     let conn = &window.connection;
 
@@ -509,7 +509,7 @@ fn send_status_rejected(
 }
 
 fn send_status_event(
-    source_window: xproto::Window, window: &WindowInner, action: Option<DndAction>,
+    source_window: xproto::Window, window: &WindowShared, action: Option<DndAction>,
 ) -> Result<(), ConnectionError> {
     let conn = &window.connection;
 
@@ -532,7 +532,7 @@ fn send_status_event(
 }
 
 pub fn send_finished_rejected(
-    source_window: xproto::Window, window: &WindowInner,
+    source_window: xproto::Window, window: &WindowShared,
 ) -> Result<(), ConnectionError> {
     let conn = &window.connection;
 
@@ -553,7 +553,7 @@ pub fn send_finished_rejected(
 }
 
 fn send_finished_event(
-    source_window: xproto::Window, window: &WindowInner, action: Option<DndAction>,
+    source_window: xproto::Window, window: &WindowShared, action: Option<DndAction>,
 ) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
     let conn = &window.connection;
     let action =
@@ -572,7 +572,7 @@ fn send_finished_event(
 }
 
 fn request_convert_selection(
-    window: &WindowInner, timestamp: Option<Timestamp>,
+    window: &WindowShared, timestamp: Option<Timestamp>,
 ) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
     window.connection.conn.convert_selection(
         window.xcb_window.id().get(),
@@ -588,7 +588,7 @@ fn decode_xy(data: u32) -> (u16, u16) {
 }
 
 fn translate_root_coordinates(
-    window: &WindowInner, x: u16, y: u16,
+    window: &WindowShared, x: u16, y: u16,
 ) -> Result<Option<PhysicalPosition<i16>>, ConnectionError> {
     let root_id = window.connection.default_screen().root;
     let x = x.try_into().unwrap_or(i16::MAX);
@@ -609,7 +609,7 @@ fn translate_root_coordinates(
     Ok(Some(PhysicalPosition::new(reply.dst_x, reply.dst_y)))
 }
 
-fn fetch_dnd_data(window: &WindowInner) -> Result<Option<DropData>, ConnectionError> {
+fn fetch_dnd_data(window: &WindowShared) -> Result<Option<DropData>, ConnectionError> {
     let conn = &window.connection;
 
     let data: Vec<u8> = match conn.get_property(