-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
Add option to create headless windows #3835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ impl Plugin for WindowRenderPlugin { | |
|
||
pub struct ExtractedWindow { | ||
pub id: WindowId, | ||
pub handle: RawWindowHandleWrapper, | ||
pub handle: Option<RawWindowHandleWrapper>, | ||
pub physical_width: u32, | ||
pub physical_height: u32, | ||
pub present_mode: PresentMode, | ||
|
@@ -125,42 +125,44 @@ pub fn prepare_windows( | |
) { | ||
let window_surfaces = window_surfaces.deref_mut(); | ||
for window in windows.windows.values_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()) | ||
}); | ||
|
||
let swap_chain_descriptor = wgpu::SurfaceConfiguration { | ||
format: TextureFormat::bevy_default(), | ||
width: window.physical_width, | ||
height: window.physical_height, | ||
usage: wgpu::TextureUsages::RENDER_ATTACHMENT, | ||
present_mode: match window.present_mode { | ||
PresentMode::Fifo => wgpu::PresentMode::Fifo, | ||
PresentMode::Mailbox => wgpu::PresentMode::Mailbox, | ||
PresentMode::Immediate => wgpu::PresentMode::Immediate, | ||
}, | ||
}; | ||
|
||
// Do the initial surface configuration if it hasn't been configured yet | ||
if window_surfaces.configured_windows.insert(window.id) || window.size_changed { | ||
render_device.configure_surface(surface, &swap_chain_descriptor); | ||
} | ||
if let Some(window_handle_wrapper) = &window.handle { | ||
let surface = window_surfaces | ||
.surfaces | ||
.entry(window.id) | ||
.or_insert_with(|| unsafe { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing safety comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. As far as I can tell, it has never had a safety comment. I am also not experienced enough with the codebase to know why this unsafe is OK here (or maybe it even isn't, and we haven't noticed yet). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright. @cart / @superdump if you can quickly explain why this is safe during review that would be appreciated, but it shouldn't block this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. from https://docs.rs/wgpu/latest/wgpu/struct.Instance.html#safety-2:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These safety requirements have always been slightly spooky. That is, imo a reasonable safety comment here is To my knowledge, it's safe, but it's very difficult to prove that. I don't think |
||
// NOTE: On some OSes this MUST be called from the main thread. | ||
render_instance.create_surface(&window_handle_wrapper.get_handle()) | ||
}); | ||
|
||
let frame = match surface.get_current_texture() { | ||
Ok(swap_chain_frame) => swap_chain_frame, | ||
Err(wgpu::SurfaceError::Outdated) => { | ||
let swap_chain_descriptor = wgpu::SurfaceConfiguration { | ||
format: TextureFormat::bevy_default(), | ||
width: window.physical_width, | ||
height: window.physical_height, | ||
usage: wgpu::TextureUsages::RENDER_ATTACHMENT, | ||
present_mode: match window.present_mode { | ||
PresentMode::Fifo => wgpu::PresentMode::Fifo, | ||
PresentMode::Mailbox => wgpu::PresentMode::Mailbox, | ||
PresentMode::Immediate => wgpu::PresentMode::Immediate, | ||
}, | ||
}; | ||
|
||
// Do the initial surface configuration if it hasn't been configured yet | ||
if window_surfaces.configured_windows.insert(window.id) || window.size_changed { | ||
render_device.configure_surface(surface, &swap_chain_descriptor); | ||
surface | ||
.get_current_texture() | ||
.expect("Error reconfiguring surface") | ||
} | ||
err => err.expect("Failed to acquire next swap chain texture!"), | ||
}; | ||
|
||
window.swap_chain_texture = Some(TextureView::from(frame)); | ||
let frame = match surface.get_current_texture() { | ||
Ok(swap_chain_frame) => swap_chain_frame, | ||
Err(wgpu::SurfaceError::Outdated) => { | ||
render_device.configure_surface(surface, &swap_chain_descriptor); | ||
surface | ||
.get_current_texture() | ||
.expect("Error reconfiguring surface") | ||
} | ||
err => err.expect("Failed to acquire next swap chain texture!"), | ||
}; | ||
|
||
window.swap_chain_texture = Some(TextureView::from(frame)); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
use bevy::prelude::*; | ||
use bevy_internal::asset::AssetPlugin; | ||
use bevy_internal::core_pipeline::CorePipelinePlugin; | ||
use bevy_internal::input::InputPlugin; | ||
use bevy_internal::render::settings::WgpuSettings; | ||
use bevy_internal::render::RenderPlugin; | ||
use bevy_internal::sprite::SpritePlugin; | ||
use bevy_internal::text::TextPlugin; | ||
use bevy_internal::ui::UiPlugin; | ||
use bevy_internal::window::{WindowId, WindowPlugin}; | ||
|
||
const WINDOW_WIDTH: u32 = 200; | ||
const WINDOW_HEIGHT: u32 = 100; | ||
|
||
struct HeadlessUiPlugin; | ||
|
||
impl Plugin for HeadlessUiPlugin { | ||
fn build(&self, app: &mut App) { | ||
// These tests are meant to be ran on systems without gpu, or display. | ||
// To make this work, we tell bevy not to look for any rendering backends. | ||
app.insert_resource(WgpuSettings { | ||
backends: None, | ||
..Default::default() | ||
}) | ||
// To test the positioning of UI elements, | ||
// we first need a window to position these elements in. | ||
.insert_resource({ | ||
let mut windows = Windows::default(); | ||
windows.add(Window::new( | ||
// At the moment, all ui elements are placed in the primary window. | ||
WindowId::primary(), | ||
&WindowDescriptor::default(), | ||
WINDOW_WIDTH, | ||
WINDOW_HEIGHT, | ||
1.0, | ||
None, | ||
// Because this test is running without a real window, we pass `None` here. | ||
None, | ||
)); | ||
windows | ||
}) | ||
.add_plugins(MinimalPlugins) | ||
.add_plugin(TransformPlugin) | ||
.add_plugin(WindowPlugin::default()) | ||
.add_plugin(InputPlugin) | ||
.add_plugin(AssetPlugin) | ||
.add_plugin(RenderPlugin) | ||
.add_plugin(CorePipelinePlugin) | ||
.add_plugin(SpritePlugin) | ||
.add_plugin(TextPlugin) | ||
.add_plugin(UiPlugin); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_button_translation() { | ||
let mut app = App::new(); | ||
app.add_plugin(HeadlessUiPlugin) | ||
.add_startup_system(setup_button_test); | ||
|
||
// First call to `update` also runs the startup systems. | ||
app.update(); | ||
|
||
let mut query = app.world.query_filtered::<Entity, With<Button>>(); | ||
let button = *query.iter(&app.world).collect::<Vec<_>>().first().unwrap(); | ||
|
||
// The button's translation got updated because the UI system had a window to place it in. | ||
// If we hadn't added a window, the button's translation would at this point be all zeros. | ||
let button_transform = app.world.entity(button).get::<Transform>().unwrap(); | ||
assert_eq!( | ||
button_transform.translation.x.floor() as u32, | ||
WINDOW_WIDTH / 2 | ||
); | ||
assert_eq!( | ||
button_transform.translation.y.floor() as u32, | ||
WINDOW_HEIGHT / 2 | ||
); | ||
} | ||
|
||
fn setup_button_test(mut commands: Commands) { | ||
commands.spawn_bundle(UiCameraBundle::default()); | ||
commands.spawn_bundle(ButtonBundle { | ||
style: Style { | ||
size: Size::new(Val::Px(150.0), Val::Px(65.0)), | ||
// Center this button in the middle of the window. | ||
margin: Rect::all(Val::Auto), | ||
..Default::default() | ||
}, | ||
..Default::default() | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should return a
Result
, not anOption
IMO.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see why that would make sense. Then the caller would know why they didn't get a surface (no window resource for example). On the other hand, we don't actually use the reason for not getting a surface anywhere (yet?).
If this returned an error, currently we'd have to convert it back into an Option to pass it into
RequestAdapterOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, that's fair. Since this isn't
pub
that's probably fine for now.