diff options
| -rw-r--r-- | crates/core/src/manager/dragresize.rs | 53 | ||||
| -rw-r--r-- | crates/core/src/manager/tests.rs | 48 | ||||
| -rw-r--r-- | crates/wayland/src/state/toplevel.rs | 20 | ||||
| -rw-r--r-- | crates/wayland/src/window_memory.rs | 16 |
4 files changed, 135 insertions, 2 deletions
diff --git a/crates/core/src/manager/dragresize.rs b/crates/core/src/manager/dragresize.rs index cb151bd..bb7264f 100644 --- a/crates/core/src/manager/dragresize.rs +++ b/crates/core/src/manager/dragresize.rs @@ -395,6 +395,59 @@ impl WindowManager { /// `crates/wayland/src/window_memory.rs` restoring what was persisted /// from a *previous* session at startup, before any real drag/resize /// has happened this run. + /// Applies a remembered geometry to a window whose `app_id` was not + /// known when it was placed. + /// + /// `add_window` looks the store up by `app_id`, but a Wayland toplevel + /// role exists before its client has sent `set_app_id` - so at placement + /// time the id is usually the empty string, the lookup misses, and every + /// window falls through to a fresh cascade. That is why "windows do not + /// remember their size and position" and "windows spawn on top of each + /// other" were the same bug: the store was written correctly and read at + /// the one moment it could not match. + /// + /// Called again from the backend the moment a real `app_id` arrives, + /// which is still before the client's first buffer, so nothing has been + /// drawn at the wrong place yet. + /// + /// Only touches a window that is still sitting where the cascade put it + /// (`size_is_provisional`). A rule's explicit `geometry`, a maximize, a + /// dialog's centring and a client's own chosen size all clear that flag, + /// and each of them is a more specific decision than "wherever I last + /// left this app". Returns whether anything moved. + pub fn apply_remembered_geometry(&mut self, id: WindowId) -> bool { + let Some(w) = self.windows.get(&id) else { return false }; + if !w.size_is_provisional || w.is_dialog || w.maximized || w.fullscreen || w.app_id.is_empty() { + return false; + } + let Some((x, y, width, height)) = self.remembered_geometry.get(&w.app_id).copied() else { return false }; + let (min_w, min_h) = w.min_size; + let (width, height) = (width.max(min_w), height.max(min_h)); + // Same two-step as `add_window`: whichever monitor the remembered + // point actually lands on - checked against full geometry, so a + // spot under a bar still counts as on-screen - else the monitor the + // window is already on. Then clamped into that monitor's *usable* + // area, which is what keeps a window from reopening beneath a bar. + let monitor = self + .monitors + .iter() + .find(|m| m.full_geometry.contains_point(x, y)) + .or_else(|| self.monitors.iter().find(|m| m.id == w.monitor)) + .map(|m| (m.id, m.geometry)); + let Some((monitor_id, area)) = monitor else { return false }; + let Some(w) = self.windows.get_mut(&id) else { return false }; + w.monitor = monitor_id; + w.geometry.width = width; + w.geometry.height = height; + w.geometry.x = x.clamp(area.x, (area.right() - width as i32).max(area.x)); + w.geometry.y = y.clamp(area.y, (area.bottom() - height as i32).max(area.y)); + // A remembered size is a real preference, not the placeholder the + // cascade handed out, so the client no longer gets to replace it + // (`adopt_provisional_size`). + w.size_is_provisional = false; + true + } + pub fn set_remembered_geometry(&mut self, app_id: String, geometry: (i32, i32, u32, u32)) { self.remembered_geometry.insert(app_id, geometry); } diff --git a/crates/core/src/manager/tests.rs b/crates/core/src/manager/tests.rs index fdf0517..e20bb74 100644 --- a/crates/core/src/manager/tests.rs +++ b/crates/core/src/manager/tests.rs @@ -116,6 +116,54 @@ assert!(!win.size_is_provisional, "a deliberately remembered size must never be second-guessed by the client's own default"); } + /// The real shape of the bug: a Wayland toplevel exists before its + /// client sends `set_app_id`, so `add_window` searched the store for the + /// empty string and every window fell through to a fresh cascade. That + /// is why "windows do not remember their size" and "windows spawn on top + /// of each other" were one bug. + #[test] + fn a_window_that_learns_its_app_id_late_still_gets_its_remembered_geometry() { + let mut wm = wm_with_monitor(); + wm.set_remembered_geometry("late-app".to_string(), (400, 300, 500, 400)); + let id = wm.alloc_window_id(); + // Added with no app_id at all, exactly as a real toplevel arrives. + wm.add_window(Window::new(id, "late")); + assert!(wm.window(id).unwrap().size_is_provisional, "should have been cascaded, nothing to look up yet"); + + wm.window_mut(id).unwrap().app_id = "late-app".to_string(); + assert!(wm.apply_remembered_geometry(id)); + + let win = wm.window(id).unwrap(); + assert_eq!((win.geometry.x, win.geometry.y), (400, 300)); + assert_eq!((win.geometry.width, win.geometry.height), (500, 400)); + assert!(!win.size_is_provisional, "a restored size is a real preference, not a guess"); + } + + /// Every decision more specific than "wherever I last left this app" + /// has to survive a late app_id. + #[test] + fn a_late_app_id_does_not_overwrite_a_more_specific_placement() { + for setup in ["dialog", "maximized", "already-sized"] { + let mut wm = wm_with_monitor(); + wm.set_remembered_geometry("late-app".to_string(), (400, 300, 500, 400)); + let id = wm.alloc_window_id(); + wm.add_window(Window::new(id, "late")); + { + let w = wm.window_mut(id).unwrap(); + w.app_id = "late-app".to_string(); + match setup { + "dialog" => w.is_dialog = true, + "maximized" => w.maximized = true, + // What a client committing its own size leaves behind. + _ => w.size_is_provisional = false, + } + } + let before = wm.window(id).unwrap().geometry; + assert!(!wm.apply_remembered_geometry(id), "{setup} should not be overridden"); + assert_eq!(wm.window(id).unwrap().geometry, before, "{setup} geometry moved"); + } + } + #[test] fn a_rules_explicit_geometry_is_never_provisional() { let mut wm = wm_with_monitor(); diff --git a/crates/wayland/src/state/toplevel.rs b/crates/wayland/src/state/toplevel.rs index 85365c8..20b55f7 100644 --- a/crates/wayland/src/state/toplevel.rs +++ b/crates/wayland/src/state/toplevel.rs @@ -73,8 +73,26 @@ pub(crate) fn sync_toplevel_metadata(state: &mut CompState, id: WindowId, surfac // window back to the front any time its title happened to // update - reported live as an older window jumping in front of // a newer, focused one with no user action to explain it. + // Before the rules, and before anything is drawn: a real `app_id` + // is the first moment the window-memory store can be looked up at + // all, since a toplevel role exists before its client sends + // `set_app_id` and `add_window` therefore searched the store for the + // empty string. See `apply_remembered_geometry`. A rule's own + // explicit `geometry` still wins, which is why this runs first and + // `reapply_rules_if_pending` runs after. + let restored = state.wm.borrow_mut().apply_remembered_geometry(id); + if restored { + // The backend keeps its own copy of "this size is only a guess" + // (`provisional_size`), and `adopt_provisional_size` reads that + // one, not the core flag. Leaving this id in it meant the + // client's next commit overwrote the size just restored with + // whatever the client would have opened at - the position came + // back and the size did not, which is a stranger result than + // nothing being restored at all. + state.provisional_size.remove(&id); + } let reapplied = state.wm.borrow_mut().reapply_rules_if_pending(id); - if reapplied { + if reapplied || restored { state.redraw_decoration_buffer(id); state.sync_geometry(id); } diff --git a/crates/wayland/src/window_memory.rs b/crates/wayland/src/window_memory.rs index 4376274..f4f0edf 100644 --- a/crates/wayland/src/window_memory.rs +++ b/crates/wayland/src/window_memory.rs @@ -106,6 +106,20 @@ pub(crate) fn load() -> HashMap<String, PersistedGeometry> { } } +/// True when this instance is nested AND has been given no state directory +/// of its own, so anything it wrote would land in the real session's store. +/// +/// The guard cannot simply be "nested", even though that is the case it +/// exists for. A test instance is the only thing that ever needs to +/// exercise saving, and refusing every nested write makes the feature +/// untestable without pointing a compositor at the owner's real desktop. +/// Pointing `SRDWM_STATE_PATH` (or `XDG_STATE_HOME`) somewhere else is a +/// deliberate act that says exactly where the writes should go, so a nested +/// instance that has done it is allowed to write there. +fn writes_would_land_in_the_real_session() -> bool { + crate::running_nested() && std::env::var_os("SRDWM_STATE_PATH").is_none() && std::env::var_os("XDG_STATE_HOME").is_none() +} + /// Overwrites the whole persisted table from `entries` - called after /// every drag/resize-end (see `input/pointer.rs`'s call site), which are /// rare, real user actions, not a per-frame event, so writing the whole @@ -123,7 +137,7 @@ pub(crate) fn load() -> HashMap<String, PersistedGeometry> { /// back over it is not. Same reasoning as `publish_gtk_stylesheet`'s own /// nested guard. pub(crate) fn save_all<'a>(entries: impl Iterator<Item = (&'a str, (i32, i32, u32, u32))>) { - if crate::running_nested() { + if writes_would_land_in_the_real_session() { return; } let apps: HashMap<String, PersistedGeometry> = |