mirror of
https://github.com/storytold/lightcraft.git
synced 2026-11-13 17:31:09 -06:00
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 <noreply@anthropic.com> * Auto Settings: a wrong photo id is refused, not skipped Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: eliets2 <287273517+eliets2@users.noreply.github.com> Co-authored-by: FredApps <FredApps@users.noreply.github.com> Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
eliets2
FredApps
FredApps
parent
7b99ed3903
commit
aa52251d2e
@@ -460,24 +460,51 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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<Value> = None;
|
||||
let mut ops: Vec<Op> = 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",
|
||||
|
||||
@@ -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<u64> = 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();
|
||||
|
||||
@@ -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<DevelopSettings>,
|
||||
source: SourceRef,
|
||||
info: SourceInfo,
|
||||
}
|
||||
struct Prepared {
|
||||
library: u64,
|
||||
commits: u64,
|
||||
inputs: Vec<Input>,
|
||||
}
|
||||
struct Completed {
|
||||
library: u64,
|
||||
commits: u64,
|
||||
/// Each photo's settings as they were read, and with Auto.
|
||||
settings: Vec<(PhotoId, Arc<DevelopSettings>, DevelopSettings)>,
|
||||
/// Photos left out because they could not be read.
|
||||
skipped: usize,
|
||||
}
|
||||
|
||||
fn prepare(app: &mut LightcraftApp, params: &Value) -> Result<Prepared, String> {
|
||||
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<Completed, String> {
|
||||
let total = self.inputs.len() as u64;
|
||||
let mut settings = Vec::with_capacity(self.inputs.len());
|
||||
let mut failed: Option<String> = 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<usize, String> {
|
||||
// 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<Op> = 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<Value, String> {
|
||||
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::<Vec<_>>()})).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());
|
||||
}
|
||||
}
|
||||
@@ -87,6 +87,7 @@ pub use widgets::rating;
|
||||
pub use widgets::text_field;
|
||||
|
||||
mod app;
|
||||
mod auto_settings;
|
||||
mod caches;
|
||||
mod clock;
|
||||
mod services;
|
||||
|
||||
@@ -154,6 +154,9 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option<Re
|
||||
let ctx = app.tasks.repaint.clone().unwrap_or_default();
|
||||
return Some(crate::lightroom_import::command(app, id, p, &ctx));
|
||||
}
|
||||
if id == "develop.auto" {
|
||||
return Some(crate::auto_settings::command(app, p));
|
||||
}
|
||||
if id == "photo.reload" {
|
||||
// the files are read in the background (a raw's decode takes a second or more)
|
||||
return crate::reload::command(app, p);
|
||||
|
||||
Reference in New Issue
Block a user