Implement OpenGL contexts for X11

This should in theory work! When requesting an OpenGL context, the
window visual is determined based on the matched framebuffer config.

src/gl/mod.rs

@@ -8,10 +8,11 @@ mod win;
 #[cfg(target_os = "windows")]
 use win as platform;
 
+// We need to use this directly within the X11 window creation to negotiate the correct visual
 #[cfg(target_os = "linux")]
-mod x11;
+pub(crate) mod x11;
 #[cfg(target_os = "linux")]
-use self::x11 as platform;
+pub(crate) use self::x11 as platform;
 
 #[cfg(target_os = "macos")]
 mod macos;
@@ -72,6 +73,7 @@ pub struct GlContext {
 }
 
 impl GlContext {
+    #[cfg(not(target_os = "linux"))]
     pub(crate) unsafe fn create(
         parent: &impl HasRawWindowHandle, config: GlConfig,
     ) -> Result<GlContext, GlError> {
@@ -79,6 +81,14 @@ impl GlContext {
             .map(|context| GlContext { context, phantom: PhantomData })
     }
 
+    /// The X11 version needs to be set up in a different way compared to the Windows and macOS
+    /// versions. So the platform-specific versions should be used to construct the context within
+    /// baseview, and then this object can be passed to the user.
+    #[cfg(target_os = "linux")]
+    pub(crate) fn new(context: platform::GlContext) -> GlContext {
+        GlContext { context, phantom: PhantomData }
+    }
+
     pub unsafe fn make_current(&self) {
         self.context.make_current();
     }

src/gl/x11.rs

@@ -13,6 +13,7 @@ mod errors;
 #[derive(Debug)]
 pub enum CreationFailedError {
     InvalidFBConfig,
+    NoVisual,
     GetProcAddressFailed,
     MakeCurrentFailed,
     ContextCreationFailed,
@@ -55,9 +56,31 @@ pub struct GlContext {
     context: glx::GLXContext,
 }
 
+/// The frame buffer configuration along with the general OpenGL configuration to somewhat minimize
+/// misuse.
+pub struct FbConfig {
+    gl_config: GlConfig,
+    fb_config: *mut glx::__GLXFBConfigRec,
+}
+
+/// The configuration a window should be created with after calling
+/// [GlContext::get_fb_config_and_visual].
+pub struct WindowConfig {
+    pub depth: u8,
+    pub visual: u32,
+}
+
 impl GlContext {
+    /// Creating an OpenGL context under X11 works slightly different. Different OpenGL
+    /// configurations require different framebuffer configurations, and to be able to use that
+    /// context with a window the window needs to be created with a matching visual. This means that
+    /// you need to decide on the framebuffer config before creating the window, ask the X11 server
+    /// for a matching visual for that framebuffer config, crate the window with that visual, and
+    /// only then create the OpenGL context.
+    ///
+    /// Use [Self::get_fb_config_and_visual] to create both of these things.
     pub unsafe fn create(
-        parent: &impl HasRawWindowHandle, config: GlConfig,
+        parent: &impl HasRawWindowHandle, config: FbConfig,
     ) -> Result<GlContext, GlError> {
         let handle = if let RawWindowHandle::Xlib(handle) = parent.raw_window_handle() {
             handle
@@ -71,6 +94,84 @@ impl GlContext {
 
         let display = handle.display as *mut xlib::_XDisplay;
 
+        errors::XErrorHandler::handle(display, |error_handler| {
+            #[allow(non_snake_case)]
+            let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe {
+                let addr = get_proc_address("glXCreateContextAttribsARB");
+                if addr.is_null() {
+                    return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed));
+                } else {
+                    std::mem::transmute(addr)
+                }
+            };
+
+            #[allow(non_snake_case)]
+            let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe {
+                let addr = get_proc_address("glXSwapIntervalEXT");
+                if addr.is_null() {
+                    return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed));
+                } else {
+                    std::mem::transmute(addr)
+                }
+            };
+
+            error_handler.check()?;
+
+            let profile_mask = match config.gl_config.profile {
+                Profile::Core => glx::arb::GLX_CONTEXT_CORE_PROFILE_BIT_ARB,
+                Profile::Compatibility => glx::arb::GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB,
+            };
+
+            #[rustfmt::skip]
+                let ctx_attribs = [
+                glx::arb::GLX_CONTEXT_MAJOR_VERSION_ARB, config.gl_config.version.0 as i32,
+                glx::arb::GLX_CONTEXT_MINOR_VERSION_ARB, config.gl_config.version.1 as i32,
+                glx::arb::GLX_CONTEXT_PROFILE_MASK_ARB, profile_mask,
+                0,
+            ];
+
+            let context = unsafe {
+                glXCreateContextAttribsARB(
+                    display,
+                    config.fb_config,
+                    std::ptr::null_mut(),
+                    1,
+                    ctx_attribs.as_ptr(),
+                )
+            };
+
+            error_handler.check()?;
+
+            if context.is_null() {
+                return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed));
+            }
+
+            unsafe {
+                let res = glx::glXMakeCurrent(display, handle.window, context);
+                error_handler.check()?;
+                if res == 0 {
+                    return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed));
+                }
+
+                glXSwapIntervalEXT(display, handle.window, config.gl_config.vsync as i32);
+                error_handler.check()?;
+
+                if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 {
+                    error_handler.check()?;
+                    return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed));
+                }
+            }
+
+            Ok(GlContext { window: handle.window, display, context })
+        })
+    }
+
+    /// Find a matching framebuffer config and window visual for the given OpenGL configuration.
+    /// 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 unsafe fn get_fb_config_and_visual(
+        display: *mut xlib::_XDisplay, config: GlConfig,
+    ) -> Result<(FbConfig, WindowConfig), GlError> {
         errors::XErrorHandler::handle(display, |error_handler| {
             let screen = unsafe { xlib::XDefaultScreen(display) };
 
@@ -99,79 +200,22 @@ impl GlContext {
             };
 
             error_handler.check()?;
-
+            if n_configs <= 0 || fb_config.is_null() {
-            if n_configs <= 0 {
                 return Err(GlError::CreationFailed(CreationFailedError::InvalidFBConfig));
             }
 
-            #[allow(non_snake_case)]
+            // Now that we have a matching framebuffer config, we need to know which visual matches
-            let glXCreateContextAttribsARB: GlXCreateContextAttribsARB = unsafe {
+            // thsi config so the window is compatible with the OpenGL context we're about to create
-                let addr = get_proc_address("glXCreateContextAttribsARB");
+            let fb_config = *fb_config;
-                if addr.is_null() {
+            let visual = glx::glXGetVisualFromFBConfig(display, fb_config);
-                    return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed));
+            if visual.is_null() {
-                } else {
+                return Err(GlError::CreationFailed(CreationFailedError::NoVisual));
-                    std::mem::transmute(addr)
-                }
-            };
-
-            #[allow(non_snake_case)]
-            let glXSwapIntervalEXT: GlXSwapIntervalEXT = unsafe {
-                let addr = get_proc_address("glXSwapIntervalEXT");
-                if addr.is_null() {
-                    return Err(GlError::CreationFailed(CreationFailedError::GetProcAddressFailed));
-                } else {
-                    std::mem::transmute(addr)
-                }
-            };
-
-            error_handler.check()?;
-
-            let profile_mask = match config.profile {
-                Profile::Core => glx::arb::GLX_CONTEXT_CORE_PROFILE_BIT_ARB,
-                Profile::Compatibility => glx::arb::GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB,
-            };
-
-            #[rustfmt::skip]
-                let ctx_attribs = [
-                glx::arb::GLX_CONTEXT_MAJOR_VERSION_ARB, config.version.0 as i32,
-                glx::arb::GLX_CONTEXT_MINOR_VERSION_ARB, config.version.1 as i32,
-                glx::arb::GLX_CONTEXT_PROFILE_MASK_ARB, profile_mask,
-                0,
-            ];
-
-            let context = unsafe {
-                glXCreateContextAttribsARB(
-                    display,
-                    *fb_config,
-                    std::ptr::null_mut(),
-                    1,
-                    ctx_attribs.as_ptr(),
-                )
-            };
-
-            error_handler.check()?;
-
-            if context.is_null() {
-                return Err(GlError::CreationFailed(CreationFailedError::ContextCreationFailed));
             }
 
-            unsafe {
+            Ok((
-                let res = glx::glXMakeCurrent(display, handle.window, context);
+                FbConfig { fb_config, gl_config: config },
-                error_handler.check()?;
+                WindowConfig { depth: (*visual).depth as u8, visual: (*visual).visualid as u32 },
-                if res == 0 {
+            ))
-                    return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed));
-                }
-
-                glXSwapIntervalEXT(display, handle.window, config.vsync as i32);
-                error_handler.check()?;
-
-                if glx::glXMakeCurrent(display, 0, std::ptr::null_mut()) == 0 {
-                    error_handler.check()?;
-                    return Err(GlError::CreationFailed(CreationFailedError::MakeCurrentFailed));
-                }
-            }
-
-            Ok(GlContext { window: handle.window, display, context })
         })
     }
 

