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 17, 2023
1 parent 0877385 commit c0c7f91
Show file tree
Hide file tree
Showing 14 changed files with 72 additions and 47 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ And please only add new entries to the top of this list, right below the `# Unre
- On Wayland, support `Occluded` event with xdg-shell v6
- Implement `AsFd`/`AsRawFd` for `EventLoop<T>` on X11 and Wayland.
- **Breaking:** Bump `ndk` version to `0.8.0`, ndk-sys to `0.5.0`, `android-activity` to `0.5.0`.
- 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 @@ -163,3 +163,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(crate) struct SendSyncWrapper<T>(pub(crate) T);

unsafe impl<T> Send for SendSyncWrapper<T> {}
unsafe impl<T> Sync for SendSyncWrapper<T> {}
7 changes: 2 additions & 5 deletions src/platform/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::event::Event;
use crate::event_loop::EventLoop;
use crate::event_loop::EventLoopWindowTarget;
use crate::window::{Window, WindowBuilder};
use crate::SendSyncWrapper;

use web_sys::HtmlCanvasElement;

Expand Down Expand Up @@ -81,26 +82,22 @@ pub trait WindowBuilderExtWebSys {

impl WindowBuilderExtWebSys for WindowBuilder {
fn with_canvas(mut self, canvas: Option<HtmlCanvasElement>) -> Self {
self.platform_specific.canvas = canvas;

self.platform_specific.canvas = SendSyncWrapper(canvas);
self
}

fn with_prevent_default(mut self, prevent_default: bool) -> Self {
self.platform_specific.prevent_default = prevent_default;

self
}

fn with_focusable(mut self, focusable: bool) -> Self {
self.platform_specific.focusable = focusable;

self
}

fn with_append(mut self, append: bool) -> Self {
self.platform_specific.append = append;

self
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/ios/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ impl WinitUIWindow {

this.setRootViewController(Some(view_controller));

match window_attributes.fullscreen.clone().map(Into::into) {
match window_attributes.fullscreen.0.clone().map(Into::into) {
Some(Fullscreen::Exclusive(ref video_mode)) => {
let monitor = video_mode.monitor();
let screen = monitor.ui_screen(mtm);
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/ios/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ impl Window {
// TODO: transparency, visible

let main_screen = UIScreen::main(mtm);
let fullscreen = window_attributes.fullscreen.clone().map(Into::into);
let fullscreen = window_attributes.fullscreen.0.clone().map(Into::into);
let screen = match fullscreen {
Some(Fullscreen::Exclusive(ref video_mode)) => video_mode.monitor.ui_screen(mtm),
Some(Fullscreen::Borderless(Some(ref monitor))) => monitor.ui_screen(mtm),
Expand Down
2 changes: 1 addition & 1 deletion src/platform_impl/linux/wayland/window/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Window {
window_state.set_resizable(attributes.resizable);

// Set startup mode.
match attributes.fullscreen.map(Into::into) {
match attributes.fullscreen.0.map(Into::into) {
Some(Fullscreen::Exclusive(_)) => {
warn!("`Fullscreen::Exclusive` is ignored on Wayland");
}
Expand Down
6 changes: 3 additions & 3 deletions src/platform_impl/linux/x11/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl UnownedWindow {
let xconn = &event_loop.xconn;
let atoms = xconn.atoms();
#[cfg(feature = "rwh_06")]
let root = match window_attrs.parent_window {
let root = match window_attrs.parent_window.0 {
Some(rwh_06::RawWindowHandle::Xlib(handle)) => handle.window as xproto::Window,
Some(rwh_06::RawWindowHandle::Xcb(handle)) => handle.window.get(),
Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
Expand Down Expand Up @@ -547,10 +547,10 @@ impl UnownedWindow {
if window_attrs.maximized {
leap!(window.set_maximized_inner(window_attrs.maximized)).ignore_error();
}
if window_attrs.fullscreen.is_some() {
if window_attrs.fullscreen.0.is_some() {
if let Some(flusher) =
leap!(window
.set_fullscreen_inner(window_attrs.fullscreen.clone().map(Into::into)))
.set_fullscreen_inner(window_attrs.fullscreen.0.clone().map(Into::into)))
{
flusher.ignore_error()
}
Expand Down
8 changes: 4 additions & 4 deletions src/platform_impl/macos/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ impl WinitWindow {
trace_scope!("WinitWindow::new");

let this = autoreleasepool(|_| {
let screen = match attrs.fullscreen.clone().map(Into::into) {
let screen = match attrs.fullscreen.0.clone().map(Into::into) {
Some(Fullscreen::Borderless(Some(monitor)))
| Some(Fullscreen::Exclusive(VideoMode { monitor, .. })) => {
monitor.ns_screen().or_else(NSScreen::main)
Expand Down Expand Up @@ -449,7 +449,7 @@ impl WinitWindow {
.ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;

#[cfg(feature = "rwh_06")]
match attrs.parent_window {
match attrs.parent_window.0 {
Some(rwh_06::RawWindowHandle::AppKit(handle)) => {
// SAFETY: Caller ensures the pointer is valid or NULL
// Unwrap is fine, since the pointer comes from `NonNull`.
Expand Down Expand Up @@ -520,14 +520,14 @@ impl WinitWindow {
}
}

let delegate = WinitWindowDelegate::new(&this, attrs.fullscreen.is_some());
let delegate = WinitWindowDelegate::new(&this, attrs.fullscreen.0.is_some());

// XXX Send `Focused(false)` right after creating the window delegate, so we won't
// obscure the real focused events on the startup.
delegate.queue_event(WindowEvent::Focused(false));

// Set fullscreen mode after we setup everything
this.set_fullscreen(attrs.fullscreen.map(Into::into));
this.set_fullscreen(attrs.fullscreen.0.map(Into::into));

// Setting the window as key has to happen *after* we set the fullscreen
// state, since otherwise we'll briefly see the window at normal size
Expand Down
4 changes: 2 additions & 2 deletions src/platform_impl/web/web_sys/canvas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,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 Expand Up @@ -127,7 +127,7 @@ impl Canvas {
super::set_canvas_position(&common.document, &common.raw, &common.style, position);
}

if attr.fullscreen.is_some() {
if attr.fullscreen.0.is_some() {
common.fullscreen_handler.request_fullscreen();
}

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 web_sys::HtmlCanvasElement;

Expand Down Expand Up @@ -455,7 +456,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 @@ -464,7 +465,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
6 changes: 3 additions & 3 deletions src/platform_impl/windows/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1185,8 +1185,8 @@ impl<'a, T: 'static> InitData<'a, T> {

win.set_enabled_buttons(attributes.enabled_buttons);

if attributes.fullscreen.is_some() {
win.set_fullscreen(attributes.fullscreen.map(Into::into));
if attributes.fullscreen.0.is_some() {
win.set_fullscreen(attributes.fullscreen.0.map(Into::into));
unsafe { force_window_active(win.window.0) };
} else {
let size = attributes
Expand Down Expand Up @@ -1272,7 +1272,7 @@ where
};

#[cfg(feature = "rwh_06")]
let parent = match attributes.parent_window {
let parent = match attributes.parent_window.0 {
Some(rwh_06::RawWindowHandle::Win32(handle)) => {
window_flags.set(WindowFlags::CHILD, true);
if pl_attribs.menu.is_some() {
Expand Down
51 changes: 26 additions & 25 deletions src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,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 @@ -141,7 +141,6 @@ pub struct WindowAttributes {
pub resizable: bool,
pub enabled_buttons: WindowButtons,
pub title: String,
pub fullscreen: Option<Fullscreen>,
pub maximized: bool,
pub visible: bool,
pub transparent: bool,
Expand All @@ -152,9 +151,10 @@ pub struct WindowAttributes {
pub resize_increments: Option<Size>,
pub content_protected: bool,
pub window_level: WindowLevel,
#[cfg(feature = "rwh_06")]
pub parent_window: Option<rwh_06::RawWindowHandle>,
pub active: bool,
#[cfg(feature = "rwh_06")]
pub(crate) parent_window: SendSyncWrapper<Option<rwh_06::RawWindowHandle>>,
pub(crate) fullscreen: SendSyncWrapper<Option<Fullscreen>>,
}

impl Default for WindowAttributes {
Expand All @@ -169,7 +169,7 @@ impl Default for WindowAttributes {
enabled_buttons: WindowButtons::all(),
title: "winit window".to_owned(),
maximized: false,
fullscreen: None,
fullscreen: SendSyncWrapper(None),
visible: true,
transparent: false,
blur: false,
Expand All @@ -180,12 +180,25 @@ impl Default for WindowAttributes {
resize_increments: None,
content_protected: false,
#[cfg(feature = "rwh_06")]
parent_window: None,
parent_window: SendSyncWrapper(None),
active: true,
}
}
}

impl WindowAttributes {
/// Get the parent window stored on the attributes.
#[cfg(feature = "rwh_06")]
pub fn parent_window(&self) -> Option<&rwh_06::RawWindowHandle> {
self.parent_window.0.as_ref()
}

/// Get `Fullscreen` option stored on the attributes.
pub fn fullscreen(&self) -> Option<&Fullscreen> {
self.fullscreen.0.as_ref()
}
}

impl WindowBuilder {
/// Initializes a new builder with default values.
#[inline]
Expand Down Expand Up @@ -303,7 +316,7 @@ impl WindowBuilder {
/// See [`Window::set_fullscreen`] for details.
#[inline]
pub fn with_fullscreen(mut self, fullscreen: Option<Fullscreen>) -> Self {
self.window.fullscreen = fullscreen;
self.window.fullscreen = SendSyncWrapper(fullscreen);
self
}

Expand Down Expand Up @@ -479,7 +492,7 @@ impl WindowBuilder {
mut self,
parent_window: Option<rwh_06::RawWindowHandle>,
) -> Self {
self.window.parent_window = parent_window;
self.window.parent_window = SendSyncWrapper(parent_window);
self
}

Expand Down Expand Up @@ -1510,12 +1523,9 @@ impl Window {
#[cfg(feature = "rwh_06")]
impl rwh_06::HasWindowHandle for Window {
fn window_handle(&self) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError> {
struct Wrapper(rwh_06::RawWindowHandle);
unsafe impl Send for Wrapper {}

let raw = self
.window
.maybe_wait_on_main(|w| w.raw_window_handle_rwh_06().map(Wrapper))?
.maybe_wait_on_main(|w| w.raw_window_handle_rwh_06().map(SendSyncWrapper))?
.0;

// SAFETY: The window handle will never be deallocated while the window is alive.
Expand All @@ -1526,12 +1536,9 @@ impl rwh_06::HasWindowHandle for Window {
#[cfg(feature = "rwh_06")]
impl rwh_06::HasDisplayHandle for Window {
fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
struct Wrapper(rwh_06::RawDisplayHandle);
unsafe impl Send for Wrapper {}

let raw = self
.window
.maybe_wait_on_main(|w| w.raw_display_handle_rwh_06().map(Wrapper))?
.maybe_wait_on_main(|w| w.raw_display_handle_rwh_06().map(SendSyncWrapper))?
.0;

// SAFETY: The window handle will never be deallocated while the window is alive.
Expand All @@ -1542,10 +1549,8 @@ impl rwh_06::HasDisplayHandle for Window {
#[cfg(feature = "rwh_05")]
unsafe impl rwh_05::HasRawWindowHandle for Window {
fn raw_window_handle(&self) -> rwh_05::RawWindowHandle {
struct Wrapper(rwh_05::RawWindowHandle);
unsafe impl Send for Wrapper {}
self.window
.maybe_wait_on_main(|w| Wrapper(w.raw_window_handle_rwh_05()))
.maybe_wait_on_main(|w| SendSyncWrapper(w.raw_window_handle_rwh_05()))
.0
}
}
Expand All @@ -1557,21 +1562,17 @@ unsafe impl rwh_05::HasRawDisplayHandle for Window {
///
/// [`EventLoop`]: crate::event_loop::EventLoop
fn raw_display_handle(&self) -> rwh_05::RawDisplayHandle {
struct Wrapper(rwh_05::RawDisplayHandle);
unsafe impl Send for Wrapper {}
self.window
.maybe_wait_on_main(|w| Wrapper(w.raw_display_handle_rwh_05()))
.maybe_wait_on_main(|w| SendSyncWrapper(w.raw_display_handle_rwh_05()))
.0
}
}

#[cfg(feature = "rwh_04")]
unsafe impl rwh_04::HasRawWindowHandle for Window {
fn raw_window_handle(&self) -> rwh_04::RawWindowHandle {
struct Wrapper(rwh_04::RawWindowHandle);
unsafe impl Send for Wrapper {}
self.window
.maybe_wait_on_main(|w| Wrapper(w.raw_window_handle_rwh_04()))
.maybe_wait_on_main(|w| SendSyncWrapper(w.raw_window_handle_rwh_04()))
.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 c0c7f91

Please sign in to comment.