From d7f868fc6dea8f6bf52ccbe16ef146d571ce895f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:00:15 +0000 Subject: [PATCH] LR-EDIT-DETAIL-DENOISE: dismissing the model setup turns AI Denoise back off (#922) Turning on AI Denoise without a model opens Settings > AI Denoise and keeps a pending request, so the switch showed checked and Settings offered "Cancel pending action". Closing the setup with no model download or installation running now drops that request: the switch goes back off and nothing claims to be pending. A download started before closing still turns AI Denoise on when it finishes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01BKWB9TCnvzJHV6xMLAj2J4 --- crates/ui-egui/src/model_setup.rs | 20 ++++++++++++++++++++ crates/ui-egui/src/tests_model_setup.rs | 21 +++++++++++++++++++++ docs/parity-checklist.md | 2 +- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/model_setup.rs b/crates/ui-egui/src/model_setup.rs index e7094954..bb088eaa 100644 --- a/crates/ui-egui/src/model_setup.rs +++ b/crates/ui-egui/src/model_setup.rs @@ -104,10 +104,30 @@ pub(crate) fn notice(app: &mut LightcraftApp, ui: &mut egui::Ui, kind: &str) { ui.add_space(6.0); } +/// The model setup was dismissed with nothing on the way: neither Settings nor a model's terms +/// are showing, no model download or installation is running, and the photo still has no model. +fn abandoned(app: &mut LightcraftApp) -> bool { + if matches!(app.ui.dialog, Some(Dialog::Settings { .. } | Dialog::DenoiseModel { .. })) { + return false; + } + let Some(photo) = app.model_setup.denoise.as_ref().map(|r| r.photo) else { return false }; + if !matches!(app.session.denoise_photo_state(photo), PhotoState::NoModel) { + return false; + } + let Ok(v) = app.session.execute("denoise.models.downloads", &json!({})) else { return false }; + !v["downloads"].as_array().into_iter().flatten().any(|d| matches!(d["state"].as_str(), Some("running" | "done" | "installing" | "installed"))) +} + pub(crate) fn pump(app: &mut LightcraftApp, ctx: &egui::Context) { if app.model_setup.denoise.is_none() { return; } + if abandoned(app) { + // the setup was closed without installing anything: the switch goes back off (issue #922) + app.model_setup.denoise = None; + app.ui.status = tr("Pending photo action cancelled").into(); + return; + } if app.session.interaction.is_some() { ctx.request_repaint_after(std::time::Duration::from_millis(250)); return; diff --git a/crates/ui-egui/src/tests_model_setup.rs b/crates/ui-egui/src/tests_model_setup.rs index 85a7ca7f..3068d822 100644 --- a/crates/ui-egui/src/tests_model_setup.rs +++ b/crates/ui-egui/src/tests_model_setup.rs @@ -200,3 +200,24 @@ fn model_installation_waits_for_an_active_slider_gesture_to_finish() { drop(app); let _ = std::fs::remove_dir_all(dir); } + +/// Issue #922: closing the setup without installing a model turns the AI Denoise switch back off +/// and drops the "Cancel pending action" offer; while the setup stays open the request waits. +#[test] +fn dismissing_the_setup_without_a_model_leaves_ai_denoise_off() { + let (mut app, dir, raw) = setup("dismissed"); + let r = app.run("denoise.toggle", json!({"id": raw.0, "enabled": true})).unwrap(); + assert_eq!(r["setupRequired"], true); + resume(&mut app); + assert!(model_setup::denoise_requested(&app, raw), "the open setup keeps the request"); + app.ui.dialog = None; + resume(&mut app); + assert!(!model_setup::denoise_requested(&app, raw), "a dismissed setup leaves nothing pending"); + assert!(app.ui.status.contains("cancelled"), "{}", app.ui.status); + // installing later changes nothing; asking again offers the setup again + install_denoise(&mut app, &dir); + resume(&mut app); + assert_eq!(amount(&app, raw), 0.0); + drop(app); + let _ = std::fs::remove_dir_all(dir); +} diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 09941108..d5667afb 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -205,7 +205,7 @@ differences and missing bindings are in [ui-parity.md](ui-parity.md#shortcuts). | LR-EDIT-DETAIL-SHARPEN | Sharpening | P0 | ✅ | `ctl:detail.sharpenAmount`, `ctl:detail.sharpenRadius`, `ctl:detail.sharpenDetail`, `ctl:detail.sharpenMasking` | no Alt-drag mask preview | | LR-EDIT-DETAIL-NR | Luminance noise reduction | P0 | ✅ | `ctl:detail.nrLuminance`, `ctl:detail.nrDetail`, `ctl:detail.nrContrast` | | | LR-EDIT-DETAIL-CNR | Colour noise reduction | P0 | ✅ | `ctl:detail.nrColor`, `ctl:detail.nrColorDetail`, `ctl:detail.nrColorSmoothness` | | -| LR-EDIT-DETAIL-DENOISE | AI denoise | P2 | 🟡 | cmd:denoise.toggle, cmd:denoise.models.install, cmd:denoise.queue, ctl:enhance.denoise, `crates/denoise/`, `docs/denoise.md` | non-destructive Bayer RAW, opt-in weights, pure-Rust CPU/GPU; GPU convolution uses independently written shared scalars (Metal equivalence regression, issue #479); X-Trans/linear RGB, model policy and Lightroom fidelity remain | +| LR-EDIT-DETAIL-DENOISE | AI denoise | P2 | 🟡 | cmd:denoise.toggle, cmd:denoise.models.install, cmd:denoise.queue, ctl:enhance.denoise, `crates/denoise/`, `docs/denoise.md` | non-destructive Bayer RAW, opt-in weights, pure-Rust CPU/GPU; GPU convolution uses independently written shared scalars (Metal equivalence regression, issue #479); X-Trans/linear RGB, model policy and Lightroom fidelity remain; closing the model setup without installing turns the switch back off (#922) | | LR-EDIT-DETAIL-RAWDETAILS | Improved demosaic toggle | P2 | ⬜ | | | | LR-EDIT-DETAIL-SUPERRES | Super resolution | P2 | ⬜ | | | | LR-EDIT-DETAIL-AISHARPEN | AI sharpen | OOS | 🚫 | | |