From 1d4c3348af557bbaf38ef6965d43170f3229b96d Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 28 Aug 2026 08:30:09 +0200 Subject: [PATCH 1/3] Let a 32-pixel face count, and move the blur floor with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 64 source pixels was too strict: it threw away 70% of everything the detector finds, and plenty of what it took were faces a person could name. Lowering it is not a one-line change, because the two floors are coupled. A face under 112 pixels is *upsampled* to reach the embedder and upsampling invents no edges, so a small face scores low on sharpness however crisp the original was. Re-measured over the reference library with `face_index --quality`: min crop min sharp size cut blur cut kept 32 0.000 44% 0% 56% 32 0.002 44% 3% 53% 32 0.005 44% 8% 48% 32 0.010 44% 16% 40% 32 0.020 44% 27% 30% 64 0.020 70% 7% 23% Holding the blur floor at 0.020 while dropping the size floor to 32 would have rejected a further 27% — for being small rather than for being blurred — and kept only 30%, barely more than the 23% the strict pair kept. Most of the point of lowering the size floor would have gone straight back out through the other gate. 0.005 removes 8% of what the size floor leaves, which is the same job 0.020 was doing at 64 (7%): the large-but-soft face this gate exists for. Together they now keep 48% of what the detector finds, against 23% before. The box pre-filter follows down to 24, staying below what the real floor accepts so it cannot reject a face that would have cleared 32. Co-Authored-By: Claude Opus 5 (1M context) --- core/dr-face/src/detect.rs | 32 ++++++++++++++++++++++++++------ ui/dr-ui/examples/face_index.rs | 11 ++++++----- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/core/dr-face/src/detect.rs b/core/dr-face/src/detect.rs index e1d9881..e6727a9 100644 --- a/core/dr-face/src/detect.rs +++ b/core/dr-face/src/detect.rs @@ -46,13 +46,13 @@ pub struct DetectOptions { /// it is measured on the aligned crop rather than the box. This one exists /// only to throw away the obviously hopeless before paying for a warp, so /// it is deliberately set *below* what the real floor will accept: the - /// aligned crop spans roughly 1.3x the box's shorter edge, so 48 here - /// cannot reject a face that would have cleared 64 there. + /// aligned crop spans roughly 1.3x the box's shorter edge, so 24 here + /// cannot reject a face that would have cleared 32 there. pub min_face_px: f32, /// Smallest face the embedder may be given, in **source pixels across the /// aligned crop** — `crop_px` in the catalog. /// - /// The honest statement of "a face must be at least 64x64", because this is + /// The honest statement of "a face must be at least 32x32", because this is /// the number of real pixels behind the 112x112 the model actually sees. /// The box's own size is not that: the ArcFace template reaches past the /// box for forehead and chin, so a 64-pixel box and a 64-pixel crop are @@ -74,6 +74,26 @@ pub struct DetectOptions { /// and a caller cannot enable one gate while forgetting the other. /// /// Zero disables it, which is what a measurement run wants. + /// + /// # It has to move with the size floor + /// + /// The two are coupled, because an upsampled face scores low here whatever + /// its original sharpness. Measured over the reference library, with the + /// size floor at 32 source pixels: + /// + /// | min sharpness | of what the size floor left, this removes | + /// |---|---| + /// | 0.002 | 3% | + /// | 0.005 | 8% | + /// | 0.010 | 16% | + /// | 0.020 | 27% | + /// + /// At a 64-pixel floor, 0.020 removed 7% — the same *kind* of face, the + /// large-but-soft one this gate exists for. Holding 0.020 while dropping + /// the size floor to 32 would have thrown away a quarter of the newly + /// admitted faces for being small rather than for being blurred, undoing + /// most of the point of lowering it. 0.005 removes 8% at 32, which is the + /// same job. pub min_sharpness: f32, } @@ -82,9 +102,9 @@ impl Default for DetectOptions { Self { confidence: 0.5, nms_iou: 0.4, - min_face_px: 48.0, - min_source_px: 64.0, - min_sharpness: 0.020, + min_face_px: 24.0, + min_source_px: 32.0, + min_sharpness: 0.005, } } } diff --git a/ui/dr-ui/examples/face_index.rs b/ui/dr-ui/examples/face_index.rs index 1e9ac70..7c1f4a9 100644 --- a/ui/dr-ui/examples/face_index.rs +++ b/ui/dr-ui/examples/face_index.rs @@ -464,13 +464,14 @@ fn report_quality( println!("{}", "-".repeat(52)); for (min_px, min_sharp) in [ (0.0_f32, 0.0_f32), - (64.0, 0.0), - (0.0, 0.010), - (64.0, 0.005), + (32.0, 0.0), + (32.0, 0.002), + (32.0, 0.005), + (32.0, 0.010), + (32.0, 0.020), + (48.0, 0.005), (64.0, 0.010), (64.0, 0.020), - (80.0, 0.010), - (96.0, 0.010), ] { let by_size = found.iter().filter(|f| f.0 < min_px).count(); let by_blur = found From c81d6865a7103745bb6c9ef4b17b00b4f0befc7c Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 28 Aug 2026 08:30:15 +0200 Subject: [PATCH 2/3] Stop the name field running through the buttons under it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Identity header was two rows pinned to 32px and 36px. Neither number was big enough for what the row held: a `Field` is `Theme.touch-target` — 44px — and a `Button` is `Theme.control-height`. Slint honours a child's own height and lets it overflow the box the layout gave it, so the name field drew 44px from the top of a 32px row while the button strip began at 38px. The overlap was 6px of text box sitting on top of "Confirm all". Neither row states a height any more. The first takes the height of what is in it, and the strip takes the height of a control — read from the theme rather than from the row inside it, since `actions` sizes itself from the Flickable's viewport and measuring it back would be a binding loop. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/ui/identity.slint | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/ui/dr-ui/ui/identity.slint b/ui/dr-ui/ui/identity.slint index 4ca96bc..3bbbb60 100644 --- a/ui/dr-ui/ui/identity.slint +++ b/ui/dr-ui/ui/identity.slint @@ -403,9 +403,17 @@ export component IdentityScreen inherits Rectangle { // 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. + // + // **Neither row states a height.** The first version of this pinned + // them to 32px and 36px, which are smaller than what they contain: + // a `Field` is `Theme.touch-target` (44px) tall and a `Button` is + // `Theme.control-height`. Slint honours the child's own height and + // lets it overflow the box it was given, so the name field ran 12px + // past its row and straight through the buttons 6px below it. Rows + // take the height of what is in them; the strip takes the height of + // a control. HorizontalLayout { spacing: Theme.gap-sm; - height: 32px; // Naming a cluster *is* the primary action of this screen, so // the name is an editable field on sight rather than something @@ -444,7 +452,10 @@ export component IdentityScreen inherits Rectangle { // merely absent. Same device as the library's filter chips, for // the same reason. Flickable { - height: 36px; + // A control's height, not a guess, and not read back from the + // row inside — `actions` sizes itself from `viewport-height`, + // so measuring it here would be a binding loop. + height: Theme.control-height; viewport-height: self.height; viewport-width: max(self.width, actions.preferred-width); From 41daa4cca7661609d39cb8efb34baab07f93c1a8 Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Fri, 28 Aug 2026 08:30:16 +0200 Subject: [PATCH 3/3] Keep a group set aside actually set aside MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Not interested" hid a group, and the next Regroup brought it straight back. Reclustering anchors the faces the user has ruled on so a pass cannot move them. It took *confirmations* as the only kind of ruling — but setting a group aside is a ruling too, and the faces it covers are only ever suggestions. So an ignored group's faces entered clustering loose, regrouped into a fresh person that carried no ignore flag, and reappeared in the rail. The original group was left behind holding nothing, hidden and empty. Anchoring them on `ignored` as well as on `confirmed` fixes it, and does one better: a face indexed later that matches a group which was set aside now merges *into* it, so a stranger photographed again stays set aside instead of arriving as somebody new. That is the case that would otherwise have made the feature feel like it only half worked. Three tests, and the first fails without the change — it reports the group coming back with its two faces while the original sits ignored and empty. Co-Authored-By: Claude Opus 5 (1M context) --- ui/dr-ui/src/faces.rs | 151 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 150 insertions(+), 1 deletion(-) diff --git a/ui/dr-ui/src/faces.rs b/ui/dr-ui/src/faces.rs index b78e31b..25fd06b 100644 --- a/ui/dr-ui/src/faces.rs +++ b/ui/dr-ui/src/faces.rs @@ -505,9 +505,22 @@ pub fn recluster( } // Which faces the user has already ruled on, so they enter as anchors. + // + // Two kinds of ruling, and the second is easy to miss. A *confirmation* is + // the obvious one. But setting a person aside is a ruling too, and the + // faces it covers are only ever suggestions — so anchoring confirmations + // alone left every ignored group's faces loose, and the next Regroup + // scattered them into fresh unnamed groups that were not ignored. The + // strangers came straight back, which is the feature not working at all. + // + // Anchoring them keeps them where the user put them, and does one better: + // a newly indexed face similar to a group that was set aside merges *into* + // it, so a stranger photographed again stays set aside instead of + // reappearing as somebody new. let mut confirmed = std::collections::HashMap::new(); for p in faces::people(conn)? { - for f in faces::for_person(conn, p.id, false)? { + // Suggestions included exactly when the group was set aside. + for f in faces::for_person(conn, p.id, p.ignored)? { confirmed.insert(f.id, p.id); } } @@ -1055,4 +1068,140 @@ mod tests { assert!((rgb[2] - 0.0).abs() < 1e-6); assert!(rgb[3..6].iter().all(|&v| v == 0.0)); } + + // ── setting a group aside has to survive regrouping ─────────────────── + + const TEST_MODEL: &str = "w600k_mbf"; + + /// A catalog holding `n` images and nothing else. + fn catalog_with(n: usize) -> Catalog { + let c = Catalog::in_memory().unwrap(); + let conn = c.connection(); + conn.execute( + "INSERT OR IGNORE INTO roots(id, kind, label) VALUES (1, 'local', 'lib')", + [], + ) + .unwrap(); + for i in 0..n { + conn.execute( + &format!( + "INSERT INTO images(id, root_id, source_ref, added_at) + VALUES ({}, 1, 'IMG_{i}.CR3', 0)", + i + 1 + ), + [], + ) + .unwrap(); + } + c + } + + /// A unit embedding pointing at `identity`, `cosine` of the way there. + fn embedding(identity: usize, cosine: f32) -> Vec { + let mut v = Box::new([0.0_f32; dr_face::EMBEDDING_DIM]); + v[identity * 2] = cosine; + v[identity * 2 + 1] = (1.0 - cosine * cosine).max(0.0).sqrt(); + dr_face::Embedding { + model: ModelId::new(TEST_MODEL.to_string()), + v, + } + .to_f16_bytes() + } + + fn put_face(catalog: &Catalog, image: u64, identity: usize, cosine: f32) { + let f = DetectedFace { + x: 0.1, + y: 0.1, + w: 0.2, + h: 0.2, + landmarks: [(0.0, 0.0); 5], + confidence: 0.9, + embedding: embedding(identity, cosine), + crop_px: 150.0, + model_id: TEST_MODEL.to_string(), + crop: Vec::new(), + }; + faces::record_detections( + catalog.connection(), + ImageId(image), + TEST_MODEL, + 1024, + std::slice::from_ref(&f), + ) + .unwrap(); + } + + /// The bug this pins: "not interested" only ever covered *suggested* faces, + /// and reclustering anchored confirmations alone. So the next Regroup cut + /// the ignored group's faces loose, built fresh unnamed groups out of them, + /// and every stranger the user had dismissed came straight back. + #[test] + fn a_group_set_aside_does_not_come_back_on_the_next_regroup() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let people = faces::people(catalog.connection()).unwrap(); + assert_eq!(people.len(), 1, "the two faces should have grouped"); + let stranger = people[0].id; + assert_eq!(people[0].suggested_faces, 2); + + faces::set_ignored(catalog.connection(), stranger, true).unwrap(); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!( + after.len(), + 1, + "regrouping resurrected the group that was set aside: {after:?}" + ); + assert_eq!(after[0].id, stranger); + assert!(after[0].ignored, "the group stopped being set aside"); + assert_eq!( + after[0].suggested_faces, 2, + "the faces left the group they were set aside in" + ); + } + + /// And it holds as the library grows: a stranger photographed again joins + /// the group that was set aside rather than arriving as somebody new. + #[test] + fn a_new_face_joins_the_group_it_matches_even_when_that_group_is_set_aside() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let stranger = faces::people(catalog.connection()).unwrap()[0].id; + faces::set_ignored(catalog.connection(), stranger, true).unwrap(); + + // The same person turns up in a third photograph. + put_face(&catalog, 3, 0, 0.98); + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!(after.len(), 1, "a new face made a second group: {after:?}"); + assert!(after[0].ignored); + assert_eq!(after[0].suggested_faces, 3); + } + + /// The other half of the promise: bringing them back really does. + #[test] + fn bringing_a_group_back_makes_it_ordinary_again() { + let catalog = catalog_with(3); + put_face(&catalog, 1, 0, 1.0); + put_face(&catalog, 2, 0, 0.99); + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + + let id = faces::people(catalog.connection()).unwrap()[0].id; + faces::set_ignored(catalog.connection(), id, true).unwrap(); + faces::set_ignored(catalog.connection(), id, false).unwrap(); + + recluster(&catalog, TEST_MODEL, dr_face::DEFAULT_MERGE_PROBABILITY).unwrap(); + let after = faces::people(catalog.connection()).unwrap(); + assert_eq!(after.len(), 1); + assert!(!after[0].ignored); + } }