From 8a60e3cd30e35cd1255f1ab1f862c3693e47a886 Mon Sep 17 00:00:00 2001 From: Jason Baldwin <33934381+bitshiftza@users.noreply.github.com> Date: Sat, 10 Oct 2026 11:51:56 +0200 Subject: [PATCH] LR-PREF-PEOPLE: Recognise faces stays off until a recognition model is chosen (#678) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings ▸ Faces let the switch be turned on with no recognition model chosen or installed: the box ticked, Settings closed without a word, and recognition was on with nothing to run. Now the box is enabled only with a chosen, installed recogniser (and a build that can run models), says "Choose a model below (Use) to start." on hover, and that hint stays under it on or off; a box left on by an older build can still be switched off. faces.enable {enabled: true} refuses without such a model, so the control channel and MCP cannot reach that state either, and removing the last recogniser switches recognition off instead of leaving it on with nothing chosen. Installing a model still chooses it and switches recognition on. Tests: tests_face_models::recognition_cannot_be_turned_on_without_a_chosen_model and tests_panels::recognise_faces_stays_off_until_a_model_is_chosen (fails on main at the first click). Docs: faces.md, parity LR-PREF-PEOPLE. --- crates/engine/src/cmd/face_models.rs | 12 ++++++- crates/engine/src/tests_face_models.rs | 29 ++++++++++++++++ crates/ui-egui/src/panels/faces.rs | 20 ++++++++--- crates/ui-egui/src/tests_panels.rs | 47 ++++++++++++++++++++++++++ docs/faces.md | 3 +- docs/parity.md | 2 +- 6 files changed, 105 insertions(+), 8 deletions(-) diff --git a/crates/engine/src/cmd/face_models.rs b/crates/engine/src/cmd/face_models.rs index 3a8e401dd..6033c4dcc 100644 --- a/crates/engine/src/cmd/face_models.rs +++ b/crates/engine/src/cmd/face_models.rs @@ -464,6 +464,8 @@ fn remove(s: &mut Session, p: &Value) -> Result { if st.embedder.as_deref() == Some(id) { // another installed recogniser takes over, so recognition keeps working; with none left nothing is chosen st.embedder = installed_models(&dir).into_iter().find(|i| i.manifest.id != id && i.manifest.role == Role::Embedder).map(|i| i.manifest.id); + // on with nothing to run is not a state (issue #678) + st.enabled &= st.embedder.is_some(); write_settings(&dir, &st)?; } Ok(json!({"removed": id, "embedder": st.embedder})) @@ -491,7 +493,15 @@ fn enable(s: &mut Session, p: &Value) -> Result { const C: &str = "faces.enable"; let dir = models_dir(s, C)?; let mut st = read_settings(&dir); - st.enabled = p.get("enabled").and_then(Value::as_bool).unwrap_or(!st.enabled); + let on = p.get("enabled").and_then(Value::as_bool).unwrap_or(!st.enabled); + // on means something only with a model to run (issue #678): a toggle that is on with nothing + // chosen would sit there doing nothing, with no word about why + let usable = + st.embedder.as_deref().is_some_and(|id| installed_models(&dir).iter().any(|i| i.manifest.id == id && i.manifest.role == Role::Embedder)); + if on && !usable { + return Err(bad(C, "choose a recognition model first (Settings ▸ Faces ▸ Use)")); + } + st.enabled = on; write_settings(&dir, &st)?; // the background scan notices at once, not within the second it trusts its last look #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/engine/src/tests_face_models.rs b/crates/engine/src/tests_face_models.rs index c71f7afeb..f1c27c849 100644 --- a/crates/engine/src/tests_face_models.rs +++ b/crates/engine/src/tests_face_models.rs @@ -584,3 +584,32 @@ fn the_users_own_catalog_adds_models_with_the_same_download_and_install() { } let _ = std::fs::remove_dir_all(&d); } + +/// Issue #678: on means something only with a model to run. Turning recognition on with no chosen +/// (installed) recogniser is refused; installing one chooses it and switches recognition on, after +/// which the switch works both ways; removing the last recogniser switches it off again. +#[test] +fn recognition_cannot_be_turned_on_without_a_chosen_model() { + let d = temp("enable-needs-model"); + let mut s = session(&d); + for p in [json!({"enabled": true}), json!({})] { + let e = s.execute("faces.enable", &p).unwrap_err().to_string(); + assert!(e.contains("recognition model"), "{p}: {e}"); + } + assert_eq!(s.execute("faces.enable", &json!({"enabled": false})).unwrap()["enabled"], false, "off is always allowed"); + assert_eq!(s.execute("faces.models.list", &json!({})).unwrap()["enabled"], false); + let f = model_file(&d, "m.onnx", 64); + let r = s.execute("faces.models.install", &json!({"path": f, "acknowledged": true})).unwrap(); + let id = r["installed"]["id"].as_str().unwrap().to_string(); + assert_eq!(r["active"]["enabled"], true, "installing chooses the model and switches recognition on: {r}"); + assert_eq!(s.execute("faces.enable", &json!({"enabled": false})).unwrap()["enabled"], false); + assert_eq!(s.execute("faces.enable", &json!({"enabled": true})).unwrap()["enabled"], true); + assert_eq!(s.execute("faces.enable", &json!({})).unwrap()["enabled"], false, "toggles when omitted"); + assert_eq!(s.execute("faces.enable", &json!({})).unwrap()["enabled"], true); + // the last recogniser goes: nothing is chosen, so recognition is off and stays off + assert_eq!(s.execute("faces.models.remove", &json!({"id": id})).unwrap()["embedder"], Value::Null); + let l = s.execute("faces.models.list", &json!({})).unwrap(); + assert_eq!((l["enabled"].clone(), l["embedder"].clone()), (json!(false), Value::Null), "{l}"); + assert!(s.execute("faces.enable", &json!({"enabled": true})).is_err()); + let _ = std::fs::remove_dir_all(&d); +} diff --git a/crates/ui-egui/src/panels/faces.rs b/crates/ui-egui/src/panels/faces.rs index 13b4b1329..2b3fe0329 100644 --- a/crates/ui-egui/src/panels/faces.rs +++ b/crates/ui-egui/src/panels/faces.rs @@ -205,15 +205,25 @@ pub fn settings_tab(app: &mut LightcraftApp, ui: &mut egui::Ui, t: &Tokens) { return; } let mut on = list["enabled"].as_bool().unwrap_or(false); - if check(ui, "settings.facesEnabled", &mut on, "Recognise faces (experimental)") { - let _ = app.run("faces.enable", json!({"enabled": on})); - app.caches.faces_epoch += 1; + let runtime = list["runtime"].as_bool() == Some(true); + // a chosen recognition model that is installed: without one the switch has nothing to run + // (issue #678), so it can only be switched off, with the hint below saying what to do first + let usable = + list["embedder"].as_str().is_some_and(|id| list["models"].as_array().into_iter().flatten().any(|m| m["id"] == id && m["installed"] == true)); + let r = ui + .add_enabled(on || (runtime && usable), egui::Checkbox::new(&mut on, crate::i18n::tr("Recognise faces (experimental)"))) + .on_disabled_hover_text(crate::i18n::tr("Choose a model below (Use) to start.")); + register(ui.ctx(), "check:settings.facesEnabled", r.rect); + if r.changed() { + match app.run("faces.enable", json!({"enabled": on})) { + Ok(_) => app.caches.faces_epoch += 1, + Err(e) => app.toast(ui.ctx(), e), + } } hint(ui, t, "Suggests who is in a photo from the faces you have named. Everything stays on your computer."); - let runtime = list["runtime"].as_bool() == Some(true); if !runtime { hint(ui, t, "This build cannot run recognition models: they can be added and chosen, not used."); - } else if on && list["embedder"].is_null() { + } else if !usable { hint(ui, t, "Choose a model below (Use) to start."); } else if on && app.caches.faces_active { scan_progress(app, ui, t); diff --git a/crates/ui-egui/src/tests_panels.rs b/crates/ui-egui/src/tests_panels.rs index 1f5cd4d46..af1173d68 100644 --- a/crates/ui-egui/src/tests_panels.rs +++ b/crates/ui-egui/src/tests_panels.rs @@ -1182,3 +1182,50 @@ fn the_tracklog_toast_shows_after_a_while() { assert!(h.app.ui.toast.as_ref().is_some_and(|t| t.0.starts_with("Tagged")), "{:?}", h.app.ui.toast); let _ = std::fs::remove_dir_all(&dir); } + +/// Issue #678: Settings ▸ Faces cannot switch recognition on until a recognition model is chosen: +/// the box is off and clicking it changes nothing. Installing a model switches recognition on by +/// itself, and from then on the box turns it off and on. +#[test] +fn recognise_faces_stays_off_until_a_model_is_chosen() { + let mut h = demo([1400.0, 900.0], json!({"view": "photoGrid"})); + let dir = std::env::temp_dir().join(format!("lc-ui-faces-toggle-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + h.app.session.face_models_dir = Some(dir.join("models")); + let r = h.request("engine.execute", json!({"command": "app.settings", "params": {"tab": "faces"}}), T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(SETTLE); + // (the first request switches the widget list on, the next frame fills it) + h.request("ui.widgets", json!({}), T); + h.step(); + let enabled = |h: &mut Headless| h.app.session.execute("faces.models.list", &json!({})).unwrap()["enabled"] == true; + let runtime = h.app.session.execute("faces.models.list", &json!({})).unwrap()["runtime"] == true; + assert!(!enabled(&mut h)); + let click = |h: &mut Headless| { + let r = h.request("ui.clickWidget", json!({"id": "check:settings.facesEnabled"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + h.step(); + }; + click(&mut h); + assert!(!enabled(&mut h), "nothing to run: the box stays off"); + assert!(h.app.session.execute("faces.enable", &json!({"enabled": true})).is_err(), "the engine refuses too"); + if !runtime { + let _ = std::fs::remove_dir_all(&dir); + return; + } + // a model: installing it chooses it and switches recognition on; the box then works both ways + let f = dir.join("m.onnx"); + std::fs::write(&f, lightcraft_faces::synthetic::tiny_embedder_model(64)).unwrap(); + h.app.session.execute("faces.models.install", &json!({"path": f.to_string_lossy(), "acknowledged": true})).unwrap(); + h.app.caches.faces_epoch += 1; + h.step(); + h.step(); + assert!(enabled(&mut h)); + click(&mut h); + assert!(!enabled(&mut h), "the box switches recognition off"); + click(&mut h); + assert!(enabled(&mut h), "and on again, with a model chosen"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/docs/faces.md b/docs/faces.md index 996c82ca5..7adc1fcaa 100644 --- a/docs/faces.md +++ b/docs/faces.md @@ -200,7 +200,8 @@ photos in the set. ## Suggesting who is in a photo -With **Recognise faces** switched on in Settings ▸ Faces and a recognition model chosen (**Use**), LightCraft works out, in +With **Recognise faces** switched on in Settings ▸ Faces and a recognition model chosen (**Use**; the switch cannot be +turned on before one is, and turns itself off when the last recogniser is removed), LightCraft works out, in the background, what each face in your library looks like to the model, and uses the faces you have already named to suggest names for the ones you have not: diff --git a/docs/parity.md b/docs/parity.md index d0bdc35c2..44d37a01a 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -492,7 +492,7 @@ under the UI command that wraps it (a no-op error); the UI command now wins. | LR-PREF-INTERFACE | Interface options | P1 | ✅ | `cmd:app.settings` | filmstrip names/badges, grid badges (auto/always/never), square-grid names, navigator, info overlay | | LR-PREF-DISPLAY | Monitor (display) colour management | P1 | 🟡 | `cmd:app.displayProfile`, `crates/codecs/src/display.rs`, `crates/engine/src/display.rs`, `crates/ui-egui/src/panels/settings.rs`, `docs/display-profiles.md` | Settings ▸ Display: a monitor ICC profile (matrix/TRC exactly, LUT-based through the CMS) chosen by hand; the loupe, Before, Compare, hover previews and the second window render into the display's primaries (its full gamut, CPU and GPU), thumbnails and stand-ins are converted from sRGB; histogram, preview caches and exports stay sRGB; soft proofing's display gamut warning uses the display's gamut. Missing: picking up the profile the OS assigns (X11 `_ICC_PROFILE`, colord, ColorSync, WCS), per-monitor profiles | | LR-PREF-PERFORMANCE | GPU / performance | P1 | ✅ | `cmd:app.gpu`, `cmd:app.memoryBudget`, `cmd:app.settings` | GPU on/off, preview size (Automatic = the whole-frame loupe render at canvas size, up to 2560 px; or a 1600–5120 px size for it; zoomed views add a window at up to 100 %, issue #323), memory budget (a quarter of it is the idle per-view stage caches' share; the open photo's views are counted and reported, and cut only past four times that, the before side's and the zoom window's first — a Before/After at 1:1 on a 2800 px canvas holds about 1 GB of GPU stages, plus the one 288 MB copy of the original, which `ui.inspect` reports as `sharedSourceBytes`), thumbnail cache size in Settings | -| LR-PREF-PEOPLE | Face recognition | P2 | 🟡 | `cmd:faces.enable`, `cmd:faces.models.list`, `cmd:faces.models.inspect`, `cmd:faces.models.install`, `cmd:faces.models.download`, `cmd:faces.models.downloads`, `cmd:faces.models.downloadCancel`, `cmd:faces.models.remove`, `cmd:faces.models.select`, `cmd:dialog.faceModel`, `crates/faces/src/lib.rs`, `crates/ui-egui/src/panels/faces.rs` | Settings ▸ Faces: an off-by-default toggle and the face model list. Models are opt-in and never bundled, the 232 KB YuNet detector included. Press Download (terms first; then a pinned address, size and SHA-256 checked, fetched over pure-Rust https with `lightcraft-fetch`), or add any `.onnx` by picking or dropping it: it is recognised by hash or described from its shape, shows its licence terms and training-data note, runs a self-test, and installs only after an "I accept" box; an installed model is chosen and recognition switched on at once (the newest wins, each model keeps its own embedding cache). Non-commercial models are add-by-file only, never offered for download. Suggestions of names run on the shared pure-Rust CPU interpreter (`crates/faces/src/net.rs`, `crates/faces/src/runtime.rs`; bounded im2col/GEMM, checked ONNX shapes/operators; no external inference runtime; 40 public crops agree within 2.1e-6, one-thread SFace 34–36 ms / AuraFace 233–252 ms, see docs/faces.md) in a background worker and are never applied automatically. Thresholds are starting points calibrated on busts, not yet on live faces +| LR-PREF-PEOPLE | Face recognition | P2 | 🟡 | `cmd:faces.enable`, `cmd:faces.models.list`, `cmd:faces.models.inspect`, `cmd:faces.models.install`, `cmd:faces.models.download`, `cmd:faces.models.downloads`, `cmd:faces.models.downloadCancel`, `cmd:faces.models.remove`, `cmd:faces.models.select`, `cmd:dialog.faceModel`, `crates/faces/src/lib.rs`, `crates/ui-egui/src/panels/faces.rs` | Settings ▸ Faces: an off-by-default toggle and the face model list. Models are opt-in and never bundled, the 232 KB YuNet detector included. Press Download (terms first; then a pinned address, size and SHA-256 checked, fetched over pure-Rust https with `lightcraft-fetch`), or add any `.onnx` by picking or dropping it: it is recognised by hash or described from its shape, shows its licence terms and training-data note, runs a self-test, and installs only after an "I accept" box; an installed model is chosen and recognition switched on at once (the newest wins, each model keeps its own embedding cache). Non-commercial models are add-by-file only, never offered for download. Suggestions of names run on the shared pure-Rust CPU interpreter (`crates/faces/src/net.rs`, `crates/faces/src/runtime.rs`; bounded im2col/GEMM, checked ONNX shapes/operators; no external inference runtime; 40 public crops agree within 2.1e-6, one-thread SFace 34–36 ms / AuraFace 233–252 ms, see docs/faces.md) in a background worker and are never applied automatically. Thresholds are starting points calibrated on busts, not yet on live face; the Recognise faces switch is off until a recognition model is chosen and installed (`faces.enable {enabled: true}` refuses otherwise; removing the last recogniser switches it off), issue #678 | | LR-PREF-WATERMARK | Watermark settings | P1 | ✅ | `cmd:export.savePreset`, `crates/ui-egui/src/panels/dialogs.rs` | set in the Export dialog; kept with Export with Previous and in saved export presets | | LR-PREF-SHORTCUTS | Shortcut customisation | — | ✅ | `cmd:app.setShortcut`, `cmd:app.resetShortcuts`, `crates/ui-egui/src/panels/keymap.rs` | an extra (the reference app has no keymap editor): Help ▸ Keyboard Shortcuts (⌘/) lists every command; click a shortcut and press the new keys (Esc cancels), × removes, ↺ restores, Reset All; a key another command had moves over; saved in `ui.json`; native menu accelerators follow. Rating/label digits and secondary keys are listed read-only and yield to a key given to a command | | LR-PREF-TECHPREVIEW | Early-access toggles | P2 | ⬜ | | |