Introduce proper, safe Xlib/XCB wrapper

src/gl/x11.rs

@@ -6,10 +6,8 @@ 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::*;
+use crate::wrappers::xlib::{XErrorHandler, XLibError};
 
 #[derive(Debug)]
 pub enum CreationFailedError {
@@ -18,12 +16,12 @@ pub enum CreationFailedError {
     GetProcAddressFailed,
     MakeCurrentFailed,
     ContextCreationFailed,
-    X11Error(errors::XLibError),
+    X11Error(XLibError),
     OpenError(OpenError),
 }
 
-impl From<errors::XLibError> for GlError {
-    fn from(e: errors::XLibError) -> Self {
+impl From<XLibError> for GlError {
+    fn from(e: XLibError) -> Self {
         GlError::CreationFailed(CreationFailedError::X11Error(e))
     }
 }
@@ -64,12 +62,14 @@ 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<XcbConnection>, config: FbConfig,
     ) -> Result<GlContext, GlError> {
         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 +79,7 @@ impl GlContext {
             };
 
             let context = create_context.call(
-                &connection,
+                xlib_connection,
                 &config.gl_config,
                 config.fb_config,
                 error_handler,
@@ -88,16 +88,22 @@ 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(
-                &connection,
-                window,
-                context.context,
-                error_handler,
-                || {
-                    swap_interval(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.as_raw(),
+                            window,
+                            config.gl_config.vsync as i32,
+                        );
+                        error_handler.check()
+                    },
+                )??;
+            }
 
             Ok(context)
         })
@@ -107,16 +113,19 @@ 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, 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) }
     }
 }

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;

src/wrappers.rs

@@ -0,0 +1,20 @@
+#![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.
+
+/// Wrappers and utilities around Xlib. (provided by x11_dl)
+#[cfg(target_os = "linux")]
+pub mod xlib;
+
+/// Wrappers and utilities around GLX
+#[cfg(all(target_os = "linux", feature = "opengl"))]
+pub mod glx;

src/gl/x11/glx.rs → 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,17 @@ impl Glx {
     }
 
     pub fn choose_best_fb_config(
-        &self, connection: &XcbConnection, config: &GlConfig, error_handler: &XErrorHandler,
+        &self, connection: &XlibConnection, config: &GlConfig, error_handler: &XErrorHandler,
     ) -> Result<GlxFbConfig, GlError> {
         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.as_raw(),
+                connection.default_screen_index(),
                 fb_attribs.as_ptr(),
                 &mut nelements,
             )
@@ -85,16 +85,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<XVisualInfo, GlError> {
-        // 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.as_raw(), fb_config.0) };
 
         error_handler.check()?;
         if result.is_null() {
@@ -106,16 +107,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.as_raw(), window_id) };
 
         Ok(error_handler.check()?)
     }
@@ -142,16 +143,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.as_raw(), 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.as_raw(), window_id, context) };
 
         error_handler.check()?;
         if res == 0 {
@@ -162,13 +164,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<T>(
-        &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<T, GlError> {
         self.make_current(connection, window_id, context, error_handler)?;
@@ -186,7 +188,7 @@ impl Glx {
 
 pub struct ContextClearOnDrop<'a> {
     glx: &'a Glx,
-    connection: &'a XcbConnection,
+    connection: &'a XlibConnection,
     error_handler: &'a XErrorHandler<'a>,
 }
 
@@ -217,13 +219,19 @@ 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<GLXContext, GlError> {
         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()?;

src/wrappers/xlib.rs

@@ -0,0 +1,12 @@
+#[cfg(feature = "opengl")]
+mod error_handler;
+mod xlib_connection;
+mod xlib_xcb;
+
+pub use xlib_xcb::XlibXcbConnection;
+
+#[cfg(feature = "opengl")]
+pub use self::{
+    error_handler::{XErrorHandler, XLibError},
+    xlib_connection::XlibConnection,
+};