Remember desktop window and panel sizes across restarts (#587)

* Remember desktop window and panel sizes across restarts

* Window layout: discard a saved ui.ron that would crash startup

eframe restores the window geometry and egui memory before the app runs, and a
zoom factor of 0, NaN or 1e30, or a NaN/huge window size aborted every launch
(dpi/wgpu assertions). Check the file first and remove it if it holds such values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Cargo.lock

@@ -8,6 +8,8 @@ version = "0.24.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d3b7f7f85a7e5f68090000ed7622545829afd484d210358702ae4cb97dd0c320"
 dependencies = [
+ "enumn",
+ "serde",
  "uuid",
 ]
 
@@ -130,6 +132,7 @@ dependencies = [
  "cfg-if",
  "getrandom 0.3.4",
  "once_cell",
+ "serde",
  "version_check",
  "zerocopy",
 ]
@@ -575,6 +578,9 @@ name = "bitflags"
 version = "2.13.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3ded4057c258ba199e2d26386d3af3780957ecaee6c4ef4041c6b4b8b97c0b06"
+dependencies = [
+ "serde_core",
+]
 
 [[package]]
 name = "bitstream-io"
@@ -1182,6 +1188,7 @@ checksum = "0656a7530b9e2841ee7a5f983965bdd9840c32f4012f759cfd44b912cced6b34"
 dependencies = [
  "bytemuck",
  "emath",
+ "serde",
 ]
 
 [[package]]
@@ -1210,6 +1217,8 @@ dependencies = [
  "pollster 1.0.1",
  "profiling",
  "raw-window-handle",
+ "ron",
+ "serde",
  "static_assertions",
  "wasm-bindgen",
  "web-sys",
@@ -1234,6 +1243,8 @@ dependencies = [
  "log",
  "nohash-hasher",
  "profiling",
+ "ron",
+ "serde",
  "smallvec",
  "unicode-segmentation",
  "web-sys",
@@ -1275,6 +1286,7 @@ dependencies = [
  "objc2-ui-kit 0.3.2",
  "profiling",
  "raw-window-handle",
+ "serde",
  "smithay-clipboard",
  "web-time",
  "winit",
@@ -1343,6 +1355,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "52c8d4141cf0f60fb2f7aefaf352d3ffa5171ef0ab09731c64391801e95e48f1"
 dependencies = [
  "bytemuck",
+ "serde",
 ]
 
 [[package]]
@@ -1392,6 +1405,17 @@ dependencies = [
  "syn 2.0.119",
 ]
 
+[[package]]
+name = "enumn"
+version = "0.1.14"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2f9ed6b3789237c8a0c1c505af1c7eb2c560df6186f01b098c3a1064ea532f38"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn 2.0.119",
+]
+
 [[package]]
 name = "epaint"
 version = "0.36.2"
@@ -1410,6 +1434,7 @@ dependencies = [
  "parking_lot",
  "profiling",
  "self_cell",
+ "serde",
  "skrifa",
  "smallvec",
  "unicode-general-category",
@@ -1616,6 +1641,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b8eb065f3251655b3c90e22e5e363f310fc5332fb3402e37bbc94752283248f6"
 dependencies = [
  "bytemuck",
+ "serde",
 ]
 
 [[package]]
@@ -3407,6 +3433,7 @@ dependencies = [
  "photocraft-tablet",
  "photocraft-ui-egui",
  "rfd",
+ "ron",
  "serde_json",
  "winresource",
 ]
@@ -4395,6 +4422,20 @@ dependencies = [
  "syn 3.0.6",
 ]
 
+[[package]]
+name = "ron"
+version = "0.12.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "81116b9531d61eabc41aeb228e4b6b2435bcca3233b98cf3b3077d4e6e9debb3"
+dependencies = [
+ "bitflags 2.13.2",
+ "once_cell",
+ "serde",
+ "serde_derive",
+ "typeid",
+ "unicode-ident",
+]
+
 [[package]]
 name = "roxmltree"
 version = "0.20.0"
@@ -4710,6 +4751,9 @@ name = "smallvec"
 version = "1.16.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f9395f0f0eee849a9b707b2f06bb92a6a422090e2123bb2ef8e87a0e61892a8e"
+dependencies = [
+ "serde",
+]
 
 [[package]]
 name = "smithay-client-toolkit"
@@ -5171,6 +5215,12 @@ dependencies = [
  "rustc-hash 2.1.3",
 ]
 
+[[package]]
+name = "typeid"
+version = "1.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "bc7d623258602320d5c55d1bc22793b57daff0ec7efc270ea7d55ce1d5f5471c"
+
 [[package]]
 name = "uds_windows"
 version = "1.2.1"

apps/photocraft/Cargo.toml

@@ -24,7 +24,7 @@ photocraft-automation = { workspace = true }
 photocraft-ui-egui = { workspace = true }
 # Pen tablet pressure/tilt on macOS and X11 (the workspace's one isolated unsafe helper crate).
 photocraft-tablet = { workspace = true }
-eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11"] }
+eframe = { workspace = true, features = ["default_fonts", "wgpu", "accesskit", "wayland", "x11", "persistence"] }
 egui = { workspace = true }
 serde_json = { workspace = true }
 log = { workspace = true }
@@ -33,6 +33,8 @@ log = { workspace = true }
 rfd = { workspace = true }
 arboard = { version = "3.6", features = ["wayland-data-control"] }
 open = "5"
+# Reads the saved window layout (eframe persistence) to drop values that would abort startup.
+ron = "0.12"
 
 [target.'cfg(target_os = "macos")'.dependencies]
 # Finder double-click / Open With / Dock drops arrive as Apple events; this small audited crate

apps/photocraft/src/main.rs

@@ -32,6 +32,7 @@ mod services;
 // Windows gets pen pressure from winit (WM_POINTER); the web runner has its own listener.
 #[cfg(any(target_os = "macos", target_os = "linux", test))]
 mod tablet;
+mod ui_state;
 
 use photocraft_engine::Session;
 use photocraft_ui_egui::PhotocraftApp;
@@ -56,6 +57,9 @@ fn native_options() -> eframe::NativeOptions {
             .with_titlebar_shown(false)
             .with_title_shown(false),
         centered: true,
+        // eframe saves native window geometry and egui panel/window sizes on exit.
+        // Keep that state beside preferences, including config overrides and portable mode.
+        persistence_path: services::config_dir().map(|dir| dir.join("ui.ron")),
         ..Default::default()
     }
 }
@@ -143,6 +147,8 @@ fn main() -> eframe::Result {
     // Brush presets load in the background; the app attaches them when they arrive.
     let presets = services::presets_dir().map(photocraft_engine::preset_store::open_dir_async);
     let mut options = native_options();
+    // eframe restores the saved window layout before our code runs; drop values that would crash it.
+    ui_state::sanitize(options.persistence_path.as_deref());
     // Crash-safe GPU startup (#4): pick the backend (a marker left by a start that died in the
     // driver moves to a safer one), and lock this start's marker until the first frames render.
     let t_sentinel = std::time::Instant::now();
@@ -323,6 +329,44 @@ fn main() -> eframe::Result {
 
 #[cfg(test)]
 mod tests {
+    #[test]
+    fn window_and_panel_geometry_survive_a_restart() {
+        let options = super::native_options();
+        assert!(options.persist_window);
+        assert_eq!(options.persistence_path, super::services::config_dir().map(|dir| dir.join("ui.ron")));
+
+        #[derive(Default)]
+        struct Storage(std::collections::BTreeMap<String, String>);
+        impl eframe::Storage for Storage {
+            fn get_string(&self, key: &str) -> Option<String> {
+                self.0.get(key).cloned()
+            }
+            fn set_string(&mut self, key: &str, value: String) {
+                self.0.insert(key.into(), value);
+            }
+            fn remove_string(&mut self, key: &str) {
+                self.0.remove(key);
+            }
+            fn flush(&mut self) {}
+        }
+        let ctx = egui::Context::default();
+        let input = || egui::RawInput { screen_rect: Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1200.0, 800.0))), ..Default::default() };
+        let mut output = ctx.run_ui(input(), |ui| {
+            egui::Panel::right("dock").exact_size(410.0).show(ui, |ui| ui.set_min_width(ui.available_width()));
+        });
+        output.textures_delta.clear();
+        let mut storage = Storage::default();
+        ctx.memory(|memory| eframe::set_value(&mut storage, "egui", memory));
+        let restored = egui::Context::default();
+        restored.memory_mut(|memory| *memory = eframe::get_value(&storage, "egui").unwrap());
+        let mut output = restored.run_ui(input(), |ui| {
+            egui::Panel::right("dock").default_size(290.0).show(ui, |ui| ui.set_min_width(ui.available_width()));
+        });
+        output.textures_delta.clear();
+        let panel = egui::containers::panel::PanelState::load(&restored, egui::Id::new("dock")).unwrap();
+        assert_eq!(panel.size().x, 410.0);
+    }
+
     #[test]
     fn the_window_opens_centred_at_its_default_size() {
         let o = super::native_options();

apps/photocraft/src/ui_state.rs

@@ -0,0 +1,133 @@
+//! The saved window geometry and egui layout (`ui.ron`, written by eframe's persistence).
+//!
+//! eframe restores this state while it creates the window, before PhotoCraft's code runs, and
+//! some values it does not check abort startup: a zoom factor of zero, NaN or 1e30 fails a
+//! `dpi` assertion, and a NaN or huge window size fails surface creation. A truncated or
+//! hand-edited file would then crash every launch. [`sanitize`] runs first and removes a file
+//! holding such values (the window and panels open at their defaults; nothing else is lost).
+//! A file that isn't RON at all is left alone: eframe already ignores it.
+
+use std::path::Path;
+
+/// Window positions, sizes and panel rects beyond this many points or pixels are corrupt.
+const MAX_COORD: f64 = 1.0e6;
+/// egui's zoom factor is the UI scale over the display scale (75% on a 2× display is 0.375,
+/// 300% on a 1× display is 3).
+const ZOOM: std::ops::RangeInclusive<f64> = 0.1..=10.0;
+
+/// Remove `path` if it holds state that would crash startup. Never fails: errors are logged.
+pub fn sanitize(path: Option<&Path>) {
+    let Some(path) = path else { return };
+    let Ok(text) = std::fs::read_to_string(path) else { return };
+    if let Err(why) = check(&text) {
+        log::warn!("ignoring saved window layout {}: {why}", path.display());
+        if let Err(e) = std::fs::remove_file(path) {
+            log::warn!("could not remove {}: {e}", path.display());
+        }
+    }
+}
+
+/// `Err` names the first value that would crash startup.
+fn check(text: &str) -> Result<(), String> {
+    // eframe stores a map of key → RON string; a file that doesn't parse is ignored by eframe.
+    let Ok(kv) = ron::from_str::<std::collections::HashMap<String, String>>(text) else { return Ok(()) };
+    for (key, value) in &kv {
+        // eframe also skips an entry that doesn't parse.
+        let Ok(v) = ron::from_str::<ron::Value>(value) else { continue };
+        floats_sane(&v, 0).map_err(|e| format!("{key}: {e}"))?;
+        let zoom = (key == "egui").then(|| field(&v, "options").and_then(|o| field(o, "zoom_factor")).and_then(as_f64)).flatten();
+        if let Some(z) = zoom.filter(|z| !ZOOM.contains(z)) {
+            return Err(format!("egui: zoom factor {z}"));
+        }
+    }
+    Ok(())
+}
+
+/// Every float finite and within [`MAX_COORD`]; depth-limited (the file is untrusted).
+fn floats_sane(v: &ron::Value, depth: usize) -> Result<(), String> {
+    if depth > 64 {
+        return Err("nested too deeply".into());
+    }
+    match v {
+        ron::Value::Number(_) => match as_f64(v) {
+            Some(f) if !f.is_finite() || f.abs() > MAX_COORD => Err(format!("value {f}")),
+            _ => Ok(()),
+        },
+        ron::Value::Option(Some(inner)) => floats_sane(inner, depth + 1),
+        ron::Value::Seq(items) => items.iter().try_for_each(|i| floats_sane(i, depth + 1)),
+        ron::Value::Map(m) => m.iter().try_for_each(|(k, v)| floats_sane(k, depth + 1).and_then(|()| floats_sane(v, depth + 1))),
+        _ => Ok(()),
+    }
+}
+
+/// A float (integers such as egui's hashed ids are not coordinates).
+fn as_f64(v: &ron::Value) -> Option<f64> {
+    match v {
+        ron::Value::Number(ron::value::Number::F32(f)) => Some(f64::from(f.get())),
+        ron::Value::Number(ron::value::Number::F64(f)) => Some(f.get()),
+        _ => None,
+    }
+}
+
+fn field<'a>(v: &'a ron::Value, name: &str) -> Option<&'a ron::Value> {
+    match v {
+        ron::Value::Map(m) => m.get(&ron::Value::String(name.into())),
+        _ => None,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::check;
+
+    fn file(window: &str, zoom: &str) -> String {
+        let egui = format!("(options:(zoom_factor:{zoom},max_passes:2),data:([(18446744073709551615,(ron:\"(x:1.0)\"))]))");
+        format!("{{\"window\":{window:?},\"egui\":{egui:?}}}")
+    }
+
+    const WINDOW: &str = "(inner_position_pixels:Some((x:616.0,y:430.0)),fullscreen:false,inner_size_points:Some((x:1100.0,y:760.0)))";
+
+    #[test]
+    fn a_saved_layout_is_kept() {
+        assert_eq!(check(&file(WINDOW, "1.0")), Ok(()));
+        assert_eq!(check(&file(WINDOW, "0.375")), Ok(()));
+    }
+
+    #[test]
+    fn values_that_abort_startup_are_rejected() {
+        // Each of these aborted the app at launch before the check (dpi / wgpu assertions).
+        for zoom in ["0.0", "NaN", "1e30", "-1.0"] {
+            assert!(check(&file(WINDOW, zoom)).is_err(), "zoom {zoom}");
+        }
+        for size in ["(x:NaN,y:760.0)", "(x:1100.0,y:1e30)", "(x:inf,y:1.0)"] {
+            let w = WINDOW.replace("(x:1100.0,y:760.0)", size);
+            assert!(check(&file(&w, "1.0")).is_err(), "size {size}");
+        }
+    }
+
+    #[test]
+    fn unreadable_files_are_left_to_eframe() {
+        // eframe ignores files and entries it can't parse, so they can't crash it.
+        for text in ["", "not ron {{{", "{\"window\":\"(((\"}", "[1,2,3]"] {
+            assert_eq!(check(text), Ok(()), "{text:?}");
+        }
+        let deep = format!("{{\"egui\":{:?}}}", "[".repeat(500) + &"]".repeat(500));
+        let _ = check(&deep);
+    }
+
+    #[test]
+    fn sanitize_removes_only_a_bad_file() {
+        let dir = std::env::temp_dir().join(format!("photocraft-ui-state-{}", std::process::id()));
+        std::fs::create_dir_all(&dir).unwrap();
+        let good = dir.join("good.ron");
+        let bad = dir.join("bad.ron");
+        std::fs::write(&good, file(WINDOW, "1.0")).unwrap();
+        std::fs::write(&bad, file(WINDOW, "0.0")).unwrap();
+        super::sanitize(Some(&good));
+        super::sanitize(Some(&bad));
+        super::sanitize(Some(&dir.join("missing.ron")));
+        super::sanitize(None);
+        assert!(good.exists() && !bad.exists());
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+}

docs/control-protocol.md

@@ -121,7 +121,8 @@ log text file. GPU on/off and the GPU tile size apply at the next launch.
 The desktop app stores them in `preferences.json` in the platform config directory (macOS
 `~/Library/Application Support/Photocraft`, Windows `%APPDATA%\Photocraft`, Linux
 `$XDG_CONFIG_HOME/photocraft`; override with `PHOTOCRAFT_CONFIG_DIR`); autosaves go to its
-`Recovery` folder. In portable mode (a `portable.txt` or `PhotoCraft.portable` file beside the
+`Recovery` folder, and the window size and position, the dock width and floating-panel geometry
+to `ui.ron` beside it (a file holding values that would crash startup is discarded). In portable mode (a `portable.txt` or `PhotoCraft.portable` file beside the
 executable, as in the Windows portable zip) that directory is `PhotoCraftData` next to the
 executable instead. The web build keeps them in `localStorage`. A save writes only the values
 this instance changed since it last loaded or saved them over what storage holds now, so a second