Skip to content

Commit

Permalink
Make WindowBuilder Send + Sync
Browse files Browse the repository at this point in the history
Window builder is always accessed by winit on the thread event loop
is on, thus it's safe to mark the data it gets as `Send + Sync`.
Each unsafe object is marked individually as `Send + Sync` instead
of just implementing `Send` and `Sync` for the whole builder.
  • Loading branch information
kchibisov committed Oct 12, 2023
1 parent 61581eb commit fa3aabe
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ And please only add new entries to the top of this list, right below the `# Unre
- Add `Window::show_window_menu` to request a titlebar/system menu; implemented on Windows for now.
- On iOS, send events `WindowEvent::Occluded(false)`, `WindowEvent::Occluded(true)` when application enters/leaves foreground.
- **Breaking** add `Event::MemoryWarning`; implemented on iOS/Android.
- Make `WindowBuilder` `Send + Sync`.

# 0.29.1-beta

Expand Down
15 changes: 15 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,18 @@ mod platform_impl;
pub mod window;

pub mod platform;

/// Wrapper for objects which winit will access on the main thread so they are effectively `Send`
/// and `Sync`, since they always excute on a single thread.
///
/// # Safety
///
/// Winit can run only one event loop at the time and the event loop itself is tied to some thread.
/// The objects could be send across the threads, but once passed to winit, they execute on the
/// mean thread if the platform demands it. Thus marking such objects as `Send + Sync` is safe.
#[doc(hidden)]
#[derive(Clone, Debug)]
pub struct SendSyncWrapper<T>(pub T);

unsafe impl<T> Send for SendSyncWrapper<T> {}
unsafe impl<T> Sync for SendSyncWrapper<T> {}
2 changes: 1 addition & 1 deletion src/platform_impl/linux/x11/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl UnownedWindow {
) -> Result<UnownedWindow, RootOsError> {
let xconn = &event_loop.xconn;
let atoms = xconn.atoms();
let root = match window_attrs.parent_window {
let root = match window_attrs.parent_window.0 {
Some(RawWindowHandle::Xlib(handle)) => handle.window as xproto::Window,
Some(RawWindowHandle::Xcb(handle)) => handle.window,
Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/macos/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ impl WinitWindow {
})
.ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;

match attrs.parent_window {
match attrs.parent_window.0 {
Some(RawWindowHandle::AppKit(handle)) => {
// SAFETY: Caller ensures the pointer is valid or NULL
let parent: Id<NSWindow> = match unsafe { Id::retain(handle.ns_window.cast()) } {
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/web/web_sys/canvas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ impl Canvas {
attr: &WindowAttributes,
platform_attr: PlatformSpecificWindowBuilderAttributes,
) -> Result<Self, RootOE> {
let canvas = match platform_attr.canvas {
let canvas = match platform_attr.canvas.0 {
Some(canvas) => canvas,
None => document
.create_element("canvas")
Expand Down
5 changes: 3 additions & 2 deletions src/platform_impl/web/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::window::{
CursorGrabMode, CursorIcon, ImePurpose, ResizeDirection, Theme, UserAttentionType,
WindowAttributes, WindowButtons, WindowId as RootWI, WindowLevel,
};
use crate::SendSyncWrapper;

use raw_window_handle::{RawDisplayHandle, RawWindowHandle, WebDisplayHandle, WebWindowHandle};
use web_sys::HtmlCanvasElement;
Expand Down Expand Up @@ -430,7 +431,7 @@ impl From<u64> for WindowId {

#[derive(Clone)]
pub struct PlatformSpecificWindowBuilderAttributes {
pub(crate) canvas: Option<backend::RawCanvasType>,
pub(crate) canvas: SendSyncWrapper<Option<backend::RawCanvasType>>,
pub(crate) prevent_default: bool,
pub(crate) focusable: bool,
pub(crate) append: bool,
Expand All @@ -439,7 +440,7 @@ pub struct PlatformSpecificWindowBuilderAttributes {
impl Default for PlatformSpecificWindowBuilderAttributes {
fn default() -> Self {
Self {
canvas: None,
canvas: SendSyncWrapper(None),
prevent_default: true,
focusable: true,
append: false,
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/windows/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ where
// so the diffing later can work.
window_flags.set(WindowFlags::CLOSABLE, true);

let parent = match attributes.parent_window {
let parent = match attributes.parent_window.0 {
Some(RawWindowHandle::Win32(handle)) => {
window_flags.set(WindowFlags::CHILD, true);
if pl_attribs.menu.is_some() {
Expand Down
16 changes: 6 additions & 10 deletions src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::{
error::{ExternalError, NotSupportedError, OsError},
event_loop::EventLoopWindowTarget,
monitor::{MonitorHandle, VideoMode},
platform_impl,
platform_impl, SendSyncWrapper,
};

pub use crate::icon::{BadIcon, Icon};
Expand Down Expand Up @@ -159,7 +159,7 @@ pub struct WindowAttributes {
pub resize_increments: Option<Size>,
pub content_protected: bool,
pub window_level: WindowLevel,
pub parent_window: Option<RawWindowHandle>,
pub parent_window: SendSyncWrapper<Option<RawWindowHandle>>,
pub active: bool,
}

Expand All @@ -185,7 +185,7 @@ impl Default for WindowAttributes {
preferred_theme: None,
resize_increments: None,
content_protected: false,
parent_window: None,
parent_window: SendSyncWrapper(None),
active: true,
}
}
Expand Down Expand Up @@ -478,7 +478,7 @@ impl WindowBuilder {
/// - **Android / iOS / Wayland / Web:** Unsupported.
#[inline]
pub unsafe fn with_parent_window(mut self, parent_window: Option<RawWindowHandle>) -> Self {
self.window.parent_window = parent_window;
self.window.parent_window = SendSyncWrapper(parent_window);
self
}

Expand Down Expand Up @@ -1522,10 +1522,8 @@ unsafe impl HasRawWindowHandle for Window {
/// [`Event::Resumed`]: crate::event::Event::Resumed
/// [`Event::Suspended`]: crate::event::Event::Suspended
fn raw_window_handle(&self) -> RawWindowHandle {
struct Wrapper(RawWindowHandle);
unsafe impl Send for Wrapper {}
self.window
.maybe_wait_on_main(|w| Wrapper(w.raw_window_handle()))
.maybe_wait_on_main(|w| SendSyncWrapper(w.raw_window_handle()))
.0
}
}
Expand All @@ -1536,10 +1534,8 @@ unsafe impl HasRawDisplayHandle for Window {
///
/// [`EventLoop`]: crate::event_loop::EventLoop
fn raw_display_handle(&self) -> RawDisplayHandle {
struct Wrapper(RawDisplayHandle);
unsafe impl Send for Wrapper {}
self.window
.maybe_wait_on_main(|w| Wrapper(w.raw_display_handle()))
.maybe_wait_on_main(|w| SendSyncWrapper(w.raw_display_handle()))
.0
}
}
Expand Down
5 changes: 5 additions & 0 deletions tests/send_objects.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ fn window_send() {
needs_send::<winit::window::Window>();
}

#[test]
fn window_builder_send() {
needs_send::<winit::window::WindowBuilder>();
}

#[test]
fn ids_send() {
// ensures that the various `..Id` types implement `Send`
Expand Down
5 changes: 5 additions & 0 deletions tests/sync_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ fn window_sync() {
// ensures that `winit::Window` implements `Sync`
needs_sync::<winit::window::Window>();
}

#[test]
fn window_builder_sync() {
needs_sync::<winit::window::WindowBuilder>();
}

0 comments on commit fa3aabe

Please sign in to comment.