From d6380fecc8b0e2616ce89e248c976edbab3f5229 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Thu, 27 Aug 2026 21:21:31 +0200 Subject: [PATCH] Make the People screen a place work can be done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five faults, all on one screen, and the Slint and Rust halves of each have to land together. **Regroup froze the window.** It ran inside the Slint callback, on the UI thread. It is much faster now, but fast is not bounded — the work grows with the library, and the one thing that must not grow with the library is how long the window stops answering. It runs on a worker thread with an mpsc channel and a 250ms poll, like every other long pass in this module, and the button says what it is doing instead of the window going quiet. Cancellation is dropping the receiver. Reclustering also prunes the empty groups the previous pass left, so pressing the button twice no longer fills the rail with "Unnamed (0 faces)". **The faces were a single row running off the screen.** The comment on the layout claimed to be a wrapping row; Slint has no flow layout and a HorizontalLayout does not wrap, so a person with forty faces was a person whose faces could not be reviewed past the fifth. It is now laid out the way the library grid lays out thumbnails, with the same arithmetic: choose how many columns of roughly the requested size fit, then divide the width between them so the cells fill the row exactly and nothing overhangs. **The header did not fit a phone.** A 240px name field beside five buttons is wider than an Android screen — and worse than not fitting, a layout cannot be narrower than its children's minimums, so the row reported that oversized minimum upwards and inflated the whole screen. The faces grid is its sibling, so it would have been measured against a width that was never on the display. The header is now two rows, the actions sit in a Flickable that scrolls rather than overflowing, and the rail narrows to 132px on the compact class. **Strangers crowded out the people who matter.** Most clusters in a real library are passers-by and other people's guests. "Not interested" sets a group aside; the rail hides it and says how many are hidden, with one button to bring them back. Reversible, and never a deletion — see the catalog commit for why. **A face was a dead end.** Identifying someone and then having no way to see their photographs is a filing cabinet with no drawer handles. "Show photos" narrows the library grid to that person and leaves a chip on the filter bar saying so, which is also how it is cleared. It is a term on `RatingFilter` rather than a grid scope of its own, exactly as that struct's own doc says new narrowing terms should be — so the count and the cells are narrowed by the same thing, and it composes with the others for free. Suggested faces count, not only confirmed ones, or a freshly grouped person would show an empty grid. Crops are read from where they are now stored, falling back to cutting one out of the proxy for faces indexed before that existed. 480 tests pass. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/faces.rs | 148 ++++++++++++++++++++++++++- ui/dr-ui/src/identity.rs | 75 +++++++++++--- ui/dr-ui/src/identity_ui.rs | 141 ++++++++++++++++++++++++-- ui/dr-ui/src/library.rs | 33 +++++++ ui/dr-ui/src/library_ui.rs | 49 +++++++++ ui/dr-ui/ui/app.slint | 22 +++++ ui/dr-ui/ui/identity.slint | 192 +++++++++++++++++++++++++++++++----- ui/dr-ui/ui/library.slint | 19 +++- 8 files changed, 634 insertions(+), 45 deletions(-) diff --git a/ui/dr-ui/src/faces.rs b/ui/dr-ui/src/faces.rs index f6c332b..01ddb5a 100644 --- a/ui/dr-ui/src/faces.rs +++ b/ui/dr-ui/src/faces.rs @@ -28,7 +28,7 @@ use std::sync::mpsc::{Receiver, Sender}; use dr_catalog::faces::{self, DetectedFace}; use dr_catalog::Catalog; -use dr_face::{align, Calibration, DetectOptions, Detector, Embedder, ModelId}; +use dr_face::{align, Calibration, DetectOptions, Detection, Detector, Embedder, ModelId}; use dr_thumbs::{ThumbSize, ThumbStore}; use dr_types::ImageId; @@ -250,6 +250,9 @@ pub fn index_proxy( embedding: embedding.to_f16_bytes(), crop_px: aligned.source_px(), model_id: embedder.model().as_str().to_string(), + // Cut here, while the buffer is still in hand. This is the only + // moment in the whole pipeline where the pixels are free. + crop: cut_crop(rgb, width, height, d).unwrap_or_default(), }); } @@ -528,6 +531,16 @@ pub fn recluster( } } + // Groups the *previous* pass created that this one left empty. Without + // this, every press of Regroup adds a rail entry per group it no longer + // believes in, and the screen fills with "Unnamed (0 faces)" — which is + // what made pressing the button twice look like it had broken something. + match faces::prune_empty_unnamed(conn) { + Ok(0) => {} + Ok(n) => log::info!("reclustering removed {n} empty group(s) from the previous pass"), + Err(e) => log::warn!("pruning empty groups: {e}"), + } + log::info!( "reclustered {} face(s) into {} group(s): {suggested} suggestion(s), {created} new", candidates.len(), @@ -536,6 +549,71 @@ pub fn recluster( Ok((suggested, created)) } +/// Progress from a regrouping pass. +#[derive(Debug, Clone, PartialEq)] +pub enum ReclusterMessage { + /// How many faces went in. Sent once, before the arithmetic starts. + Started { faces: usize }, + /// It finished. + Finished { suggested: usize, created: usize }, + /// It did not. + Failed(String), +} + +/// Group the library's faces into people, **on a worker thread**. +/// +/// The reason this exists rather than callers just invoking [`recluster`]: it +/// used to run inside the Slint callback, on the UI thread, and clustering a +/// real library is not something a callback can do. The window froze for as +/// long as it took, with no progress, no cancel and no repaint — the button +/// looked broken because from the outside it was indistinguishable from broken. +/// +/// It is much faster now (see [`dr_face::cluster`]), but *fast* is not the same +/// as *bounded*: the work grows with the library and the one thing that must +/// not grow with the library is how long the window stops answering. So it runs +/// where every other long pass in this module runs. +/// +/// Cancellation is dropping the receiver, exactly as with the indexing sweep. +/// Nothing is left half-written: [`recluster`] does its work in the catalog's +/// own transactions, and a pass abandoned partway simply leaves the previous +/// grouping in place to be redone. +pub fn spawn_recluster( + catalog_path: PathBuf, + model_id: String, + min_probability: f32, +) -> Receiver { + let (tx, rx) = std::sync::mpsc::channel(); + + std::thread::spawn(move || { + let catalog = match Catalog::open(&catalog_path) { + Ok(c) => c, + Err(e) => { + let _ = tx.send(ReclusterMessage::Failed(format!( + "cannot open catalog: {e}" + ))); + return; + } + }; + + // Announced before the work so the screen can say what it is chewing + // on. Cheap: it is a count, not the embeddings themselves. + let count = faces::embeddings(catalog.connection(), &model_id) + .map(|e| e.len()) + .unwrap_or(0); + if tx.send(ReclusterMessage::Started { faces: count }).is_err() { + return; + } + + let msg = match recluster(&catalog, &model_id, min_probability) { + Ok((suggested, created)) => ReclusterMessage::Finished { suggested, created }, + Err(e) => ReclusterMessage::Failed(e.to_string()), + }; + let _ = tx.send(msg); + }); + + rx +} + /// Mean calibrated probability between one member and the rest of its group. fn group_probability( candidates: &[dr_face::Candidate], @@ -643,6 +721,74 @@ pub fn crop_face( }) } +/// Edge of the crop stored with a face. +/// +/// Above both [`crate::identity::FACE_CROP_EDGE`] (128) and +/// `COVER_CROP_EDGE` (80), so the stored image is downsampled to draw and never +/// upsampled — a stored crop the same size as the grid cell would go soft the +/// moment either constant grew. 160 px of JPEG is a few KB, which is nothing +/// beside the 250 KB proxy it saves decoding. +pub const STORED_CROP_EDGE: u32 = 160; + +/// Cut one detected face out of the buffer it was found in, as a JPEG. +/// +/// The same square [`crop_face`] would cut — centred on the box, widened by +/// [`CROP_MARGIN`] — so a face drawn from its stored crop and one drawn the old +/// way from the proxy are the same picture. Working in pixels rather than +/// normalised coordinates because at this point in the pipeline that is what +/// there is; the normalising happens afterwards. +/// +/// **Out-of-frame samples clamp to the edge rather than going transparent.** +/// [`crop_face`] leaves them clear, which is right when the caller can composite +/// them; JPEG has no alpha, so the same choice here would bake a black bar into +/// every face near the edge of its photograph — and then drag that face's mean +/// luma down far enough for `load_cover` to reject it as too dark. +/// +/// `None` where the geometry is degenerate, which is the caller's cue to store +/// nothing and fall back to the proxy. +fn cut_crop(rgb: &[f32], width: usize, height: usize, d: &Detection) -> Option> { + if width == 0 || height == 0 { + return None; + } + let cx = d.bbox.0 + d.width() * 0.5; + let cy = d.bbox.1 + d.height() * 0.5; + let half = d.width().max(d.height()) * 0.5 * (1.0 + CROP_MARGIN); + if !(half.is_finite() && half > 0.5 && cx.is_finite() && cy.is_finite()) { + return None; + } + + let edge = STORED_CROP_EDGE; + let mut out = vec![0u8; (edge * edge * 4) as usize]; + let step = (half * 2.0) / edge as f32; + + for oy in 0..edge { + let sy = cy - half + (oy as f32 + 0.5) * step; + let sy = (sy.max(0.0) as usize).min(height - 1); + for ox in 0..edge { + let sx = cx - half + (ox as f32 + 0.5) * step; + let sx = (sx.max(0.0) as usize).min(width - 1); + + let i = (sy * width + sx) * 3; + let o = ((oy * edge + ox) * 4) as usize; + if i + 3 > rgb.len() { + continue; + } + for c in 0..3 { + out[o + c] = (rgb[i + c].clamp(0.0, 1.0) * 255.0).round() as u8; + } + out[o + 3] = 255; + } + } + + match dr_thumbs::encode_rgba(edge, edge, &out) { + Ok(bytes) => Some(bytes), + Err(e) => { + log::debug!("encoding a face crop: {e}"); + None + } + } +} + /// Detect and embed every face in a decoded preview. /// /// The counterpart to [`index_proxy`] for the fetching sweep, which holds a diff --git a/ui/dr-ui/src/identity.rs b/ui/dr-ui/src/identity.rs index acb16e9..90cf3a3 100644 --- a/ui/dr-ui/src/identity.rs +++ b/ui/dr-ui/src/identity.rs @@ -51,6 +51,8 @@ pub struct PersonRow { pub suggested_faces: u64, /// The face to show as this person's portrait, if any is loaded. pub cover: Option, + /// Set aside by the user — see `faces::set_ignored`. + pub ignored: bool, } impl PersonRow { @@ -138,6 +140,7 @@ pub fn load_people( confirmed_faces: p.confirmed_faces, suggested_faces: p.suggested_faces, cover: None, + ignored: p.ignored, }) .collect(); @@ -168,21 +171,31 @@ pub fn load_faces( let conn = catalog.connection(); let rows = faces::for_person(conn, person, true)?; - // One decode per *image*, not per face: a group photograph holding six - // faces of one family is one JPEG, and decoding it six times is the kind of - // waste that only shows up on a slow machine. + // The crops kept at detection time, in one query. Where a face has one this + // is the whole cost of drawing it — no proxy, no full-size JPEG decode, and + // no dependence on the thumbnail cache still holding the photograph. + let stored = faces::crops_for_person(conn, person, true)?; + + // The fallback path, for faces indexed before crops were kept. One decode + // per *image*, not per face: a group photograph holding six faces of one + // family is one JPEG, and decoding it six times is the kind of waste that + // only shows up on a slow machine. let mut decoded: std::collections::HashMap)>> = std::collections::HashMap::new(); let mut out = Vec::with_capacity(rows.len()); for f in rows { - let entry = decoded - .entry(f.image_id) - .or_insert_with(|| decode_proxy(catalog, store, f.image_id)); - - let crop = entry - .as_ref() - .and_then(|(w, h, rgba)| crop_face(rgba, *w, *h, &f, FACE_CROP_EDGE)); + let crop = match stored.get(&f.id).and_then(|b| decode_crop(b)) { + Some(c) => Some(c), + None => { + let entry = decoded + .entry(f.image_id) + .or_insert_with(|| decode_proxy(catalog, store, f.image_id)); + entry + .as_ref() + .and_then(|(w, h, rgba)| crop_face(rgba, *w, *h, &f, FACE_CROP_EDGE)) + } + }; out.push(FaceCell { face: f.id, @@ -231,12 +244,25 @@ pub fn load_cover( // already in preference order — so this gives up size only when the // preferred face is genuinely too dark to recognise. let mut fallback: Option = None; + let conn = catalog.connection(); for face in rows.iter().take(COVER_CANDIDATES) { - let Some((w, h, rgba)) = decode_proxy(catalog, store, face.image_id) else { - continue; - }; - let Some(crop) = crop_face(&rgba, w, h, face, COVER_CROP_EDGE) else { - continue; + // A stored crop costs a small JPEG decode; the fallback costs a full + // proxy decode, which is what made the rail expensive to build. + let crop = match faces::crop(conn, face.id) + .ok() + .flatten() + .and_then(|b| decode_crop(&b)) + { + Some(c) => c, + None => { + let Some((w, h, rgba)) = decode_proxy(catalog, store, face.image_id) else { + continue; + }; + let Some(c) = crop_face(&rgba, w, h, face, COVER_CROP_EDGE) else { + continue; + }; + c + } }; if mean_luma(&crop) >= MIN_COVER_LUMA { return Ok(Some(crop)); @@ -277,6 +303,21 @@ fn mean_luma(crop: &FaceCrop) -> f32 { } } +/// Turn a stored crop back into pixels. +/// +/// Returned at whatever size it was stored (`faces::STORED_CROP_EDGE`) rather +/// than resampled down to the cell: the grid and the rail scale it themselves, +/// and doing it here would mean two resamples where one will do — and a soft +/// portrait for the trouble. +fn decode_crop(bytes: &[u8]) -> Option { + let (w, h, rgba) = dr_thumbs::codec::decode_rgba(bytes).ok()?; + Some(FaceCrop { + width: w, + height: h, + rgba, + }) +} + fn decode_proxy( catalog: &Catalog, store: &ThumbStore, @@ -485,6 +526,7 @@ pub fn namesake( confirmed_faces: p.confirmed_faces, suggested_faces: p.suggested_faces, cover: None, + ignored: p.ignored, })) } @@ -641,6 +683,7 @@ mod tests { embedding: vec![seed; 1024], crop_px: 180.0, model_id: "w600k_mbf".into(), + crop: Vec::new(), } } @@ -652,6 +695,7 @@ mod tests { confirmed_faces: 0, suggested_faces: 12, cover: None, + ignored: false, }; assert_eq!(row.display_name(), "Unnamed (12 faces)"); assert!(row.is_unconfirmed()); @@ -665,6 +709,7 @@ mod tests { confirmed_faces: 4, suggested_faces: 2, cover: None, + ignored: false, }; assert_eq!(row.display_name(), "Anna"); assert!(!row.is_unconfirmed()); diff --git a/ui/dr-ui/src/identity_ui.rs b/ui/dr-ui/src/identity_ui.rs index 377443f..87f503c 100644 --- a/ui/dr-ui/src/identity_ui.rs +++ b/ui/dr-ui/src/identity_ui.rs @@ -75,6 +75,14 @@ pub struct IdentityController { /// two people called Anna are exactly the case this feature exists for, so /// re-deriving the target from the displayed text could pick the wrong one. merge_offer: std::cell::Cell>, + /// The running regrouping pass, if any. + /// + /// Same shape as `sweep`, and for the same reason: holding the receiver is + /// the handle, and clustering a real library is not something a Slint + /// callback may do on the UI thread. + regroup: RefCell>>, + /// Whether the rail is showing the people the user has set aside. + show_ignored: std::cell::Cell, /// Rail portraits, kept between refreshes. /// /// Every mutating action reloads the whole screen, and cutting a portrait @@ -153,10 +161,13 @@ pub fn refresh( suggested_faces: p.suggested_faces as i32, has_cover: cover.is_some(), cover: cover.unwrap_or_default(), + ignored: p.ignored, } }) .collect(); window.set_identity_people(ModelRc::new(VecModel::from(rows))); + window.set_identity_ignored_count(view.people.iter().filter(|p| p.ignored).count() as i32); + window.set_identity_show_ignored(ctl.show_ignored.get()); *ctl.people.borrow_mut() = view.people; // The selected person may have just been merged away or deleted. @@ -195,10 +206,18 @@ pub fn refresh( .unwrap_or_default(); window.set_identity_selected(person.0 as i32); window.set_identity_selected_name(name.into()); + window.set_identity_selected_ignored( + ctl.people + .borrow() + .iter() + .find(|p| p.id == person) + .is_some_and(|p| p.ignored), + ); } _ => { window.set_identity_selected(-1); window.set_identity_selected_name(Default::default()); + window.set_identity_selected_ignored(false); window.set_identity_faces(ModelRc::new(VecModel::from(Vec::::new()))); ctl.faces.borrow_mut().clear(); } @@ -662,15 +681,88 @@ pub fn wire( let ctl = ctl.clone(); let catalog = catalog.clone(); let store = store.clone(); + let paths = paths.clone(); window.on_identity_recluster(move || { let Some(w) = weak.upgrade() else { return }; - if let Some(cat) = catalog.borrow().as_ref() { - match crate::faces::recluster(cat, MODEL_ID, dr_face::DEFAULT_MERGE_PROBABILITY) { - Ok((s, c)) => log::info!("identity: {s} suggestion(s), {c} new group(s)"), - Err(e) => log::warn!("identity: recluster: {e}"), - } + // One at a time. Two passes over the same faces would each create + // their own groups for the same clusters, and the second would + // undo the first's pruning. + if ctl.regroup.borrow().is_some() { + return; } - reload!(w, ctl, catalog, store); + let Some((_, _, catalog_path, _)) = paths() else { + return; + }; + + w.set_identity_regrouping(true); + w.set_identity_regroup_status("regrouping…".into()); + *ctl.regroup.borrow_mut() = Some(crate::faces::spawn_recluster( + catalog_path, + MODEL_ID.to_string(), + dr_face::DEFAULT_MERGE_PROBABILITY, + )); + + // Polled from the UI thread, like the indexing sweep: the worker + // is a plain thread with a channel, and every Slint property write + // has to happen where Slint requires it. + let timer = slint::Timer::default(); + let weak_tick = w.as_weak(); + let ctl_tick = ctl.clone(); + let catalog_tick = catalog.clone(); + let store_tick = store.clone(); + timer.start( + slint::TimerMode::Repeated, + Duration::from_millis(100), + move || { + let Some(w) = weak_tick.upgrade() else { return }; + let mut done = false; + { + let borrow = ctl_tick.regroup.borrow(); + let Some(rx) = borrow.as_ref() else { return }; + while let Ok(msg) = rx.try_recv() { + match msg { + crate::faces::ReclusterMessage::Started { faces } => { + w.set_identity_regroup_status( + format!("regrouping {faces} face(s)…").into(), + ); + } + crate::faces::ReclusterMessage::Finished { + suggested, + created, + } => { + log::info!( + "identity: {suggested} suggestion(s), {created} new group(s)" + ); + w.set_identity_regroup_status( + format!( + "{created} new group(s), {suggested} suggestion(s)" + ) + .into(), + ); + done = true; + } + crate::faces::ReclusterMessage::Failed(e) => { + log::warn!("identity: recluster: {e}"); + w.set_identity_regroup_status( + format!("regrouping failed: {e}").into(), + ); + done = true; + } + } + } + } + if done { + *ctl_tick.regroup.borrow_mut() = None; + w.set_identity_regrouping(false); + // Portraits are keyed on the person, and reclustering + // makes new people; a stale cache would draw the + // previous pass's faces beside the new groups. + ctl_tick.covers.borrow_mut().clear(); + refresh(&w, &ctl_tick, &catalog_tick, store_tick().as_deref()); + } + }, + ); + park_timer(timer); }); } @@ -810,6 +902,43 @@ pub fn wire( }); } + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + let catalog = catalog.clone(); + let store = store.clone(); + window.on_identity_ignore_person(move |id, on| { + let Some(w) = weak.upgrade() else { return }; + let person = PersonId(id.max(0) as u64); + if let Some(cat) = catalog.borrow().as_ref() { + if let Err(e) = dr_catalog::faces::set_ignored(cat.connection(), person, on) { + log::warn!("identity: setting a person aside: {e}"); + return; + } + } + // Move off the person just set aside, or the screen sits on a + // group the rail no longer shows — which reads as the button + // having done nothing. + if on && ctl.selected.get() == Some(person) { + ctl.selected.set(None); + ctl.clear_picks(); + } + reload!(w, ctl, catalog, store); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + let catalog = catalog.clone(); + let store = store.clone(); + window.on_identity_toggle_show_ignored(move || { + let Some(w) = weak.upgrade() else { return }; + ctl.show_ignored.set(!ctl.show_ignored.get()); + reload!(w, ctl, catalog, store); + }); + } + { let weak = window.as_weak(); let ctl = ctl.clone(); diff --git a/ui/dr-ui/src/library.rs b/ui/dr-ui/src/library.rs index d204e09..3d15e46 100644 --- a/ui/dr-ui/src/library.rs +++ b/ui/dr-ui/src/library.rs @@ -249,6 +249,25 @@ pub struct RatingFilter { /// above: the count and the cells must be narrowed by the same thing. pub captured_from: Option, pub captured_to: Option, + /// TRACES: FR-CULL-11 + /// Only photographs this person appears in. + /// + /// The way back from a face to the pictures it came from, which is the + /// question the People screen leaves the user holding: they have just + /// identified someone, and what they want next is *everything with them in + /// it*. Without this the identification is a dead end. + /// + /// Here rather than a grid scope of its own, for the reason `local_only` + /// gives above — the count and the cells must be narrowed by the same + /// thing, and this struct is the one narrowing every query path already + /// threads. It composes with the rest for free: three-star photographs of + /// Anna from last summer is this term ANDed with two others. + /// + /// Suggested faces count, not only confirmed ones. A user who has just + /// grouped someone and not yet confirmed a single face would otherwise get + /// an empty grid, which reads as "no photographs of this person" rather + /// than "you have not ticked anything yet". + pub person: Option, } impl RatingFilter { @@ -260,6 +279,7 @@ impl RatingFilter { && !self.local_only && self.captured_from.is_none() && self.captured_to.is_none() + && self.person.is_none() } /// Whether a date range is narrowing the grid. @@ -333,6 +353,17 @@ impl RatingFilter { ); } + if let Some(person) = self.person { + // An integer this code owns, like every other term here. `EXISTS` + // rather than a join, so a photograph holding three faces of the + // same person appears once — the grid shows pictures, not faces. + terms.push(format!( + "EXISTS (SELECT 1 FROM faces f + JOIN face_person fp ON fp.face_id = f.id + WHERE f.image_id = i.id AND fp.person_id = {person})" + )); + } + if let Some(flag) = self.flag { terms.push(format!( "coalesce((SELECT dv.flag FROM versions dv @@ -4587,6 +4618,7 @@ mod tests { confidence: 0.9, embedding: vec![0u8; 1024], crop_px: 120.0, + crop: Vec::new(), model_id: "w600k_mbf".into(), }; dr_catalog::faces::record_detections( @@ -4635,6 +4667,7 @@ mod tests { confidence: 0.9, embedding: vec![0u8; 1024], crop_px: 120.0, + crop: Vec::new(), model_id: "w600k_mbf".into(), }; let orphan = ids[40]; diff --git a/ui/dr-ui/src/library_ui.rs b/ui/dr-ui/src/library_ui.rs index e7b120e..f500a6d 100644 --- a/ui/dr-ui/src/library_ui.rs +++ b/ui/dr-ui/src/library_ui.rs @@ -5046,6 +5046,55 @@ pub fn wire( }); } + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_identity_show_photos(move |id| { + let Some(w) = weak.upgrade() else { return }; + let person = dr_catalog::faces::PersonId(id.max(0) as u64); + + // The chip needs a name, and an unnamed group has none — so it + // borrows the rail's own wording rather than inventing a second + // way to describe the same thing. + let label = { + let borrow = ctl.catalog.borrow(); + borrow + .as_ref() + .and_then(|cat| dr_catalog::faces::people(cat.connection()).ok()) + .and_then(|people| people.into_iter().find(|p| p.id == person)) + .map(|p| { + if p.name.trim().is_empty() { + format!("Unnamed ({} faces)", p.confirmed_faces + p.suggested_faces) + } else { + p.name + } + }) + .unwrap_or_else(|| "Person".to_string()) + }; + + ctl.filter.borrow_mut().person = Some(person.0); + w.set_library_filter_person_name(label.into()); + + // Leaving the Identity screen for the grid is the whole point of + // the button: the answer to "who is this" is a set of photographs, + // and they are shown where photographs are shown. + w.set_show_identity(false); + w.set_show_library(true); + refilter(&w, &ctl); + }); + } + + { + let weak = window.as_weak(); + let ctl = ctl.clone(); + window.on_library_filter_person_cleared(move || { + let Some(w) = weak.upgrade() else { return }; + ctl.filter.borrow_mut().person = None; + w.set_library_filter_person_name(Default::default()); + refilter(&w, &ctl); + }); + } + { let weak = window.as_weak(); let ctl = ctl.clone(); diff --git a/ui/dr-ui/ui/app.slint b/ui/dr-ui/ui/app.slint index 0d64579..8480db3 100644 --- a/ui/dr-ui/ui/app.slint +++ b/ui/dr-ui/ui/app.slint @@ -590,6 +590,14 @@ export component AppWindow inherits Window { callback identity-confirm-all(); callback identity-split-picked(); callback identity-recluster(); + in property identity-regrouping: false; + in property identity-regroup-status; + in property identity-ignored-count: 0; + in property identity-show-ignored: false; + in property identity-selected-ignored: false; + callback identity-toggle-show-ignored(); + callback identity-ignore-person(int, bool); + callback identity-show-photos(int); callback identity-index(); callback identity-stop-indexing(); callback identity-check-coverage(); @@ -765,6 +773,9 @@ export component AppWindow inherits Window { /// the two arguments is -1, saying which axis was *not* meant. callback library-judged(int, int); + /// Whose photographs the grid is narrowed to. Empty when it is not. + in property library-filter-person-name; + callback library-filter-person-cleared(); in property library-filter-min-rating: 0; in property library-filter-unjudged: false; in property library-filter-flag: 0; @@ -1402,6 +1413,15 @@ in property panel-visible: true; toggle-pick(id) => { root.identity-toggle-pick(id); } confirm-all() => { root.identity-confirm-all(); } split-picked() => { root.identity-split-picked(); } + compact: !root.expanded; + regrouping: root.identity-regrouping; + regroup-status: root.identity-regroup-status; + ignored-count: root.identity-ignored-count; + show-ignored: root.identity-show-ignored; + selected-ignored: root.identity-selected-ignored; + toggle-show-ignored() => { root.identity-toggle-show-ignored(); } + ignore-person(id, on) => { root.identity-ignore-person(id, on); } + show-photos(id) => { root.identity-show-photos(id); } recluster() => { root.identity-recluster(); } index-faces() => { root.identity-index(); } stop-indexing() => { root.identity-stop-indexing(); } @@ -1627,6 +1647,7 @@ in property panel-visible: true; root.library-remove-from-collection(); } + filter-person-name: root.library-filter-person-name; filter-min-rating: root.library-filter-min-rating; filter-unjudged: root.library-filter-unjudged; filter-flag: root.library-filter-flag; @@ -1657,6 +1678,7 @@ in property panel-visible: true; root.library-filter-unjudged-changed(on); } filter-flag-changed(f) => { root.library-filter-flag-changed(f); } + filter-person-cleared() => { root.library-filter-person-cleared(); } } } } diff --git a/ui/dr-ui/ui/identity.slint b/ui/dr-ui/ui/identity.slint index e2d77bd..3465417 100644 --- a/ui/dr-ui/ui/identity.slint +++ b/ui/dr-ui/ui/identity.slint @@ -32,6 +32,9 @@ export struct IdentityPerson { suggested-faces: int, cover: image, has-cover: bool, + // Set aside by the user: correctly found, correctly grouped, and not + // someone they want to identify. Hidden from the rail unless they ask. + ignored: bool, } export struct IdentityFace { @@ -61,10 +64,21 @@ component FaceCell inherits Rectangle { callback reject(); callback toggle-pick(); - property edge: root.compact ? 96px : 128px; + /// The drawn size, set by the grid that lays these out. + /// + /// An `in` property with a default rather than a derived one: the grid + /// divides its width into whole columns and tells each cell what it came + /// to, exactly as the library grid does. The default is what a caller that + /// does not care still gets. + in property edge: root.compact ? 96px : 128px; + + /// Height of the caption strip under the crop. Named because the grid has + /// to add it to the row pitch, and a grid using a different number from + /// the cell is a grid whose rows creep. + out property caption-height: 34px; width: edge; - height: edge + 34px; + height: edge + caption-height; background: face.picked ? Theme.selected : transparent; border-radius: Theme.radius; border-width: face.picked ? 1px : 0; @@ -148,8 +162,18 @@ export component IdentityScreen inherits Rectangle { in property <[IdentityPerson]> people; in property <[IdentityFace]> faces; + /// TRACES: FR-UI-1 + /// The narrow layout. Follows window width, not device type — a narrow + /// desktop window gets it exactly as a phone does. Set from Rust beside + /// `layout-class`, for the reason app.slint gives: a width read inside the + /// layout that also feeds it is a binding loop. + in property compact: false; in property selected-person: -1; in property selected-name; + /// Whether the selected person is set aside, so one button can be both + /// "Not interested" and "Bring back" — two buttons where only one is ever + /// applicable is a row that teaches the user to ignore half of it. + in property selected-ignored: false; // Faces belonging to nobody. Shown as a count rather than hidden, because // "why is this photograph not under anyone" deserves an answer. in property unassigned: 0; @@ -206,6 +230,18 @@ export component IdentityScreen inherits Rectangle { /// cluster before. It also cannot be `changed selected-person`, since a /// merge can land the user back on the person they were already on. in property name-revision: 0; + /// Regrouping is running. It is off the UI thread, so the screen stays + /// live and has to say what it is doing rather than simply stopping. + in property regrouping: false; + in property regroup-status; + /// People the user has set aside, and whether the rail is showing them. + in property ignored-count: 0; + in property show-ignored: false; + callback toggle-show-ignored(); + /// Set a person aside, or bring them back. + callback ignore-person(int, bool); + /// Show every photograph this person appears in, in the library grid. + callback show-photos(int); callback recluster(); callback index-faces(); callback stop-indexing(); @@ -225,7 +261,11 @@ export component IdentityScreen inherits Rectangle { HorizontalLayout { // ── the people rail ─────────────────────────────────────────────── Panel { - width: 260px; + // 260px of a 360px phone is the screen. The rail still has to + // carry a portrait and two lines of text, so it narrows rather + // than disappearing — 132px keeps the 40px cover and elides the + // labels, and leaves the faces grid enough width for two columns. + width: root.compact ? 132px : 260px; VerticalLayout { padding: Theme.gap; @@ -260,7 +300,12 @@ export component IdentityScreen inherits Rectangle { alignment: start; for p[i] in root.people: Rectangle { - height: 52px; + // Height and visibility rather than filtering the + // model in Rust: the rail is rebuilt on every + // action, and re-deriving a second filtered list + // per rebuild is work the toggle can do for free. + visible: root.show-ignored || !p.ignored; + height: (root.show-ignored || !p.ignored) ? 52px : 0px; border-radius: Theme.radius; background: p.id == root.selected-person ? Theme.selected @@ -313,6 +358,17 @@ export component IdentityScreen inherits Rectangle { Rectangle { } + // What the "not interested" button has hidden, and the way + // back to it. A count with no way to reveal it would make the + // action feel like a deletion, which is exactly what it is + // not. + if root.ignored-count > 0: Button { + text: root.show-ignored + ? "Hide " + root.ignored-count + " set aside" + : "Show " + root.ignored-count + " set aside"; + clicked => { root.toggle-show-ignored(); } + } + // The NFR-SEC-5 control lives here rather than three levels // down in settings: this is where the user's face data // visibly is, and a delete-everything button they cannot find @@ -330,6 +386,16 @@ export component IdentityScreen inherits Rectangle { spacing: Theme.gap-sm; // Header: who is selected, and what can be done to them. + // + // Two rows rather than one. The action row has grown — show + // photos, set aside, confirm, split, index, regroup — and a + // 240px name field beside five buttons does not fit a phone. Worse + // than not fitting: **a layout cannot be narrower than its + // children's minimums**, so an overflowing row reports that + // oversized minimum upwards and inflates the whole screen, which + // is the fault library.slint's filter bar documents at length. The + // faces grid is a sibling of this row, so it would have been laid + // out against a width that was never on the screen. HorizontalLayout { spacing: Theme.gap-sm; height: 32px; @@ -344,7 +410,11 @@ export component IdentityScreen inherits Rectangle { name-field := Field { text: root.selected-name; placeholder: "Name this person"; - width: root.selected-person >= 0 ? 240px : 0px; + // Capped by what there is, so a narrow window shortens the + // field instead of pushing the row past the edge. + width: root.selected-person >= 0 + ? min(240px, root.width - 2 * Theme.gap) + : 0px; visible: root.selected-person >= 0; edited(t) => { root.name-edited(t); } accepted(t) => { root.rename(t); } @@ -357,6 +427,25 @@ export component IdentityScreen inherits Rectangle { } Rectangle { } + } + + // The actions, in a strip that scrolls rather than overflowing. + // + // A Flickable's own minimum is nothing — it is built to be smaller + // than what it holds — so wrapping the row stops it inflating the + // screen and makes the buttons past the edge reachable instead of + // merely absent. Same device as the library's filter chips, for + // the same reason. + Flickable { + height: 36px; + viewport-height: self.height; + viewport-width: max(self.width, actions.preferred-width); + + actions := HorizontalLayout { + width: parent.viewport-width; + height: parent.viewport-height; + spacing: Theme.gap-sm; + alignment: start; if root.picked-count > 0: Text { text: root.picked-count + " selected"; @@ -385,10 +474,28 @@ export component IdentityScreen inherits Rectangle { text: "Stop"; clicked => { root.stop-indexing(); } } + // The way back from a face to the photographs. This is the + // point of having identified anybody, and without it the + // screen is a filing cabinet with no drawer handles. + if root.selected-person >= 0: Button { + text: "Show photos"; + clicked => { root.show-photos(root.selected-person); } + } + // Most clusters in a real library are strangers — passers-by, + // other people's guests, a face on a poster. Naming them is + // not the job and neither is looking at them again. + if root.selected-person >= 0: Button { + text: root.selected-ignored ? "Bring back" : "Not interested"; + clicked => { + root.ignore-person(root.selected-person, !root.selected-ignored); + } + } Button { - text: "Regroup"; + text: root.regrouping ? "Regrouping…" : "Regroup"; + enabled: !root.regrouping; clicked => { root.recluster(); } } + } } // The namesake offer. Sits directly under the name field that @@ -475,23 +582,64 @@ export component IdentityScreen inherits Rectangle { font-size: Theme.text-sm; } - Flickable { - VerticalLayout { - alignment: start; - HorizontalLayout { - // Slint has no flow layout, so the grid is a wrapping - // row built from the model's own order; the cells are - // fixed-size, which is what makes that tractable. - spacing: Theme.gap-sm; - alignment: start; + if root.regroup-status != "": Text { + text: root.regroup-status; + color: Theme.ink-dim; + font-size: Theme.text-sm; + wrap: word-wrap; + } - for f[i] in root.faces: FaceCell { - face: f; - confirm => { root.confirm-face(f.id); } - reject => { root.reject-face(f.id); } - toggle-pick => { root.toggle-pick(f.id); } - } - } + // --- the faces, as a grid ------------------------------------ + // + // This was one `HorizontalLayout` holding every face. The comment + // on it claimed to be a wrapping row; Slint has no flow layout and + // a HorizontalLayout does not wrap, so what it actually drew was a + // single row running off the right-hand edge with everything past + // the fourth or fifth face unreachable. A person with forty faces + // was a person whose faces could not be reviewed. + // + // Laid out the way the library grid lays out thumbnails, for the + // same reasons and with the same arithmetic: choose how many + // columns of roughly the requested size fit, then divide the width + // between them so the cells fill the row exactly and nothing + // overhangs. Cells are placed absolutely inside the Flickable, + // which is what makes the wrap possible at all. + faces-area := Flickable { + /// About this big. A request, not a measurement — `cell` is + /// what is actually drawn. + property requested: root.compact ? 96px : 128px; + + /// Rounded rather than floored, so a width nine tenths of the + /// way to another column takes it instead of stranding it. + property columns: + max(1, round((self.width - Theme.gap) + / (self.requested + Theme.gap))); + + /// The cells share the width exactly: `columns` of them and + /// the `columns + 1` gaps around them come to the full width, + /// so there is no remainder left as a dead strip down the + /// edge — which on a phone is a quarter of the screen. + property cell: + max(64px, (self.width - Theme.gap * (self.columns + 1)) / self.columns); + + /// The pitch between rows. The caption under each crop is part + /// of the cell, so it is part of the pitch. + property row-pitch: self.cell + 34px + Theme.gap; + property rows: ceil(root.faces.length / max(1, self.columns)); + + // Horizontal extent is exactly the viewport: the grid wraps, + // so there is nothing to scroll sideways to. + viewport-width: self.width; + viewport-height: Theme.gap + self.rows * self.row-pitch; + + for f[i] in root.faces: FaceCell { + x: Theme.gap + mod(i, faces-area.columns) * (faces-area.cell + Theme.gap); + y: Theme.gap + floor(i / faces-area.columns) * faces-area.row-pitch; + edge: faces-area.cell; + face: f; + confirm => { root.confirm-face(f.id); } + reject => { root.reject-face(f.id); } + toggle-pick => { root.toggle-pick(f.id); } } } } diff --git a/ui/dr-ui/ui/library.slint b/ui/dr-ui/ui/library.slint index 0b8c1fa..be84676 100644 --- a/ui/dr-ui/ui/library.slint +++ b/ui/dr-ui/ui/library.slint @@ -1253,6 +1253,10 @@ export component LibraryGrid inherits Rectangle { /// grid reads as broken. in property <[int]> rating-counts; + /// Whose photographs the grid is narrowed to, for the chip that says so + /// and clears it. Empty when no person filter is applied. + in property filter-person-name; + callback filter-person-cleared(); callback filter-min-rating-changed(int); callback filter-unjudged-toggled(bool); callback filter-flag-changed(int); @@ -1813,7 +1817,7 @@ export component LibraryGrid inherits Rectangle { // Hidden while there is nothing to filter: an empty library offering // six rating buttons is chrome describing data that does not exist. if root.total > 0 || root.filter-min-rating > 0 || root.filter-unjudged - || root.filter-flag > 0: Rectangle { + || root.filter-flag > 0 || root.filter-person-name != "": Rectangle { height: 34px; background: Theme.surface; @@ -1850,6 +1854,19 @@ export component LibraryGrid inherits Rectangle { spacing: 4px; alignment: start; + // First on the bar, ahead of "Show". Arriving here from the + // People screen replaces the whole grid, and a chip explaining + // that has to be the first thing read — a user who does not + // find it is looking at a library that has apparently lost + // most of its photographs. + if root.filter-person-name != "": FilterChip { + icon: "cross"; + label: root.filter-person-name; + active: true; + y: (parent.height - self.height) / 2; + clicked => { root.filter-person-cleared(); } + } + Caption { text: "Show"; vertical-alignment: center;