src/x11/window.rs

@@ -7,6 +7,8 @@ use std::thread;
 use std::time::*;
 
 use raw_window_handle::{HasRawWindowHandle, RawWindowHandle, XlibHandle};
+use xcb::ffi::xcb_screen_t;
+use xcb::StructPtr;
 
 use super::XcbConnection;
 use crate::{
@@ -17,7 +19,7 @@ use crate::{
 use super::keyboard::{convert_key_press_event, convert_key_release_event};
 
 #[cfg(feature = "opengl")]
-use crate::gl::GlContext;
+use crate::gl::{platform, GlContext};
 
 pub struct WindowHandle {
     raw_window_handle: Option<RawWindowHandle>,
@@ -107,6 +109,12 @@ pub struct Window {
 // Hack to allow sending a RawWindowHandle between threads. Do not make public
 struct SendableRwh(RawWindowHandle);
 
+/// Quick wrapper to satisfy [HasRawWindowHandle], because of course a raw window handle wouldn't
+/// have a raw window handle, that would be silly.
+struct RawWindowHandleWrapper {
+    handle: RawWindowHandle,
+}
+
 unsafe impl Send for SendableRwh {}
 
 type WindowOpenResult = Result<SendableRwh, ()>;
@@ -211,23 +219,33 @@ impl Window {
 
         let window_info = WindowInfo::from_logical_size(options.size, scaling);
 
-        // The window need to have a depth of 32 so so we can create graphics contexts with alpha
+        // Now it starts becoming fun. If we're creating an OpenGL context, then we need to create
-        // buffers
+        // the window with a visual that matches the framebuffer used for the OpenGL context. So the
-        let mut depth = xcb::COPY_FROM_PARENT as u8;
+        // idea is that we first retrieve a framebuffer config that matches our wanted OpenGL
-        let mut visual = xcb::COPY_FROM_PARENT as u32;
+        // configuration, find the visual that matches that framebuffer config, create the window
-        'match_visual: for candidate_depth in screen.allowed_depths() {
+        // with that visual, and then finally create an OpenGL context for the window. If we don't
-            if candidate_depth.depth() != 32 {
+        // use OpenGL, then we'll just take a random visual with a 32-bit depth.
-                continue;
+        let create_default_config = || {
-            }
+            Self::find_visual_for_depth(&screen, 32)
-
+                .map(|visual| (32, visual))
-            for candidate_visual in candidate_depth.visuals() {
+                .unwrap_or((xcb::COPY_FROM_PARENT as u8, xcb::COPY_FROM_PARENT as u32))
-                if candidate_visual.class() == xcb::VISUAL_CLASS_TRUE_COLOR as u8 {
+        };
-                    depth = candidate_depth.depth();
+        #[cfg(feature = "opengl")]
-                    visual = candidate_visual.visual_id();
+        let (fb_config, (depth, visual)) = match options.gl_config {
-                    break 'match_visual;
+            Some(gl_config) => unsafe {
-                }
+                platform::GlContext::get_fb_config_and_visual(
-            }
+                    xcb_connection.conn.get_raw_dpy(),
-        }
+                    gl_config,
+                )
+                .map(|(fb_config, window_config)| {
+                    (Some(fb_config), (window_config.depth, window_config.visual))
+                })
+                .expect("Could not fetch framebuffer config")
+            },
+            None => (None, create_default_config()),
+        };
+        #[cfg(not(feature = "opengl"))]
+        let (depth, visual) = create_default_config();
 
         // For this 32-bith depth to work, you also need to define a color map and set a border
         // pixel: https://cgit.freedesktop.org/xorg/xserver/tree/dix/window.c#n818
@@ -300,6 +318,22 @@ impl Window {
 
         xcb_connection.conn.flush();
 
+        // TODO: These APIs could use a couple tweaks now that everything is internal and there is
+        //       no error handling anymore at this point. Everything is more or less unchanged
+        //       compared to when raw-gl-context was a separate crate.
+        #[cfg(feature = "opengl")]
+        let gl_context = fb_config.map(|fb_config| {
+            let mut handle = XlibHandle::empty();
+            handle.window = window_id as c_ulong;
+            handle.display = xcb_connection.conn.get_raw_dpy() as *mut c_void;
+            let handle = RawWindowHandleWrapper { handle: RawWindowHandle::Xlib(handle) };
+
+            // Because of the visual negotation we had to take some extra steps to create this context
+            let context = unsafe { platform::GlContext::create(&handle, fb_config) }
+                .expect("Could not create OpenGL context");
+            GlContext::new(context)
+        });
+
         let mut window = Self {
             xcb_connection,
             window_id,
@@ -314,7 +348,7 @@ impl Window {
             parent_handle,
 
             #[cfg(feature = "opengl")]
-            gl_context: todo!("Create the X11 OpenGL context"),
+            gl_context,
         };
 
         let mut handler = build(&mut crate::Window::new(&mut window));
@@ -360,6 +394,22 @@ impl Window {
         self.gl_context.as_ref()
     }
 
+    fn find_visual_for_depth(screen: &StructPtr<xcb_screen_t>, depth: u8) -> Option<u32> {
+        for candidate_depth in screen.allowed_depths() {
+            if candidate_depth.depth() != depth {
+                continue;
+            }
+
+            for candidate_visual in candidate_depth.visuals() {
+                if candidate_visual.class() == xcb::VISUAL_CLASS_TRUE_COLOR as u8 {
+                    return Some(candidate_visual.visual_id());
+                }
+            }
+        }
+
+        None
+    }
+
     #[inline]
     fn drain_xcb_events(&mut self, handler: &mut dyn WindowHandler) {
         // the X server has a tendency to send spurious/extraneous configure notify events when a
@@ -617,6 +667,12 @@ unsafe impl HasRawWindowHandle for Window {
     }
 }
 
+unsafe impl HasRawWindowHandle for RawWindowHandleWrapper {
+    fn raw_window_handle(&self) -> RawWindowHandle {
+        self.handle
+    }
+}
+
 fn mouse_id(id: u8) -> MouseButton {
     match id {
         1 => MouseButton::Left,