Cleanup X11 impl

Author: prokopyl

src/platform/x11/drag_n_drop.rs

@@ -401,7 +401,7 @@ impl DragNDropState {
         };
 
         // Ignore if this was meant for another window (?)
-        if event.requestor != window.window_id.get() {
+        if event.requestor != window.xcb_window.id().get() {
             return Ok(());
         }
 
@@ -486,7 +486,7 @@ fn send_status_rejected(
         response_type: xproto::CLIENT_MESSAGE_EVENT,
         window: source_window,
         format: 32,
-        data: [window.window_id.get(), 0, 0, 0, conn.atoms.None as _].into(),
+        data: [window.xcb_window.id().get(), 0, 0, 0, conn.atoms.None as _].into(),
         sequence: 0,
         type_: conn.atoms.XdndStatus,
     };
@@ -508,7 +508,7 @@ fn send_status_event(
         response_type: xproto::CLIENT_MESSAGE_EVENT,
         window: source_window,
         format: 32,
-        data: [window.window_id.get(), 1, 0, 0, action as _].into(),
+        data: [window.xcb_window.id().get(), 1, 0, 0, action as _].into(),
         sequence: 0,
         type_: conn.atoms.XdndStatus,
     };
@@ -527,7 +527,7 @@ pub fn send_finished_rejected(
         response_type: xproto::CLIENT_MESSAGE_EVENT,
         window: source_window,
         format: 32,
-        data: [window.window_id.get(), 1, window.connection.atoms.None as _, 0, 0].into(),
+        data: [window.xcb_window.id().get(), 1, window.connection.atoms.None as _, 0, 0].into(),
         sequence: 0,
         type_: conn.atoms.XdndFinished as _,
     };
@@ -548,7 +548,7 @@ fn send_finished_event(
         response_type: xproto::CLIENT_MESSAGE_EVENT,
         window: source_window,
         format: 32,
-        data: [window.window_id.get(), 1, action as _, 0, 0].into(),
+        data: [window.xcb_window.id().get(), 1, action as _, 0, 0].into(),
         sequence: 0,
         type_: conn.atoms.XdndFinished as _,
     };
@@ -564,7 +564,7 @@ fn request_convert_selection(
     let conn = &window.connection;
 
     conn.conn.convert_selection(
-        window.window_id.get(),
+        window.xcb_window.id().get(),
         conn.atoms.XdndSelection,
         conn.atoms.TextUriList,
         conn.atoms.XdndSelection,
@@ -585,14 +585,14 @@ fn translate_root_coordinates(
     let x = x.try_into().unwrap_or(i16::MAX);
     let y = y.try_into().unwrap_or(i16::MAX);
 
-    if root_id == window.window_id.get() {
+    if root_id == window.xcb_window.id().get() {
         return Ok(PhysicalPosition::new(x, y));
     }
 
     let reply = window
         .connection
         .conn
-        .translate_coordinates(root_id, window.window_id.get(), x, y)?
+        .translate_coordinates(root_id, window.xcb_window.id().get(), x, y)?
         .reply()?;
 
     Ok(PhysicalPosition::new(reply.dst_x, reply.dst_y))
@@ -602,7 +602,7 @@ fn fetch_dnd_data(window: &WindowInner) -> Result<DropData, Box<dyn Error>> {
     let conn = &window.connection;
 
     let data: Vec<u8> = conn.get_property(
-        window.window_id.get(),
+        window.xcb_window.id().get(),
         conn.atoms.XdndSelection,
         conn.atoms.TextUriList,
     )?;

src/platform/x11/gl.rs

@@ -3,8 +3,8 @@ use crate::gl::*;
 use crate::wrappers::glx::*;
 use crate::wrappers::xlib::{XErrorHandler, XLibError};
 
-use std::ffi::{c_void, CString};
-use std::os::raw::c_ulong;
+use crate::platform::x11::xcb_window::XcbWindow;
+use std::ffi::{c_ulong, c_void, CString};
 use std::rc::Rc;
 use x11_dl::error::OpenError;
 use x11_dl::glx::GLXContext;
@@ -36,7 +36,7 @@ pub type GlContext = Rc<GlContextInner>;
 
 pub struct GlContextInner {
     glx: Glx,
-    window: c_ulong,
+    window: NonZeroU32,
     connection: Rc<X11Connection>,
     context: GLXContext,
 }
@@ -65,8 +65,8 @@ impl GlContextInner {
     ///
     /// Use [Self::get_fb_config_and_visual] to create both of these things.
     pub fn create(
-        window: c_ulong, connection: Rc<X11Connection>, config: FbConfig,
-    ) -> Result<GlContextInner, GlError> {
+        window: &XcbWindow, connection: Rc<X11Connection>, config: FbConfig,
+    ) -> Result<Rc<GlContextInner>, GlError> {
         let glx = Glx::open()?;
 
         let xlib_connection = connection.conn.xlib_connection();
@@ -87,28 +87,33 @@ impl GlContextInner {
                 error_handler,
             )?;
 
+            let window_id = window.id().get().into();
             // Create context object here so that error or panic will properly free the context
-            let context =
-                GlContextInner { glx, window, connection: Rc::clone(&connection), context };
+            let context = GlContextInner {
+                glx,
+                window: window.id(),
+                connection: Rc::clone(&connection),
+                context,
+            };
 
             unsafe {
                 context.glx.with_current_context(
                     xlib_connection,
-                    window,
+                    window_id,
                     context.context,
                     error_handler,
                     || {
                         swap_interval(
                             xlib_connection.as_raw(),
-                            window,
+                            window_id,
                             config.gl_config.vsync as i32,
                         );
                         error_handler.check()
                     },
                 )??;
             }
 
-            Ok(context)
+            Ok(Rc::new(context))
         })
     }
 
@@ -142,7 +147,7 @@ impl GlContextInner {
             self.glx
                 .make_current(
                     self.connection.conn.xlib_connection(),
-                    self.window,
+                    self.window_id(),
                     self.context,
                     error_handler,
                 )
@@ -156,6 +161,10 @@ impl GlContextInner {
         })
     }
 
+    fn window_id(&self) -> c_ulong {
+        self.window.get().into()
+    }
+
     pub fn get_proc_address(&self, symbol: &str) -> *const c_void {
         let symbol = CString::new(symbol).unwrap();
 
@@ -168,7 +177,11 @@ impl GlContextInner {
     pub fn swap_buffers(&self) {
         XErrorHandler::handle(self.connection.conn.xlib_connection(), |error_handler| {
             self.glx
-                .swap_buffers(self.connection.conn.xlib_connection(), self.window, error_handler)
+                .swap_buffers(
+                    self.connection.conn.xlib_connection(),
+                    self.window_id(),
+                    error_handler,
+                )
                 .unwrap()
         })
     }

src/platform/x11/mod.rs

@@ -2,7 +2,7 @@ mod xcb_connection;
 
 use raw_window_handle::{DisplayHandle, HasWindowHandle, RawWindowHandle, XcbWindowHandle};
 use std::fmt::Formatter;
-use std::num::NonZero;
+use std::num::{NonZero, NonZeroU32};
 use std::rc::Rc;
 use std::sync::Arc;
 pub(crate) use xcb_connection::X11Connection;
@@ -15,6 +15,7 @@ mod drag_n_drop;
 mod event_loop;
 mod keyboard;
 mod visual_info;
+mod xcb_window;
 
 mod window_shared;
 
@@ -59,14 +60,14 @@ impl std::fmt::Debug for PlatformHandle {
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct ParentWindowHandle {
-    window_id: u32,
+    window_id: NonZeroU32,
 }
 
 impl ParentWindowHandle {
     pub fn extract(window: &impl HasWindowHandle) -> Self {
         let window_id = match window.window_handle().unwrap().as_raw() {
-            RawWindowHandle::Xlib(h) => h.window as u32,
-            RawWindowHandle::Xcb(h) => h.window.get(),
+            RawWindowHandle::Xlib(h) => NonZeroU32::new(h.window.try_into().unwrap()).unwrap(),
+            RawWindowHandle::Xcb(h) => h.window,
             h => panic!("unsupported parent handle type {:?}", h),
         };
 

src/platform/x11/window.rs

@@ -8,17 +8,14 @@ use std::sync::Arc;
 use std::thread::{self, JoinHandle};
 
 use x11rb::connection::Connection;
-use x11rb::protocol::xproto::{
-    AtomEnum, ConnectionExt, CreateGCAux, CreateWindowAux, EventMask, PropMode, WindowClass,
-};
-use x11rb::wrapper::ConnectionExt as _;
 
 use super::X11Connection;
 use super::{event_loop::EventLoop, visual_info::WindowVisualConfig};
-use crate::context::WindowContext;
 use crate::handler::WindowHandlerBuilder;
 use crate::platform::x11::window_shared::WindowInner;
-use crate::{WindowOpenOptions, WindowScalePolicy, WindowSize};
+use crate::platform::x11::xcb_window::XcbWindow;
+use crate::wrappers::xkbcommon::XkbcommonState;
+use crate::*;
 
 pub struct WindowHandle {
     window_id: Option<NonZero<x11rb::protocol::xproto::Window>>,
@@ -105,22 +102,10 @@ impl Window {
         tx: mpsc::SyncSender<WindowOpenResult>, parent_handle: Option<ParentHandle>,
     ) -> Result<(), Box<dyn Error>> {
         // Connect to the X server
-        // FIXME: baseview error type instead of unwrap()
         let xcb_connection = X11Connection::new()?;
 
         // Setup xkbcommon
-        let xkb_state = crate::wrappers::xkbcommon::XkbcommonState::new(&xcb_connection);
-
-        // Get screen information
-        let screen = xcb_connection.screen();
-        let parent_id = options.parent.map(|p| p.window_id).unwrap_or(screen.root);
-
-        let gc_id = xcb_connection.conn.generate_id()?;
-        xcb_connection.conn.create_gc(
-            gc_id,
-            parent_id,
-            &CreateGCAux::new().foreground(screen.black_pixel).graphics_exposures(0),
-        )?;
+        let xkb_state = XkbcommonState::new(&xcb_connection);
 
         let scaling = match options.scale {
             WindowScalePolicy::SystemScaleFactor => xcb_connection.get_scaling(),
@@ -136,88 +121,33 @@ impl Window {
         #[cfg(not(feature = "opengl"))]
         let visual_info = WindowVisualConfig::find_best_visual_config(&xcb_connection)?;
 
-        let Some(window_id) = NonZero::new(xcb_connection.conn.generate_id()?) else {
-            unreachable!();
-        };
-
-        xcb_connection.conn.create_window(
-            visual_info.visual_depth,
-            window_id.get(),
-            parent_id,
-            0,                    // x coordinate of the new window
-            0,                    // y coordinate of the new window
-            physical_size.width,  // window width
-            physical_size.height, // window height
-            0,                    // window border
-            WindowClass::INPUT_OUTPUT,
-            visual_info.visual_id,
-            &CreateWindowAux::new()
-                .event_mask(
-                    EventMask::EXPOSURE
-                        | EventMask::POINTER_MOTION
-                        | EventMask::BUTTON_PRESS
-                        | EventMask::BUTTON_RELEASE
-                        | EventMask::KEY_PRESS
-                        | EventMask::KEY_RELEASE
-                        | EventMask::STRUCTURE_NOTIFY
-                        | EventMask::ENTER_WINDOW
-                        | EventMask::LEAVE_WINDOW
-                        | EventMask::FOCUS_CHANGE,
-                )
-                // As mentioned above, these two values are needed to be able to create a window
-                // with a depth of 32-bits when the parent window has a different depth
-                .colormap(visual_info.color_map)
-                .border_pixel(0),
-        )?;
-        xcb_connection.conn.map_window(window_id.get())?;
-
-        // Change window title
-        let title = options.title;
-        xcb_connection.conn.change_property8(
-            PropMode::REPLACE,
-            window_id.get(),
-            AtomEnum::WM_NAME,
-            AtomEnum::STRING,
-            title.as_bytes(),
-        )?;
+        let xcb_connection = Rc::new(xcb_connection);
 
-        xcb_connection.conn.change_property32(
-            PropMode::REPLACE,
-            window_id.get(),
-            xcb_connection.atoms.WM_PROTOCOLS,
-            AtomEnum::ATOM,
-            &[xcb_connection.atoms.WM_DELETE_WINDOW],
+        let x_window = XcbWindow::new(
+            Rc::clone(&xcb_connection),
+            physical_size,
+            &visual_info,
+            options.parent.map(|p| p.window_id),
         )?;
 
-        // Enable drag and drop (TODO: Make this toggleable?)
-        xcb_connection.conn.change_property32(
-            PropMode::REPLACE,
-            window_id.get(),
-            xcb_connection.atoms.XdndAware,
-            AtomEnum::ATOM,
-            &[5u32], // Latest version; hasn't changed since 2002
-        )?;
+        x_window.map_window()?;
+        x_window.set_title(&options.title)?;
+        x_window.enable_wm_protocols()?;
+        x_window.enable_dnd_protocols()?;
 
         xcb_connection.conn.flush()?;
-        let xcb_connection = Rc::new(xcb_connection);
 
         #[cfg(feature = "opengl")]
         let gl_context = visual_info.fb_config.map(|fb_config| {
-            use std::ffi::c_ulong;
-
-            let window = window_id.get() as c_ulong;
-
             // Because of the visual negotation we had to take some extra steps to create this context
-            let context =
-                super::gl::GlContextInner::create(window, Rc::clone(&xcb_connection), fb_config)
-                    .expect("Could not create OpenGL context");
-
-            Rc::new(context)
+            super::gl::GlContextInner::create(&x_window, Rc::clone(&xcb_connection), fb_config)
+                .expect("Could not create OpenGL context")
         });
 
+        let window_id = x_window.id();
         let inner = Rc::new(WindowInner::new(
             xcb_connection,
-            window_id,
+            x_window,
             physical_size,
             scaling,
             visual_info.visual_id.try_into()?,
@@ -227,10 +157,6 @@ impl Window {
 
         let handler = build.build(WindowContext::new(Rc::clone(&inner)));
 
-        // Send an initial window resized event so the user is alerted of
-        // the correct dpi scaling.
-        handler.resized(WindowSize::from_physical(physical_size.cast(), scaling));
-
         let _ = tx.send(Ok(window_id));
 
         EventLoop::new(inner, handler, parent_handle, xkb_state).run()?;