From 7c662bc5d6e54ddbe080b01b0f4f227bd7296bc4 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:38:44 -0400 Subject: [PATCH 01/12] Issue #27: Menus: keep Quit last in the in-window File menu Registered File commands the explicit layout doesn't list were appended after Quit LightCraft. Items in LAST (app.quit) are now held back and placed after the fallback entries, in their own separator group. The macOS native menu filters Quit into the app menu and is unchanged. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/menubar.rs | 36 +++++++++++++++++++++++++++++++++-- docs/parity.md | 2 +- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/menubar.rs b/crates/ui-egui/src/menubar.rs index 051341da..f0a46f95 100644 --- a/crates/ui-egui/src/menubar.rs +++ b/crates/ui-egui/src/menubar.rs @@ -47,7 +47,8 @@ impl MenuNode { pub const MENUS: &[&str] = &["File", "Edit", "View", "Photo", "Window", "Help"]; /// Order and grouping per menu: command ids, `---` separators and `@Submenu` placeholders. -/// Entries with a menu path that aren't listed are appended at the end of their menu. +/// Entries with a menu path that aren't listed are appended at the end of their menu, before any +/// [`LAST`] item. const LAYOUT: &[(&str, &[&str])] = &[ ( "File", @@ -219,6 +220,10 @@ const LAYOUT: &[(&str, &[&str])] = &[ ), ]; +/// Items that always end their menu in their own separator group, after the entries appended +/// because the layout doesn't list them (Quit is the last item of the in-window File menu). +const LAST: &[&str] = &["app.quit"]; + /// Registry entries that are reached another way (parameterized commands are expanded into /// submenus below; others duplicate a dialog command). const HIDDEN: &[&str] = &[ @@ -535,6 +540,7 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec)> { for title in MENUS { let layout = LAYOUT.iter().find(|(t, _)| t == title).map(|(_, l)| *l).unwrap_or(&[]); let mut items = Vec::new(); + let mut last = Vec::new(); let sub = |name: &str, used: &mut [bool]| -> Vec { let mut children = expanded(app, name).unwrap_or_default(); let mut dialogs = Vec::new(); @@ -559,7 +565,7 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec)> { items.push(MenuNode::Submenu { label: name.to_string(), children }); } else if let Some(i) = entries.iter().position(|e| e.id == *slot && e.menu.first().map(String::as_str) == Some(title)) { used[i] = true; - items.push(node(app, &entries[i])); + if LAST.contains(slot) { last.push(node(app, &entries[i])) } else { items.push(node(app, &entries[i])) } } } // everything else that names this menu @@ -580,6 +586,10 @@ pub fn menu_bar(app: &LightcraftApp) -> Vec<(String, Vec)> { let children = sub(&name, &mut used); items.push(MenuNode::Submenu { label: name, children }); } + if !last.is_empty() { + items.push(MenuNode::Separator); + items.extend(last); + } bar.push((title.to_string(), tidy(items))); } bar @@ -773,6 +783,28 @@ mod tests { assert_eq!(*checked, Some(app.ui.filmstrip)); } + #[test] + fn quit_ends_the_file_menu_after_unlisted_entries() { + let app = app(); + // the File menu has registry entries the layout doesn't list (appended as fallbacks) + let listed = LAYOUT.iter().find(|(t, _)| *t == "File").unwrap().1; + let unlisted: Vec = crate::menus::menu_entries(&app) + .into_iter() + .filter(|e| e.menu.first().map(String::as_str) == Some("File") && !listed.contains(&e.id.as_str()) && !HIDDEN.contains(&e.id.as_str())) + .map(|e| e.id) + .collect(); + assert!(!unlisted.is_empty(), "no fallback File entries to order"); + let bar = menu_bar(&app); + let file = &bar.iter().find(|(t, _)| t == "File").unwrap().1; + // Quit is the last item, alone in its separator group + assert!(matches!(file.last(), Some(MenuNode::Item { id, .. }) if id == "app.quit"), "{file:?}"); + assert_eq!(file.get(file.len() - 2), Some(&MenuNode::Separator)); + // every fallback entry is still reachable, before Quit + for id in &unlisted { + assert!(find(&file[..file.len() - 1], id).is_some(), "{id} missing before Quit"); + } + } + #[test] fn export_with_preset_submenu() { let mut app = app(); diff --git a/docs/parity.md b/docs/parity.md index a33caa46..17411235 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -504,7 +504,7 @@ visible menu bar yet: the menu model is only exposed through the control channel | MENU-APP-SYNC | Sync status / pause | OOS | 🚫 | | | | MENU-APP-SIGNOUT | Sign out | OOS | 🚫 | | | | MENU-APP-HIDE | Hide / hide others / show all | P1 | ✅ | `apps/lightcraft/src/native_menu.rs` | the system's own items in the app menu (⌘H, ⌥⌘H) | -| MENU-APP-QUIT | Quit | P0 | ✅ | `cmd:app.quit`, `apps/lightcraft/src/native_menu.rs` | macOS: app menu (native); elsewhere: File → Quit LightCraft | +| MENU-APP-QUIT | Quit | P0 | ✅ | `cmd:app.quit`, `apps/lightcraft/src/native_menu.rs` | macOS: app menu (native); elsewhere: File → Quit LightCraft, always the last item in its own group (after commands the File layout does not list) | | MENU-FILE-ADDPHOTOS | Add Photos… | P0 | ✅ | `cmd:file.addPhotos` | | | MENU-FILE-ADDFOLDER | Add Folder… | P0 | ✅ | `cmd:file.addFolder`, `cmd:library.importPreview` | folder picker (desktop) → the import review, subfolders included; `path` param for agents | | MENU-FILE-MIGRATE | Migrate photos | OOS | 🚫 | | | From 20d156c8784d9c221f76d48f573ae33e31bd1122 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:39:11 -0400 Subject: [PATCH 02/12] Issue #34: Missing Photos checks library photos only Missing Photos (sidebar count, the view, library.missing, Find Missing) now shares one scope rule, cmd::missing::checked_path: non-deleted library photos backed by a file. Local browse records, which can outnumber the library by orders of magnitude, are never stat'ed. The Missing view checks only the photos its query kept instead of every catalog record. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/missing.rs | 40 ++++++++++++++++++++++++------- crates/engine/src/lib.rs | 5 ++-- crates/engine/src/tests_import.rs | 28 ++++++++++++++++++++++ crates/ui-egui/src/panels/left.rs | 19 +++++---------- docs/parity.md | 2 +- 5 files changed, 70 insertions(+), 24 deletions(-) diff --git a/crates/engine/src/cmd/missing.rs b/crates/engine/src/cmd/missing.rs index 8ec96355..c7d1a2ec 100644 --- a/crates/engine/src/cmd/missing.rs +++ b/crates/engine/src/cmd/missing.rs @@ -5,24 +5,48 @@ use std::collections::HashMap; use std::path::Path; -use lightcraft_catalog::{Op, PhotoId, Source}; +use lightcraft_catalog::{Catalog, Op, Photo, PhotoId, Source}; use serde_json::{Value, json}; use super::{CommandSpec, always, bad, cmd, str_param}; use crate::{Result, Session}; +/// The file Missing Photos checks for `p`, if it is in scope: a library photo (not in Recently +/// Deleted, not a Local-only record seen while browsing a folder) whose original is a file. +/// +/// Local browse records are out of scope on purpose: browsing can leave tens of thousands of +/// them in the catalog, they never show in library views (Missing Photos included), and a +/// folder that is gone is simply not browsed again. The sidebar count, the Missing Photos view, +/// `library.missing` and Find Missing Photos all use this one rule. +pub fn checked_path(p: &Photo) -> Option<&str> { + match &p.source { + Source::File { path } if p.in_library() => Some(path), + _ => None, + } +} + +/// Every in-scope photo's file ([`checked_path`]), without touching the disk. +pub fn candidates(cat: &Catalog) -> Vec { + cat.photos().filter_map(|p| checked_path(p).map(str::to_string)).collect() +} + +/// [`missing`] with the existence check supplied (tests count the file-system calls). +pub fn missing_with(cat: &Catalog, mut exists: impl FnMut(&str) -> bool) -> Vec<(PhotoId, String)> { + cat.photos().filter_map(|p| checked_path(p).filter(|f| !exists(f)).map(|f| (p.id, f.to_string()))).collect() +} + /// Library photos whose original file can't be found: (id, path). pub fn missing(s: &Session) -> Vec<(PhotoId, String)> { if cfg!(target_arch = "wasm32") { return Vec::new(); } - s.catalog - .photos() - .filter_map(|p| match &p.source { - Source::File { path } if !p.deleted && !Path::new(path).exists() => Some((p.id, path.clone())), - _ => None, - }) - .collect() + missing_with(&s.catalog, |f| Path::new(f).exists()) +} + +/// Whether photo `id` is in scope and its file is gone (the Missing Photos view checks only +/// the photos its query already narrowed to). +pub fn is_missing(cat: &Catalog, id: PhotoId) -> bool { + !cfg!(target_arch = "wasm32") && cat.photo(id).and_then(|p| checked_path(p)).is_some_and(|f| !Path::new(f).exists()) } fn relink_op(id: PhotoId, path: &str) -> Op { diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index ce0404f2..192d90be 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -507,8 +507,9 @@ impl Session { }); } if self.source == LibrarySource::Missing { - let missing: std::collections::HashSet = cmd::missing::missing(self).into_iter().map(|m| m.0).collect(); - self.visible.retain(|id| missing.contains(id)); + // only the photos the query kept (library photos, not Local browse records) are checked + let cat = &self.catalog; + self.visible.retain(|id| cmd::missing::is_missing(cat, *id)); } if self.source != LibrarySource::RecentlyDeleted { self.visible = self.catalog.arrange_stacks(&self.visible); diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 2a1480ba..b927dd1b 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -585,3 +585,31 @@ fn smart_previews_folder_is_chosen_per_library() { assert_eq!(r["custom"], false, "{r}"); assert_eq!(r["path"], lib.join("Smart Previews").to_string_lossy().as_ref()); } + +/// Missing Photos covers library photos only: a small library next to many Local browse +/// records (and a deleted photo) checks — and lists, and counts — the library files alone. +#[test] +fn missing_photos_skip_local_browse_records() { + use lightcraft_catalog::{Op, Photo, PhotoId, Source}; + let gone = std::env::temp_dir().join(format!("lc-missing-scope-{}", std::process::id())); + let mut s = Session::new(); + let mut ops = Vec::new(); + for i in 0..205u64 { + let path = gone.join(format!("IMG_{i:04}.jpg")).to_string_lossy().to_string(); + let mut p = Photo::new(PhotoId(i + 1), Source::File { path }, &format!("IMG_{i:04}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + p.local = i >= 5; // 5 library photos, 200 seen while browsing + p.deleted = i == 4; + ops.push(Op::AddPhoto { photo: Box::new(p) }); + } + s.commit("Add", Op::Batch { ops }).unwrap(); + let mut checked = 0; + let lost = crate::cmd::missing::missing_with(&s.catalog, |_| { + checked += 1; + false + }); + assert_eq!((checked, lost.len()), (4, 4), "only the non-deleted library files are checked"); + assert_eq!(crate::cmd::missing::candidates(&s.catalog).len(), 4, "the sidebar count checks the same files"); + assert_eq!(s.execute("library.missing", &json!({})).unwrap().as_array().map(Vec::len), Some(4)); + s.execute("library.source", &json!({"kind": "missing"})).unwrap(); + assert_eq!(s.visible_cloned().len(), 4, "the view lists what the count counts"); +} diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 72516322..9e68ac7a 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -146,9 +146,10 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { }); } -/// How many library photos have no file. Checking stats every file, which on a network share -/// takes seconds, so it runs on a worker thread: the count shown is the last finished one, -/// refreshed at most every 5 s (or when the catalog changed). +/// How many library photos have no file (Local browse records are not checked; see +/// `cmd::missing::checked_path`). Checking stats every file, which on a network share takes +/// seconds, so it runs on a worker thread: the count shown is the last finished one, refreshed +/// at most every 5 s, and at once (after the running check) when the catalog changed. fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize { use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering::Relaxed}; #[derive(Clone, Default)] @@ -164,16 +165,8 @@ fn missing_count(app: &mut LightcraftApp, ui: &mut egui::Ui) -> usize { let mut job: Job = ui.data(|d| d.get_temp(id)).unwrap_or(Job { rev: u64::MAX, at: f64::MIN, ..Default::default() }); let fresh = now - job.at < 5.0 && job.rev == rev; if !fresh && !job.running.load(Relaxed) { - let paths: Vec = app - .session - .catalog - .photos() - .filter(|p| !p.deleted) - .filter_map(|p| match &p.source { - lightcraft_catalog::Source::File { path } => Some(path.clone()), - _ => None, - }) - .collect(); + // the same scope as the Missing Photos view: library photos only, never Local browse records + let paths = lightcraft_engine::cmd::missing::candidates(&app.session.catalog); job.running.store(true, Relaxed); job.at = now; job.rev = rev; diff --git a/docs/parity.md b/docs/parity.md index a33caa46..51edb835 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -708,7 +708,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-PEOPLE | People view | P2 | ⬜ | | | | LRC-LIB-COMMENTS | Comments panel | P2 | ⬜ | | | | LRC-LIB-VISUALSEARCH | Find similar photos | P2 | ✅ | `cmd:library.findSimilar`, `crates/pipeline/src/cull.rs` (`signature`) | photo menu / Photo ▸ Find Similar Photos: look-alike photos (composition and tones), most similar first, as a filter (`only`) | -| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate` | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. No missing-photos collection | +| LRC-LIB-MISSING | Missing files & relink | P1 | ✅ | `cmd:library.missing`, `cmd:photo.relink`, `cmd:library.findMissing`, `cmd:file.findMissing`, `cmd:photo.locate`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (same name and size anywhere in a folder, one undo step), photo menu → Locate Missing File…; unreadable files show "!" in the grid and a reason in the loupe; undo never moves files. Missing Photos in the sidebar (counted on a worker thread); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike | | LRC-LIB-CONVERT | Convert to DNG | P2 | ✅ | `cmd:photo.convertToDng`, `crates/engine/src/cmd/convert.rs` | Photo ▸ Convert to DNG: lossless DNG next to the raw with the settings embedded, photo (and its virtual copies) relinked; originals kept; undoable | | LRC-LIB-PREVIEWS | Build / discard previews | P1 | ✅ | `cmd:library.buildPreviews`, `cmd:library.previewProgress`, `cmd:library.cancelPreviews`, `cmd:library.clearPreviews`, `crates/engine/src/cmd/previews.rs`, `crates/preview/src/lib.rs` | File ▸ Previews: build standard-sized (Settings → Performance size) or 1:1 previews of the selected / visible photos in the background (progress toasts, stop), discard the cache; disk thumbnail + view cache; no smart previews (offline proxies) | | LRC-LIB-SLIDESHOW-IMPROMPTU | Impromptu slideshow | P2 | ✅ | `cmd:view.slideshow` | see LR-VIEW-SLIDESHOW | From 37ab41478d1837db835aa55d611c6259cae5195e Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:40:54 -0400 Subject: [PATCH 03/12] Issue #19: XMP sidecar capture time fills in when the file has none A sidecar's exif:DateTimeOriginal / photoshop:DateCreated / xmp:CreateDate now reaches Photo.captured on import and on Read Metadata from File when the file has no embedded capture time (embedded wins). On import the sidecar is read before copying, so the date also files and names copies. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/import.rs | 39 ++++++++++++++----------- crates/engine/src/sidecar.rs | 17 +++++++++-- crates/engine/src/tests_xmp.rs | 52 ++++++++++++++++++++++++++++++++++ docs/parity.md | 2 +- docs/xmp-interop.md | 5 +++- 5 files changed, 95 insertions(+), 20 deletions(-) diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 5b9211d3..bc57fe6d 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -8,8 +8,9 @@ //! 4. Skip **duplicates by content** (same bytes already in the library, or twice in this batch). //! 5. *Add* in place (the photo points at the original file) or *copy* into the library's //! `Originals/YYYY/YYYY-MM-DD/` folder (names made unique) and point at the copy. -//! 6. Read each photo's XMP sidecar (or a raw/DNG file's embedded XMP): the sidecar wins for -//! metadata and develop settings are restored (see [`crate::sidecar`]). +//! 6. Apply each photo's XMP sidecar (or a raw/DNG file's embedded XMP): the sidecar wins for +//! metadata and develop settings are restored (see [`crate::sidecar`]). Its capture time fills +//! in when the file has none (read before step 5, so it also files and names the copy). //! 7. Optionally apply a preset and add keywords (the import dialog's options). //! 8. Commit all new photos as one undoable op. //! @@ -553,6 +554,23 @@ pub fn import_with(s: &mut Session, paths: &[String], opts: &ImportOptions) -> c report.duplicates.push(Duplicate { path, existing: Some(id.0), reason: "content" }); continue; } + // the XMP sidecar (or a raw's embedded XMP), read before copying: its capture time files + // and names the copy when the file itself has none + let raw = info.kind == MediaKind::Raw; + let packet = crate::sidecar::find_sidecar(&path, s.xmp.naming) + .and_then(|f| std::fs::read_to_string(f).ok()) + .or_else(|| info.xmp.clone().filter(|_| raw)); + let sidecar = packet.and_then(|x| match crate::sidecar::parse_sidecar(&x, raw).map(|sc| sc.resolve_label(&s.catalog)) { + Ok(sc) => (sc != crate::sidecar::SidecarData::default()).then_some(sc), + Err(e) => { + log::warn!("import {path}: XMP: {e}"); + None + } + }); + let mut info = info; + if info.captured.is_none() { + info.captured = sidecar.as_ref().and_then(|sc| sc.captured.clone()); + } let stored = match (mode, ©_root) { (ImportMode::Copy, Some(root)) if !Path::new(&path).starts_with(root) && !lib_dir.as_ref().is_some_and(|l| Path::new(&path).starts_with(l)) => @@ -583,7 +601,6 @@ pub fn import_with(s: &mut Session, paths: &[String], opts: &ImportOptions) -> c } // Copy as DNG: the copied raw becomes a DNG (the copy is ours to replace) let mut stored = stored; - let mut info = info; if opts.convert_dng && mode == ImportMode::Copy && stored != path && info.kind == MediaKind::Raw && !info.format.eq_ignore_ascii_case("DNG") { match crate::cmd::convert::write_dng_for(s, &stored, String::new()) { Ok(dng) => { @@ -606,19 +623,9 @@ pub fn import_with(s: &mut Session, paths: &[String], opts: &ImportOptions) -> c p.embedded_lens = info.embedded_lens; p.preview_only = info.preview_only.clone(); apply_import_defaults(s, &mut p); - let raw = p.kind == lightcraft_catalog::MediaKind::Raw; - let packet = crate::sidecar::find_sidecar(&path, s.xmp.naming) - .and_then(|f| std::fs::read_to_string(f).ok()) - .or_else(|| info.xmp.clone().filter(|_| raw)); - if let Some(x) = packet { - match crate::sidecar::parse_sidecar(&x, raw).map(|sc| sc.resolve_label(&s.catalog)) { - Ok(sc) if sc != crate::sidecar::SidecarData::default() => { - crate::sidecar::merge_into(&mut p, &sc, &now); - report.sidecars += 1; - } - Ok(_) => {} - Err(e) => log::warn!("import {path}: XMP: {e}"), - } + if let Some(sc) = &sidecar { + crate::sidecar::merge_into(&mut p, sc, &now); + report.sidecars += 1; } if let Some(mp) = opts.metadata_preset.as_ref().and_then(|n| s.metadata_presets.iter().find(|m| m.name.eq_ignore_ascii_case(n))) { crate::cmd::metadata::apply_to(&mut p.meta, &mp.fields); diff --git a/crates/engine/src/sidecar.rs b/crates/engine/src/sidecar.rs index 714478d3..5deedd3f 100644 --- a/crates/engine/src/sidecar.rs +++ b/crates/engine/src/sidecar.rs @@ -5,7 +5,9 @@ //! creator, keywords, capture time and GPS in the standard namespaces, plus our complete develop //! settings (`lc:settings`, JSON), the pick/reject flag (`lc:flag`) and location (`lc:location`). //! -//! **Read** (on import, and `photo.readMetadataFromFile`): the sidecar wins for metadata; develop +//! **Read** (on import, and `photo.readMetadataFromFile`): the sidecar wins for metadata, except +//! the capture time: a time embedded in the file wins, and the sidecar's (`exif:DateTimeOriginal`, +//! else `photoshop:DateCreated`, else `xmp:CreateDate`) fills in only when the file has none; develop //! settings are restored from `lc:settings` when present, else mapped from interoperable `crs:` //! fields ([`crate::crs`]). For raw/DNG files without a sidecar the file's embedded XMP is used. @@ -80,6 +82,9 @@ pub struct SidecarData { pub alt_text: Option, pub extended_description: Option, pub keywords: Option>, + /// Capture time (ISO 8601) from `exif:DateTimeOriginal`, `photoshop:DateCreated` or + /// `xmp:CreateDate` (first found). Used only when the file itself has no capture time. + pub captured: Option, pub develop: Option, } @@ -108,6 +113,7 @@ pub fn parse_sidecar(xmp: &str, raw: bool) -> std::result::Result std::result::Result bool { + if p.captured.is_none() { + p.captured = sc.captured.clone(); + } if let Some(r) = sc.rating { p.rating = r; } @@ -306,6 +316,9 @@ impl Session { Op::SetLabel { id, label: q.label }, Op::SetMeta { id, meta: Box::new(q.meta.clone()) }, ]; + if q.captured != p.captured { + ops.push(Op::SetCaptured { id, captured: q.captured.clone() }); + } if develop_changed { ops.extend(self.develop_op(id, (*q.develop).clone(), "Read Metadata from File")); } diff --git a/crates/engine/src/tests_xmp.rs b/crates/engine/src/tests_xmp.rs index 2f00a315..c0331ea7 100644 --- a/crates/engine/src/tests_xmp.rs +++ b/crates/engine/src/tests_xmp.rs @@ -445,3 +445,55 @@ fn dng_export_compression_choices() { } assert!(sizes[2].1 > sizes[0].1 && sizes[2].1 > sizes[1].1, "uncompressed is the biggest: {sizes:?}"); } + +fn write_jpeg(path: &Path, capture: Option<&str>) { + let (w, h) = (32usize, 24usize); + let data: Vec<[u8; 4]> = (0..w * h).map(|i| [(i % w * 7) as u8, (i / w * 9) as u8, 90, 255]).collect(); + let img = lightcraft_raster::Rgba8 { width: w, height: h, data }; + let exif = capture.map(|c| { + lightcraft_meta::write_exif(&lightcraft_meta::Metadata { capture_time: lightcraft_meta::DateTime::parse_iso(c), ..Default::default() }) + }); + let meta = lightcraft_codecs::EncodeMeta { exif: exif.as_deref(), ..Default::default() }; + let bytes = lightcraft_codecs::encode_jpeg(&lightcraft_codecs::EncodeImage::rgba8(&img), 90, Default::default(), &meta).unwrap(); + std::fs::write(path, bytes).unwrap(); +} + +const DATED_SIDECAR: &str = r#" + + + +"#; + +#[test] +fn sidecar_capture_time_fills_in_when_the_file_has_none() { + let src = temp_dir("sidecar-date"); + write_jpeg(&src.join("sample.jpg"), None); + std::fs::write(src.join("sample.jpg.xmp"), DATED_SIDECAR).unwrap(); + // the embedded capture time wins over the sidecar's + write_jpeg(&src.join("dated.jpg"), Some("2020-05-06T07:08:09")); + std::fs::write(src.join("dated.jpg.xmp"), DATED_SIDECAR).unwrap(); + let lib = temp_dir("sidecar-date-lib"); + let mut s = Session::new().with_fs(); + s.open_library(&lib, false).unwrap(); + // copy: the sidecar's date also files the copy + let r = s.execute("library.import", &json!({"paths": [src.to_string_lossy()], "mode": "copy"})).unwrap(); + assert_eq!(r["sidecars"], 2, "{r}"); + let by_name = |s: &Session, n: &str| s.catalog.photos().find(|p| p.file_name == n).unwrap().clone(); + let sample = by_name(&s, "sample.jpg"); + assert_eq!(sample.captured.as_deref(), Some("2026-01-14T05:58:48")); + let lightcraft_catalog::Source::File { path } = &sample.source else { panic!("not a file") }; + assert!(Path::new(path).starts_with(lib.join("Originals").join("2026").join("2026-01-14")), "{path}"); + assert_eq!(by_name(&s, "dated.jpg").captured.as_deref(), Some("2020-05-06T07:08:09")); + + // Read Metadata from File fills in a missing capture time too + let id = sample.id; + s.catalog.apply(lightcraft_catalog::Op::SetCaptured { id, captured: None }).unwrap(); + std::fs::copy(src.join("sample.jpg.xmp"), format!("{path}.xmp")).unwrap(); + s.execute("library.select", &json!({"ids": [id.0]})).unwrap(); + s.execute("photo.readMetadataFromFile", &json!({})).unwrap(); + assert_eq!(s.catalog.photo(id).unwrap().captured.as_deref(), Some("2026-01-14T05:58:48")); + let _ = std::fs::remove_dir_all(&src); + let _ = std::fs::remove_dir_all(&lib); +} diff --git a/docs/parity.md b/docs/parity.md index a33caa46..cb4462dd 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -194,7 +194,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-META-COPYRIGHT-DEFAULT | Default copyright on import | P1 | ✅ | `cmd:library.preferences` (`import.copyright`, `import.creator`), `crates/ui-egui/src/panels/settings.rs` | Settings → Import → Metadata; fills only photos without their own | | LR-META-LOCATION | Location editing | P2 | 🟡 | `cmd:photo.setMeta` (`location`, `city`, `state`, `country`, `gps`) | place fields and GPS (decimal or degrees / minutes / seconds) edited in Info, read/written as IPTC Core / Photoshop / EXIF XMP; Show on Map opens OpenStreetMap; no embedded map, no geocoding | | LR-META-COPYPASTE | Copy / paste metadata | P2 | ✅ | `cmd:photo.copyMetadata`, `cmd:photo.pasteMetadata` | title, caption, alt text, extended description, copyright, creator, place fields, keywords; `fields` picks a subset | -| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | | +| LR-META-XMP | XMP read/write | P0 | ✅ | `cmd:photo.saveMetadataToFile`, `cmd:photo.readMetadataFromFile`, `cmd:library.xmpPreferences`, `crates/engine/src/sidecar.rs`, `docs/xmp-interop.md` | sidecar wins for metadata; a sidecar capture time (`exif:DateTimeOriginal` / `photoshop:DateCreated` / `xmp:CreateDate`) fills in only when the file has none | | LR-META-EXIF-FULL | Full EXIF/IPTC [Classic] | P1 | ✅ | `cmd:photo.allMetadata`, `cmd:dialog.allMetadata`, `crates/meta/src/tags.rs`, `crates/meta/src/exif.rs`, `crates/meta/src/iptc.rs` | read and written on export; Info ▸ All Metadata…: every TIFF / EXIF / GPS / interop tag (named, common values spelled out) and the XMP fields, searchable; metadata presets | ## F. Edit panel — global adjustments (EDIT) diff --git a/docs/xmp-interop.md b/docs/xmp-interop.md index eea43e2a..f9a68ea9 100644 --- a/docs/xmp-interop.md +++ b/docs/xmp-interop.md @@ -29,7 +29,10 @@ What we write (standard namespaces, so other tools can read the metadata): `lc:` is `http://ns.lightcraft.app/lc/1.0/`. Reading merges into the catalog with the **sidecar winning** for every field it states; fields it doesn't state are -kept. `xmp:Rating="-1"` (the XMP convention for rejected) sets the reject flag. Develop settings come from +kept. The one exception is the **capture time**: a time embedded in the file (EXIF/IPTC) always wins; when the file +has none, the sidecar's `exif:DateTimeOriginal`, else `photoshop:DateCreated`, else `xmp:CreateDate` (ISO 8601) is +used — on import (it then also files a copied photo in its date folder) and by `photo.readMetadataFromFile` (undoable). +`xmp:Rating="-1"` (the XMP convention for rejected) sets the reject flag. Develop settings come from `lc:settings` when present (exact); otherwise from the `crs:` fields below (approximate). ## Reading `crs:` develop fields From 1a6a9de7eedb72d95a87dca1e16fff71e56a7661 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:50:29 -0400 Subject: [PATCH 04/12] Issue #33: Devices: serve the cached card list and rescan in the background MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Menus are rebuilt every frame, and File → Add from Device called devices(), which scanned every mount (D:..Z: on Windows) while holding the cache mutex once its 2 s cache expired. devices() now returns the last scan at once and starts at most one background rescan when it is stale; the scan runs without the lock. A changed result calls an on_change hook (the UI requests a repaint), so hot-plugged cards still appear. library.devices and Add from Device without a path scan synchronously (devices_now). Volume roots are probed for DCIM directly instead of listing them (Windows never lists a root). No threads or clock on wasm. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 2 +- crates/engine/src/devices.rs | 221 ++++++++++++++++++++++++++++--- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/menus.rs | 2 +- docs/parity.md | 2 +- 5 files changed, 207 insertions(+), 23 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index b4c3b292..d8fad84b 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -250,7 +250,7 @@ pub fn specs() -> Vec { Ok(json!({"selected": s.selection.ids.len()})) }), cmd!(query "library.devices", "Cameras and Cards", [], None, "{} → [{name, path (its DCIM folder), root}] — mounted volumes with a DCIM folder", always, |_, _| { - Ok(serde_json::to_value(crate::devices::devices()).unwrap_or_default()) + Ok(serde_json::to_value(crate::devices::devices_now()).unwrap_or_default()) }), cmd!(query "photo.copyMetadata", "Copy Metadata", ["Photo"], None, "{} — title, caption, copyright, creator, location and keywords of the active photo", has_active, |s, _| { let id = s.active().ok_or_else(|| bad("photo.copyMetadata", "no active photo"))?; diff --git a/crates/engine/src/devices.rs b/crates/engine/src/devices.rs index 0ad26922..bc82a727 100644 --- a/crates/engine/src/devices.rs +++ b/crates/engine/src/devices.rs @@ -1,6 +1,15 @@ //! Cameras and memory cards: mounted volumes with a `DCIM` folder (the DCF layout every camera //! writes). Listed by `library.devices`; the app offers them under File → Add from Device, and //! the import review copies from them into the library. +//! +//! Menus are rebuilt every frame (even with their popups closed), so [`devices`] never touches the +//! disk: it serves the last scan and, when that is older than [`MAX_AGE`], starts one background +//! rescan (coalesced; no lock is held while it runs). A changed result calls the [`on_change`] +//! hook, which the UI uses to repaint, so hot-plugged cards still appear. [`devices_now`] scans +//! synchronously for explicit requests (`library.devices`, Add from Device without a path). + +use std::sync::{Arc, Mutex}; +use std::time::Duration; use serde::Serialize; @@ -14,6 +23,9 @@ pub struct Device { pub root: String, } +/// How long a scan is served before [`devices`] starts another one. +pub const MAX_AGE: Duration = Duration::from_secs(2); + /// Folders whose children are mounted volumes on this platform. `LIGHTCRAFT_DEVICE_ROOTS` /// (paths joined like `PATH`) replaces them (tests, unusual mounts). fn mount_parents() -> Vec { @@ -34,21 +46,114 @@ fn mount_parents() -> Vec { out } -/// Mounted volumes that look like a camera or a memory card. Cached for a couple of seconds: -/// menus ask often, and listing mounts can be slow (network volumes). -pub fn devices() -> Vec { - use std::sync::Mutex; - use std::time::{Duration, Instant}; - static CACHE: Mutex)>> = Mutex::new(None); - let mut c = CACHE.lock().unwrap_or_else(|e| e.into_inner()); - if let Some((t, d)) = c.as_ref() - && t.elapsed() < Duration::from_secs(2) - { - return d.clone(); +type Scanner = dyn Fn() -> Vec + Send + Sync; +type Notify = dyn Fn() + Send + Sync; + +#[derive(Default)] +struct WatchState { + list: Vec, + /// When the served list was scanned (`None`: never). No clock (and no threads) on wasm, where + /// there are no mounted volumes either. + #[cfg(not(target_arch = "wasm32"))] + scanned: Option, + /// A background scan is running. + refreshing: bool, +} + +/// A cached device list, refreshed off the calling thread. +pub struct DeviceWatch { + scanner: Box, + #[cfg_attr(target_arch = "wasm32", allow(dead_code))] + max_age: Duration, + state: Mutex, + notify: Mutex>>, +} + +impl DeviceWatch { + pub fn new(scanner: impl Fn() -> Vec + Send + Sync + 'static, max_age: Duration) -> Arc { + Arc::new(DeviceWatch { scanner: Box::new(scanner), max_age, state: Mutex::default(), notify: Mutex::new(None) }) } - let d = scan(); - *c = Some((Instant::now(), d.clone())); - d + + fn state(&self) -> std::sync::MutexGuard<'_, WatchState> { + self.state.lock().unwrap_or_else(|e| e.into_inner()) + } + + /// Called (on the scanning thread) whenever a scan changes the list. + pub fn on_change(&self, f: impl Fn() + Send + Sync + 'static) { + *self.notify.lock().unwrap_or_else(|e| e.into_inner()) = Some(Arc::new(f)); + } + + /// The last scan, immediately; starts a background rescan when it is stale and none is running. + pub fn get(self: &Arc) -> Vec { + let mut s = self.state(); + #[cfg(not(target_arch = "wasm32"))] + if !s.refreshing && s.scanned.is_none_or(|t| t.elapsed() >= self.max_age) { + s.refreshing = true; + let me = self.clone(); + // the scan runs without the state lock; `get` only ever waits for a list clone + let spawned = std::thread::Builder::new().name("lightcraft-devices".into()).spawn(move || { + let list = (me.scanner)(); + me.store(list); + }); + if spawned.is_err() { + s.refreshing = false; + } + } + s.list.clone() + } + + /// Scan now on this thread, and serve the result to later [`DeviceWatch::get`] calls. + pub fn scan_now(&self) -> Vec { + let list = (self.scanner)(); + self.store(list.clone()); + list + } + + fn store(&self, list: Vec) { + let changed = { + let mut s = self.state(); + s.refreshing = false; + #[cfg(not(target_arch = "wasm32"))] + { + s.scanned = Some(std::time::Instant::now()); + } + let changed = s.list != list; + s.list = list; + changed + }; + if changed { + let f = self.notify.lock().unwrap_or_else(|e| e.into_inner()).clone(); + if let Some(f) = f { + f(); + } + } + } + + /// A background scan is running. + pub fn refreshing(&self) -> bool { + self.state().refreshing + } +} + +fn global() -> &'static Arc { + static WATCH: std::sync::OnceLock> = std::sync::OnceLock::new(); + WATCH.get_or_init(|| DeviceWatch::new(scan, MAX_AGE)) +} + +/// Mounted volumes that look like a camera or a memory card, without blocking: the last scan, +/// refreshed in the background when older than [`MAX_AGE`] (empty until the first scan lands). +pub fn devices() -> Vec { + global().get() +} + +/// Scan the mounted volumes now (blocking), for explicit requests. +pub fn devices_now() -> Vec { + global().scan_now() +} + +/// Run `f` whenever a scan changes the device list (the UI requests a repaint). +pub fn on_change(f: impl Fn() + Send + Sync + 'static) { + global().on_change(f); } fn scan() -> Vec { @@ -73,10 +178,7 @@ pub fn devices_in(parents: &[std::path::PathBuf], extra: Vec roots.extend(extra); let mut out: Vec = Vec::new(); for root in roots { - let Ok(rd) = std::fs::read_dir(&root) else { continue }; - let Some(dcim) = rd.flatten().map(|e| e.path()).find(|p| p.is_dir() && p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM"))) else { - continue; - }; + let Some(dcim) = dcim_in(&root) else { continue }; let name = root.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| root.to_string_lossy().to_string()); if !out.iter().any(|d| d.root == root.to_string_lossy()) { out.push(Device { name, path: dcim.to_string_lossy().to_string(), root: root.to_string_lossy().to_string() }); @@ -85,9 +187,22 @@ pub fn devices_in(parents: &[std::path::PathBuf], extra: Vec out } +/// The volume's `DCIM` folder: the usual spellings are checked directly, so a volume's root is +/// only listed (other spellings on a case-sensitive volume) off Windows, whose drives aren't. +fn dcim_in(root: &std::path::Path) -> Option { + if let Some(p) = ["DCIM", "dcim"].iter().map(|n| root.join(n)).find(|p| p.is_dir()) { + return Some(p); + } + if cfg!(windows) { + return None; + } + std::fs::read_dir(root).ok()?.flatten().map(|e| e.path()).find(|p| p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM")) && p.is_dir()) +} + #[cfg(test)] mod tests { use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; #[test] fn volumes_with_a_dcim_folder_are_devices() { @@ -96,10 +211,76 @@ mod tests { std::fs::create_dir_all(base.join("EOS_DIGITAL/DCIM/100CANON")).unwrap(); std::fs::create_dir_all(base.join("Backup/Photos")).unwrap(); std::fs::create_dir_all(base.join("SD/dcim")).unwrap(); + std::fs::create_dir_all(base.join("X/Dcim")).unwrap(); let d = devices_in(std::slice::from_ref(&base), Vec::new()); let names: Vec<&str> = d.iter().map(|x| x.name.as_str()).collect(); - assert_eq!(names, ["EOS_DIGITAL", "SD"], "{d:?}"); - assert!(d[0].path.ends_with("DCIM") && d[1].path.ends_with("dcim")); + assert_eq!(names, ["EOS_DIGITAL", "SD", "X"], "{d:?}"); + // the DCIM folder itself, in a spelling that opens on that volume + for x in &d { + let p = std::path::Path::new(&x.path); + assert!(p.is_dir() && p.file_name().is_some_and(|n| n.eq_ignore_ascii_case("DCIM")), "{x:?}"); + } let _ = std::fs::remove_dir_all(&base); } + + fn card(name: &str) -> Device { + Device { name: name.into(), path: format!("/v/{name}/DCIM"), root: format!("/v/{name}") } + } + + /// Wait (bounded) for `f`. + fn until(f: impl Fn() -> bool) -> bool { + let t = std::time::Instant::now(); + while t.elapsed() < Duration::from_secs(20) { + if f() { + return true; + } + std::thread::sleep(Duration::from_millis(5)); + } + false + } + + #[test] + fn slow_scans_never_block_and_hot_plug_still_arrives() { + // a scanner that takes 1.5 s (a sleeping network drive) and counts its runs + const SLOW: Duration = Duration::from_millis(1500); + let mounted = Arc::new(Mutex::new(Vec::::new())); + let runs = Arc::new(AtomicUsize::new(0)); + let (m, r) = (mounted.clone(), runs.clone()); + let watch = DeviceWatch::new( + move || { + r.fetch_add(1, Ordering::SeqCst); + std::thread::sleep(SLOW); + m.lock().unwrap().clone() + }, + Duration::from_millis(20), + ); + let changes = Arc::new(AtomicUsize::new(0)); + let c = changes.clone(); + watch.on_change(move || { + c.fetch_add(1, Ordering::SeqCst); + }); + // cold: served at once (empty) while one scan runs in the background + let t = std::time::Instant::now(); + for _ in 0..50 { + assert!(watch.get().is_empty()); + } + assert!(t.elapsed() < SLOW / 2, "get() waited for the scan: {:?}", t.elapsed()); + assert!(watch.refreshing()); + assert!(until(|| !watch.refreshing())); + assert_eq!(runs.load(Ordering::SeqCst), 1, "requests while scanning are coalesced"); + assert_eq!(changes.load(Ordering::SeqCst), 0, "nothing changed"); + // a card is plugged in: the next stale request starts a rescan, still without waiting + mounted.lock().unwrap().push(card("EOS_DIGITAL")); + std::thread::sleep(Duration::from_millis(40)); + let t = std::time::Instant::now(); + assert!(watch.get().is_empty(), "the old list is served while rescanning"); + assert!(t.elapsed() < SLOW / 2, "warm get() waited: {:?}", t.elapsed()); + assert!(until(|| changes.load(Ordering::SeqCst) == 1), "a changed scan notifies"); + assert_eq!(watch.get(), [card("EOS_DIGITAL")]); + // explicit requests scan synchronously + assert!(until(|| !watch.refreshing())); + mounted.lock().unwrap().clear(); + assert!(watch.scan_now().is_empty()); + assert_eq!(changes.load(Ordering::SeqCst), 2); + } } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e42ed5ea..684fed5b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -404,6 +404,9 @@ impl LightcraftApp { if !self.styled { theme::install_fonts(ctx); theme::apply(ctx); + // File → Add from Device lists cards scanned in the background: show hot-plugs + let repaint = ctx.clone(); + lightcraft_engine::devices::on_change(move || repaint.request_repaint()); self.styled = true; } else { self.fonts_ready = true; diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index fde1e087..7fd93b80 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -915,7 +915,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option x.to_string(), - None => match lightcraft_engine::devices::devices().into_iter().next() { + None => match lightcraft_engine::devices::devices_now().into_iter().next() { Some(d) => d.path, None => return Some(Err("no camera or memory card found".into())), }, diff --git a/docs/parity.md b/docs/parity.md index a33caa46..45f32a51 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -102,7 +102,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-ADD-DIALOG | Add photos/folders | P0 | ✅ | `cmd:file.addPhotos`, `cmd:library.importPreview`, `cmd:library.import` (`mode`, `album`, `albumName`, `preset`, `keywords`), `crates/ui-egui/src/import.rs` | review dialog: candidate grid with thumbnails and checkboxes (duplicates by path/content marked and unchecked), destination, album (existing/new), preset, keywords; the folder scan runs on a background thread with a progress window and Cancel (a NAS folder no longer freezes the window); batched import with a progress window; one undo step. Drag-and-drop still imports directly | | LR-IMP-DRAGDROP | Drop files/folders to import | P0 | ✅ | `crates/ui-egui/src/lib.rs` (dropped files → `cmd:library.import`) | dropping onto a specific album not supported | | LR-IMP-DUPES | Skip duplicates by content | P1 | ✅ | `crates/engine/src/import.rs`, `crates/engine/src/tests_import.rs` | | -| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Add from Device → the import review, copying into the library by default; no PTP/MTP (cameras that don't mount as a disk) | +| LR-IMP-DEVICE | Import from camera/card | P1 | ✅ | `cmd:library.devices`, `cmd:file.addFromDevice`, `crates/engine/src/devices.rs` | mounted volumes with a DCIM folder (macOS /Volumes, Linux /media and /run/media, Windows drive letters); File → Add from Device → the import review, copying into the library by default; menus serve the last scan and rescan in the background (never blocking a frame; hot-plugs repaint); no PTP/MTP (cameras that don't mount as a disk) | | LR-IMP-AUTO | Watched-folder auto import | P2 | ✅ | `cmd:library.autoImport`, `cmd:library.autoImportScan`, `crates/ui-egui/src/panels/settings.rs` | Settings ▸ Import ▸ Auto Import: a watched folder whose new photos are added (in place or copied) once complete, into an optional album; scanned every 3 s | | LR-IMP-PRESET | Preset on import | P2 | ✅ | `cmd:library.import` (`preset`) | chosen in the import review; one History entry | | LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it | From 889583433cd55449bd42ed8581324030b2dcbd7b Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 10:51:10 -0400 Subject: [PATCH 05/12] Issue #36: cache smart-album counts in the sidebar The album rows read their counts from Caches::album_counts, recomputed only when the catalog revision changes (photos, metadata and album rules all bump it) and, while a smart album uses an "in the last..." rule, when the session clock enters a new minute. Catalog::album_count counts smart-album matches in place instead of building the id list for its length. RuleSet/Filter::depends_on_now tell clock-relative rules apart. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/lib.rs | 3 +- crates/catalog/src/query.rs | 5 ++ crates/catalog/src/rules.rs | 9 +++ crates/ui-egui/src/lib.rs | 94 +++++++++++++++++++++++++++++++ crates/ui-egui/src/panels/left.rs | 4 +- docs/parity.md | 2 +- 6 files changed, 114 insertions(+), 3 deletions(-) diff --git a/crates/catalog/src/lib.rs b/crates/catalog/src/lib.rs index 79318235..1828399b 100644 --- a/crates/catalog/src/lib.rs +++ b/crates/catalog/src/lib.rs @@ -289,7 +289,8 @@ impl Catalog { /// Number of photos shown for an album in the sources list (excludes deleted photos). pub fn album_count(&self, id: AlbumId) -> usize { match self.albums.get(&id) { - Some(Album { smart: Some(_), .. }) => self.album_photos(id).len(), + // counted in place: no id list is built just for its length + Some(Album { smart: Some(_), .. }) => self.photos.values().filter(|p| self.album_contains(id, p)).count(), Some(a) => a.photos.iter().filter(|p| self.photos.get(p).is_some_and(|p| !p.deleted)).count(), None => 0, } diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index bac31f48..084d6d6f 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -174,6 +174,11 @@ impl Filter { if v.is_empty() { "all photos".into() } else { v.join(", ") } } + /// Whether matches depend on the clock (only "in the last…" rules do). + pub fn depends_on_now(&self) -> bool { + self.rule_set.as_ref().is_some_and(crate::RuleSet::depends_on_now) + } + pub fn matches(&self, p: &Photo, cat: &Catalog) -> bool { if p.deleted != self.deleted { return false; diff --git a/crates/catalog/src/rules.rs b/crates/catalog/src/rules.rs index b5925781..f4137cb6 100644 --- a/crates/catalog/src/rules.rs +++ b/crates/catalog/src/rules.rs @@ -341,6 +341,15 @@ impl RuleSet { } } + /// Whether matches depend on the clock ("in the last…" rules, nested groups included): the + /// same photos can enter or leave the set without any catalog change. + pub fn depends_on_now(&self) -> bool { + self.rules.iter().any(|r| match r { + Rule::Group { group } => group.depends_on_now(), + Rule::Field { op, .. } => op == "inLast" || op == "notInLast", + }) + } + /// Unknown fields or operators (for command validation), as readable messages. pub fn problems(&self) -> Vec { let mut out = Vec::new(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e42ed5ea..a9341b9b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -658,6 +658,9 @@ pub struct Caches { counts: Option<(u64, LibraryCounts)>, date_groups: Option<(u64, std::sync::Arc>)>, filter_values: Option<(u64, std::sync::Arc)>, + album_counts: Option<(u64, std::sync::Arc>)>, + /// How often the album counts were recomputed (tests check that unchanged frames don't). + pub album_count_scans: usize, /// The grid's layout (by photos, shapes, width, thumbnail size, grouping). pub grid_layout: Option<(u64, std::sync::Arc)>, } @@ -779,4 +782,95 @@ impl Caches { } } } + /// Every album's photo count for the sidebar (a smart album evaluates its rules, which + /// scans the catalog). Recomputed when the catalog changes (photos, metadata, album rules + /// all bump its revision) and — only while some smart album has an "in the last…" rule — + /// when `now` (the session clock, ISO) enters a new minute, so such counts follow the clock + /// within a minute without rescanning every frame. + pub fn album_counts( + &mut self, + cat: &lightcraft_catalog::Catalog, + now: &str, + ) -> std::sync::Arc> { + let relative = cat.albums().any(|a| a.smart.as_ref().is_some_and(|f| f.depends_on_now())); + let minute = if relative { now.get(..16).unwrap_or(now) } else { "" }; + let k = key_of((cat.revision, minute)); + match &self.album_counts { + Some((h, v)) if *h == k => v.clone(), + _ => { + if relative { + // "in the last N days" counts back from the session clock, as in the grid + lightcraft_catalog::rules::set_now(Some(now.to_string())); + } + let v: std::sync::Arc> = + std::sync::Arc::new(cat.albums().map(|a| (a.id, cat.album_count(a.id))).collect()); + self.album_count_scans += 1; + self.album_counts = Some((k, v.clone())); + v + } + } + } +} + +#[cfg(test)] +mod cache_tests { + use lightcraft_catalog::{Album, AlbumId, Catalog, Filter, Op, Photo, PhotoId, RuleSet, Source}; + + fn photo(id: u64, captured: &str, rating: u8) -> Op { + let mut p = Photo::new(PhotoId(id), Source::Demo { scene: 0 }, &format!("p{id}.jpg"), "JPEG", 60, 40, "2026-01-01T00:00:00"); + p.captured = Some(captured.into()); + p.rating = rating; + Op::AddPhoto { photo: Box::new(p) } + } + + fn smart(id: u64, rules: serde_json::Value) -> Op { + let mut a = Album::new(AlbumId(id), format!("smart {id}")); + let rules: RuleSet = serde_json::from_value(rules).unwrap(); + a.smart = Some(Box::new(Filter { rule_set: Some(rules), ..Default::default() })); + Op::AddAlbum { album: a } + } + + /// Smart-album counts: unchanged frames reuse them; metadata and rule changes and (for + /// "in the last…" rules) the clock crossing into a new minute recompute them. + #[test] + fn smart_album_counts_are_cached_until_something_changes() { + let mut cat = Catalog::default(); + let mut c = super::Caches::default(); + for (i, d) in ["2026-09-30T11:59:30", "2026-09-29T08:00:00", "2026-01-01T08:00:00"].iter().enumerate() { + cat.apply(photo(i as u64 + 1, d, if i == 0 { 5 } else { 1 })).unwrap(); + } + cat.apply(smart(10, serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": 3}]}))).unwrap(); + cat.apply(smart(11, serde_json::json!({"rules": [{"field": "captureDate", "op": "inLast", "value": {"n": 1, "unit": "hours"}}]}))).unwrap(); + let now = "2026-09-30T12:00:00"; + let n = c.album_counts(&cat, now); + assert_eq!((n[&AlbumId(10)], n[&AlbumId(11)]), (1, 1)); + assert_eq!(c.album_count_scans, 1); + for _ in 0..10 { + c.album_counts(&cat, "2026-09-30T12:00:40"); + } + assert_eq!(c.album_count_scans, 1, "unchanged frames in the same minute don't rescan"); + // metadata change + cat.apply(Op::SetRating { id: PhotoId(2), rating: 4 }).unwrap(); + assert_eq!(c.album_counts(&cat, now)[&AlbumId(10)], 2); + // rule change + let rs: RuleSet = serde_json::from_value(serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": 5}]})).unwrap(); + cat.apply(Op::SetAlbumRules { id: AlbumId(10), rules: Box::new(Filter { rule_set: Some(rs), ..Default::default() }) }).unwrap(); + assert_eq!(c.album_counts(&cat, now)[&AlbumId(10)], 1); + // an hour later the 11:59:30 photo has left "in the last hour", with no catalog change + assert_eq!(c.album_counts(&cat, "2026-09-30T13:00:10")[&AlbumId(11)], 0); + assert_eq!(c.album_count_scans, 4); + lightcraft_catalog::rules::set_now(None); + } + + /// Without "in the last…" rules the clock never causes a rescan. + #[test] + fn absolute_rules_ignore_the_clock() { + let mut cat = Catalog::default(); + let mut c = super::Caches::default(); + cat.apply(photo(1, "2026-09-30T11:59:30", 5)).unwrap(); + cat.apply(smart(10, serde_json::json!({"rules": [{"field": "rating", "op": "gte", "value": 3}]}))).unwrap(); + c.album_counts(&cat, "2026-09-30T12:00:00"); + c.album_counts(&cat, "2027-01-01T00:00:00"); + assert_eq!(c.album_count_scans, 1); + } } diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 72516322..03524d46 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -398,7 +398,9 @@ fn albums_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, all: &[Album], parent } else { let sel = app.session.source == LibrarySource::Album(a.id); let icon = if a.is_smart() { Icon::SmartAlbum } else { Icon::Album }; - let n = app.session.catalog.album_count(a.id); + // cached: a smart album's count scans the catalog + let now = (app.session.clock)(); + let n = app.caches.album_counts(&app.session.catalog, &now).get(&a.id).copied().unwrap_or(0); // the album B adds to is marked "+" let target = app.session.target_album.filter(|t| app.session.catalog.album(*t).is_some()).or_else(|| app.session.catalog.quick_collection()); diff --git a/docs/parity.md b/docs/parity.md index a33caa46..84b23a04 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -123,7 +123,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-LIB-BYDATE | Browse by date | P1 | ✅ | `cmd:library.filter` (`date`), `crates/ui-egui/src/panels/left.rs` (`date_row`), `crates/catalog/src/query.rs` (`date_groups`) | year → month → day tree with library-wide counts; a row filters the current source by that date. Active filters show as removable chips under the grid header (`crates/ui-egui/src/panels/chips.rs`, with “+N more” and Clear all), the header counts “N of M photos” (matching of the source's total), a badge on the top-bar filter icon counts them, and an empty result says no photos match the active filters | | LR-LIB-ALBUM | Albums | P0 | ✅ | `cmd:album.create`, `cmd:album.rename`, `cmd:album.delete`, `cmd:album.addPhotos`, `cmd:album.removePhotos`, `cmd:dialog.newAlbum` | no drag photos onto album (LR-BEHAV-DRAGDROP), no album sort | | LR-LIB-FOLDER | Folders of albums | P0 | ✅ | `cmd:album.create` (`folder`), `cmd:album.move` | moving is command-only (no drag, no "Move to" menu) | -| LR-LIB-SMARTALBUM | Smart albums | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `crates/catalog/src/query.rs`, `crates/ui-egui/src/panels/filterbar.rs` | saved filters (rating/flag/label/kind/edited/keyword/camera/lens/date range/text/album), live; match-all only (no any/none rule groups, no rule editor dialog — rules come from the filter bar or `album.setRules`) | +| LR-LIB-SMARTALBUM | Smart albums | P1 | ✅ | `cmd:album.createSmart`, `cmd:album.setRules`, `crates/catalog/src/query.rs`, `crates/ui-egui/src/panels/filterbar.rs`, `crates/ui-egui/src/lib.rs` (`album_counts`) | saved filters (rating/flag/label/kind/edited/keyword/camera/lens/date range/text/album), live; match-all only (no any/none rule groups, no rule editor dialog — rules come from the filter bar or `album.setRules`); sidebar counts are cached per catalog revision (and per minute while an “in the last…” rule is used), counted without building id lists | | LR-LIB-SHARED-ALBUM | Shared albums | P2 | ⬜ | | needs a sharing service | | LR-LIB-OFFLINE | Keep album offline | P2 | 🚫 | | not applicable: local-first library | | LR-LIB-TARGET | Target album | P2 | ✅ | `cmd:album.setTarget`, `cmd:album.toggleTarget` | see LRC-LIB-COLLECTIONS | From ce3e2215fef9e55b388d5fadbcec46a5729bffef Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:04:21 -0400 Subject: [PATCH 06/12] Issue #22: Local: recognise equivalent folder paths Local compared folder paths as raw strings, so a built-in location built as "{home}/{sub}" and the same folder returned by library.browse with native separators could be listed twice. Folder identity now goes through catalog::query::folder_key (both separators, trailing/doubled separators, ./.., verbatim and UNC prefixes, drive-letter case; whole path case-folded on Windows): the sidebar's root list, the highlighted and expanded-on-the-way folder, saved Browse Folder roots and local.hide / local.restoreHidden all compare by it. Built-in paths are joined with the platform separator. Co-Authored-By: Claude Opus 5.5 --- crates/catalog/src/query.rs | 46 ++++++++++++++++++ crates/catalog/src/tests.rs | 33 +++++++++++++ crates/ui-egui/src/menus.rs | 4 +- crates/ui-egui/src/panels/left.rs | 79 ++++++++++++++++++++++++------- docs/parity.md | 2 +- 5 files changed, 145 insertions(+), 19 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index bac31f48..417d6bf0 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -346,6 +346,52 @@ pub fn in_folder(path: &str, dir: &str, deep: bool) -> bool { !rest.is_empty() && (deep || !rest.contains(['/', '\\'])) } +/// A folder path's identity, for telling whether two spellings name the same folder: `/` and +/// `\\` are both separators, repeated separators, `.` and a trailing separator are dropped, +/// `..` is resolved lexically, a Windows verbatim prefix (`\\?\`) is removed and the drive +/// letter lower-cased; on Windows (case-insensitive file names) the whole path is lower-cased. +/// No file-system access: the paths compared should already be absolute. +pub fn folder_key(path: &str) -> String { + let mut s = path.replace('\\', "/"); + if let Some(rest) = s.strip_prefix("//?/").or_else(|| s.strip_prefix("//./")) { + s = match rest.strip_prefix("UNC/") { + Some(unc) => format!("//{unc}"), + None => rest.to_string(), + }; + } + // the part `..` can't climb above: `//server/share`, `c:` or `/` + let (prefix, rest) = if let Some(unc) = s.strip_prefix("//").filter(|u| !u.is_empty() && !u.starts_with('/')) { + let mut it = unc.splitn(3, '/'); + let (server, share, rest) = (it.next().unwrap_or(""), it.next().unwrap_or(""), it.next().unwrap_or("")); + (format!("//{server}/{share}"), rest.to_string()) + } else if s.len() >= 2 && s.as_bytes()[1] == b':' && s.as_bytes()[0].is_ascii_alphabetic() { + (s[..2].to_ascii_lowercase(), s[2..].to_string()) + } else { + (String::new(), s.clone()) + }; + let absolute = rest.starts_with('/') || !prefix.is_empty(); + let mut parts: Vec<&str> = Vec::new(); + for c in rest.split('/') { + match c { + "" | "." => {} + ".." if parts.last().is_some_and(|p| *p != "..") => { + parts.pop(); + } + ".." if absolute => {} + c => parts.push(c), + } + } + let key = if absolute { format!("{prefix}/{}", parts.join("/")) } else { parts.join("/") }; + if cfg!(windows) { key.to_lowercase() } else { key } +} + +/// Whether `path` is folder `root` or lies somewhere inside it, however either is spelled +/// (see [`folder_key`]). +pub fn folder_within(path: &str, root: &str) -> bool { + let (p, r) = (folder_key(path), folder_key(root)); + p == r || (p.starts_with(&r) && (r.ends_with('/') || p[r.len()..].starts_with('/'))) +} + /// The kind of Photo Merge result a file is, from the name merges give it (`IMG_1-HDR.dng`, /// `IMG_1-Pano.dng`, `IMG_1-HDR-Pano.dng` — also the names other raw editors use): /// `hdr`, `panorama` or `hdrPanorama`. diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 94004eb7..86442d2a 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -264,3 +264,36 @@ fn preview_only_survives_serde_and_set_content_undo() { assert_eq!(c.photo(a).unwrap().preview_only.as_deref(), Some("Nikon Huffman-compressed NEF")); assert_eq!(c.photo(a).unwrap().width, 6000); } + +/// Equivalent spellings of one folder (separators, trailing or doubled separators, `.`/`..`, +/// drive-letter case, verbatim and UNC prefixes) share an identity; different folders don't. +#[test] +fn folder_identity_ignores_spelling() { + use crate::query::{folder_key, folder_within}; + let same = |a: &str, b: &str| assert_eq!(folder_key(a), folder_key(b), "{a} vs {b}"); + let differ = |a: &str, b: &str| assert_ne!(folder_key(a), folder_key(b), "{a} vs {b}"); + for (a, b) in [ + ("D:/Example/Photos", "D:\\Example\\Photos"), + ("D:/Example/Photos", "d:\\Example\\Photos\\"), + ("D:\\Example/Photos", "D:/Example//Photos/."), + ("D:/Example/Photos", "D:/Example/Other/../Photos"), + ("D:/Example/Photos", "\\\\?\\D:\\Example\\Photos"), + ("\\\\server\\share\\Photos", "//server/share/Photos/"), + ("\\\\?\\UNC\\server\\share\\Photos", "//server/share/Photos"), + ("/home/me/Photos", "/home/me/Photos/"), + ("/home/me/Photos", "/home//me/./Photos"), + ("/", "//"), + ("C:\\", "c:/"), + ] { + same(a, b); + } + differ("D:/Example/Photos", "D:/Example/Photos2"); + differ("D:/Example/Photos", "E:/Example/Photos"); + differ("/home/me/Photos", "/home/me"); + assert_eq!(folder_key("/a/../../b"), "/b", "`..` stops at the root"); + assert!(folder_within("D:\\Example\\Photos\\2026", "D:/Example/Photos/")); + assert!(folder_within("D:/Example/Photos", "d:\\Example\\Photos")); + assert!(!folder_within("D:/Example/Photos2", "D:/Example/Photos")); + assert!(folder_within("/a/b", "/")); + assert!(folder_within("C:\\x", "c:\\")); +} diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index fde1e087..6f29a310 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -557,7 +557,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option match p.get("path").and_then(Value::as_str) { Some(path) => { - if !app.ui.hidden_locations.iter().any(|h| h == path) { + if !app.ui.hidden_locations.iter().any(|h| crate::panels::left::same_folder(h, path)) { app.ui.hidden_locations.push(path.to_string()); } Ok(json!({"hidden": app.ui.hidden_locations})) @@ -567,7 +567,7 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { // one path, or (no path) every hidden location match p.get("path").and_then(Value::as_str) { - Some(path) => app.ui.hidden_locations.retain(|h| h != path), + Some(path) => app.ui.hidden_locations.retain(|h| !crate::panels::left::same_folder(h, path)), None => app.ui.hidden_locations.clear(), } Ok(json!({"hidden": app.ui.hidden_locations})) diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 72516322..bd4201c8 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -203,27 +203,19 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { let (lr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); ui.painter().text(pos2(lr.left() + 18.0, lr.center().y), Align2::LEFT_CENTER, "Local", t.semibold(13.5), t.text_label); let home = std::env::var("HOME").or_else(|_| std::env::var("USERPROFILE")).unwrap_or_default(); - let mut places: Vec<(String, String)> = Vec::new(); + let mut builtin: Vec<(String, String)> = Vec::new(); if !home.is_empty() { for (name, sub) in [("Pictures", "Pictures"), ("Desktop", "Desktop"), ("Downloads", "Downloads"), ("Home", "")] { - let p = if sub.is_empty() { home.clone() } else { format!("{home}/{sub}") }; + // joined with the platform's separator, like the paths browsing gives back + let p = if sub.is_empty() { home.clone() } else { std::path::Path::new(&home).join(sub).to_string_lossy().to_string() }; if std::path::Path::new(&p).is_dir() { - places.push((name.to_string(), p)); + builtin.push((name.to_string(), p)); } } } let browsing = app.session.browse.clone().filter(|_| app.session.source == LibrarySource::Folder); - // picked folders stay listed; the folder being browsed is listed even when it wasn't picked - let roots = app.ui.local_roots.iter().cloned().chain(browsing.iter().map(|b| b.path.clone())); - for path in roots { - if !places.iter().any(|(_, p)| *p == path) { - let name = std::path::Path::new(&path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.clone()); - places.push((name, path)); - } - } let current = browsing.as_ref().map(|b| b.path.clone()); - // hidden locations stay reachable: breadcrumbs, Browse Folder…, and the row below - places.retain(|(_, p)| !app.ui.hidden_locations.contains(p)); + let places = local_places(builtin, &app.ui.local_roots, current.as_deref(), &app.ui.hidden_locations); for (name, path) in places { folder_tree(app, ui, &name, &path, 0.0, current.as_deref()); } @@ -235,7 +227,7 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { match app.run("library.browse", json!({"path": path})) { Ok(r) => { let dir = r["path"].as_str().unwrap_or(&path).to_string(); - if !app.ui.local_roots.contains(&dir) { + if !app.ui.local_roots.iter().any(|p| same_folder(p, &dir)) { app.ui.local_roots.push(dir); } } @@ -256,6 +248,28 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { ui.add_space(10.0); } +/// Whether two paths name the same folder, however they are spelled (separators, trailing +/// slash, `.`/`..`, drive-letter case; see `lightcraft_catalog::query::folder_key`). +pub(crate) fn same_folder(a: &str, b: &str) -> bool { + a == b || lightcraft_catalog::query::folder_key(a) == lightcraft_catalog::query::folder_key(b) +} + +/// Local's top-level folders, in order: the built-in places, the folders picked with Browse +/// Folder…, then the folder being browsed when it isn't one of those. Each folder is listed +/// once however its path is spelled (the first spelling wins), and hidden ones are left out — +/// they stay reachable through breadcrumbs and Browse Folder…. +pub(crate) fn local_places(builtin: Vec<(String, String)>, saved: &[String], browsing: Option<&str>, hidden: &[String]) -> Vec<(String, String)> { + let mut places = builtin; + for path in saved.iter().map(String::as_str).chain(browsing) { + if !places.iter().any(|(_, p)| same_folder(p, path)) { + let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); + places.push((name, path.to_string())); + } + } + places.retain(|(_, p)| !hidden.iter().any(|h| same_folder(h, p))); + places +} + /// The subfolders of `path` (not hidden ones), sorted; listed at most every 2 s per folder. fn subfolders(ui: &egui::Ui, path: &str) -> Vec<(String, String)> { let id = egui::Id::new(("subfolders", path.to_string())); @@ -286,9 +300,9 @@ fn subfolders(ui: &egui::Ui, path: &str) -> Vec<(String, String)> { fn folder_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, name: &str, path: &str, indent: f32, current: Option<&str>) { let t = Tokens::get(ui.ctx()); let open_id = egui::Id::new(("folder-open", path.to_string())); - let on_the_way = current.is_some_and(|c| c != path && std::path::Path::new(c).starts_with(path)); + let sel = current.is_some_and(|c| same_folder(c, path)); + let on_the_way = !sel && current.is_some_and(|c| lightcraft_catalog::query::folder_within(c, path)); let mut open: bool = ui.data(|d| d.get_temp(open_id)).unwrap_or(on_the_way); - let sel = current == Some(path); let resp = row(app, ui, &format!("local:{path}"), Icon::Folder, name, None, sel, indent + 12.0).on_hover_text(path); let c = pos2(resp.rect.left() + 10.0 + indent, resp.rect.center().y); let tri = Rect::from_center_size(c, vec2(14.0, 14.0)); @@ -597,3 +611,36 @@ fn keyword_rows(app: &mut LightcraftApp, ui: &mut egui::Ui, nodes: &[KeywordNode } } } + +#[cfg(test)] +mod tests { + use super::local_places; + + fn names(v: &[(String, String)]) -> Vec<&str> { + v.iter().map(|(n, _)| n.as_str()).collect() + } + + /// A built-in location browsed (or picked) under another spelling of its path is the same + /// row, not a second root — for every built-in, not just named ones. + #[test] + fn equivalent_paths_are_one_local_location() { + for (home, sep) in [("D:\\Example", '\\'), ("D:/Example", '/'), ("/home/example", '/')] { + let builtin: Vec<(String, String)> = + ["Pictures", "Desktop", "Downloads"].iter().map(|s| (s.to_string(), format!("{home}{sep}{s}"))).collect(); + for (_, p) in builtin.clone() { + // the browsed spelling: other separators, a trailing one, a different drive-letter case + let flipped = p.replace(['/', '\\'], if sep == '/' { "\\" } else { "/" }); + for browsing in [flipped.clone(), format!("{p}{sep}"), p.replacen("D:", "d:", 1)] { + let places = local_places(builtin.clone(), std::slice::from_ref(&browsing), Some(&browsing), &[]); + assert_eq!(names(&places), ["Pictures", "Desktop", "Downloads"], "{p} browsed as {browsing}"); + } + // hiding under one spelling hides the other + let places = local_places(builtin.clone(), &[], None, std::slice::from_ref(&flipped)); + assert_eq!(places.len(), 2, "hidden {flipped}"); + } + } + // a different folder is still added + let places = local_places(vec![("Pictures".into(), "/home/example/Pictures".into())], &[], Some("/home/example/Pictures2"), &[]); + assert_eq!(names(&places), ["Pictures", "Pictures2"]); + } +} diff --git a/docs/parity.md b/docs/parity.md index a33caa46..2d1b7392 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -108,7 +108,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it | | LR-IMP-MIGRATE | Migrate other catalogs | OOS | 🚫 | | | | LR-IMP-PROFILES | Import profiles & presets | P1 | ✅ | `cmd:file.importPresets`, `cmd:preset.import`, `cmd:profile.import`, `crates/engine/src/preset_import.rs`, `crates/engine/src/preset_luminar.rs` | presets: .lcpreset, XMP, classic .lrtemplate, photos carrying edits ("DNG presets"), Luminar looks (.lmp, .mplumpack collections; sliders with a counterpart), .zip bundles, folders (folder → group), drag & drop; masks carried over; unmapped settings reported; profiles: .cube 3D LUTs; Adobe profile formats deliberately unsupported | -| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back | +| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back; a folder is one location however its path is spelled (`/` or `\`, trailing separator, `.`/`..`, drive-letter case; `folder_key` in `crates/catalog/src/query.rs`) | | LR-IMP-SIDECAR-SPLIT | Separate XMP sidecar variants | P2 | ⬜ | `cmd:library.xmpPreferences` | sidecar naming option exists (stem/full), no split sidecars | | LR-IMP-FORMATS | Supported formats | P0 | 🟡 | `crates/codecs/src/lib.rs`, `crates/raw/src/lib.rs` | JPEG, PNG, TIFF, WebP, JXL, PSD, GIF, BMP; DNG (incl. lossy / Smart Preview DNG), CR2, ARW, NEF, RAF, RW2, PEF, ORF. Missing: CR3, compressed NEF/RAF/ORF, RW2 v4 (preview only: imported with `preview_only` = the decoder's reason, rendered as a rendered JPEG, flagged by a grid badge and loupe/Edit/Info notices, `previewOnly` in `catalog.query`; Reload clears it once the file decodes), HEIC/AVIF decode | | LR-IMP-CULL-AT-IMPORT | Culling analysis at import | P2 | 🟡 | `cmd:photo.analyze` | run Assisted Culling on the imported photos (they're selected after an import); not automatic | From 8be82cacf460183f0c01eed8f4a622d432204a47 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:04:27 -0400 Subject: [PATCH 07/12] Issue #31: Import: tag help beside the Rename field rename::TOKENS / DATE_DIRECTIVES / TEMPLATE_NOTES are now the one list of file-name template tags (a test checks expand_tokens knows each, and that the rename/import command descriptions mention them all). A Tags toggle beside the import Rename field (also Rename Photos and export File name) lists every tag with its meaning and a live example, the {date:...} directives and the rules (extension appended, blank keeps names, missing date/metadata, unknown tags stay literal); clicking a tag inserts it at the text cursor. Unknown tags are flagged under the field. New query command photo.renameTokens returns the same help as JSON. The import preview now uses the probe's metadata and the import time for undated photos, as the import itself does. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 2 +- crates/engine/src/cmd/manage.rs | 7 +- crates/engine/src/rename.rs | 142 +++++++++++++++++++++++++++ crates/ui-egui/src/headless.rs | 46 +++++++++ crates/ui-egui/src/import.rs | 100 ++++++++++++++++++- crates/ui-egui/src/panels/dialogs.rs | 26 ++++- docs/parity.md | 4 +- 7 files changed, 313 insertions(+), 14 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index b4c3b292..b0d65f31 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -735,7 +735,7 @@ pub fn specs() -> Vec { "Add Photos…", ["File"], Some("Cmd+Shift+I"), - "{paths: [file or folder (recursive)], mode?: add|copy (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/), destination?: folder for copies, organize?: date|month|flat, rename?: file-name template for copies ({name} {seq:N} {date:%Y%m%d} {camera} {title}), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG), local?: bool (browsing: the photos stay out of the library, like library.browse), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..]} → {imported, duplicates, failed, album?}", + "{paths: [file or folder (recursive)], mode?: add|copy (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/), destination?: folder for copies, organize?: date|month|flat, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG), local?: bool (browsing: the photos stay out of the library, like library.browse), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..]} → {imported, duplicates, failed, album?}", always, |s, p| { let paths = strs(p, "paths"); diff --git a/crates/engine/src/cmd/manage.rs b/crates/engine/src/cmd/manage.rs index 84b46ccc..8b9ee8a2 100644 --- a/crates/engine/src/cmd/manage.rs +++ b/crates/engine/src/cmd/manage.rs @@ -61,7 +61,10 @@ fn set_names_ops(s: &crate::Session, names: &[String; 5]) -> Vec { pub fn specs() -> Vec { vec![ - cmd!(query "photo.renamePreview", "Rename Preview", [], None, "{template: e.g. `{date}_{name}`, `Trip-{seq:3}` (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}), start?: first sequence number (1), ids?} — renames the files on disk (sidecars too, never overwriting: collisions get -1, -2…); undoable", has_selection, |s, p| { + cmd!(query "photo.renameTokens", "Rename Template Tags", [], None, "{} → {tokens: [{tag, aliases, meaning, example}], dateDirectives: [{directive, meaning}], notes: [..], sample} — the file-name template tags shared by photo.rename, library.import (rename) and app.export (naming); examples are for a sample photo", always, |_, _| { + Ok(crate::rename::token_help_json()) + }), + cmd!(query "photo.renamePreview", "Rename Preview", [], None, "{template: e.g. `{date}_{name}`, `Trip-{seq:3}` (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each), start?: first sequence number (1), ids?} — renames the files on disk (sidecars too, never overwriting: collisions get -1, -2…); undoable", has_selection, |s, p| { let (ids, template, start) = rename_args(s, p, "photo.renamePreview")?; Ok(serde_json::to_value(s.plan_rename(&ids, &template, start)).unwrap_or_default()) }), @@ -70,7 +73,7 @@ pub fn specs() -> Vec { "Rename Photos", [], None, - "{template: e.g. `{date}_{name}`, `Trip-{seq:3}` (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}), start?: first sequence number (1), ids?} — renames the files on disk (sidecars too, never overwriting: collisions get -1, -2…); undoable", + "{template: e.g. `{date}_{name}`, `Trip-{seq:3}` (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each), start?: first sequence number (1), ids?} — renames the files on disk (sidecars too, never overwriting: collisions get -1, -2…); undoable", has_selection, |s, p| { let (ids, template, start) = rename_args(s, p, "photo.rename")?; diff --git a/crates/engine/src/rename.rs b/crates/engine/src/rename.rs index 2b540c85..0c077d27 100644 --- a/crates/engine/src/rename.rs +++ b/crates/engine/src/rename.rs @@ -82,6 +82,113 @@ fn format_date(iso: &str, fmt: &str) -> String { out } +/// One template token, for help texts and tag pickers. [`TOKENS`] is the single list every UI and +/// command description shows; a test checks that [`expand_tokens`] knows each of them. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize)] +pub struct TokenHelp { + /// The token as typed, e.g. `{seq:3}`. + pub tag: &'static str, + /// Other spellings with the same meaning. + pub aliases: &'static [&'static str], + pub meaning: &'static str, +} + +/// Every template token, in the order the help shows them. +pub const TOKENS: &[TokenHelp] = &[ + TokenHelp { tag: "{name}", aliases: &["{filename}"], meaning: "Original file name without its extension" }, + TokenHelp { tag: "{num}", aliases: &[], meaning: "The number at the end of the original name (IMG_0042 → 0042)" }, + TokenHelp { tag: "{seq}", aliases: &["{n}"], meaning: "Sequence number, counting from the start number" }, + TokenHelp { tag: "{seq:3}", aliases: &["{n:3}"], meaning: "Sequence number zero-padded to N digits (1–9)" }, + TokenHelp { tag: "{date}", aliases: &[], meaning: "Capture date as YYYYMMDD" }, + TokenHelp { tag: "{date:%Y%m%d_%H%M%S}", aliases: &[], meaning: "Capture date and time in your own format (directives below)" }, + TokenHelp { tag: "{folder}", aliases: &[], meaning: "Name of the folder the original is in" }, + TokenHelp { tag: "{camera}", aliases: &[], meaning: "Camera make and model" }, + TokenHelp { tag: "{lens}", aliases: &[], meaning: "Lens" }, + TokenHelp { tag: "{iso}", aliases: &[], meaning: "ISO speed" }, + TokenHelp { tag: "{rating}", aliases: &[], meaning: "Star rating (0–5)" }, + TokenHelp { tag: "{title}", aliases: &[], meaning: "Title metadata" }, + TokenHelp { tag: "{creator}", aliases: &[], meaning: "Creator metadata" }, + TokenHelp { tag: "{ext}", aliases: &[], meaning: "Original extension without the dot" }, +]; + +/// The `%` directives of `{date:…}`. +pub const DATE_DIRECTIVES: &[(&str, &str)] = &[ + ("%Y", "year, 4 digits"), + ("%y", "year, 2 digits"), + ("%m", "month 01–12"), + ("%d", "day 01–31"), + ("%H", "hour 00–23"), + ("%M", "minute"), + ("%S", "second"), + ("%%", "a literal %"), +]; + +/// How templates behave, one sentence each (shown under the token list). +pub const TEMPLATE_NOTES: &[&str] = &[ + "The original extension is always added; {ext} only puts it inside the name as well.", + "A blank template keeps the original names.", + "{date} is the capture time; a photo without one uses the time it was imported.", + "Missing metadata ({camera}, {title}…) leaves an empty gap; a name that comes out empty keeps the original name.", + "Unknown tags stay as typed — check the preview for a {typo}.", + "Characters not allowed in file names (/ \\ : * ? \" < > |) become -; an existing name gets -1, -2….", +]; + +/// The tokens as one line (`{name} {num} {seq} …`), for compact hints. +pub fn token_summary() -> String { + TOKENS.iter().map(|t| t.tag).collect::>().join(" ") +} + +/// A fixed photo the help's examples are computed from (`IMG_0042.CR3`, 14 Jan 2026 05:58:48). +pub fn sample_photo() -> Photo { + let mut p = + Photo::new(PhotoId(0), Source::File { path: "/Card/DCIM/IMG_0042.CR3".into() }, "IMG_0042.CR3", "CR3", 6000, 4000, "2026-01-20T10:00:00"); + p.captured = Some("2026-01-14T05:58:48".into()); + p.rating = 4; + p.meta.camera = "Canon EOS R5".into(); + p.meta.lens = "RF24-70mm F2.8".into(); + p.meta.iso = Some(400); + p.meta.title = "Harbour".into(); + p.meta.creator = "Ann Lee".into(); + p +} + +/// The token as expanded for [`sample_photo`] (sequence number 1), for the help's example column. +pub fn token_example(tag: &str) -> String { + expand_tokens(tag, &sample_photo(), 1, 1) +} + +/// The `{…}` tags in `template` that aren't tokens (they stay as typed), for a warning. +pub fn unknown_tokens(template: &str) -> Vec { + let p = sample_photo(); + let mut out = Vec::new(); + let mut rest = template; + while let Some(i) = rest.find('{') { + let Some(j) = rest[i..].find('}') else { break }; + let tag = &rest[i..i + j + 1]; + if expand_tokens(tag, &p, 1, 1) == tag && !out.iter().any(|t| t == tag) { + out.push(tag.to_string()); + } + rest = &rest[i + j + 1..]; + } + out +} + +/// Every token with its meaning and example, the date directives and the notes, as JSON (the +/// `photo.renameTokens` command). +pub fn token_help_json() -> serde_json::Value { + let tokens: Vec = TOKENS + .iter() + .map(|t| serde_json::json!({"tag": t.tag, "aliases": t.aliases, "meaning": t.meaning, "example": token_example(t.tag)})) + .collect(); + let directives: Vec = DATE_DIRECTIVES.iter().map(|(d, m)| serde_json::json!({"directive": d, "meaning": m})).collect(); + serde_json::json!({ + "tokens": tokens, + "dateDirectives": directives, + "notes": TEMPLATE_NOTES, + "sample": "IMG_0042.CR3, captured 2026-01-14 05:58:48", + }) +} + /// Expand `template` for `p` (sequence number `seq`) into a file name with `p`'s extension. pub fn expand(template: &str, p: &Photo, seq: usize) -> String { let (stem, ext) = split_ext(&p.file_name); @@ -358,6 +465,41 @@ impl Session { mod tests { use super::*; + /// The help lists exactly what `expand_tokens` understands: every listed tag (and alias) is + /// replaced, never left literal, and the help's examples match the documented behaviour. + #[test] + fn token_help_matches_the_implementation() { + let p = sample_photo(); + for t in TOKENS { + for tag in std::iter::once(&t.tag).chain(t.aliases) { + let v = expand_tokens(tag, &p, 1, 1); + assert!(!v.contains('{'), "{tag} is not a known token (expanded to {v:?})"); + assert!(!v.is_empty(), "{tag}: the sample photo should give an example"); + } + } + assert_eq!(token_example("{seq:3}"), "001"); + assert_eq!(token_example("{date}"), "20260114"); + assert_eq!(token_example("{date:%Y%m%d_%H%M%S}"), "20260114_055848"); + assert_eq!(token_example("{ext}"), "CR3"); + assert_eq!(expand("{date:%Y%m%d_%H%M%S}_{seq:3}", &p, 1), "20260114_055848_001.CR3"); + for (d, _) in DATE_DIRECTIVES { + let v = format_date("2026-01-14T05:58:48", d); + assert!(!v.contains('%') || *d == "%%", "{d} is not a known directive"); + } + assert_eq!(unknown_tokens("{date}_{camra}-{seq:2}{x}{camra}"), vec!["{camra}".to_string(), "{x}".to_string()]); + assert!(unknown_tokens("{name}{ext}{date:%Y}").is_empty()); + let json = token_help_json(); + assert_eq!(json["tokens"].as_array().unwrap().len(), TOKENS.len()); + assert_eq!(json["tokens"][0]["example"], "IMG_0042"); + // the command descriptions that list the tokens list all of them + for id in ["photo.rename", "photo.renamePreview", "library.import"] { + let spec = crate::cmd::command_specs().iter().find(|c| c.id == id).unwrap(); + for t in TOKENS.iter().filter(|t| !t.tag.contains(':')) { + assert!(spec.params.contains(t.tag), "{id} does not mention {}", t.tag); + } + } + } + #[test] fn templates() { let mut p = Photo::new(PhotoId(1), Source::Demo { scene: 1 }, "IMG_0042.CR2", "CR2", 1, 1, "2026-01-01T00:00:00"); diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index b911896b..f89e324f 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1211,6 +1211,52 @@ mod tests { let _ = std::fs::remove_dir_all(&base); } + /// Import review, Copy: Tags beside the Rename field lists the template tags; clicking one + /// inserts it at the text cursor (not appended, not replacing the template). + #[test] + fn import_rename_tags_insert_at_the_cursor() { + let dir = std::env::temp_dir().join(format!("lc-ui-import-tags-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[90, 3, 9, 255]; 64] }; + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); + std::fs::write(dir.join("IMG_0007.png"), png).unwrap(); + let lib = dir.join("lib"); + let mut session = lightcraft_engine::Session::with_demo().with_fs(); + session.open_library(&lib, false).unwrap(); + let mut app = LightcraftApp::new(session, crate::Services { png: None, ..Default::default() }); + app.ui.view = crate::state::ViewMode::PhotoGrid; + let mut h = Headless::new(app, [1300.0, 1000.0], 1.0); + let t = Duration::from_secs(10); + let r = + h.request("engine.execute", json!({"command": "file.addPhotos", "params": {"paths": [dir.join("IMG_0007.png").to_string_lossy()]}}), t); + assert_eq!(r["result"]["scanning"], true, "{r}"); + h.settle(SETTLE); + let r = h.request("ui.clickWidget", json!({"id": "button:importCopy"}), t); + assert_eq!(r["ok"], true, "{r}"); + if let Some(crate::state::Dialog::Import { opts }) = &mut h.app.ui.dialog { + opts.rename = "Trip-_x".into(); + } + // the cursor sits after "Trip-" + let id = egui::Id::new("import-rename"); + let mut st = egui::text_edit::TextEditState::default(); + st.cursor.set_char_range(Some(egui::text::CCursorRange::one(egui::text::CCursor::new(5)))); + st.store(&h.view.ctx, id); + let r = h.request("ui.clickWidget", json!({"id": "button:importRenameTags"}), t); + assert_eq!(r["ok"], true, "{r}"); + let seq3 = lightcraft_engine::rename::TOKENS.iter().position(|x| x.tag == "{seq:3}").unwrap(); + let r = h.request("ui.clickWidget", json!({"id": format!("button:importRenameTag-{seq3}")}), t); + assert_eq!(r["ok"], true, "{r}"); + let Some(crate::state::Dialog::Import { opts }) = &h.app.ui.dialog else { panic!("no import dialog") }; + assert_eq!(opts.rename, "Trip-{seq:3}_x", "inserted at the cursor"); + // a second tag goes after the first (the cursor moved past it) + let r = h.request("ui.clickWidget", json!({"id": "button:importRenameTag-0"}), t); + assert_eq!(r["ok"], true, "{r}"); + let Some(crate::state::Dialog::Import { opts }) = &h.app.ui.dialog else { panic!("no import dialog") }; + assert_eq!(opts.rename, "Trip-{seq:3}{name}_x"); + let _ = std::fs::remove_dir_all(&dir); + } + /// Settings (⌘,): tabs switch, app settings change the UI state, library settings go through /// the engine; the delete confirmation guards ⌫. #[test] diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 719b235a..58f6b302 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -449,26 +449,39 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { let r = ui.checkbox(&mut d.dng, "Copy as DNG"); register(ui.ctx(), "check:importDng", r.rect); }); - field(ui, "Rename", |ui| { - let r = ui.add(egui::TextEdit::singleline(&mut d.rename).hint_text("keep names — or e.g. {date}_{seq:3}").desired_width(f32::INFINITY)); + let rename_id = egui::Id::new("import-rename"); + let tags_open = field(ui, "Rename", |ui| { + ui.spacing_mut().item_spacing.x = 4.0; + let w = (ui.available_width() - 50.0).max(80.0); + let r = ui.add(egui::TextEdit::singleline(&mut d.rename).id(rename_id).hint_text("keep names — or e.g. {date}_{seq:3}").desired_width(w)); register(ui.ctx(), "field:importRename", r.rect); + tag_toggle(ui, "importRename") }); + if tags_open { + tag_help(ui, "importRename", &mut d.rename, rename_id); + } if !d.rename.trim().is_empty() && let Some(c) = d.candidates.iter().zip(&d.checked).find(|(c, on)| **on && c.duplicate.is_none()).map(|(c, _)| c) { let mut q = lightcraft_catalog::Photo::new( lightcraft_catalog::PhotoId(0), - lightcraft_catalog::Source::Demo { scene: 0 }, + lightcraft_catalog::Source::File { path: c.path.clone() }, &c.name, &c.format, 0, 0, - "", + // a photo without a capture time is dated by the import (now) + &(app.session.clock)(), ); q.captured = c.captured.clone(); + // the probe's metadata, so {camera}, {title}… preview as they will import + if let Some(info) = app.session.import_probes.get(&c.path) { + q.meta = info.meta.clone(); + } let example = lightcraft_engine::rename::expand(d.rename.trim(), &q, 1); ui.label(egui::RichText::new(format!("{} → {example}", c.name)).color(Tokens::get(ui.ctx()).text_dim)); } + unknown_tags_warning(ui, &d.rename); } let albums: Vec<(u64, String)> = { let mut v: Vec<(u64, String)> = @@ -602,6 +615,85 @@ fn candidate_cell(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDial } } +/// The "Tags" toggle beside a template field (`button:Tags`); returns whether the tag help +/// ([`tag_help`]) is open. +pub(crate) fn tag_toggle(ui: &mut egui::Ui, key: &str) -> bool { + let id = egui::Id::new((key, "tags-open")); + let mut open = ui.ctx().data(|d| d.get_temp::(id)).unwrap_or(false); + let r = crate::widgets::text_button(ui, &format!("{key}Tags"), "Tags", open).on_hover_text("Show the template tags; click one to insert it"); + if r.clicked() { + open = !open; + ui.ctx().data_mut(|d| d.insert_temp(id, open)); + } + open +} + +/// "Unknown tag {x} stays as typed" under a template field, when it has one. +pub(crate) fn unknown_tags_warning(ui: &mut egui::Ui, template: &str) { + let unknown = lightcraft_engine::rename::unknown_tokens(template); + if !unknown.is_empty() { + let s = if unknown.len() == 1 { "" } else { "s" }; + let text = format!("Unknown tag{s} {} stay{} as typed (see Tags)", unknown.join(" "), if unknown.len() == 1 { "s" } else { "" }); + ui.label(egui::RichText::new(text).color(Tokens::get(ui.ctx()).caution)); + } +} + +/// Insert `tag` into `text` at the text cursor of the field `edit_id` (replacing its selection); +/// appended when the field never had a cursor. The cursor ends up after the tag. +pub(crate) fn insert_at_cursor(ctx: &egui::Context, edit_id: egui::Id, text: &mut String, tag: &str) { + use egui::text::{CCursor, CCursorRange}; + let mut state = egui::text_edit::TextEditState::load(ctx, edit_id).unwrap_or_default(); + let n = text.chars().count(); + let (a, b) = state.cursor.char_range().map_or((n, n), |r| { + let (x, y) = (r.primary.index.0.min(n), r.secondary.index.0.min(n)); + (x.min(y), x.max(y)) + }); + let byte = |c: usize| text.char_indices().nth(c).map_or(text.len(), |(i, _)| i); + let (ba, bb) = (byte(a), byte(b)); + text.replace_range(ba..bb, tag); + state.cursor.set_char_range(Some(CCursorRange::one(CCursor::new(a + tag.chars().count())))); + state.store(ctx, edit_id); + ctx.memory_mut(|m| m.request_focus(edit_id)); +} + +/// The template tag help: every tag (from the engine's one list, `rename::TOKENS`) with its +/// meaning and an example, the `{date:…}` directives and how templates behave. Clicking a tag +/// (`button:Tag-`) inserts it at the cursor of the field `edit_id`. +pub(crate) fn tag_help(ui: &mut egui::Ui, key: &str, text: &mut String, edit_id: egui::Id) { + use lightcraft_engine::rename::{DATE_DIRECTIVES, TEMPLATE_NOTES, TOKENS, token_example}; + let t = Tokens::get(ui.ctx()); + egui::Frame::new().fill(t.inset).corner_radius(4.0).inner_margin(6.0).show(ui, |ui| { + ui.label( + egui::RichText::new("Click a tag to insert it. Examples are for IMG_0042.CR3 taken 14 Jan 2026, 05:58:48.").color(t.text_dim).small(), + ); + egui::ScrollArea::vertical().id_salt((key, "tags")).max_height(170.0).show(ui, |ui| { + egui::Grid::new((key, "tag-grid")).num_columns(3).spacing([10.0, 2.0]).show(ui, |ui| { + for (i, tok) in TOKENS.iter().enumerate() { + let r = ui.add(egui::Button::new(egui::RichText::new(tok.tag).monospace().color(t.text)).small()).on_hover_text( + if tok.aliases.is_empty() { + "Insert at the cursor".to_string() + } else { + format!("Insert at the cursor (also written {})", tok.aliases.join(", ")) + }, + ); + register(ui.ctx(), format!("button:{key}Tag-{i}"), r.rect); + if r.clicked() { + insert_at_cursor(ui.ctx(), edit_id, text, tok.tag); + } + ui.label(egui::RichText::new(tok.meaning).color(t.text_label)); + ui.label(egui::RichText::new(token_example(tok.tag)).monospace().color(t.text_dim)); + ui.end_row(); + } + }); + let directives: Vec = DATE_DIRECTIVES.iter().map(|(d, m)| format!("{d} {m}")).collect(); + ui.label(egui::RichText::new(format!("{{date:…}} directives: {}", directives.join(" · "))).color(t.text_dim).small()); + for n in TEMPLATE_NOTES { + ui.label(egui::RichText::new(format!("• {n}")).color(t.text_dim).small()); + } + }); + }); +} + /// A labelled row (fixed label column). fn field(ui: &mut egui::Ui, label: &str, add: impl FnOnce(&mut egui::Ui) -> R) -> R { let t = Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 24153a8b..1cb732f4 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -289,10 +289,18 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { Dialog::Rename { template, start } => { let n = app.session.targets(&json!({})).len(); ui.label(egui::RichText::new(format!("{n} photo{}", if n == 1 { "" } else { "s" })).color(t.text_dim)); - field(ui, "Template", |ui| { - let r = ui.add(egui::TextEdit::singleline(template).hint_text("{name}").desired_width(f32::INFINITY)); + let template_id = egui::Id::new("rename-template"); + let tags_open = field(ui, "Template", |ui| { + ui.spacing_mut().item_spacing.x = 4.0; + let w = (ui.available_width() - 50.0).max(80.0); + let r = ui.add(egui::TextEdit::singleline(template).id(template_id).hint_text("{name}").desired_width(w)); crate::widgets::register(ui.ctx(), "field:renameTemplate", r.rect); + crate::import::tag_toggle(ui, "renameTemplate") }); + if tags_open { + crate::import::tag_help(ui, "renameTemplate", template, template_id); + } + crate::import::unknown_tags_warning(ui, template); field(ui, "Presets", |ui| { ui.spacing_mut().item_spacing.x = 4.0; for (i, (label, tpl)) in @@ -305,7 +313,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { }); field(ui, "Start at", |ui| ui.add(egui::DragValue::new(start).range(0..=999_999))); ui.label( - egui::RichText::new("Tokens: {name} {num} {seq} {seq:3} {date} {date:%Y-%m-%d} {folder} {camera} {lens} {iso} {rating} {title} {creator}. Files are renamed on disk (with their XMP sidecars); existing names get -1, -2…") + egui::RichText::new("Tags: see Tags beside the template. Files are renamed on disk (with their XMP sidecars); existing names get -1, -2…") .color(t.text_dim), ); let preview = app.session.execute("photo.renamePreview", &json!({"template": template, "start": start})).unwrap_or_default(); @@ -576,9 +584,17 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { use lightcraft_engine::export::DngCompression as Z; choices(ui, "Compression", "exportDngCompression", &[(Z::Lossless, "Lossless"), (Z::Deflate, "ZIP"), (Z::Uncompressed, "None")], &mut opts.dng_compression); } - field(ui, "File name", |ui| { - ui.add(egui::TextEdit::singleline(&mut opts.naming).hint_text("{name}-{seq} · {date} · {title} · {folder}").desired_width(f32::INFINITY)) + let naming_id = egui::Id::new("export-naming"); + let tags_open = field(ui, "File name", |ui| { + ui.spacing_mut().item_spacing.x = 4.0; + let w = (ui.available_width() - 50.0).max(80.0); + ui.add(egui::TextEdit::singleline(&mut opts.naming).id(naming_id).hint_text("{name}-{seq} · {date} · {title} · {folder}").desired_width(w)); + crate::import::tag_toggle(ui, "exportNaming") }); + if tags_open { + crate::import::tag_help(ui, "exportNaming", &mut opts.naming, naming_id); + } + crate::import::unknown_tags_warning(ui, &opts.naming); if opts.naming.contains("{seq") { let mut v = opts.start_number as f64; if num(ui, &START_NUMBER, &mut v) { diff --git a/docs/parity.md b/docs/parity.md index a33caa46..c718a5aa 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -137,7 +137,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-LIB-DELETE | Delete / Recently Deleted | P0 | ✅ | `cmd:photo.delete`, `cmd:photo.restore`, `cmd:photo.deletePermanently` | no confirmation dialog, no auto-purge after N days, no "Empty" | | LR-LIB-REMOVE-ALBUM | Remove from album | P0 | ✅ | `cmd:album.removePhotos` | | | LR-LIB-DUPLICATE | Duplicate a photo | P2 | ✅ | `cmd:photo.duplicate` | Photo ▸ Duplicate: a real `-copy` file with the same settings, metadata and albums (virtual copies share the file) | -| LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming); preview; renames files on disk with their XMP sidecars, never overwriting (-1, -2… suffixes), rolls back on failure; undo/redo move the files; virtual copies follow | +| LR-LIB-RENAME | Batch rename | P1 | ✅ | `cmd:photo.rename`, `cmd:photo.renamePreview`, `cmd:photo.renameTokens`, `cmd:dialog.rename`, `crates/engine/src/rename.rs` (`TOKENS`) | template tokens {name} {num} {seq:N} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext} (shared with export naming and import rename); a Tags picker beside every template field (Rename Photos, import Rename, export File name) lists each tag with meaning + live example, the date directives and the rules, and inserts the clicked tag at the text cursor; unknown tags are flagged; preview; renames files on disk with their XMP sidecars, never overwriting (-1, -2… suffixes), rolls back on failure; undo/redo move the files; virtual copies follow | | LR-LIB-CAPTURETIME | Edit capture time | P1 | ✅ | `cmd:photo.setCaptureTime`, `cmd:dialog.captureTime`, `crates/catalog/src/dates.rs` | set (the other selected photos shift by the same amount, or `each`), shift by days/hours/minutes, time-zone shift; one undo step, journaled; Info panel button; no “revert to original capture time” | | LR-LIB-SHOWFINDER | Reveal original in file manager | P0 | ✅ | `cmd:app.showInFinder` | ⌘R (see MENU-FILE-SHOWFINDER) | | LR-LIB-COVER | Album cover | P2 | ✅ | `cmd:album.setCover` | | @@ -686,7 +686,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LRC-LIB-IMPORT | Full import dialog | P1 | ✅ | `cmd:library.import` (`mode`, `destination`, `organize`, `rename`, `metadataPreset`, `preset`, `keywords`, `album`), `crates/ui-egui/src/import.rs` | review grid with duplicates skipped; add in place or copy (library Originals or any folder; by day / by month / one folder; rename template with live example, numbered across the import); develop preset, metadata preset, keywords, album on import | +| LRC-LIB-IMPORT | Full import dialog | P1 | ✅ | `cmd:library.import` (`mode`, `destination`, `organize`, `rename`, `metadataPreset`, `preset`, `keywords`, `album`), `crates/ui-egui/src/import.rs` | review grid with duplicates skipped; add in place or copy (library Originals or any folder; by day / by month / one folder; rename template with live example, Tags picker (insert at cursor) and unknown-tag warning, numbered across the import); develop preset, metadata preset, keywords, album on import | | LRC-LIB-AUTOIMPORT | Watched-folder import | P2 | ✅ | `cmd:library.autoImport` | see LR-IMP-AUTO | | LRC-LIB-TETHER | Tethered capture | P2 | ⬜ | | | | LRC-LIB-VIEWS | Grid / loupe / compare / survey / people | P1 | 🟡 | `cmd:view.photoGrid`, `cmd:view.squareGrid`, `cmd:view.detail`, `cmd:view.compare`, `cmd:view.survey`, `cmd:view.gridInfo` | grid, square grid, loupe, compare, survey, second window; square-grid captions: file name, exposure or capture date (View ▸ Grid Info); no people view | From a16a81ad086e74ce58d911159db683e782d8a7ac Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:15:12 -0400 Subject: [PATCH 08/12] Issue #21: Local: kept folders stay roots while browsing below them Browsing a folder inside a listed Local location (built-in or kept) no longer adds it as a root of its own: it is highlighted inside that location's tree, which opens down to it once per browsed folder, so its ancestors and siblings stay reachable. A folder outside every listed location gets a session-only row that stays while browsing below it and offers Keep in Local; subfolder rows offer Add to Local. New UI command local.addRoot keeps a folder in Local (saved with ui.json, so it survives restarts; it un-hides it too); Browse Folder... uses it. Removal stays sidebar-only (local.hide). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/headless.rs | 48 +++++++++ crates/ui-egui/src/menus.rs | 16 +++ crates/ui-egui/src/panels/left.rs | 165 +++++++++++++++++++++++++----- crates/ui-egui/src/state.rs | 8 +- docs/parity.md | 2 +- 5 files changed, 211 insertions(+), 28 deletions(-) diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index b911896b..69c0a1f8 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -827,6 +827,54 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// Local: a kept folder stays a root while its subfolders (and other locations) are + /// browsed — the subfolder is highlighted inside its tree, its siblings stay listed — and + /// the kept folders survive a save/load of the UI state (a restart). + #[test] + fn kept_local_root_stays_while_browsing_below_and_elsewhere() { + let mut h = demo([1300.0, 1400.0]); + let t = Duration::from_secs(10); + let base = std::env::temp_dir().join(format!("lc-ui-roots-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + for d in ["Photos/2026/20260101", "Photos/2026/20260114", "Other"] { + std::fs::create_dir_all(base.join(d)).unwrap(); + } + let s = |p: std::path::PathBuf| p.to_string_lossy().to_string(); + let (photos, day1, day2, other) = + (s(base.join("Photos")), s(base.join("Photos/2026/20260101")), s(base.join("Photos/2026/20260114")), s(base.join("Other"))); + let exec = |h: &mut Headless, c: &str, p: Value| h.request("engine.execute", json!({"command": c, "params": p}), t); + let rects = |h: &mut Headless| -> std::collections::HashMap { + let w = h.request("ui.widgets", json!({"filter": "lc-ui-roots-"}), t); + w["result"].as_array().unwrap().iter().map(|x| (x["id"].as_str().unwrap().to_string(), x["rect"][0].as_f64().unwrap_or(0.0))).collect() + }; + h.request("ui.set", json!({"leftPanel": true}), t); + assert_eq!(exec(&mut h, "local.addRoot", json!({"path": photos}))["ok"], true); + assert_eq!(exec(&mut h, "local.addRoot", json!({"path": format!("{photos}/")}))["result"]["roots"].as_array().map(Vec::len), Some(1)); + exec(&mut h, "library.browse", json!({"path": day1})); + h.settle(SETTLE); + h.step(); + let r = rects(&mut h); + assert!(r.contains_key(&format!("source:local:{photos}")), "the kept root stays: {r:?}"); + assert!(r.contains_key(&format!("source:local:{day2}")), "the sibling stays reachable: {r:?}"); + let (root_x, child_x) = (r[&format!("folderToggle:{photos}")], r[&format!("folderToggle:{day1}")]); + assert!(child_x > root_x, "the browsed folder is inside the root's tree, not a root of its own ({child_x} vs {root_x})"); + assert_eq!(h.request("ui.clickWidget", json!({"id": format!("source:local:{day2}")}), t)["ok"], true); + h.settle(SETTLE); + assert_eq!(h.app.session.browse.as_ref().map(|b| b.path.clone()), Some(day2.clone()), "the sibling is browsed"); + // another location: the kept root stays listed + exec(&mut h, "library.browse", json!({"path": other})); + h.settle(SETTLE); + h.step(); + let r = rects(&mut h); + assert!(r.contains_key(&format!("source:local:{photos}")) && r.contains_key(&format!("source:local:{other}")), "{r:?}"); + // restart: the kept roots come back with the saved UI state + let saved = serde_json::to_string(&h.app.ui).unwrap(); + let back: crate::state::UiState = serde_json::from_str(&saved).unwrap(); + assert_eq!(back.local_roots, vec![photos.clone()]); + let _ = std::fs::remove_dir_all(&base); + h.settle(SETTLE); + } + /// Versions panel: resting on a version previews it in the loupe without changing the photo; /// a click restores it. #[test] diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 6f29a310..0bff45ae 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -53,6 +53,7 @@ pub const UI_COMMANDS: &[UiCommand] = &[ ("view.back", "Back to Grid", Some("Escape"), ""), ("tool.done", "Done", Some("Enter"), ""), ("view.filterBar", "Filter Bar", Some("Shift+F"), "View"), + ("local.addRoot", "Add Folder to Local", None, ""), ("local.hide", "Remove from Local", None, ""), ("local.restoreHidden", "Show Hidden Local Locations", None, ""), ("view.fullScreenPreview", "Full Screen Preview", Some("F"), "View"), @@ -555,6 +556,21 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option { + // a folder kept in Local's sidebar (saved with the UI state); nothing on disk changes + let Some(path) = p.get("path").and_then(Value::as_str) else { return Some(Err("local.addRoot needs a path".into())) }; + let abs = std::path::absolute(path).map(|a| a.to_string_lossy().trim_end_matches(['/', '\\']).to_string()).unwrap_or(path.into()); + let abs = if abs.is_empty() { path.to_string() } else { abs }; + if !std::path::Path::new(&abs).is_dir() { + return Some(Err(format!("{path}: not a folder"))); + } + use crate::panels::left::same_folder; + if !app.ui.local_roots.iter().any(|r| same_folder(r, &abs)) { + app.ui.local_roots.push(abs.clone()); + } + app.ui.hidden_locations.retain(|h| !same_folder(h, &abs)); + Ok(json!({"roots": app.ui.local_roots})) + } "local.hide" => match p.get("path").and_then(Value::as_str) { Some(path) => { if !app.ui.hidden_locations.iter().any(|h| crate::panels::left::same_folder(h, path)) { diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index bd4201c8..ff88a334 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -215,21 +215,23 @@ fn local_section(app: &mut LightcraftApp, ui: &mut egui::Ui) { } let browsing = app.session.browse.clone().filter(|_| app.session.source == LibrarySource::Folder); let current = browsing.as_ref().map(|b| b.path.clone()); - let places = local_places(builtin, &app.ui.local_roots, current.as_deref(), &app.ui.hidden_locations); - for (name, path) in places { - folder_tree(app, ui, &name, &path, 0.0, current.as_deref()); + let local = local_places(builtin, &app.ui.local_roots, current.as_deref(), app.ui.local_browse_root.as_deref(), &app.ui.hidden_locations); + if current.is_some() { + app.ui.local_browse_root = local.browse_root.clone(); + } + for (i, (name, path)) in local.places.iter().enumerate() { + let transient = local.browse_root.as_deref() == Some(path.as_str()); + let reveal = local.owner == Some(i); + folder_tree(app, ui, name, path, 0.0, current.as_deref(), reveal, transient); } if app.services.pick_folder.is_some() && row(app, ui, "local:browse", Icon::Plus, "Browse Folder…", None, false, 0.0).clicked() { let picked = app.services.pick_folder.as_mut().and_then(|f| f()); if let Some(path) = picked { - // choosing a folder again puts it back in the list - let _ = app.run("local.restoreHidden", json!({"path": path})); match app.run("library.browse", json!({"path": path})) { + // the picked folder stays in Local (and comes back if it was hidden) Ok(r) => { let dir = r["path"].as_str().unwrap_or(&path).to_string(); - if !app.ui.local_roots.iter().any(|p| same_folder(p, &dir)) { - app.ui.local_roots.push(dir); - } + let _ = app.run("local.addRoot", json!({"path": dir})); } Err(e) => app.toast(ui.ctx(), e), } @@ -254,20 +256,61 @@ pub(crate) fn same_folder(a: &str, b: &str) -> bool { a == b || lightcraft_catalog::query::folder_key(a) == lightcraft_catalog::query::folder_key(b) } -/// Local's top-level folders, in order: the built-in places, the folders picked with Browse -/// Folder…, then the folder being browsed when it isn't one of those. Each folder is listed +/// Local's top-level folders and how the folder being browsed sits among them. +#[derive(Debug, Default, PartialEq)] +pub(crate) struct LocalPlaces { + /// (label, path) of each top-level folder, in order. + pub places: Vec<(String, String)>, + /// The top-level folder the browsed folder lies in (the innermost one): its tree opens on + /// the way down to it. + pub owner: Option, + /// A folder listed only for this session because the browsed folder is in no saved + /// location (browsed from a breadcrumb, the CLI…); it stays while browsing below it. + pub browse_root: Option, +} + +fn folder_label(path: &str) -> String { + std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()) +} + +/// Local's top-level folders, in order: the built-in places, the folders kept with Browse +/// Folder… / Keep in Local (`saved`), then — only when the browsed folder lies in none of those — +/// a session root for it: the previous one (`browse_root`) while browsing stays below it, else +/// the browsed folder itself. A folder inside a listed one is shown inside that one's tree (the +/// root stays; its siblings stay reachable), never as a root of its own. Each folder is listed /// once however its path is spelled (the first spelling wins), and hidden ones are left out — /// they stay reachable through breadcrumbs and Browse Folder…. -pub(crate) fn local_places(builtin: Vec<(String, String)>, saved: &[String], browsing: Option<&str>, hidden: &[String]) -> Vec<(String, String)> { +pub(crate) fn local_places( + builtin: Vec<(String, String)>, + saved: &[String], + browsing: Option<&str>, + browse_root: Option<&str>, + hidden: &[String], +) -> LocalPlaces { + use lightcraft_catalog::query::{folder_key, folder_within}; + let is_hidden = |p: &str| hidden.iter().any(|h| same_folder(h, p)); let mut places = builtin; - for path in saved.iter().map(String::as_str).chain(browsing) { + for path in saved { if !places.iter().any(|(_, p)| same_folder(p, path)) { - let name = std::path::Path::new(path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string()); - places.push((name, path.to_string())); + places.push((folder_label(path), path.clone())); } } - places.retain(|(_, p)| !hidden.iter().any(|h| same_folder(h, p))); - places + places.retain(|(_, p)| !is_hidden(p)); + let mut out = LocalPlaces::default(); + if let Some(c) = browsing + && !places.iter().any(|(_, p)| folder_within(c, p)) + { + let root = browse_root.filter(|r| folder_within(c, r)).unwrap_or(c); + if !is_hidden(root) { + places.push((folder_label(root), root.to_string())); + out.browse_root = Some(root.to_string()); + } + } + if let Some(c) = browsing { + out.owner = places.iter().enumerate().filter(|(_, (_, p))| folder_within(c, p)).max_by_key(|(_, (_, p))| folder_key(p).len()).map(|(i, _)| i); + } + out.places = places; + out } /// The subfolders of `path` (not hidden ones), sorted; listed at most every 2 s per folder. @@ -296,13 +339,37 @@ fn subfolders(ui: &egui::Ui, path: &str) -> Vec<(String, String)> { } /// A folder on disk with a disclosure triangle: click browses it, the triangle lists its -/// subfolders (expanded on the way to the folder being browsed). -fn folder_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, name: &str, path: &str, indent: f32, current: Option<&str>) { +/// subfolders. In the tree that holds the folder being browsed (`reveal`), the folders on the +/// way down to it open whenever the browsed folder changes, so it shows highlighted in place. +/// `transient`: a top-level row listed for this session only (it offers Keep in Local). +#[allow(clippy::too_many_arguments)] +fn folder_tree( + app: &mut LightcraftApp, + ui: &mut egui::Ui, + name: &str, + path: &str, + indent: f32, + current: Option<&str>, + reveal: bool, + transient: bool, +) { let t = Tokens::get(ui.ctx()); let open_id = egui::Id::new(("folder-open", path.to_string())); let sel = current.is_some_and(|c| same_folder(c, path)); let on_the_way = !sel && current.is_some_and(|c| lightcraft_catalog::query::folder_within(c, path)); - let mut open: bool = ui.data(|d| d.get_temp(open_id)).unwrap_or(on_the_way); + let mut open: bool = ui.data(|d| d.get_temp(open_id)).unwrap_or(false); + if reveal && on_the_way { + // opened once per browsed folder: collapsing it again afterwards sticks + let revealed_id = egui::Id::new(("folder-revealed", path.to_string())); + let target = current.map(str::to_string); + if ui.data(|d| d.get_temp::>(revealed_id)) != Some(target.clone()) { + open = true; + ui.data_mut(|d| { + d.insert_temp(open_id, true); + d.insert_temp(revealed_id, target); + }); + } + } let resp = row(app, ui, &format!("local:{path}"), Icon::Folder, name, None, sel, indent + 12.0).on_hover_text(path); let c = pos2(resp.rect.left() + 10.0 + indent, resp.rect.center().y); let tri = Rect::from_center_size(c, vec2(14.0, 14.0)); @@ -324,6 +391,12 @@ fn folder_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, name: &str, path: &st app.toast(ui.ctx(), e); } resp.context_menu(|ui| { + if (transient || indent > 0.0) + && ui.button(if transient { "Keep in Local" } else { "Add to Local" }).on_hover_text("List this folder in Local from now on").clicked() + { + let _ = app.run("local.addRoot", json!({"path": path})); + ui.close(); + } if indent == 0.0 && ui.button("Remove from Local").on_hover_text("Hides this shortcut only; the folder and its photos stay as they are").clicked() { @@ -358,7 +431,7 @@ fn folder_tree(app: &mut LightcraftApp, ui: &mut egui::Ui, name: &str, path: &st }); if open && indent < 12.0 * 8.0 { for (n, p) in subfolders(ui, path) { - folder_tree(app, ui, &n, &p, indent + 12.0, current); + folder_tree(app, ui, &n, &p, indent + 12.0, current, reveal, false); } } } @@ -631,16 +704,56 @@ mod tests { // the browsed spelling: other separators, a trailing one, a different drive-letter case let flipped = p.replace(['/', '\\'], if sep == '/' { "\\" } else { "/" }); for browsing in [flipped.clone(), format!("{p}{sep}"), p.replacen("D:", "d:", 1)] { - let places = local_places(builtin.clone(), std::slice::from_ref(&browsing), Some(&browsing), &[]); - assert_eq!(names(&places), ["Pictures", "Desktop", "Downloads"], "{p} browsed as {browsing}"); + let l = local_places(builtin.clone(), std::slice::from_ref(&browsing), Some(&browsing), None, &[]); + assert_eq!(names(&l.places), ["Pictures", "Desktop", "Downloads"], "{p} browsed as {browsing}"); + assert_eq!(l.places[l.owner.unwrap()].1, p, "the built-in row is the one highlighted"); } // hiding under one spelling hides the other - let places = local_places(builtin.clone(), &[], None, std::slice::from_ref(&flipped)); - assert_eq!(places.len(), 2, "hidden {flipped}"); + let l = local_places(builtin.clone(), &[], None, None, std::slice::from_ref(&flipped)); + assert_eq!(l.places.len(), 2, "hidden {flipped}"); } } // a different folder is still added - let places = local_places(vec![("Pictures".into(), "/home/example/Pictures".into())], &[], Some("/home/example/Pictures2"), &[]); - assert_eq!(names(&places), ["Pictures", "Pictures2"]); + let l = local_places(vec![("Pictures".into(), "/home/example/Pictures".into())], &[], Some("/home/example/Pictures2"), None, &[]); + assert_eq!(names(&l.places), ["Pictures", "Pictures2"]); + } + + /// A folder inside a kept root is shown inside that root's tree: the root stays (its other + /// subfolders stay reachable) and no second root appears; other kept roots stay too. + #[test] + fn browsing_below_a_kept_root_keeps_the_root() { + let photos = "/data/Photos".to_string(); + let other = "/data/Scans".to_string(); + let saved = [photos.clone(), other.clone()]; + let builtin = || vec![("Home".to_string(), "/home/example".to_string())]; + for browsing in ["/data/Photos/2026/20260101", "/data/Photos/2026", "/data/Photos", "D:\\x"] { + let l = local_places(builtin(), &saved, Some(browsing), None, &[]); + let roots: Vec<&str> = l.places.iter().map(|(_, p)| p.as_str()).collect(); + if browsing.starts_with("/data") { + assert_eq!(roots, ["/home/example", "/data/Photos", "/data/Scans"], "{browsing}"); + assert_eq!(l.owner, Some(1), "the Photos tree opens down to {browsing}"); + assert_eq!(l.browse_root, None); + } else { + assert_eq!(roots, ["/home/example", "/data/Photos", "/data/Scans", "D:\\x"], "a folder outside them gets a row"); + } + } + // the innermost containing root is the one that opens + let l = local_places(builtin(), &["/home/example/Pictures".into()], Some("/home/example/Pictures/Trip"), None, &[]); + assert_eq!(l.owner, Some(1)); + } + + /// A browsed folder outside every kept root gets a session row, which stays while browsing + /// below it and gives way when browsing moves elsewhere. + #[test] + fn session_root_stays_while_browsing_below_it() { + let l = local_places(Vec::new(), &[], Some("/t/base"), None, &[]); + assert_eq!(l.browse_root.as_deref(), Some("/t/base")); + let l = local_places(Vec::new(), &[], Some("/t/base/Trip/Day 1"), l.browse_root.as_deref(), &[]); + assert_eq!((l.places.len(), l.browse_root.as_deref(), l.owner), (1, Some("/t/base"), Some(0)), "{l:?}"); + let l = local_places(Vec::new(), &[], Some("/elsewhere"), l.browse_root.as_deref(), &[]); + assert_eq!(l.browse_root.as_deref(), Some("/elsewhere")); + // hidden: no row at all + let l = local_places(Vec::new(), &[], Some("/t/base"), None, &["/t/base/".into()]); + assert!(l.places.is_empty() && l.browse_root.is_none()); } } diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index a9749f89..87154661 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -289,8 +289,13 @@ pub struct UiState { pub spots_threshold: f32, /// The library filter bar above the grid. pub filter_bar: bool, - /// Folders picked with Local → Browse Folder…, kept in the sidebar after browsing elsewhere. + /// Folders kept in Local (Browse Folder…, Keep in Local / Add to Local; `local.addRoot`): + /// listed after the built-in locations, across restarts, whatever is browsed. pub local_roots: Vec, + /// The top-level Local row listed for this session only, for a browsed folder outside every + /// kept location; it stays while browsing below it (see `panels::left::local_places`). + #[serde(skip)] + pub local_browse_root: Option, /// Culling: after a rating, flag or colour-label key, move to the next photo. pub auto_advance: bool, /// Full-screen preview (F): the photo alone on black, no chrome. @@ -544,6 +549,7 @@ impl Default for UiState { window_fullscreen: None, filter_bar: false, local_roots: Vec::new(), + local_browse_root: None, compare: None, reference: None, toast: None, diff --git a/docs/parity.md b/docs/parity.md index 2d1b7392..1c2e46dc 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -108,7 +108,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-IMP-RAWDEFAULT | Raw defaults | P1 | ✅ | `cmd:library.preferences`, `crates/engine/src/import.rs`, `crates/ui-egui/src/panels/settings.rs` | LightCraft default / a preset / per camera (make + model); non-raw default too; the preset look counts as unedited and Reset returns to it | | LR-IMP-MIGRATE | Migrate other catalogs | OOS | 🚫 | | | | LR-IMP-PROFILES | Import profiles & presets | P1 | ✅ | `cmd:file.importPresets`, `cmd:preset.import`, `cmd:profile.import`, `crates/engine/src/preset_import.rs`, `crates/engine/src/preset_luminar.rs` | presets: .lcpreset, XMP, classic .lrtemplate, photos carrying edits ("DNG presets"), Luminar looks (.lmp, .mplumpack collections; sliders with a counterpart), .zip bundles, folders (folder → group), drag & drop; masks carried over; unmapped settings reported; profiles: .cube 3D LUTs; Adobe profile formats deliberately unsupported | -| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back; a folder is one location however its path is spelled (`/` or `\`, trailing separator, `.`/`..`, drive-letter case; `folder_key` in `crates/catalog/src/query.rs`) | +| LR-IMP-LOCAL | Work on files in place | P0 | ✅ | `cmd:library.browse`, `cmd:photo.addToLibrary`, `cmd:library.import` (mode add), `cmd:local.addRoot`, `cmd:local.hide`, `cmd:local.restoreHidden`, `crates/engine/src/cmd/browse.rs`, `crates/ui-egui/src/panels/left.rs` (`local_section`), `crates/ui-egui/src/panels/grid.rs` (`folder_header`) | Local: browse Pictures / Desktop / Downloads / Home or any folder without adding it (breadcrumb, Include subfolders, Add N to My Photos); browsed photos stay out of All Photos, albums and counts; folders picked with Browse Folder… (or Keep in Local / Add to Local on a folder row) stay listed across restarts, and browsing a folder inside a listed one highlights it inside that tree (opened down to it, siblings reachable) instead of adding a root; edits go to XMP sidecars; importing promotes them; right-click a top-level location → Remove from Local hides the shortcut only (nothing on disk or in the catalog changes), saved with the UI state, and “Show N hidden locations” (or Browse Folder… on it) brings it back; a folder is one location however its path is spelled (`/` or `\`, trailing separator, `.`/`..`, drive-letter case; `folder_key` in `crates/catalog/src/query.rs`) | | LR-IMP-SIDECAR-SPLIT | Separate XMP sidecar variants | P2 | ⬜ | `cmd:library.xmpPreferences` | sidecar naming option exists (stem/full), no split sidecars | | LR-IMP-FORMATS | Supported formats | P0 | 🟡 | `crates/codecs/src/lib.rs`, `crates/raw/src/lib.rs` | JPEG, PNG, TIFF, WebP, JXL, PSD, GIF, BMP; DNG (incl. lossy / Smart Preview DNG), CR2, ARW, NEF, RAF, RW2, PEF, ORF. Missing: CR3, compressed NEF/RAF/ORF, RW2 v4 (preview only: imported with `preview_only` = the decoder's reason, rendered as a rendered JPEG, flagged by a grid badge and loupe/Edit/Info notices, `previewOnly` in `catalog.query`; Reload clears it once the file decodes), HEIC/AVIF decode | | LR-IMP-CULL-AT-IMPORT | Culling analysis at import | P2 | 🟡 | `cmd:photo.analyze` | run Assisted Culling on the imported photos (they're selected after an import); not automatic | From d6476e749736ee80bd3137728417ae4629cb65b6 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:18:04 -0400 Subject: [PATCH 09/12] Issue #20: Panels: resize the left sidebar and the right panel by dragging their edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both side panels had an exact width. They are now resizable egui panels whose width lives in UiState (leftWidth / rightWidth, serde, sanitized on load) so it survives panel switches, view changes and restarts; egui's own remembered size is dropped each frame and only a drag of the edge writes back (measured from the fixed edge to the pointer, so content wider than the panel can't creep it). Limits: left 200–480 pt, right 250–520 pt, and the photo area keeps at least 360 pt; a narrower window shrinks the panels without forgetting the chosen width. The bottom bar, which sits between the panels, now drops Copy/Paste Edit Settings and then the rating pill when its centre has no room instead of overlapping the zoom controls. The unused left_w theme token is gone. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/lib.rs | 2 + crates/ui-egui/src/panels/bottombar.rs | 354 +++++++++++++------------ crates/ui-egui/src/panels/left.rs | 161 ++++++----- crates/ui-egui/src/panels/mod.rs | 32 +++ crates/ui-egui/src/panels/right.rs | 55 ++-- crates/ui-egui/src/state.rs | 30 +++ crates/ui-egui/src/tests_panels.rs | 95 +++++++ crates/ui-egui/src/theme.rs | 2 - docs/parity.md | 7 +- 9 files changed, 459 insertions(+), 279 deletions(-) create mode 100644 crates/ui-egui/src/tests_panels.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index e42ed5ea..128f3c03 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -27,6 +27,8 @@ mod tests_curve; #[cfg(test)] mod tests_masking; #[cfg(test)] +mod tests_panels; +#[cfg(test)] mod tests_scroll; use std::sync::mpsc::{Receiver, Sender}; diff --git a/crates/ui-egui/src/panels/bottombar.rs b/crates/ui-egui/src/panels/bottombar.rs index 28cee606..a904c87c 100644 --- a/crates/ui-egui/src/panels/bottombar.rs +++ b/crates/ui-egui/src/panels/bottombar.rs @@ -17,179 +17,199 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { .frame(egui::Frame::NONE.fill(t.chrome).inner_margin(egui::Margin { left: 12, right: 12, top: 0, bottom: 0 })) .show(ui, |ui| { let full = ui.max_rect(); - ui.horizontal_centered(|ui| { - ui.spacing_mut().item_spacing.x = 2.0; - for (id, icon, mode, tip) in [ - ("photoGrid", Icon::GridPhoto, ViewMode::PhotoGrid, "Photo Grid (G)"), - ("squareGrid", Icon::GridSquare, ViewMode::SquareGrid, "Square Grid (G toggles)"), - ("detail", Icon::Single, ViewMode::Detail, "Detail (D)"), - ("compare", Icon::Compare, ViewMode::Compare, "Compare (Shift+C)"), - ("survey", Icon::Survey, ViewMode::Survey, "Survey (N)"), - ] { - if icon_button(ui, id, icon, vec2(32.0, 32.0), app.ui.view == mode, true, tip).clicked() { - let _ = app.run(&format!("view.{id}"), json!({})); - } - } - ui.add_space(10.0); - let (sep, _) = ui.allocate_exact_size(vec2(1.0, 22.0), Sense::hover()); - ui.painter().rect_filled(sep, 0.0, t.button_border); - ui.add_space(10.0); - let sort = icon_button(ui, "sort", Icon::Sort, vec2(30.0, 30.0), false, true, "Sort"); - egui::Popup::menu(&sort).show(|ui| sort_menu(app, ui)); - }); - - // centre: rating/flag pill + copy/paste settings - let active = app.session.active().and_then(|id| app.session.catalog.photo(id).cloned()); - let pill = Rect::from_center_size(pos2(full.center().x - 60.0, full.center().y), vec2(196.0, 30.0)); - ui.painter().rect_filled(pill, 15.0, t.canvas); - let mut pill_ui = - ui.new_child(egui::UiBuilder::new().max_rect(pill.shrink2(vec2(10.0, 4.0))).layout(egui::Layout::left_to_right(egui::Align::Center))); - pill_ui.spacing_mut().item_spacing.x = 3.0; - let rating = active.as_ref().map(|p| p.rating).unwrap_or(0); - if let Some(r) = stars(&mut pill_ui, "bottom", rating, 19.0) { - let mut params = json!({"rating": r}); - crate::panels::compare::target_active(app, &mut params); - let _ = app.run("photo.rate", params); - } - pill_ui.add_space(6.0); - let flag = active.as_ref().map(|p| p.flag).unwrap_or_default(); - for (id, icon, f) in - [("unflag", Icon::Circle, Flag::None), ("pick", Icon::FlagPick, Flag::Pick), ("reject", Icon::FlagReject, Flag::Reject)] - { - let (r, resp) = pill_ui.allocate_exact_size(vec2(22.0, 22.0), Sense::click()); - register(pill_ui.ctx(), format!("flag:{id}"), r); - let on = flag == f && f != Flag::None; - let c = if on { - if f == Flag::Reject { t.reject } else { t.pick } - } else if resp.hovered() { - t.text - } else { - t.icon - }; - paint(pill_ui.painter(), r.shrink(3.0), icon, c); - if resp.clicked() { - let mut params = json!({"flag": match f { Flag::Pick => "pick", Flag::Reject => "reject", Flag::None => "none" }}); - crate::panels::compare::target_active(app, &mut params); - let _ = app.run("photo.flag", params); - } - } - // copy / paste settings - let copy_r = Rect::from_min_size(pos2(pill.right() + 10.0, pill.top()), vec2(136.0, 30.0)); - let has_clip = app.session.clipboard.is_some(); - let label = if has_clip { "Paste Edit Settings" } else { "Copy Edit Settings" }; - let cresp = ui.interact(copy_r, egui::Id::new("copy-settings"), Sense::click()); - register(ui.ctx(), "button:copySettings", copy_r); - ui.painter().rect_filled(copy_r, 15.0, if cresp.hovered() { t.hover } else { t.canvas }); - ui.painter().text( - copy_r.center(), - Align2::CENTER_CENTER, - label, - t.font(13.0), - if active.is_some() { t.text_label } else { t.text_disabled }, - ); - if cresp.clicked() && active.is_some() { - let _ = if has_clip { app.run("develop.paste", json!({})) } else { app.run("develop.copy", json!({})) }; - let msg = if has_clip { "Settings pasted" } else { "Edit settings copied" }; - app.toast(ui.ctx(), msg); - } - let gear_r = Rect::from_min_size(pos2(copy_r.right() + 4.0, pill.top()), vec2(30.0, 30.0)); - let gresp = ui.interact(gear_r, egui::Id::new("copy-gear"), Sense::click()); - register(ui.ctx(), "icon:copyGear", gear_r); - ui.painter().rect_filled(gear_r, 15.0, if gresp.hovered() { t.hover } else { t.canvas }); - paint(ui.painter(), gear_r.shrink(7.0), Icon::Gear, t.icon); - if gresp.clicked() { - let _ = app.run("dialog.copySettings", json!({})); - } - - // right side - let mut child = ui.new_child( - egui::UiBuilder::new() - .max_rect(Rect::from_min_max(pos2(full.right() - 360.0, full.top()), full.right_bottom())) - .layout(egui::Layout::right_to_left(egui::Align::Center)), - ); - child.spacing_mut().item_spacing.x = 6.0; - if matches!(app.ui.view, ViewMode::Compare | ViewMode::Survey) - && icon_button( - &mut child, - "autoAdvance", - Icon::ChevronRight, - vec2(30.0, 30.0), - app.ui.auto_advance, - true, - "Auto Advance after rating or flagging", - ) - .clicked() - { - let _ = app.run("view.autoAdvance", json!({})); - } - if app.ui.view == ViewMode::Survey { - // no zoom or thumbnail size in Survey - } else if matches!(app.ui.view, ViewMode::Detail | ViewMode::Compare) { - if icon_button( - &mut child, - "beforeAfter", - Icon::BeforeAfter, - vec2(30.0, 30.0), - app.ui.before_after != BeforeAfter::Off, - true, - "Before/After (Y)", - ) - .clicked() - { - let _ = app.run("view.beforeAfter", json!({})); - } - if icon_button(&mut child, "filmstrip", Icon::Filmstrip, vec2(30.0, 30.0), app.ui.filmstrip, true, "Filmstrip (/)").clicked() { - let _ = app.run("view.filmstrip", json!({})); - } - child.add_space(10.0); - let zoom_label = match app.ui.zoom { - Zoom::Fit => "Fit".to_string(), - Zoom::Fill => "Fill".to_string(), - Zoom::Percent(p) => format!("{p}%"), - }; - let zr = crate::widgets::dropdown(&mut child, "zoom", &zoom_label, t.font(13.0), t.text_label); - egui::Popup::menu(&zr).show(|ui| { - for (label, z) in [ - ("Fit", Zoom::Fit), - ("Fill", Zoom::Fill), - ("50%", Zoom::Percent(50)), - ("100%", Zoom::Percent(100)), - ("200%", Zoom::Percent(200)), - ("400%", Zoom::Percent(400)), + let left_end = ui + .horizontal_centered(|ui| { + ui.spacing_mut().item_spacing.x = 2.0; + for (id, icon, mode, tip) in [ + ("photoGrid", Icon::GridPhoto, ViewMode::PhotoGrid, "Photo Grid (G)"), + ("squareGrid", Icon::GridSquare, ViewMode::SquareGrid, "Square Grid (G toggles)"), + ("detail", Icon::Single, ViewMode::Detail, "Detail (D)"), + ("compare", Icon::Compare, ViewMode::Compare, "Compare (Shift+C)"), + ("survey", Icon::Survey, ViewMode::Survey, "Survey (N)"), ] { - if ui.selectable_label(app.ui.zoom == z, label).clicked() { - app.ui.zoom = z; - app.ui.pan = (0.5, 0.5); + if icon_button(ui, id, icon, vec2(32.0, 32.0), app.ui.view == mode, true, tip).clicked() { + let _ = app.run(&format!("view.{id}"), json!({})); } } - }); - child.add_space(6.0); - let cz = - crate::widgets::dropdown(&mut child, "clickZoom", &format!("Click {}:1", app.ui.click_zoom / 100), t.font(13.0), t.text_label); - egui::Popup::menu(&cz).show(|ui| { - for pct in crate::state::CLICK_ZOOMS { - if ui.selectable_label(app.ui.click_zoom == pct, format!("{}:1", pct / 100)).clicked() { - let _ = app.run("view.clickZoom", json!({"ratio": pct / 100})); - } - } - }); - } else { - // thumbnail size slider - let (r, resp) = child.allocate_exact_size(vec2(110.0, 20.0), Sense::click_and_drag()); - register(child.ctx(), "slider:thumbSize", r); - let p = child.painter(); - let y = r.center().y; - p.line_segment([pos2(r.left(), y), pos2(r.right(), y)], egui::Stroke::new(2.0, t.track)); - let f = (app.ui.thumb_size - 90.0) / (480.0 - 90.0); - let x = r.left() + f * r.width(); - p.rect_filled(Rect::from_center_size(pos2(x, y), vec2(3.0, 14.0)), 1.0, t.thumb_hover); - if let Some(pp) = resp.interact_pointer_pos() - && (resp.dragged() || resp.clicked()) - { - app.ui.thumb_size = 90.0 + ((pp.x - r.left()) / r.width()).clamp(0.0, 1.0) * (480.0 - 90.0); + ui.add_space(10.0); + let (sep, _) = ui.allocate_exact_size(vec2(1.0, 22.0), Sense::hover()); + ui.painter().rect_filled(sep, 0.0, t.button_border); + ui.add_space(10.0); + let sort = icon_button(ui, "sort", Icon::Sort, vec2(30.0, 30.0), false, true, "Sort"); + egui::Popup::menu(&sort).show(|ui| sort_menu(app, ui)); + }) + .response + .rect + .right(); + let right_start = right_side(app, ui, full); + centre(app, ui, full, left_end + 12.0, right_start - 12.0); + }); +} + +/// Width of the centre group: the rating/flag pill, then Copy/Paste Edit Settings and its gear. +const PILL_W: f32 = 196.0; +const CENTRE_W: f32 = PILL_W + 10.0 + 136.0 + 4.0 + 30.0; + +/// The rating/flag pill and copy/paste settings, between `from` and `to` (the side groups): the +/// copy buttons go first when there is no room (they are in the Edit menu too), then the pill. +fn centre(app: &mut LightcraftApp, ui: &mut egui::Ui, full: Rect, from: f32, to: f32) { + let t = Tokens::get(ui.ctx()); + let room = to - from; + if room < PILL_W { + return; + } + let with_copy = room >= CENTRE_W; + let w = if with_copy { CENTRE_W } else { PILL_W }; + // where it sits with room to spare (slightly left of centre), kept between the side groups + let left = (full.center().x - 60.0 - PILL_W / 2.0).clamp(from, to - w); + let active = app.session.active().and_then(|id| app.session.catalog.photo(id).cloned()); + let pill = Rect::from_min_size(pos2(left, full.center().y - 15.0), vec2(PILL_W, 30.0)); + ui.painter().rect_filled(pill, 15.0, t.canvas); + let mut pill_ui = + ui.new_child(egui::UiBuilder::new().max_rect(pill.shrink2(vec2(10.0, 4.0))).layout(egui::Layout::left_to_right(egui::Align::Center))); + pill_ui.spacing_mut().item_spacing.x = 3.0; + let rating = active.as_ref().map(|p| p.rating).unwrap_or(0); + if let Some(r) = stars(&mut pill_ui, "bottom", rating, 19.0) { + let mut params = json!({"rating": r}); + crate::panels::compare::target_active(app, &mut params); + let _ = app.run("photo.rate", params); + } + pill_ui.add_space(6.0); + let flag = active.as_ref().map(|p| p.flag).unwrap_or_default(); + for (id, icon, f) in [("unflag", Icon::Circle, Flag::None), ("pick", Icon::FlagPick, Flag::Pick), ("reject", Icon::FlagReject, Flag::Reject)] { + let (r, resp) = pill_ui.allocate_exact_size(vec2(22.0, 22.0), Sense::click()); + register(pill_ui.ctx(), format!("flag:{id}"), r); + let on = flag == f && f != Flag::None; + let c = if on { + if f == Flag::Reject { t.reject } else { t.pick } + } else if resp.hovered() { + t.text + } else { + t.icon + }; + paint(pill_ui.painter(), r.shrink(3.0), icon, c); + if resp.clicked() { + let mut params = json!({"flag": match f { Flag::Pick => "pick", Flag::Reject => "reject", Flag::None => "none" }}); + crate::panels::compare::target_active(app, &mut params); + let _ = app.run("photo.flag", params); + } + } + // copy / paste settings + if !with_copy { + return; + } + let copy_r = Rect::from_min_size(pos2(pill.right() + 10.0, pill.top()), vec2(136.0, 30.0)); + let has_clip = app.session.clipboard.is_some(); + let label = if has_clip { "Paste Edit Settings" } else { "Copy Edit Settings" }; + let cresp = ui.interact(copy_r, egui::Id::new("copy-settings"), Sense::click()); + register(ui.ctx(), "button:copySettings", copy_r); + ui.painter().rect_filled(copy_r, 15.0, if cresp.hovered() { t.hover } else { t.canvas }); + ui.painter().text(copy_r.center(), Align2::CENTER_CENTER, label, t.font(13.0), if active.is_some() { t.text_label } else { t.text_disabled }); + if cresp.clicked() && active.is_some() { + let _ = if has_clip { app.run("develop.paste", json!({})) } else { app.run("develop.copy", json!({})) }; + let msg = if has_clip { "Settings pasted" } else { "Edit settings copied" }; + app.toast(ui.ctx(), msg); + } + let gear_r = Rect::from_min_size(pos2(copy_r.right() + 4.0, pill.top()), vec2(30.0, 30.0)); + let gresp = ui.interact(gear_r, egui::Id::new("copy-gear"), Sense::click()); + register(ui.ctx(), "icon:copyGear", gear_r); + ui.painter().rect_filled(gear_r, 15.0, if gresp.hovered() { t.hover } else { t.canvas }); + paint(ui.painter(), gear_r.shrink(7.0), Icon::Gear, t.icon); + if gresp.clicked() { + let _ = app.run("dialog.copySettings", json!({})); + } +} + +/// The right-hand group (zoom, view toggles or thumbnail size); returns its left edge. +fn right_side(app: &mut LightcraftApp, ui: &mut egui::Ui, full: Rect) -> f32 { + let t = Tokens::get(ui.ctx()); + let mut child = ui.new_child( + egui::UiBuilder::new() + .max_rect(Rect::from_min_max(pos2(full.right() - 360.0, full.top()), full.right_bottom())) + .layout(egui::Layout::right_to_left(egui::Align::Center)), + ); + child.spacing_mut().item_spacing.x = 6.0; + if matches!(app.ui.view, ViewMode::Compare | ViewMode::Survey) + && icon_button( + &mut child, + "autoAdvance", + Icon::ChevronRight, + vec2(30.0, 30.0), + app.ui.auto_advance, + true, + "Auto Advance after rating or flagging", + ) + .clicked() + { + let _ = app.run("view.autoAdvance", json!({})); + } + if app.ui.view == ViewMode::Survey { + // no zoom or thumbnail size in Survey + } else if matches!(app.ui.view, ViewMode::Detail | ViewMode::Compare) { + if icon_button( + &mut child, + "beforeAfter", + Icon::BeforeAfter, + vec2(30.0, 30.0), + app.ui.before_after != BeforeAfter::Off, + true, + "Before/After (Y)", + ) + .clicked() + { + let _ = app.run("view.beforeAfter", json!({})); + } + if icon_button(&mut child, "filmstrip", Icon::Filmstrip, vec2(30.0, 30.0), app.ui.filmstrip, true, "Filmstrip (/)").clicked() { + let _ = app.run("view.filmstrip", json!({})); + } + child.add_space(10.0); + let zoom_label = match app.ui.zoom { + Zoom::Fit => "Fit".to_string(), + Zoom::Fill => "Fill".to_string(), + Zoom::Percent(p) => format!("{p}%"), + }; + let zr = crate::widgets::dropdown(&mut child, "zoom", &zoom_label, t.font(13.0), t.text_label); + egui::Popup::menu(&zr).show(|ui| { + for (label, z) in [ + ("Fit", Zoom::Fit), + ("Fill", Zoom::Fill), + ("50%", Zoom::Percent(50)), + ("100%", Zoom::Percent(100)), + ("200%", Zoom::Percent(200)), + ("400%", Zoom::Percent(400)), + ] { + if ui.selectable_label(app.ui.zoom == z, label).clicked() { + app.ui.zoom = z; + app.ui.pan = (0.5, 0.5); } } }); + child.add_space(6.0); + let cz = crate::widgets::dropdown(&mut child, "clickZoom", &format!("Click {}:1", app.ui.click_zoom / 100), t.font(13.0), t.text_label); + egui::Popup::menu(&cz).show(|ui| { + for pct in crate::state::CLICK_ZOOMS { + if ui.selectable_label(app.ui.click_zoom == pct, format!("{}:1", pct / 100)).clicked() { + let _ = app.run("view.clickZoom", json!({"ratio": pct / 100})); + } + } + }); + } else { + // thumbnail size slider + let (r, resp) = child.allocate_exact_size(vec2(110.0, 20.0), Sense::click_and_drag()); + register(child.ctx(), "slider:thumbSize", r); + let p = child.painter(); + let y = r.center().y; + p.line_segment([pos2(r.left(), y), pos2(r.right(), y)], egui::Stroke::new(2.0, t.track)); + let f = (app.ui.thumb_size - 90.0) / (480.0 - 90.0); + let x = r.left() + f * r.width(); + p.rect_filled(Rect::from_center_size(pos2(x, y), vec2(3.0, 14.0)), 1.0, t.thumb_hover); + if let Some(pp) = resp.interact_pointer_pos() + && (resp.dragged() || resp.clicked()) + { + app.ui.thumb_size = 90.0 + ((pp.x - r.left()) / r.width()).clamp(0.0, 1.0) * (480.0 - 90.0); + } + } + if child.min_rect().width() > 0.0 { child.min_rect().left() } else { full.right() } } fn sort_menu(app: &mut LightcraftApp, ui: &mut egui::Ui) { diff --git a/crates/ui-egui/src/panels/left.rs b/crates/ui-egui/src/panels/left.rs index 72516322..60ef6004 100644 --- a/crates/ui-egui/src/panels/left.rs +++ b/crates/ui-egui/src/panels/left.rs @@ -56,94 +56,93 @@ fn row( pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); - egui::Panel::left("left_panel") - .exact_size(t.left_w) - .resizable(false) - .frame(egui::Frame::NONE.fill(t.chrome).stroke(egui::Stroke::new(1.0, t.divider))) - .show(ui, |ui| { - ui.spacing_mut().item_spacing.y = 0.0; - let (hr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 44.0), Sense::hover()); - ui.painter().text(pos2(hr.left() + 18.0, hr.center().y), Align2::LEFT_CENTER, "My Photos", t.semibold(15.0), t.text); - let counts = app.caches.counts(&app.session.catalog); - let (total, picks, deleted) = (counts.total, counts.picks, counts.deleted); - egui::ScrollArea::vertical().id_salt("left-scroll").auto_shrink([false, false]).show(ui, |ui| { - let src = app.session.source; - for (id, icon, label, count, s) in [ - ("all", Icon::Photos, "All Photos", Some(total), LibrarySource::All), - ("recentlyAdded", Icon::Clock, "Recently Added", None, LibrarySource::RecentlyAdded), - ("picks", Icon::FlagPick, "Picks", Some(picks), LibrarySource::Picks), - ] { - if row(app, ui, id, icon, label, count, src == s, 0.0).clicked() { - let _ = app.run("library.source", json!({"kind": id})); - } + let frame = egui::Frame::NONE.fill(t.chrome).stroke(egui::Stroke::new(1.0, t.divider)); + let width = app.ui.left_width; + let resized = super::resizable_side(ui, true, "left_panel", frame, width, crate::state::LEFT_WIDTH, 0.0, |ui| { + ui.spacing_mut().item_spacing.y = 0.0; + let (hr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 44.0), Sense::hover()); + ui.painter().text(pos2(hr.left() + 18.0, hr.center().y), Align2::LEFT_CENTER, "My Photos", t.semibold(15.0), t.text); + let counts = app.caches.counts(&app.session.catalog); + let (total, picks, deleted) = (counts.total, counts.picks, counts.deleted); + egui::ScrollArea::vertical().id_salt("left-scroll").auto_shrink([false, false]).show(ui, |ui| { + let src = app.session.source; + for (id, icon, label, count, s) in [ + ("all", Icon::Photos, "All Photos", Some(total), LibrarySource::All), + ("recentlyAdded", Icon::Clock, "Recently Added", None, LibrarySource::RecentlyAdded), + ("picks", Icon::FlagPick, "Picks", Some(picks), LibrarySource::Picks), + ] { + if row(app, ui, id, icon, label, count, src == s, 0.0).clicked() { + let _ = app.run("library.source", json!({"kind": id})); } - // photos whose files can't be found (checked every few seconds, not every frame) - let missing = missing_count(app, ui); - if (missing > 0 || src == LibrarySource::Missing) - && row(app, ui, "missing", Icon::Folder, "Missing Photos", Some(missing), src == LibrarySource::Missing, 0.0).clicked() - { - let _ = app.run("library.source", json!({"kind": "missing"})); + } + // photos whose files can't be found (checked every few seconds, not every frame) + let missing = missing_count(app, ui); + if (missing > 0 || src == LibrarySource::Missing) + && row(app, ui, "missing", Icon::Folder, "Missing Photos", Some(missing), src == LibrarySource::Missing, 0.0).clicked() + { + let _ = app.run("library.source", json!({"kind": "missing"})); + } + ui.add_space(10.0); + // Albums header + let (ar, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); + ui.painter().text(pos2(ar.left() + 18.0, ar.center().y), Align2::LEFT_CENTER, "Albums", t.semibold(13.5), t.text_label); + let mut hdr = ui.new_child( + egui::UiBuilder::new() + .max_rect(Rect::from_min_max(pos2(ar.right() - 50.0, ar.top()), ar.right_bottom())) + .layout(egui::Layout::right_to_left(egui::Align::Center)), + ); + let plus = icon_button(&mut hdr, "albumNew", Icon::Plus, vec2(26.0, 26.0), false, true, "Create Album"); + egui::Popup::menu(&plus).show(|ui| { + if ui.button("Create Album…").clicked() { + app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: false }); } - ui.add_space(10.0); - // Albums header - let (ar, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); - ui.painter().text(pos2(ar.left() + 18.0, ar.center().y), Align2::LEFT_CENTER, "Albums", t.semibold(13.5), t.text_label); - let mut hdr = ui.new_child( - egui::UiBuilder::new() - .max_rect(Rect::from_min_max(pos2(ar.right() - 50.0, ar.top()), ar.right_bottom())) - .layout(egui::Layout::right_to_left(egui::Align::Center)), - ); - let plus = icon_button(&mut hdr, "albumNew", Icon::Plus, vec2(26.0, 26.0), false, true, "Create Album"); - egui::Popup::menu(&plus).show(|ui| { - if ui.button("Create Album…").clicked() { - app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: false }); - } - if ui.button("Create Smart Album…").clicked() { - app.ui.dialog = Some(crate::state::Dialog::SmartRules { - id: None, - name: String::new(), - rules: lightcraft_catalog::RuleSet { rules: vec![crate::panels::rules_editor::new_rule()], ..Default::default() }, - }); - } - if ui.button("Create Smart Album from Filter…").clicked() { - app.ui.dialog = Some(crate::state::Dialog::NewSmartAlbum { name: String::new() }); - } - if ui.button("Create Folder…").clicked() { - app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: true }); - } - }); - let albums: Vec = app.session.catalog.albums().cloned().collect(); - albums_tree(app, ui, &albums, None, 0.0); - ui.add_space(10.0); - local_section(app, ui); - // By date - let (dr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); - ui.painter().text(pos2(dr.left() + 18.0, dr.center().y), Align2::LEFT_CENTER, "By Date", t.semibold(13.5), t.text_label); - for g in app.caches.date_groups(&app.session.catalog).iter() { - // year → month → day; a click filters by that prefix, the triangle opens a level - if date_row(app, ui, &g.year, &g.year, g.count, 0.0) { - for (m, n) in &g.months { - let label = lightcraft_catalog::dates::group_label(m).split(' ').next().unwrap_or(m).to_string(); - if date_row(app, ui, m, &label, *n, 16.0) { - for (d, n) in g.days.iter().filter(|(d, _)| d.starts_with(m.as_str())) { - let label = lightcraft_catalog::dates::group_label(d); - // "Sunday, 20 September 2026" → "Sunday, 20" - let label = label.rsplitn(3, ' ').nth(2).unwrap_or(&label).to_string(); - date_row(app, ui, d, &label, *n, 32.0); - } + if ui.button("Create Smart Album…").clicked() { + app.ui.dialog = Some(crate::state::Dialog::SmartRules { + id: None, + name: String::new(), + rules: lightcraft_catalog::RuleSet { rules: vec![crate::panels::rules_editor::new_rule()], ..Default::default() }, + }); + } + if ui.button("Create Smart Album from Filter…").clicked() { + app.ui.dialog = Some(crate::state::Dialog::NewSmartAlbum { name: String::new() }); + } + if ui.button("Create Folder…").clicked() { + app.ui.dialog = Some(crate::state::Dialog::NewAlbum { name: String::new(), folder: true }); + } + }); + let albums: Vec = app.session.catalog.albums().cloned().collect(); + albums_tree(app, ui, &albums, None, 0.0); + ui.add_space(10.0); + local_section(app, ui); + // By date + let (dr, _) = ui.allocate_exact_size(vec2(ui.available_width(), 34.0), Sense::hover()); + ui.painter().text(pos2(dr.left() + 18.0, dr.center().y), Align2::LEFT_CENTER, "By Date", t.semibold(13.5), t.text_label); + for g in app.caches.date_groups(&app.session.catalog).iter() { + // year → month → day; a click filters by that prefix, the triangle opens a level + if date_row(app, ui, &g.year, &g.year, g.count, 0.0) { + for (m, n) in &g.months { + let label = lightcraft_catalog::dates::group_label(m).split(' ').next().unwrap_or(m).to_string(); + if date_row(app, ui, m, &label, *n, 16.0) { + for (d, n) in g.days.iter().filter(|(d, _)| d.starts_with(m.as_str())) { + let label = lightcraft_catalog::dates::group_label(d); + // "Sunday, 20 September 2026" → "Sunday, 20" + let label = label.rsplitn(3, ' ').nth(2).unwrap_or(&label).to_string(); + date_row(app, ui, d, &label, *n, 32.0); } } } } - keywords_section(app, ui); - ui.add_space(10.0); - if row(app, ui, "recentlyDeleted", Icon::Trash, "Recently Deleted", Some(deleted), src == LibrarySource::RecentlyDeleted, 0.0) - .clicked() - { - let _ = app.run("library.source", json!({"kind": "recentlyDeleted"})); - } - }); + } + keywords_section(app, ui); + ui.add_space(10.0); + if row(app, ui, "recentlyDeleted", Icon::Trash, "Recently Deleted", Some(deleted), src == LibrarySource::RecentlyDeleted, 0.0).clicked() { + let _ = app.run("library.source", json!({"kind": "recentlyDeleted"})); + } }); + }); + if let Some(w) = resized { + app.ui.left_width = w; + } } /// How many library photos have no file. Checking stats every file, which on a network share diff --git a/crates/ui-egui/src/panels/mod.rs b/crates/ui-egui/src/panels/mod.rs index 06f1bf90..726dc599 100644 --- a/crates/ui-egui/src/panels/mod.rs +++ b/crates/ui-egui/src/panels/mod.rs @@ -25,6 +25,38 @@ use egui::{Align2, Rect, pos2, vec2}; use crate::LightcraftApp; use crate::theme::Tokens; +/// Show a side panel the user resizes by dragging its inner edge. `width` (in [`crate::UiState`], +/// so it survives navigation and restarts) is the source of truth: egui's own remembered size is +/// dropped every frame, and a drag of the edge returns the new width to store. The panel never +/// takes more than `available − reserve − MIN_PHOTO_WIDTH` (its minimum permitting), so the photo +/// stays usable; a narrower window shrinks it without forgetting the chosen width. +pub fn resizable_side( + ui: &mut egui::Ui, + left: bool, + id: &str, + frame: egui::Frame, + width: f32, + limits: crate::state::PanelWidth, + reserve: f32, + add: impl FnOnce(&mut egui::Ui), +) -> Option { + let pid = egui::Id::new(id); + ui.ctx().data_mut(|d| d.remove::(pid)); + let avail = ui.available_rect_before_wrap(); + let max = (avail.width() - reserve - crate::state::MIN_PHOTO_WIDTH).clamp(limits.min, limits.max); + let panel = if left { egui::Panel::left(pid) } else { egui::Panel::right(pid) }; + let r = panel.frame(frame).resizable(true).size_range(limits.min..=max).default_size(limits.clamp(width).min(max)).show(ui, add); + crate::widgets::register(ui.ctx(), format!("panel:{id}"), r.response.rect); + // egui names the edge's drag widget `.with("__resize")`; the width is measured from + // the panel's fixed edge to the pointer (contents wider than the panel don't count) + if !ui.ctx().is_being_dragged(pid.with("__resize")) { + return None; + } + let x = ui.ctx().pointer_interact_pos()?.x; + let w = if left { x - avail.left() } else { avail.right() - x }; + Some(w.round().clamp(limits.min, max)) +} + /// The HUD toast at the bottom centre of the canvas. pub fn toast(app: &mut LightcraftApp, ctx: &egui::Context) { let now = ctx.input(|i| i.time); diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index ab31da25..b8aedb96 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -13,33 +13,36 @@ use crate::widgets::{divider, register, slider, text_button}; pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); - egui::Panel::right("right_panel") - .exact_size(t.panel_w) - .resizable(false) - .frame(egui::Frame::NONE.fill(t.chrome).stroke(egui::Stroke::new(1.0, t.divider))) - .show(ui, |ui| { - let Some(id) = app.session.active() else { - let r = ui.max_rect(); - super::empty_message(ui, r, "No photo selected", "Select a photo to edit"); - return; - }; - egui::ScrollArea::vertical().id_salt("right-scroll").auto_shrink([false, false]).show(ui, |ui| { - ui.spacing_mut().item_spacing.y = 0.0; - match app.ui.right { - RightPanel::Edit => super::edit::show(app, ui, id), - RightPanel::Profiles => super::profiles::show(app, ui, id), - RightPanel::Crop => crop(app, ui, id), - RightPanel::Remove => remove(app, ui, id), - RightPanel::Masking => super::masking::show(app, ui, id), - RightPanel::RedEye => red_eye(app, ui, id), - RightPanel::Info => info(app, ui, id), - RightPanel::Keywords => keywords(app, ui, id), - RightPanel::Versions => versions(app, ui, id), - RightPanel::Activity => activity(app, ui, id), - RightPanel::None => {} - } - }); + let frame = egui::Frame::NONE.fill(t.chrome).stroke(egui::Stroke::new(1.0, t.divider)); + // the presets column and the left sidebar are laid out after this panel: leave them their room + let reserve = if app.ui.presets { t.panel_w } else { 0.0 } + if app.ui.left_panel { crate::state::LEFT_WIDTH.min } else { 0.0 }; + let width = app.ui.right_width; + let resized = super::resizable_side(ui, false, "right_panel", frame, width, crate::state::RIGHT_WIDTH, reserve, |ui| { + let Some(id) = app.session.active() else { + let r = ui.max_rect(); + super::empty_message(ui, r, "No photo selected", "Select a photo to edit"); + return; + }; + egui::ScrollArea::vertical().id_salt("right-scroll").auto_shrink([false, false]).show(ui, |ui| { + ui.spacing_mut().item_spacing.y = 0.0; + match app.ui.right { + RightPanel::Edit => super::edit::show(app, ui, id), + RightPanel::Profiles => super::profiles::show(app, ui, id), + RightPanel::Crop => crop(app, ui, id), + RightPanel::Remove => remove(app, ui, id), + RightPanel::Masking => super::masking::show(app, ui, id), + RightPanel::RedEye => red_eye(app, ui, id), + RightPanel::Info => info(app, ui, id), + RightPanel::Keywords => keywords(app, ui, id), + RightPanel::Versions => versions(app, ui, id), + RightPanel::Activity => activity(app, ui, id), + RightPanel::None => {} + } }); + }); + if let Some(w) = resized { + app.ui.right_width = w; + } } pub fn header(ui: &mut egui::Ui, title: &str) { diff --git a/crates/ui-egui/src/state.rs b/crates/ui-egui/src/state.rs index a9749f89..116c6c6b 100644 --- a/crates/ui-egui/src/state.rs +++ b/crates/ui-egui/src/state.rs @@ -175,6 +175,28 @@ pub const PREVIEW_EDGES: [u32; 4] = [1600, 2560, 3840, 5120]; /// Click-zoom ratios offered (percent): 1:1, 2:1, 3:1, 4:1, 8:1. pub const CLICK_ZOOMS: [u32; 5] = [100, 200, 300, 400, 800]; +/// Width limits of a side panel the user resizes (points). +#[derive(Clone, Copy, Debug, PartialEq)] +pub struct PanelWidth { + pub min: f32, + pub default: f32, + pub max: f32, +} + +impl PanelWidth { + /// `w` within the limits (the default when it isn't a number). + pub fn clamp(&self, w: f32) -> f32 { + if w.is_finite() { w.clamp(self.min, self.max) } else { self.default } + } +} + +/// The left sidebar (sources, albums, folders). +pub const LEFT_WIDTH: PanelWidth = PanelWidth { min: 200.0, default: 268.0, max: 480.0 }; +/// The right panel (Edit, Masking, Info, …). +pub const RIGHT_WIDTH: PanelWidth = PanelWidth { min: 250.0, default: 270.0, max: 520.0 }; +/// The photo area the side panels always leave free (as far as their minimum widths allow). +pub const MIN_PHOTO_WIDTH: f32 = 360.0; + #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] pub struct UiState { @@ -184,6 +206,10 @@ pub struct UiState { pub view: ViewMode, pub left_panel: bool, pub right: RightPanel, + /// Widths of the left sidebar and of the right panel in points (dragging their inner edge + /// resizes them; within [`LEFT_WIDTH`] / [`RIGHT_WIDTH`]). + pub left_width: f32, + pub right_width: f32, /// Presets column open (opens to the left of the Edit panel). pub presets: bool, /// Presets column: show a live thumbnail of the photo with each preset. @@ -477,6 +503,8 @@ impl Default for UiState { luminance_map_restore: None, view: ViewMode::Detail, left_panel: false, + left_width: LEFT_WIDTH.default, + right_width: RIGHT_WIDTH.default, right: RightPanel::Edit, presets: false, preset_thumbs: false, @@ -580,6 +608,8 @@ impl UiState { /// Clamp values restored from disk. pub fn sanitized(mut self) -> Self { self.thumb_size = self.thumb_size.clamp(90.0, 480.0); + self.left_width = LEFT_WIDTH.clamp(self.left_width); + self.right_width = RIGHT_WIDTH.clamp(self.right_width); self.brush_size = self.brush_size.clamp(0.002, 0.5); self.dialog = None; self.fullscreen = false; diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs new file mode 100644 index 00000000..2f6873ed --- /dev/null +++ b/crates/ui-egui/src/tests_panels.rs @@ -0,0 +1,95 @@ +//! Headless tests of the side panels: dragging their inner edge resizes them within limits, and +//! the chosen widths survive switching panels and a save/load of the UI state (issue #20). + +use std::time::Duration; + +use serde_json::json; + +use crate::headless::Headless; +use crate::state::{LEFT_WIDTH, MIN_PHOTO_WIDTH, RIGHT_WIDTH}; +use crate::{LightcraftApp, Services}; + +const T: Duration = Duration::from_secs(20); +const SETTLE: Duration = Duration::from_secs(120); + +fn demo(size: [f32; 2], ui: serde_json::Value) -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, size, 1.0); + let r = h.request("ui.set", ui, T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + h +} + +fn widget(h: &Headless, id: &str) -> egui::Rect { + h.app.widgets.iter().find(|(w, _)| w == id).map(|(_, r)| *r).unwrap_or_else(|| panic!("no widget {id}")) +} + +/// Drag horizontally from `x` by `dx` (within the 1400 pt window) at mid height. +fn drag(h: &mut Headless, x: f32, dx: f32) { + let to = (x + dx).clamp(1.0, 1399.0); + let r = h.request("ui.drag", json!({"x": x, "y": 500.0, "toX": to, "toY": 500.0, "steps": 12}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); +} + +#[test] +fn side_panels_resize_by_their_inner_edge_and_remember_it() { + let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "edit", "leftPanel": true})); + let right = widget(&h, "panel:right_panel"); + let left = widget(&h, "panel:left_panel"); + assert_eq!((left.width(), right.width()), (LEFT_WIDTH.default, RIGHT_WIDTH.default)); + // the right panel's left edge: 120 pt wider + drag(&mut h, right.left() + 2.0, -120.0); + assert!((h.app.ui.right_width - (RIGHT_WIDTH.default + 120.0)).abs() <= 2.0, "{}", h.app.ui.right_width); + assert!((widget(&h, "panel:right_panel").width() - h.app.ui.right_width).abs() <= 1.0); + // the left sidebar's right edge: 70 pt wider + drag(&mut h, left.right() - 2.0, 70.0); + assert!((h.app.ui.left_width - (LEFT_WIDTH.default + 70.0)).abs() <= 2.0, "{}", h.app.ui.left_width); + // kept across panel switches and view changes + let (lw, rw) = (h.app.ui.left_width, h.app.ui.right_width); + for set in [json!({"right": "info"}), json!({"view": "photoGrid"}), json!({"right": "masking", "view": "detail"})] { + h.request("ui.set", set, T); + h.step(); + assert_eq!((widget(&h, "panel:left_panel").width(), widget(&h, "panel:right_panel").width()), (lw, rw)); + } + // and across a save/load of the UI state (ui.json) + let saved = serde_json::to_string(&h.app.ui).unwrap(); + let back = serde_json::from_str::(&saved).unwrap().sanitized(); + assert_eq!((back.left_width, back.right_width), (lw, rw)); + // limits: the photo area stays usable however far the edges are dragged… + let right = widget(&h, "panel:right_panel"); + drag(&mut h, right.left() + 2.0, -1400.0); + let left = widget(&h, "panel:left_panel"); + drag(&mut h, left.right() - 2.0, 1400.0); + h.step(); + let (left, right) = (widget(&h, "panel:left_panel"), widget(&h, "panel:right_panel")); + assert!(right.width() <= RIGHT_WIDTH.max && left.width() <= LEFT_WIDTH.max, "{left:?} {right:?}"); + assert!(right.left() - left.right() >= MIN_PHOTO_WIDTH - 1.0, "photo area {} wide", right.left() - left.right()); + // …and neither panel gets narrower than its minimum + drag(&mut h, right.left() + 2.0, 1400.0); + let left = widget(&h, "panel:left_panel"); + drag(&mut h, left.right() - 2.0, -1400.0); + assert_eq!((h.app.ui.left_width, h.app.ui.right_width), (LEFT_WIDTH.min, RIGHT_WIDTH.min)); + // out-of-range saved widths are clamped on load + let mut u = crate::UiState { left_width: 5.0, right_width: f32::NAN, ..Default::default() }.sanitized(); + assert_eq!((u.left_width, u.right_width), (LEFT_WIDTH.min, RIGHT_WIDTH.default)); + u.right_width = 9000.0; + assert_eq!(u.sanitized().right_width, RIGHT_WIDTH.max); +} + +#[test] +fn a_narrow_window_shrinks_the_panels_without_forgetting_their_width() { + let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "edit", "leftPanel": true, "rightWidth": 480.0, "leftWidth": 400.0})); + assert_eq!(widget(&h, "panel:right_panel").width(), 480.0); + h.request("ui.resize", json!({"width": 1000.0, "height": 800.0}), T); + h.settle(SETTLE); + let (left, right) = (widget(&h, "panel:left_panel"), widget(&h, "panel:right_panel")); + assert!(right.left() - left.right() >= MIN_PHOTO_WIDTH - 1.0, "{left:?} {right:?}"); + // the chosen widths come back when the window does + assert_eq!((h.app.ui.left_width, h.app.ui.right_width), (400.0, 480.0)); + h.request("ui.resize", json!({"width": 1400.0, "height": 900.0}), T); + h.settle(SETTLE); + assert_eq!(widget(&h, "panel:right_panel").width(), 480.0); +} diff --git a/crates/ui-egui/src/theme.rs b/crates/ui-egui/src/theme.rs index c3f0b322..65e9c3ef 100644 --- a/crates/ui-egui/src/theme.rs +++ b/crates/ui-egui/src/theme.rs @@ -47,7 +47,6 @@ pub struct Tokens { pub bottom_bar_h: f32, pub panel_w: f32, pub strip_w: f32, - pub left_w: f32, pub slider_row_h: f32, pub section_h: f32, pub film_h: f32, @@ -88,7 +87,6 @@ impl Default for Tokens { bottom_bar_h: 48.0, panel_w: 270.0, strip_w: 48.0, - left_w: 268.0, slider_row_h: 45.0, section_h: 57.0, film_h: 142.0, diff --git a/docs/parity.md b/docs/parity.md index a33caa46..55cf2f73 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -44,13 +44,13 @@ and prints the summary; `cargo xtask parity --write` refreshes the summary table | U. Map & location (MAP) | 0 | 1 | 1 | 0 | — | 0/1 (0%) | | V. Preferences (PREF) | 5 | 0 | 3 | 3 | 1/1 (100%) | 4/4 (100%) | | W. Cloud & AI infrastructure (CLOUD / AI) | 1 | 0 | 1 | 2 | — | — | -| X. Cross-cutting behaviours (BEHAV) | 14 | 1 | 2 | 1 | 8/8 (100%) | 5/5 (100%) | +| X. Cross-cutting behaviours (BEHAV) | 15 | 1 | 2 | 1 | 8/8 (100%) | 6/6 (100%) | | Y. Menus | 81 | 0 | 4 | 8 | 47/47 (100%) | 23/23 (100%) | | Z. Keyboard shortcuts (desktop) | 74 | 3 | 3 | 1 | 49/52 (94%) | 22/23 (96%) | | Lightroom Classic extras | 29 | 9 | 41 | 9 | — | 20/21 (95%) | -| **Total** | 383 | 25 | 92 | 37 | 194/198 (98%) | 136/144 (94%) | +| **Total** | 384 | 25 | 92 | 37 | 194/198 (98%) | 137/145 (94%) | -Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **79.1%** of 500 in-scope rows — P0 99.0% of 198 · P1 96.2% of 144 · P2 38.6% of 158. +Weighted completion (✅ = 1, 🟡 = ½, 🚫 left out): **79.1%** of 501 in-scope rows — P0 99.0% of 198 · P1 96.2% of 145 · P2 38.6% of 158. ## Top gaps @@ -482,6 +482,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-BEHAV-GPU | GPU acceleration | P0 | ✅ | `crates/gpu/src/render.rs`, `cmd:app.gpu`, `docs/gpu-pipeline.md` | CPU fallback | | LR-BEHAV-DRAGDROP | Drag and drop | P1 | ✅ | `crates/ui-egui/src/lib.rs`, `crates/ui-egui/src/panels/grid.rs` (`drag_feedback`), `crates/ui-egui/src/panels/left.rs` (`drop_target`) | files → app (import); grid photos → an album row (adds the selection, with a count badge while dragging) | | LR-BEHAV-TOAST | Toast notifications | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` | | +| LR-BEHAV-PANEL-RESIZE | Resizable side panels | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` (`resizable_side`), `crates/ui-egui/src/state.rs` (`LEFT_WIDTH`, `RIGHT_WIDTH`), `crates/ui-egui/src/tests_panels.rs` | drag the left sidebar's right edge (200–480 pt) or the right panel's left edge (250–520 pt); the photo area keeps ≥ 360 pt; widths are kept across panels, views and restarts (`leftWidth` / `rightWidth` in the UI state); the Presets column stays fixed | | LR-BEHAV-EMPTY-STATES | Empty states | P1 | ✅ | `crates/ui-egui/src/panels/mod.rs` (`empty_message`) | | | LR-BEHAV-TOOLTIPS | Tooltips with shortcuts | P0 | ✅ | `crates/ui-egui/src/panels/bottombar.rs` | | | LR-BEHAV-ACCESS | Accessibility | P2 | 🟡 | `crates/ui-egui/src/widgets.rs`, `crates/ui-egui/src/panels/grid.rs` | AccessKit (VoiceOver / Narrator / AT-SPI): sliders announce control and value, buttons / icon buttons / dropdowns / section headers / sources their labels and state, grid thumbnails file name, rating, flag and label; the canvas tools (crop, masks) are pointer-only; not audited with a screen reader | From 24cf3abb623177d7ebb4b7c00ed2300d492c30e8 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:36:38 -0400 Subject: [PATCH 10/12] Issue #32: Import: custom date-based folder template for Copy library.import organize now also takes a folder template, e.g. {date:%Y}/{date:%Y%m%d} -> 2026/20260114 (Organize::Template). The template's own / (or \) make the folder levels; each level is expanded with the rename tokens and sanitized, so metadata can never add a level or climb out of the destination; absolute paths, drive letters, ~ and . / .. levels are refused. Dated by capture time, else the import time; a level whose metadata is missing becomes 'unknown'. The import dialog's Folders menu gains 'Custom template...' with a template field, tag help, live validation and an example full destination for every mode; the dialog stays open (with a toast) when the import is refused. The MCP import helper passes mode/destination/organize/rename through (docs/mcp.md). Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/library.rs | 16 +++- crates/engine/src/import.rs | 50 ++++++---- crates/engine/src/rename.rs | 60 ++++++++++++ crates/engine/src/tests_import.rs | 31 ++++++ crates/mcp/src/tests.rs | 20 ++++ crates/mcp/src/tools.rs | 15 ++- crates/ui-egui/src/control.rs | 9 +- crates/ui-egui/src/headless.rs | 53 +++++++++++ crates/ui-egui/src/import.rs | 137 +++++++++++++++++++++------ crates/ui-egui/src/panels/dialogs.rs | 7 +- docs/mcp.md | 2 +- docs/parity.md | 2 +- 12 files changed, 344 insertions(+), 58 deletions(-) diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index b0d65f31..29826e63 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -735,7 +735,7 @@ pub fn specs() -> Vec { "Add Photos…", ["File"], Some("Cmd+Shift+I"), - "{paths: [file or folder (recursive)], mode?: add|copy (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/), destination?: folder for copies, organize?: date|month|flat, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG), local?: bool (browsing: the photos stay out of the library, like library.browse), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..]} → {imported, duplicates, failed, album?}", + "{paths: [file or folder (recursive)], mode?: add|copy (add = reference the files in place; copy = into the library's Originals/YYYY/YYYY-MM-DD/), destination?: folder for copies, organize?: date (YYYY/YYYY-MM-DD) | month (YYYY/YYYY-MM) | flat | a folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → 2026/20260114 (the template's `/` make the folders, each level expanded with the rename tokens and made a safe folder name: never outside the destination; must be relative, no `..`; a level with missing metadata is `unknown`) — dated by capture time, else the import time, rename?: file-name template for copies, original extension added (tokens: {name} {num} {seq} {seq:N} {date} {date:%Y%m%d} {folder} {camera} {lens} {iso} {rating} {title} {creator} {ext}; photo.renameTokens explains each; blank = keep names), renameStart?: 1, metadataPreset?: name, dng?: bool (copy raws as DNG), local?: bool (browsing: the photos stay out of the library, like library.browse), album?: albumId, albumName?: new album, preset?: presetId, keywords?: [..]} → {imported, duplicates, failed, album?}", always, |s, p| { let paths = strs(p, "paths"); @@ -760,11 +760,19 @@ pub fn specs() -> Vec { return Err(bad("library.import", "album must be a regular album")); } let organize = match str_param(p, "organize") { - Some(o) => { - crate::import::Organize::parse(o).ok_or_else(|| bad("library.import", format!("unknown organize `{o}` (date|month|flat)")))? - } + Some(o) => crate::import::Organize::parse(o).ok_or_else(|| { + bad( + "library.import", + format!("unknown organize `{o}` (date|month|flat, or a folder template like {{date:%Y}}/{{date:%Y%m%d}})"), + ) + })?, None => Default::default(), }; + if let crate::import::Organize::Template(t) = &organize + && let Some(e) = crate::rename::folder_template_error(t) + { + return Err(bad("library.import", e)); + } let metadata_preset = str_param(p, "metadataPreset").map(str::trim).filter(|n| !n.is_empty()).map(str::to_string); if let Some(n) = &metadata_preset && !s.metadata_presets.iter().any(|m| m.name.eq_ignore_ascii_case(n)) diff --git a/crates/engine/src/import.rs b/crates/engine/src/import.rs index 5b9211d3..40970b97 100644 --- a/crates/engine/src/import.rs +++ b/crates/engine/src/import.rs @@ -66,8 +66,9 @@ pub struct ImportOptions { pub convert_dng: bool, } -/// How copies are filed in the destination. -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +/// How copies are filed in the destination. The date is the capture time, else the time of the +/// import. +#[derive(Clone, Debug, Default, PartialEq, Eq)] pub enum Organize { /// `YYYY/YYYY-MM-DD/` by capture date. #[default] @@ -76,17 +77,36 @@ pub enum Organize { ByMonth, /// All in the destination itself. Flat, + /// A folder template, e.g. `{date:%Y}/{date:%Y%m%d}` → `2026/20260114/`: the template's own + /// `/` make the levels, each expanded with the file-name tokens ([`crate::rename::expand_folder`]; + /// never outside the destination). + Template(String), } impl Organize { + /// `date` | `month` | `flat`, or a folder template (anything with a `{`, `/` or `\`). pub fn parse(s: &str) -> Option { match s { "date" | "day" | "byDay" => Some(Organize::ByDay), "month" | "byMonth" => Some(Organize::ByMonth), "flat" | "none" | "intoOneFolder" => Some(Organize::Flat), + t if t.contains(['{', '/', '\\']) => Some(Organize::Template(t.trim().to_string())), _ => None, } } + + /// The folders (inside the destination) a copy of `p` goes to. + pub fn folders(&self, p: &Photo) -> Vec { + let date = p.date(); + let day = date.get(..10).filter(|d| d.len() == 10).unwrap_or("undated"); + let year = day.get(..4).unwrap_or("undated"); + match self { + Organize::ByDay => vec![year.into(), day.into()], + Organize::ByMonth => vec![year.into(), day.get(..7).unwrap_or("undated").into()], + Organize::Flat => Vec::new(), + Organize::Template(t) => crate::rename::expand_folder(t, p, 1), + } + } } /// A file found by [`scan`], for the import review. @@ -322,16 +342,10 @@ fn probe_all(probe: Option<&crate::media::FileProbe>, paths: &[String], progress .collect() } -/// Copy `src` into `root` (filed per `organize` by `date`) as `name` (default: its own name), -/// made unique with -1, -2…; returns the new path. -fn copy_into(root: &Path, src: &str, date: &str, organize: Organize, name: Option<&str>) -> Result { - let day = date.get(..10).filter(|d| d.len() == 10).unwrap_or("undated"); - let year = day.get(..4).unwrap_or("undated"); - let dir = match organize { - Organize::ByDay => root.join(year).join(day), - Organize::ByMonth => root.join(year).join(day.get(..7).unwrap_or("undated")), - Organize::Flat => root.to_path_buf(), - }; +/// Copy `src` into `root`/`folders` as `name` (default: its own name), made unique with -1, -2…; +/// returns the new path. +fn copy_into(root: &Path, src: &str, folders: &[String], name: Option<&str>) -> Result { + let dir = folders.iter().fold(root.to_path_buf(), |d, f| d.join(f)); std::fs::create_dir_all(&dir).map_err(|e| format!("{}: {e}", dir.display()))?; let name = name .map(str::to_string) @@ -557,17 +571,17 @@ pub fn import_with(s: &mut Session, paths: &[String], opts: &ImportOptions) -> c (ImportMode::Copy, Some(root)) if !Path::new(&path).starts_with(root) && !lib_dir.as_ref().is_some_and(|l| Path::new(&path).starts_with(l)) => { + // the templates see the photo as it will be catalogued (undated: the import time) + let own = Path::new(&path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); + let mut q = Photo::new(PhotoId(0), Source::File { path: path.clone() }, &own, &info.format, 0, 0, &now); + q.captured = info.captured.clone(); + q.meta = info.meta.clone(); let name = opts.rename.as_deref().filter(|t| !t.trim().is_empty()).map(|t| { - // the template sees the photo as it will be catalogued - let own = Path::new(&path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default(); - let mut q = Photo::new(PhotoId(0), Source::File { path: path.clone() }, &own, &info.format, 0, 0, &now); - q.captured = info.captured.clone(); - q.meta = info.meta.clone(); let n = crate::rename::expand(t, &q, seq); seq += 1; n }); - match copy_into(root, &path, info.captured.as_deref().unwrap_or(&now), opts.organize, name.as_deref()) { + match copy_into(root, &path, &opts.organize.folders(&q), name.as_deref()) { Ok(p) => p, Err(e) => { report.failed.push((path, e)); diff --git a/crates/engine/src/rename.rs b/crates/engine/src/rename.rs index 0c077d27..4b956c84 100644 --- a/crates/engine/src/rename.rs +++ b/crates/engine/src/rename.rs @@ -157,6 +157,41 @@ pub fn token_example(tag: &str) -> String { expand_tokens(tag, &sample_photo(), 1, 1) } +/// Why a folder template (`{date:%Y}/{date:%Y%m%d}`) can't be used, if it can't: it must be +/// relative (no leading `/` or `\`, drive letter or `~`) and have no `.` / `..` levels. +pub fn folder_template_error(template: &str) -> Option { + let t = template.trim(); + if t.is_empty() { + return Some("the folder template is empty".into()); + } + let b = t.as_bytes(); + if t.starts_with(['/', '\\', '~']) || (b.len() >= 2 && b[0].is_ascii_alphabetic() && b[1] == b':') { + return Some("the folder template must be relative to the destination (no leading /, \\, ~ or drive)".into()); + } + if t.split(['/', '\\']).any(|s| matches!(s.trim(), "." | "..")) { + return Some("the folder template may not contain . or .. folders".into()); + } + None +} + +/// The folders (relative to the destination) a folder template gives `p`: the template's own `/` +/// (or `\`) separate levels; each level's tokens are expanded and the result made a safe folder +/// name — a value can never add a level or climb out (`/`, `\`, `:`… become `-`, leading and +/// trailing dots are dropped). A level that comes out empty (missing metadata) is `unknown`; empty +/// levels in the template (`a//b`) are skipped. `{date}` is the capture time, else [`Photo::date`]'s +/// fallback (the import time). Check [`folder_template_error`] first; levels it rejects are skipped. +pub fn expand_folder(template: &str, p: &Photo, seq: usize) -> Vec { + template + .split(['/', '\\']) + .map(str::trim) + .filter(|s| !s.is_empty() && !matches!(*s, "." | "..")) + .map(|s| { + let v = sanitize(&expand_tokens(s, p, seq, 1)); + if v.is_empty() { "unknown".to_string() } else { v } + }) + .collect() +} + /// The `{…}` tags in `template` that aren't tokens (they stay as typed), for a warning. pub fn unknown_tokens(template: &str) -> Vec { let p = sample_photo(); @@ -465,6 +500,31 @@ impl Session { mod tests { use super::*; + #[test] + fn folder_templates_stay_inside_the_destination() { + let mut p = sample_photo(); + assert_eq!(expand_folder("{date:%Y}/{date:%Y%m%d}", &p, 1), vec!["2026", "20260114"]); + assert_eq!(expand_folder("{date:%Y}\\{date:%Y-%m}", &p, 1), vec!["2026", "2026-01"], "backslash separates too"); + // metadata can't add levels or climb out + p.meta.camera = "../../etc/x".into(); + p.meta.title = "..".into(); + assert_eq!(expand_folder("{camera}/{title}/{date:%Y}", &p, 1), vec!["-..-etc-x", "unknown", "2026"]); + p.meta.camera = "C:\\Windows".into(); + assert_eq!(expand_folder("{camera}", &p, 1), vec!["C--Windows"]); + // missing metadata → `unknown`; missing capture time → the import time + p.meta.camera.clear(); + p.captured = None; + assert_eq!(expand_folder("{camera}//{date:%Y%m%d}", &p, 1), vec!["unknown", "20260120"]); + for bad in ["", " ", "/abs/{date}", "\\\\server\\x", "C:/x", "c:x", "~/x", "../{date}", "{date}/../x", "a/./b", "{date:%Y}/.."] { + assert!(folder_template_error(bad).is_some(), "{bad:?} should be rejected"); + } + for ok in ["{date:%Y}/{date:%Y%m%d}", "Trips/{camera}", "{date}", "a//b"] { + assert_eq!(folder_template_error(ok), None, "{ok:?}"); + } + // even unchecked, rejected levels are skipped rather than followed + assert_eq!(expand_folder("/../{date:%Y}/./x", &p, 1), vec!["2026", "x"]); + } + /// The help lists exactly what `expand_tokens` understands: every listed tag (and alias) is /// replaced, never left literal, and the help's examples match the documented behaviour. #[test] diff --git a/crates/engine/src/tests_import.rs b/crates/engine/src/tests_import.rs index 2a1480ba..53f9feaf 100644 --- a/crates/engine/src/tests_import.rs +++ b/crates/engine/src/tests_import.rs @@ -257,6 +257,37 @@ fn copy_with_destination_organize_rename_and_metadata_preset() { } } +/// Copy with a custom folder template: `{date:%Y}/{date:%Y%m%d}` files a photo under +/// `2026/20260114/` (an undated file by the import time); templates that would leave the +/// destination are refused. +#[test] +fn copy_with_a_folder_template() { + let src = temp_dir("tplsrc"); + let dest = temp_dir("tpldest"); + write_png(&src.join("a.png"), 1); + let mut s = Session::new().with_fs(); + s.clock = Box::new(|| "2026-01-14T05:58:48".to_string()); + for bad in ["../{date}", "/tmp/{date}", "{date:%Y}/../x", "C:/x"] { + let r = s.execute( + "library.import", + &json!({"paths": [src.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": bad}), + ); + assert!(r.is_err(), "{bad} should be refused"); + } + assert_eq!(s.catalog.len(), 0); + let r = s + .execute( + "library.import", + &json!({"paths": [src.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "{date:%Y}/{date:%Y%m%d}"}), + ) + .unwrap(); + assert_eq!(ids(&r, "imported"), 1, "{r}"); + assert!(dest.join("2026").join("20260114").join("a.png").is_file()); + for d in [&src, &dest] { + let _ = std::fs::remove_dir_all(d); + } +} + /// Auto import: files in the watched folder are added once their size held between two scans, /// into the named album; non-photos are tried once; the selection stays put. #[test] diff --git a/crates/mcp/src/tests.rs b/crates/mcp/src/tests.rs index 0a7bbc57..0bf78908 100644 --- a/crates/mcp/src/tests.rs +++ b/crates/mcp/src/tests.rs @@ -72,6 +72,26 @@ fn tools_list_has_helpers_and_every_command() { assert!(r["result"]["tools"].as_array().unwrap().iter().all(|t| !t["name"].as_str().unwrap().starts_with("cmd_"))); } +/// The `import` helper passes the copy options through: a folder template files the copy. +#[test] +fn import_tool_copies_with_a_folder_template() { + let base = std::env::temp_dir().join(format!("lc-mcp-import-tpl-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (src, dest) = (base.join("card"), base.join("out")); + std::fs::create_dir_all(&src).unwrap(); + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[20, 3, 9, 255]; 64] }; + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); + std::fs::write(src.join("a.png"), png).unwrap(); + let mut b = Headless::demo(); + b.session.clock = Box::new(|| "2026-01-14T05:58:48".to_string()); + let args = + json!({"paths": [src.to_string_lossy()], "mode": "copy", "destination": dest.to_string_lossy(), "organize": "{date:%Y}/{date:%Y%m%d}"}); + let r = call_tool(&mut b, "import", &args); + assert!(!r.is_error, "{r:?}"); + assert!(dest.join("2026").join("20260114").join("a.png").is_file()); + let _ = std::fs::remove_dir_all(&base); +} + #[test] fn command_tools_run_commands() { let mut b = Headless::demo(); diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index cbca4ed2..77f610ef 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -96,7 +96,14 @@ pub fn helper_tools(has_ui: bool) -> Vec { "import", "Import photos", "Add image/raw files to the library. Folders are scanned recursively for photos; relative paths resolve against the server's working directory. The first imported photo becomes active. Returns the new photo ids.", - json!({"paths": {"type": "array", "items": {"type": "string"}, "description": "Files and/or folders"}, "album": {"type": "integer", "description": "Also add to this album id"}}), + json!({ + "paths": {"type": "array", "items": {"type": "string"}, "description": "Files and/or folders"}, + "album": {"type": "integer", "description": "Also add to this album id"}, + "mode": {"type": "string", "enum": ["add", "copy"], "description": "add (default) = reference the files in place; copy = copy them into `destination` (default: the library's Originals/)"}, + "destination": {"type": "string", "description": "Copy: destination folder"}, + "organize": {"type": "string", "description": "Copy: folders inside the destination — date (YYYY/YYYY-MM-DD, default), month (YYYY/YYYY-MM), flat, or a folder template such as {date:%Y}/{date:%Y%m%d} (→ 2026/20260114; the template's / make the levels, tokens as for rename; relative, no ..). Dated by capture time, else the import time"}, + "rename": {"type": "string", "description": "Copy: file-name template, e.g. {date:%Y%m%d_%H%M%S}_{seq:3} (run_command photo.renameTokens lists the tags); the extension is kept"} + }), &["paths"], ), tool( @@ -454,8 +461,10 @@ pub fn call_tool(b: &mut dyn Backend, name: &str, args: &Value) -> ToolResult { return ToolResult::error(format!("no photos found in {paths:?}")); } let mut p = json!({"paths": files}); - if let Some(a) = args.get("album") { - p["album"] = a.clone(); + for k in ["album", "mode", "destination", "organize", "rename"] { + if let Some(v) = args.get(k).filter(|v| !v.is_null()) { + p[k] = v.clone(); + } } ToolResult::from(exec(b, "library.import", p).map(|mut r| { if let Some(o) = r.as_object_mut() { diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 33a7a497..33e64a04 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -268,7 +268,14 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context, req: &ControlRequest } } "ui.dialog.confirm" => match app.ui.dialog.take() { - Some(d) => wrap(crate::panels::dialogs::confirm_dialog(app, &d)), + Some(d) => { + let r = crate::panels::dialogs::confirm_dialog(app, &d); + // the import review stays open on an error, as with its button + if r.is_err() && matches!(d, crate::state::Dialog::Import { .. }) { + app.ui.dialog = Some(d); + } + wrap(r) + } None => err("no dialog open"), }, "ui.dialog.cancel" => { diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index f89e324f..cba31161 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -1211,6 +1211,59 @@ mod tests { let _ = std::fs::remove_dir_all(&base); } + /// Import review, Copy, Folders → Custom template…: the default `{date:%Y}/{date:%Y%m%d}` + /// files copies under `2026/20260114/`, the example line shows the full destination, and a + /// template that would climb out of the destination is refused. + #[test] + fn import_copy_into_a_custom_folder_template() { + let base = std::env::temp_dir().join(format!("lc-ui-import-tpl-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&base); + let (src, dest) = (base.join("card"), base.join("out")); + std::fs::create_dir_all(&src).unwrap(); + let img = lightcraft_raster::Rgba8 { width: 8, height: 8, data: vec![[40, 3, 9, 255]; 64] }; + let png = lightcraft_codecs::encode_png(&lightcraft_codecs::EncodeImage::rgba8(&img), &lightcraft_codecs::EncodeMeta::default()).unwrap(); + std::fs::write(src.join("IMG_01.png"), png).unwrap(); + let dest_s = dest.to_string_lossy().to_string(); + let services = crate::Services { png: None, pick_folder: Some(Box::new(move || Some(dest_s.clone()))), ..Default::default() }; + let mut session = lightcraft_engine::Session::with_demo().with_fs(); + // undated files are filed by the import time + session.clock = Box::new(|| "2026-01-14T05:58:48".to_string()); + let mut app = LightcraftApp::new(session, services); + app.ui.view = crate::state::ViewMode::PhotoGrid; + let mut h = Headless::new(app, [1300.0, 1000.0], 1.0); + let t = Duration::from_secs(10); + let r = h.request("engine.execute", json!({"command": "file.addPhotos", "params": {"paths": [src.to_string_lossy()]}}), t); + assert_eq!(r["result"]["scanning"], true, "{r}"); + h.settle(SETTLE); + for id in ["button:importCopy", "button:importDest"] { + let r = h.request("ui.clickWidget", json!({"id": id}), t); + assert_eq!(r["ok"], true, "{id}: {r}"); + } + if let Some(crate::state::Dialog::Import { opts }) = &mut h.app.ui.dialog { + opts.organize = "custom".into(); + opts.folder_template = "../{date:%Y}".into(); + } + h.step(); + // refused: the dialog stays open, nothing is imported + let r = h.request("ui.dialog.confirm", json!({}), t); + assert_ne!(r["ok"], true, "{r}"); + assert!(h.app.import.is_none()); + let Some(crate::state::Dialog::Import { opts }) = &mut h.app.ui.dialog else { panic!("dialog closed") }; + opts.folder_template = crate::import::DEFAULT_FOLDER_TEMPLATE.into(); + let opts = opts.clone(); + let sep = std::path::MAIN_SEPARATOR; + let example = crate::import::example_destination(&h.app, &opts).expect("example"); + assert_eq!(example, format!("{}{sep}2026{sep}20260114{sep}IMG_01.png", dest.to_string_lossy())); + h.step(); + let r = h.request("ui.widgets", json!({}), t); + assert!(r.to_string().contains("label:importExample"), "example shown"); + let r = h.request("ui.dialog.confirm", json!({}), t); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + assert!(dest.join("2026").join("20260114").join("IMG_01.png").is_file()); + let _ = std::fs::remove_dir_all(&base); + } + /// Import review, Copy: Tags beside the Rename field lists the template tags; clicking one /// inserts it at the text cursor (not appended, not replacing the template). #[test] diff --git a/crates/ui-egui/src/import.rs b/crates/ui-egui/src/import.rs index 58f6b302..72b00c84 100644 --- a/crates/ui-egui/src/import.rs +++ b/crates/ui-egui/src/import.rs @@ -1,6 +1,7 @@ //! The import review dialog (File → Add Photos…): the files found under the chosen files and //! folders as a grid of thumbnails with checkboxes (duplicates marked and unchecked), the -//! destination (add in place / copy into the library's dated `Originals/` folders), an album +//! destination (add in place / copy into the library's `Originals/` or a chosen folder, filed by day, by month, +//! into one folder or by a custom folder template, optionally renamed), an album //! (existing or new), a preset and keywords to apply. Importing runs in small batches, one per //! frame, with a progress window; the whole import is one undo step. @@ -35,8 +36,10 @@ pub struct ImportDialog { pub keywords: String, /// Copy: destination folder ("" = the library's Originals/). pub destination: String, - /// Copy: `date` (YYYY/YYYY-MM-DD), `month` or `flat`. + /// Copy: `date` (YYYY/YYYY-MM-DD), `month`, `flat` or `custom` ([`ImportDialog::folder_template`]). pub organize: String, + /// Copy, `custom`: the folder template, e.g. `{date:%Y}/{date:%Y%m%d}`. + pub folder_template: String, /// Copy: file-name template for the copies ("" = keep the names). pub rename: String, /// Metadata preset name ("" = none). @@ -61,6 +64,22 @@ impl ImportDialog { .map(|(c, _)| c.path.clone()) .collect() } + /// The `organize` param of `library.import` (`None` = the default, by day); an unusable + /// custom folder template is an error. + pub fn organize_param(&self) -> Result, String> { + match self.organize.as_str() { + "" => Ok(None), + "custom" => { + let t = self.folder_template.trim(); + if let Some(e) = lightcraft_engine::rename::folder_template_error(t) { + return Err(e); + } + // a plain folder name ("Imports") is a one-level template too + Ok(Some(if t.contains(['{', '/', '\\']) { t.to_string() } else { format!("{t}/") })) + } + o => Ok(Some(o.to_string())), + } + } fn keywords(&self) -> Vec { self.keywords.split(',').map(str::trim).filter(|k| !k.is_empty()).map(str::to_string).collect() } @@ -277,8 +296,8 @@ pub fn start(app: &mut LightcraftApp, d: &ImportDialog) -> Result if !d.destination.trim().is_empty() { params["destination"] = json!(d.destination.trim()); } - if !d.organize.is_empty() { - params["organize"] = json!(d.organize); + if let Some(o) = d.organize_param()? { + params["organize"] = json!(o); } if !d.rename.trim().is_empty() { params["rename"] = json!(d.rename.trim()); @@ -434,17 +453,53 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { } }); field(ui, "Folders", |ui| { - let opts = [("date", "By day (YYYY/YYYY-MM-DD)"), ("month", "By month (YYYY/YYYY-MM)"), ("flat", "Into one folder")]; + let opts = [ + ("date", "By day (YYYY/YYYY-MM-DD)"), + ("month", "By month (YYYY/YYYY-MM)"), + ("flat", "Into one folder"), + ("custom", "Custom template…"), + ]; let key = if d.organize.is_empty() { "date".to_string() } else { d.organize.clone() }; let cur = opts.iter().find(|o| o.0 == key).map_or(opts[0].1, |o| o.1); - egui::ComboBox::from_id_salt("import-organize").selected_text(cur).show_ui(ui, |ui| { + let combo = egui::ComboBox::from_id_salt("import-organize").selected_text(cur).show_ui(ui, |ui| { for (k, label) in opts { - if ui.selectable_label(key == k, label).clicked() { + let r = ui.selectable_label(key == k, label); + register(ui.ctx(), format!("button:importOrganize-{k}"), r.rect); + if r.clicked() { d.organize = k.to_string(); + if k == "custom" && d.folder_template.trim().is_empty() { + d.folder_template = DEFAULT_FOLDER_TEMPLATE.into(); + } } } }); + register(ui.ctx(), "combo:importOrganize", combo.response.rect); }); + if d.organize == "custom" { + let folders_id = egui::Id::new("import-folder-template"); + let tags_open = field(ui, "Template", |ui| { + ui.spacing_mut().item_spacing.x = 4.0; + let w = (ui.available_width() - 50.0).max(80.0); + let r = ui.add(egui::TextEdit::singleline(&mut d.folder_template).id(folders_id).hint_text(DEFAULT_FOLDER_TEMPLATE).desired_width(w)); + register(ui.ctx(), "field:importFolderTemplate", r.rect); + tag_toggle(ui, "importFolders") + }); + if tags_open { + tag_help(ui, "importFolders", &mut d.folder_template, folders_id); + } + let t = Tokens::get(ui.ctx()); + match lightcraft_engine::rename::folder_template_error(&d.folder_template) { + Some(e) => { + ui.label(egui::RichText::new(e).color(t.caution)); + } + None => unknown_tags_warning(ui, &d.folder_template), + } + ui.label( + egui::RichText::new("Each / starts a folder level; tags are filled in per photo (a level with missing metadata is \"unknown\").") + .color(t.text_dim) + .small(), + ); + } field(ui, "Raw files", |ui| { let r = ui.checkbox(&mut d.dng, "Copy as DNG"); register(ui.ctx(), "check:importDng", r.rect); @@ -460,28 +515,15 @@ pub fn body(app: &mut LightcraftApp, ui: &mut egui::Ui, d: &mut ImportDialog) { if tags_open { tag_help(ui, "importRename", &mut d.rename, rename_id); } - if !d.rename.trim().is_empty() - && let Some(c) = d.candidates.iter().zip(&d.checked).find(|(c, on)| **on && c.duplicate.is_none()).map(|(c, _)| c) - { - let mut q = lightcraft_catalog::Photo::new( - lightcraft_catalog::PhotoId(0), - lightcraft_catalog::Source::File { path: c.path.clone() }, - &c.name, - &c.format, - 0, - 0, - // a photo without a capture time is dated by the import (now) - &(app.session.clock)(), - ); - q.captured = c.captured.clone(); - // the probe's metadata, so {camera}, {title}… preview as they will import - if let Some(info) = app.session.import_probes.get(&c.path) { - q.meta = info.meta.clone(); - } - let example = lightcraft_engine::rename::expand(d.rename.trim(), &q, 1); - ui.label(egui::RichText::new(format!("{} → {example}", c.name)).color(Tokens::get(ui.ctx()).text_dim)); - } unknown_tags_warning(ui, &d.rename); + if let Some(example) = example_destination(app, d) { + let t = Tokens::get(ui.ctx()); + let r = ui.label(egui::RichText::new(format!("Example: {example}")).color(t.text_dim)); + register(ui.ctx(), "label:importExample", r.rect); + ui.label( + egui::RichText::new("Folders and {date} use the capture time; a photo without one uses today's date.").color(t.text_dim).small(), + ); + } } let albums: Vec<(u64, String)> = { let mut v: Vec<(u64, String)> = @@ -694,6 +736,45 @@ pub(crate) fn tag_help(ui: &mut egui::Ui, key: &str, text: &mut String, edit_id: }); } +/// The folder template the Custom choice starts with (`2026/20260114/`). +pub const DEFAULT_FOLDER_TEMPLATE: &str = "{date:%Y}/{date:%Y%m%d}"; + +/// Where the first selected photo would be copied to (destination, folders, name), for the +/// dialog's example line; `None` without a photo or with an unusable folder template. +pub fn example_destination(app: &LightcraftApp, d: &ImportDialog) -> Option { + let c = d.candidates.iter().zip(&d.checked).find(|(c, on)| **on && c.duplicate.is_none() && c.error.is_none()).map(|(c, _)| c)?; + let organize = match d.organize_param().ok()? { + Some(o) => lightcraft_engine::import::Organize::parse(&o)?, + None => Default::default(), + }; + let mut q = lightcraft_catalog::Photo::new( + lightcraft_catalog::PhotoId(0), + lightcraft_catalog::Source::File { path: c.path.clone() }, + &c.name, + &c.format, + 0, + 0, + // a photo without a capture time is dated by the import (now) + &(app.session.clock)(), + ); + q.captured = c.captured.clone(); + // the probe's metadata, so {camera}, {title}… preview as they will import + if let Some(info) = app.session.import_probes.get(&c.path) { + q.meta = info.meta.clone(); + } + let name = if d.rename.trim().is_empty() { c.name.clone() } else { lightcraft_engine::rename::expand(d.rename.trim(), &q, 1) }; + let root = if d.destination.trim().is_empty() { + app.session.library.as_ref().map_or_else(|| "Originals".to_string(), |l| l.dir.join("Originals").to_string_lossy().to_string()) + } else { + d.destination.trim().trim_end_matches(['/', '\\']).to_string() + }; + let sep = std::path::MAIN_SEPARATOR_STR; + let mut parts = vec![root]; + parts.extend(organize.folders(&q)); + parts.push(name); + Some(parts.join(sep)) +} + /// A labelled row (fixed label column). fn field(ui: &mut egui::Ui, label: &str, add: impl FnOnce(&mut egui::Ui) -> R) -> R { let t = Tokens::get(ui.ctx()); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index 1cb732f4..78651bd8 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -745,8 +745,11 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { close = true; } if confirm { - let _ = confirm_dialog(app, &dlg); - close = true; + match confirm_dialog(app, &dlg) { + // the import review stays open on an error (e.g. an unusable folder template) + Err(e) if matches!(dlg, Dialog::Import { .. }) => app.toast(ctx, e), + _ => close = true, + } } app.ui.dialog = if close { None } else { Some(dlg) }; } diff --git a/docs/mcp.md b/docs/mcp.md index 2220edfe..32c155fa 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -76,7 +76,7 @@ During development you can also point the client at `cargo run --release -p ligh |---|---| | `list_commands {filter?}` | Every command: id, label, menu, shortcut, parameter doc, enabled now | | `run_command {command, params?}` | Run any command by id | -| `import {paths, album?}` | Import files/folders (folders are scanned recursively); the first new photo becomes active | +| `import {paths, album?, mode?, destination?, organize?, rename?}` | Import files/folders (folders are scanned recursively); the first new photo becomes active. `mode: "copy"` copies into `destination` (default: the library's Originals/), filed by `organize`: `date` (YYYY/YYYY-MM-DD), `month`, `flat` or a folder template such as `{date:%Y}/{date:%Y%m%d}` (→ `2026/20260114`; capture date, else the import date; always inside the destination), and named by the `rename` template (`run_command photo.renameTokens` lists the tags) | | `query_photos {filter?, sort?, offset?, limit?}` | Photos in the current view (or matching a catalog `Filter`) | | `select_photos {ids, active?, mode?}` | Set the selection / active photo | | `list_controls {section?}` | Every develop slider: id (`light.exposure`…), range, default, current value | diff --git a/docs/parity.md b/docs/parity.md index c718a5aa..9422b194 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -686,7 +686,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LRC-LIB-IMPORT | Full import dialog | P1 | ✅ | `cmd:library.import` (`mode`, `destination`, `organize`, `rename`, `metadataPreset`, `preset`, `keywords`, `album`), `crates/ui-egui/src/import.rs` | review grid with duplicates skipped; add in place or copy (library Originals or any folder; by day / by month / one folder; rename template with live example, Tags picker (insert at cursor) and unknown-tag warning, numbered across the import); develop preset, metadata preset, keywords, album on import | +| LRC-LIB-IMPORT | Full import dialog | P1 | ✅ | `cmd:library.import` (`mode`, `destination`, `organize`, `rename`, `metadataPreset`, `preset`, `keywords`, `album`), `crates/ui-egui/src/import.rs` | review grid with duplicates skipped; add in place or copy (library Originals or any folder; by day / by month / one folder / custom folder template such as `{date:%Y}/{date:%Y%m%d}` → 2026/20260114, kept inside the destination; example destination shown; rename template with live example, Tags picker (insert at cursor) and unknown-tag warning, numbered across the import); develop preset, metadata preset, keywords, album on import | | LRC-LIB-AUTOIMPORT | Watched-folder import | P2 | ✅ | `cmd:library.autoImport` | see LR-IMP-AUTO | | LRC-LIB-TETHER | Tethered capture | P2 | ⬜ | | | | LRC-LIB-VIEWS | Grid / loupe / compare / survey / people | P1 | 🟡 | `cmd:view.photoGrid`, `cmd:view.squareGrid`, `cmd:view.detail`, `cmd:view.compare`, `cmd:view.survey`, `cmd:view.gridInfo` | grid, square grid, loupe, compare, survey, second window; square-grid captions: file name, exposure or capture date (View ▸ Grid Info); no people view | From 0ad313d445fdeea88f72aa3a5cc9dadeef2f87a4 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:41:35 -0400 Subject: [PATCH 11/12] Issue #47: Masking: keep the panel's contents within its width when a mask is selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting a mask added the Add/Subtract/Intersect/Invert/Delete row in one non-wrapping horizontal layout (~273 pt in a ~220 pt column), and the Create New Mask grid needed 226 pt: the panel's contents grew wider than the panel (a 250 pt panel came out 275 pt wide) and were drawn shifted left, clipped by the photo area. The action row now wraps, the tile grid sizes its tiles and columns to the available width (four tiles of 48–52 pt, else three), and a component's name is truncated before its options button. A headless test checks that every widget stays inside the right panel at 250, 270, 330 and 520 pt, with and without a selected mask and with the brush settings open. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/panels/masking.rs | 31 +++++++++++++--- crates/ui-egui/src/tests_panels.rs | 55 ++++++++++++++++++++++++++++ docs/parity.md | 2 +- 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/panels/masking.rs b/crates/ui-egui/src/panels/masking.rs index 0f501f9f..ab494432 100644 --- a/crates/ui-egui/src/panels/masking.rs +++ b/crates/ui-egui/src/panels/masking.rs @@ -58,6 +58,16 @@ fn kind_label(s: &MaskShape) -> (&'static str, Icon) { } } +const TILE: f32 = 52.0; +const TILE_GAP: f32 = 6.0; + +/// Columns and tile width of the Create New Mask grid in `width`: four tiles of up to 52 pt (at +/// least 48, so the labels fit), else three (narrowed if even those don't fit). +fn tile_layout(width: f32) -> (usize, f32) { + let tile = |n: f32| ((width - (n - 1.0) * TILE_GAP) / n).floor().min(TILE); + if tile(4.0) >= 48.0 { (4, tile(4.0)) } else { (3, tile(3.0).max(24.0)) } +} + pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { let t = Tokens::get(ui.ctx()); let d = app.session.develop_of(id).unwrap_or_default(); @@ -75,9 +85,11 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { ("luminanceRange", "Luminance", Icon::Sliders), ("colorRange", "Color", Icon::Picker), ]; - egui::Grid::new("mask-tiles").spacing(vec2(6.0, 6.0)).show(ui, |ui| { + // four 52 pt tiles a row when they fit, else as many as fit (at least three, shrunk) + let (cols, tile) = tile_layout(ui.available_width()); + egui::Grid::new("mask-tiles").spacing(vec2(TILE_GAP, TILE_GAP)).show(ui, |ui| { for (i, (kind, label, icon)) in tiles.iter().enumerate() { - let (r, resp) = ui.allocate_exact_size(vec2(52.0, 52.0), Sense::click()); + let (r, resp) = ui.allocate_exact_size(vec2(tile, 52.0), Sense::click()); register(ui.ctx(), format!("maskNew:{kind}"), r); ui.painter().rect_filled(r, 4.0, if resp.hovered() { t.hover } else { t.inset }); paint(ui.painter(), Rect::from_center_size(r.center() - vec2(0.0, 7.0), vec2(20.0, 20.0)), *icon, t.text_label); @@ -101,7 +113,7 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } } } - if i % 4 == 3 { + if i % cols == cols - 1 { ui.end_row(); } } @@ -228,7 +240,14 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { } return; } - let resp = ui.add(egui::Label::new(format!("{op}{label}{}", if c.invert { " (inverted)" } else { "" })).sense(Sense::click())); + // a long name is cut short (with …) before the options button, not past the panel + let text = format!("{op}{label}{}", if c.invert { " (inverted)" } else { "" }); + let room = (ui.available_width() - 30.0).max(0.0); + let resp = ui.scope(|ui| { + ui.set_max_width(room); + ui.add(egui::Label::new(text).truncate().sense(Sense::click())) + }); + let resp = resp.inner; register(ui.ctx(), format!("component:{i}"), resp.rect); if resp.double_clicked() { app.ui.renaming_component = Some((m.id, i, label.clone())); @@ -243,7 +262,9 @@ pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui, id: PhotoId) { range_controls(app, ui, i, &c.shape); } ui.add_space(6.0); - ui.horizontal(|ui| { + // wraps: the five actions are wider than a narrow panel + ui.horizontal_wrapped(|ui| { + ui.spacing_mut().item_spacing.y = 6.0; let add = text_button(ui, "maskAddComp", "Add", false); egui::Popup::menu(&add).show(|ui| component_menu(app, ui, "add")); let sub = text_button(ui, "maskSubComp", "Subtract", false); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 2f6873ed..7de05e0b 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -79,6 +79,61 @@ fn side_panels_resize_by_their_inner_edge_and_remember_it() { assert_eq!(u.sanitized().right_width, RIGHT_WIDTH.max); } +/// Every widget drawn over the right panel lies within it (issue #47: selecting a mask added a +/// row wider than the panel, which pushed the panel's contents left, clipping them). +fn assert_inside_right_panel(h: &Headless, what: &str) { + let panel = widget(h, "panel:right_panel"); + let mut n = 0; + for (id, r) in &h.app.widgets { + // widgets of the photo area (pins, filmstrip cells clipped at its edge) and of the tool + // strip are outside on purpose + if ["panel:", "film:", "maskPin"].iter().any(|p| id.starts_with(p)) + || r.bottom() <= panel.top() + || r.right() <= panel.left() + 1.5 + || r.left() >= panel.right() - 1.5 + { + continue; + } + n += 1; + assert!(r.left() >= panel.left() - 0.5 && r.right() <= panel.right() + 0.5, "{what}: {id} {r:?} sticks out of the panel {panel:?}"); + } + assert!(n > 10, "{what}: only {n} widgets in the panel"); +} + +#[test] +fn masking_contents_fit_the_right_panel_at_any_width() { + let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "masking"})); + for (kind, op) in [("subject", None), ("linear", None), ("luminanceRange", Some("subtract")), ("radial", Some("intersect"))] { + let (command, params) = match op { + None => ("mask.add", json!({"kind": kind})), + Some(op) => ("mask.addComponent", json!({"kind": kind, "op": op})), + }; + let r = h.request("engine.execute", json!({"command": command, "params": params}), T); + assert_eq!(r["ok"], true, "{r}"); + } + let r = h.request( + "engine.execute", + json!({"command": "mask.component", "params": {"component": 0, "action": "rename", "name": "A rather long component name for this mask"}}), + T, + ); + assert_eq!(r["ok"], true, "{r}"); + for width in [RIGHT_WIDTH.min, RIGHT_WIDTH.default, 330.0, RIGHT_WIDTH.max] { + for (selected, tool) in [(false, ""), (true, ""), (true, "brush")] { + let mask = if selected { json!(2) } else { serde_json::Value::Null }; + h.request("engine.execute", json!({"command": "mask.select", "params": {"id": mask}}), T); + h.request("ui.set", json!({"rightWidth": width, "tool": tool}), T); + h.step(); + h.step(); + let what = format!("width {width}, mask selected {selected}, tool {tool:?}"); + assert_eq!(widget(&h, "panel:right_panel").width(), width, "{what}"); + if selected { + assert!(h.app.widgets.iter().any(|(id, _)| id == "button:maskInvert"), "{what}: no mask actions"); + } + assert_inside_right_panel(&h, &what); + } + } +} + #[test] fn a_narrow_window_shrinks_the_panels_without_forgetting_their_width() { let mut h = demo([1400.0, 900.0], json!({"view": "detail", "right": "edit", "leftPanel": true, "rightWidth": 480.0, "leftWidth": 400.0})); diff --git a/docs/parity.md b/docs/parity.md index 55cf2f73..3699bea8 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -309,7 +309,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | Id | Feature | Tier | Status | Evidence | Missing / notes | |---|---|---|---|---|---| -| LR-MASK-PANEL | Masks panel | P0 | ✅ | `cmd:panel.masking`, `cmd:mask.add`, `cmd:mask.select`, `cmd:mask.rename`, `cmd:mask.duplicate` (`invert`), `cmd:mask.move`, `cmd:mask.visible`, `cmd:mask.delete`, `crates/ui-egui/src/panels/masking.rs` | list with per-mask show/hide eye, double-click rename, right-click menu (duplicate, duplicate and invert, invert, hide, move up/down, rename, delete); no drag-to-reorder | +| LR-MASK-PANEL | Masks panel | P0 | ✅ | `cmd:panel.masking`, `cmd:mask.add`, `cmd:mask.select`, `cmd:mask.rename`, `cmd:mask.duplicate` (`invert`), `cmd:mask.move`, `cmd:mask.visible`, `cmd:mask.delete`, `crates/ui-egui/src/panels/masking.rs` | list with per-mask show/hide eye, double-click rename, right-click menu (duplicate, duplicate and invert, invert, hide, move up/down, rename, delete); fits any panel width (create tiles go to three columns, mask actions wrap, long component names are cut short); no drag-to-reorder | | LR-MASK-SUBJECT | Select subject | P2 | 🟡 | `cmd:mask.add` (`subject`), `crates/pipeline/src/masks.rs` | saliency heuristic, no segmentation model | | LR-MASK-SKY | Select sky | P2 | 🟡 | `cmd:mask.add` (`sky`) | heuristic | | LR-MASK-BACKGROUND | Select background | P2 | 🟡 | `cmd:mask.add` (`background`) | inverse of the subject heuristic | From a8c3f01e1edf7ce5e76a171f7605c3767aa40d77 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:53:43 -0400 Subject: [PATCH 12/12] CI: clean under clippy 1.99 (as_chunks for constant chunk sizes, ? in export resize parsing) Rust 1.99 (stable since 2026-09-28) adds clippy::chunks_exact_to_as_chunks and widens clippy::question_mark, which failed `cargo xtask ci` on main. Same behaviour: the remainder that chunks_exact skipped is the remainder as_chunks returns separately. Co-Authored-By: Claude Opus 5.5 --- crates/codecs/src/convert.rs | 4 +++- crates/codecs/src/encode.rs | 4 ++-- crates/codecs/src/icc.rs | 2 +- crates/codecs/src/jpeg.rs | 4 ++-- crates/codecs/src/png_codec.rs | 2 +- crates/engine/src/export.rs | 9 ++++----- crates/pipeline/src/output.rs | 4 ++-- crates/preview/src/disk.rs | 2 +- crates/raster/src/lib.rs | 2 +- crates/raw/src/demosaic/mod.rs | 2 +- crates/raw/src/opcodes.rs | 2 +- crates/raw/src/unpack.rs | 2 +- crates/raw/src/vendor/raf.rs | 2 +- crates/tiff/src/reader.rs | 26 ++++++++++++++------------ crates/ui-egui/src/softpaint.rs | 2 +- 15 files changed, 36 insertions(+), 33 deletions(-) diff --git a/crates/codecs/src/convert.rs b/crates/codecs/src/convert.rs index 93023486..02652db5 100644 --- a/crates/codecs/src/convert.rs +++ b/crates/codecs/src/convert.rs @@ -323,7 +323,9 @@ fn cmyk(raw: &Raw, ch: usize, icc_bytes: Option<&[u8]>, info: Option<&icc::IccIn } // Naive: RGB = (1 - C)(1 - K) … treated as sRGB-encoded. let px = samples - .chunks_exact(4) + .as_chunks::<4>() + .0 + .iter() .map(|c| { let k = 1.0 - c[3]; [(1.0 - c[0]) * k, (1.0 - c[1]) * k, (1.0 - c[2]) * k].map(lightcraft_color::transfer::srgb_to_linear) diff --git a/crates/codecs/src/encode.rs b/crates/codecs/src/encode.rs index 0e56dfe9..59622073 100644 --- a/crates/codecs/src/encode.rs +++ b/crates/codecs/src/encode.rs @@ -91,7 +91,7 @@ pub fn encode_jpeg(img: &EncodeImage, quality: u8, subsampling: ChromaSubsamplin let n = img.width as usize * img.height as usize; let (data, ct): (std::borrow::Cow<[u8]>, jpeg_encoder::ColorType) = match img.channels { 1 => (data[..n].into(), jpeg_encoder::ColorType::Luma), - 2 => (data.chunks_exact(2).take(n).map(|c| c[0]).collect::>().into(), jpeg_encoder::ColorType::Luma), + 2 => (data.as_chunks::<2>().0.iter().take(n).map(|c| c[0]).collect::>().into(), jpeg_encoder::ColorType::Luma), 3 => (data[..n * 3].into(), jpeg_encoder::ColorType::Rgb), _ => (data[..n * 4].into(), jpeg_encoder::ColorType::Rgba), }; @@ -254,7 +254,7 @@ pub fn encode_tiff(img: &EncodeImage, compression: TiffCompression, meta: &Encod }}; } fn expand_ga(s: &[T], n: usize) -> Vec { - s.chunks_exact(2).take(n).flat_map(|c| [c[0], c[0], c[0], c[1]]).collect() + s.as_chunks::<2>().0.iter().take(n).flat_map(|c| [c[0], c[0], c[0], c[1]]).collect() } match img.samples { Samples::U8(s) => match ch { diff --git a/crates/codecs/src/icc.rs b/crates/codecs/src/icc.rs index 7850e61b..65112259 100644 --- a/crates/codecs/src/icc.rs +++ b/crates/codecs/src/icc.rs @@ -236,7 +236,7 @@ pub(crate) fn cms_to_linear_rec2020(icc: &[u8], samples: &[f32], channels: usize for (s, d) in src_chunks.zip(dst_chunks) { t.transform(s, d).ok()?; } - Some(out.chunks_exact(3).map(|c| [c[0], c[1], c[2]]).collect()) + Some(out.as_chunks::<3>().0.iter().map(|c| [c[0], c[1], c[2]]).collect()) }; std::panic::catch_unwind(run).ok().flatten() } diff --git a/crates/codecs/src/jpeg.rs b/crates/codecs/src/jpeg.rs index 988065eb..0096f905 100644 --- a/crates/codecs/src/jpeg.rs +++ b/crates/codecs/src/jpeg.rs @@ -159,7 +159,7 @@ fn decode_jpeg_decoder(bytes: &[u8], m: &Markers, scale_to: Option<(u32, u32)>) let (model, buf, depth) = match info.pixel_format { PixelFormat::L8 => (Model::Gray, Buf::U8(px), 8), PixelFormat::L16 => { - let v = px.chunks_exact(2).map(|c| u16::from_be_bytes([c[0], c[1]])).collect(); + let v = px.as_chunks::<2>().0.iter().map(|c| u16::from_be_bytes([c[0], c[1]])).collect(); (Model::Gray, Buf::U16(v), 16) } PixelFormat::RGB24 => (Model::Rgb, Buf::U8(px), 8), @@ -226,7 +226,7 @@ fn mpf_images(m: &Markers) -> Vec<(usize, usize)> { let Some(e) = entries.iter().find(|e| e.tag == 0xB002) else { return vec![] }; let Some(list) = t.bytes(e) else { return vec![] }; let mut out = Vec::new(); - for (i, rec) in list.chunks_exact(16).enumerate().take(16) { + for (i, rec) in list.as_chunks::<16>().0.iter().enumerate().take(16) { if i == 0 { continue; } diff --git a/crates/codecs/src/png_codec.rs b/crates/codecs/src/png_codec.rs index a6704495..529d120d 100644 --- a/crates/codecs/src/png_codec.rs +++ b/crates/codecs/src/png_codec.rs @@ -37,7 +37,7 @@ pub(crate) fn decode(bytes: &[u8], opts: &DecodeOptions) -> Result { }; buf.truncate(out.buffer_size()); let (samples, depth) = match out.bit_depth { - png::BitDepth::Sixteen => (Buf::U16(buf.chunks_exact(2).map(|c| u16::from_be_bytes([c[0], c[1]])).collect()), 16), + png::BitDepth::Sixteen => (Buf::U16(buf.as_chunks::<2>().0.iter().map(|c| u16::from_be_bytes([c[0], c[1]])).collect()), 16), _ => (Buf::U8(buf), info.bit_depth as u8), }; let raw = Raw { width: out.width as usize, height: out.height as usize, model, alpha, premultiplied: false, buf: samples, bit_depth: depth }; diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index 388e0b8b..ed9490db 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -553,10 +553,9 @@ impl ExportOptions { r(ResizeMode::ShortEdge, v) } else if let Some(v) = f("megapixels") { r(ResizeMode::Megapixels, v) - } else if let Some(v) = f("percent") { - r(ResizeMode::Percent, v) } else { - return None; + let v = f("percent")?; + r(ResizeMode::Percent, v) } } }) @@ -659,7 +658,7 @@ pub fn output_sharpen_deep(img: &mut DeepImage, target: SharpenFor, amount: Shar let (w, h) = (img.width, img.height); match &mut img.samples { DeepSamples::U16(v) => { - let src: Vec<[f32; 3]> = v.chunks_exact(3).map(|c| [c[0] as f32, c[1] as f32, c[2] as f32]).collect(); + let src: Vec<[f32; 3]> = v.as_chunks::<3>().0.iter().map(|c| [c[0] as f32, c[1] as f32, c[2] as f32]).collect(); if let Some(out) = unsharp(w, h, &src, target, amount) { for (d, s) in v.iter_mut().zip(out.as_flattened()) { *d = s.round().clamp(0.0, 65535.0) as u16; @@ -667,7 +666,7 @@ pub fn output_sharpen_deep(img: &mut DeepImage, target: SharpenFor, amount: Shar } } DeepSamples::F32(v) => { - let src: Vec<[f32; 3]> = v.chunks_exact(3).map(|c| [c[0], c[1], c[2]]).collect(); + let src: Vec<[f32; 3]> = v.as_chunks::<3>().0.iter().map(|c| [c[0], c[1], c[2]]).collect(); if let Some(out) = unsharp(w, h, &src, target, amount) { for (d, s) in v.iter_mut().zip(out.as_flattened()) { *d = s.max(0.0); diff --git a/crates/pipeline/src/output.rs b/crates/pipeline/src/output.rs index 6f2e353c..5272b54d 100644 --- a/crates/pipeline/src/output.rs +++ b/crates/pipeline/src/output.rs @@ -217,12 +217,12 @@ impl DeepImage { let trc = self.space.trc(); match &self.samples { DeepSamples::U16(v) => { - for (p, c) in out.data.iter_mut().zip(v.chunks_exact(3)) { + for (p, c) in out.data.iter_mut().zip(v.as_chunks::<3>().0) { *p = [c[0], c[1], c[2]].map(|x| ((x as u32 * 255 + 32767) / 65535) as u8).into_rgba(); } } DeepSamples::F32(v) => { - for (p, c) in out.data.iter_mut().zip(v.chunks_exact(3)) { + for (p, c) in out.data.iter_mut().zip(v.as_chunks::<3>().0) { *p = [c[0], c[1], c[2]].map(|x| (trc.encode(x) * 255.0 + 0.5) as u8).into_rgba(); } } diff --git a/crates/preview/src/disk.rs b/crates/preview/src/disk.rs index 995709a0..7dbdda1e 100644 --- a/crates/preview/src/disk.rs +++ b/crates/preview/src/disk.rs @@ -173,6 +173,6 @@ pub fn decode_jpeg(bytes: &[u8]) -> Option { if px.len() < w * h * 3 { return None; } - let data = px.chunks_exact(3).take(w * h).map(|c| [c[0], c[1], c[2], 255]).collect(); + let data = px.as_chunks::<3>().0.iter().take(w * h).map(|c| [c[0], c[1], c[2], 255]).collect(); Some(Rgba8 { width: w, height: h, data }) } diff --git a/crates/raster/src/lib.rs b/crates/raster/src/lib.rs index 50593f29..2ea21fa1 100644 --- a/crates/raster/src/lib.rs +++ b/crates/raster/src/lib.rs @@ -243,7 +243,7 @@ impl Rgba8 { if bytes.len() != width * height * 4 { return None; } - Some(Self { width, height, data: bytes.chunks_exact(4).map(|c| [c[0], c[1], c[2], c[3]]).collect() }) + Some(Self { width, height, data: bytes.as_chunks::<4>().0.iter().map(|c| [c[0], c[1], c[2], c[3]]).collect() }) } /// Linear Rec.709/sRGB primaries float image from 8-bit sRGB. pub fn to_linear(&self) -> Rgb32f { diff --git a/crates/raw/src/demosaic/mod.rs b/crates/raw/src/demosaic/mod.rs index f00913c5..1ab3675c 100644 --- a/crates/raw/src/demosaic/mod.rs +++ b/crates/raw/src/demosaic/mod.rs @@ -70,7 +70,7 @@ pub fn demosaic(n: &Normalized, method: Method) -> Rgb32f { return Rgb32f::new(w, h); } match (&n.cfa, n.cpp) { - (_, 3) => Rgb32f { width: w, height: h, data: n.data.chunks_exact(3).map(|c| [c[0], c[1], c[2]]).collect() }, + (_, 3) => Rgb32f { width: w, height: h, data: n.data.as_chunks::<3>().0.iter().map(|c| [c[0], c[1], c[2]]).collect() }, (_, cpp) if cpp != 1 => Rgb32f { width: w, height: h, data: n.data.chunks_exact(cpp).map(|c| [c[0], c[1], c[2]]).collect() }, (None, _) => Rgb32f { width: w, height: h, data: n.data.iter().map(|&v| [v; 3]).collect() }, (Some(cfa), _) => { diff --git a/crates/raw/src/opcodes.rs b/crates/raw/src/opcodes.rs index 03ceebc8..f63eaf17 100644 --- a/crates/raw/src/opcodes.rs +++ b/crates/raw/src/opcodes.rs @@ -511,7 +511,7 @@ pub fn apply_list3(list: &[Opcode], img: &mut Rgb32f) { other => { let mut flat: Vec = img.data.iter().flat_map(|p| *p).collect(); apply_list(std::slice::from_ref(other), &mut flat, w, h, 3, None, 1.0); - for (d, c) in img.data.iter_mut().zip(flat.chunks_exact(3)) { + for (d, c) in img.data.iter_mut().zip(flat.as_chunks::<3>().0) { *d = [c[0], c[1], c[2]]; } } diff --git a/crates/raw/src/unpack.rs b/crates/raw/src/unpack.rs index 6d86e02f..15451548 100644 --- a/crates/raw/src/unpack.rs +++ b/crates/raw/src/unpack.rs @@ -41,7 +41,7 @@ pub fn unpack_lsb(src: &[u8], bits: u32, out: &mut [u16]) { } pub fn read_u16s(src: &[u8], order: ByteOrder, out: &mut [u16]) { - for (o, c) in out.iter_mut().zip(src.chunks_exact(2)) { + for (o, c) in out.iter_mut().zip(src.as_chunks::<2>().0) { *o = order.u16([c[0], c[1]]); } } diff --git a/crates/raw/src/vendor/raf.rs b/crates/raw/src/vendor/raf.rs index e8ee126e..ad9eaf85 100644 --- a/crates/raw/src/vendor/raf.rs +++ b/crates/raw/src/vendor/raf.rs @@ -98,7 +98,7 @@ fn cfa(h: &Header) -> Cfa { /// Unpack one row of `bits`-bit samples in Fujifilm's packing. pub(crate) fn unpack_row(src: &[u8], bits: u32, out: &mut [u16]) { match bits { - 16 => out.iter_mut().zip(src.chunks_exact(2)).for_each(|(o, c)| *o = u16::from_le_bytes([c[0], c[1]])), + 16 => out.iter_mut().zip(src.as_chunks::<2>().0).for_each(|(o, c)| *o = u16::from_le_bytes([c[0], c[1]])), 12 => unpack_lsb(src, 12, out), _ => { let swapped: Vec = src diff --git a/crates/tiff/src/reader.rs b/crates/tiff/src/reader.rs index a491f0dd..e1c6dd3a 100644 --- a/crates/tiff/src/reader.rs +++ b/crates/tiff/src/reader.rs @@ -220,17 +220,19 @@ pub(crate) fn decode(o: ByteOrder, typ: FieldType, b: &[u8]) -> Value { Value::Ascii(String::from_utf8_lossy(&b[..end]).into_owned()) } FieldType::SByte => Value::SByte(b.iter().map(|&x| x as i8).collect()), - FieldType::Short => Value::Short(b.chunks_exact(2).map(|c| o.u16(arr(c))).collect()), - FieldType::SShort => Value::SShort(b.chunks_exact(2).map(|c| o.u16(arr(c)) as i16).collect()), - FieldType::Long => Value::Long(b.chunks_exact(4).map(|c| o.u32(arr(c))).collect()), - FieldType::Ifd => Value::Ifd(b.chunks_exact(4).map(|c| o.u32(arr(c))).collect()), - FieldType::SLong => Value::SLong(b.chunks_exact(4).map(|c| o.u32(arr(c)) as i32).collect()), - FieldType::Float => Value::Float(b.chunks_exact(4).map(|c| f32::from_bits(o.u32(arr(c)))).collect()), - FieldType::Rational => Value::Rational(b.chunks_exact(8).map(|c| (o.u32(arr(&c[..4])), o.u32(arr(&c[4..])))).collect()), - FieldType::SRational => Value::SRational(b.chunks_exact(8).map(|c| (o.u32(arr(&c[..4])) as i32, o.u32(arr(&c[4..])) as i32)).collect()), - FieldType::Double => Value::Double(b.chunks_exact(8).map(|c| f64::from_bits(o.u64(arr(c)))).collect()), - FieldType::Long8 => Value::Long8(b.chunks_exact(8).map(|c| o.u64(arr(c))).collect()), - FieldType::Ifd8 => Value::Ifd8(b.chunks_exact(8).map(|c| o.u64(arr(c))).collect()), - FieldType::SLong8 => Value::SLong8(b.chunks_exact(8).map(|c| o.u64(arr(c)) as i64).collect()), + FieldType::Short => Value::Short(b.as_chunks::<2>().0.iter().map(|c| o.u16(arr(c))).collect()), + FieldType::SShort => Value::SShort(b.as_chunks::<2>().0.iter().map(|c| o.u16(arr(c)) as i16).collect()), + FieldType::Long => Value::Long(b.as_chunks::<4>().0.iter().map(|c| o.u32(arr(c))).collect()), + FieldType::Ifd => Value::Ifd(b.as_chunks::<4>().0.iter().map(|c| o.u32(arr(c))).collect()), + FieldType::SLong => Value::SLong(b.as_chunks::<4>().0.iter().map(|c| o.u32(arr(c)) as i32).collect()), + FieldType::Float => Value::Float(b.as_chunks::<4>().0.iter().map(|c| f32::from_bits(o.u32(arr(c)))).collect()), + FieldType::Rational => Value::Rational(b.as_chunks::<8>().0.iter().map(|c| (o.u32(arr(&c[..4])), o.u32(arr(&c[4..])))).collect()), + FieldType::SRational => { + Value::SRational(b.as_chunks::<8>().0.iter().map(|c| (o.u32(arr(&c[..4])) as i32, o.u32(arr(&c[4..])) as i32)).collect()) + } + FieldType::Double => Value::Double(b.as_chunks::<8>().0.iter().map(|c| f64::from_bits(o.u64(arr(c)))).collect()), + FieldType::Long8 => Value::Long8(b.as_chunks::<8>().0.iter().map(|c| o.u64(arr(c))).collect()), + FieldType::Ifd8 => Value::Ifd8(b.as_chunks::<8>().0.iter().map(|c| o.u64(arr(c))).collect()), + FieldType::SLong8 => Value::SLong8(b.as_chunks::<8>().0.iter().map(|c| o.u64(arr(c)) as i64).collect()), } } diff --git a/crates/ui-egui/src/softpaint.rs b/crates/ui-egui/src/softpaint.rs index fb7632d7..66f8b491 100644 --- a/crates/ui-egui/src/softpaint.rs +++ b/crates/ui-egui/src/softpaint.rs @@ -216,7 +216,7 @@ fn paint_band(prepared: &[Prepared<'_>], band: &mut [Color32], y0: usize, w: usi continue; } let clip = [p.clip[0], cy0, p.clip[2], cy1]; - for tri in p.indices.chunks_exact(3) { + for tri in p.indices.as_chunks::<3>().0 { let (Some(a), Some(b), Some(c)) = (p.verts.get(tri[0] as usize), p.verts.get(tri[1] as usize), p.verts.get(tri[2] as usize)) else { continue; };