mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-14 19:27:01 -04:00
LR-PREF-PEOPLE: Recognise faces stays off until a recognition model is chosen (#678)
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.
This commit is contained in:
1 parent
b70ae1741e
commit
8a60e3cd30
6 files changed
+105
-8
No files matched your search
@@ -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"))]
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
+2
-1
@@ -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:
|
||||
|
||||
|
||||
+1
-1
@@ -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 | ⬜ | | |
|
||||
|
||||
Reference in new issue
Block a user