Fix window memory never saving on close, and split-screen icon/primary bugs

Screenshotted the just-split display on request rather than guessing --
it showed why windows never seem to remember placement/size, plus two
real split-screen bugs.

Window memory (WindowManager::remembered_geometry) was correctly wired
on the read side, but the only writes came from end_drag/end_resize in
dragresize.rs - a real drag or resize. A window the user opens, looks
at, and closes without ever touching its edges had nothing recorded, so
reopening it always fell back to a fresh cascade placement, for what is
probably most ordinary window lifecycles. remove_window now also
snapshots geometry (same app_id-non-empty gate the drag/resize sites
use), persisted at both of its wayland-side call sites the same way the
drag/resize-release site already does.

desktop_icon_origins mirrored the full icon set onto every Monitor entry
when general.desktop_icons_all_monitors is on - which, after a
srd.monitor.split, is one entry per split part of the same physical
screen, not one per real monitor. Extracted into a separately-tested
icon_origins_for that collapses split parts of the same connector back
to one origin, keeping a genuinely separate monitor's own origin intact.

Found while fixing that: every split part also reported primary: true
(computed from the connector's name, which doesn't vary per part) --
fixed by gating on part == 0 too.

6 files changed, 165 insertions, 14 deletions

diff --git a/crates/core/src/manager/tests.rs b/crates/core/src/manager/tests.rs
index c82d3dd..3d628a8 100644
--- a/crates/core/src/manager/tests.rs
+++ b/crates/core/src/manager/tests.rs
@@ -363,6 +363,35 @@
     }
 
     #[test]
