diff options
| author | srdusr <[email protected]> | 2026-08-11 01:35:00 +0200 |
|---|---|---|
| committer | srdusr <[email protected]> | 2026-08-11 01:35:00 +0200 |
| commit | 38d899d19b5e5204062ecab4bb4105d51afddb6b (patch) | |
| tree | eaad80d42786a717c44a0d38555609aa61b13207 /crates/core | |
| parent | 9c1673fa73bb49433370a60a7b4bb16abee98a9b (diff) | |
| download | srdwm-38d899d19b5e5204062ecab4bb4105d51afddb6b.tar.gz srdwm-38d899d19b5e5204062ecab4bb4105d51afddb6b.zip | |
Read the window-memory store at a moment when it can actually match
Reported twice, as two complaints: windows do not remember their size or
position across a close or a reboot, and windows spawn stacked on one side
with no smart placement. One bug.
`add_window` looks the store up by app_id. A Wayland toplevel role exists
before its client sends set_app_id, so at the moment srdwm placed a window
the app_id was the empty string, every lookup missed, and every window fell
through to the cascade - which is exactly what "they all open on top of
each other" looks like. The store was being written correctly the whole
time and read at the one moment it could not match.
The lookup now runs again the instant a real app_id arrives, which is still
before the client's first buffer, so nothing is drawn in the wrong place
first. It only moves a window still sitting where the cascade put it: a
rule's explicit geometry, a maximize, a dialog's centring and a client's own
committed size are each more specific than "wherever I last left this app",
and a test asserts none of them is overridden.
A second bug sat underneath the first and only appeared once it was fixed:
the position came back and the size did not, which is stranger than nothing
being restored. The backend keeps its own copy of "this size is only a
guess" (provisional_size) and adopt_provisional_size reads that one rather
than the core flag, so the client's next commit overwrote the size that had
just been restored. Cleared with the same call.
Verified end to end in a nested compositor, driving a real edge-drag with
the virtual-pointer tool:
seeded store 400,300 500x400 -> opened at exactly 400,300 500x400
dragged the right edge -> 646 wide, store rewritten to 646 on release
closed and reopened -> 400,300 646x400
Before this the same first step opened at 30,30 800x600.
Also: window_memory::save_all's nested guard now allows a write when the
instance was given its own state directory (SRDWM_STATE_PATH or
XDG_STATE_HOME). The blanket refusal added earlier kept the owner's store
safe but made the feature impossible to test without pointing a test
compositor at the real desktop, which is how this went unverified in the
first place.
Diffstat (limited to 'crates/core')
| -rw-r--r-- | crates/core/src/manager/dragresize.rs | 53 | ||||
| -rw-r--r-- | crates/core/src/manager/tests.rs | 48 |
2 files changed, 101 insertions, 0 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(); |