Skip to content

Commit

Permalink
Ensure safety of creating windows
Browse files Browse the repository at this point in the history
  • Loading branch information
MDeiml committed Nov 7, 2022
1 parent ac6fa7d commit 4290e3b
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 24 deletions.
8 changes: 5 additions & 3 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,11 +146,13 @@ impl Plugin for RenderPlugin {
let instance = wgpu::Instance::new(backends);
let surface = {
let windows = app.world.resource_mut::<bevy_window::Windows>();
let raw_handle = windows.get_primary().and_then(|window| unsafe {
let raw_handle = windows.get_primary().and_then(|window| {
match window.window_handle() {
AbstractWindowHandle::RawWindowHandle(handle) => {
AbstractWindowHandle::RawWindowHandle(handle) => unsafe {
// SAFETY: This is only run on the main thread. Also the caller of `RawHandleWrapper::new` has
// ensured that the window was created on the main thread.
Some(instance.create_surface(&handle.get_handle()))
}
},
AbstractWindowHandle::Virtual => None,
}
});
Expand Down
29 changes: 17 additions & 12 deletions crates/bevy_window/src/raw_handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,24 @@ use raw_window_handle::{
/// thread-safe.
#[derive(Debug, Clone)]
pub struct RawHandleWrapper {
pub window_handle: RawWindowHandle,
pub display_handle: RawDisplayHandle,
window_handle: RawWindowHandle,
display_handle: RawDisplayHandle,
}

impl RawHandleWrapper {
/// Create a new `RawHandleWrapper` from its components
///
/// # Safety
///
/// If the particular underlying handle can only be used on the same thread, the caller should ensure that the window
/// was created on the main thread.
pub unsafe fn new(window_handle: RawWindowHandle, display_handle: RawDisplayHandle) -> Self {
RawHandleWrapper {
window_handle,
display_handle,
}
}

/// Returns a [`HasRawWindowHandle`] + [`HasRawDisplayHandle`] impl, which exposes [`RawWindowHandle`] and [`RawDisplayHandle`].
///
/// # Safety
Expand All @@ -23,14 +36,6 @@ impl RawHandleWrapper {
pub unsafe fn get_handle(&self) -> ThreadLockedRawWindowHandleWrapper {
ThreadLockedRawWindowHandleWrapper(self.clone())
}

pub fn get_display_handle(&self) -> RawDisplayHandle {
self.display_handle
}

pub fn get_window_handle(&self) -> RawWindowHandle {
self.window_handle
}
}

// SAFETY: [`RawHandleWrapper`] is just a normal "raw pointer", which doesn't impl Send/Sync. However the pointer is only
Expand Down Expand Up @@ -58,7 +63,7 @@ pub struct ThreadLockedRawWindowHandleWrapper(RawHandleWrapper);
// and so exposing a safe method to get a [`RawWindowHandle`] directly would be UB.
unsafe impl HasRawWindowHandle for ThreadLockedRawWindowHandleWrapper {
fn raw_window_handle(&self) -> RawWindowHandle {
self.0.get_window_handle()
self.0.window_handle
}
}

Expand All @@ -70,6 +75,6 @@ unsafe impl HasRawWindowHandle for ThreadLockedRawWindowHandleWrapper {
// and so exposing a safe method to get a [`RawDisplayHandle`] directly would be UB.
unsafe impl HasRawDisplayHandle for ThreadLockedRawWindowHandleWrapper {
fn raw_display_handle(&self) -> RawDisplayHandle {
self.0.get_display_handle()
self.0.display_handle
}
}
13 changes: 8 additions & 5 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,11 +685,14 @@ fn handle_create_window_events(
#[cfg(not(any(target_os = "windows", target_feature = "x11")))]
let mut window_resized_events = world.resource_mut::<Events<WindowResized>>();
for create_window_event in create_window_event_reader.iter(&create_window_events) {
let window = winit_windows.create_window(
event_loop,
create_window_event.id,
&create_window_event.descriptor,
);
let window = unsafe {
// SAFETY: This is only called from the main thread
winit_windows.create_window(
event_loop,
create_window_event.id,
&create_window_event.descriptor,
)
};
// This event is already sent on windows, x11, and xwayland.
// TODO: we aren't yet sure about native wayland, so we might be able to exclude it,
// but sending a duplicate event isn't problematic, as windows already does this.
Expand Down
14 changes: 10 additions & 4 deletions crates/bevy_winit/src/winit_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@ pub struct WinitWindows {
}

impl WinitWindows {
pub fn create_window(
/// # SAFETY
///
/// This should only be called from the main thread, as it might create windows that cannot be used across threads
pub unsafe fn create_window(
&mut self,
event_loop: &winit::event_loop::EventLoopWindowTarget<()>,
window_id: WindowId,
Expand Down Expand Up @@ -199,9 +202,12 @@ impl WinitWindows {
.map(|position| IVec2::new(position.x, position.y));
let inner_size = winit_window.inner_size();
let scale_factor = winit_window.scale_factor();
let raw_handle = RawHandleWrapper {
window_handle: winit_window.raw_window_handle(),
display_handle: winit_window.raw_display_handle(),
let raw_handle = unsafe {
// SAFETY: Caller ensured that this is run on the main thread
RawHandleWrapper::new(
winit_window.raw_window_handle(),
winit_window.raw_display_handle(),
)
};
self.windows.insert(winit_window.id(), winit_window);
Window::new(
Expand Down

0 comments on commit 4290e3b

Please sign in to comment.