+    fn closing_a_window_remembers_its_geometry_even_if_it_was_never_dragged_or_resized() {
+        // Real report: "windows don't remember their placement/size" --
+        // true for any window the user never manually touched, since only
+        // `end_drag`/`end_resize` used to write `remembered_geometry` at
+        // all. A window that was simply placed by SmartPlacement, looked
+        // at, and closed had nothing recorded, so reopening it always fell
+        // back to a fresh placement - indistinguishable from the memory
+        // feature not existing at all for that (extremely common) case.
+        let mut wm = wm_with_monitor();
+        wm.set_layout(wm.current_workspace(), "tiling");
+        let a = wm.alloc_window_id();
+        let mut w = Window::new(a, "a");
+        w.app_id = "alacritty".into();
+        w.geometry = Rect::new(321, 111, 444, 222);
+        wm.add_window(w);
+        // Never dragged, never resized - closed exactly as SmartPlacement
+        // left it.
+        wm.remove_window(a);
+
+        let b = wm.alloc_window_id();
+        let mut w2 = Window::new(b, "b");
+        w2.app_id = "alacritty".into();
+        w2.geometry = Rect::new(0, 0, 800, 600);
+        wm.add_window(w2);
+        let placed = wm.window(b).unwrap().geometry;
+        assert_eq!((placed.x, placed.y, placed.width, placed.height), (321, 111, 444, 222), "the next alacritty window must open where/how large the first one was when it closed");
+    }
+
+    #[test]
     fn a_remembered_position_on_a_monitor_that_no_longer_exists_falls_back_to_placement() {
         let mut wm = wm_with_monitor();
         wm.set_layout(wm.current_workspace(), "dynamic");
diff --git a/crates/core/src/manager/windows.rs b/crates/core/src/manager/windows.rs
index 5468796..40b4c17 100644
--- a/crates/core/src/manager/windows.rs
+++ b/crates/core/src/manager/windows.rs
@@ -261,7 +261,30 @@ impl WindowManager {
         if self.focused == Some(id) {
             self.focused = self.order.last().copied();
         }
-        self.windows.remove(&id)
+        let window = self.windows.remove(&id);
+        // Remembers wherever this app's window actually ended up, not just
+        // wherever a manual drag/resize left it (`dragresize.rs`'s own
+        // `end_drag`/`end_resize` sites) - without this, an app the user
+        // never dragged or resized had nothing recorded at all, so closing
+        // and reopening it always fell back to a fresh cascade placement
+        // regardless of where it had actually been sitting. Reported live
+        // as "windows don't remember their placement", indistinguishable
+        // from a broken feature even though the underlying store and its
+        // read side (`WindowManager::add_window`'s own `remembered_
+        // geometry` lookup) were already both correct - this was the one
+        // write path that never fired for an app the user just opens,
+        // looks at, and closes. Same `app_id`-non-empty gate as the
+        // drag/resize sites, and the same reasoning for not also gating on
+        // `floating`: a tiled window's geometry is layout-computed and
+        // simply never consulted again on the read side once `layout_name
+        // == "tiling"`, so remembering it anyway is harmless, not wasted
+        // work worth a special case.
+        if let Some(w) = &window {
+            if !w.app_id.is_empty() {
+                self.remembered_geometry.insert(w.app_id.clone(), (w.geometry.x, w.geometry.y, w.geometry.width, w.geometry.height));
+            }
+        }
+        window
     }
 
     pub fn window(&self, id: WindowId) -> Option<&Window> {
diff --git a/crates/wayland/src/state/desktop_icons.rs b/crates/wayland/src/state/desktop_icons.rs
index a1b1968..8ed22a5 100644
--- a/crates/wayland/src/state/desktop_icons.rs
+++ b/crates/wayland/src/state/desktop_icons.rs
@@ -72,22 +72,16 @@ impl CompState {
         self.desktop_icon_buffers.clear();
     }
 
-    /// One grid origin per monitor icons should mirror onto: every enabled
-    /// monitor when `general.desktop_icons_all_monitors` is on (the
-    /// default - see that field's own doc comment), otherwise just the
-    /// primary monitor's, matching the original single-monitor behaviour.
-    /// Sorted by monitor id so the list (and therefore which origin
-    /// `icon_at` matches first) is stable call to call, not at the mercy of
-    /// `WindowManager::monitors()`'s own iteration order.
+    /// One grid origin per monitor icons should mirror onto - see
+    /// `icon_origins_for`'s own doc comment for the actual selection
+    /// logic. Sorted by monitor id first so the list (and therefore which
+    /// origin `icon_at` matches first) is stable call to call, not at the
+    /// mercy of `WindowManager::monitors()`'s own iteration order.
     fn desktop_icon_origins(&self) -> Vec<(i32, i32)> {
         let wm = self.wm.borrow();
         let mut monitors = wm.monitors().to_vec();
         monitors.sort_by_key(|m| m.id);
-        if wm.desktop_icons_all_monitors {
-            monitors.iter().map(|m| (m.geometry.x + GRID_MARGIN, m.geometry.y + GRID_MARGIN)).collect()
-        } else {
-            monitors.iter().find(|m| m.primary).map(|m| vec![(m.geometry.x + GRID_MARGIN, m.geometry.y + GRID_MARGIN)]).unwrap_or_default()
-        }
+        icon_origins_for(&monitors, wm.desktop_icons_all_monitors)
     }
 
     /// Re-derives the icon list from the real filesystem (a new/removed
@@ -706,6 +700,47 @@ fn on_path(bin: &str) -> bool {
     std::env::var_os("PATH").map(|paths| std::env::split_paths(&paths).any(|dir| dir.join(bin).is_file())).unwrap_or(false)
 }
 
+/// The actual logic behind [`CompState::desktop_icon_origins`] - pulled
+/// out so it's testable without a real `WindowManager`/`Output`, the same
+/// reasoning `udev/outputs.rs::next_logical_x` already applies. `monitors`
+/// must already be sorted by id.
+///
+/// One origin per real, physically distinct screen when `all_monitors` is
+/// set - not one per `Monitor` entry, which `srd.monitor.split` can
+/// multiply several of out of the *same* real output purely for
+/// placement/tiling purposes (see `Monitor::split`'s own doc comment: "not
+/// a second wl_output, not a second physical connector"). Mirroring the
+/// full icon set onto every split part put two, visually side by side, on
+/// what is still one continuous physical desktop - reported live as
+/// "doesn't look like 2 more monitors, just showing double desktop icons"
+/// the moment a two-way split was tried. A split part's own name is
+/// always `"{connector}-{part}"` (`platform.rs`'s `sub_name`
+/// construction) - recovering the real connector from it and keeping
+/// only the first (lowest-id) part per connector collapses every split
+/// group back to the one real screen it actually is, while a genuinely
+/// separate additional monitor (real or fake, `split == false`) still
+/// gets its own origin exactly as before. When `all_monitors` is unset,
+/// just the primary monitor's origin, matching the original single-
+/// monitor behaviour.
+fn icon_origins_for(monitors: &[srdwm_core::Monitor], all_monitors: bool) -> Vec<(i32, i32)> {
+    if all_monitors {
+        let mut seen_split_connectors = std::collections::HashSet::new();
+        monitors
+            .iter()
+            .filter(|m| {
+                if !m.split {
+                    return true;
+                }
+                let connector = m.name.rsplit_once('-').map(|(base, _)| base).unwrap_or(&m.name);
+                seen_split_connectors.insert(connector.to_string())
+            })
+            .map(|m| (m.geometry.x + GRID_MARGIN, m.geometry.y + GRID_MARGIN))
+            .collect()
+    } else {
+        monitors.iter().find(|m| m.primary).map(|m| vec![(m.geometry.x + GRID_MARGIN, m.geometry.y + GRID_MARGIN)]).unwrap_or_default()
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -745,4 +780,50 @@ mod tests {
         let (c, r) = nearest_free_cell((0, 0), &occupied);
         assert!(c >= 0 && r >= 0);
     }
+
+    fn split_part(id: srdwm_core::MonitorId, name: &str, x: i32) -> srdwm_core::Monitor {
+        let mut m = srdwm_core::Monitor::new(id, name, srdwm_core::Rect::new(x, 0, 960, 1080));
+        m.split = true;
+        m
+    }
+
+    #[test]
+    fn a_two_way_split_screen_gets_only_one_icon_origin_not_two() {
+        // The live-reported bug: "doesn't look like 2 more monitors, just
+        // showing double desktop icons" - a split screen is still one
+        // physical desktop, so mirroring the icon column onto both halves
+        // read as a visual duplication bug, not a second monitor.
+        let monitors = vec![split_part(0, "eDP-1-1", 0), split_part(1, "eDP-1-2", 960)];
+        let origins = icon_origins_for(&monitors, true);
+        assert_eq!(origins.len(), 1, "a split screen must contribute exactly one icon origin, not one per split part");
+        assert_eq!(origins[0], (GRID_MARGIN, GRID_MARGIN), "the one origin kept must be the first (lowest-id) split part's");
+    }
+
+    #[test]
+    fn a_split_screen_plus_a_genuinely_separate_monitor_gets_two_origins() {
+        let mut real_second = srdwm_core::Monitor::new(2, "HDMI-A-1", srdwm_core::Rect::new(1920, 0, 1920, 1080));
+        real_second.split = false;
+        let monitors = vec![split_part(0, "eDP-1-1", 0), split_part(1, "eDP-1-2", 960), real_second];
+        let origins = icon_origins_for(&monitors, true);
+        assert_eq!(origins.len(), 2, "a genuinely separate monitor must still get its own origin alongside the split screen's one");
+    }
+
+    #[test]
+    fn two_different_split_connectors_each_get_their_own_origin() {
+        // Guards the grouping logic against merging two *different* real
+        // outputs that both happen to be split, not just deduplicating one
+        // connector's own parts.
+        let monitors = vec![split_part(0, "eDP-1-1", 0), split_part(1, "eDP-1-2", 960), split_part(2, "HDMI-A-1-1", 1920), split_part(3, "HDMI-A-1-2", 2880)];
+        let origins = icon_origins_for(&monitors, true);
+        assert_eq!(origins.len(), 2, "each split connector is its own physical screen and must get its own origin");
+    }
+
+    #[test]
+    fn without_all_monitors_only_the_primary_gets_an_origin_even_when_split() {
+        let mut a = split_part(0, "eDP-1-1", 0);
+        a.primary = true;
+        let b = split_part(1, "eDP-1-2", 960);
+        let origins = icon_origins_for(&[a, b], false);
+        assert_eq!(origins, vec![(GRID_MARGIN, GRID_MARGIN)]);
+    }
 }
diff --git a/crates/wayland/src/state/lifecycle.rs b/crates/wayland/src/state/lifecycle.rs
index 53a5e86..1749a3c 100644
--- a/crates/wayland/src/state/lifecycle.rs
+++ b/crates/wayland/src/state/lifecycle.rs
@@ -301,6 +301,11 @@ impl CompState {
             self.close_snap_flyout();
         }
         self.wm.borrow_mut().remove_window(id);
+        // Persists whatever `remove_window` just snapshotted into
+        // `remembered_geometry` - see that function's own doc comment for
+        // why a window closing, not just a manual drag/resize release,
+        // needs to reach disk too.
+        crate::window_memory::save_all(self.wm.borrow().all_remembered_geometry());
         self.pending.borrow_mut().push(CoreEvent::WindowDestroyed(id));
         foreign_toplevel::window_closed(self, id);
         // `remove_window` may have picked a new focused window on its own
diff --git a/crates/wayland/src/udev/platform.rs b/crates/wayland/src/udev/platform.rs
index 37abf5d..d7132ee 100644
--- a/crates/wayland/src/udev/platform.rs
+++ b/crates/wayland/src/udev/platform.rs
@@ -856,7 +856,16 @@ impl Platform for UdevPlatform {
                 // erasing the split it was placed to respect.
                 m.full_geometry = srdwm_core::monitor::split_rect(full, part, parts, rows);
                 m.maximize_geometry = srdwm_core::monitor::split_rect(maximize, part, parts, rows);
-                m.primary = primary_name.as_deref() == Some(name.as_str());
+                // Only the first part of a split connector, not every one
+                // of them - `primary_name` names the *connector*, which
+                // doesn't change across `0..parts`, so this used to mark
+                // every split part primary at once. Two (or more) `Monitor`
+                // entries all claiming `primary: true` broke the "exactly
+                // one primary" assumption every caller of this field
+                // reasonably makes (`desktop_icon_origins`'s own single-
+                // monitor branch, concretely, which just took whichever
+                // `.find(|m| m.primary)` happened to match first).
+                m.primary = part == 0 && primary_name.as_deref() == Some(name.as_str());
                 m.split = parts > 1;
                 m.scale = scale;
                 out.push(m);
diff --git a/crates/wayland/src/xwayland.rs b/crates/wayland/src/xwayland.rs
index 5b537d2..9fba4e5 100644
--- a/crates/wayland/src/xwayland.rs
+++ b/crates/wayland/src/xwayland.rs
@@ -759,6 +759,10 @@ impl CompState {
             self.close_snap_flyout();
         }
         self.wm.borrow_mut().remove_window(id);
+        // Same reason as `state/lifecycle.rs`'s native `remove_window`:
+        // persist whatever `remove_window` just snapshotted into
+        // `remembered_geometry`, not just a manual drag/resize release.
+        crate::window_memory::save_all(self.wm.borrow().all_remembered_geometry());
         self.pending.borrow_mut().push(CoreEvent::WindowDestroyed(id));
         crate::foreign_toplevel::window_closed(self, id);
         // Same reason as the equivalent call in `state/lifecycle.rs`'s native