mirror of
https://github.com/storytold/lightcraft.git
synced 2026-10-24 10:09:01 -06:00
render --settings applies the preset in a .lcpreset, and refuses files it can't read (fixes #1730)
This commit is contained in:
@@ -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 <in> -o <out> [--set control=value]… [--settings FILE.json] [--preset ID] [--size N] [--quality Q]
|
||||
//! lightcraft-cli render <in> -o <out> [--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<SettingsFile, String> {
|
||||
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::<Value>(&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<Value> = None;
|
||||
let mut settings = None;
|
||||
let mut preset = None;
|
||||
let mut size: Option<u64> = 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}))?;
|
||||
|
||||
@@ -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() {
|
||||
|
||||
Reference in New Issue
Block a user