From 53f8e4049fce7a18fc4d4a016d3c4e975cdbe2cd Mon Sep 17 00:00:00 2001 From: Sheldon M <46656642+Shel-M@users.noreply.github.com> Date: Sat, 4 Feb 2023 07:33:32 -0700 Subject: [PATCH] Position persistence and sane clamping to still-available monitors for Windows (#2583) * Attempt to fix monitor clamping on Windows so window positions can be restored between sessions. * Missed a change. * Renamed variables, reorganized some lines of code, and added some more comments. * Cargo fmt run * Updated CHANGELOG.md to briefly describe my change * Updated CHANGELOG.md to briefly describe my change * Applied suggested fixes from emilk Discovered an issue where putting the monitor off a non-primary monitor to the left causes the position to be off the monitor x and y range, clamping to the primary instead of the non-primary. * Fix for matching negative restored window positions. Should clamp if any part of the window had been visible on a remaining monitor. * Apparently compiler attributes on statements have been marked unstable. Rather than just wrap in blocks, I kind of prefer the more explicit if cfg! call for line 114. CHANGELOG.md - correct a missing paren I noticed * I was being silly, I don't need to clone inner_size_points on line 112 * Cargo fmt run * Update crates/egui-winit/CHANGELOG.md emilk suggested changelog formatting Co-authored-by: Emil Ernerfeldt * Update window_settings.rs Satisfy CI Error * clippy --------- Co-authored-by: Emil Ernerfeldt --- crates/eframe/src/native/epi_integration.rs | 2 + crates/egui-winit/CHANGELOG.md | 1 + crates/egui-winit/src/window_settings.rs | 69 ++++++++++++++++++--- 3 files changed, 62 insertions(+), 10 deletions(-) diff --git a/crates/eframe/src/native/epi_integration.rs b/crates/eframe/src/native/epi_integration.rs index faa26537..0ec6f779 100644 --- a/crates/eframe/src/native/epi_integration.rs +++ b/crates/eframe/src/native/epi_integration.rs @@ -108,6 +108,8 @@ pub fn build_window( 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.inner_size_points() } else { diff --git a/crates/egui-winit/CHANGELOG.md b/crates/egui-winit/CHANGELOG.md index 91b24cf1..e3cf1a26 100644 --- a/crates/egui-winit/CHANGELOG.md +++ b/crates/egui-winit/CHANGELOG.md @@ -3,6 +3,7 @@ All notable changes to the `egui-winit` integration will be noted in this file. ## Unreleased +* Fixed window position persistence for Windows ([#2583](https://github.com/emilk/egui/issues/2583)). * Update to `winit` 0.28, adding support for mac trackpad zoom ([#2654](https://github.com/emilk/egui/pull/2654)). * Remove the `screen_reader` feature. Use the `accesskit` feature flag instead ([#2669](https://github.com/emilk/egui/pull/2669)). diff --git a/crates/egui-winit/src/window_settings.rs b/crates/egui-winit/src/window_settings.rs index d7d099c8..13efc99c 100644 --- a/crates/egui-winit/src/window_settings.rs +++ b/crates/egui-winit/src/window_settings.rs @@ -53,16 +53,13 @@ impl WindowSettings { // 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 is hidden and very difficult to find. - // So we don't restore window positions on Windows. - let try_restore_position = !cfg!(target_os = "windows"); - if try_restore_position { - if let Some(pos) = self.position { - window = window.with_position(winit::dpi::PhysicalPosition { - x: pos.x as f64, - y: pos.y as f64, - }); - } + // If this happens on Windows, the clamping behavior is managed by the function + // clamp_window_to_sane_position. + if let Some(pos) = self.position { + window = window.with_position(winit::dpi::PhysicalPosition { + x: pos.x as f64, + y: pos.y as f64, + }); } if let Some(inner_size_points) = self.inner_size_points { @@ -90,4 +87,56 @@ impl WindowSettings { *size = size.at_most(max_size); } } + + pub fn clamp_window_to_sane_position( + &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; + } + } + + 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 = active_monitor.size(); + *position = position.clamp( + monitor_position, + // 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. + monitor_position + + egui::Vec2::new(monitor_size.width as f32, monitor_size.height as f32) + - inner_size_pixels, + ); + } + } }