diff --git a/apps/photocraft-cli/tests/cli.rs b/apps/photocraft-cli/tests/cli.rs index b71c0f9a..ea330ac1 100644 --- a/apps/photocraft-cli/tests/cli.rs +++ b/apps/photocraft-cli/tests/cli.rs @@ -221,6 +221,24 @@ fn run_errors() { assert!(e.contains("layer.new.layer") && e.contains("must be a JSON object"), "{e}"); } +#[test] +fn run_rejects_invalid_filter_parameters_without_exporting() { + let d = tmp("invalid-filter-params"); + let out = d.join("result.png"); + for (params, key) in [(r#"{"radius":"big"}"#, "radius"), (r#"{"radus":3}"#, "radus"), (r#"{"radius":5000}"#, "radius"), (r#"{"radius":-4}"#, "radius")] { + let result = bin() + .args(["run", "--new", r#"{"width":8,"height":8}"#, "--cmd", "filter.blur.gaussianBlur", "--params", params, "--out"]) + .arg(&out) + .output() + .unwrap(); + assert_eq!(result.status.code(), Some(1)); + let error = String::from_utf8_lossy(&result.stderr); + assert!(error.contains("filter.blur.gaussianBlur") && error.contains(key), "{error}"); + assert!(!out.exists()); + } + std::fs::remove_dir_all(d).unwrap(); +} + #[test] fn batch_applies_actions_to_directory() { let d = tmp("batch"); diff --git a/crates/automation/src/rpc.rs b/crates/automation/src/rpc.rs index 520d3715..597494b2 100644 --- a/crates/automation/src/rpc.rs +++ b/crates/automation/src/rpc.rs @@ -306,6 +306,25 @@ mod tests { Mutex::new(Headless::trusted_local()) } + #[test] + fn execute_reports_invalid_filter_parameters_without_changing_history() { + let h = session(); + assert_eq!(respond(&h, r#"{"method":"doc.new","params":{"width":8,"height":8}}"#)["ok"], true); + let before = h.lock().unwrap().inspect(None).unwrap(); + for (params, key) in + [(json!({"radius":"big"}), "radius"), (json!({"radus":3}), "radus"), (json!({"radius":5000}), "radius"), (json!({"radius":-4}), "radius")] + { + for wait in [true, false] { + let request = json!({"method":"engine.execute","params":{"command":"filter.blur.gaussianBlur","params":params,"wait":wait}}); + let reply = respond(&h, &request.to_string()); + assert_eq!(reply["ok"], false); + let error = reply["error"].as_str().unwrap(); + assert!(error.contains("filter.blur.gaussianBlur") && error.contains(key), "{error}"); + assert_eq!(h.lock().unwrap().inspect(None).unwrap(), before); + } + } + } + #[test] fn lines_round_trip_and_errors() { let h = session(); diff --git a/crates/automation/src/server.rs b/crates/automation/src/server.rs index 41e282f2..7165636a 100644 --- a/crates/automation/src/server.rs +++ b/crates/automation/src/server.rs @@ -894,6 +894,27 @@ mod tests { assert!(downscale_png(&png, 1024).is_err()); } + #[tokio::test(flavor = "multi_thread")] + async fn mcp_reports_invalid_filter_parameters_without_changing_history() { + let mcp = PhotocraftMcp::headless(); + mcp.headless_op(|h| h.command_run("file.new", json!({"width":8,"height":8}))).await.unwrap().unwrap(); + let before = mcp.headless_op(|h| h.inspect(None)).await.unwrap().unwrap(); + for (params, key) in + [(json!({"radius":"big"}), "radius"), (json!({"radus":3}), "radus"), (json!({"radius":5000}), "radius"), (json!({"radius":-4}), "radius")] + { + for wait in [true, false] { + let result = mcp + .command_run(Parameters(RunParams { id: "filter.blur.gaussianBlur".into(), params: Some(params.clone()), wait: Some(wait) })) + .await + .unwrap(); + assert_eq!(result.is_error, Some(true)); + let error = &result.content.first().and_then(|c| c.as_text()).unwrap().text; + assert!(error.contains("filter.blur.gaussianBlur") && error.contains(key), "{error}"); + assert_eq!(mcp.headless_op(|h| h.inspect(None)).await.unwrap().unwrap(), before); + } + } + } + #[tokio::test(flavor = "multi_thread")] async fn a_panicking_command_does_not_wedge_the_session() { let mcp = PhotocraftMcp::headless(); diff --git a/crates/engine/src/filters.rs b/crates/engine/src/filters.rs index 379a6b7a..c34c9e56 100644 --- a/crates/engine/src/filters.rs +++ b/crates/engine/src/filters.rs @@ -13,6 +13,9 @@ use serde_json::{Value, json}; use crate::commands::CommandSpec; use crate::{EngineError, Result, Session}; +mod validation; +pub(crate) use validation::validate_params; + fn f(p: &Value, k: &str, d: f32) -> f32 { p.get(k).and_then(Value::as_f64).map_or(d, |v| v as f32) } @@ -27,6 +30,10 @@ fn b(p: &Value, k: &str, d: bool) -> bool { fn s<'a>(p: &'a Value, k: &str, d: &'a str) -> &'a str { p.get(k).and_then(Value::as_str).unwrap_or(d) } +fn seed(p: &Value) -> u32 { + // New calls accept the entire documented u32 range; keep legacy record decoding otherwise. + p.get("seed").and_then(Value::as_u64).and_then(|n| u32::try_from(n).ok()).unwrap_or_else(|| i(p, "seed", 0) as u32) +} fn undefined(p: &Value) -> UndefinedAreas { match s(p, "undefinedAreas", "wrap") { "repeat" => UndefinedAreas::Repeat, @@ -55,7 +62,8 @@ fn preset(id: &str) -> Option<(&'static str, FilterParams)> { } /// Builds the algorithm parameters for a filter command id from JSON params -/// (Photoshop dialog units). +/// (Photoshop dialog units). Stored smart filters and previews retain their tolerant decoding; +/// new command invocations pass through `validate_params` before any document changes. pub fn params_for(id: &str, p: &Value) -> Option { if let Some((_, fp)) = preset(id) { return Some(fp); @@ -94,7 +102,7 @@ pub fn params_for(id: &str, p: &Value) -> Option { amount: f(p, "amount", 12.5).clamp(0.1, 400.0), distribution: if s(p, "distribution", "uniform") == "gaussian" { Distribution::Gaussian } else { Distribution::Uniform }, monochromatic: b(p, "monochromatic", false), - seed: i(p, "seed", 0) as u32, + seed: seed(p), }, "filter.noise.median" => FilterParams::Median { radius: f(p, "radius", 1.0).clamp(1.0, 500.0) }, "filter.noise.dustAndScratches" => { @@ -131,7 +139,7 @@ pub fn params_for(id: &str, p: &Value) -> Option { _ => WaveType::Sine, }, undefined: undefined(p), - seed: i(p, "seed", 0) as u32, + seed: seed(p), }, "filter.distort.ripple" => FilterParams::Ripple { amount: f(p, "amount", 100.0).clamp(-999.0, 999.0), @@ -200,6 +208,7 @@ pub(crate) fn has_filterable_layer(s: &Session) -> std::result::Result<(), Strin } pub(crate) fn run_filter(s: &mut Session, id: &str, p: &Value) -> Result { + validate_params(id, p)?; // Session state some filters read (colours) becomes explicit params; external inputs resolve here. let prepared = crate::filters_ext::prepare(s, id, p); let p = &prepared; diff --git a/crates/engine/src/filters/validation.rs b/crates/engine/src/filters/validation.rs new file mode 100644 index 00000000..a0eb171f --- /dev/null +++ b/crates/engine/src/filters/validation.rs @@ -0,0 +1,335 @@ +//! Validate new core-filter invocations, without changing how old smart-filter records decode. + +use serde_json::Value; + +use crate::{EngineError, Result}; + +#[derive(Clone, Copy)] +enum Rule { + Number(f64, f64), + Unsigned(u64), + Choice(&'static str), + Bool, +} + +impl Rule { + fn accepts(self, value: &Value) -> bool { + match self { + Self::Number(min, max) => value.as_f64().is_some_and(|n| n.is_finite() && (min..=max).contains(&n)), + Self::Unsigned(max) => value.as_u64().is_some_and(|n| n <= max), + Self::Choice(choices) => value.as_str().is_some_and(|s| choices.split('|').any(|choice| choice == s)), + Self::Bool => value.is_boolean(), + } + } + + fn expected(self) -> String { + match self { + Self::Number(min, max) if min == f64::MIN && max == f64::MAX => "a finite number".into(), + Self::Number(min, max) => format!("a number in {min}..{max}"), + Self::Unsigned(max) => format!("an integer in 0..{max}"), + Self::Choice(choices) => format!("one of {choices}"), + Self::Bool => "a boolean".into(), + } + } +} + +// The bounds and choices are the core filters' public dialog-unit contract. Defaults remain in +// params_for; extended/gallery filters keep their own parsing until their validation is added. +fn fields(id: &str) -> Option<&'static [(&'static str, Rule)]> { + use Rule::{Bool, Choice, Number, Unsigned}; + Some(match id { + "filter.blur.gaussianBlur" | "filter.other.highPass" => &[("radius", Number(0.1, 1000.0))], + "filter.blur.boxBlur" => &[("radius", Number(1.0, 2000.0))], + "filter.blur.motionBlur" => &[("angle", Number(-360.0, 360.0)), ("distance", Number(1.0, 2000.0))], + "filter.blur.radialBlur" => &[ + ("amount", Number(1.0, 100.0)), + ("method", Choice("spin|zoom")), + ("quality", Choice("draft|good|best")), + ("centerX", Number(0.0, 1.0)), + ("centerY", Number(0.0, 1.0)), + ], + "filter.blur.surfaceBlur" => &[("radius", Number(1.0, 100.0)), ("threshold", Number(2.0, 255.0))], + "filter.sharpen.unsharpMask" => &[("amount", Number(1.0, 500.0)), ("radius", Number(0.1, 1000.0)), ("threshold", Number(0.0, 255.0))], + "filter.sharpen.smartSharpen" => &[("amount", Number(1.0, 500.0)), ("radius", Number(0.1, 64.0)), ("reduceNoise", Number(0.0, 100.0))], + "filter.noise.addNoise" => { + &[("amount", Number(0.1, 400.0)), ("distribution", Choice("uniform|gaussian")), ("monochromatic", Bool), ("seed", Unsigned(u32::MAX as u64))] + } + "filter.noise.median" => &[("radius", Number(1.0, 500.0))], + "filter.noise.dustAndScratches" => &[("radius", Number(1.0, 500.0)), ("threshold", Number(0.0, 255.0))], + "filter.pixelate.mosaic" => &[("cellSize", Number(2.0, 200.0))], + "filter.stylize.emboss" => &[("angle", Number(-180.0, 180.0)), ("height", Number(1.0, 100.0)), ("amount", Number(1.0, 500.0))], + "filter.distort.twirl" => &[("angle", Number(-999.0, 999.0))], + "filter.distort.pinch" => &[("amount", Number(-100.0, 100.0))], + "filter.distort.spherize" => &[("amount", Number(-100.0, 100.0)), ("mode", Choice("normal|horizontalOnly|verticalOnly"))], + "filter.distort.wave" => &[ + ("generators", Number(1.0, 999.0)), + ("wavelengthMin", Number(1.0, 998.0)), + ("wavelengthMax", Number(2.0, 999.0)), + ("amplitudeMin", Number(1.0, 998.0)), + ("amplitudeMax", Number(1.0, 999.0)), + ("type", Choice("sine|triangle|square")), + ("undefinedAreas", Choice("wrap|repeat")), + ("seed", Unsigned(u32::MAX as u64)), + ], + "filter.distort.ripple" => &[("amount", Number(-999.0, 999.0)), ("size", Choice("small|medium|large"))], + "filter.distort.polarCoordinates" => &[("mode", Choice("rectangularToPolar|polarToRectangular"))], + "filter.other.minimum" | "filter.other.maximum" => &[("radius", Number(0.2, 500.0)), ("preserve", Choice("squareness|roundness"))], + "filter.other.offset" => &[ + // The hint specifies pixels, with no range: keep the existing rounding/saturation. + ("horizontal", Number(f64::MIN, f64::MAX)), + ("vertical", Number(f64::MIN, f64::MAX)), + ("undefinedAreas", Choice("wrap|repeat|transparent")), + ], + "filter.blur.blur" + | "filter.blur.blurMore" + | "filter.sharpen.sharpen" + | "filter.sharpen.sharpenMore" + | "filter.sharpen.sharpenEdges" + | "filter.noise.despeckle" + | "filter.stylize.findEdges" + | "filter.stylize.solarize" => &[], + _ => return None, + }) +} + +/// Runs both at dispatch (before committing a floating selection) and at the shared runner +/// (direct specs/Last Filter). Filters outside this module's core family are unaffected. +pub(crate) fn validate_params(id: &str, params: &Value) -> Result<()> { + let Some(fields) = fields(id) else { return Ok(()) }; + let bad = |msg| EngineError::BadParams { cmd: id.into(), msg }; + if params.is_null() { + return Ok(()); + } + let params = params.as_object().ok_or_else(|| bad("params must be a JSON object".into()))?; + for (key, value) in params { + let (valid, expected) = if let Some((_, rule)) = fields.iter().find(|(name, _)| *name == key) { + (rule.accepts(value), rule.expected()) + } else { + match key.as_str() { + "layer" => (value.as_u64().is_some(), "an unsigned layer id".into()), + "coalesce" => (value.is_string(), "a string".into()), + "target" => ( + value.as_str().is_some_and(|s| matches!(s, "pixels" | "mask" | "quickMask")) + || value.as_object().is_some_and(|o| o.len() == 1 && o.get("channel").and_then(Value::as_u64).is_some()), + "pixels, mask, quickMask or {\"channel\":index}".into(), + ), + _ => return Err(bad(format!("unknown parameter `{key}`"))), + } + }; + if !valid { + return Err(bad(format!("`{key}` must be {expected}"))); + } + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use photocraft_geom::Rect; + use serde_json::json; + + use super::*; + use crate::Session; + + fn session(depth: u32) -> Session { + let mut s = Session::new(); + s.execute("file.new", json!({"width":16,"height":12,"depth":depth})).unwrap(); + s.execute("layer.new.layer", json!({})).unwrap(); + s.edit("pattern", |doc, active| { + let surface = doc.layer_mut(active.unwrap()).unwrap().surface_mut().unwrap(); + surface.fill_rect(Rect::new(0, 0, 8, 12), &[0.2, 0.6, 0.8, 1.0]); + Ok(()) + }) + .unwrap(); + s + } + + fn rejected(s: &mut Session, id: &str, key: &str, value: Value) { + let doc = s.active().unwrap().doc.clone(); + let history = s.active().unwrap().history.entries(); + let journal = s.journal.clone(); + let active = s.active().unwrap().active_layer; + let error = s.execute(id, json!({key:value})).unwrap_err(); + assert!(matches!(error, EngineError::BadParams { .. }), "{id}: {error}"); + let error = error.to_string(); + assert!(error.contains(id) && error.contains(key), "{error}"); + assert!(Arc::ptr_eq(&s.active().unwrap().doc, &doc), "{id}: {key}"); + assert_eq!(s.active().unwrap().history.entries(), history); + assert_eq!(s.active().unwrap().active_layer, active); + assert_eq!(s.journal, journal); + assert!(s.jobs().is_empty()); + } + + #[test] + fn every_core_filter_rejects_unknown_keys_without_editing() { + for depth in [8, 16, 32] { + let mut s = session(depth); + for spec in crate::filters::specs().into_iter().filter(|s| s.id != "filter.lastFilter") { + rejected(&mut s, spec.id, "radus", json!(3)); + } + } + } + + #[test] + fn core_filter_numbers_reject_wrong_types_and_documented_range_violations() { + // Each numeric field in the core registry, independently of the validation table. + let cases = [ + ("blur.gaussianBlur", "radius", 0.1, 1000.0), + ("blur.boxBlur", "radius", 1.0, 2000.0), + ("blur.motionBlur", "angle", -360.0, 360.0), + ("blur.motionBlur", "distance", 1.0, 2000.0), + ("blur.radialBlur", "amount", 1.0, 100.0), + ("blur.radialBlur", "centerX", 0.0, 1.0), + ("blur.radialBlur", "centerY", 0.0, 1.0), + ("blur.surfaceBlur", "radius", 1.0, 100.0), + ("blur.surfaceBlur", "threshold", 2.0, 255.0), + ("sharpen.unsharpMask", "amount", 1.0, 500.0), + ("sharpen.unsharpMask", "radius", 0.1, 1000.0), + ("sharpen.unsharpMask", "threshold", 0.0, 255.0), + ("sharpen.smartSharpen", "amount", 1.0, 500.0), + ("sharpen.smartSharpen", "radius", 0.1, 64.0), + ("sharpen.smartSharpen", "reduceNoise", 0.0, 100.0), + ("noise.addNoise", "amount", 0.1, 400.0), + ("noise.median", "radius", 1.0, 500.0), + ("noise.dustAndScratches", "radius", 1.0, 500.0), + ("noise.dustAndScratches", "threshold", 0.0, 255.0), + ("pixelate.mosaic", "cellSize", 2.0, 200.0), + ("stylize.emboss", "angle", -180.0, 180.0), + ("stylize.emboss", "height", 1.0, 100.0), + ("stylize.emboss", "amount", 1.0, 500.0), + ("distort.twirl", "angle", -999.0, 999.0), + ("distort.pinch", "amount", -100.0, 100.0), + ("distort.spherize", "amount", -100.0, 100.0), + ("distort.wave", "generators", 1.0, 999.0), + ("distort.wave", "wavelengthMin", 1.0, 998.0), + ("distort.wave", "wavelengthMax", 2.0, 999.0), + ("distort.wave", "amplitudeMin", 1.0, 998.0), + ("distort.wave", "amplitudeMax", 1.0, 999.0), + ("distort.ripple", "amount", -999.0, 999.0), + ("other.highPass", "radius", 0.1, 1000.0), + ("other.minimum", "radius", 0.2, 500.0), + ("other.maximum", "radius", 0.2, 500.0), + ]; + for depth in [8, 16, 32] { + let mut s = session(depth); + for (suffix, key, min, max) in cases { + let id = format!("filter.{suffix}"); + for value in [json!("big"), json!(true), Value::Null, json!([]), json!({}), json!(min - 0.01), json!(max + 0.01)] { + rejected(&mut s, &id, key, value); + } + // Large limits are validated without allocating a large blur kernel. + for value in [min, max] { + validate_params(&id, &json!({key:value})).unwrap(); + } + } + for key in ["horizontal", "vertical"] { + rejected(&mut s, "filter.other.offset", key, json!("4")); + } + } + } + + #[test] + fn core_filter_choices_booleans_seeds_and_shared_fields_are_validated() { + let mut s = session(8); + for (suffix, key, choices) in [ + ("blur.radialBlur", "method", "spin|zoom"), + ("blur.radialBlur", "quality", "draft|good|best"), + ("noise.addNoise", "distribution", "uniform|gaussian"), + ("distort.spherize", "mode", "normal|horizontalOnly|verticalOnly"), + ("distort.wave", "type", "sine|triangle|square"), + ("distort.wave", "undefinedAreas", "wrap|repeat"), + ("distort.ripple", "size", "small|medium|large"), + ("distort.polarCoordinates", "mode", "rectangularToPolar|polarToRectangular"), + ("other.minimum", "preserve", "squareness|roundness"), + ("other.maximum", "preserve", "squareness|roundness"), + ("other.offset", "undefinedAreas", "wrap|repeat|transparent"), + ] { + let id = format!("filter.{suffix}"); + for value in [json!("typo"), json!(false), json!(1), Value::Null] { + rejected(&mut s, &id, key, value); + } + for value in choices.split('|') { + validate_params(&id, &json!({key:value})).unwrap(); + } + } + for value in [json!("true"), json!(0), Value::Null] { + rejected(&mut s, "filter.noise.addNoise", "monochromatic", value); + } + for id in ["filter.noise.addNoise", "filter.distort.wave"] { + for value in [json!(-1), json!(4_294_967_296_u64), json!(0.5), json!("12")] { + rejected(&mut s, id, "seed", value); + } + // A valid u32 seed must not be saturated through an i32 helper. + let result = s.execute(id, json!({"seed":u32::MAX})).unwrap(); + assert_eq!(result["filter"]["seed"], json!(u32::MAX)); + } + for (key, value) in [("layer", json!("2")), ("target", json!("typo")), ("target", json!({"channel":-1})), ("coalesce", json!(false))] { + rejected(&mut s, "filter.blur.gaussianBlur", key, value); + } + } + + #[test] + fn invalid_filters_leave_floating_selection_and_background_jobs_alone() { + let mut s = session(16); + s.execute("select.rect", json!({"x":0,"y":0,"width":4,"height":4})).unwrap(); + s.execute("select.float", json!({"dx":8})).unwrap(); + let doc = s.active().unwrap().doc.clone(); + let history = s.active().unwrap().history.entries(); + for params in [json!({"radius":"big"}), json!({"radus":3}), json!({"radius":5000}), json!({"radius":-4})] { + assert!(s.start("filter.blur.gaussianBlur", params.clone()).is_err()); + let spec = crate::commands::find("filter.blur.gaussianBlur").unwrap(); + assert!((spec.run)(&mut s, ¶ms).is_err()); + assert!(crate::float_cmds::floating(s.active().unwrap()).is_some()); + assert!(Arc::ptr_eq(&s.active().unwrap().doc, &doc)); + assert_eq!(s.active().unwrap().history.entries(), history); + assert!(s.jobs().is_empty()); + } + } + + #[test] + fn valid_filters_keep_defaults_explicit_targets_and_undo_redo() { + for depth in [8, 16, 32] { + let mut s = session(depth); + let layer = s.active().unwrap().active_layer.unwrap(); + let pixels = |s: &Session| s.active().unwrap().doc.layer(layer).unwrap().surface().unwrap().read_region(Rect::new(0, 0, 16, 12)); + let before = pixels(&s); + for params in [Value::Null, json!({})] { + let result = s.execute("filter.blur.gaussianBlur", params).unwrap(); + assert_eq!(result["filter"]["radius"], json!(1.0)); + assert_ne!(pixels(&s), before); + s.undo(); + assert_eq!(pixels(&s), before); + } + s.execute("layer.new.layer", json!({})).unwrap(); + let other = s.active().unwrap().active_layer.unwrap(); + let result = s.execute("filter.blur.gaussianBlur", json!({"radius":3,"layer":layer.0,"target":"pixels","coalesce":"blur"})).unwrap(); + assert_eq!(result["filter"]["radius"], json!(3.0)); + let after = pixels(&s); + assert_ne!(after, before); + assert_eq!(s.active().unwrap().doc.layer(other).unwrap().surface().unwrap().tile_count(), 0); + s.undo(); + assert_eq!(pixels(&s), before); + s.redo(); + assert_eq!(pixels(&s), after); + } + } + + #[test] + fn injected_channel_targets_and_extended_filter_colours_keep_working() { + let mut s = session(8); + s.execute("channel.new", json!({"fill":"white"})).unwrap(); + let pixels = s.active().unwrap().doc.layers.clone(); + s.execute("filter.noise.addNoise", json!({"amount":40,"seed":2})).unwrap(); + assert_eq!(s.active().unwrap().doc.layers, pixels); + assert!(s.active().unwrap().doc.channels[0].surface.read_region(Rect::new(0, 0, 16, 12)).iter().any(|&v| v < 0.99)); + s.execute("channel.target", json!({"channel":"composite"})).unwrap(); + s.tools.foreground = [0.0, 0.0, 1.0, 1.0]; + s.tools.background = [1.0, 0.0, 0.0, 1.0]; + let params = crate::filters_ext::prepare(&s, "filter.render.fibers", &json!({})); + assert_eq!(params["foreground"], json!(s.tools.foreground)); + assert_eq!(params["background"], json!(s.tools.background)); + s.execute("filter.render.fibers", params).unwrap(); + } +} diff --git a/crates/engine/src/jobs.rs b/crates/engine/src/jobs.rs index 5838b4eb..b469fc77 100644 --- a/crates/engine/src/jobs.rs +++ b/crates/engine/src/jobs.rs @@ -618,6 +618,7 @@ impl Session { if id == "file.new" { crate::document_preset_cmds::validate_new_params(self, ¶ms)?; } + crate::filters::validate_params(id, ¶ms)?; // A floating selection drops before any other command (Undo puts it back instead). if let Some(v) = crate::float_cmds::before_command(self, id)? { return Ok(Started::Done(v)); diff --git a/crates/ui-egui/src/filter_dialog.rs b/crates/ui-egui/src/filter_dialog.rs index d2d17695..420abe2e 100644 --- a/crates/ui-egui/src/filter_dialog.rs +++ b/crates/ui-egui/src/filter_dialog.rs @@ -527,11 +527,19 @@ pub fn preview_document_with( if k > 1 && let Some(o) = p.as_object_mut() { + let spec = photocraft_engine::commands::find(command).map(|c| parse_spec(c.params)).unwrap_or_default(); for (key, v) in o.iter_mut() { if is_pixel_param(key) && let Some(x) = v.as_f64() { - *v = json!((x / k as f64).max(if key == "cellSize" { 1.0 } else { 0.1 })); + // Match the engine's minimum after scaling (e.g. Box Blur 1, Mosaic 2). + // Previously its tolerant decoder clamped these; new calls validate first. + let minimum = spec.iter().find(|p| p.key == key.as_str()).and_then(|p| match &p.kind { + Kind::Range { min, .. } => Some(f64::from(*min)), + _ => None, + }); + let floor = minimum.unwrap_or(0.0).max(if key == "cellSize" { 1.0 } else { 0.1 }); + *v = json!((x / k as f64).max(floor)); } } } @@ -732,6 +740,42 @@ mod tests { assert_eq!(label("wavelengthMin"), "Wavelength Min"); } + #[test] + fn core_filter_previews_keep_minimum_settings_after_downsampling() { + use photocraft_doc::{Color, ColorMode, SampleType, Size}; + use photocraft_geom::Rect; + + for depth in [SampleType::U8, SampleType::U16, SampleType::F32] { + let mut doc = Document::with_background("preview", Size::new(32, 32), ColorMode::Rgb, depth, Color::WHITE); + let layer = doc.layers[0].id; + doc.layers[0].surface_mut().unwrap().fill_rect(Rect::new(0, 0, 16, 32), &[0.0, 0.0, 0.0, 1.0]); + for (suffix, params) in [ + ("blur.boxBlur", json!({"radius":1})), + ("blur.motionBlur", json!({"distance":1})), + ("blur.surfaceBlur", json!({"radius":1})), + ("noise.median", json!({"radius":1})), + ("noise.dustAndScratches", json!({"radius":1})), + ("pixelate.mosaic", json!({"cellSize":2})), + ("stylize.emboss", json!({"height":1})), + ("other.minimum", json!({"radius":0.2})), + ("other.maximum", json!({"radius":0.2})), + ("distort.wave", json!({"wavelengthMin":1,"wavelengthMax":2,"amplitudeMin":1,"amplitudeMax":1})), + ] { + let command = format!("filter.{suffix}"); + for k in [2, 4] { + let proxy = crate::proxy::proxy_document(&doc, k); + let expected = preview_document(&proxy, Some(layer), &command, ¶ms, 1).unwrap(); + let out = preview_document(&doc, Some(layer), &command, ¶ms, k).unwrap_or_else(|| panic!("{command}: {depth:?}, proxy factor {k}")); + assert_eq!( + out.layers[0].surface().unwrap().read_region(proxy.bounds()), + expected.layers[0].surface().unwrap().read_region(proxy.bounds()), + "{command}: {depth:?}, proxy factor {k}" + ); + } + } + } + } + #[test] fn colour_params_parse_with_and_without_a_default() { let p = parse_spec(r##"{"color":color=#ffffff,"vineColor":color,"bad":color=#zz}"##);