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 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); + } } 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);