Merge PR #288

# Conflicts:
#	docs/whats-new.md

apps/lightcraft/src/main.rs

@@ -93,10 +93,23 @@ fn gpu_marker_path() -> Option<std::path::PathBuf> {
 
 /// Arm the GPU crash sentinel; if the previous launch left it behind (it died inside the GPU
 /// driver while creating the device), the notice to show — GPU rendering then starts off.
-fn gpu_crash_check() -> Option<String> {
-    let marker = gpu_marker_path();
-    let left = marker.as_deref().and_then(lightcraft_engine::gpu::backend::take_init_marker);
-    lightcraft_engine::gpu::backend::set_init_marker(marker);
+///
+/// A `--memory` session writes nothing (issue #169): it neither arms the sentinel — which would
+/// create the settings folder — nor removes a marker it finds. It still honours one, so the GPU
+/// starts off there too, and the next ordinary launch reports and clears it.
+fn gpu_crash_check(in_memory: bool) -> Option<String> {
+    gpu_crash_check_at(gpu_marker_path(), in_memory)
+}
+
+fn gpu_crash_check_at(marker: Option<std::path::PathBuf>, in_memory: bool) -> Option<String> {
+    use lightcraft_engine::gpu::backend;
+    let left = if in_memory {
+        marker.as_deref().and_then(backend::read_init_marker)
+    } else {
+        let left = marker.as_deref().and_then(backend::take_init_marker);
+        backend::set_init_marker(marker);
+        left
+    };
     left.map(|what| gpu_crash_notice(&what))
 }
 
@@ -112,19 +125,28 @@ fn config_dir() -> Option<std::path::PathBuf> {
     lightcraft_engine::camera_profiles::config_dir()
 }
 
+/// `<config>/ui.json`, the saved UI state and app settings — `None` when nothing is read or
+/// written: `LIGHTCRAFT_NO_PREFS` (tests, scripts) or no home folder.
+fn prefs_path() -> Option<std::path::PathBuf> {
+    if std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() {
+        return None;
+    }
+    config_dir().map(|d| d.join("ui.json"))
+}
+
 /// The saved UI state and app settings (`<config>/ui.json`), if any, and a warning for the user
 /// when the file exists but can't be used (issue #103): a damaged file is kept as
 /// `ui.json.corrupt-<unix time>` first, so the next save can't lose the library location in it;
 /// one that can't be read at all is not written this session (`keep_file`).
-fn load_prefs() -> (Option<UiState>, Option<String>, bool) {
-    if std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() {
-        return (None, None, false);
-    }
-    let Some(path) = config_dir().map(|d| d.join("ui.json")) else { return (None, None, false) };
-    load_prefs_at(&path)
+///
+/// A `--memory` session reads the settings (so it looks like the user's app) but writes nothing
+/// (issue #164), so a damaged file is left as it is there, not set aside.
+fn load_prefs(in_memory: bool) -> (Option<UiState>, Option<String>, bool) {
+    let Some(path) = prefs_path() else { return (None, None, false) };
+    load_prefs_at(&path, !in_memory)
 }
 
-fn load_prefs_at(path: &std::path::Path) -> (Option<UiState>, Option<String>, bool) {
+fn load_prefs_at(path: &std::path::Path, set_aside_damaged: bool) -> (Option<UiState>, Option<String>, bool) {
     let bytes = match std::fs::read(path) {
         Ok(b) => b,
         Err(e) if e.kind() == std::io::ErrorKind::NotFound => return (None, None, false),
@@ -135,6 +157,10 @@ fn load_prefs_at(path: &std::path::Path) -> (Option<UiState>, Option<String>, bo
     };
     match serde_json::from_slice::<UiState>(&bytes) {
         Ok(ui) => (Some(ui.sanitized()), None, false),
+        Err(e) if !set_aside_damaged => {
+            let msg = format!("The app settings ({}) are damaged ({e}). Defaults are used; nothing is saved in this session.", path.display());
+            (None, Some(msg), true)
+        }
         Err(e) => {
             let secs = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs()).unwrap_or(0);
             let keep = path.with_file_name(format!("ui.json.corrupt-{secs}"));
@@ -177,8 +203,15 @@ fn write_atomic(path: &std::path::Path, bytes: &[u8]) -> std::io::Result<()> {
 
 /// Saves `ui.json` (app settings, incl. the library to open at launch): as soon as the library
 /// changes, every few seconds when anything else changed, and at exit.
+///
+/// Nothing is written while the session is one that promises to save nothing: `--memory`
+/// (`path` is `None`; issue #164), or the one running because the library couldn't be opened — the
+/// question window and Continue Without Saving (issue #100; `app.openLibrary` ends it, and the
+/// library it opened is saved then).
 #[derive(Default)]
 struct PrefsWriter {
+    /// Where to write; `None` writes nothing (`--memory`, `LIGHTCRAFT_NO_PREFS`, no home folder).
+    path: Option<std::path::PathBuf>,
     written: Vec<u8>,
     library: String,
     checked: f64,
@@ -190,19 +223,26 @@ struct PrefsWriter {
 
 impl PrefsWriter {
     fn save(&mut self, app: &LightcraftApp) -> Result<(), String> {
-        if self.keep_file || std::env::var_os("LIGHTCRAFT_NO_PREFS").is_some() {
+        self.save_ui(&app.ui, app.library_problem.is_some())
+    }
+
+    /// Write `ui` if it changed since the last write; `temporary` (the library couldn't be
+    /// opened) saves nothing.
+    fn save_ui(&mut self, ui: &UiState, temporary: bool) -> Result<(), String> {
+        if self.keep_file || temporary {
             return Ok(());
         }
-        let Some(d) = config_dir() else { return Ok(()) };
-        let bytes = serde_json::to_vec_pretty(&app.ui).map_err(|e| e.to_string())?;
+        let Some(path) = self.path.clone() else { return Ok(()) };
+        let bytes = serde_json::to_vec_pretty(ui).map_err(|e| e.to_string())?;
         if bytes == self.written {
             return Ok(());
         }
-        std::fs::create_dir_all(&d)
-            .and_then(|()| write_atomic(&d.join("ui.json"), &bytes))
+        path.parent()
+            .map_or(Ok(()), std::fs::create_dir_all)
+            .and_then(|()| write_atomic(&path, &bytes))
             .map_err(|e| format!("saving the app settings failed: {e}"))?;
         self.written = bytes;
-        self.library = app.ui.settings.library_path.clone();
+        self.library = ui.settings.library_path.clone();
         Ok(())
     }
 
@@ -471,7 +511,7 @@ fn main() -> eframe::Result {
             _ => files.push(a),
         }
     }
-    let (prefs, prefs_warning, keep_prefs_file) = load_prefs();
+    let (prefs, prefs_warning, keep_prefs_file) = load_prefs(in_memory);
     // --library, else the library last opened from Settings, else the default location
     let library_dir = library_dir.or_else(|| {
         prefs
@@ -483,7 +523,7 @@ fn main() -> eframe::Result {
     let library_dir = library_dir.or_else(lightcraft_engine::library::default_dir);
     // GPU compute: off if the preference says so, or if the last launch died creating the device
     // (issue #136) — before anything can create it
-    let gpu_crash = gpu_crash_check();
+    let gpu_crash = gpu_crash_check(in_memory);
     let gpu_on = prefs.as_ref().is_none_or(|u| u.settings.gpu) && gpu_crash.is_none();
     lightcraft_engine::gpu::set_enabled(gpu_on);
     let options = eframe::NativeOptions {
@@ -521,6 +561,7 @@ fn main() -> eframe::Result {
             app.notices.extend(prefs_warning);
             // what's on disk now: only changes are written
             let writer = PrefsWriter {
+                path: if in_memory { None } else { prefs_path() },
                 written: serde_json::to_vec_pretty(&app.ui).unwrap_or_default(),
                 library: app.ui.settings.library_path.clone(),
                 keep_file: keep_prefs_file,
@@ -576,7 +617,7 @@ mod tests {
         let d = dir("damaged");
         let path = d.join("ui.json");
         std::fs::write(&path, br#"{"settings": {"library_path": "/Volumes/Photos/Lib"#).unwrap();
-        let (ui, warning, keep) = load_prefs_at(&path);
+        let (ui, warning, keep) = load_prefs_at(&path, true);
         assert!(ui.is_none() && !keep);
         let warning = warning.unwrap();
         assert!(warning.contains("damaged") && warning.contains("ui.json.corrupt-"), "{warning}");
@@ -584,7 +625,80 @@ mod tests {
         let kept: Vec<_> = std::fs::read_dir(&d).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().to_string()).collect();
         assert!(kept.len() == 1 && kept[0].starts_with("ui.json.corrupt-"), "{kept:?}");
         // a missing file is just "no settings yet"
-        assert_eq!(load_prefs_at(&path).1, None);
+        assert_eq!(load_prefs_at(&path, true).1, None);
+        let _ = std::fs::remove_dir_all(&d);
+    }
+
+    /// Issue #164: a `--memory` session reads the settings but writes nothing — a damaged file is
+    /// reported and left where it is, not set aside.
+    #[test]
+    fn memory_session_leaves_a_damaged_ui_json_alone() {
+        let d = dir("damaged-memory");
+        let path = d.join("ui.json");
+        let bytes = br#"{"settings": {"library_path": "/Volumes/Photos/Lib"#;
+        std::fs::write(&path, bytes).unwrap();
+        let (ui, warning, keep) = load_prefs_at(&path, false);
+        assert!(ui.is_none() && keep);
+        let warning = warning.unwrap();
+        assert!(warning.contains("damaged") && warning.contains("nothing is saved"), "{warning}");
+        assert_eq!(std::fs::read(&path).unwrap(), bytes);
+        assert_eq!(std::fs::read_dir(&d).unwrap().count(), 1);
+        let _ = std::fs::remove_dir_all(&d);
+    }
+
+    /// Issue #164: the writer saves changed settings to its file — and nothing at all without one
+    /// (`--memory`) or while the session is the temporary one offered when the library can't open.
+    #[test]
+    fn prefs_writer_writes_nothing_in_a_memory_or_temporary_session() {
+        let d = dir("writer");
+        let path = d.join("sub").join("ui.json");
+        let mut ui = UiState::default();
+        // an ordinary session: a change is written, the folder created on the way
+        let mut w = PrefsWriter { path: Some(path.clone()), ..Default::default() };
+        ui.thumb_size += 50.0;
+        w.save_ui(&ui, false).unwrap();
+        let saved = std::fs::read(&path).unwrap();
+        assert!(String::from_utf8_lossy(&saved).contains("thumbSize"), "{}", String::from_utf8_lossy(&saved));
+        // the temporary session (library problem): a further change is not written…
+        ui.thumb_size += 50.0;
+        w.save_ui(&ui, true).unwrap();
+        assert_eq!(std::fs::read(&path).unwrap(), saved);
+        // …until a library opens
+        w.save_ui(&ui, false).unwrap();
+        assert_ne!(std::fs::read(&path).unwrap(), saved);
+        // --memory: no path, so no file and no folder, whatever changes
+        let d2 = dir("writer-memory");
+        let _ = std::fs::remove_dir_all(&d2);
+        let mut w = PrefsWriter { path: None, ..Default::default() };
+        ui.thumb_size += 50.0;
+        w.save_ui(&ui, false).unwrap();
+        assert!(w.written.is_empty() && !d2.exists());
+        let _ = std::fs::remove_dir_all(&d);
+    }
+
+    /// Issue #169: a `--memory` session neither arms the GPU crash sentinel (which would create
+    /// the settings folder) nor removes a marker a crashed launch left; it still reports it, and
+    /// the next ordinary launch finds it, reports it and clears it.
+    #[test]
+    fn memory_session_leaves_the_gpu_marker_and_folder_alone() {
+        let d = dir("gpu-marker-memory");
+        let _ = std::fs::remove_dir_all(&d);
+        let m = d.join("gpu-init.marker");
+        // nothing left behind: no notice, and the folder is not created
+        assert_eq!(gpu_crash_check_at(Some(m.clone()), true), None);
+        assert!(!d.exists(), "a --memory session must not create the settings folder");
+        // a marker left by a crashed launch: reported, kept
+        std::fs::create_dir_all(&d).unwrap();
+        std::fs::write(&m, "GPU device creation started (backends METAL)").unwrap();
+        let notice = gpu_crash_check_at(Some(m.clone()), true).unwrap();
+        assert!(notice.contains("GPU rendering is now off"), "{notice}");
+        assert!(m.exists(), "the marker stays for the next ordinary launch");
+        // the next ordinary launch reports it once and removes it
+        assert!(gpu_crash_check_at(Some(m.clone()), false).is_some());
+        lightcraft_engine::gpu::backend::set_init_marker(None);
+        assert!(!m.exists());
+        assert_eq!(gpu_crash_check_at(Some(m.clone()), false), None);
+        lightcraft_engine::gpu::backend::set_init_marker(None);
         let _ = std::fs::remove_dir_all(&d);
     }
 
@@ -631,7 +745,7 @@ mod tests {
         assert_eq!(std::fs::read(&path).unwrap(), b"{\"new\": true}");
         let ui = UiState::default();
         write_atomic(&path, &serde_json::to_vec_pretty(&ui).unwrap()).unwrap();
-        let (loaded, warning, _) = load_prefs_at(&path);
+        let (loaded, warning, _) = load_prefs_at(&path, true);
         assert!(loaded.is_some() && warning.is_none());
         let _ = std::fs::remove_dir_all(&d);
     }

crates/gpu/src/backend.rs

@@ -155,8 +155,16 @@ pub(crate) fn with_init_marker<T>(backends: Backends, f: impl FnOnce() -> T) ->
 /// If the init marker `path` is present — the last process died while creating the GPU device —
 /// remove it and return what it recorded.
 pub fn take_init_marker(path: &std::path::Path) -> Option<String> {
-    let text = std::fs::read_to_string(path).ok()?;
+    let text = read_init_marker(path)?;
     let _ = std::fs::remove_file(path);
+    Some(text)
+}
+
+/// What the init marker `path` recorded, if it is present — without removing it. For sessions
+/// that must leave the disk as they found it (`--memory`, issues #164 and #169): GPU rendering
+/// still starts off, and the marker stays for the next ordinary launch to report and clear.
+pub fn read_init_marker(path: &std::path::Path) -> Option<String> {
+    let text = std::fs::read_to_string(path).ok()?;
     Some(text.trim().to_string())
 }
 
@@ -225,10 +233,13 @@ mod tests {
         set_init_marker(None);
         assert!(seen, "the marker exists while the device is created");
         assert!(!m.exists(), "and is removed afterwards");
-        // a marker left behind by a crashed process is reported once
+        // a marker left behind by a crashed process is reported once; reading it leaves it in place
         std::fs::write(&m, "GPU device creation started (backends DX12)").unwrap();
+        assert!(read_init_marker(&m).unwrap().contains("DX12"));
+        assert!(m.exists(), "read_init_marker leaves the marker for the next launch");
         assert!(take_init_marker(&m).unwrap().contains("DX12"));
         assert_eq!(take_init_marker(&m), None);
+        assert_eq!(read_init_marker(&m), None);
         let _ = std::fs::remove_dir_all(&dir);
     }
 }

docs/gpu-pipeline.md

@@ -55,7 +55,9 @@ Settings ▸ Performance ▸ *Use the GPU for rendering* unchecked (applied befo
 returns, successfully or not. If the marker is still there at the next launch, the process died inside
 the driver: LightCraft starts with GPU rendering off (the preference is saved unchecked), removes the
 marker and says so in a notice. Checking *Use the GPU for rendering* again tries the GPU once more
-(and re-arms the sentinel). Not with `LIGHTCRAFT_NO_PREFS` (tests, scripts). Killing the app during
+(and re-arms the sentinel). Not with `LIGHTCRAFT_NO_PREFS` (tests, scripts), and not in a `--memory` session,
+which writes nothing: it neither arms the sentinel nor removes a marker it finds (it still starts with GPU
+rendering off when one is there, and the next ordinary launch reports and clears it). Killing the app during
 the ~0.3 s of device creation, or two instances starting at the same moment, can trip it falsely —
 harmless: rendering is then on the CPU until the box is checked again. The sentinel only covers the
 compute device; a crash while the window's renderer starts is avoided by the backend defaults above

docs/whats-new.md

@@ -18,6 +18,9 @@
   labels 6–9, P / U and the rest did nothing, because macOS only passes ⌘ / ⌃ combinations and function keys
   to the menu bar and the app ignored those keys, assuming the menu bar would handle them. Keys outside the menus
   (Space, G, X, ⌫) and ⌘ shortcuts were not affected.
+- `--memory` sessions keep their promise to save nothing (issues #164, #169): UI changes made in one no longer
+  land in `ui.json` (where they replaced the saved settings), and the GPU crash sentinel no longer creates the
+  settings folder there. The same goes for the temporary session offered when the library can't be opened.
 - LightCraft no longer crashes at launch on Windows PCs whose Vulkan driver is broken (issue #136, e.g. some Intel UHD
   630 drivers): on Windows the window and GPU rendering use DirectX 12 only and never load the Vulkan driver unless
   asked to. `LIGHTCRAFT_GPU_BACKEND=dx12 | vulkan | metal | off` (or wgpu's `WGPU_BACKEND`, which GPU rendering