Implement Send and Sync for softbuffer types

Cargo.toml

@@ -43,14 +43,14 @@ 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"] }
 core-graphics = "0.23.1"
 foreign-types = "0.5.0"
 objc2 = "0.5.1"
-objc2-foundation = { version = "0.2.0", features = ["NSThread"] }
+objc2-foundation = { version = "0.2.0", features = ["dispatch", "NSThread"] }
 objc2-app-kit = { version = "0.2.0", features = ["NSResponder", "NSView", "NSWindow"] }
 objc2-quartz-core = { version = "0.2.0", features = ["CALayer", "CATransaction"] }
 

examples/utils/winit_app.rs

@@ -13,12 +13,13 @@ mod winit_app {
     pub(crate) fn run_app(event_loop: EventLoop<()>, mut app: impl ApplicationHandler<()> + 'static) {
         #[cfg(not(any(target_arch = "wasm32", target_arch = "wasm64")))]
         event_loop.run_app(&mut app).unwrap();
-        
+
         #[cfg(any(target_arch = "wasm32", target_arch = "wasm64"))]
         winit::platform::web::EventLoopExtWebSys::spawn_app(event_loop, app);
     }
 
     /// Create a window from a set of window attributes.
+    #[allow(dead_code)]
     pub(crate) fn make_window(elwt: &ActiveEventLoop, f: impl FnOnce(WindowAttributes) -> WindowAttributes) -> Rc<Window> {
         let attributes = f(WindowAttributes::default());
         #[cfg(target_arch = "wasm32")]
@@ -29,7 +30,7 @@ mod winit_app {
         let window = elwt.create_window(attributes);
         Rc::new(window.unwrap())
     }
-    
+
     /// Easily constructable winit application.
     pub(crate) struct WinitApp<T, Init, Handler> {
         /// Closure to initialize state.
@@ -51,7 +52,7 @@ mod winit_app {
         _marker: PhantomData<Option<T>>,
     }
 
-    impl<T, Init> WinitAppBuilder<T, Init> 
+    impl<T, Init> WinitAppBuilder<T, Init>
     where Init: FnMut(&ActiveEventLoop) -> T,
     {
         /// Create with an "init" closure.
@@ -70,7 +71,7 @@ mod winit_app {
         }
     }
 
-    impl<T, Init, Handler> WinitApp<T, Init, Handler> 
+    impl<T, Init, Handler> WinitApp<T, Init, Handler>
     where Init: FnMut(&ActiveEventLoop) -> T,
           Handler: FnMut(&mut T, Event<()>, &ActiveEventLoop)
     {
@@ -84,7 +85,7 @@ mod winit_app {
         }
     }
 
-    impl<T, Init, Handler> ApplicationHandler for WinitApp<T, Init, Handler> 
+    impl<T, Init, Handler> ApplicationHandler for WinitApp<T, Init, Handler>
     where Init: FnMut(&ActiveEventLoop) -> T,
           Handler: FnMut(&mut T, Event<()>, &ActiveEventLoop) {
 
@@ -97,7 +98,7 @@ mod winit_app {
             let state = self.state.take();
             debug_assert!(state.is_some());
             drop(state);
-        }        
+        }
 
         fn window_event(
             &mut self,

examples/winit_multithread.rs

@@ -0,0 +1,133 @@
+//! `Surface` implements `Send`. This makes sure that multithreading can work here.
+
+#[cfg(not(target_family = "wasm"))]
+mod ex {
+    use std::num::NonZeroU32;
+    use std::sync::{mpsc, Arc, Mutex};
+    use winit::event::{Event, KeyEvent, WindowEvent};
+    use winit::event_loop::{ControlFlow, EventLoop};
+    use winit::keyboard::{Key, NamedKey};
+    use winit::window::Window;
+
+    include!("utils/winit_app.rs");
+
+    type Surface = softbuffer::Surface<Arc<Window>, Arc<Window>>;
+
+    fn render_thread(
+        window: Arc<Window>,
+        surface: Arc<Mutex<Surface>>,
+        do_render: mpsc::Receiver<()>,
+        done: mpsc::Sender<()>,
+    ) {
+        loop {
+            println!("waiting for render...");
+            if do_render.recv().is_err() {
+                // Main thread is dead.
+                break;
+            }
+
+            // Perform the rendering.
+            let mut surface = surface.lock().unwrap();
+            if let (Some(width), Some(height)) = {
+                let size = window.inner_size();
+                println!("got size: {size:?}");
+                (NonZeroU32::new(size.width), NonZeroU32::new(size.height))
+            } {
+                println!("resizing...");
+                surface.resize(width, height).unwrap();
+
+                let mut buffer = surface.buffer_mut().unwrap();
+                for y in 0..height.get() {
+                    for x in 0..width.get() {
+                        let red = x % 255;
+                        let green = y % 255;
+                        let blue = (x * y) % 255;
+                        let index = y as usize * width.get() as usize + x as usize;
+                        buffer[index] = blue | (green << 8) | (red << 16);
+                    }
+                }
+
+                println!("presenting...");
+                buffer.present().unwrap();
+            }
+
+            // We're done, tell the main thread to keep going.
+            done.send(()).ok();
+        }
+    }
+
+    pub(super) fn entry() {
+        let event_loop = EventLoop::new().unwrap();
+
+        let app = winit_app::WinitAppBuilder::with_init(|elwt| {
+            let attributes = Window::default_attributes();
+            #[cfg(target_arch = "wasm32")]
+            let attributes =
+                winit::platform::web::WindowAttributesExtWebSys::with_append(attributes, true);
+            let window = Arc::new(elwt.create_window(attributes).unwrap());
+
+            let context = softbuffer::Context::new(window.clone()).unwrap();
+            let surface = {
+                println!("making surface...");
+                let surface = softbuffer::Surface::new(&context, window.clone()).unwrap();
+                Arc::new(Mutex::new(surface))
+            };
+
+            // Spawn a thread to handle rendering.
+            let (start_render, do_render) = mpsc::channel();
+            let (render_done, finish_render) = mpsc::channel();
+            println!("starting thread...");
+            std::thread::spawn({
+                let window = window.clone();
+                let surface = surface.clone();
+                move || render_thread(window, surface, do_render, render_done)
+            });
+
+            (window, surface, start_render, finish_render)
+        })
+        .with_event_handler(|state, event, elwt| {
+            let (window, _surface, start_render, finish_render) = state;
+            elwt.set_control_flow(ControlFlow::Wait);
+
+            match event {
+                Event::WindowEvent {
+                    window_id,
+                    event: WindowEvent::RedrawRequested,
+                } if window_id == window.id() => {
+                    // Start the render and then finish it.
+                    start_render.send(()).unwrap();
+                    finish_render.recv().unwrap();
+                }
+                Event::WindowEvent {
+                    event:
+                        WindowEvent::CloseRequested
+                        | WindowEvent::KeyboardInput {
+                            event:
+                                KeyEvent {
+                                    logical_key: Key::Named(NamedKey::Escape),
+                                    ..
+                                },
+                            ..
+                        },
+                    window_id,
+                } if window_id == window.id() => {
+                    elwt.exit();
+                }
+                _ => {}
+            }
+        });
+
+        winit_app::run_app(event_loop, app);
+    }
+}
+
+#[cfg(target_family = "wasm")]
+mod ex {
+    pub(crate) fn entry() {
+        eprintln!("winit_multithreaded doesn't work on WASM");
+    }
+}
+
+fn main() {
+    ex::entry();
+}

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")]

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,8 @@ impl<'a, D: HasDisplayHandle, W: HasWindowHandle> BufferInterface for BufferImpl
         self.present()
     }
 }
+
+struct SendCGColorSpace(CGColorSpace);
+// SAFETY: `CGColorSpace` is immutable, and can freely be shared between threads.
+unsafe impl Send for SendCGColorSpace {}
+unsafe impl Sync for SendCGColorSpace {}

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) {

src/backends/orbital.rs

@@ -56,17 +56,21 @@ impl Drop for OrbitalMap {
 }
 
 pub struct OrbitalImpl<D, W> {
-    handle: OrbitalWindowHandle,
+    handle: ThreadSafeWindowHandle,
     width: u32,
     height: u32,
     presented: bool,
     window_handle: W,
     _display: PhantomData<D>,
 }
 
+struct ThreadSafeWindowHandle(OrbitalWindowHandle);
+unsafe impl Send for ThreadSafeWindowHandle {}
+unsafe impl Sync for ThreadSafeWindowHandle {}
+
 impl<D: HasDisplayHandle, W: HasWindowHandle> OrbitalImpl<D, W> {
     fn window_fd(&self) -> usize {
-        self.handle.window.as_ptr() as usize
+        self.handle.0.window.as_ptr() as usize
     }
 
     // Read the current width and size
@@ -134,7 +138,7 @@ impl<D: HasDisplayHandle, W: HasWindowHandle> SurfaceInterface<D, W> for Orbital
         };
 
         Ok(Self {
-            handle,
+            handle: ThreadSafeWindowHandle(handle),
             width: 0,
             height: 0,
             presented: false,