From e4ded778bf0ae28d4c5bbcc0c8bac3488e9d834c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 11 Aug 2023 16:04:42 +0200 Subject: [PATCH] eframe: Better restore Window position on Mac when on secondary monitor --- crates/eframe/src/native/epi_integration.rs | 15 +- crates/eframe/src/native/run.rs | 12 +- crates/egui-winit/src/window_settings.rs | 193 ++++++++++++-------- 3 files changed, 136 insertions(+), 84 deletions(-) diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index a0bec23bdcc..725522eb070 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -140,10 +140,11 @@ pub fn window_builder( let inner_size_points = if let Some(mut window_settings) = window_settings { // Restore pos/size from previous session - window_settings.clamp_to_sane_values(largest_monitor_point_size(event_loop)); - #[cfg(windows)] - window_settings.clamp_window_to_sane_position(event_loop); - window_builder = window_settings.initialize_window(window_builder); + + window_settings.clamp_size_to_sane_values(largest_monitor_point_size(event_loop)); + window_settings.clamp_position_to_monitors(event_loop); + + window_builder = window_settings.initialize_window_builder(window_builder); window_settings.inner_size_points() } else { if let Some(pos) = *initial_window_pos { @@ -173,12 +174,14 @@ pub fn window_builder( } } } + window_builder } pub fn apply_native_options_to_window( window: &winit::window::Window, native_options: &crate::NativeOptions, + window_settings: Option, ) { use winit::window::WindowLevel; window.set_window_level(if native_options.always_on_top { @@ -186,6 +189,10 @@ pub fn apply_native_options_to_window( } else { WindowLevel::Normal }); + + if let Some(window_settings) = window_settings { + window_settings.initialize_window(window); + } } fn largest_monitor_point_size(event_loop: &EventLoopWindowTarget) -> egui::Vec2 { diff --git a/crates/eframe/src/native/run.rs b/crates/eframe/src/native/run.rs index af8d3aed6fb..00bc23bac3c 100644 --- a/crates/eframe/src/native/run.rs +++ b/crates/eframe/src/native/run.rs @@ -675,7 +675,11 @@ mod glow_integration { glutin_window_context.on_resume(event_loop)?; if let Some(window) = &glutin_window_context.window { - epi_integration::apply_native_options_to_window(window, native_options); + epi_integration::apply_native_options_to_window( + window, + native_options, + window_settings, + ); } let gl = unsafe { @@ -1128,7 +1132,11 @@ mod wgpu_integration { let window_builder = epi_integration::window_builder(event_loop, title, native_options, window_settings); let window = window_builder.build(event_loop)?; - epi_integration::apply_native_options_to_window(&window, native_options); + epi_integration::apply_native_options_to_window( + &window, + native_options, + window_settings, + ); Ok(window) } diff --git a/crates/egui-winit/src/window_settings.rs b/crates/egui-winit/src/window_settings.rs index 0137af22e85..5d685765eb6 100644 --- a/crates/egui-winit/src/window_settings.rs +++ b/crates/egui-winit/src/window_settings.rs @@ -1,11 +1,13 @@ /// Can be used to store native window settings (position and size). -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Default)] #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))] +#[cfg_attr(feature = "serde", serde(default))] pub struct WindowSettings { - /// Position of window in physical pixels. This is either - /// the inner or outer position depending on the platform. - /// See [`winit::window::WindowBuilder::with_position`] for details. - position: Option, + /// Position of window content in physical pixels. + inner_position_pixels: Option, + + /// Position of window frame/titlebar in physical pixels. + outer_position_pixels: Option, fullscreen: bool, @@ -16,22 +18,20 @@ pub struct WindowSettings { impl WindowSettings { pub fn from_display(window: &winit::window::Window) -> Self { let inner_size_points = window.inner_size().to_logical::(window.scale_factor()); - let position = if cfg!(macos) { - // MacOS uses inner position when positioning windows. - window - .inner_position() - .ok() - .map(|p| egui::pos2(p.x as f32, p.y as f32)) - } else { - // Other platforms use the outer position. - window - .outer_position() - .ok() - .map(|p| egui::pos2(p.x as f32, p.y as f32)) - }; + + let inner_position_pixels = window + .inner_position() + .ok() + .map(|p| egui::pos2(p.x as f32, p.y as f32)); + + let outer_position_pixels = window + .outer_position() + .ok() + .map(|p| egui::pos2(p.x as f32, p.y as f32)); Self { - position, + inner_position_pixels, + outer_position_pixels, fullscreen: window.fullscreen().is_some(), @@ -46,19 +46,21 @@ impl WindowSettings { self.inner_size_points } - pub fn initialize_window( + pub fn initialize_window_builder( &self, mut window: winit::window::WindowBuilder, ) -> winit::window::WindowBuilder { - // If the app last ran on two monitors and only one is now connected, then - // the given position is invalid. - // If this happens on Mac, the window is clamped into valid area. - // If this happens on Windows, the clamping behavior is managed by the function - // clamp_window_to_sane_position. - if let Some(pos) = self.position { + // `WindowBuilder::with_position` expects inner position in Macos, and outer position elsewhere + // See [`winit::window::WindowBuilder::with_position`] for details. + let pos_px = if cfg!(target_os = "macos") { + self.inner_position_pixels + } else { + self.outer_position_pixels + }; + if let Some(pos_px) = pos_px { window = window.with_position(winit::dpi::PhysicalPosition { - x: pos.x as f64, - y: pos.y as f64, + x: pos_px.x as f64, + y: pos_px.y as f64, }); } @@ -77,68 +79,103 @@ impl WindowSettings { } } - pub fn clamp_to_sane_values(&mut self, max_size: egui::Vec2) { + pub fn initialize_window(&self, window: &winit::window::Window) { + if cfg!(target_os = "macos") { + // Mac sometimes has problems restoring the window to secondary monitors + // using only `WindowBuilder::with_position`, so we need this extra step: + if let Some(pos) = self.outer_position_pixels { + window.set_outer_position(winit::dpi::PhysicalPosition { x: pos.x, y: pos.y }); + } + } + } + + pub fn clamp_size_to_sane_values(&mut self, largest_monitor_size_points: egui::Vec2) { use egui::NumExt as _; if let Some(size) = &mut self.inner_size_points { - // Prevent ridiculously small windows + // Prevent ridiculously small windows: let min_size = egui::Vec2::splat(64.0); *size = size.at_least(min_size); - *size = size.at_most(max_size); + + // Make sure we don't try to create a window larger than the largest monitor + // because on Linux that can lead to a crash. + *size = size.at_most(largest_monitor_size_points); } } - pub fn clamp_window_to_sane_position( + pub fn clamp_position_to_monitors( &mut self, event_loop: &winit::event_loop::EventLoopWindowTarget, ) { - if let (Some(position), Some(inner_size_points)) = - (&mut self.position, &self.inner_size_points) - { - let monitors = event_loop.available_monitors(); - // default to primary monitor, in case the correct monitor was disconnected. - let mut active_monitor = if let Some(active_monitor) = event_loop - .primary_monitor() - .or_else(|| event_loop.available_monitors().next()) - { - active_monitor - } else { - return; // no monitors 🤷 - }; - for monitor in monitors { - let monitor_x_range = (monitor.position().x - inner_size_points.x as i32) - ..(monitor.position().x + monitor.size().width as i32); - let monitor_y_range = (monitor.position().y - inner_size_points.y as i32) - ..(monitor.position().y + monitor.size().height as i32); - - if monitor_x_range.contains(&(position.x as i32)) - && monitor_y_range.contains(&(position.y as i32)) - { - active_monitor = monitor; - } - } + // If the app last ran on two monitors and only one is now connected, then + // the given position is invalid. + // If this happens on Mac, the window is clamped into valid area. + // If this happens on Windows, the window becomes invisible to the user 🤦‍♂️ + // So on Windows we clamp the position to the monitor it is on. + if !cfg!(target_os = "windows") { + return; + } - let mut inner_size_pixels = *inner_size_points * (active_monitor.scale_factor() as f32); - // Add size of title bar. This is 32 px by default in Win 10/11. - if cfg!(target_os = "windows") { - inner_size_pixels += - egui::Vec2::new(0.0, 32.0 * active_monitor.scale_factor() as f32); - } - let monitor_position = egui::Pos2::new( - active_monitor.position().x as f32, - active_monitor.position().y as f32, - ); - let monitor_size = egui::Vec2::new( - active_monitor.size().width as f32, - active_monitor.size().height as f32, - ); - - // Window size cannot be negative or the subsequent `clamp` will panic. - let window_size = (monitor_size - inner_size_pixels).max(egui::Vec2::ZERO); - // To get the maximum position, we get the rightmost corner of the display, then - // subtract the size of the window to get the bottom right most value window.position - // can have. - *position = position.clamp(monitor_position, monitor_position + window_size); + let Some(inner_size_points) = self.inner_size_points else { return; }; + + if let Some(pos_px) = &mut self.inner_position_pixels { + clamp_pos_to_monitors(event_loop, inner_size_points, pos_px); + } + if let Some(pos_px) = &mut self.outer_position_pixels { + clamp_pos_to_monitors(event_loop, inner_size_points, pos_px); + } + } +} + +fn clamp_pos_to_monitors( + event_loop: &winit::event_loop::EventLoopWindowTarget, + window_size_pts: egui::Vec2, + position_px: &mut egui::Pos2, +) { + let monitors = event_loop.available_monitors(); + + // default to primary monitor, in case the correct monitor was disconnected. + let mut active_monitor = if let Some(active_monitor) = event_loop + .primary_monitor() + .or_else(|| event_loop.available_monitors().next()) + { + active_monitor + } else { + return; // no monitors 🤷 + }; + + for monitor in monitors { + let window_size_px = window_size_pts * (monitor.scale_factor() as f32); + let monitor_x_range = (monitor.position().x - window_size_px.x as i32) + ..(monitor.position().x + monitor.size().width as i32); + let monitor_y_range = (monitor.position().y - window_size_px.y as i32) + ..(monitor.position().y + monitor.size().height as i32); + + if monitor_x_range.contains(&(position_px.x as i32)) + && monitor_y_range.contains(&(position_px.y as i32)) + { + active_monitor = monitor; } } + + let mut window_size_px = window_size_pts * (active_monitor.scale_factor() as f32); + // Add size of title bar. This is 32 px by default in Win 10/11. + if cfg!(target_os = "windows") { + window_size_px += egui::Vec2::new(0.0, 32.0 * active_monitor.scale_factor() as f32); + } + let monitor_position = egui::Pos2::new( + active_monitor.position().x as f32, + active_monitor.position().y as f32, + ); + let monitor_size_px = egui::Vec2::new( + active_monitor.size().width as f32, + active_monitor.size().height as f32, + ); + + // Window size cannot be negative or the subsequent `clamp` will panic. + let window_size = (monitor_size_px - window_size_px).max(egui::Vec2::ZERO); + // To get the maximum position, we get the rightmost corner of the display, then + // subtract the size of the window to get the bottom right most value window.position + // can have. + *position_px = position_px.clamp(monitor_position, monitor_position + window_size); }