From d562ceaaf4fc209a691ee4ec8707dcf6f00172f3 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Wed, 26 Aug 2026 22:52:38 +0200 Subject: [PATCH] Show a face beside each name in the people rail The rail was drawing an empty square for every person: cover was hardcoded to a default image. That is the one place a portrait matters most, because the rail is how the user decides which unnamed group to open first, and a list of "Unnamed (24 faces)" rows tells them nothing. The portrait is the person's confirmed face with the largest crop_px -- the most source pixels the face actually occupied, so the one they have the best chance of recognising -- falling back to a suggestion so a freshly clustered group still has a face beside it. Cached in the controller, because every mutating action reloads the whole screen and cutting a portrait costs a JPEG decode per person. Without the cache, confirming one face would re-decode a proxy for every person in the library, and the rail does not change when a suggestion is accepted. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/identity.rs | 38 ++++++++++++++++++++++++++++++++ ui/dr-ui/src/identity_ui.rs | 44 ++++++++++++++++++++++++++++++------- 2 files changed, 74 insertions(+), 8 deletions(-) diff --git a/ui/dr-ui/src/identity.rs b/ui/dr-ui/src/identity.rs index 6aa2121..e934d29 100644 --- a/ui/dr-ui/src/identity.rs +++ b/ui/dr-ui/src/identity.rs @@ -34,6 +34,13 @@ use crate::faces::{crop_face, FaceCrop, FACE_TIER}; /// the same person* — is one the user cannot make from a postage stamp. pub const FACE_CROP_EDGE: u32 = 128; +/// Edge of the portrait beside a name in the people rail. +/// +/// Small on purpose: the rail is for *recognising* a person you already know, +/// not for judging a likeness, and forty of them at grid size would be a +/// second grid competing with the real one. +pub const COVER_CROP_EDGE: u32 = 80; + /// A row in the people rail. #[derive(Debug, Clone, PartialEq)] pub struct PersonRow { @@ -185,6 +192,37 @@ pub fn load_faces( Ok(out) } +/// The face to show as a person's portrait. +/// +/// Confirmed faces first, then the largest — `crop_px` is the number of source +/// pixels the face actually occupied, so the largest is the one the user has +/// the best chance of recognising. Falling back to a suggestion means a +/// freshly clustered group still has a face beside it, which is the moment the +/// portrait matters most: the rail is how the user decides which unnamed group +/// to open first. +pub fn load_cover( + catalog: &Catalog, + store: &ThumbStore, + person: PersonId, +) -> Result, dr_catalog::CatalogError> { + let rows = faces::for_person(catalog.connection(), person, true)?; + let Some(best) = rows + .into_iter() + .max_by(|a, b| { + a.confirmed + .cmp(&b.confirmed) + .then(a.crop_px.total_cmp(&b.crop_px)) + }) + else { + return Ok(None); + }; + + let Some((w, h, rgba)) = decode_proxy(catalog, store, best.image_id) else { + return Ok(None); + }; + Ok(crop_face(&rgba, w, h, &best, COVER_CROP_EDGE)) +} + fn decode_proxy( catalog: &Catalog, store: &ThumbStore, diff --git a/ui/dr-ui/src/identity_ui.rs b/ui/dr-ui/src/identity_ui.rs index 9ededd2..1ddb821 100644 --- a/ui/dr-ui/src/identity_ui.rs +++ b/ui/dr-ui/src/identity_ui.rs @@ -43,6 +43,13 @@ pub struct IdentityController { /// Images visited so far in the current sweep, and the total it announced. progress: std::cell::Cell<(usize, usize)>, faces_found: std::cell::Cell, + /// Rail portraits, kept between refreshes. + /// + /// Every mutating action reloads the whole screen, and cutting a portrait + /// costs a JPEG decode per person. Without this, confirming one face would + /// re-decode a proxy for every person in the library — and the rail does + /// not change when a suggestion is accepted. + covers: RefCell>, } impl IdentityController { @@ -85,17 +92,37 @@ pub fn refresh( window.set_identity_unassigned(view.unassigned as i32); window.set_identity_calibrated(view.calibrated); + // Drop portraits for people who no longer exist, so a long session that + // merges and splits repeatedly does not accumulate them. + { + let live: std::collections::HashSet = + view.people.iter().map(|p| p.id).collect(); + ctl.covers.borrow_mut().retain(|id, _| live.contains(id)); + } + let rows: Vec = view .people .iter() - .map(|p| IdentityPerson { - id: p.id.0 as i32, - label: p.display_name().into(), - unconfirmed: p.is_unconfirmed(), - confirmed_faces: p.confirmed_faces as i32, - suggested_faces: p.suggested_faces as i32, - cover: slint::Image::default(), - has_cover: false, + .map(|p| { + let cover = store.and_then(|store| { + if let Some(img) = ctl.covers.borrow().get(&p.id) { + return Some(img.clone()); + } + let crop = identity::load_cover(cat, store, p.id).ok().flatten()?; + let img = to_slint_image(crop.width, crop.height, &crop.rgba); + ctl.covers.borrow_mut().insert(p.id, img.clone()); + Some(img) + }); + + IdentityPerson { + id: p.id.0 as i32, + label: p.display_name().into(), + unconfirmed: p.is_unconfirmed(), + confirmed_faces: p.confirmed_faces as i32, + suggested_faces: p.suggested_faces as i32, + has_cover: cover.is_some(), + cover: cover.unwrap_or_default(), + } }) .collect(); window.set_identity_people(ModelRc::new(VecModel::from(rows))); @@ -549,6 +576,7 @@ pub fn wire( } ctl.selected.set(None); ctl.clear_picks(); + ctl.covers.borrow_mut().clear(); reload!(w, ctl, catalog, store); }); }