From f360ad949d21e2dc56d775864cf0645bd039f3e0 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 16:22:07 +0200 Subject: [PATCH] render --settings applies the preset in a .lcpreset, and refuses files it can't read (fixes #1730) --- apps/lightcraft-cli/src/main.rs | 53 ++++++++++++++++++++++++++------ apps/lightcraft-cli/tests/cli.rs | 45 +++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 10 deletions(-) diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index b6cad57e0..7dd1cac18 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -3,7 +3,7 @@ //! ```text //! lightcraft-cli run [--demo | --library DIR | --connect [ADDR]] [--import PATH]… CMD [key=value…]… //! lightcraft-cli mcp [--connect [ADDR]] [--demo] [--compact] [FILES/FOLDERS…] -//! lightcraft-cli render -o [--set control=value]… [--settings FILE.json] [--preset ID] [--size N] [--quality Q] +//! lightcraft-cli render -o [--set control=value]… [--settings FILE.json|FILE.lcpreset] [--preset ID] [--size N] [--quality Q] //! lightcraft-cli snapshot [--library DIR | --demo] [--script FILE.jsonl] [-o OUT.png] [--size WxH] [--scale S] [FILES…] //! lightcraft-cli merge hdr|panorama|hdr-panorama [OPTIONS] FILES… //! lightcraft-cli synth-merge hdr|panorama -o DIR @@ -55,7 +55,8 @@ USAGE: Develop one file and export it (.jpg, .png, .tif, .webp, .avif, .jxl, .psd or .dng by extension) with the same encoder as the app's Export dialog. Options: --set CONTROL=VALUE set a develop slider, repeatable (e.g. --set light.exposure=0.5) - --settings FILE merge a partial develop-settings JSON file + --settings FILE merge a partial develop-settings JSON file, or apply the one preset of a + .lcpreset (preset.export) or XMP preset --preset ID apply a preset (see `commands`/presets.list) --size N long edge in pixels (default: full size, cropped) --quality Q JPEG/AVIF/JPEG XL quality 1..100 (default 92) @@ -635,11 +636,41 @@ fn connect_port(addr: &str) -> &str { addr.rsplit_once(':').map_or(addr, |(_, port)| port) } +/// What `render --settings FILE` applies: a partial develop-settings object, or the one preset of +/// a preset file (`.lcpreset` from `preset.export`, XMP), applied as the app applies it. Merging a +/// `.lcpreset` as settings dropped its wrapper and applied nothing (#1730). +enum SettingsFile { + Partial(Value), + Preset(lightcraft_engine::develop::Preset), +} + +fn read_settings(p: &str) -> Result { + use lightcraft_engine::presets::{LCPRESET_EXT, parse_preset_file}; + let bytes = std::fs::read(p).map_err(|e| format!("{p}: {e}"))?; + let ext = Path::new(p).extension().map(|e| e.to_string_lossy().to_ascii_lowercase()).unwrap_or_default(); + let json = serde_json::from_slice::(&bytes); + // a settings object never has a top-level `presets` or `settings` key; a preset file always does + let preset_file = + ext == LCPRESET_EXT || ext == "xmp" || json.as_ref().is_ok_and(|v| v.is_array() || v.get("presets").is_some() || v.get("settings").is_some()); + if preset_file { + let mut list = parse_preset_file(p, &bytes).map_err(|e| format!("{p}: {e}"))?; + return match (list.pop(), list.len()) { + (Some(preset), 0) => Ok(SettingsFile::Preset(preset)), + (None, _) => Err(format!("{p}: no presets in this file")), + (Some(_), n) => Err(format!("{p}: {} presets in this file; --settings applies one, export it on its own", n + 1)), + }; + } + match json.map_err(|e| format!("{p}: {e}"))? { + v @ Value::Object(_) => Ok(SettingsFile::Partial(v)), + _ => Err(format!("{p}: not a develop-settings object or a preset file")), + } +} + fn render(args: &[String]) -> Result<(), String> { let mut input = None; let mut output = None; let mut values = serde_json::Map::new(); - let mut settings: Option = None; + let mut settings = None; let mut preset = None; let mut size: Option = None; let mut quality = 92u8; @@ -654,11 +685,7 @@ fn render(args: &[String]) -> Result<(), String> { let v: f64 = v.trim().parse().map_err(|_| format!("--set {k}: `{v}` is not a number"))?; values.insert(k.trim().to_string(), json!(v)); } - "--settings" => { - let p = take_value(args, &mut i, "--settings")?; - let text = std::fs::read_to_string(p).map_err(|e| format!("{p}: {e}"))?; - settings = Some(serde_json::from_str(&text).map_err(|e| format!("{p}: {e}"))?); - } + "--settings" => settings = Some(read_settings(take_value(args, &mut i, "--settings")?)?), "--preset" => preset = Some(take_value(args, &mut i, "--preset")?.to_string()), "--size" => size = Some(take_value(args, &mut i, "--size")?.parse().map_err(|_| "--size expects a number")?), "--quality" => quality = take_value(args, &mut i, "--quality")?.parse().map_err(|_| "--quality expects 1..100")?, @@ -684,8 +711,14 @@ fn render(args: &[String]) -> Result<(), String> { if let Some(p) = preset { run(&mut s, "preset.apply", json!({"id": p}))?; } - if let Some(st) = settings { - run(&mut s, "develop.merge", json!({"settings": st}))?; + match settings { + Some(SettingsFile::Partial(st)) => run(&mut s, "develop.merge", json!({"settings": st}))?, + Some(SettingsFile::Preset(mut p)) => { + p.id = "cli.settings".into(); + s.presets.push(p); + run(&mut s, "preset.apply", json!({"id": "cli.settings"}))?; + } + None => {} } if !values.is_empty() { run(&mut s, "develop.set", json!({"values": values}))?; diff --git a/apps/lightcraft-cli/tests/cli.rs b/apps/lightcraft-cli/tests/cli.rs index bf56b02ab..a64e9d3f7 100644 --- a/apps/lightcraft-cli/tests/cli.rs +++ b/apps/lightcraft-cli/tests/cli.rs @@ -139,6 +139,51 @@ fn render_subcommand() { assert!(String::from_utf8_lossy(&o.stderr).contains("unknown control")); } +/// Issue #1730: `render --settings` took a `.lcpreset` from `preset.export`, ignored its wrapper +/// and rendered the photo unedited. +#[test] +fn render_settings_applies_an_lcpreset_and_refuses_unknown_files() { + let input = tmp("lcpreset-in.png"); + gradient_png(&input); + let preset = tmp("bright.lcpreset"); + let (ok, lines, err) = run_cli( + &[ + "--import", + input.to_str().unwrap(), + "develop.set", + "control=light.exposure", + "value=1", + "preset.create", + "name=Bright", + "preset.export", + &format!("path={}", preset.display()), + ], + None, + ); + assert!(ok, "{err}: {lines:?}"); + let plain = tmp("lcpreset-plain.png"); + let st = Command::new(BIN).args(["render", input.to_str().unwrap(), "-o", plain.to_str().unwrap()]).status().unwrap(); + assert!(st.success()); + let bright = tmp("lcpreset-bright.png"); + let o = Command::new(BIN) + .args(["render", input.to_str().unwrap(), "-o", bright.to_str().unwrap(), "--settings", preset.to_str().unwrap()]) + .output() + .unwrap(); + assert!(o.status.success(), "{}", String::from_utf8_lossy(&o.stderr)); + assert!(mean(&bright) > mean(&plain) + 10.0, "the preset's exposure was not applied"); + + let other = tmp("other.lcpreset"); + std::fs::write(&other, r#"{"format": "other", "version": 1, "presets": []}"#).unwrap(); + let out = tmp("lcpreset-other.png"); + let o = Command::new(BIN) + .args(["render", input.to_str().unwrap(), "-o", out.to_str().unwrap(), "--settings", other.to_str().unwrap()]) + .output() + .unwrap(); + assert!(!o.status.success(), "an unreadable settings file was accepted"); + assert!(String::from_utf8_lossy(&o.stderr).contains("other.lcpreset"), "{}", String::from_utf8_lossy(&o.stderr)); + assert!(!out.exists(), "nothing is written when the settings can't be read"); +} + /// Issue #93: `render IMG -o IMG` replaced its own input with the render. #[test] fn render_refuses_to_overwrite_its_input() {