From aa52251d2ef3bb2e4014f20943c4c4c83ce0d931 Mon Sep 17 00:00:00 2001 From: eliets2 Date: Thu, 5 Nov 2026 23:12:18 +0200 Subject: [PATCH] Apply Auto settings to every selected photo (#3808) * Apply Auto settings to every selected photo Users expect Auto and preset application to affect the whole selection, as Quick Develop already does. `develop.auto` is the only edit command in that file that hard-codes the active photo instead of the selection. Make `develop.auto` accept the same target rule as the other selection commands (`ids` when given, else the selection), analyse each target photo's own source with `auto_tone`, and commit the results as one batched undo step labelled "Auto". Test: auto_applies_to_every_selected_photo failed before the change and passes after. * Run multi-photo Auto settings off the UI thread Assisted-by: OpenAI Codex * Auto Settings: skip unreadable photos and refuse stale results One offline photo stopped Auto for the whole selection. The background result is checked against each photo's settings, because an undo changes them without a commit, and it commits through the command wrapper so sidecars and the durable save follow. Co-Authored-By: Claude Opus 5.5 * Auto Settings: a wrong photo id is refused, not skipped Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: eliets2 <287273517+eliets2@users.noreply.github.com> Co-authored-by: FredApps Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/develop.rs | 63 ++++++++--- crates/engine/src/tests.rs | 33 ++++++ crates/ui-egui/src/auto_settings.rs | 163 ++++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/menus/run.rs | 3 + 5 files changed, 245 insertions(+), 18 deletions(-) create mode 100644 crates/ui-egui/src/auto_settings.rs diff --git a/crates/engine/src/cmd/develop.rs b/crates/engine/src/cmd/develop.rs index 42e2793daf..5399aee9b8 100644 --- a/crates/engine/src/cmd/develop.rs +++ b/crates/engine/src/cmd/develop.rs @@ -460,24 +460,51 @@ pub fn specs() -> Vec { Ok(json!({"mix": m})) } ), - cmd!("develop.auto", "Auto Settings", ["Photo"], Some("Shift+A"), "{}", has_active, |s, _| { - let id = active(s, "develop.auto")?; - let src = s.source_now(id, SourceLevel::Thumb).map_err(|e| bad("develop.auto", e))?; - let info = s.source_info(id); - let d = s.develop_of(id).unwrap_or_default(); - let a = lightcraft_pipeline::auto::auto_tone(&src, &info, &d); - edit(s, "develop.auto", "Auto", |d| { - d.light.exposure = a.exposure; - d.light.contrast = a.contrast; - d.light.highlights = a.highlights; - d.light.shadows = a.shadows; - d.light.whites = a.whites; - d.light.blacks = a.blacks; - d.color.vibrance = a.vibrance; - d.color.saturation = a.saturation; - Ok(()) - })?; - Ok(serde_json::to_value(a).unwrap_or_default()) + cmd!("develop.auto", "Auto Settings", ["Photo"], Some("Shift+A"), "{}", has_selection, |s, p| { + const C: &str = "develop.auto"; + let ids = s.targets(p)?; + // a wrong id is a bad request, unlike a photo that can't be read just now + if let Some(id) = ids.iter().find(|id| s.catalog.photo(**id).is_none()) { + return Err(bad(C, format!("no such photo: {}", id.0))); + } + let mut first: Option = None; + let mut ops: Vec = Vec::new(); + let mut failed = None; + for id in ids { + // a photo that can't be read (offline, no decoder) is left out, not the whole selection + let src = match s.source_now(id, SourceLevel::Thumb) { + Ok(src) => src, + Err(e) => { + failed.get_or_insert(e); + continue; + } + }; + let info = s.source_info(id); + let d = s.develop_of(id).unwrap_or_default(); + let a = lightcraft_pipeline::auto::auto_tone(&src, &info, &d); + if first.is_none() { + first = Some(serde_json::to_value(a).unwrap_or_default()); + } + let mut nd = (*d).clone(); + nd.light.exposure = a.exposure; + nd.light.contrast = a.contrast; + nd.light.highlights = a.highlights; + nd.light.shadows = a.shadows; + nd.light.whites = a.whites; + nd.light.blacks = a.blacks; + nd.color.vibrance = a.vibrance; + nd.color.saturation = a.saturation; + if let Some(op) = s.develop_op(id, nd, "Auto") { + ops.push(op); + } + } + if let (None, Some(e)) = (&first, failed) { + return Err(bad(C, e)); + } + if !ops.is_empty() { + s.commit("Auto", Op::Batch { ops })?; + } + Ok(first.unwrap_or_default()) }), cmd!( "develop.wb", diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 03592df41b..632814334d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -499,6 +499,39 @@ fn masks_crop_and_render() { s.execute("develop.wbPick", &json!({"x": 0.5, "y": 0.5})).unwrap(); } +/// Auto applies to every selected photo, not only the highlighted one (#2184). +#[test] +fn auto_applies_to_every_selected_photo() { + use lightcraft_catalog::PhotoId; + let mut s = demo(); + let ids: Vec = s.catalog.photos().take(3).map(|p| p.id.0).collect(); + assert_eq!(ids.len(), 3, "the demo library has enough photos to select"); + s.execute("library.select", &json!({"ids": ids.clone(), "active": ids[1]})).unwrap(); + s.execute("develop.auto", &json!({})).unwrap(); + for id in ids { + let d = (*s.develop_of(PhotoId(id)).unwrap()).clone(); + assert_eq!(d.light.contrast, 68.0, "photo {id} got Auto's contrast"); + assert_eq!(d.color.vibrance, 30.0, "photo {id} got Auto's vibrance"); + } +} + +/// A photo that can't be read is left out: the rest of the selection still gets Auto. +#[test] +fn auto_skips_a_selected_photo_it_cannot_read() { + use lightcraft_catalog::{Op, Photo, PhotoId, Source}; + let mut s = demo(); + let good = s.catalog.photos().next().unwrap().id; + let missing = s.catalog.alloc_photo_id(); + let photo = Photo::new(missing, Source::File { path: "/lightcraft-test/missing.jpg".into() }, "missing.jpg", "JPEG", 100, 100, ""); + s.catalog.apply(Op::AddPhoto { photo: Box::new(photo) }).unwrap(); + s.execute("library.select", &json!({"ids": [missing.0, good.0], "active": good.0})).unwrap(); + s.execute("develop.auto", &json!({})).unwrap(); + assert_eq!(s.develop_of(PhotoId(good.0)).unwrap().color.vibrance, 30.0, "the readable photo got Auto"); + s.execute("library.select", &json!({"ids": [missing.0], "active": missing.0})).unwrap(); + assert!(s.execute("develop.auto", &json!({})).is_err(), "with nothing readable the error is reported"); + assert!(s.execute("develop.auto", &json!({"ids": [good.0, 999_999]})).is_err(), "a wrong id is refused, not skipped"); +} + #[test] fn versions_history_and_reset() { let mut s = demo(); diff --git a/crates/ui-egui/src/auto_settings.rs b/crates/ui-egui/src/auto_settings.rs new file mode 100644 index 0000000000..47fbdfe478 --- /dev/null +++ b/crates/ui-egui/src/auto_settings.rs @@ -0,0 +1,163 @@ +//! Auto Settings decodes a selection off the UI thread and commits it as one undo step. +use crate::LightcraftApp; +use lightcraft_catalog::{Op, PhotoId}; +use lightcraft_develop::DevelopSettings; +use lightcraft_engine::media::{SourceLevel, SourceRef}; +use lightcraft_pipeline::SourceInfo; +use serde_json::{Value, json}; +use std::sync::{Arc, atomic::AtomicBool}; + +const LABEL: &str = "Auto Settings"; +struct Input { + id: PhotoId, + settings: Arc, + source: SourceRef, + info: SourceInfo, +} +struct Prepared { + library: u64, + commits: u64, + inputs: Vec, +} +struct Completed { + library: u64, + commits: u64, + /// Each photo's settings as they were read, and with Auto. + settings: Vec<(PhotoId, Arc, DevelopSettings)>, + /// Photos left out because they could not be read. + skipped: usize, +} + +fn prepare(app: &mut LightcraftApp, params: &Value) -> Result { + let ids = app.session.targets(params).map_err(|e| e.to_string())?; + let mut inputs = Vec::with_capacity(ids.len()); + for id in ids { + let photo = app.session.catalog.photo(id).ok_or("Auto Settings: no such photo")?.clone(); + let settings = app.session.develop_of(id).ok_or("Auto Settings: no develop settings")?; + let info = app.session.source_info(id); + let source = app.session.media.source_ref(&photo, SourceLevel::Thumb); + inputs.push(Input { id, settings, source, info }); + } + Ok(Prepared { library: app.session.library_generation(), commits: app.session.commits(), inputs }) +} +impl Prepared { + fn run(self, progress: Option<&lightcraft_engine::activity::TaskHandle>) -> Result { + let total = self.inputs.len() as u64; + let mut settings = Vec::with_capacity(self.inputs.len()); + let mut failed: Option = None; + let mut skipped = 0; + for (i, input) in self.inputs.into_iter().enumerate() { + if progress.is_some_and(|p| p.is_cancelled()) { + return Err("Auto Settings cancelled".into()); + } + // a photo that can't be read (offline, no decoder) is left out, not the whole selection + let source = match input.source.load_source() { + Ok(source) => source, + Err(error) => { + failed.get_or_insert(error); + skipped += 1; + continue; + } + }; + let info = source.info_or(input.info); + let auto = lightcraft_pipeline::auto::auto_tone(&source.image, &info, &input.settings); + let mut develop = (*input.settings).clone(); + develop.light.exposure = auto.exposure; + develop.light.contrast = auto.contrast; + develop.light.highlights = auto.highlights; + develop.light.shadows = auto.shadows; + develop.light.whites = auto.whites; + develop.light.blacks = auto.blacks; + develop.color.vibrance = auto.vibrance; + develop.color.saturation = auto.saturation; + settings.push((input.id, input.settings, develop)); + if let Some(p) = progress { + p.progress(i as u64 + 1, total); + } + } + if progress.is_some_and(|p| p.is_cancelled()) { + return Err("Auto Settings cancelled".into()); + } + if let (true, Some(error)) = (settings.is_empty(), failed) { + return Err(error); + } + Ok(Completed { library: self.library, commits: self.commits, settings, skipped }) + } +} +fn apply(app: &mut LightcraftApp, completed: Completed) -> Result { + // An undo or a live edit changes a photo's settings without a commit: compare the settings too. + let moved = completed.settings.iter().any(|(id, before, _)| app.session.develop_of(*id).is_none_or(|now| now != *before)); + if app.session.library_generation() != completed.library || app.session.commits() != completed.commits || moved { + return Err("Auto Settings was not applied because the library changed; try again".into()); + } + let ops: Vec = completed.settings.into_iter().filter_map(|(id, _, d)| app.session.develop_op(id, d, "Auto")).collect(); + if !ops.is_empty() { + // through the command wrapper, so sidecars and the durable save follow as for any edit + app.session + .execute_fn("develop.auto", |s| { + s.commit("Auto", Op::Batch { ops })?; + Ok(Value::Null) + }) + .map_err(|e| e.to_string())?; + } + Ok(completed.skipped) +} +pub(crate) fn command(app: &mut LightcraftApp, params: &Value) -> Result { + if app.tasks.is_running(LABEL) { + return Err("Auto Settings is already running".into()); + } + let prepared = prepare(app, params)?; + let count = prepared.inputs.len(); + let cancel = Arc::new(AtomicBool::new(false)); + crate::tasks::spawn_tracked( + app, + LABEL, + "auto", + cancel, + move |progress| prepared.run(Some(&progress)), + |app, ctx, result| match result.and_then(|done| apply(app, done)) { + Ok(0) => app.toast(ctx, crate::i18n::tr("Auto Settings").to_owned()), + Ok(skipped) => app.toast(ctx, format!("{}: {skipped} photo(s) could not be read", crate::i18n::tr("Auto Settings"))), + Err(error) => app.toast(ctx, error), + }, + )?; + Ok(json!({"pending": true, "photos": count})) +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + fn app() -> LightcraftApp { + LightcraftApp::new(lightcraft_engine::Session::with_demo(), crate::Services::default()) + } + #[test] + fn selection_auto_is_deferred_and_commits_once() { + let mut app = app(); + let ids: Vec<_> = app.session.catalog.photos().take(3).map(|p| p.id).collect(); + app.session.execute("library.select", &json!({"ids": ids.iter().map(|i| i.0).collect::>()})).unwrap(); + let before = app.session.commits(); + let reply = app.run("develop.auto", json!({})).unwrap(); + assert_eq!(reply["pending"], true); + assert_eq!(reply["photos"], 3); + assert_eq!(app.session.commits(), before, "decoding cannot mutate the UI session"); + assert!(app.tasks.is_running(LABEL)); + assert!(crate::tasks::wait_for(&mut app, &egui::Context::default(), LABEL, std::time::Duration::from_secs(60))); + assert_eq!(app.session.commits(), before + 1); + for id in ids { + assert_eq!(app.session.develop_of(id).unwrap().color.vibrance, 30.0); + } + } + #[test] + fn selection_auto_does_not_overwrite_newer_edits_or_another_library() { + let mut app = app(); + let done = prepare(&mut app, &json!({})).unwrap().run(None).unwrap(); + app.session.execute("develop.set", &json!({"control":"light.exposure", "value": 1.25})).unwrap(); + let before = app.session.commits(); + assert!(apply(&mut app, done).is_err()); + assert_eq!(app.session.commits(), before); + let done = prepare(&mut app, &json!({})).unwrap().run(None).unwrap(); + app.session = lightcraft_engine::Session::with_demo(); + assert!(apply(&mut app, done).is_err()); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c9457e4417..5bfccc8fb7 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -87,6 +87,7 @@ pub use widgets::rating; pub use widgets::text_field; mod app; +mod auto_settings; mod caches; mod clock; mod services; diff --git a/crates/ui-egui/src/menus/run.rs b/crates/ui-egui/src/menus/run.rs index 730bde3df4..9791f0cac3 100644 --- a/crates/ui-egui/src/menus/run.rs +++ b/crates/ui-egui/src/menus/run.rs @@ -154,6 +154,9 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option