Position persistence and sane clamping to still-available monitors fo…

…r Windows (emilk#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 <[email protected]>

* Update window_settings.rs

Satisfy CI Error

* clippy

---------

Co-authored-by: Emil Ernerfeldt <[email protected]>

crates/eframe/src/native/epi_integration.rs

@@ -108,6 +108,8 @@ pub fn build_window<E>(
     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 {

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)).
 

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<E>(
+        &mut self,
+        event_loop: &winit::event_loop::EventLoopWindowTarget<E>,
+    ) {
+        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,
+            );
+        }
+    }
 }