feat: Implement Send for softbuffer types

Previously, types in softbuffer were !Send and !Sync. However it would be nice if types could be shared across threads. Therefore I've made the following changes: - Context<D> is Send+Sync iff D is Send+Sync - Surface<D, W> is Send iff D is Send+Sync and W is Send - Buffer<'x, D, W> is Send iff D if Send+Sync and W is Send Materially, I've made the following changes across the backends: - X11, Wayland and KMS use Arc for their displays instead of Rc. - MacOS uses MainThreadBound to secure windowing resources. This restriction was already implicitly enforced anyhow. - Windows uses a subclassing strategy to manage creating and destroying resources. This subclass is applied to windows that are registered into softbuffer, and is then used to manage resources. Closes #205 Signed-off-by: John Nunley <[email protected]>
rust-windowing · May 8, 2024 · 87572ba · 87572ba
1 parent 0e6706c
commit 87572ba
Show file tree

Hide file tree

Showing 9 changed files with 245 additions and 57 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -43,7 +43,7 @@ x11rb = { version = "0.13.0", features = ["allow-unsafe-code", "shm"], optional
 
 [target.'cfg(target_os = "windows")'.dependencies.windows-sys]
 version = "0.52.0"
-features = ["Win32_Graphics_Gdi", "Win32_UI_WindowsAndMessaging", "Win32_Foundation"]
+features = ["Win32_Graphics_Gdi", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging", "Win32_Foundation"]
 
 [target.'cfg(target_os = "macos")'.dependencies]
 bytemuck = { version = "1.12.3", features = ["extern_crate_alloc"] }

diff --git a/src/backend_dispatch.rs b/src/backend_dispatch.rs
@@ -5,7 +5,7 @@ use crate::{backend_interface::*, backends, InitError, Rect, SoftBufferError};
 use raw_window_handle::{HasDisplayHandle, HasWindowHandle};
 use std::num::NonZeroU32;
 #[cfg(any(wayland_platform, x11_platform, kms_platform))]
-use std::rc::Rc;
+use std::sync::Arc;
 
 /// A macro for creating the enum used to statically dispatch to the platform-specific implementation.
 macro_rules! make_dispatch {
@@ -179,11 +179,11 @@ macro_rules! make_dispatch {
 make_dispatch! {
     <D, W> =>
     #[cfg(x11_platform)]
-    X11(Rc<backends::x11::X11DisplayImpl<D>>, backends::x11::X11Impl<D, W>, backends::x11::BufferImpl<'a, D, W>),
+    X11(Arc<backends::x11::X11DisplayImpl<D>>, backends::x11::X11Impl<D, W>, backends::x11::BufferImpl<'a, D, W>),
     #[cfg(wayland_platform)]
-    Wayland(Rc<backends::wayland::WaylandDisplayImpl<D>>, backends::wayland::WaylandImpl<D, W>, backends::wayland::BufferImpl<'a, D, W>),
+    Wayland(Arc<backends::wayland::WaylandDisplayImpl<D>>, backends::wayland::WaylandImpl<D, W>, backends::wayland::BufferImpl<'a, D, W>),
     #[cfg(kms_platform)]
-    Kms(Rc<backends::kms::KmsDisplayImpl<D>>, backends::kms::KmsImpl<D, W>, backends::kms::BufferImpl<'a, D, W>),
+    Kms(Arc<backends::kms::KmsDisplayImpl<D>>, backends::kms::KmsImpl<D, W>, backends::kms::BufferImpl<'a, D, W>),
     #[cfg(target_os = "windows")]
     Win32(D, backends::win32::Win32Impl<D, W>, backends::win32::BufferImpl<'a, D, W>),
     #[cfg(target_os = "macos")]

diff --git a/src/backends/cg.rs b/src/backends/cg.rs
@@ -14,7 +14,7 @@ use foreign_types::ForeignType;
 use objc2::msg_send;
 use objc2::rc::Id;
 use objc2_app_kit::{NSAutoresizingMaskOptions, NSView, NSWindow};
-use objc2_foundation::MainThreadMarker;
+use objc2_foundation::{MainThreadBound, MainThreadMarker};
 use objc2_quartz_core::{kCAGravityTopLeft, CALayer, CATransaction};
 
 use std::marker::PhantomData;
@@ -30,9 +30,9 @@ impl AsRef<[u8]> for Buffer {
 }
 
 pub struct CGImpl<D, W> {
-    layer: Id<CALayer>,
-    window: Id<NSWindow>,
-    color_space: CGColorSpace,
+    layer: MainThreadBound<Id<CALayer>>,
+    window: MainThreadBound<Id<NSWindow>>,
+    color_space: SendCGColorSpace,
     size: Option<(NonZeroU32, NonZeroU32)>,
     window_handle: W,
     _display: PhantomData<D>,
@@ -84,9 +84,9 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> for CGImpl<
         unsafe { view.addSubview(&subview) };
         let color_space = CGColorSpace::create_device_rgb();
         Ok(Self {
-            layer,
-            window,
-            color_space,
+            layer: MainThreadBound::new(layer, mtm),
+            window: MainThreadBound::new(window, mtm),
+            color_space: SendCGColorSpace(color_space),
             size: None,
             _display: PhantomData,
             window_handle: window_src,
@@ -144,27 +144,30 @@ impl<'a, D: HasDisplayHandle, W: HasWindowHandle> BufferInterface for BufferImpl
             8,
             32,
             (width.get() * 4) as usize,
-            &self.imp.color_space,
+            &self.imp.color_space.0,
             kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
             &data_provider,
             false,
             kCGRenderingIntentDefault,
         );
 
+        // TODO: Use run_on_main() instead.
+        let mtm = MainThreadMarker::new().ok_or(SoftBufferError::PlatformError(
+            Some("can only access AppKit / macOS handles from the main thread".to_string()),
+            None,
+        ))?;
+
         // The CALayer has a default action associated with a change in the layer contents, causing
         // a quarter second fade transition to happen every time a new buffer is applied. This can
         // be mitigated by wrapping the operation in a transaction and disabling all actions.
         CATransaction::begin();
         CATransaction::setDisableActions(true);
 
-        self.imp
-            .layer
-            .setContentsScale(self.imp.window.backingScaleFactor());
+        let layer = self.imp.layer.get(mtm);
+        layer.setContentsScale(self.imp.window.get(mtm).backingScaleFactor());
 
         unsafe {
-            self.imp
-                .layer
-                .setContents((image.as_ptr() as *mut AnyObject).as_ref());
+            layer.setContents((image.as_ptr() as *mut AnyObject).as_ref());
         };
 
         CATransaction::commit();
@@ -176,3 +179,7 @@ impl<'a, D: HasDisplayHandle, W: HasWindowHandle> BufferInterface for BufferImpl
         self.present()
     }
 }
+
+struct SendCGColorSpace(CGColorSpace);
+// SAFETY: I'm 99% sure this is thread safe.
+unsafe impl Send for SendCGColorSpace {}
diff --git a/src/backends/kms.rs b/src/backends/kms.rs
@@ -15,7 +15,7 @@ use std::collections::HashSet;
 use std::marker::PhantomData;
 use std::num::NonZeroU32;
 use std::os::unix::io::{AsFd, BorrowedFd};
-use std::rc::Rc;
+use std::sync::Arc;
 
 use crate::backend_interface::*;
 use crate::error::{InitError, SoftBufferError, SwResultExt};
@@ -38,7 +38,7 @@ impl<D: ?Sized> AsFd for KmsDisplayImpl<D> {
 impl<D: ?Sized> Device for KmsDisplayImpl<D> {}
 impl<D: ?Sized> CtrlDevice for KmsDisplayImpl<D> {}
 
-impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Rc<KmsDisplayImpl<D>> {
+impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Arc<KmsDisplayImpl<D>> {
     fn new(display: D) -> Result<Self, InitError<D>>
     where
         D: Sized,
@@ -54,7 +54,7 @@ impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Rc<KmsDisplayImpl<D>>
         // SAFETY: Invariants guaranteed by the user.
         let fd = unsafe { BorrowedFd::borrow_raw(fd) };
 
-        Ok(Rc::new(KmsDisplayImpl {
+        Ok(Arc::new(KmsDisplayImpl {
             fd,
             _display: display,
         }))
@@ -65,7 +65,7 @@ impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Rc<KmsDisplayImpl<D>>
 #[derive(Debug)]
 pub(crate) struct KmsImpl<D: ?Sized, W: ?Sized> {
     /// The display implementation.
-    display: Rc<KmsDisplayImpl<D>>,
+    display: Arc<KmsDisplayImpl<D>>,
 
     /// The connectors to use.
     connectors: Vec<connector::Handle>,
@@ -133,11 +133,11 @@ struct SharedBuffer {
 }
 
 impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> SurfaceInterface<D, W> for KmsImpl<D, W> {
-    type Context = Rc<KmsDisplayImpl<D>>;
+    type Context = Arc<KmsDisplayImpl<D>>;
     type Buffer<'a> = BufferImpl<'a, D, W> where Self: 'a;
 
     /// Create a new KMS backend.
-    fn new(window: W, display: &Rc<KmsDisplayImpl<D>>) -> Result<Self, InitError<W>> {
+    fn new(window: W, display: &Arc<KmsDisplayImpl<D>>) -> Result<Self, InitError<W>> {
         // Make sure that the window handle is valid.
         let plane_handle = match window.window_handle()?.as_raw() {
             RawWindowHandle::Drm(drm) => match NonZeroU32::new(drm.plane) {

diff --git a/src/backends/wayland/mod.rs b/src/backends/wayland/mod.rs
@@ -5,9 +5,8 @@ use crate::{
 };
 use raw_window_handle::{HasDisplayHandle, HasWindowHandle, RawDisplayHandle, RawWindowHandle};
 use std::{
-    cell::RefCell,
     num::{NonZeroI32, NonZeroU32},
-    rc::Rc,
+    sync::{Arc, Mutex},
 };
 use wayland_client::{
     backend::{Backend, ObjectId},
@@ -23,7 +22,7 @@ struct State;
 
 pub struct WaylandDisplayImpl<D: ?Sized> {
     conn: Option<Connection>,
-    event_queue: RefCell<EventQueue<State>>,
+    event_queue: Mutex<EventQueue<State>>,
     qh: QueueHandle<State>,
     shm: wl_shm::WlShm,
 
@@ -40,7 +39,7 @@ impl<D: HasDisplayHandle + ?Sized> WaylandDisplayImpl<D> {
     }
 }
 
-impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Rc<WaylandDisplayImpl<D>> {
+impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Arc<WaylandDisplayImpl<D>> {
     fn new(display: D) -> Result<Self, InitError<D>>
     where
         D: Sized,
@@ -59,9 +58,9 @@ impl<D: HasDisplayHandle + ?Sized> ContextInterface<D> for Rc<WaylandDisplayImpl
         let shm: wl_shm::WlShm = globals
             .bind(&qh, 1..=1, ())
             .swbuf_err("Failed to instantiate Wayland Shm")?;
-        Ok(Rc::new(WaylandDisplayImpl {
+        Ok(Arc::new(WaylandDisplayImpl {
             conn: Some(conn),
-            event_queue: RefCell::new(event_queue),
+            event_queue: Mutex::new(event_queue),
             qh,
             shm,
             _display: display,
@@ -77,7 +76,7 @@ impl<D: ?Sized> Drop for WaylandDisplayImpl<D> {
 }
 
 pub struct WaylandImpl<D: ?Sized, W: ?Sized> {
-    display: Rc<WaylandDisplayImpl<D>>,
+    display: Arc<WaylandDisplayImpl<D>>,
     surface: Option<wl_surface::WlSurface>,
     buffers: Option<(WaylandBuffer, WaylandBuffer)>,
     size: Option<(NonZeroI32, NonZeroI32)>,
@@ -98,7 +97,8 @@ impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> WaylandImpl<D, W> {
         let _ = self
             .display
             .event_queue
-            .borrow_mut()
+            .lock()
+            .unwrap_or_else(|x| x.into_inner())
             .dispatch_pending(&mut State);
 
         if let Some((front, back)) = &mut self.buffers {
@@ -136,7 +136,12 @@ impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> WaylandImpl<D, W> {
             self.surface().commit();
         }
 
-        let _ = self.display.event_queue.borrow_mut().flush();
+        let _ = self
+            .display
+            .event_queue
+            .lock()
+            .unwrap_or_else(|x| x.into_inner())
+            .flush();
 
         Ok(())
     }
@@ -145,10 +150,10 @@ impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> WaylandImpl<D, W> {
 impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> SurfaceInterface<D, W>
     for WaylandImpl<D, W>
 {
-    type Context = Rc<WaylandDisplayImpl<D>>;
+    type Context = Arc<WaylandDisplayImpl<D>>;
     type Buffer<'a> = BufferImpl<'a, D, W> where Self: 'a;
 
-    fn new(window: W, display: &Rc<WaylandDisplayImpl<D>>) -> Result<Self, InitError<W>> {
+    fn new(window: W, display: &Arc<WaylandDisplayImpl<D>>) -> Result<Self, InitError<W>> {
         // Get the raw Wayland window.
         let raw = window.window_handle()?.as_raw();
         let wayland_handle = match raw {
@@ -199,7 +204,11 @@ impl<D: HasDisplayHandle + ?Sized, W: HasWindowHandle> SurfaceInterface<D, W>
         if let Some((_front, back)) = &mut self.buffers {
             // Block if back buffer not released yet
             if !back.released() {
-                let mut event_queue = self.display.event_queue.borrow_mut();
+                let mut event_queue = self
+                    .display
+                    .event_queue
+                    .lock()
+                    .unwrap_or_else(|x| x.into_inner());
                 while !back.released() {
                     event_queue.blocking_dispatch(&mut State).map_err(|err| {
                         SoftBufferError::PlatformError(