Merge pull request #468 from carkrueger/fix/cascade-on-real-monitors

window: cascade new windows on the real monitor

crates/window/README.md

@@ -99,6 +99,7 @@ window bookkeeping. The core has no egui dependency.
 | `WindowList<T>` | Open windows with the app's per-window data `T`. Opening order (stable, for menus) and most-recent order; the first in most-recent order is the active window, which menu commands go to. `close` returns the data and, when the active window closed, the window to focus next. `find` returns the most recent window whose data matches, e.g. the window already showing a file. |
 | `WindowId` | A window's id: `Copy`, `Hash`, `Ord`, from one process-wide counter, so never reused while the app runs. Use it as the key for per-window state (UI state, pending dialogs). |
 | `cascade` | Places a new window one step (about a title bar, `CASCADE_STEP`) right of and below the most recent window. Each axis that would run off the bounds (monitor or work area) starts over at the bounds' edge. |
+| `monitor_of` | The monitor a window is on: the one showing most of it or, when none shows it, the nearest. |
 | `WindowMenu` | The Window menu (macOS) or View › Switch Windows list: (id, title, dirty, active) per window, in opening order. Windows on the same document are titled `Name:1`, `Name:2`, numbered by opening order; a document with one window keeps its plain name. The app supplies each window's document key, name and dirty flag. |
 
 With the `egui` feature, `Viewports` shows the windows as egui immediate viewports:
@@ -109,6 +110,11 @@ With the `egui` feature, `Viewports` shows the windows as egui immediate viewpor
 - New windows get `decorate` (the frame decision above) and a cascade position from the most
   recently focused window. On Wayland, where windows have no known position, the compositor places
   them.
+- Give the driver the monitors with `set_monitors`, in physical pixels from `available_monitors()`:
+  egui reports only a monitor's size, so without them the driver guesses that monitors tile the
+  desktop, which is wrong when monitors differ in size or sit at other offsets. It asks only when
+  it places a new window. A window keeps to the monitor of the window before it, at any size,
+  place or scale; `set_bounds` overrides that with an area the app chooses.
 - The root viewport stays the app's. `set_root(Some(id))` draws window `id` in it. When the root's
   window closes while others stay open, `adopt` moves another window into the root viewport (its
   position, size and focus); that window's id and data stay the same.

crates/window/src/lib.rs

@@ -138,7 +138,7 @@ mod viewports;
 
 pub use list::{Closed, WindowId, WindowList};
 pub use menu::{Document, MenuEntry, WindowMenu};
-pub use place::{CASCADE_STEP, Rect, cascade};
+pub use place::{CASCADE_STEP, Rect, cascade, monitor_of};
 #[cfg(feature = "egui")]
 pub use viewports::{Viewports, WindowEvent, child_viewport_id};
 

crates/window/src/place.rs

@@ -21,6 +21,12 @@ impl Rect {
     pub fn bottom(&self) -> f32 {
         self.y + self.height
     }
+
+    /// The rectangle with its position and size times `factor`: from points to physical pixels
+    /// and back.
+    pub fn scaled(&self, factor: f32) -> Self {
+        Self::new(self.x * factor, self.y * factor, self.width * factor, self.height * factor)
+    }
 }
 
 #[cfg(feature = "egui")]
@@ -54,12 +60,62 @@ pub fn cascade(previous: Rect, size: [f32; 2], bounds: Rect, step: f32) -> Rect
     Rect::new(axis(previous.x, size[0], bounds.x, bounds.right()), axis(previous.y, size[1], bounds.y, bounds.bottom()), size[0], size[1])
 }
 
