Make WindowBuilder Send + Sync

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.
This commit is contained in:
Kirill Chibisov 2023-10-17 04:54:12 +04:00 committed by GitHub
parent 3ad64fb811
commit 801fddbfcf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 72 additions and 47 deletions

View file

@ -36,6 +36,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 - On Wayland, support `Occluded` event with xdg-shell v6
- Implement `AsFd`/`AsRawFd` for `EventLoop<T>` on X11 and Wayland. - 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`. - **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 # 0.29.1-beta

View file

@ -163,3 +163,18 @@ mod platform_impl;
pub mod window; pub mod window;
pub mod platform; 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> {}

View file

@ -31,6 +31,7 @@ use crate::event::Event;
use crate::event_loop::EventLoop; use crate::event_loop::EventLoop;
use crate::event_loop::EventLoopWindowTarget; use crate::event_loop::EventLoopWindowTarget;
use crate::window::{Window, WindowBuilder}; use crate::window::{Window, WindowBuilder};
use crate::SendSyncWrapper;
use web_sys::HtmlCanvasElement; use web_sys::HtmlCanvasElement;
@ -81,26 +82,22 @@ pub trait WindowBuilderExtWebSys {
impl WindowBuilderExtWebSys for WindowBuilder { impl WindowBuilderExtWebSys for WindowBuilder {
fn with_canvas(mut self, canvas: Option<HtmlCanvasElement>) -> Self { fn with_canvas(mut self, canvas: Option<HtmlCanvasElement>) -> Self {
self.platform_specific.canvas = canvas; self.platform_specific.canvas = SendSyncWrapper(canvas);
self self
} }
fn with_prevent_default(mut self, prevent_default: bool) -> Self { fn with_prevent_default(mut self, prevent_default: bool) -> Self {
self.platform_specific.prevent_default = prevent_default; self.platform_specific.prevent_default = prevent_default;
self self
} }
fn with_focusable(mut self, focusable: bool) -> Self { fn with_focusable(mut self, focusable: bool) -> Self {
self.platform_specific.focusable = focusable; self.platform_specific.focusable = focusable;
self self
} }
fn with_append(mut self, append: bool) -> Self { fn with_append(mut self, append: bool) -> Self {
self.platform_specific.append = append; self.platform_specific.append = append;
self self
} }
} }

View file

@ -473,7 +473,7 @@ impl WinitUIWindow {
this.setRootViewController(Some(view_controller)); 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)) => { Some(Fullscreen::Exclusive(ref video_mode)) => {
let monitor = video_mode.monitor(); let monitor = video_mode.monitor();
let screen = monitor.ui_screen(mtm); let screen = monitor.ui_screen(mtm);

View file

@ -422,7 +422,7 @@ impl Window {
// TODO: transparency, visible // TODO: transparency, visible
let main_screen = UIScreen::main(mtm); 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 { let screen = match fullscreen {
Some(Fullscreen::Exclusive(ref video_mode)) => video_mode.monitor.ui_screen(mtm), Some(Fullscreen::Exclusive(ref video_mode)) => video_mode.monitor.ui_screen(mtm),
Some(Fullscreen::Borderless(Some(ref monitor))) => monitor.ui_screen(mtm), Some(Fullscreen::Borderless(Some(ref monitor))) => monitor.ui_screen(mtm),

View file

@ -151,7 +151,7 @@ impl Window {
window_state.set_resizable(attributes.resizable); window_state.set_resizable(attributes.resizable);
// Set startup mode. // Set startup mode.
match attributes.fullscreen.map(Into::into) { match attributes.fullscreen.0.map(Into::into) {
Some(Fullscreen::Exclusive(_)) => { Some(Fullscreen::Exclusive(_)) => {
warn!("`Fullscreen::Exclusive` is ignored on Wayland"); warn!("`Fullscreen::Exclusive` is ignored on Wayland");
} }

View file

@ -150,7 +150,7 @@ impl UnownedWindow {
let xconn = &event_loop.xconn; let xconn = &event_loop.xconn;
let atoms = xconn.atoms(); let atoms = xconn.atoms();
#[cfg(feature = "rwh_06")] #[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::Xlib(handle)) => handle.window as xproto::Window,
Some(rwh_06::RawWindowHandle::Xcb(handle)) => handle.window.get(), Some(rwh_06::RawWindowHandle::Xcb(handle)) => handle.window.get(),
Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"), Some(raw) => unreachable!("Invalid raw window handle {raw:?} on X11"),
@ -547,10 +547,10 @@ impl UnownedWindow {
if window_attrs.maximized { if window_attrs.maximized {
leap!(window.set_maximized_inner(window_attrs.maximized)).ignore_error(); 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) = if let Some(flusher) =
leap!(window 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() flusher.ignore_error()
} }

View file

@ -295,7 +295,7 @@ impl WinitWindow {
trace_scope!("WinitWindow::new"); trace_scope!("WinitWindow::new");
let this = autoreleasepool(|_| { 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::Borderless(Some(monitor)))
| Some(Fullscreen::Exclusive(VideoMode { monitor, .. })) => { | Some(Fullscreen::Exclusive(VideoMode { monitor, .. })) => {
monitor.ns_screen().or_else(NSScreen::main) monitor.ns_screen().or_else(NSScreen::main)
@ -449,7 +449,7 @@ impl WinitWindow {
.ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?; .ok_or_else(|| os_error!(OsError::CreationError("Couldn't create `NSWindow`")))?;
#[cfg(feature = "rwh_06")] #[cfg(feature = "rwh_06")]
match attrs.parent_window { match attrs.parent_window.0 {
Some(rwh_06::RawWindowHandle::AppKit(handle)) => { Some(rwh_06::RawWindowHandle::AppKit(handle)) => {
// SAFETY: Caller ensures the pointer is valid or NULL // SAFETY: Caller ensures the pointer is valid or NULL
// Unwrap is fine, since the pointer comes from `NonNull`. // Unwrap is fine, since the pointer comes from `NonNull`.
@ -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 // XXX Send `Focused(false)` right after creating the window delegate, so we won't
// obscure the real focused events on the startup. // obscure the real focused events on the startup.
delegate.queue_event(WindowEvent::Focused(false)); delegate.queue_event(WindowEvent::Focused(false));
// Set fullscreen mode after we setup everything // 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 // 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 // state, since otherwise we'll briefly see the window at normal size

View file

@ -63,7 +63,7 @@ impl Canvas {
attr: &WindowAttributes, attr: &WindowAttributes,
platform_attr: PlatformSpecificWindowBuilderAttributes, platform_attr: PlatformSpecificWindowBuilderAttributes,
) -> Result<Self, RootOE> { ) -> Result<Self, RootOE> {
let canvas = match platform_attr.canvas { let canvas = match platform_attr.canvas.0 {
Some(canvas) => canvas, Some(canvas) => canvas,
None => document None => document
.create_element("canvas") .create_element("canvas")
@ -127,7 +127,7 @@ impl Canvas {
super::set_canvas_position(&common.document, &common.raw, &common.style, position); 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(); common.fullscreen_handler.request_fullscreen();
} }

View file

@ -5,6 +5,7 @@ use crate::window::{
CursorGrabMode, CursorIcon, ImePurpose, ResizeDirection, Theme, UserAttentionType, CursorGrabMode, CursorIcon, ImePurpose, ResizeDirection, Theme, UserAttentionType,
WindowAttributes, WindowButtons, WindowId as RootWI, WindowLevel, WindowAttributes, WindowButtons, WindowId as RootWI, WindowLevel,
}; };
use crate::SendSyncWrapper;
use web_sys::HtmlCanvasElement; use web_sys::HtmlCanvasElement;
@ -455,7 +456,7 @@ impl From<u64> for WindowId {
#[derive(Clone)] #[derive(Clone)]
pub struct PlatformSpecificWindowBuilderAttributes { pub struct PlatformSpecificWindowBuilderAttributes {
pub(crate) canvas: Option<backend::RawCanvasType>, pub(crate) canvas: SendSyncWrapper<Option<backend::RawCanvasType>>,
pub(crate) prevent_default: bool, pub(crate) prevent_default: bool,
pub(crate) focusable: bool, pub(crate) focusable: bool,
pub(crate) append: bool, pub(crate) append: bool,
@ -464,7 +465,7 @@ pub struct PlatformSpecificWindowBuilderAttributes {
impl Default for PlatformSpecificWindowBuilderAttributes { impl Default for PlatformSpecificWindowBuilderAttributes {
fn default() -> Self { fn default() -> Self {
Self { Self {
canvas: None, canvas: SendSyncWrapper(None),
prevent_default: true, prevent_default: true,
focusable: true, focusable: true,
append: false, append: false,

View file

@ -1185,8 +1185,8 @@ impl<'a, T: 'static> InitData<'a, T> {
win.set_enabled_buttons(attributes.enabled_buttons); win.set_enabled_buttons(attributes.enabled_buttons);
if attributes.fullscreen.is_some() { if attributes.fullscreen.0.is_some() {
win.set_fullscreen(attributes.fullscreen.map(Into::into)); win.set_fullscreen(attributes.fullscreen.0.map(Into::into));
unsafe { force_window_active(win.window.0) }; unsafe { force_window_active(win.window.0) };
} else { } else {
let size = attributes let size = attributes
@ -1272,7 +1272,7 @@ where
}; };
#[cfg(feature = "rwh_06")] #[cfg(feature = "rwh_06")]
let parent = match attributes.parent_window { let parent = match attributes.parent_window.0 {
Some(rwh_06::RawWindowHandle::Win32(handle)) => { Some(rwh_06::RawWindowHandle::Win32(handle)) => {
window_flags.set(WindowFlags::CHILD, true); window_flags.set(WindowFlags::CHILD, true);
if pl_attribs.menu.is_some() { if pl_attribs.menu.is_some() {

View file

@ -6,7 +6,7 @@ use crate::{
error::{ExternalError, NotSupportedError, OsError}, error::{ExternalError, NotSupportedError, OsError},
event_loop::EventLoopWindowTarget, event_loop::EventLoopWindowTarget,
monitor::{MonitorHandle, VideoMode}, monitor::{MonitorHandle, VideoMode},
platform_impl, platform_impl, SendSyncWrapper,
}; };
pub use crate::icon::{BadIcon, Icon}; pub use crate::icon::{BadIcon, Icon};
@ -141,7 +141,6 @@ pub struct WindowAttributes {
pub resizable: bool, pub resizable: bool,
pub enabled_buttons: WindowButtons, pub enabled_buttons: WindowButtons,
pub title: String, pub title: String,
pub fullscreen: Option<Fullscreen>,
pub maximized: bool, pub maximized: bool,
pub visible: bool, pub visible: bool,
pub transparent: bool, pub transparent: bool,
@ -152,9 +151,10 @@ pub struct WindowAttributes {
pub resize_increments: Option<Size>, pub resize_increments: Option<Size>,
pub content_protected: bool, pub content_protected: bool,
pub window_level: WindowLevel, pub window_level: WindowLevel,
#[cfg(feature = "rwh_06")]
pub parent_window: Option<rwh_06::RawWindowHandle>,
pub active: bool, 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 { impl Default for WindowAttributes {
@ -169,7 +169,7 @@ impl Default for WindowAttributes {
enabled_buttons: WindowButtons::all(), enabled_buttons: WindowButtons::all(),
title: "winit window".to_owned(), title: "winit window".to_owned(),
maximized: false, maximized: false,
fullscreen: None, fullscreen: SendSyncWrapper(None),
visible: true, visible: true,
transparent: false, transparent: false,
blur: false, blur: false,
@ -180,12 +180,25 @@ impl Default for WindowAttributes {
resize_increments: None, resize_increments: None,
content_protected: false, content_protected: false,
#[cfg(feature = "rwh_06")] #[cfg(feature = "rwh_06")]
parent_window: None, parent_window: SendSyncWrapper(None),
active: true, 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 { impl WindowBuilder {
/// Initializes a new builder with default values. /// Initializes a new builder with default values.
#[inline] #[inline]
@ -303,7 +316,7 @@ impl WindowBuilder {
/// See [`Window::set_fullscreen`] for details. /// See [`Window::set_fullscreen`] for details.
#[inline] #[inline]
pub fn with_fullscreen(mut self, fullscreen: Option<Fullscreen>) -> Self { pub fn with_fullscreen(mut self, fullscreen: Option<Fullscreen>) -> Self {
self.window.fullscreen = fullscreen; self.window.fullscreen = SendSyncWrapper(fullscreen);
self self
} }
@ -479,7 +492,7 @@ impl WindowBuilder {
mut self, mut self,
parent_window: Option<rwh_06::RawWindowHandle>, parent_window: Option<rwh_06::RawWindowHandle>,
) -> Self { ) -> Self {
self.window.parent_window = parent_window; self.window.parent_window = SendSyncWrapper(parent_window);
self self
} }
@ -1510,12 +1523,9 @@ impl Window {
#[cfg(feature = "rwh_06")] #[cfg(feature = "rwh_06")]
impl rwh_06::HasWindowHandle for Window { impl rwh_06::HasWindowHandle for Window {
fn window_handle(&self) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError> { fn window_handle(&self) -> Result<rwh_06::WindowHandle<'_>, rwh_06::HandleError> {
struct Wrapper(rwh_06::RawWindowHandle);
unsafe impl Send for Wrapper {}
let raw = self let raw = self
.window .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; .0;
// SAFETY: The window handle will never be deallocated while the window is alive. // SAFETY: The window handle will never be deallocated while the window is alive.
@ -1526,12 +1536,9 @@ impl rwh_06::HasWindowHandle for Window {
#[cfg(feature = "rwh_06")] #[cfg(feature = "rwh_06")]
impl rwh_06::HasDisplayHandle for Window { impl rwh_06::HasDisplayHandle for Window {
fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> { fn display_handle(&self) -> Result<rwh_06::DisplayHandle<'_>, rwh_06::HandleError> {
struct Wrapper(rwh_06::RawDisplayHandle);
unsafe impl Send for Wrapper {}
let raw = self let raw = self
.window .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; .0;
// SAFETY: The window handle will never be deallocated while the window is alive. // SAFETY: The window handle will never be deallocated while the window is alive.
@ -1542,10 +1549,8 @@ impl rwh_06::HasDisplayHandle for Window {
#[cfg(feature = "rwh_05")] #[cfg(feature = "rwh_05")]
unsafe impl rwh_05::HasRawWindowHandle for Window { unsafe impl rwh_05::HasRawWindowHandle for Window {
fn raw_window_handle(&self) -> rwh_05::RawWindowHandle { fn raw_window_handle(&self) -> rwh_05::RawWindowHandle {
struct Wrapper(rwh_05::RawWindowHandle);
unsafe impl Send for Wrapper {}
self.window 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 .0
} }
} }
@ -1557,10 +1562,8 @@ unsafe impl rwh_05::HasRawDisplayHandle for Window {
/// ///
/// [`EventLoop`]: crate::event_loop::EventLoop /// [`EventLoop`]: crate::event_loop::EventLoop
fn raw_display_handle(&self) -> rwh_05::RawDisplayHandle { fn raw_display_handle(&self) -> rwh_05::RawDisplayHandle {
struct Wrapper(rwh_05::RawDisplayHandle);
unsafe impl Send for Wrapper {}
self.window 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 .0
} }
} }
@ -1568,10 +1571,8 @@ unsafe impl rwh_05::HasRawDisplayHandle for Window {
#[cfg(feature = "rwh_04")] #[cfg(feature = "rwh_04")]
unsafe impl rwh_04::HasRawWindowHandle for Window { unsafe impl rwh_04::HasRawWindowHandle for Window {
fn raw_window_handle(&self) -> rwh_04::RawWindowHandle { fn raw_window_handle(&self) -> rwh_04::RawWindowHandle {
struct Wrapper(rwh_04::RawWindowHandle);
unsafe impl Send for Wrapper {}
self.window 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 .0
} }
} }

View file

@ -16,6 +16,11 @@ fn window_send() {
needs_send::<winit::window::Window>(); needs_send::<winit::window::Window>();
} }
#[test]
fn window_builder_send() {
needs_send::<winit::window::WindowBuilder>();
}
#[test] #[test]
fn ids_send() { fn ids_send() {
// ensures that the various `..Id` types implement `Send` // ensures that the various `..Id` types implement `Send`

View file

@ -6,3 +6,8 @@ fn window_sync() {
// ensures that `winit::Window` implements `Sync` // ensures that `winit::Window` implements `Sync`
needs_sync::<winit::window::Window>(); needs_sync::<winit::window::Window>();
} }
#[test]
fn window_builder_sync() {
needs_sync::<winit::window::WindowBuilder>();
}