Merge PR #698

crates/engine/src/cmd/face_models.rs

@@ -464,6 +464,8 @@ fn remove(s: &mut Session, p: &Value) -> Result<Value> {
     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<Value> {
     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"))]

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

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

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

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:
 

docs/parity-checklist.md

@@ -426,7 +426,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts).
 | 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 faces; 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 | ⬜ | | |