From e41edf1478fec6d5d94a793a43ae8323b3ce7244 Mon Sep 17 00:00:00 2001 From: targrub Date: Mon, 17 Oct 2022 14:19:24 +0000 Subject: [PATCH] Make `raw_window_handle` field in `Window` and `ExtractedWindow` an `Option`. (#6114) # Objective - Trying to make it possible to do write tests that don't require a raw window handle. - Fixes https://github.com/bevyengine/bevy/issues/6106. ## Solution - Make the interface and type changes. Avoid accessing `None`. --- ## Changelog - Converted `raw_window_handle` field in both `Window` and `ExtractedWindow` to `Option`. - Revised accessor function `Window::raw_window_handle()` to return `Option`. - Skip conditions in loops that would require a raw window handle (to create a `Surface`, for example). ## Migration Guide `Window::raw_window_handle()` now returns `Option`. Co-authored-by: targrub <62773321+targrub@users.noreply.github.com> --- crates/bevy_render/src/lib.rs | 19 ++++--- crates/bevy_render/src/view/window.rs | 16 ++++-- crates/bevy_window/src/window.rs | 69 +++++++++++++++++++++++--- crates/bevy_winit/src/winit_windows.rs | 2 +- 4 files changed, 87 insertions(+), 19 deletions(-) diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 62d0bdb7badd6..1d289fe13eab8 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -144,14 +144,21 @@ impl Plugin for RenderPlugin { .register_type::(); if let Some(backends) = options.backends { + let windows = app.world.resource_mut::(); let instance = wgpu::Instance::new(backends); let surface = { - let windows = app.world.resource_mut::(); - let raw_handle = windows.get_primary().map(|window| unsafe { - let handle = window.raw_window_handle().get_handle(); - instance.create_surface(&handle) - }); - raw_handle + if let Some(window) = windows.get_primary() { + if let Some(raw_window_handle) = window.raw_window_handle() { + unsafe { + let handle = raw_window_handle.get_handle(); + Some(instance.create_surface(&handle)) + } + } else { + None + } + } else { + None + } }; let request_adapter_options = wgpu::RequestAdapterOptions { power_preference: options.power_preference, diff --git a/crates/bevy_render/src/view/window.rs b/crates/bevy_render/src/view/window.rs index fbe9ba4f66775..a44610d0dc81d 100644 --- a/crates/bevy_render/src/view/window.rs +++ b/crates/bevy_render/src/view/window.rs @@ -38,7 +38,7 @@ impl Plugin for WindowRenderPlugin { pub struct ExtractedWindow { pub id: WindowId, - pub handle: RawWindowHandleWrapper, + pub raw_window_handle: Option, pub physical_width: u32, pub physical_height: u32, pub present_mode: PresentMode, @@ -83,7 +83,7 @@ fn extract_windows( .entry(window.id()) .or_insert(ExtractedWindow { id: window.id(), - handle: window.raw_window_handle(), + raw_window_handle: window.raw_window_handle(), physical_width: new_width, physical_height: new_height, present_mode: window.present_mode(), @@ -161,14 +161,20 @@ pub fn prepare_windows( render_instance: Res, render_adapter: Res, ) { - let window_surfaces = window_surfaces.deref_mut(); - for window in windows.windows.values_mut() { + for window in windows + .windows + .values_mut() + // value of raw_winndow_handle only None if synthetic test + .filter(|x| x.raw_window_handle.is_some()) + { + let window_surfaces = window_surfaces.deref_mut(); let surface = window_surfaces .surfaces .entry(window.id) .or_insert_with(|| unsafe { // NOTE: On some OSes this MUST be called from the main thread. - render_instance.create_surface(&window.handle.get_handle()) + render_instance + .create_surface(&window.raw_window_handle.as_ref().unwrap().get_handle()) }); let swap_chain_descriptor = wgpu::SurfaceConfiguration { diff --git a/crates/bevy_window/src/window.rs b/crates/bevy_window/src/window.rs index 8ebf978efcfd6..c0eae67f02973 100644 --- a/crates/bevy_window/src/window.rs +++ b/crates/bevy_window/src/window.rs @@ -187,6 +187,59 @@ impl WindowResizeConstraints { /// } /// } /// ``` +/// To test code that uses `Window`s, one can test it with varying `Window` parameters by +/// creating `WindowResizeConstraints` or `WindowDescriptor` structures. +/// values by setting +/// +/// ``` +/// # use bevy_utils::default; +/// # use bevy_window::{Window, WindowCommand, WindowDescriptor, WindowId, WindowResizeConstraints}; +/// # fn compute_window_area(w: &Window) -> f32 { +/// # w.width() * w.height() +/// # } +/// # fn grow_window_to_text_size(_window: &mut Window, _text: &str) {} +/// # fn set_new_title(window: &mut Window, text: String) { window.set_title(text); } +/// # fn a_window_resize_test() { +/// let resize_constraints = WindowResizeConstraints { +/// min_width: 400.0, +/// min_height: 300.0, +/// max_width: 1280.0, +/// max_height: 1024.0, +/// }; +/// let window_descriptor = WindowDescriptor { +/// width: 800.0, +/// height: 600.0, +/// resizable: true, +/// resize_constraints, +/// ..default() +/// }; +/// let mut window = Window::new( +/// WindowId::new(), +/// &window_descriptor, +/// 100, // physical_width +/// 100, // physical_height +/// 1.0, // scale_factor +/// None, None); +/// +/// let area = compute_window_area(&window); +/// assert_eq!(area, 100.0 * 100.0); +/// +/// grow_window_to_text_size(&mut window, "very long text that does not wrap"); +/// assert_eq!(window.physical_width(), window.requested_width() as u32); +/// grow_window_to_text_size(&mut window, "very long text that does wrap, creating a maximum width window"); +/// assert_eq!(window.physical_width(), window.requested_width() as u32); +/// +/// set_new_title(&mut window, "new title".to_string()); +/// let mut found_command = false; +/// for command in window.drain_commands() { +/// if command == (WindowCommand::SetTitle{ title: "new title".to_string() }) { +/// found_command = true; +/// break; +/// } +/// } +/// assert_eq!(found_command, true); +/// } +/// ``` #[derive(Debug)] pub struct Window { id: WindowId, @@ -206,7 +259,7 @@ pub struct Window { cursor_visible: bool, cursor_locked: bool, physical_cursor_position: Option, - raw_window_handle: RawWindowHandleWrapper, + raw_window_handle: Option, focused: bool, mode: WindowMode, canvas: Option, @@ -217,7 +270,7 @@ pub struct Window { /// /// Bevy apps don't interact with this `enum` directly. Instead, they should use the methods on [`Window`]. /// This `enum` is meant for authors of windowing plugins. See the documentation on [`crate::WindowPlugin`] for more information. -#[derive(Debug)] +#[derive(Debug, PartialEq)] #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] pub enum WindowCommand { /// Set the window's [`WindowMode`]. @@ -315,7 +368,7 @@ impl Window { physical_height: u32, scale_factor: f64, position: Option, - raw_window_handle: RawWindowHandle, + raw_window_handle: Option, ) -> Self { Window { id, @@ -335,7 +388,7 @@ impl Window { cursor_locked: window_descriptor.cursor_locked, cursor_icon: CursorIcon::Default, physical_cursor_position: None, - raw_window_handle: RawWindowHandleWrapper::new(raw_window_handle), + raw_window_handle: raw_window_handle.map(RawWindowHandleWrapper::new), focused: true, mode: window_descriptor.mode, canvas: window_descriptor.canvas.clone(), @@ -719,9 +772,11 @@ impl Window { pub fn is_focused(&self) -> bool { self.focused } - /// Get the [`RawWindowHandleWrapper`] corresponding to this window - pub fn raw_window_handle(&self) -> RawWindowHandleWrapper { - self.raw_window_handle.clone() + /// Get the [`RawWindowHandleWrapper`] corresponding to this window if set. + /// + /// During normal use, this can be safely unwrapped; the value should only be [`None`] when synthetically constructed for tests. + pub fn raw_window_handle(&self) -> Option { + self.raw_window_handle.as_ref().cloned() } /// The "html canvas" element selector. diff --git a/crates/bevy_winit/src/winit_windows.rs b/crates/bevy_winit/src/winit_windows.rs index 4467574874c61..47423957cfbc5 100644 --- a/crates/bevy_winit/src/winit_windows.rs +++ b/crates/bevy_winit/src/winit_windows.rs @@ -201,7 +201,7 @@ impl WinitWindows { inner_size.height, scale_factor, position, - raw_window_handle, + Some(raw_window_handle), ) }