From e14db8946205595f0fdc54b1054d747106f2c0d0 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:01:46 +0200 Subject: [PATCH] LR-EXP-BITDEPTH: app.export refuses a bitDepth the format doesn't have (part of #1952) --- apps/lightcraft-cli/src/main.rs | 7 ++-- crates/engine/src/export.rs | 66 ++++++++++++++++++++++++++++++++- crates/ui-egui/src/headless.rs | 22 +++++++++++ crates/ui-egui/src/menus.rs | 3 +- 4 files changed, 93 insertions(+), 5 deletions(-) diff --git a/apps/lightcraft-cli/src/main.rs b/apps/lightcraft-cli/src/main.rs index 01dae3055..c8371cbb9 100644 --- a/apps/lightcraft-cli/src/main.rs +++ b/apps/lightcraft-cli/src/main.rs @@ -732,12 +732,13 @@ fn render(args: &[String]) -> Result<(), String> { for (k, v) in opts { p[k] = v; } - let mut o = ExportOptions::from_params(&p).map_err(|e| e.to_string())?; if p.get("format").is_none() { let ext = Path::new(&output).extension().map(|e| e.to_string_lossy().to_string()).unwrap_or_default(); - o.format = ExportFormat::parse(&ext) - .ok_or_else(|| format!("{output}: unknown extension (use .jpg .png .tif .webp .avif .dng or --opt format=…)"))?; + ExportFormat::parse(&ext).ok_or_else(|| format!("{output}: unknown extension (use .jpg .png .tif .webp .avif .dng or --opt format=…)"))?; + // given before validating, so `bitDepth` is checked against the format written (issue #1952) + p["format"] = json!(ext); } + let o = ExportOptions::from_params(&p).map_err(|e| e.to_string())?; // Add to Apple Photos: refused before anything is written when it can't be done let after = s.after_export(&o)?; let e = export_photo(&mut s, lightcraft_engine::catalog::PhotoId(id), &o, 1)?; diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index d294ed4d9..f0c8f0033 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -816,11 +816,18 @@ impl ExportOptions { /// newer version may have written and this one doesn't know dropped, also inside `watermark`. /// [`ExportOptions::validate`] is strict about what a caller types; settings saved on disk are /// not the caller's typo, and refusing them would make a preset or Export with Previous fail - /// for good after an update. + /// for good after an update. A `bitDepth` the stored format doesn't have (written at the + /// format's own depth before issue #1952) is dropped too. pub fn known_keys_only(p: &serde_json::Value) -> serde_json::Value { let mut p = p.clone(); if let Some(o) = p.as_object_mut() { o.retain(|k, _| OPTION_PARAMS.contains(&k.as_str()) || TARGET_PARAMS.contains(&k.as_str())); + let format = o.get("format").and_then(serde_json::Value::as_str).map_or(Some(ExportFormat::default()), ExportFormat::parse); + if let (Some(format), Some(depth)) = (format, o.get("bitDepth").and_then(whole)) + && !Self::bit_depths(format).iter().any(|d| i64::from(d.0) == depth) + { + o.remove("bitDepth"); + } if let Some(w) = o.get_mut("watermark").and_then(serde_json::Value::as_object_mut) { w.retain(|k, _| WATERMARK_PARAMS.contains(&k.as_str())); } @@ -949,6 +956,26 @@ impl ExportOptions { _ => return Err(bad(unknown_key("parameter", k, &known))), } } + // issue #1952: a depth the format doesn't have would be written at the format's own depth; + // the format is the one exported: `format`, else the one `path` names, else the default + if let Some(depth) = obj.get("bitDepth").and_then(whole) { + let format = match (obj.get("format").and_then(Value::as_str), obj.get("path").and_then(Value::as_str)) { + (Some(f), _) => ExportFormat::parse(f), + (None, Some(path)) => std::path::Path::new(path).extension().and_then(|e| e.to_str()).and_then(ExportFormat::parse), + (None, None) => Some(ExportFormat::default()), + }; + if let Some(format) = format { + let mut depths: Vec = Self::bit_depths(format).iter().map(|d| d.0).collect(); + if !depths.iter().any(|d| i64::from(*d) == depth) { + depths.sort_unstable(); + let mut list: Vec = depths.iter().map(u8::to_string).collect(); + let last = list.pop().unwrap_or_default(); + let list = if list.is_empty() { last } else { format!("{} or {last}", list.join(", ")) }; + let name = serde_json::to_value(format).ok().and_then(|v| v.as_str().map(str::to_string)).unwrap_or_default(); + return Err(bad(format!("`bitDepth` must be {list} for {name}"))); + } + } + } Ok(()) } @@ -3167,6 +3194,7 @@ mod tests { resize: Some(Resize::default()), limit_kb: Some(500), watermark: Some(Watermark { text: "©".into(), ..Default::default() }), + format: ExportFormat::Png, bit_depth: Some(16), ..Default::default() }; @@ -3203,6 +3231,42 @@ mod tests { ExportOptions::from_params(&json!({"quality": 92, "longEdge": null, "watermark": null})).unwrap(); } + /// Issue #1952: a `bitDepth` the format doesn't have is refused (the Export dialog never offers + /// it), not written at the format's own depth; the error names the depths the format takes. + #[test] + fn bit_depth_must_suit_the_format() { + use serde_json::json; + let err = |p: serde_json::Value| ExportOptions::from_params(&p).err().map(|e| e.to_string()).unwrap_or_default(); + assert!(err(json!({"format": "jpeg", "bitDepth": 16})).contains("`bitDepth` must be 8 for jpeg")); + assert!(err(json!({"format": "png", "bitDepth": 10})).contains("`bitDepth` must be 8 or 16 for png")); + assert!(err(json!({"format": "tiff", "bitDepth": 10})).contains("`bitDepth` must be 8, 16 or 32 for tiff")); + // no `format`: the one `path` names, else the default (JPEG) + assert!(err(json!({"path": "/out/a.png", "bitDepth": 32})).contains("for png")); + assert!(err(json!({"bitDepth": 16})).contains("for jpeg")); + assert!(ExportOptions::from_params(&json!({"path": "/out/a.tif", "bitDepth": 32})).is_ok()); + // stored settings (a preset, Export with Previous) lose such a depth instead of failing + let stored = ExportOptions::known_keys_only(&json!({"format": "jpeg", "bitDepth": 16})); + assert_eq!(stored.get("bitDepth"), None); + assert_eq!(ExportOptions::known_keys_only(&json!({"format": "tiff", "bitDepth": 32}))["bitDepth"], 32); + for f in [ + ExportFormat::Jpeg, + ExportFormat::Png, + ExportFormat::Tiff, + ExportFormat::Psd, + ExportFormat::Webp, + ExportFormat::Avif, + ExportFormat::Jxl, + ExportFormat::Original, + ExportFormat::Dng, + ExportFormat::RenderedDng, + ] { + for &(b, _) in ExportOptions::bit_depths(f) { + let o = ExportOptions { format: f, bit_depth: Some(b), ..Default::default() }; + assert_eq!(ExportOptions::from_params(&o.to_json()).ok(), Some(o), "{f:?} {b}"); + } + } + } + /// Issue #956: `destinationMode` picks a specific folder (the default) or each photo's own /// folder; it is checked like the other params and kept in presets and Export with Previous. #[test] diff --git a/crates/ui-egui/src/headless.rs b/crates/ui-egui/src/headless.rs index 6178990b0..70f128f22 100644 --- a/crates/ui-egui/src/headless.rs +++ b/crates/ui-egui/src/headless.rs @@ -2174,6 +2174,28 @@ mod tests { assert!(rect[2].as_f64().unwrap() > 50.0 && rect[3].as_f64().unwrap() > 50.0, "{rect}"); } + /// Issue #1952: a previous export saved with a depth its format doesn't have (an API call made + /// before `app.export` refused it) opens at a depth the format offers, so the dialog's Export, + /// which has no depth to pick for JPEG, doesn't fail; a depth the format has is kept. + #[test] + fn export_dialog_drops_a_depth_the_format_lacks() { + let mut h = demo([1200.0, 900.0]); + let t = Duration::from_secs(10); + let opened = |h: &mut Headless, last: Value| { + h.app.ui.dialog = None; + h.app.session.last_export = Some(last); + h.request("engine.execute", json!({"command": "dialog.export", "params": {}}), t); + match &h.app.ui.dialog { + Some(crate::state::Dialog::Export { opts, .. }) => opts.clone(), + _ => panic!("the export dialog is open"), + } + }; + let o = opened(&mut h, json!({"format": "jpeg", "bitDepth": 16})); + assert_eq!(o.bit_depth, None); + lightcraft_engine::export::ExportOptions::from_params(&o.to_json()).expect("the dialog's settings export"); + assert_eq!(opened(&mut h, json!({"format": "tiff", "bitDepth": 32})).bit_depth, Some(32)); + } + #[test] fn export_dialog_runs_in_the_background() { let mut h = demo([1200.0, 900.0]); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9fcfc8b9d..9703a5da1 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1410,7 +1410,8 @@ pub fn run_ui_command(app: &mut LightcraftApp, id: &str, p: &Value) -> Option