collective
Browse / github.com/storytold/lightcraft / patch
LR-BEHAV-WINDOW: remember where the window was, not only its size (#2967)
Window location/position/sizing across multiple monitors is lost Open Lightcraft on a Mac with dual monitors. It opens on the default monitor. Drag the window to a second monitor. Close and reopen. The window reopens on the default monitor. Expected behavior is for it to open on the monitor it was on when it was last quit, retaining the window sizing and positioning it had. Tested v0.14.0. on an M1 MacBook Pro.
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- emmertex (imported from pull request #3019, open)
- Base
387bed199fca44d76af5aa1c24dd7cc3de4c8db2- Size
- +98 −19 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
830c8617843f95d2ae361172
How it went
No other operator has reported using it yet.
Diff
raw · apps/lightcraft/src/main.rs crates/ui-egui/src/window_place.rs
@@ -231,10 +231,14 @@ fn config_dir() -> Option<std::path::PathBuf> { } /// The window at the size it was last used at, zoomed if it was (`window`), or at the usual size-/// when nothing is remembered. A zoomed window is given the size it goes back to. Where it-/// opens is the system's to say.+/// when nothing is remembered, where it was left if that is known (issue #2967). A zoomed window is+/// given the size and place it goes back to. fn as_remembered(builder: egui::ViewportBuilder, window: Option<lightcraft_ui_egui::window_place::WindowPlace>) -> egui::ViewportBuilder { let builder = builder.with_inner_size(window.map_or(lightcraft_ui_egui::window_place::DEFAULT_SIZE, |w| w.size));+ let builder = match window.and_then(|w| w.position) {+ Some(at) => builder.with_position(at),+ None => builder,+ }; // (only ever asked to zoom: asked not to, macOS un-zooms a window whose size fills the screen // to a size of its own choosing) if window.is_some_and(|w| w.zoomed) { builder.with_maximized(true) } else { builder }@@ -1520,11 +1524,17 @@ mod tests { use lightcraft_ui_egui::window_place::{DEFAULT_SIZE, WindowPlace}; let fresh = as_remembered(egui::ViewportBuilder::default(), None); assert_eq!((fresh.inner_size, fresh.position, fresh.maximized), (Some(egui::Vec2::from(DEFAULT_SIZE)), None, None));- let b = as_remembered(egui::ViewportBuilder::default(), Some(WindowPlace { size: [1300.0, 800.0], zoomed: true }));+ let b = as_remembered(egui::ViewportBuilder::default(), Some(WindowPlace { size: [1300.0, 800.0], zoomed: true, position: None })); assert_eq!((b.inner_size, b.position, b.maximized), (Some(egui::vec2(1300.0, 800.0)), None, Some(true))); // (not asked to un-zoom: macOS would then resize a window that fills its screen)- let b = as_remembered(egui::ViewportBuilder::default(), Some(WindowPlace { size: [1300.0, 800.0], zoomed: false }));+ let b = as_remembered(egui::ViewportBuilder::default(), Some(WindowPlace { size: [1300.0, 800.0], zoomed: false, position: None })); assert_eq!((b.inner_size, b.position, b.maximized), (Some(egui::vec2(1300.0, 800.0)), None, None));+ // where it was left (issue #2967)+ let b = as_remembered(+ egui::ViewportBuilder::default(),+ Some(WindowPlace { size: [1300.0, 800.0], zoomed: false, position: Some([-900.0, 40.0]) }),+ );+ assert_eq!((b.inner_size, b.position), (Some(egui::vec2(1300.0, 800.0)), Some(egui::pos2(-900.0, 40.0)))); } /// Given a window that rests at a size, when the settings are written and read again, then@@ -1541,7 +1551,7 @@ mod tests { let mut writer = PrefsWriter { path: Some(path.clone()), ..Default::default() }; writer.save(&app).unwrap(); assert!(!std::fs::read_to_string(&path).unwrap().contains("\"window\""), "nothing to remember yet");- let window = WindowPlace { size: [1300.0, 800.0], zoomed: true };+ let window = WindowPlace { size: [1300.0, 800.0], zoomed: true, position: None }; app.ui.window = Some(window); writer.save(&app).unwrap(); let (restored, warning, _) = load_prefs_at(&path, false);@@ -1560,11 +1570,11 @@ mod tests { // resized, and the app quits a moment later: the new size is what is written let mut writer = PrefsWriter { path: Some(path.clone()), ..Default::default() };- let rest = Some(WindowPlace { size: [1300.0, 800.0], zoomed: false });+ let rest = Some(WindowPlace { size: [1300.0, 800.0], zoomed: false, position: None }); writer.window.note(rest, &Seen { size: Some([1500.0, 900.0]), ..Default::default() }, 0.0); writer.window_since = std::time::Instant::now().checked_sub(std::time::Duration::from_millis(400)); assert!(writer.window_since.is_some());- assert_eq!(writer.window_at_exit(rest), Some(WindowPlace { size: [1500.0, 900.0], zoomed: false }));+ assert_eq!(writer.window_at_exit(rest), Some(WindowPlace { size: [1500.0, 900.0], zoomed: false, position: None })); assert_eq!(PrefsWriter::default().window_at_exit(rest), rest, "a window that was never seen"); let _ = std::fs::remove_dir_all(&d); }@@ -1590,14 +1600,14 @@ mod tests { for _ in 0..3 { pass(&mut |_| {}); }- app.ui.window = Some(WindowPlace { size: [f32::NAN, 800.0], zoomed: true });+ app.ui.window = Some(WindowPlace { size: [f32::NAN, 800.0], zoomed: true, position: None }); let asked = pass(&mut |ctx| writer.tick(&mut app, ctx)); assert_eq!(app.ui.window, None, "no window, and not yet at rest"); assert!(asked.is_some_and(|wait| wait <= std::time::Duration::from_secs(1)), "{asked:?}"); // a second later writer.window_since = writer.window_since.and_then(|since| since.checked_sub(std::time::Duration::from_secs(1))); pass(&mut |ctx| writer.tick(&mut app, ctx));- assert_eq!(app.ui.window, Some(WindowPlace { size: [1300.0, 800.0], zoomed: false }));+ assert_eq!(app.ui.window, Some(WindowPlace { size: [1300.0, 800.0], zoomed: false, position: None })); let asked = pass(&mut |ctx| writer.tick(&mut app, ctx)); assert!(asked.is_none_or(|wait| wait > std::time::Duration::from_secs(3600)), "at rest and remembered: {asked:?}"); }@@ -3,9 +3,11 @@ //! //! The size is the window's as it is when neither zoomed nor full screen: a zoomed window opens //! zoomed, and goes back to that size when it is un-zoomed. Full screen is not remembered (the-//! app opens as a window), and neither is where on the desktop the window was: the system places-//! it. A place can be on a display that is gone, and only the window system knows which-//! displays there are.+//! app opens as a window). Where on the desktop the window was (its outer top-left, in the+//! system's points) is remembered too and asked for when the window opens (issue #2967: macOS+//! otherwise opens it where it likes every time). A place that is no place (not a number, or+//! further out than any desktop reaches) is not kept, and a place the window system does not+//! report (Wayland) is never written: the system places the window there. //! //! Windows and Linux say whether a window is maximized. macOS says so only when the window opens //! and when the app zooms it itself, so there the window's size is gone by as well (`Unsaid`): a@@ -57,6 +59,10 @@ pub struct WindowPlace { /// Zoomed (macOS) / maximized. #[serde(default)] pub zoomed: bool,+ /// Its outer top-left while neither zoomed nor full screen, in the system's points (`None`:+ /// unknown, the system places it).+ #[serde(default, skip_serializing_if = "Option::is_none")]+ pub position: Option<[f32; 2]>, } fn size_is_one(size: [f32; 2]) -> bool {@@ -69,6 +75,12 @@ fn at_least_min(size: [f32; 2]) -> [f32; 2] { [size[0].max(MIN_SIZE[0]), size[1].max(MIN_SIZE[1])] } +/// A top-left a window can have: a number, and no further from the main display's corner than+/// the largest desktop reaches.+fn position_is_one(pos: [f32; 2]) -> bool {+ pos.iter().all(|p| p.is_finite() && (-MAX_SIDE..=MAX_SIDE).contains(p))+}+ fn same(a: [f32; 2], b: [f32; 2]) -> bool { (a[0] - b[0]).abs() <= SAME_WITHIN && (a[1] - b[1]).abs() <= SAME_WITHIN }@@ -88,7 +100,11 @@ impl WindowPlace { /// What the settings hold (which a person or another version may have written), as /// something a window can be: a size within bounds. `None` when the size is no size at all. pub fn checked(self) -> Option<WindowPlace> {- size_is_one(self.size).then(|| WindowPlace { size: at_least_min(self.size), zoomed: self.zoomed })+ size_is_one(self.size).then(|| WindowPlace {+ size: at_least_min(self.size),+ zoomed: self.zoomed,+ position: self.position.filter(|p| position_is_one(*p)),+ }) } /// Reads the window from the settings without letting it cost the rest of them: anything that@@ -107,6 +123,8 @@ pub struct Seen { pub size: Option<[f32; 2]>, /// The size of the screen it is on. pub screen: Option<[f32; 2]>,+ /// Its outer top-left, where the window system reports one (not Wayland).+ pub position: Option<[f32; 2]>, /// What the window system last said of its being zoomed / maximized. pub zoomed: bool, pub full_screen: bool,@@ -123,6 +141,7 @@ impl Seen { Seen { size: Some(scaled(content.size())).filter(|s| size_is_one(*s)), screen: info.monitor_size.map(scaled).filter(|s| size_is_one(*s)),+ position: info.outer_rect.map(|r| scaled(r.min.to_vec2())).filter(|p| position_is_one(*p)), zoomed: info.maximized == Some(true), full_screen: info.fullscreen == Some(true), minimized: info.minimized == Some(true),@@ -213,7 +232,11 @@ impl WindowWatch { if zoomed { // (its size now is the screen's, not the window's own: the size it goes back to stays) self.resting = None;- return Some(WindowPlace { size: remembered.map_or(DEFAULT_SIZE, |r| r.size), zoomed: true });+ return Some(WindowPlace {+ size: remembered.map_or(DEFAULT_SIZE, |r| r.size),+ zoomed: true,+ position: remembered.and_then(|r| r.position),+ }); } let Some(size) = seen.size else { self.resting = None;@@ -224,7 +247,9 @@ impl WindowWatch { self.resting = None; return remembered; }- let place = WindowPlace { size: at_least_min(size), zoomed: false };+ // (where no place is reported, the one remembered stays)+ let position = seen.position.or_else(|| remembered.and_then(|r| r.position));+ let place = WindowPlace { size: at_least_min(size), zoomed: false, position }; if Some(place) == remembered { self.resting = None; return remembered;@@ -329,7 +354,7 @@ mod tests { } fn place(size: [f32; 2], zoomed: bool) -> WindowPlace {- WindowPlace { size, zoomed }+ WindowPlace { size, zoomed, position: None } } /// Given a window that is resized and comes to rest, when it is watched, then its new size@@ -851,6 +876,35 @@ mod tests { assert_eq!(WindowWatch::told_live(true).at_exit(None, 3.0), None); } + /// Given a window that is moved (issue #2967), when it rests at its new place, then the place+ /// is remembered with its size; zoomed, the place it goes back to stays; and where the system+ /// reports no place (Wayland), the one remembered is kept.+ #[test]+ fn where_the_window_rests_is_remembered() {+ let at = |pos: [f32; 2]| Seen { position: Some(pos), ..window([1300.0, 800.0]) };+ let rest = Some(WindowPlace { position: Some([100.0, 80.0]), ..place([1300.0, 800.0], false) });+ let moved = Some(WindowPlace { position: Some([900.0, 300.0]), ..place([1300.0, 800.0], false) });+ let mut watch = WindowWatch::told_live(true);+ assert_eq!(watch.note(rest, &at([100.0, 80.0]), 0.0), rest);+ assert_eq!(watch.note(rest, &at([900.0, 300.0]), 1.0), rest, "not at rest yet");+ assert_eq!(watch.note(rest, &at([900.0, 300.0]), 1.6), moved);+ // quits just after a move: the place it was let go at+ let mut watch = WindowWatch::told_live(true);+ watch.note(rest, &at([900.0, 300.0]), 1.0);+ assert_eq!(watch.at_exit(rest, 1.3), moved);+ // zoomed: where it goes back to stays+ let mut watch = WindowWatch::told_live(true);+ let zoomed = Seen { zoomed: true, position: Some([0.0, 25.0]), ..window([2560.0, 1415.0]) };+ assert_eq!(watch.note(moved, &zoomed, 2.0), moved.map(|m| WindowPlace { zoomed: true, ..m }));+ // no place reported: the one remembered stays with a new size+ let mut watch = WindowWatch::told_live(true);+ watch.note(moved, &window([1500.0, 900.0]), 3.0);+ assert_eq!(+ watch.note(moved, &window([1500.0, 900.0]), 3.6),+ Some(WindowPlace { position: Some([900.0, 300.0]), ..place([1500.0, 900.0], false) })+ );+ }+ /// What egui holds of the window is read in the system's points: a size every platform /// reports (the rectangle the app draws in), brought back from the interface's own scale. #[test]@@ -860,7 +914,15 @@ mod tests { let content = Rect::from_min_size(pos2(0.0, 0.0), vec2(650.0, 400.0)); // (the interface at twice its size: egui's points are half the system's) let seen = Seen::of(&info, content, 2.0);- assert_eq!(seen, Seen { size: Some([1300.0, 800.0]), screen: Some([2560.0, 1440.0]), zoomed: true, full_screen: false, minimized: false });+ assert_eq!(+ seen,+ Seen { size: Some([1300.0, 800.0]), screen: Some([2560.0, 1440.0]), position: None, zoomed: true, full_screen: false, minimized: false }+ );+ // a window on a display left of the main one, its place in the system's points+ let placed = ViewportInfo { outer_rect: Some(Rect::from_min_size(pos2(-600.0, 50.0), vec2(650.0, 430.0))), ..info.clone() };+ assert_eq!(Seen::of(&placed, content, 2.0).position, Some([-1200.0, 100.0]));+ let lost = ViewportInfo { outer_rect: Some(Rect::from_min_size(pos2(f32::NAN, 0.0), vec2(1.0, 1.0))), ..info.clone() };+ assert_eq!(Seen::of(&lost, content, 1.0).position, None, "no place"); let away = ViewportInfo { maximized: Some(false), fullscreen: Some(true), minimized: Some(true), ..Default::default() }; let seen = Seen::of(&away, content, 1.0); assert!(!seen.zoomed && seen.full_screen && seen.minimized);@@ -886,8 +948,15 @@ mod tests { let read = |window: &str| serde_json::from_str::<Settings>(&format!(r#"{{"window":{window},"other":7}}"#)).map(|s| s.window); assert_eq!(read(r#"{"size":[1300.0,800.0],"zoomed":true}"#).unwrap(), Some(place([1300.0, 800.0], true))); assert_eq!(read(r#"{"size":[1300,800]}"#).unwrap(), Some(place([1300.0, 800.0], false)));- // (a place on the desktop, as a version that remembered one would write it, is passed over)- assert_eq!(read(r#"{"size":[1300,800],"position":[40,60],"zoomed":false}"#).unwrap(), Some(place([1300.0, 800.0], false)));+ // a place on the desktop is kept; one that is no place is dropped, and the size kept+ let at = |x: f32, y: f32| Some(WindowPlace { position: Some([x, y]), ..place([1300.0, 800.0], false) });+ assert_eq!(read(r#"{"size":[1300,800],"position":[40,60],"zoomed":false}"#).unwrap(), at(40.0, 60.0));+ assert_eq!(read(r#"{"size":[1300,800],"position":[-2000,60]}"#).unwrap(), at(-2000.0, 60.0));+ for no_place in [r#"[1e30,60]"#, r#"[40]"#, r#""left""#, r#"[40,-1e9]"#] {+ let got = read(&format!(r#"{{"size":[1300,800],"position":{no_place}}}"#)).unwrap();+ assert!(got.is_none() || got == Some(place([1300.0, 800.0], false)), "{no_place}: {got:?}");+ }+ assert_eq!(WindowPlace { position: Some([f32::NAN, 0.0]), ..place([1300.0, 800.0], false) }.checked(), Some(place([1300.0, 800.0], false))); assert_eq!(read(r#"{"size":[200,100]}"#).unwrap().map(|p| p.size), Some(MIN_SIZE), "too small to be the app's window"); for odd in [ "null",