Skip to content
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

[Merged by Bors] - Add option to center a window #4999

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bevy_window/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub mod prelude {
#[doc(hidden)]
pub use crate::{
CursorEntered, CursorIcon, CursorLeft, CursorMoved, FileDragAndDrop, ReceivedCharacter,
Window, WindowDescriptor, WindowMoved, Windows,
Window, WindowDescriptor, WindowMoved, WindowPosition, Windows,
};
}

Expand Down
35 changes: 30 additions & 5 deletions crates/bevy_window/src/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ pub enum WindowCommand {
SetPosition {
position: IVec2,
},
/// Modifies the position of the window to be in the center of the current monitor
Center,
/// Set the window's [`WindowResizeConstraints`]
SetResizeConstraints {
resize_constraints: WindowResizeConstraints,
Expand Down Expand Up @@ -416,6 +418,16 @@ impl Window {
.push(WindowCommand::SetPosition { position });
}

/// Modifies the position of the window to be in the center of the current monitor
///
/// # Platform-specific
/// - iOS: Can only be called on the main thread.
/// - Web / Android / Wayland: Unsupported.
#[inline]
pub fn center_window(&mut self) {
self.command_queue.push(WindowCommand::Center);
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be a new method, you could change the existing set_position to take a WindowPosition. It would support all possible extensions like if we add WindowPosition::BottomRight... but then you could call set_position(WindowPosition::Automatic) which doesn't make sense...

What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think your proposed design is better. I would just document that setting the window position to automatic is a no-op.

Copy link

@CalinZBaenen CalinZBaenen Jun 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mockersf Interesting idea suggesting other places to put the window except in the center.

[P.S. : I feel like this sounds unnecessarily passive-aggressive, so I wanted to clarify it's not meant to be. I just have no other comment to tack on.]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered the fact that we might want to add more positions, such as bottom-right etc., but I wasn't sure if there would be a use case for that and where do we draw the line of being a window manager.
But if you expect that there would be a use case for more values, then changing set_position signature makes sense.
To avoid code duplication (between set_position and create_window) we would need to abstract out WindowPosition -> actual position code, right? So perhaps a function that takes WindowPosition, window size and screen size and returns screen space position?

/// Modifies the minimum and maximum window bounds for resizing in logical pixels.
#[inline]
pub fn set_resize_constraints(&mut self, resize_constraints: WindowResizeConstraints) {
Expand Down Expand Up @@ -714,6 +726,21 @@ impl Window {
}
}

/// Defines where window should be placed at on creation.
#[derive(Debug, Clone)]
pub enum WindowPosition {
/// Position will be set by the window manager
Automatic,
/// Window will be centered on the primary monitor
///
/// Note that this does not account for window decorations.
Centered,
/// The window's top-left corner will be placed at the specified position (in pixels)
///
/// (0,0) represents top-left corner of screen space.
At(Vec2),
LoipesMas marked this conversation as resolved.
Show resolved Hide resolved
}

/// Describes the information needed for creating a window.
///
/// This should be set up before adding the [`WindowPlugin`](crate::WindowPlugin).
Expand All @@ -732,10 +759,8 @@ pub struct WindowDescriptor {
///
/// May vary from the physical height due to different pixel density on different monitors.
pub height: f32,
/// The position on the screen that the window will be centered at.
///
/// If set to `None`, some platform-specific position will be chosen.
pub position: Option<Vec2>,
/// The position on the screen that the window will be placed at.
pub position: WindowPosition,
/// Sets minimum and maximum resize limits.
pub resize_constraints: WindowResizeConstraints,
/// Overrides the window's ratio of physical pixels to logical pixels.
Expand Down Expand Up @@ -799,7 +824,7 @@ impl Default for WindowDescriptor {
title: "app".to_string(),
width: 1280.,
height: 720.,
position: None,
position: WindowPosition::Automatic,
resize_constraints: WindowResizeConstraints::default(),
scale_factor_override: None,
present_mode: PresentMode::Fifo,
Expand Down
15 changes: 15 additions & 0 deletions crates/bevy_winit/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,21 @@ fn change_window(
y: position[1],
});
}
bevy_window::WindowCommand::Center => {
let window = winit_windows.get_window(id).unwrap();

// What to do if current_monitor is None?
// Abort?
// Or use primary_monitor? And then what if that also is None?
let screen_size = window.current_monitor().unwrap().size();
LoipesMas marked this conversation as resolved.
Show resolved Hide resolved

let window_size = window.outer_size();

window.set_outer_position(PhysicalPosition {
x: (screen_size.width - window_size.width) as f64 / 2.,
y: (screen_size.height - window_size.height) as f64 / 2.,
});
}
LoipesMas marked this conversation as resolved.
Show resolved Hide resolved
bevy_window::WindowCommand::SetResizeConstraints { resize_constraints } => {
let window = winit_windows.get_window(id).unwrap();
let constraints = resize_constraints.check_constraints();
Expand Down
59 changes: 38 additions & 21 deletions crates/bevy_winit/src/winit_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bevy_math::IVec2;
use bevy_utils::HashMap;
use bevy_window::{Window, WindowDescriptor, WindowId, WindowMode};
use raw_window_handle::HasRawWindowHandle;
use winit::dpi::LogicalSize;
use winit::dpi::{LogicalPosition, LogicalSize, PhysicalPosition};

#[derive(Debug, Default)]
pub struct WinitWindows {
Expand Down Expand Up @@ -49,30 +49,47 @@ impl WinitWindows {
..
} = window_descriptor;

if let Some(position) = position {
if let Some(sf) = scale_factor_override {
winit_window_builder = winit_window_builder.with_position(
winit::dpi::LogicalPosition::new(
position[0] as f64,
position[1] as f64,
)
.to_physical::<f64>(*sf),
);
} else {
winit_window_builder =
winit_window_builder.with_position(winit::dpi::LogicalPosition::new(
position[0] as f64,
position[1] as f64,
));
use bevy_window::WindowPosition::*;
match position {
Automatic => { /* Window manager will handle position */ }
Centered => {
if let Some(monitor) = event_loop.primary_monitor() {
let screen_size = monitor.size();

let scale_factor = scale_factor_override.unwrap_or(1.0);

// Logical to physical window size
let (width, height): (f64, f64) = LogicalSize::new(*width, *height)
.to_physical::<f64>(scale_factor)
.into();

let position = PhysicalPosition::new(
(screen_size.width as f64 - width) / 2.,
(screen_size.height as f64 - height) / 2.,
);

winit_window_builder = winit_window_builder.with_position(position);
}
}
At(position) => {
if let Some(sf) = scale_factor_override {
winit_window_builder = winit_window_builder.with_position(
LogicalPosition::new(position[0] as f64, position[1] as f64)
.to_physical::<f64>(*sf),
);
} else {
winit_window_builder = winit_window_builder.with_position(
LogicalPosition::new(position[0] as f64, position[1] as f64),
);
}
}
}

if let Some(sf) = scale_factor_override {
winit_window_builder.with_inner_size(
winit::dpi::LogicalSize::new(*width, *height).to_physical::<f64>(*sf),
)
} else {
winit_window_builder
.with_inner_size(winit::dpi::LogicalSize::new(*width, *height))
.with_inner_size(LogicalSize::new(*width, *height).to_physical::<f64>(*sf))
} else {
winit_window_builder.with_inner_size(LogicalSize::new(*width, *height))
}
}
.with_resizable(window_descriptor.resizable)
Expand Down