chore: Clean up win32 and x11

src/error.rs

@@ -3,6 +3,7 @@ use raw_window_handle::{RawDisplayHandle, RawWindowHandle};
 use thiserror::Error;
 
 #[derive(Error, Debug)]
+#[non_exhaustive]
 pub enum SwBufError {
     #[error(
         "The provided window returned an unsupported platform: {human_readable_window_platform_name}, {human_readable_display_platform_name}."
@@ -13,6 +14,13 @@ pub enum SwBufError {
         window_handle: RawWindowHandle,
         display_handle: RawDisplayHandle
     },
+
+    #[error("The provided window handle is null.")]
+    IncompleteWindowHandle,
+
+    #[error("The provided display handle is null.")]
+    IncompleteDisplayHandle,
+
     #[error("Platform error")]
     PlatformError(Option<String>, Option<Box<dyn Error>>)
 }

src/win32.rs

@@ -1,20 +1,30 @@
+//! Implementation of software buffering for Windows.
+//! 
+//! This module converts the input buffer into a bitmap and then stretches it to the window.
+
 use crate::{GraphicsContextImpl, SwBufError};
 use raw_window_handle::Win32WindowHandle;
+
+use std::mem;
+use std::io;
 use std::os::raw::c_int;
 
 use windows_sys::Win32::Foundation::HWND;
 use windows_sys::Win32::Graphics::Gdi::{
-    StretchDIBits, BITMAPINFOHEADER, BI_BITFIELDS, RGBQUAD, HDC,
+    GetDC, StretchDIBits, ValidateRect, BITMAPINFOHEADER, BI_BITFIELDS, DIB_RGB_COLORS, HDC,
-    ValidateRect, GetDC, SRCCOPY, DIB_RGB_COLORS,
+    RGBQUAD, SRCCOPY,
 };
 
+/// The handle to a window for software buffering.
 pub struct Win32Impl {
+    /// The window handle.
     window: HWND,
+
+    /// The device context for the window.
     dc: HDC,
 }
 
