Skip to content

Commit

Permalink
feat: Implement Send and Sync for softbuffer types
Browse files Browse the repository at this point in the history
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 creates a thread specifically for allocating and then
  deallocating device contexts. This lets us get around the thread
  affinity problem.

Closes #205

Signed-off-by: John Nunley <[email protected]>
Co-authored-by: Mads Marquart <[email protected]>
  • Loading branch information
notgull and madsmtm authored Jun 1, 2024
1 parent 88740e2 commit 56ae973
Show file tree
Hide file tree
Showing 12 changed files with 400 additions and 60 deletions.
4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }

Expand Down
13 changes: 7 additions & 6 deletions examples/utils/winit_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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)
{
Expand All @@ -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) {

Expand All @@ -97,7 +98,7 @@ mod winit_app {
let state = self.state.take();
debug_assert!(state.is_some());
drop(state);
}
}

fn window_event(
&mut self,
Expand Down
133 changes: 133 additions & 0 deletions examples/winit_multithread.rs
Original file line number Diff line number Diff line change
@@ -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();
}
8 changes: 4 additions & 4 deletions src/backend_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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")]
Expand Down
36 changes: 22 additions & 14 deletions src/backends/cg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand All @@ -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 {}
12 changes: 6 additions & 6 deletions src/backends/kms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand All @@ -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,
}))
Expand All @@ -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>,
Expand Down Expand Up @@ -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) {
Expand Down
10 changes: 7 additions & 3 deletions src/backends/orbital.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Loading

0 comments on commit 56ae973

Please sign in to comment.