srdusr
aboutsummaryrefslogtreecommitdiffstats
path: root/crates
diff options
context:
space:
mode:
Diffstat (limited to 'crates')
-rw-r--r--crates/core/src/manager/tests.rs29
-rw-r--r--crates/core/src/manager/windows.rs25
-rw-r--r--crates/wayland/src/state/desktop_icons.rs105
-rw-r--r--crates/wayland/src/state/lifecycle.rs5
-rw-r--r--crates/wayland/src/udev/platform.rs11
-rw-r--r--crates/wayland/src/xwayland.rs4
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