-// Wrap this so we can have a proper number of bmiColors to write in
+/// The Win32-compatible bitmap information.
-// From minifb
 #[repr(C)]
 struct BitmapInfo {
     pub bmi_header: BITMAPINFOHEADER,
@@ -22,27 +32,43 @@ struct BitmapInfo {
 }
 
 impl Win32Impl {
+    /// Create a new `Win32Impl` from a `Win32WindowHandle`.
+    /// 
+    /// # Safety
+    /// 
+    /// The `Win32WindowHandle` must be a valid window handle.
     pub unsafe fn new(handle: &Win32WindowHandle) -> Result<Self, crate::SwBufError> {
-        let dc = GetDC(handle.hwnd as HWND);
+        // It is valid for the window handle to be null here. Error out if it is.
-
+        if handle.hwnd.is_null() {
-        if dc == 0 {
+            return Err(SwBufError::IncompleteWindowHandle);
-            return Err(SwBufError::PlatformError(Some("Device Context is null".into()), None));
         }
 
-        Ok(
+        // Get the handle to the device context.
-            Self {
+        // SAFETY: We have confirmed that the window handle is valid.
-                dc,
+        let hwnd = handle.hwnd as HWND;
-                window: handle.hwnd as HWND,
+        let dc = GetDC(hwnd);
-            }
+
-        )
+        // GetDC returns null if there is a platform error.
+        if dc == 0 {
+            return Err(SwBufError::PlatformError(
+                Some("Device Context is null".into()),
+                Some(Box::new(io::Error::last_os_error())),
+            ));
+        }
+
+        Ok(Self {
+            dc,
+            window: hwnd,
+        })
     }
 }
 
 impl GraphicsContextImpl for Win32Impl {
     unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) {
-        let mut bitmap_info: BitmapInfo = std::mem::zeroed();
+        // Create a new bitmap info struct.
+        let mut bitmap_info: BitmapInfo =mem::zeroed();
 
-        bitmap_info.bmi_header.biSize = std::mem::size_of::<BITMAPINFOHEADER>() as u32;
+        bitmap_info.bmi_header.biSize = mem::size_of::<BITMAPINFOHEADER>() as u32;
         bitmap_info.bmi_header.biPlanes = 1;
         bitmap_info.bmi_header.biBitCount = 32;
         bitmap_info.bmi_header.biCompression = BI_BITFIELDS;
@@ -52,6 +78,10 @@ impl GraphicsContextImpl for Win32Impl {
         bitmap_info.bmi_colors[1].rgbGreen = 0xff;
         bitmap_info.bmi_colors[2].rgbBlue = 0xff;
 
+        // Stretch the bitmap onto the window.
+        // SAFETY:
+        //  - The bitmap information is valid.
+        //  - The buffer is a valid pointer to image data.
         StretchDIBits(
             self.dc,
             0,
@@ -62,12 +92,13 @@ impl GraphicsContextImpl for Win32Impl {
             0,
             width as c_int,
             height as c_int,
-            std::mem::transmute(buffer.as_ptr()),
+            buffer.as_ptr().cast(),
-            std::mem::transmute(&bitmap_info),
+            &bitmap_info as *const BitmapInfo as *const _,
             DIB_RGB_COLORS,
             SRCCOPY,
         );
 
+        // Validate the window.
         ValidateRect(self.window, std::ptr::null_mut());
     }
 }

src/x11.rs

@@ -1,44 +1,93 @@
+//! Implementation of software buffering for X11.
+//! 
+//! This module converts the input buffer into an XImage and then sends it over the wire to be
+//! drawn. A more effective implementation would use shared memory instead of the wire. In
+//! addition, we may also want to blit to a pixmap instead of a window.
+
 use crate::{GraphicsContextImpl, SwBufError};
 use raw_window_handle::{XlibDisplayHandle, XlibWindowHandle};
 use std::os::raw::{c_char, c_uint};
 use x11_dl::xlib::{Display, Visual, Xlib, ZPixmap, GC};
 
+/// The handle to an X11 drawing context.
 pub struct X11Impl {
+    /// The window handle.
     window_handle: XlibWindowHandle,
+
+    /// The display handle.
     display_handle: XlibDisplayHandle,
+
+    /// Reference to the X11 shared library.
     lib: Xlib,
+
+    /// The graphics context for drawing.
     gc: GC,
+
+    /// Information about the screen to use for drawing.
     visual: *mut Visual,
+
+    /// The depth (bits per pixel) of the drawing context.
     depth: i32,
 }
 
 impl X11Impl {
-    pub unsafe fn new(window_handle: XlibWindowHandle, display_handle: XlibDisplayHandle) -> Result<Self, SwBufError> {
+    /// Create a new `X11Impl` from a `XlibWindowHandle` and `XlibDisplayHandle`.
+    /// 
+    /// # Safety
+    /// 
+    /// The `XlibWindowHandle` and `XlibDisplayHandle` must be valid.
+    pub unsafe fn new(
+        window_handle: XlibWindowHandle,
+        display_handle: XlibDisplayHandle,
+    ) -> Result<Self, SwBufError> {
+        // Try to open the X11 shared library.
         let lib = match Xlib::open() {
             Ok(lib) => lib,
-            Err(e) => return Err(SwBufError::PlatformError(Some("Failed to open Xlib".into()), Some(Box::new(e))))
+            Err(e) => {
+                return Err(SwBufError::PlatformError(
+                    Some("Failed to open Xlib".into()),
+                    Some(Box::new(e)),
+                ))
+            }
         };
-        let screen = (lib.XDefaultScreen)(display_handle.display as *mut Display);
+
+        // Validate the handles to ensure that they aren't incomplete.
+        if display_handle.display.is_null() {
+            return Err(SwBufError::IncompleteDisplayHandle);
+        }
+
+        if window_handle.window.is_null() {
+            return Err(SwBufError::IncompleteWindowHandle);
+        }
+
+        // Get the screen number from the handle.
+        // NOTE: By default, XlibDisplayHandle sets the screen number to 0. If we see a zero,
+        // it could mean either screen index zero, or that the screen number was not set. We
+        // can't tell which, so we'll just assume that the screen number was not set.
+        let screen = match display_handle.screen {
+            0 => (lib.XDefaultScreen)(display_handle.display as *mut Display),
+            screen => screen,
+        };
+
+        // Use the default graphics context, visual and depth for this screen.
         let gc = (lib.XDefaultGC)(display_handle.display as *mut Display, screen);
         let visual = (lib.XDefaultVisual)(display_handle.display as *mut Display, screen);
         let depth = (lib.XDefaultDepth)(display_handle.display as *mut Display, screen);
 
-        Ok(
+        Ok(Self {
-            Self {
+            window_handle,
-                window_handle,
+            display_handle,
-                display_handle,
+            lib,
-                lib,
+            gc,
-                gc,
+            visual,
-                visual,
+            depth,
-                depth,
+        })
-            }
-        )
     }
 }
 
 impl GraphicsContextImpl for X11Impl {
     unsafe fn set_buffer(&mut self, buffer: &[u32], width: u16, height: u16) {
-        //create image
+        // Create the image from the buffer.
         let image = (self.lib.XCreateImage)(
             self.display_handle.display as *mut Display,
             self.visual,
@@ -52,7 +101,7 @@ impl GraphicsContextImpl for X11Impl {
             (width * 4) as i32,
         );
 
-        //push image to window
+        // Draw the image to the window.
         (self.lib.XPutImage)(
             self.display_handle.display as *mut Display,
             self.window_handle.window,
@@ -66,6 +115,7 @@ impl GraphicsContextImpl for X11Impl {
             height as c_uint,
         );
 
+        // Delete the image data.
         (*image).data = std::ptr::null_mut();
         (self.lib.XDestroyImage)(image);
     }