diff --git a/crates/catalog/src/local.rs b/crates/catalog/src/local.rs index 9cfc14f35..093c22427 100644 --- a/crates/catalog/src/local.rs +++ b/crates/catalog/src/local.rs @@ -183,15 +183,7 @@ impl Catalog { let mut plan = ForgetPlan { now: now.to_string(), ..Default::default() }; let now_s = crate::stacks::iso_seconds(now); let max_age = i64::from(days) * 86_400; - let mut referenced: HashSet = HashSet::new(); - for a in self.albums.values() { - referenced.extend(a.photos.iter().copied()); - referenced.extend(a.cover); - } - for s in self.stacks.values() { - referenced.extend(s.photos.iter().copied()); - } - referenced.extend(self.photos.values().filter_map(|p| p.copy_of)); + let referenced = self.referenced(); // the fingerprints are the costly part: work them out for the candidates in parallel let candidates: Vec<&Photo> = self .photos @@ -237,6 +229,30 @@ impl Catalog { plan.unstamp = self.browsed.keys().filter(|f| !remaining.contains(*f)).cloned().collect(); plan } + + /// Of `ids`, the untouched Local records nothing refers to (no album, stack or virtual copy): + /// the ones [`Catalog::forget_local_plan`] could forget, whatever their folder's time. + pub fn forgettable_local(&self, ids: &[PhotoId]) -> Vec { + let referenced = self.referenced(); + let candidates: Vec<&Photo> = + ids.iter().filter(|id| !referenced.contains(id)).filter_map(|id| self.photos.get(id)).filter(|p| p.local).map(|p| &**p).collect(); + let untouched = untouched_of(&candidates); + ids.iter().copied().filter(|id| untouched.contains(id)).collect() + } + + /// Photos an album (as a member or its cover), a stack or a virtual copy refers to. + fn referenced(&self) -> HashSet { + let mut referenced: HashSet = HashSet::new(); + for a in self.albums.values() { + referenced.extend(a.photos.iter().copied()); + referenced.extend(a.cover); + } + for s in self.stacks.values() { + referenced.extend(s.photos.iter().copied()); + } + referenced.extend(self.photos.values().filter_map(|p| p.copy_of)); + referenced + } } /// A folder browsed at `at` is past the period (never when `days` is 0 or a time is unreadable). diff --git a/crates/engine/src/cmd/browse.rs b/crates/engine/src/cmd/browse.rs index a681ec14b..df8de4301 100644 --- a/crates/engine/src/cmd/browse.rs +++ b/crates/engine/src/cmd/browse.rs @@ -6,11 +6,14 @@ //! Each browse records when the folders it listed were browsed. Untouched records of folders //! not browsed for [`Session::forget_local_days`] are forgotten (when the library opens, or with //! `library.forgetLocal`); see [`lightcraft_catalog::local`] for what "untouched" means. Files -//! and sidecars on disk are never touched, and browsing the folder again brings them back. +//! and sidecars on disk are never touched, and browsing the folder again brings them back. A +//! browse also forgets at once the untouched records of the folder's files that are gone from +//! the disk (deleted or moved by another app), so the folder shows what is on disk. use std::path::Path; -use lightcraft_catalog::{Op, PhotoId}; +use lightcraft_catalog::query::in_folder; +use lightcraft_catalog::{Op, PhotoId, Source}; use serde_json::{Value, json}; use super::{CommandSpec, always, bad, bool_or, cmd, str_param}; @@ -42,11 +45,72 @@ fn browse(s: &mut Session, p: &Value) -> Result { // looking at a folder never brings its deleted photos back (importing the files does) let opts = ImportOptions { mode: ImportMode::Add, local: true, on_deleted: crate::import::OnDeleted::Skip, ..Default::default() }; let report = import_with(s, &files, &opts)?; + let forgotten = forget_gone(s, &dir_s, subfolders, &files); stamp_browsed(s, &dir_s, &files); s.browse = Some(Browse { path: dir_s.clone(), subfolders }); s.source = LibrarySource::Folder; let shown = s.visible().len(); - Ok(json!({"path": dir_s, "subfolders": subfolders, "photos": shown, "new": report.imported.len(), "failed": report.failed.len()})) + Ok(json!({ + "path": dir_s, + "subfolders": subfolders, + "photos": shown, + "new": report.imported.len(), + "failed": report.failed.len(), + "forgotten": forgotten, + })) +} + +impl Session { + /// Forget the untouched Local records of files gone from the folder `dir` (with its subfolders when + /// `subfolders`) that a browse just listed as `files`: what `library.browse` does after reading a folder, for a + /// UI that reads it on a worker thread instead. Returns how many went. + pub fn forget_gone_from(&mut self, dir: &str, subfolders: bool, files: &[String]) -> usize { + forget_gone(self, dir, subfolders, files) + } +} + +/// Local records in the browsed folder whose files are no longer on disk (deleted or moved by +/// another app) and that nothing was changed on are forgotten, as `library.forgetLocal` forgets +/// them: the folder shows what is on disk, not photos that fail to load (and log an error) on +/// every visit (issue #1392). Records the user changed stay, shown as missing, and so do those +/// the session holds (selected, edited, in the undo history) or with a smart preview. Returns how +/// many went. +fn forget_gone(s: &mut Session, dir: &str, subfolders: bool, files: &[String]) -> usize { + let listed: std::collections::HashSet<&str> = files.iter().map(String::as_str).collect(); + let held = held(s); + let smart = s.media.smart_dir.clone(); + let gone: Vec = s + .catalog + .photos() + .filter(|p| p.local && !p.deleted && !held.contains(&p.id)) + // a smart preview may be all that is left of the picture + .filter(|p| !smart.as_ref().is_some_and(|d| crate::smart::file_path(d, p).exists())) + .filter(|p| match &p.source { + // a file left out of the listing is checked on disk: another spelling of its path, or + // a listing that missed it, keeps it + Source::File { path } => in_folder(path, dir, subfolders) && !listed.contains(path.as_str()) && !Path::new(path).exists(), + Source::Demo { .. } => false, + }) + .map(|p| p.id) + .collect(); + if gone.is_empty() { + return 0; + } + let mut forgotten = 0; + for chunk in s.catalog.forgettable_local(&gone).chunks(2000) { + let op = Op::Batch { ops: chunk.iter().map(|id| Op::RemovePhoto { id: *id }).collect() }; + match s.catalog.apply(op.clone()) { + Ok(_) => { + s.pending_log.push(op); + forgotten += chunk.len(); + } + Err(e) => log::warn!("forget Local records of files gone from {dir}: {e}"), + } + } + if forgotten > 0 { + log::info!("forgot {forgotten} untouched Local record(s) of files gone from {dir}"); + } + forgotten } /// Folders whose browse time is refreshed at most this often (keeps repeated browsing from @@ -82,7 +146,15 @@ fn stamp_browsed(s: &mut Session, root: &str, files: &[String]) { /// Photos the session still needs, whatever their state: shown, selected, being edited, or /// referred to by an undo / redo step. fn in_use(s: &mut Session) -> std::collections::HashSet { - let mut keep: std::collections::HashSet = s.visible().iter().copied().collect(); + let mut keep = held(s); + keep.extend(s.visible().iter().copied()); + keep +} + +/// Photos the session holds whatever is shown: selected, being edited, or referred to by an undo +/// / redo step. +fn held(s: &Session) -> std::collections::HashSet { + let mut keep: std::collections::HashSet = std::collections::HashSet::new(); keep.extend(s.selection.ids.iter().copied()); keep.extend(s.selection.active); keep.extend(s.previous_active); diff --git a/crates/engine/src/tests_forget_local.rs b/crates/engine/src/tests_forget_local.rs index 14a985cd4..229b262a0 100644 --- a/crates/engine/src/tests_forget_local.rs +++ b/crates/engine/src/tests_forget_local.rs @@ -216,3 +216,46 @@ fn upgraded_catalogs_forget_nothing_at_first() { assert!(local_paths(&s).is_empty(), "the untouched legacy record goes once 30 days passed"); let _ = std::fs::remove_dir_all(&root); } + +/// Issue #1392: files deleted by another app after their folder was browsed in Local stayed in +/// it, failing to load (and logging an error) on every visit. Browsing the folder again forgets +/// the untouched records of files that are gone; a rated one stays (shown as missing), so does a +/// photo added to the library, and nothing on disk is touched. +#[test] +fn browsing_again_forgets_untouched_records_of_deleted_files() { + let root = temp_dir("gone"); + let (a, lib) = (root.join("a"), root.join("lib")); + for (i, n) in ["a1.png", "a2.png", "a3.png", "a4.png"].iter().enumerate() { + png(&a.join(n), i as u8); + } + let clock = Arc::new(Mutex::new("2026-08-01T10:00:00".to_string())); + let mut s = open(&lib, &clock); + let r = s.execute("library.browse", &json!({"path": a.to_string_lossy()})).unwrap(); + assert_eq!((r["new"].as_u64(), r["forgotten"].as_u64()), (Some(4), Some(0)), "{r}"); + let id = |s: &Session, n: &str| s.catalog.photos().find(|p| p.file_name == n).unwrap().id; + let (a2, a4) = (id(&s, "a2.png"), id(&s, "a4.png")); + s.selection = Selection::single(a2); + s.execute("photo.rate", &json!({"rating": 3})).unwrap(); + s.execute("photo.addToLibrary", &json!({"ids": [a4.0]})).unwrap(); + look_elsewhere(&mut s); + for n in ["a1.png", "a2.png", "a4.png"] { + std::fs::remove_file(a.join(n)).unwrap(); + } + + let r = s.execute("library.browse", &json!({"path": a.to_string_lossy()})).unwrap(); + assert_eq!((r["new"].as_u64(), r["forgotten"].as_u64(), r["photos"].as_u64()), (Some(0), Some(1), Some(3)), "{r}"); + assert_eq!(local_paths(&s), vec!["a2.png", "a3.png"], "the untouched record of a1 is forgotten, the rated a2 stays"); + assert!(s.catalog.photo(a4).is_some_and(|p| !p.local), "a library photo of a deleted file stays"); + assert!(a.join("a3.png").exists()); + assert!(s.undo.is_empty(), "not an undo step"); + // browsing once more changes nothing + let r = s.execute("library.browse", &json!({"path": a.to_string_lossy()})).unwrap(); + assert_eq!(r["forgotten"], 0, "{r}"); + + // journaled: the library opens in the same state + let expect = s.catalog.to_snapshot(); + drop(s); + let s = open(&lib, &clock); + assert_eq!(s.catalog.to_snapshot(), expect); + let _ = std::fs::remove_dir_all(&root); +} diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index a230bd55d..57ff437c6 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -3178,6 +3178,43 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Browsing a folder again from the sidebar forgets the untouched Local records of files deleted from it by + /// another app, as `library.browse` does (issue #1392). + #[test] + fn browsing_again_from_the_ui_forgets_deleted_files() { + let mut h = demo([1200.0, 900.0]); + let t = Duration::from_secs(10); + let dir = std::env::temp_dir().join(format!("lc-browse-gone-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let img = lightcraft_raster::Rgba8::from_fn(8, 8, |x, y| [(x * 9) as u8, (y * 12) as u8, 80, 255]); + let o = lightcraft_engine::export::ExportOptions { format: lightcraft_engine::export::ExportFormat::Png, ..Default::default() }; + let png = lightcraft_engine::export::encode_image(&img, &o).unwrap(); + std::fs::write(dir.join("a.png"), &png).unwrap(); + std::fs::write(dir.join("b.png"), &png).unwrap(); + let path = dir.to_string_lossy().to_string(); + let browse = |h: &mut Headless| { + let r = h.request("engine.execute", json!({"command": "library.browse", "params": {"path": path}}), t); + assert_eq!(r["result"]["scanning"], true, "{r}"); + h.settle(SETTLE); + }; + browse(&mut h); + assert_eq!(h.app.session.visible_cloned().len(), 2); + // another app deletes the photo that isn't selected (a selected or open one stays, shown as missing) + let held: Vec = + h.app.session.selection.ids.iter().copied().chain(h.app.session.selection.active).chain(h.app.session.previous_active).collect(); + let name = |id: &lightcraft_catalog::PhotoId| h.app.session.catalog.photo(*id).map(|p| p.file_name.clone()).unwrap_or_default(); + let gone = if held.iter().any(|id| name(id) == "b.png") { "a.png" } else { "b.png" }; + std::fs::remove_file(dir.join(gone)).unwrap(); + // a later session: the browse's "Add 2 Photos" step is no longer in the undo history (photos it refers to + // are kept, like selected ones) + h.app.session.undo.clear(); + h.app.session.redo.clear(); + // the folder clicked again in the sidebar + browse(&mut h); + assert_eq!(h.app.session.visible_cloned().len(), 1, "the deleted file's record is forgotten"); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn add_folder_scans_in_the_background() { let mut h = demo([1200.0, 900.0]); diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 785f8f994..149efe727 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -581,6 +581,12 @@ pub fn poll_scan(app: &mut LightcraftApp, ctx: &egui::Context) { } app.session.import_probes = out.probes; if task.browse { + // records of files gone from the folder (deleted or moved by another app) that nothing was changed on go, + // as `library.browse` forgets them (issue #1392) + if let Some(b) = app.session.browse.clone() { + let listed: Vec = out.candidates.iter().map(|c| c.path.clone()).collect(); + app.session.forget_gone_from(&b.path, b.subfolders, &listed); + } // what the library doesn't know yet joins the Local view let queue: Vec = out.candidates.iter().filter(|c| c.duplicate != Some("path".into()) && c.error.is_none()).map(|c| c.path.clone()).collect(); diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 0b5c56c30..b7f0f000e 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -846,6 +846,7 @@ Latin stays upright. Advanced Japanese composition remains open. Regression cove | 2026-10-13 | patch | LR-EXP-QUALITY: JPEG exports use Huffman tables made for each image (same pixels, 12–14 % smaller files, issue #1373); status unchanged | | 2026-10-13 | patch | LR-BEHAV-GPU: a GPU lost while LightCraft runs (driver reset) is reported as a stop, not a failed start, and the next start renders on the CPU and says why (issue #1337); status unchanged | | 2026-10-13 | minor | LR-META-INFO: Nikon DSLR raws show their lens from the maker note; status unchanged (✅) | +| 2026-10-13 | patch | LR-IMP-LOCAL: browsing a Local folder again forgets the untouched records of files deleted or moved by another app (issue #1392); status unchanged | | 2026-10-11 | minor | KEY-SLIDER-TYPE: honor immediate Return/Escape and keep typed keys in the value editor; add regression references | | 2026-10-11 | minor | LR-PROF-CAMERACOLOR: raws with a black-and-white camera JPEG take its tone curve (issue #577) | | 2026-10-11 | minor | LR-VIEW-ZOOM and LR-VIEW-COMPARE: Compare and Reference are sharp at 1:1 (zoom windows per pane, two originals kept decoded while they are open); statuses unchanged | diff --git a/docs/whats-new.md b/docs/whats-new.md index 276c464f4..a61a6f059 100644 --- a/docs/whats-new.md +++ b/docs/whats-new.md @@ -86,6 +86,11 @@ is connected, the same review File → Import from Device opens. It's off by default. A card that's already in when LightCraft starts doesn't ask, and if a dialog is open or an import is running it waits until that's done. +### Local folders +- Photos deleted (or moved) by another app no longer stay in a Local folder, failing to load and logging an error + each time you look (issue #1392): the next time you browse the folder, in a later session, they're left out. A photo you had rated, flagged or + edited stays, marked missing, so its changes aren't lost; photos in My Photos are unchanged. + ### Hard-edged Heal and Clone spots - A spot with Feather at 0 no longer draws black pixels exactly on its radius. This also fixes spots at 0% Opacity changing pixels along their edge. The hard edge now leaves pixels on the radius unchanged.