From b9621eb5a3737be94bb2ce81f826a37c2df84abb Mon Sep 17 00:00:00 2001 From: Daniel Voshart <205798369+voshart@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:48:41 -0400 Subject: [PATCH] People view: offer only people in the photos the active filters let through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The view listed everyone in the library, so with another filter active (a date, a keyword…) picking a person could end in an empty grid. The list now covers the photos matching every filter except `person` itself, counts are within them, and the filter chips show above the cards (removable, Clear all). `Catalog::people_in(&Filter)`; the cache is keyed on the catalog revision and the filter. Co-Authored-By: Claude Sonnet 5.5 --- crates/catalog/src/query.rs | 10 +++++++++- crates/catalog/src/tests.rs | 17 +++++++++++++++++ crates/ui-egui/src/lib.rs | 18 ++++++++++++------ crates/ui-egui/src/panels/people.rs | 20 ++++++++++---------- docs/parity.md | 2 +- 5 files changed, 49 insertions(+), 18 deletions(-) diff --git a/crates/catalog/src/query.rs b/crates/catalog/src/query.rs index e25e889a1..384c2ae55 100644 --- a/crates/catalog/src/query.rs +++ b/crates/catalog/src/query.rs @@ -355,8 +355,16 @@ impl Catalog { /// then by name. Names that differ only in case are one person, shown as first seen; a person /// twice in one photo counts once. pub fn people(&self) -> Vec { + self.people_in(&Filter::default()) + } + + /// [`Self::people`] among the photos `filter` lets through (its `person` is ignored): what the + /// People view offers while other filters (a date, a rating, an album…) are active, so picking + /// a person never ends in an empty grid. + pub fn people_in(&self, filter: &Filter) -> Vec { + let filter = Filter { person: None, ..filter.clone() }; let mut m: std::collections::HashMap = Default::default(); - for p in self.photos().filter(|p| p.in_library()) { + for p in self.photos().filter(|p| filter.matches(p, self)) { let mut seen: Vec = Vec::new(); for r in p.meta.regions.iter().filter(|r| r.kind == lightcraft_meta::RegionKind::Face) { let Some(name) = r.name.as_deref().map(str::trim).filter(|n| !n.is_empty()) else { continue }; diff --git a/crates/catalog/src/tests.rs b/crates/catalog/src/tests.rs index 3c257da1a..7897f7972 100644 --- a/crates/catalog/src/tests.rs +++ b/crates/catalog/src/tests.rs @@ -342,4 +342,21 @@ fn people_from_named_face_regions() { assert!(q(Filter { person: Some("Rex".into()), ..Default::default() }).is_empty(), "a pet is not a person"); assert_eq!(q(Filter { text: "person:SAM".into(), ..Default::default() }), vec![e], "the search token finds a person, any case"); assert!(q(Filter { person: Some("Jane Doe".into()), ..Default::default() }).contains(&a)); + + // other filters narrow who is offered (so picking a person never ends in an empty grid); the + // `person` filter itself does not + c.apply(Op::SetRating { id: b, rating: 3 }).unwrap(); + let names = |f: Filter| c.people_in(&f).into_iter().map(|p| (p.name, p.count)).collect::>(); + let rated = Filter { rating: 1, ..Default::default() }; + assert_eq!( + names(rated.clone()), + vec![("JANE DOE".to_string(), 1), ("John Roe".to_string(), 1)], + "only the rated photo's people, counted within it, spelled as first seen there" + ); + assert_eq!( + names(Filter { person: Some("Sam".into()), ..rated }), + names(Filter { rating: 1, ..Default::default() }), + "the person filter is ignored" + ); + assert_eq!(names(Filter { rating: 5, ..Default::default() }), vec![], "nobody in the filtered photos"); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index d0c33e6a2..d870e7188 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -790,7 +790,7 @@ mod drop_tests { #[derive(Default)] pub struct Caches { keyword_tree: Option<(u64, std::sync::Arc>)>, - people: Option<(u64, std::sync::Arc>)>, + people: Option<(u64, lightcraft_catalog::Filter, std::sync::Arc>)>, suggestions: Option<(u64, std::sync::Arc>)>, counts: Option<(u64, LibraryCounts)>, date_groups: Option<(u64, std::sync::Arc>)>, @@ -837,13 +837,19 @@ impl Caches { } } } - /// The people named on faces, with photo counts. - pub fn people(&mut self, cat: &lightcraft_catalog::Catalog) -> std::sync::Arc> { + /// The people named on faces among the photos the filter lets through (its own `person` aside), + /// with photo counts. + pub fn people( + &mut self, + cat: &lightcraft_catalog::Catalog, + filter: &lightcraft_catalog::Filter, + ) -> std::sync::Arc> { + let key = lightcraft_catalog::Filter { person: None, ..filter.clone() }; match &self.people { - Some((r, t)) if *r == cat.revision => t.clone(), + Some((r, f, t)) if *r == cat.revision && *f == key => t.clone(), _ => { - let t = std::sync::Arc::new(cat.people()); - self.people = Some((cat.revision, t.clone())); + let t = std::sync::Arc::new(cat.people_in(&key)); + self.people = Some((cat.revision, key, t.clone())); t } } diff --git a/crates/ui-egui/src/panels/people.rs b/crates/ui-egui/src/panels/people.rs index 52108c53e..118fbd1c3 100644 --- a/crates/ui-egui/src/panels/people.rs +++ b/crates/ui-egui/src/panels/people.rs @@ -20,20 +20,20 @@ const HEADER_H: f32 = 44.0; pub fn show(app: &mut LightcraftApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); - let people = app.caches.people(&app.session.catalog); + let people = app.caches.people(&app.session.catalog, &app.session.filter); let (head, _) = ui.allocate_exact_size(vec2(ui.available_width(), HEADER_H), Sense::hover()); ui.painter().text(pos2(head.left() + PAD, head.center().y), Align2::LEFT_CENTER, "Named People", t.semibold(15.0), t.text); ui.painter().text(pos2(head.right() - PAD, head.center().y), Align2::RIGHT_CENTER, people.len().to_string(), t.font(13.0), t.text_dim); + // the filters narrowing the list (a date, a keyword…), removable here + let chips = lightcraft_engine::filter_chips(&app.session.filter, &app.session.catalog); + super::chips::show(app, ui, &chips); if people.is_empty() { - let c = ui.available_rect_before_wrap().center(); - ui.painter().text(c, Align2::CENTER_CENTER, "No named people yet", t.semibold(15.0), t.text_label); - ui.painter().text( - c + vec2(0.0, 26.0), - Align2::CENTER_CENTER, - "Face names written to XMP by Lightroom and other apps show up here.", - t.font(13.0), - t.text_dim, - ); + let (title, body) = if chips.is_empty() { + ("No named people yet", "Face names written to XMP by Lightroom and other apps show up here.") + } else { + ("No named people in these photos", "Remove a filter above, or choose Clear all") + }; + super::empty_message(ui, ui.available_rect_before_wrap(), title, body); return; } let ppp = ui.ctx().pixels_per_point(); diff --git a/docs/parity.md b/docs/parity.md index c73133b8f..b29c5e069 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -724,7 +724,7 @@ From `08-lightroom-classic-extras.md` (Classic-only features) and part 2 of `06- | LRC-LIB-KEYWORDS | Hierarchical keywords, sets, painter | P1 | ✅ | `crates/catalog/src/keywords.rs`, `cmd:keyword.list`, `cmd:keyword.sets`, `cmd:keyword.toggleFromSet`, `cmd:keyword.saveSet`, `cmd:tool.keywordPainter` | hierarchical `a\|b\|c` keywords (tree, parent filters include children, rename moves children); keyword sets + Recent Keywords (⌥1–⌥9); keyword painter (Keywords panel ▸ Paint: click photos in the grid to toggle a keyword, Esc stops) | | LRC-LIB-METADATA | Metadata panel & presets | P1 | ✅ | `cmd:photo.setMeta`, `cmd:metadata.savePreset`, `cmd:metadata.applyPreset`, `cmd:metadata.presets`, `cmd:metadata.deletePreset`, `cmd:dialog.captureTime` | IPTC core, accessibility, place, capture-time edit; copyright status (unknown / copyrighted / public domain), rights usage terms and copyright info URL (`photo.setMeta` `copyrightStatus`, `usageTerms`, `copyrightUrl`; XMP Rights Management fields, kept by "copyright only" exports, a smart-album rule); metadata presets (Photo → Metadata Preset, Save Metadata Preset… from the active photo; applied on import from Settings) | | LRC-LIB-QUICKDEV | Quick develop | P2 | ✅ | `cmd:develop.quickAdjust`, `crates/ui-egui/src/panels/edit.rs` (`quick_develop`) | grid with several photos selected: Edit panel ▸ Quick Develop steps (exposure ⅓ / 1 stop, contrast, highlights, shadows, whites, blacks, clarity, vibrance, temperature) added to each photo's own value, one undo step | -| LRC-LIB-PEOPLE | People view | P2 | 🟡 | `cmd:view.people`, `crates/ui-egui/src/panels/people.rs` | Named People cards (face close-up, name, photos); no Unnamed People, no per-person page, no confirm / reject | +| LRC-LIB-PEOPLE | People view | P2 | 🟡 | `cmd:view.people`, `crates/ui-egui/src/panels/people.rs` | Named People cards (face close-up, name, photos) for the photos the active filters let through, with the filter chips shown above them; no Unnamed People, no per-person page, no confirm / reject | | 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`, `crates/engine/src/cmd/missing.rs` (`checked_path`) | File → Find Missing Photos… (anywhere in a folder: same name, size and content hash; renamed files by size + content hash; same name and size alone only when unambiguous — look-alikes are skipped and reported; searched on a worker thread, 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; the view, the Info panel, the grid's context menu and thumbnails read file availability from a cache a worker thread fills, so an offline NAS never stalls a frame); it covers library photos only — Local browse records are never checked, for the count, the view, `library.missing` and Find Missing alike |