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;