From 7e0a660d2e3f220ac2c0ce651eb5a7070621320e Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sat, 16 May 2026 08:14:46 +0200 Subject: [PATCH 1/9] wip --- src/x11/mod.rs | 2 + src/x11/window.rs | 2 +- src/x11/xcb_connection.rs | 43 +++---------- src/x11/xlib_xcb.rs | 131 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 35 deletions(-) create mode 100644 src/x11/xlib_xcb.rs diff --git a/src/x11/mod.rs b/src/x11/mod.rs index f1c14892..d3ed7dd0 100644 --- a/src/x11/mod.rs +++ b/src/x11/mod.rs @@ -8,3 +8,5 @@ mod cursor; mod event_loop; mod keyboard; mod visual_info; + +mod xlib_xcb; diff --git a/src/x11/window.rs b/src/x11/window.rs index 7ab44cae..aa2adcac 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -364,7 +364,7 @@ unsafe impl<'a> HasRawWindowHandle for Window<'a> { unsafe impl<'a> HasRawDisplayHandle for Window<'a> { fn raw_display_handle(&self) -> RawDisplayHandle { - let display = self.inner.xcb_connection.dpy; + let display = self.inner.xcb_connection.conn.xlib_display(); let mut handle = XlibDisplayHandle::empty(); handle.display = display as *mut c_void; diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index c8725917..35b4cd07 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -2,19 +2,14 @@ use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::error::Error; use std::ffi::c_int; -use std::sync::Arc; -use x11_dl::xlib::Xlib; -use x11_dl::xlib_xcb::Xlib_xcb; -use x11_dl::{xlib::Display, xlib_xcb}; use x11rb::connection::Connection; use x11rb::cursor::Handle as CursorHandle; use x11rb::protocol::xproto::{Cursor, Screen}; use x11rb::resource_manager; -use x11rb::xcb_ffi::XCBConnection; - -use crate::MouseCursor; use super::cursor; +use crate::x11::xlib_xcb::XlibXcbConnection; +use crate::MouseCursor; x11rb::atom_manager! { pub Atoms: AtomsCookie { @@ -27,9 +22,7 @@ x11rb::atom_manager! { /// /// Keeps track of the xcb connection itself and the xlib display ID that was used to connect. pub struct XcbConnection { - pub(crate) xlib: Arc, - pub(crate) dpy: *mut Display, - pub(crate) conn: XCBConnection, + pub(crate) conn: XlibXcbConnection, pub(crate) screen: c_int, pub(crate) atoms: Atoms, pub(crate) resources: resource_manager::Database, @@ -39,25 +32,15 @@ pub struct XcbConnection { impl XcbConnection { pub fn new() -> Result> { - let xlib = Xlib::open()?; - let dpy = unsafe { (xlib.XOpenDisplay)(std::ptr::null()) }; - assert!(!dpy.is_null()); - let xlib_xcb = Xlib_xcb::open()?; - let xcb_connection = unsafe { (xlib_xcb.XGetXCBConnection)(dpy) }; - assert!(!xcb_connection.is_null()); - let screen = unsafe { (xlib.XDefaultScreen)(dpy) }; - let conn = unsafe { XCBConnection::from_raw_xcb_connection(xcb_connection, false)? }; - unsafe { - (xlib_xcb.XSetEventQueueOwner)(dpy, xlib_xcb::XEventQueueOwner::XCBOwnsEventQueue) - }; + let conn = XlibXcbConnection::open()?; + let screen = conn.default_screen(); + let xcb_conn = conn.xcb_connection(); - let atoms = Atoms::new(&conn)?.reply()?; - let resources = resource_manager::new_from_default(&conn)?; - let cursor_handle = CursorHandle::new(&conn, screen as usize, &resources)?.reply()?; + let atoms = Atoms::new(xcb_conn)?.reply()?; + let resources = resource_manager::new_from_default(xcb_conn)?; + let cursor_handle = CursorHandle::new(xcb_conn, screen as usize, &resources)?.reply()?; Ok(Self { - xlib: Arc::new(xlib), - dpy, conn, screen, atoms, @@ -132,11 +115,3 @@ impl XcbConnection { &self.conn.setup().roots[self.screen as usize] } } - -impl Drop for XcbConnection { - fn drop(&mut self) { - unsafe { - (self.xlib.XCloseDisplay)(self.dpy); - } - } -} diff --git a/src/x11/xlib_xcb.rs b/src/x11/xlib_xcb.rs new file mode 100644 index 00000000..d3ea8e48 --- /dev/null +++ b/src/x11/xlib_xcb.rs @@ -0,0 +1,131 @@ +use std::error::Error; +use std::fmt::Formatter; +use std::ops::Deref; +use std::os::raw::c_int; +use std::ptr::NonNull; +use std::sync::Arc; +use x11_dl::xlib::{Display, Xlib}; +use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; +use x11rb::xcb_ffi::XCBConnection; + +/// A Xlib/XCB connection object. +/// +/// This exposes both a raw Xlib display pointer, and a x11rb XCBConnection object. +/// +/// This allows us to interface with the same connection using Xlib (needed for GLX, and for FFI), +/// as well as XCB (needed to preserve our sanity). +pub struct XlibXcbConnection { + // SAFETY: Drop order matters here! We *MUST* Drop the XCBConnection first, as it essentially + // borrows the Xlib/XCB connection + xcb_connection: XCBConnection, + xlib_connection: OwnedDisplayConnection, +} + +impl XlibXcbConnection { + pub fn open() -> Result> { + let xlib_xcb = Xlib_xcb::open()?; + // Open the connection to the X11 server as a Xlib/XCB connection object + let xlib_connection = OwnedDisplayConnection::open()?; + // Set the XCB end of the Xlib/XCB connection object as the queue owner. + // From now on, we'll use XCB (i.e. X11rb) to interface with the event queue + xlib_connection.set_xcb_queue_owner(&xlib_xcb); + + // Extract the XCB connection object pointer from the Xlib/XCB connection object + // SAFETY: This is always safe to call as long as the OwnedDisplayConnection is alive + let xcb_connection = + unsafe { (xlib_xcb.XGetXCBConnection)(xlib_connection.display.as_ptr()) }; + + // The XGetXCBConnection function is not documented to ever be able to return NULL. + // Still, this is cheap to check, just in case. + assert!(!xcb_connection.is_null()); + + // Wrap the XCB connection object in a x11rb connection object + // SAFETY: The xcb_connection pointer should be valid. We also enforce the drop order in this + let xcb_connection = + unsafe { XCBConnection::from_raw_xcb_connection(xcb_connection, false)? }; + + Ok(Self { xcb_connection, xlib_connection }) + } + + pub fn default_screen(&self) -> c_int { + self.xlib_connection.default_screen() + } + + pub fn xlib(&self) -> &Xlib { + &self.xlib_connection.xlib + } + + pub fn xlib_display(&self) -> *mut Display { + self.xlib_connection.display.as_ptr() + } + + pub fn xcb_connection(&self) -> &XCBConnection { + &self.xcb_connection + } +} + +// For convenience +impl Deref for XlibXcbConnection { + type Target = XCBConnection; + + fn deref(&self) -> &Self::Target { + &self.xcb_connection + } +} + +/// An owned Xlib Display connection. +/// +/// This type guarantees the inner display connection object to be alive and valid, as long as this +/// is alive. +/// +/// It will also always close the display connection on drop. +struct OwnedDisplayConnection { + display: NonNull, + xlib: Arc, +} + +impl OwnedDisplayConnection { + pub fn open() -> Result> { + let xlib = Arc::new(Xlib::open()?); + + // SAFETY: It's always safe to call XOpenDisplay with a NULL display_name + let ptr = unsafe { (xlib.XOpenDisplay)(core::ptr::null()) }; + + let Some(display) = NonNull::new(ptr) else { return Err(DisplayOpenFailedError.into()) }; + + Ok(Self { display, xlib }) + } + + fn set_xcb_queue_owner(&self, xlib_xcb: &Xlib_xcb) { + // SAFETY: This type ensures the display pointer is always valid. + unsafe { + (xlib_xcb.XSetEventQueueOwner)( + self.display.as_ptr(), + XEventQueueOwner::XCBOwnsEventQueue, + ) + } + } + + /// Safe wrapper for XDefaultScreen + fn default_screen(&self) -> c_int { + // SAFETY: This type ensures the display pointer is always valid. + unsafe { (self.xlib.XDefaultScreen)(self.display.as_ptr()) } + } +} + +impl Drop for OwnedDisplayConnection { + fn drop(&mut self) { + // SAFETY: This type guarantees the display pointer is valid. + // This being `Drop` also prevents any double-free. + unsafe { (self.xlib.XCloseDisplay)(self.display.as_ptr()) }; + } +} + +#[derive(Debug)] +struct DisplayOpenFailedError; +impl std::fmt::Display for DisplayOpenFailedError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to open X11 display connection: XOpenDisplay() failed") + } +} +impl Error for DisplayOpenFailedError {} From d3cdf2b1c3eaed3c0a7ff358c200bf2cd9d1a51d Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sat, 16 May 2026 08:29:36 +0200 Subject: [PATCH 2/9] wip --- src/x11/xlib_xcb.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/x11/xlib_xcb.rs b/src/x11/xlib_xcb.rs index d3ea8e48..ec562415 100644 --- a/src/x11/xlib_xcb.rs +++ b/src/x11/xlib_xcb.rs @@ -14,6 +14,8 @@ use x11rb::xcb_ffi::XCBConnection; /// /// This allows us to interface with the same connection using Xlib (needed for GLX, and for FFI), /// as well as XCB (needed to preserve our sanity). +/// +/// (Note: The term Xlib/XCB means "Xlib over XCB", not "Xlib or XCB"). pub struct XlibXcbConnection { // SAFETY: Drop order matters here! We *MUST* Drop the XCBConnection first, as it essentially // borrows the Xlib/XCB connection From 8961b5dcd9e12216376c84f2ee9435010c75d173 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 00:37:53 +0200 Subject: [PATCH 3/9] Move to separate wrappers module --- src/gl/x11.rs | 60 +++++--- src/lib.rs | 2 + src/wrappers.rs | 17 +++ src/{gl/x11 => wrappers}/glx.rs | 51 +++---- src/wrappers/xlib.rs | 7 + .../xlib/error_handler.rs} | 38 ++--- src/wrappers/xlib/xlib_connection.rs | 119 ++++++++++++++++ src/wrappers/xlib/xlib_xcb.rs | 72 ++++++++++ src/x11/mod.rs | 2 - src/x11/xcb_connection.rs | 2 +- src/x11/xlib_xcb.rs | 133 ------------------ 11 files changed, 294 insertions(+), 209 deletions(-) create mode 100644 src/wrappers.rs rename src/{gl/x11 => wrappers}/glx.rs (80%) create mode 100644 src/wrappers/xlib.rs rename src/{gl/x11/errors.rs => wrappers/xlib/error_handler.rs} (79%) create mode 100644 src/wrappers/xlib/xlib_connection.rs create mode 100644 src/wrappers/xlib/xlib_xcb.rs delete mode 100644 src/x11/xlib_xcb.rs diff --git a/src/gl/x11.rs b/src/gl/x11.rs index c9b0a22b..b85474a6 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -6,10 +6,9 @@ use std::rc::Rc; use x11_dl::error::OpenError; use x11_dl::glx::GLXContext; -mod errors; -mod glx; -use crate::gl::x11::glx::GlxFbConfig; -use glx::Glx; +use crate::wrappers::glx::Glx; +use crate::wrappers::glx::GlxFbConfig; +use crate::wrappers::xlib::{XErrorHandler, XLibError}; #[derive(Debug)] pub enum CreationFailedError { @@ -18,12 +17,12 @@ pub enum CreationFailedError { GetProcAddressFailed, MakeCurrentFailed, ContextCreationFailed, - X11Error(errors::XLibError), + X11Error(XLibError), OpenError(OpenError), } -impl From for GlError { - fn from(e: errors::XLibError) -> Self { +impl From for GlError { + fn from(e: XLibError) -> Self { GlError::CreationFailed(CreationFailedError::X11Error(e)) } } @@ -69,7 +68,9 @@ impl GlContext { ) -> Result { let glx = Glx::open()?; - errors::XErrorHandler::handle(&connection, |error_handler| { + let xlib_connection = connection.conn.xlib_connection(); + + XErrorHandler::handle(xlib_connection, |error_handler| { let Some(create_context) = glx.get_glx_create_context_attribs_arb() else { return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed)); }; @@ -79,7 +80,7 @@ impl GlContext { }; let context = create_context.call( - &connection, + xlib_connection, &config.gl_config, config.fb_config, error_handler, @@ -89,12 +90,12 @@ impl GlContext { let context = GlContext { glx, window, connection: Rc::clone(&connection), context }; context.glx.with_current_context( - &connection, + xlib_connection, window, context.context, error_handler, || { - swap_interval(connection.dpy, window, config.gl_config.vsync as i32); + swap_interval(xlib_connection.dpy(), window, config.gl_config.vsync as i32); error_handler.check() }, )??; @@ -107,16 +108,24 @@ impl GlContext { /// This needs to be passed to [Self::create] along with a handle to a window that was created /// using the visual also returned from this function. pub fn get_fb_config_and_visual( - conn: &XcbConnection, config: GlConfig, + connection: &XcbConnection, config: GlConfig, ) -> Result<(FbConfig, WindowConfig), GlError> { let glx = Glx::open()?; - errors::XErrorHandler::handle(conn, |error_handler| { - let fb_config = glx.choose_best_fb_config(conn, &config, error_handler)?; + let xlib_connection = connection.conn.xlib_connection(); + + XErrorHandler::handle(xlib_connection, |error_handler| { + let fb_config = glx.choose_best_fb_config( + xlib_connection, + &config, + connection.screen, + error_handler, + )?; // Now that we have a matching framebuffer config, we need to know which visual matches // this config so the window is compatible with the OpenGL context we're about to create - let visual = glx.get_visual_from_fb_config(conn, fb_config, error_handler)?; + let visual = + glx.get_visual_from_fb_config(xlib_connection, fb_config, error_handler)?; Ok(( FbConfig { fb_config, gl_config: config }, @@ -126,16 +135,21 @@ impl GlContext { } pub unsafe fn make_current(&self) { - errors::XErrorHandler::handle(&self.connection, |error_handler| { + XErrorHandler::handle(self.connection.conn.xlib_connection(), |error_handler| { self.glx - .make_current(&self.connection, self.window, self.context, error_handler) + .make_current( + self.connection.conn.xlib_connection(), + self.window, + self.context, + error_handler, + ) .unwrap(); }) } pub unsafe fn make_not_current(&self) { - errors::XErrorHandler::handle(&self.connection, |error_handler| { - self.glx.clear_current(&self.connection, error_handler).unwrap(); + XErrorHandler::handle(self.connection.conn.xlib_connection(), |error_handler| { + self.glx.clear_current(self.connection.conn.xlib_connection(), error_handler).unwrap(); }) } @@ -149,14 +163,16 @@ impl GlContext { } pub fn swap_buffers(&self) { - errors::XErrorHandler::handle(&self.connection, |error_handler| { - self.glx.swap_buffers(&self.connection, self.window, error_handler).unwrap() + XErrorHandler::handle(self.connection.conn.xlib_connection(), |error_handler| { + self.glx + .swap_buffers(self.connection.conn.xlib_connection(), self.window, error_handler) + .unwrap() }) } } impl Drop for GlContext { fn drop(&mut self) { - unsafe { self.glx.destroy_context(&self.connection, self.context) } + unsafe { self.glx.destroy_context(self.connection.conn.xlib_connection(), self.context) } } } diff --git a/src/lib.rs b/src/lib.rs index 7ec7971d..29a7129a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -22,3 +22,5 @@ pub use mouse_cursor::MouseCursor; pub use window::*; pub use window_info::*; pub use window_open_options::*; + +pub(crate) mod wrappers; diff --git a/src/wrappers.rs b/src/wrappers.rs new file mode 100644 index 00000000..739635d5 --- /dev/null +++ b/src/wrappers.rs @@ -0,0 +1,17 @@ +#![allow(unsafe_code)] + +//! A set of safe wrappers around C or platform APIs. +//! +//! This module is designed to contain all unsafe code necessary for baseview to work, where safe +//! APIs do not yet exist in the Rust ecosystem (e.g. Win32, Xlib, GLX), or are not practical to use. +//! +//! These wrappers are not designed to fully encapsulate their respective APIs, only the bits used +//! by baseview. +//! +//! However, all of these APIs should always be sound (i.e. no UB can be triggered by safe code). +//! Otherwise, this should be considered a bug and reported accordingly. + +pub mod glx; +/// Wrappers and utilities around Xlib. (provided by x11_dl) +#[cfg(target_os = "linux")] +pub mod xlib; diff --git a/src/gl/x11/glx.rs b/src/wrappers/glx.rs similarity index 80% rename from src/gl/x11/glx.rs rename to src/wrappers/glx.rs index d794469f..0bd11095 100644 --- a/src/gl/x11/glx.rs +++ b/src/wrappers/glx.rs @@ -1,7 +1,7 @@ +use super::xlib::*; use crate::gl::platform::CreationFailedError; -use crate::gl::x11::errors::XErrorHandler; use crate::gl::{GlConfig, GlError, Profile}; -use crate::x11::XcbConnection; + use std::ffi::{c_ulong, c_void, CStr}; use std::os::raw::c_int; use std::ptr::NonNull; @@ -58,17 +58,18 @@ impl Glx { } pub fn choose_best_fb_config( - &self, connection: &XcbConnection, config: &GlConfig, error_handler: &XErrorHandler, + &self, connection: &XlibConnection, config: &GlConfig, screen: c_int, + error_handler: &XErrorHandler, ) -> Result { let fb_attribs = Self::get_fb_attribs(config); let mut nelements = 0; - // SAFETY: XcbConnection guarantees the inner dpy is valid. + // SAFETY: XlibConnection guarantees the inner dpy is valid. // The fb_attribs and nelements pointers come from references and are therefore valid. let result = unsafe { (self.inner.glXChooseFBConfig)( - connection.dpy, - connection.screen, + connection.dpy(), + screen, fb_attribs.as_ptr(), &mut nelements, ) @@ -85,16 +86,17 @@ impl Glx { // SAFETY: for the same reasons above, glXChooseFBConfig returned a valid array // that we must free ourselves. - unsafe { (connection.xlib.XFree)(result.cast()) }; + unsafe { (connection.xlib().XFree)(result.cast()) }; Ok(GlxFbConfig(first_result)) } pub fn get_visual_from_fb_config( - &self, connection: &XcbConnection, fb_config: GlxFbConfig, error_handler: &XErrorHandler, + &self, connection: &XlibConnection, fb_config: GlxFbConfig, error_handler: &XErrorHandler, ) -> Result { - // SAFETY: XcbConnection guarantees the inner dpy is valid. - let result = unsafe { (self.inner.glXGetVisualFromFBConfig)(connection.dpy, fb_config.0) }; + // SAFETY: XlibConnection guarantees the inner dpy is valid. + let result = + unsafe { (self.inner.glXGetVisualFromFBConfig)(connection.dpy(), fb_config.0) }; error_handler.check()?; if result.is_null() { @@ -106,16 +108,16 @@ impl Glx { let visual = unsafe { result.read() }; // SAFETY: for the same reasons above, glXGetVisualFromFBConfig returned a valid array // that we must free ourselves. - unsafe { (connection.xlib.XFree)(result.cast()) }; + unsafe { (connection.xlib().XFree)(result.cast()) }; Ok(visual) } pub fn swap_buffers( - &self, connection: &XcbConnection, window_id: c_ulong, error_handler: &XErrorHandler, + &self, connection: &XlibConnection, window_id: c_ulong, error_handler: &XErrorHandler, ) -> Result<(), GlError> { - // SAFETY: XcbConnection guarantees the inner dpy is valid. - unsafe { (self.inner.glXSwapBuffers)(connection.dpy, window_id) }; + // SAFETY: XlibConnection guarantees the inner dpy is valid. + unsafe { (self.inner.glXSwapBuffers)(connection.dpy(), window_id) }; Ok(error_handler.check()?) } @@ -142,16 +144,17 @@ impl Glx { })) } - pub unsafe fn destroy_context(&self, connection: &XcbConnection, context: GLXContext) { - // SAFETY: - unsafe { (self.inner.glXDestroyContext)(connection.dpy, context) }; + pub unsafe fn destroy_context(&self, connection: &XlibConnection, context: GLXContext) { + // SAFETY: XlibConnection guarantees the inner dpy is valid. + unsafe { (self.inner.glXDestroyContext)(connection.dpy(), context) }; } pub unsafe fn make_current( - &self, connection: &XcbConnection, window_id: c_ulong, context: GLXContext, + &self, connection: &XlibConnection, window_id: c_ulong, context: GLXContext, error_handler: &XErrorHandler, ) -> Result<(), GlError> { - let res = unsafe { (self.inner.glXMakeCurrent)(connection.dpy, window_id, context) }; + // SAFETY: XlibConnection guarantees the inner dpy is valid. + let res = unsafe { (self.inner.glXMakeCurrent)(connection.dpy(), window_id, context) }; error_handler.check()?; if res == 0 { @@ -162,13 +165,13 @@ impl Glx { } pub unsafe fn clear_current( - &self, connection: &XcbConnection, error_handler: &XErrorHandler, + &self, connection: &XlibConnection, error_handler: &XErrorHandler, ) -> Result<(), GlError> { self.make_current(connection, 0, core::ptr::null_mut(), error_handler) } pub unsafe fn with_current_context( - &self, connection: &XcbConnection, window_id: c_ulong, context: GLXContext, + &self, connection: &XlibConnection, window_id: c_ulong, context: GLXContext, error_handler: &XErrorHandler, closure: impl FnOnce() -> T, ) -> Result { self.make_current(connection, window_id, context, error_handler)?; @@ -186,7 +189,7 @@ impl Glx { pub struct ContextClearOnDrop<'a> { glx: &'a Glx, - connection: &'a XcbConnection, + connection: &'a XlibConnection, error_handler: &'a XErrorHandler<'a>, } @@ -217,13 +220,13 @@ impl GlxCreateContextAttribsARB { } pub fn call( - &self, connection: &XcbConnection, gl_config: &GlConfig, glx_fb_config: GlxFbConfig, + &self, connection: &XlibConnection, gl_config: &GlConfig, glx_fb_config: GlxFbConfig, error_handler: &XErrorHandler, ) -> Result { let ctx_attribs = Self::get_ctx_attribs(gl_config); let context = unsafe { - self.0(connection.dpy, glx_fb_config.0, std::ptr::null_mut(), 1, ctx_attribs.as_ptr()) + self.0(connection.dpy(), glx_fb_config.0, std::ptr::null_mut(), 1, ctx_attribs.as_ptr()) }; error_handler.check()?; diff --git a/src/wrappers/xlib.rs b/src/wrappers/xlib.rs new file mode 100644 index 00000000..32559030 --- /dev/null +++ b/src/wrappers/xlib.rs @@ -0,0 +1,7 @@ +mod error_handler; +mod xlib_connection; +mod xlib_xcb; + +pub use error_handler::{XErrorHandler, XLibError}; +pub use xlib_connection::XlibConnection; +pub use xlib_xcb::XlibXcbConnection; diff --git a/src/gl/x11/errors.rs b/src/wrappers/xlib/error_handler.rs similarity index 79% rename from src/gl/x11/errors.rs rename to src/wrappers/xlib/error_handler.rs index f1808861..3a07bd5b 100644 --- a/src/gl/x11/errors.rs +++ b/src/wrappers/xlib/error_handler.rs @@ -1,8 +1,7 @@ -use std::ffi::CStr; use std::fmt::{Debug, Display, Formatter}; use x11_dl::xlib; -use crate::x11::XcbConnection; +use super::xlib_connection::XlibConnection; use std::cell::Cell; use std::error::Error; use std::os::raw::{c_int, c_uchar, c_ulong}; @@ -17,7 +16,7 @@ thread_local! { /// A helper struct for safe X11 error handling pub struct XErrorHandler<'a> { - conn: &'a XcbConnection, + conn: &'a XlibConnection, error: &'a Cell>, } @@ -25,7 +24,7 @@ impl<'a> XErrorHandler<'a> { /// Syncs and checks if any previous X11 calls from the given display returned an error pub fn check(&self) -> Result<(), XLibError> { // Flush all possible previous errors - unsafe { (self.conn.xlib.XSync)(self.conn.dpy, 0) }; + self.conn.sync(); let error = self.error.take(); @@ -37,7 +36,7 @@ impl<'a> XErrorHandler<'a> { /// Sets up a temporary X11 error handler for the duration of the given closure, and allows /// that closure to check on the latest X11 error at any time. - pub fn handle T>(conn: &XcbConnection, handler: F) -> T { + pub fn handle T>(conn: &XlibConnection, handler: F) -> T { /// # Safety /// The given display and error pointers *must* be valid for the duration of this function. unsafe extern "C" fn error_handler( @@ -60,19 +59,19 @@ impl<'a> XErrorHandler<'a> { } // Flush all possible previous errors - unsafe { (conn.xlib.XSync)(conn.dpy, 0) }; + conn.sync(); CURRENT_X11_ERROR.with(|error| { // Make sure to clear any errors from the last call to this function error.set(None); - let old_handler = unsafe { (conn.xlib.XSetErrorHandler)(Some(error_handler)) }; + let old_handler = conn.set_error_handler(Some(error_handler)); let panic_result = std::panic::catch_unwind(AssertUnwindSafe(|| { let mut h = XErrorHandler { conn, error }; handler(&mut h) })); // Whatever happened, restore old error handler - unsafe { (conn.xlib.XSetErrorHandler)(old_handler) }; + conn.set_error_handler(old_handler); match panic_result { Ok(v) => v, @@ -112,26 +111,11 @@ pub struct XLibError { } impl XLibError { - fn from_inner(inner: CaughtXLibError, conn: &XcbConnection) -> Self { - Self { display_name: Self::get_display_name(&inner, conn), inner } - } - - fn get_display_name(error: &CaughtXLibError, conn: &XcbConnection) -> Box { + fn from_inner(inner: CaughtXLibError, conn: &XlibConnection) -> Self { let mut buf = [0; 255]; - unsafe { - (conn.xlib.XGetErrorText)( - conn.dpy, - error.error_code.into(), - buf.as_mut_ptr().cast(), - (buf.len() - 1) as i32, - ) - }; - - *buf.last_mut().unwrap() = 0; - // SAFETY: whatever XGetErrorText did or not, we guaranteed there is a nul byte at the end of the buffer - let cstr = unsafe { CStr::from_ptr(buf.as_mut_ptr().cast()) }; - - cstr.to_string_lossy().into() + let cstr = conn.get_error_text(&mut buf, inner.error_code); + + Self { display_name: cstr.to_string_lossy().into(), inner } } } diff --git a/src/wrappers/xlib/xlib_connection.rs b/src/wrappers/xlib/xlib_connection.rs new file mode 100644 index 00000000..8d04afdf --- /dev/null +++ b/src/wrappers/xlib/xlib_connection.rs @@ -0,0 +1,119 @@ +use std::error::Error; +use std::ffi::CStr; +use std::fmt::Formatter; +use std::os::raw::{c_int, c_uchar}; +use std::ptr::NonNull; +use std::sync::Arc; +use x11_dl::xlib::{Display, XErrorEvent, Xlib}; +use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; + +/// An owned Xlib Display connection. +/// +/// This type guarantees the inner display connection object to be alive and valid, as long as this +/// is alive. +/// +/// It will also always close the display connection on drop. +pub struct XlibConnection { + display: NonNull, + xlib: Arc, +} + +impl XlibConnection { + pub fn open() -> Result> { + let xlib = Arc::new(Xlib::open()?); + + // SAFETY: It's always safe to call XOpenDisplay with a NULL display_name + let ptr = unsafe { (xlib.XOpenDisplay)(core::ptr::null()) }; + + let Some(display) = NonNull::new(ptr) else { return Err(DisplayOpenFailedError.into()) }; + + Ok(Self { display, xlib }) + } + + pub fn set_xcb_queue_owner(&self, xlib_xcb: &Xlib_xcb) { + // SAFETY: This type ensures the display pointer is always valid. + unsafe { + (xlib_xcb.XSetEventQueueOwner)( + self.display.as_ptr(), + XEventQueueOwner::XCBOwnsEventQueue, + ) + } + } + + /// Safe wrapper for XDefaultScreen + pub fn default_screen(&self) -> c_int { + // SAFETY: This type ensures the display pointer is always valid. + unsafe { (self.xlib.XDefaultScreen)(self.display.as_ptr()) } + } + + pub fn dpy(&self) -> *mut Display { + self.display.as_ptr() + } + + pub fn xlib(&self) -> &Xlib { + &self.xlib + } + + /// Calls XSync(0) + pub fn sync(&self) { + // SAFETY: This type ensures the display pointer is always valid. + unsafe { (self.xlib.XSync)(self.display.as_ptr(), 0) }; + } + + pub fn get_error_text(&self, buf: &mut [u8], error_code: c_uchar) -> &CStr { + if buf.len() == 0 { + return c""; + } + + // PANIC: we just checked above that buf.len > 0 + let buf_len = buf.len() - 1; + let Ok(buf_len) = buf_len.try_into() else { + // Buffers should never get that big, something went horribly wrong. + return c""; + }; + + // SAFETY: This type ensures the display pointer is always valid. + // Moreover, the buffer pointer is guaranteed to be valid for writes for the given length, as it comes from the given mutable slice. + unsafe { + (self.xlib.XGetErrorText)( + self.dpy(), + error_code.into(), + buf.as_mut_ptr().cast(), + buf_len, + ) + }; + + // PANIC: we checked above that buf.len > 0 + *buf.last_mut().unwrap() = 0; + + // SAFETY: whatever XGetErrorText did or not, we guaranteed there is a nul byte at the end of the buffer + unsafe { CStr::from_ptr(buf.as_mut_ptr().cast()) } + } + + pub fn set_error_handler( + &self, new_error_handler: Option, + ) -> Option { + // SAFETY: XSetErrorHandler is always safe to call as long as the function pointer is valid, + // which this guarantees + unsafe { (self.xlib.XSetErrorHandler)(new_error_handler) } + } +} + +type ErrorHandler = unsafe extern "C" fn(*mut Display, *mut XErrorEvent) -> c_int; + +impl Drop for XlibConnection { + fn drop(&mut self) { + // SAFETY: This type guarantees the display pointer is valid. + // This being `Drop` also prevents any double-free. + unsafe { (self.xlib.XCloseDisplay)(self.display.as_ptr()) }; + } +} + +#[derive(Debug)] +struct DisplayOpenFailedError; +impl std::fmt::Display for DisplayOpenFailedError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str("Failed to open X11 display connection: XOpenDisplay() failed") + } +} +impl Error for DisplayOpenFailedError {} diff --git a/src/wrappers/xlib/xlib_xcb.rs b/src/wrappers/xlib/xlib_xcb.rs new file mode 100644 index 00000000..6d251cae --- /dev/null +++ b/src/wrappers/xlib/xlib_xcb.rs @@ -0,0 +1,72 @@ +use crate::wrappers::xlib::xlib_connection::XlibConnection; +use std::error::Error; +use std::ops::Deref; +use std::os::raw::c_int; +use x11_dl::xlib::Display; +use x11_dl::xlib_xcb::Xlib_xcb; +use x11rb::xcb_ffi::XCBConnection; + +/// A Xlib/XCB connection object. +/// +/// This exposes both a raw Xlib display pointer, and a x11rb XCBConnection object. +/// +/// This allows us to interface with the same connection using Xlib (needed for GLX, and for FFI), +/// as well as XCB (needed to preserve our sanity). +/// +/// (Note: The term Xlib/XCB means "Xlib over XCB", not "Xlib or XCB"). +pub struct XlibXcbConnection { + // SAFETY: Drop order matters here! We *MUST* Drop the XCBConnection first, as it essentially + // borrows the Xlib/XCB connection + xcb_connection: XCBConnection, + xlib_connection: XlibConnection, +} + +impl XlibXcbConnection { + pub fn open() -> Result> { + let xlib_xcb = Xlib_xcb::open()?; + // Open the connection to the X11 server as a Xlib/XCB connection object + let xlib_connection = XlibConnection::open()?; + // Set the XCB end of the Xlib/XCB connection object as the queue owner. + // From now on, we'll use XCB (i.e. X11rb) to interface with the event queue + xlib_connection.set_xcb_queue_owner(&xlib_xcb); + + // Extract the XCB connection object pointer from the Xlib/XCB connection object + // SAFETY: This is always safe to call as long as the OwnedDisplayConnection is alive + let xcb_connection = unsafe { (xlib_xcb.XGetXCBConnection)(xlib_connection.dpy()) }; + + // The XGetXCBConnection function is not documented to ever be able to return NULL. + // Still, this is cheap to check, just in case. + assert!(!xcb_connection.is_null()); + + // Wrap the XCB connection object in a x11rb connection object + // SAFETY: The xcb_connection pointer should be valid. We also enforce the drop order in this + let xcb_connection = + unsafe { XCBConnection::from_raw_xcb_connection(xcb_connection, false)? }; + + Ok(Self { xcb_connection, xlib_connection }) + } + + pub fn default_screen(&self) -> c_int { + self.xlib_connection.default_screen() + } + + pub fn xlib_display(&self) -> *mut Display { + self.xlib_connection.dpy() + } + + pub fn xcb_connection(&self) -> &XCBConnection { + &self.xcb_connection + } + pub fn xlib_connection(&self) -> &XlibConnection { + &self.xlib_connection + } +} + +// For convenience +impl Deref for XlibXcbConnection { + type Target = XCBConnection; + + fn deref(&self) -> &Self::Target { + &self.xcb_connection + } +} diff --git a/src/x11/mod.rs b/src/x11/mod.rs index d3ed7dd0..f1c14892 100644 --- a/src/x11/mod.rs +++ b/src/x11/mod.rs @@ -8,5 +8,3 @@ mod cursor; mod event_loop; mod keyboard; mod visual_info; - -mod xlib_xcb; diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index 35b4cd07..6a7b69d3 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -8,7 +8,7 @@ use x11rb::protocol::xproto::{Cursor, Screen}; use x11rb::resource_manager; use super::cursor; -use crate::x11::xlib_xcb::XlibXcbConnection; +use crate::wrappers::xlib::XlibXcbConnection; use crate::MouseCursor; x11rb::atom_manager! { diff --git a/src/x11/xlib_xcb.rs b/src/x11/xlib_xcb.rs deleted file mode 100644 index ec562415..00000000 --- a/src/x11/xlib_xcb.rs +++ /dev/null @@ -1,133 +0,0 @@ -use std::error::Error; -use std::fmt::Formatter; -use std::ops::Deref; -use std::os::raw::c_int; -use std::ptr::NonNull; -use std::sync::Arc; -use x11_dl::xlib::{Display, Xlib}; -use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; -use x11rb::xcb_ffi::XCBConnection; - -/// A Xlib/XCB connection object. -/// -/// This exposes both a raw Xlib display pointer, and a x11rb XCBConnection object. -/// -/// This allows us to interface with the same connection using Xlib (needed for GLX, and for FFI), -/// as well as XCB (needed to preserve our sanity). -/// -/// (Note: The term Xlib/XCB means "Xlib over XCB", not "Xlib or XCB"). -pub struct XlibXcbConnection { - // SAFETY: Drop order matters here! We *MUST* Drop the XCBConnection first, as it essentially - // borrows the Xlib/XCB connection - xcb_connection: XCBConnection, - xlib_connection: OwnedDisplayConnection, -} - -impl XlibXcbConnection { - pub fn open() -> Result> { - let xlib_xcb = Xlib_xcb::open()?; - // Open the connection to the X11 server as a Xlib/XCB connection object - let xlib_connection = OwnedDisplayConnection::open()?; - // Set the XCB end of the Xlib/XCB connection object as the queue owner. - // From now on, we'll use XCB (i.e. X11rb) to interface with the event queue - xlib_connection.set_xcb_queue_owner(&xlib_xcb); - - // Extract the XCB connection object pointer from the Xlib/XCB connection object - // SAFETY: This is always safe to call as long as the OwnedDisplayConnection is alive - let xcb_connection = - unsafe { (xlib_xcb.XGetXCBConnection)(xlib_connection.display.as_ptr()) }; - - // The XGetXCBConnection function is not documented to ever be able to return NULL. - // Still, this is cheap to check, just in case. - assert!(!xcb_connection.is_null()); - - // Wrap the XCB connection object in a x11rb connection object - // SAFETY: The xcb_connection pointer should be valid. We also enforce the drop order in this - let xcb_connection = - unsafe { XCBConnection::from_raw_xcb_connection(xcb_connection, false)? }; - - Ok(Self { xcb_connection, xlib_connection }) - } - - pub fn default_screen(&self) -> c_int { - self.xlib_connection.default_screen() - } - - pub fn xlib(&self) -> &Xlib { - &self.xlib_connection.xlib - } - - pub fn xlib_display(&self) -> *mut Display { - self.xlib_connection.display.as_ptr() - } - - pub fn xcb_connection(&self) -> &XCBConnection { - &self.xcb_connection - } -} - -// For convenience -impl Deref for XlibXcbConnection { - type Target = XCBConnection; - - fn deref(&self) -> &Self::Target { - &self.xcb_connection - } -} - -/// An owned Xlib Display connection. -/// -/// This type guarantees the inner display connection object to be alive and valid, as long as this -/// is alive. -/// -/// It will also always close the display connection on drop. -struct OwnedDisplayConnection { - display: NonNull, - xlib: Arc, -} - -impl OwnedDisplayConnection { - pub fn open() -> Result> { - let xlib = Arc::new(Xlib::open()?); - - // SAFETY: It's always safe to call XOpenDisplay with a NULL display_name - let ptr = unsafe { (xlib.XOpenDisplay)(core::ptr::null()) }; - - let Some(display) = NonNull::new(ptr) else { return Err(DisplayOpenFailedError.into()) }; - - Ok(Self { display, xlib }) - } - - fn set_xcb_queue_owner(&self, xlib_xcb: &Xlib_xcb) { - // SAFETY: This type ensures the display pointer is always valid. - unsafe { - (xlib_xcb.XSetEventQueueOwner)( - self.display.as_ptr(), - XEventQueueOwner::XCBOwnsEventQueue, - ) - } - } - - /// Safe wrapper for XDefaultScreen - fn default_screen(&self) -> c_int { - // SAFETY: This type ensures the display pointer is always valid. - unsafe { (self.xlib.XDefaultScreen)(self.display.as_ptr()) } - } -} - -impl Drop for OwnedDisplayConnection { - fn drop(&mut self) { - // SAFETY: This type guarantees the display pointer is valid. - // This being `Drop` also prevents any double-free. - unsafe { (self.xlib.XCloseDisplay)(self.display.as_ptr()) }; - } -} - -#[derive(Debug)] -struct DisplayOpenFailedError; -impl std::fmt::Display for DisplayOpenFailedError { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.write_str("Failed to open X11 display connection: XOpenDisplay() failed") - } -} -impl Error for DisplayOpenFailedError {} From 75c6a2bcf0a3ed0d9c7e059436c0c1859ab1f58c Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 01:46:18 +0200 Subject: [PATCH 4/9] Fix --- src/wrappers.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/wrappers.rs b/src/wrappers.rs index 739635d5..033e741c 100644 --- a/src/wrappers.rs +++ b/src/wrappers.rs @@ -11,7 +11,10 @@ //! However, all of these APIs should always be sound (i.e. no UB can be triggered by safe code). //! Otherwise, this should be considered a bug and reported accordingly. -pub mod glx; /// Wrappers and utilities around Xlib. (provided by x11_dl) #[cfg(target_os = "linux")] pub mod xlib; + +/// Wrappers and utilities around GLX +#[cfg(target_os = "linux")] +pub mod glx; From d7d1ac6552f4b8c78e907681739abd286a81db3f Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 01:52:35 +0200 Subject: [PATCH 5/9] fixes --- src/gl/x11.rs | 3 +-- src/wrappers.rs | 2 +- src/wrappers/xlib/xlib_connection.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/gl/x11.rs b/src/gl/x11.rs index b85474a6..b8af41c8 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -6,8 +6,7 @@ use std::rc::Rc; use x11_dl::error::OpenError; use x11_dl::glx::GLXContext; -use crate::wrappers::glx::Glx; -use crate::wrappers::glx::GlxFbConfig; +use crate::wrappers::glx::*; use crate::wrappers::xlib::{XErrorHandler, XLibError}; #[derive(Debug)] diff --git a/src/wrappers.rs b/src/wrappers.rs index 033e741c..a392a348 100644 --- a/src/wrappers.rs +++ b/src/wrappers.rs @@ -16,5 +16,5 @@ pub mod xlib; /// Wrappers and utilities around GLX -#[cfg(target_os = "linux")] +#[cfg(all(target_os = "linux", feature = "opengl"))] pub mod glx; diff --git a/src/wrappers/xlib/xlib_connection.rs b/src/wrappers/xlib/xlib_connection.rs index 8d04afdf..50eb9137 100644 --- a/src/wrappers/xlib/xlib_connection.rs +++ b/src/wrappers/xlib/xlib_connection.rs @@ -61,7 +61,7 @@ impl XlibConnection { } pub fn get_error_text(&self, buf: &mut [u8], error_code: c_uchar) -> &CStr { - if buf.len() == 0 { + if buf.is_empty() { return c""; } From a1aec09c22e5e064b2335bc06467419422450861 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 02:02:05 +0200 Subject: [PATCH 6/9] More fixes for gl-less build --- src/wrappers/xlib.rs | 9 +++++++-- src/wrappers/xlib/xlib_connection.rs | 17 +++++++++++------ src/wrappers/xlib/xlib_xcb.rs | 2 ++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/wrappers/xlib.rs b/src/wrappers/xlib.rs index 32559030..e8db61d3 100644 --- a/src/wrappers/xlib.rs +++ b/src/wrappers/xlib.rs @@ -1,7 +1,12 @@ +#[cfg(feature = "opengl")] mod error_handler; mod xlib_connection; mod xlib_xcb; -pub use error_handler::{XErrorHandler, XLibError}; -pub use xlib_connection::XlibConnection; pub use xlib_xcb::XlibXcbConnection; + +#[cfg(feature = "opengl")] +pub use self::{ + error_handler::{XErrorHandler, XLibError}, + xlib_connection::XlibConnection, +}; diff --git a/src/wrappers/xlib/xlib_connection.rs b/src/wrappers/xlib/xlib_connection.rs index 50eb9137..a366a64d 100644 --- a/src/wrappers/xlib/xlib_connection.rs +++ b/src/wrappers/xlib/xlib_connection.rs @@ -1,10 +1,9 @@ use std::error::Error; -use std::ffi::CStr; use std::fmt::Formatter; -use std::os::raw::{c_int, c_uchar}; +use std::os::raw::c_int; use std::ptr::NonNull; use std::sync::Arc; -use x11_dl::xlib::{Display, XErrorEvent, Xlib}; +use x11_dl::xlib::{Display, Xlib}; use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; /// An owned Xlib Display connection. @@ -49,7 +48,10 @@ impl XlibConnection { pub fn dpy(&self) -> *mut Display { self.display.as_ptr() } +} +#[cfg(feature = "opengl")] +impl XlibConnection { pub fn xlib(&self) -> &Xlib { &self.xlib } @@ -60,7 +62,9 @@ impl XlibConnection { unsafe { (self.xlib.XSync)(self.display.as_ptr(), 0) }; } - pub fn get_error_text(&self, buf: &mut [u8], error_code: c_uchar) -> &CStr { + pub fn get_error_text( + &self, buf: &mut [u8], error_code: core::ffi::c_uchar, + ) -> &core::ffi::CStr { if buf.is_empty() { return c""; } @@ -87,7 +91,7 @@ impl XlibConnection { *buf.last_mut().unwrap() = 0; // SAFETY: whatever XGetErrorText did or not, we guaranteed there is a nul byte at the end of the buffer - unsafe { CStr::from_ptr(buf.as_mut_ptr().cast()) } + unsafe { std::ffi::CStr::from_ptr(buf.as_mut_ptr().cast()) } } pub fn set_error_handler( @@ -99,7 +103,8 @@ impl XlibConnection { } } -type ErrorHandler = unsafe extern "C" fn(*mut Display, *mut XErrorEvent) -> c_int; +#[cfg(feature = "opengl")] +type ErrorHandler = unsafe extern "C" fn(*mut Display, *mut x11_dl::xlib::XErrorEvent) -> c_int; impl Drop for XlibConnection { fn drop(&mut self) { diff --git a/src/wrappers/xlib/xlib_xcb.rs b/src/wrappers/xlib/xlib_xcb.rs index 6d251cae..c8bd517c 100644 --- a/src/wrappers/xlib/xlib_xcb.rs +++ b/src/wrappers/xlib/xlib_xcb.rs @@ -57,6 +57,8 @@ impl XlibXcbConnection { pub fn xcb_connection(&self) -> &XCBConnection { &self.xcb_connection } + + #[cfg(feature = "opengl")] pub fn xlib_connection(&self) -> &XlibConnection { &self.xlib_connection } From 2adb18f5e5fce15b59fa47b981d7250deb19d53e Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 13:36:58 +0200 Subject: [PATCH 7/9] Cleanup unnecessary unsafe --- src/gl/x11.rs | 24 +++++++++++++----------- src/x11/window.rs | 5 ++--- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/gl/x11.rs b/src/gl/x11.rs index b8af41c8..81c73c88 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -62,7 +62,7 @@ impl GlContext { /// only then create the OpenGL context. /// /// Use [Self::get_fb_config_and_visual] to create both of these things. - pub unsafe fn create( + pub fn create( window: c_ulong, connection: Rc, config: FbConfig, ) -> Result { let glx = Glx::open()?; @@ -88,16 +88,18 @@ impl GlContext { // Create context object here so that error or panic will properly free the context let context = GlContext { glx, window, connection: Rc::clone(&connection), context }; - context.glx.with_current_context( - xlib_connection, - window, - context.context, - error_handler, - || { - swap_interval(xlib_connection.dpy(), window, config.gl_config.vsync as i32); - error_handler.check() - }, - )??; + unsafe { + context.glx.with_current_context( + xlib_connection, + window, + context.context, + error_handler, + || { + swap_interval(xlib_connection.dpy(), window, config.gl_config.vsync as i32); + error_handler.check() + }, + )??; + } Ok(context) }) diff --git a/src/x11/window.rs b/src/x11/window.rs index aa2adcac..f17380c3 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -264,10 +264,9 @@ impl<'a> Window<'a> { let window = window_id as c_ulong; // Because of the visual negotation we had to take some extra steps to create this context - let context = unsafe { + let context = platform::GlContext::create(window, Rc::clone(&xcb_connection), fb_config) - } - .expect("Could not create OpenGL context"); + .expect("Could not create OpenGL context"); GlContext::new(context) }); From 1eb84de9d928c8e9e2de45693462bba15103b0f0 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 18:00:35 +0200 Subject: [PATCH 8/9] Save the screen_idx in the XlibConnection --- src/gl/x11.rs | 7 +------ src/wrappers/glx.rs | 5 ++--- src/wrappers/xlib/xlib_connection.rs | 19 ++++++++++++++----- src/wrappers/xlib/xlib_xcb.rs | 2 +- src/x11/window.rs | 2 +- src/x11/xcb_connection.rs | 7 ++----- 6 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/gl/x11.rs b/src/gl/x11.rs index 81c73c88..72b700d0 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -116,12 +116,7 @@ impl GlContext { let xlib_connection = connection.conn.xlib_connection(); XErrorHandler::handle(xlib_connection, |error_handler| { - let fb_config = glx.choose_best_fb_config( - xlib_connection, - &config, - connection.screen, - error_handler, - )?; + let fb_config = glx.choose_best_fb_config(xlib_connection, &config, error_handler)?; // Now that we have a matching framebuffer config, we need to know which visual matches // this config so the window is compatible with the OpenGL context we're about to create diff --git a/src/wrappers/glx.rs b/src/wrappers/glx.rs index 0bd11095..9a200f78 100644 --- a/src/wrappers/glx.rs +++ b/src/wrappers/glx.rs @@ -58,8 +58,7 @@ impl Glx { } pub fn choose_best_fb_config( - &self, connection: &XlibConnection, config: &GlConfig, screen: c_int, - error_handler: &XErrorHandler, + &self, connection: &XlibConnection, config: &GlConfig, error_handler: &XErrorHandler, ) -> Result { let fb_attribs = Self::get_fb_attribs(config); @@ -69,7 +68,7 @@ impl Glx { let result = unsafe { (self.inner.glXChooseFBConfig)( connection.dpy(), - screen, + connection.default_screen_index(), fb_attribs.as_ptr(), &mut nelements, ) diff --git a/src/wrappers/xlib/xlib_connection.rs b/src/wrappers/xlib/xlib_connection.rs index a366a64d..b1589fea 100644 --- a/src/wrappers/xlib/xlib_connection.rs +++ b/src/wrappers/xlib/xlib_connection.rs @@ -2,7 +2,6 @@ use std::error::Error; use std::fmt::Formatter; use std::os::raw::c_int; use std::ptr::NonNull; -use std::sync::Arc; use x11_dl::xlib::{Display, Xlib}; use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; @@ -14,19 +13,24 @@ use x11_dl::xlib_xcb::{XEventQueueOwner, Xlib_xcb}; /// It will also always close the display connection on drop. pub struct XlibConnection { display: NonNull, - xlib: Arc, + xlib: Box, + default_screen: c_int, } impl XlibConnection { pub fn open() -> Result> { - let xlib = Arc::new(Xlib::open()?); + let xlib = Box::new(Xlib::open()?); // SAFETY: It's always safe to call XOpenDisplay with a NULL display_name let ptr = unsafe { (xlib.XOpenDisplay)(core::ptr::null()) }; let Some(display) = NonNull::new(ptr) else { return Err(DisplayOpenFailedError.into()) }; - Ok(Self { display, xlib }) + let mut this = Self { display, xlib, default_screen: 0 }; + + this.default_screen = this.fetch_default_screen(); + + Ok(this) } pub fn set_xcb_queue_owner(&self, xlib_xcb: &Xlib_xcb) { @@ -39,8 +43,13 @@ impl XlibConnection { } } + /// Returns the index of the default screen for this X server. + pub fn default_screen_index(&self) -> c_int { + self.default_screen + } + /// Safe wrapper for XDefaultScreen - pub fn default_screen(&self) -> c_int { + fn fetch_default_screen(&self) -> c_int { // SAFETY: This type ensures the display pointer is always valid. unsafe { (self.xlib.XDefaultScreen)(self.display.as_ptr()) } } diff --git a/src/wrappers/xlib/xlib_xcb.rs b/src/wrappers/xlib/xlib_xcb.rs index c8bd517c..b0074f39 100644 --- a/src/wrappers/xlib/xlib_xcb.rs +++ b/src/wrappers/xlib/xlib_xcb.rs @@ -47,7 +47,7 @@ impl XlibXcbConnection { } pub fn default_screen(&self) -> c_int { - self.xlib_connection.default_screen() + self.xlib_connection.default_screen_index() } pub fn xlib_display(&self) -> *mut Display { diff --git a/src/x11/window.rs b/src/x11/window.rs index f17380c3..6414486d 100644 --- a/src/x11/window.rs +++ b/src/x11/window.rs @@ -367,7 +367,7 @@ unsafe impl<'a> HasRawDisplayHandle for Window<'a> { let mut handle = XlibDisplayHandle::empty(); handle.display = display as *mut c_void; - handle.screen = self.inner.xcb_connection.screen; + handle.screen = self.inner.xcb_connection.conn.default_screen(); RawDisplayHandle::Xlib(handle) } diff --git a/src/x11/xcb_connection.rs b/src/x11/xcb_connection.rs index 6a7b69d3..68ed2db8 100644 --- a/src/x11/xcb_connection.rs +++ b/src/x11/xcb_connection.rs @@ -1,7 +1,6 @@ use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::error::Error; -use std::ffi::c_int; use x11rb::connection::Connection; use x11rb::cursor::Handle as CursorHandle; use x11rb::protocol::xproto::{Cursor, Screen}; @@ -23,7 +22,6 @@ x11rb::atom_manager! { /// Keeps track of the xcb connection itself and the xlib display ID that was used to connect. pub struct XcbConnection { pub(crate) conn: XlibXcbConnection, - pub(crate) screen: c_int, pub(crate) atoms: Atoms, pub(crate) resources: resource_manager::Database, pub(crate) cursor_handle: CursorHandle, @@ -42,7 +40,6 @@ impl XcbConnection { Ok(Self { conn, - screen, atoms, resources, cursor_handle, @@ -101,7 +98,7 @@ impl XcbConnection { Entry::Vacant(entry) => { let cursor = cursor::get_xcursor( &self.conn, - self.screen as usize, + self.conn.default_screen() as usize, &self.cursor_handle, cursor, )?; @@ -112,6 +109,6 @@ impl XcbConnection { } pub fn screen(&self) -> &Screen { - &self.conn.setup().roots[self.screen as usize] + &self.conn.setup().roots[self.conn.default_screen() as usize] } } From a17ad4224ddc9954f7320f4466720f76201773c3 Mon Sep 17 00:00:00 2001 From: Adrien Prokopowicz <6529475+prokopyl@users.noreply.github.com> Date: Sun, 17 May 2026 18:02:54 +0200 Subject: [PATCH 9/9] Minor rename --- src/gl/x11.rs | 6 +++++- src/wrappers/glx.rs | 18 ++++++++++++------ src/wrappers/xlib/xlib_connection.rs | 4 ++-- src/wrappers/xlib/xlib_xcb.rs | 4 ++-- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/gl/x11.rs b/src/gl/x11.rs index 72b700d0..53f60fee 100644 --- a/src/gl/x11.rs +++ b/src/gl/x11.rs @@ -95,7 +95,11 @@ impl GlContext { context.context, error_handler, || { - swap_interval(xlib_connection.dpy(), window, config.gl_config.vsync as i32); + swap_interval( + xlib_connection.as_raw(), + window, + config.gl_config.vsync as i32, + ); error_handler.check() }, )??; diff --git a/src/wrappers/glx.rs b/src/wrappers/glx.rs index 9a200f78..be6ba4aa 100644 --- a/src/wrappers/glx.rs +++ b/src/wrappers/glx.rs @@ -67,7 +67,7 @@ impl Glx { // The fb_attribs and nelements pointers come from references and are therefore valid. let result = unsafe { (self.inner.glXChooseFBConfig)( - connection.dpy(), + connection.as_raw(), connection.default_screen_index(), fb_attribs.as_ptr(), &mut nelements, @@ -95,7 +95,7 @@ impl Glx { ) -> Result { // SAFETY: XlibConnection guarantees the inner dpy is valid. let result = - unsafe { (self.inner.glXGetVisualFromFBConfig)(connection.dpy(), fb_config.0) }; + unsafe { (self.inner.glXGetVisualFromFBConfig)(connection.as_raw(), fb_config.0) }; error_handler.check()?; if result.is_null() { @@ -116,7 +116,7 @@ impl Glx { &self, connection: &XlibConnection, window_id: c_ulong, error_handler: &XErrorHandler, ) -> Result<(), GlError> { // SAFETY: XlibConnection guarantees the inner dpy is valid. - unsafe { (self.inner.glXSwapBuffers)(connection.dpy(), window_id) }; + unsafe { (self.inner.glXSwapBuffers)(connection.as_raw(), window_id) }; Ok(error_handler.check()?) } @@ -145,7 +145,7 @@ impl Glx { pub unsafe fn destroy_context(&self, connection: &XlibConnection, context: GLXContext) { // SAFETY: XlibConnection guarantees the inner dpy is valid. - unsafe { (self.inner.glXDestroyContext)(connection.dpy(), context) }; + unsafe { (self.inner.glXDestroyContext)(connection.as_raw(), context) }; } pub unsafe fn make_current( @@ -153,7 +153,7 @@ impl Glx { error_handler: &XErrorHandler, ) -> Result<(), GlError> { // SAFETY: XlibConnection guarantees the inner dpy is valid. - let res = unsafe { (self.inner.glXMakeCurrent)(connection.dpy(), window_id, context) }; + let res = unsafe { (self.inner.glXMakeCurrent)(connection.as_raw(), window_id, context) }; error_handler.check()?; if res == 0 { @@ -225,7 +225,13 @@ impl GlxCreateContextAttribsARB { let ctx_attribs = Self::get_ctx_attribs(gl_config); let context = unsafe { - self.0(connection.dpy(), glx_fb_config.0, std::ptr::null_mut(), 1, ctx_attribs.as_ptr()) + self.0( + connection.as_raw(), + glx_fb_config.0, + std::ptr::null_mut(), + 1, + ctx_attribs.as_ptr(), + ) }; error_handler.check()?; diff --git a/src/wrappers/xlib/xlib_connection.rs b/src/wrappers/xlib/xlib_connection.rs index b1589fea..b43a7c53 100644 --- a/src/wrappers/xlib/xlib_connection.rs +++ b/src/wrappers/xlib/xlib_connection.rs @@ -54,7 +54,7 @@ impl XlibConnection { unsafe { (self.xlib.XDefaultScreen)(self.display.as_ptr()) } } - pub fn dpy(&self) -> *mut Display { + pub fn as_raw(&self) -> *mut Display { self.display.as_ptr() } } @@ -89,7 +89,7 @@ impl XlibConnection { // Moreover, the buffer pointer is guaranteed to be valid for writes for the given length, as it comes from the given mutable slice. unsafe { (self.xlib.XGetErrorText)( - self.dpy(), + self.as_raw(), error_code.into(), buf.as_mut_ptr().cast(), buf_len, diff --git a/src/wrappers/xlib/xlib_xcb.rs b/src/wrappers/xlib/xlib_xcb.rs index b0074f39..88e84396 100644 --- a/src/wrappers/xlib/xlib_xcb.rs +++ b/src/wrappers/xlib/xlib_xcb.rs @@ -32,7 +32,7 @@ impl XlibXcbConnection { // Extract the XCB connection object pointer from the Xlib/XCB connection object // SAFETY: This is always safe to call as long as the OwnedDisplayConnection is alive - let xcb_connection = unsafe { (xlib_xcb.XGetXCBConnection)(xlib_connection.dpy()) }; + let xcb_connection = unsafe { (xlib_xcb.XGetXCBConnection)(xlib_connection.as_raw()) }; // The XGetXCBConnection function is not documented to ever be able to return NULL. // Still, this is cheap to check, just in case. @@ -51,7 +51,7 @@ impl XlibXcbConnection { } pub fn xlib_display(&self) -> *mut Display { - self.xlib_connection.dpy() + self.xlib_connection.as_raw() } pub fn xcb_connection(&self) -> &XCBConnection {