From c888c30c089e096a47b222ff6c9053ff2018e3bc Mon Sep 17 00:00:00 2001 From: Mollusk Date: Sat, 13 Jun 2026 16:51:22 -0400 Subject: [PATCH] feat(window): restore window position on X11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X11 sessions now persist and restore the window position (window_x/y) in addition to size. Gated to X11: Wayland's xdg-shell gives clients no way to self-position, so we center there (and iced never emits Moved on Wayland, so window_x/y stay None). No drift across save/restore — iced's Moved event and Position::Specific both use the window's outer position. Also confirmed peerspeak already runs on X11 out of the box (winit compiles both backends and auto-selects via WAYLAND_DISPLAY/DISPLAY) and documented X11/Wayland support in FEATURES.md. - config: window_x/window_y: Option (serde-default None). - app: is_wayland() + pure initial_window_position() helper; a Moved handler records position; the close path persists it. - tests: +3 initial_window_position (X11 restore / Wayland centers / partial-or-missing centers), +2 config (round-trip incl. negative coords; backward-compat load without the new fields). 130 -> 135 lib. Verified live on X11/XWayland: saved an off-center position, the window reopened there (not centered). clippy clean incl. --all-targets. Co-Authored-By: Claude Opus 4.8 --- docs/FEATURES.md | 10 ++++++ src/app/mod.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++---- src/config.rs | 43 ++++++++++++++++++++++- 3 files changed, 133 insertions(+), 8 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index c1c9f6f..cc3a31b 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -122,6 +122,16 @@ covers internals). When you ship a feature, add it here. |---|---|---| | Config file | ✅ | `~/.config/peerspeak/config.json`. | | Backward-compatible loading | ✅ | serde `default`s fill missing fields; unknown fields tolerated. | +| Window size restored | ✅ | `window_width`/`window_height`, saved on close. | +| Window position restored | ✅ | `window_x`/`window_y`, saved on close. **X11 only** — see Platform support. | + +## Platform support (Linux desktop) + +| Concern | Status | Notes | +|---|---|---| +| Wayland | ✅ | Default on this dev box; winit's Wayland backend. App/taskbar icon comes from the `.desktop` file matched by `application_id = "peerspeak"`. | +| X11 (incl. XWayland) | ✅ | winit's X11 backend (both backends compile in by default; winit auto-selects — Wayland if `WAYLAND_DISPLAY` is set, else X11 via `DISPLAY`). Launch verified rendering on X11. The embedded RGBA window icon (`from_rgba`) is honored on X11 even without the `.desktop` installed. | +| Window position restore | X11 only | Wayland's xdg-shell gives clients no way to place their own window, so we center there. On X11 the saved `window_x`/`window_y` is restored via `Position::Specific` (outer position; no drift across save/restore — iced's `Moved` and `Position::Specific` both use the outer position). Verified live on X11 (saved an off-center position; the window reopened there, not centered). Gated by `is_wayland()` (`src/app/mod.rs`); decision logic unit-tested (`initial_window_position`). | --- diff --git a/src/app/mod.rs b/src/app/mod.rs index 7765b7f..11c6cc9 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -322,11 +322,12 @@ fn theme(_state: &AppState) -> Theme { } pub fn run_gui() -> iced::Result { - // Restore the last window size (saved on close). Position can't be restored on - // Wayland — xdg-shell gives clients no way to set their own position — so we - // only persist size and leave placement to the compositor. + // Restore the last window size (saved on close). Position is restored too, + // but only on X11 — Wayland's xdg-shell gives clients no way to set their own + // position, so we center there and leave placement to the compositor. let saved = AppConfig::load(); let init_size = iced::Size::new(saved.window_width, saved.window_height); + let init_position = initial_window_position(saved.window_x, saved.window_y, is_wayland()); iced::application(AppState::default, update, view) .title("PeerSpeak P2P Voice Chat") .theme(theme) @@ -335,7 +336,7 @@ pub fn run_gui() -> iced::Result { // Restored from config (defaults 900×760: taller so the bottom chat // dock doesn't squeeze the controls column). Layout is responsive. size: init_size, - position: iced::window::Position::Centered, + position: init_position, // App/taskbar icon (mainly used on X11/XWayland; native Wayland takes // the icon from the .desktop file matched by app_id instead). icon: window_icon(), @@ -360,6 +361,31 @@ fn window_icon() -> Option { iced::window::icon::from_rgba(RGBA.to_vec(), 128, 128).ok() } +/// True when running under a Wayland compositor (winit will use its Wayland +/// backend). Mirrors winit's own selection: it prefers Wayland when +/// `WAYLAND_DISPLAY` is set, otherwise falls back to X11 via `DISPLAY`. +fn is_wayland() -> bool { + std::env::var_os("WAYLAND_DISPLAY").is_some_and(|v| !v.is_empty()) +} + +/// Decide the initial window position from the saved coordinates. +/// +/// Position restore only works on **X11** — Wayland's xdg-shell gives clients no +/// way to place their own window, so we center there and let the compositor +/// decide. Returns `Centered` when we're on Wayland or have no saved position. +fn initial_window_position( + saved_x: Option, + saved_y: Option, + is_wayland: bool, +) -> iced::window::Position { + match (saved_x, saved_y) { + (Some(x), Some(y)) if !is_wayland => { + iced::window::Position::Specific(iced::Point::new(x as f32, y as f32)) + } + _ => iced::window::Position::Centered, + } +} + fn subscription(_state: &AppState) -> Subscription { let core_sub = Subscription::run(core_subscription).map(AppMessage::UiEventReceived); let event_sub = iced::event::listen().map(AppMessage::EventOccurred); @@ -777,10 +803,18 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { state.config.chat_drawer_width = clamp_chat_drawer_width(state.config.chat_drawer_width, size.width); } + AppMessage::EventOccurred(Event::Window(iced::window::Event::Moved(position))) => { + // Remember the position in-memory; written to disk once on close. + // Negative coords are valid (a monitor left of/above the primary), so + // we don't clamp. On Wayland iced doesn't report position, so this + // arm simply never fires there and window_x/y stay None. + state.config.window_x = Some(position.x as i32); + state.config.window_y = Some(position.y as i32); + } AppMessage::EventOccurred(Event::Window(iced::window::Event::CloseRequested)) => { // We took over the close path (exit_on_close_request:false) so we can - // persist the final window size before quitting. The latest size is - // already mirrored into config by the Resized handler above. + // persist the final window size + position before quitting. Both are + // already mirrored into config by the Resized/Moved handlers above. state.config.save(); return iced::exit(); } @@ -2513,7 +2547,47 @@ impl Program for Icon { #[cfg(test)] mod tests { - use super::{format_duration, reconnect_attempt_chime, reconnected_chime, GateMeter, METER_MAX}; + use super::{ + format_duration, initial_window_position, reconnect_attempt_chime, reconnected_chime, + GateMeter, METER_MAX, + }; + + #[test] + fn x11_restores_saved_window_position() { + // On X11 (is_wayland = false) a saved position becomes Specific(x, y). + match initial_window_position(Some(120), Some(-40), false) { + iced::window::Position::Specific(p) => { + assert_eq!(p.x, 120.0); + assert_eq!(p.y, -40.0); // negative coords (left/above primary) preserved + } + other => panic!("expected Specific, got {other:?}"), + } + } + + #[test] + fn wayland_always_centers_even_with_saved_position() { + assert!(matches!( + initial_window_position(Some(120), Some(40), true), + iced::window::Position::Centered + )); + } + + #[test] + fn missing_or_partial_saved_position_centers() { + assert!(matches!( + initial_window_position(None, None, false), + iced::window::Position::Centered + )); + // A half-saved position (one axis missing) is not enough to restore. + assert!(matches!( + initial_window_position(Some(10), None, false), + iced::window::Position::Centered + )); + assert!(matches!( + initial_window_position(None, Some(10), false), + iced::window::Position::Centered + )); + } #[test] fn format_duration_renders_mss_and_hmmss() { diff --git a/src/config.rs b/src/config.rs index b5096cd..de2f43a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -158,11 +158,19 @@ pub struct AppConfig { #[serde(default)] pub pixelpass_path: Option, /// Last window size (px), restored as the initial size on next launch. - /// Saved on close. (Window *position* can't be restored on Wayland.) + /// Saved on close. #[serde(default = "default_window_width")] pub window_width: f32, #[serde(default = "default_window_height")] pub window_height: f32, + /// Last window position (px). Saved on close, restored on next launch — but + /// only on **X11**: Wayland's xdg-shell gives clients no way to place their + /// own window, so we center there and let the compositor decide. `None` = + /// never saved a position yet (e.g. always run under Wayland) → center. + #[serde(default)] + pub window_x: Option, + #[serde(default)] + pub window_y: Option, } impl Default for AppConfig { @@ -193,6 +201,8 @@ impl Default for AppConfig { pixelpass_path: None, window_width: default_window_width(), window_height: default_window_height(), + window_x: None, + window_y: None, } } } @@ -290,6 +300,37 @@ mod tests { assert_eq!(back.window_height, 720.0); } + #[test] + fn test_window_position_fields() { + // Position is unset by default (only ever saved on X11). + let def = AppConfig::default(); + assert_eq!(def.window_x, None); + assert_eq!(def.window_y, None); + // A saved position (incl. negative coords) round-trips. + let cfg = AppConfig { + window_x: Some(200), + window_y: Some(-50), + ..AppConfig::default() + }; + let json = serde_json::to_string(&cfg).unwrap(); + let back: AppConfig = serde_json::from_str(&json).unwrap(); + assert_eq!(back.window_x, Some(200)); + assert_eq!(back.window_y, Some(-50)); + } + + #[test] + fn old_config_without_position_loads() { + // A config written before window_x/window_y existed still deserializes + // (serde default → None), so upgrades don't wipe a user's settings. + // (The three device/gate fields have no serde default, so any valid + // config must carry them — mirrors test_backward_compat_default_fill.) + let json = r#"{"input_device":"","output_device":"","noise_gate_threshold":0.01,"window_width":1024.0,"window_height":768.0}"#; + let cfg: AppConfig = serde_json::from_str(json).unwrap(); + assert_eq!(cfg.window_x, None); + assert_eq!(cfg.window_y, None); + assert_eq!(cfg.window_width, 1024.0); + } + #[test] fn test_input_output_volume_fields() { // Default impl is unity gain.