+/// The monitor `window` is on: the one showing most of it or, when none shows it, the nearest.
+/// By area, not by a corner: on Windows a maximized window's corner is outside its monitor. `None`
+/// only when there are no monitors.
+pub fn monitor_of(window: Rect, monitors: &[Rect]) -> Option<Rect> {
+    let shown = |m: &Rect| {
+        let width = window.right().min(m.right()) - window.x.max(m.x);
+        let height = window.bottom().min(m.bottom()) - window.y.max(m.y);
+        width.max(0.0) * height.max(0.0)
+    };
+    let gap = |m: &Rect| {
+        let across = (m.x - window.right()).max(window.x - m.right()).max(0.0);
+        let down = (m.y - window.bottom()).max(window.y - m.bottom()).max(0.0);
+        across * across + down * down
+    };
+    monitors.iter().copied().max_by(|a, b| shown(a).total_cmp(&shown(b)).then_with(|| gap(b).total_cmp(&gap(a))))
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
     const SCREEN: Rect = Rect::new(0.0, 0.0, 1000.0, 800.0);
 
+    #[test]
+    fn a_window_is_on_the_monitor_showing_most_of_it() {
+        let left = Rect::new(-1920.0, 200.0, 1920.0, 1080.0);
+        let main = Rect::new(0.0, 0.0, 3840.0, 1600.0);
+        let below = Rect::new(718.0, 1600.0, 1280.0, 720.0);
+        let desktop = [left, main, below];
+        assert_eq!(monitor_of(Rect::new(-8.0, -8.0, 3856.0, 1576.0), &desktop), Some(main), "maximized, its corner outside");
+        assert_eq!(monitor_of(Rect::new(-1928.0, 192.0, 1936.0, 1056.0), &desktop), Some(left), "maximized, on the left");
+        assert_eq!(monitor_of(Rect::new(800.0, 1700.0, 400.0, 300.0), &desktop), Some(below), "offset and smaller");
+        assert_eq!(monitor_of(Rect::new(-100.0, 300.0, 1000.0, 500.0), &desktop), Some(main), "across two, mostly on one");
+        assert_eq!(monitor_of(Rect::new(3400.0, 100.0, 800.0, 600.0), &[main]), Some(main), "past the right edge");
+    }
+
+    #[test]
+    fn a_window_no_monitor_shows_goes_to_the_nearest() {
+        let main = Rect::new(0.0, 0.0, 1920.0, 1080.0);
+        let right = Rect::new(1920.0, 0.0, 1920.0, 1080.0);
+        assert_eq!(monitor_of(Rect::new(-3840.0, -1600.0, 1456.0, 959.0), &[right, main]), Some(main));
+        assert_eq!(monitor_of(Rect::new(5000.0, 100.0, 800.0, 600.0), &[main, right]), Some(right));
+        assert_eq!(monitor_of(Rect::new(0.0, 0.0, 800.0, 600.0), &[]), None);
+    }
+
+    #[test]
+    fn nonsense_numbers_still_give_a_monitor() {
+        let main = Rect::new(0.0, 0.0, 1920.0, 1080.0);
+        assert_eq!(monitor_of(Rect::new(f32::NAN, f32::INFINITY, f32::NAN, 1.0), &[main]), Some(main));
+    }
+
+    #[test]
+    fn scaling_changes_position_and_size() {
+        assert_eq!(Rect::new(10.0, 20.0, 30.0, 40.0).scaled(2.0), Rect::new(20.0, 40.0, 60.0, 80.0));
+    }
+
     #[test]
     fn steps_down_and_right() {
         let prev = Rect::new(100.0, 50.0, 400.0, 300.0);

crates/window/src/viewports.rs

@@ -5,7 +5,7 @@ use std::collections::HashMap;
 use egui::{Context, Ui, ViewportBuilder, ViewportCommand, ViewportId};
 
 use crate::list::{WindowId, WindowList};
-use crate::place::{CASCADE_STEP, Rect, cascade};
+use crate::place::{CASCADE_STEP, Rect, cascade, monitor_of};
 use crate::{Frame, decorate};
 
 /// Something that happened to a window this frame. The app decides what follows.
@@ -28,6 +28,8 @@ struct Seen {
     focused: bool,
     inner_size: Option<egui::Vec2>,
     monitor_size: Option<egui::Vec2>,
+    /// Physical pixels per point: the window reports its rectangle in points of its own scale.
+    scale: Option<f32>,
     /// The cascade position chosen when the window was created; repeated every frame so egui sees
     /// an unchanged builder.
     position: Option<egui::Pos2>,
@@ -46,6 +48,7 @@ pub struct Viewports {
     root: Option<WindowId>,
     step: f32,
     bounds: Option<Rect>,
+    monitors: Option<Box<dyn Fn() -> Vec<Rect>>>,
     seen: HashMap<WindowId, Seen>,
 }
 
@@ -53,7 +56,7 @@ impl Viewports {
     /// `frame` is the app's frame decision ([`crate::Choice::probe`]), applied to every new
     /// window's builder.
     pub fn new(frame: Frame) -> Self {
-        Self { frame, root: None, step: CASCADE_STEP, bounds: None, seen: HashMap::new() }
+        Self { frame, root: None, step: CASCADE_STEP, bounds: None, monitors: None, seen: HashMap::new() }
     }
 
     /// The cascade offset for new windows, in points. Defaults to [`CASCADE_STEP`].
@@ -63,11 +66,21 @@ impl Viewports {
     }
 
     /// The area new windows cascade within, when the app knows the work area. Without it the
-    /// driver uses the size of the monitor the previous window is on, as egui reports it.
+    /// driver uses the monitor the previous window is on: from [`Viewports::set_monitors`], or
+    /// else guessed from the size egui reports.
     pub fn set_bounds(&mut self, bounds: Option<Rect>) {
         self.bounds = bounds;
     }
 
+    /// How to list the desktop's monitors, in physical pixels: `eframe`'s
+    /// `CreationContext::winit_window()` gives them through `available_monitors()`. New windows
+    /// then cascade on the real monitor of the window before them, whatever the monitors' sizes,
+    /// places and scales; egui reports only a monitor's size, which can't tell any of that.
+    /// Asked only when a new window is placed.
+    pub fn set_monitors(&mut self, monitors: impl Fn() -> Vec<Rect> + 'static) {
+        self.monitors = Some(Box::new(monitors));
+    }
+
     /// The window drawn in the root viewport.
     pub fn root(&self) -> Option<WindowId> {
         self.root
@@ -177,11 +190,12 @@ impl Viewports {
 
     /// Take in window `id`'s viewport state for this frame.
     fn record<T>(&mut self, snapshot: Snapshot, windows: &mut WindowList<T>, id: WindowId, events: &mut Vec<WindowEvent>) {
-        let Snapshot { close, focused, outer, inner, monitor } = snapshot;
+        let Snapshot { close, focused, outer, inner, monitor, scale } = snapshot;
         windows.set_outer_rect(id, outer.map(Rect::from));
         let seen = self.seen.entry(id).or_default();
         seen.inner_size = inner.map(|r| r.size()).or(seen.inner_size);
         seen.monitor_size = monitor.or(seen.monitor_size);
+        seen.scale = Some(scale);
         if focused && !seen.focused {
             windows.focus(id);
             events.push(WindowEvent::Focused(id));
@@ -203,13 +217,22 @@ impl Viewports {
             .map(|inner| egui::vec2(previous.width - inner.x, previous.height - inner.y))
             .unwrap_or_default();
         let size = viewport.inner_size.map(|s| s + frame).unwrap_or(egui::vec2(previous.width, previous.height));
-        let bounds = self.bounds.or_else(|| {
-            let monitor = previous_id.and_then(|w| self.seen.get(&w)).and_then(|s| s.monitor_size)?;
-            Some(monitor_bounds(previous, monitor))
-        })?;
+        let seen = previous_id.and_then(|w| self.seen.get(&w));
+        let bounds = self
+            .bounds
+            .or_else(|| self.real_monitor(previous, seen.and_then(|s| s.scale)))
+            .or_else(|| Some(monitor_bounds(previous, seen?.monitor_size?)))?;
         let placed = cascade(previous, [size.x, size.y], bounds, self.step);
         Some(egui::pos2(placed.x, placed.y))
     }
+
+    /// The real monitor of the window at `rect`, in that window's points. A window's rectangle is
+    /// in points of its own scale, so the monitor is picked in physical pixels.
+    fn real_monitor(&self, rect: Rect, scale: Option<f32>) -> Option<Rect> {
+        let monitors = self.monitors.as_ref()?();
+        let scale = scale.filter(|s| s.is_finite() && *s > 0.0).unwrap_or(1.0);
+        Some(monitor_of(rect.scaled(scale), &monitors)?.scaled(1.0 / scale))
+    }
 }
 
 /// A viewport's state for this frame, read inside its own pass (egui's input is per viewport).
@@ -219,6 +242,7 @@ struct Snapshot {
     outer: Option<egui::Rect>,
     inner: Option<egui::Rect>,
     monitor: Option<egui::Vec2>,
+    scale: f32,
 }
 
 impl Snapshot {
@@ -231,6 +255,7 @@ impl Snapshot {
                 outer: v.outer_rect,
                 inner: v.inner_rect,
                 monitor: v.monitor_size,
+                scale: i.pixels_per_point(),
             }
         })
     }
@@ -241,11 +266,17 @@ pub fn child_viewport_id(id: WindowId) -> ViewportId {
     ViewportId::from_hash_of(("craft_window", id.get()))
 }
 
-/// egui reports a monitor's size but not its origin. Assume monitors of that size tile the
-/// desktop and take the tile holding the window's top-left corner, which is right for a single
-/// monitor and for side-by-side monitors of equal size.
+/// How far past its monitor's corner a window's frame can reach, in points: on Windows a maximized
+/// window's corner is about 8 points outside its monitor.
+const FRAME_OVERSHOOT: f32 = 16.0;
+
+/// The monitor to assume when the app doesn't list them ([`Viewports::set_monitors`]). egui reports
+/// a monitor's size but not its origin. Assume monitors of that size tile the desktop and take the
+/// tile holding the window's top-left corner, moved in by [`FRAME_OVERSHOOT`] so a maximized
+/// window's corner doesn't count as the monitor before its own. That is right for a single monitor
+/// and for side-by-side monitors of equal size, and only a guess for any other desktop.
 fn monitor_bounds(window: Rect, monitor: egui::Vec2) -> Rect {
-    let tile = |pos: f32, len: f32| if len > 0.0 { (pos / len).floor() * len } else { 0.0 };
+    let tile = |pos: f32, len: f32| if len > 0.0 { ((pos + FRAME_OVERSHOOT) / len).floor() * len } else { 0.0 };
     Rect::new(tile(window.x, monitor.x), tile(window.y, monitor.y), monitor.x, monitor.y)
 }
 
@@ -392,6 +423,108 @@ mod tests {
         assert_eq!(position, None, "the first window is placed by the desktop");
     }
 
+    /// Opens a second window from one at `outer` on `monitor_size`, in `scale`, and returns where
+    /// the second window goes.
+    fn opened_from(outer: egui::Rect, monitor_size: egui::Vec2, scale: f32, driver: &mut Viewports) -> Option<egui::Pos2> {
+        let ctx = backend();
+        let mut windows = WindowList::new();
+        let a = windows.open("a");
+        let va = child_viewport_id(a);
+        info(va, |v| {
+            v.outer_rect = Some(outer);
+            v.inner_rect = Some(outer);
+            v.monitor_size = Some(monitor_size);
+            v.native_pixels_per_point = Some(scale);
+        });
+        frame(&ctx, driver, &mut windows);
+        let b = windows.open("b");
+        frame(&ctx, driver, &mut windows);
+        BUILDERS.with(|m| m.borrow().get(&child_viewport_id(b)).and_then(|b| b.position))
+    }
+
+    fn rect(x: f32, y: f32, w: f32, h: f32) -> egui::Rect {
+        egui::Rect::from_min_size(egui::pos2(x, y), egui::vec2(w, h))
+    }
+
+    /// A monitor whose place isn't a multiple of its size: no guess from the size can find it.
+    #[test]
+    fn a_new_window_stays_on_a_monitor_of_another_size_and_place() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 2560.0, 1440.0), Rect::new(2560.0, 200.0, 1920.0, 1080.0)]);
+        // A guess takes 1920–3840 for the monitor, so the step past 3840 would wrap to the first one.
+        let place = opened_from(rect(3500.0, 300.0, 400.0, 300.0), egui::vec2(1920.0, 1080.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(3528.0, 328.0)));
+    }
+
+    /// A monitor below a wider one, offset sideways: the step past its bottom edge wraps to the
+    /// monitor's own top, not to a line of the size's multiples.
+    #[test]
+    fn a_new_window_wraps_at_the_edges_of_the_real_monitor() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 3440.0, 1440.0), Rect::new(718.0, 1440.0, 1920.0, 1080.0)]);
+        let place = opened_from(rect(900.0, 2200.0, 400.0, 300.0), egui::vec2(1920.0, 1080.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(928.0, 1440.0)));
+    }
+
+    #[test]
+    fn a_new_window_beside_a_maximized_one_stays_on_its_monitor() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 3840.0, 1600.0)]);
+        let place = opened_from(rect(-8.0, -8.0, 3856.0, 1576.0), egui::vec2(3840.0, 1600.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(20.0, 20.0)));
+    }
+
+    /// Most of the window past the right edge: its centre is off the monitor, its monitor isn't.
+    #[test]
+    fn a_new_window_from_one_hanging_off_the_edge_stays_on_the_monitor() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 1920.0, 1080.0)]);
+        let place = opened_from(rect(1600.0, 100.0, 800.0, 600.0), egui::vec2(1920.0, 1080.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(0.0, 128.0)));
+    }
+
+    /// A window off every monitor: the new one still opens on one.
+    #[test]
+    fn a_new_window_from_one_off_screen_opens_on_a_monitor() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 1920.0, 1080.0)]);
+        let place = opened_from(rect(-1920.0, -1080.0, 800.0, 600.0), egui::vec2(1920.0, 1080.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(0.0, 0.0)));
+    }
+
+    /// At twice the scale, on the second monitor: physically at (3000, 100), and that monitor is
+    /// 960–2880 in the window's points. Read as if points were pixels the window is on the first
+    /// monitor, and a guess from the size takes 0–1920; either would wrap the new window to 0.
+    #[test]
+    fn a_new_window_stays_on_a_monitor_of_another_scale() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 1920.0, 1080.0), Rect::new(1920.0, 0.0, 3840.0, 2160.0)]);
+        let place = opened_from(rect(1500.0, 50.0, 400.0, 300.0), egui::vec2(1920.0, 1080.0), 2.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(1528.0, 78.0)));
+    }
+
+    /// `set_bounds` is the app's own word: it wins over the monitors.
+    #[test]
+    fn set_bounds_wins_over_the_monitors() {
+        let mut driver = Viewports::new(Frame::Desktop);
+        driver.set_monitors(|| vec![Rect::new(0.0, 0.0, 1920.0, 1080.0)]);
+        driver.set_bounds(Some(Rect::new(100.0, 100.0, 1000.0, 800.0)));
+        let place = opened_from(rect(5.0, 5.0, 400.0, 300.0), egui::vec2(1920.0, 1080.0), 1.0, &mut driver);
+        assert_eq!(place, Some(egui::pos2(100.0, 100.0)));
+    }
+
+    /// An app that doesn't list its monitors: a maximized window's frame doesn't move the guess
+    /// to a monitor that isn't there, and a window past the right edge stays on its monitor.
+    #[test]
+    fn without_the_monitors_a_maximized_window_still_finds_its_own() {
+        let monitor = egui::vec2(1920.0, 1080.0);
+        let mut driver = Viewports::new(Frame::Desktop);
+        assert_eq!(opened_from(rect(-8.0, -8.0, 1936.0, 1056.0), monitor, 1.0, &mut driver), Some(egui::pos2(20.0, 20.0)));
+        let mut driver = Viewports::new(Frame::Desktop);
+        assert_eq!(opened_from(rect(1600.0, 100.0, 800.0, 600.0), monitor, 1.0, &mut driver), Some(egui::pos2(0.0, 128.0)));
+        assert_eq!(monitor_bounds(Rect::new(-1928.0, -8.0, 1936.0, 1056.0), monitor), Rect::new(-1920.0, 0.0, 1920.0, 1080.0));
+    }
+
     #[test]
     fn monitor_bounds_follow_the_window() {
         let monitor = egui::vec2(1920.0, 1080.0);