Plug-ins: scale "unit": "px" params on a reduced live preview (#7524)

A filter plug-in's live preview runs on a reduced copy of a large
document, but its params only went through the built-in name list, so a
pixel length with another name (Chromatic Aberration's Amount) previewed
2-4 times too large, and a unitless `size` or `radius` was divided.

number and int params now take an optional "unit" ("px" or "none", the
default; anything else is a manifest error). The preview divides px
params by the reduction (defaults included, ints rounded, kept in range)
and limits the factor so a px param with a positive minimum never drops
below it. Unitless params are never scaled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Higgs32584
2026-10-21 08:00:57 -04:00
co-authored by Claude Opus 5.5
parent 04d0212c7a
commit 65f38bf3ed
6 changed files with 303 additions and 13 deletions
+46 -6
View File
@@ -47,16 +47,56 @@ fn plugin_params(p: &Value) -> Value {
return v.clone();
}
match p {
Value::Object(m) => Value::Object(
m.iter()
.filter(|(k, _)| !matches!(k.as_str(), "id" | "layer" | "target" | "channel") && !k.starts_with("__"))
.map(|(k, v)| (k.clone(), v.clone()))
.collect(),
),
Value::Object(m) => Value::Object(m.iter().filter(|(k, _)| !command_key(k)).map(|(k, v)| (k.clone(), v.clone())).collect()),
_ => Value::Object(Map::new()),
}
}
/// A top-level `plugin.run` key the command itself uses (or a dialog's private `__` key), not a
/// plug-in parameter.
fn command_key(k: &str) -> bool {
matches!(k, "id" | "layer" | "target" | "channel") || k.starts_with("__")
}
/// Proxy factor for a live preview of `plugin.run` with `p` on a document reduced `k` times: the
/// plug-in's [`photocraft_plugins::Manifest::preview_factor`] (its `"unit": "px"` parameters set
/// the limit, not the built-in filters' parameter names). `k` when the plug-in isn't installed.
pub fn preview_factor(p: &Value, k: u32) -> u32 {
let Ok(plugin) = plugin_by_id(RUN, p) else { return k };
match plugin_params(p) {
Value::Object(params) => plugin.manifest().preview_factor(&params, k),
// `null` means the defaults (see `Manifest::resolve_params`).
Value::Null => plugin.manifest().preview_factor(&Map::new(), k),
_ => k,
}
}
/// Rewrites `plugin.run` params `p` for a live preview on a copy of the document reduced `k`
/// times: the plug-in's pixel-sized parameters (`"unit": "px"`, under `params` or at the top
/// level, as [`plugin_params`] reads them) are divided by `k`, so the preview shows the lengths OK
/// applies (#7524). Nothing changes when the plug-in isn't installed (the run then fails as usual)
/// or for values of the wrong type (the run rejects them).
pub fn scale_for_preview(p: &mut Value, k: u32) {
if k <= 1 {
return;
}
let Ok(plugin) = plugin_by_id(RUN, p) else { return };
let nested = p.get("params").is_some();
let mut params = match plugin_params(p) {
Value::Object(params) => params,
Value::Null => Map::new(),
_ => return,
};
plugin.manifest().scale_for_preview(&mut params, k);
let Value::Object(o) = p else { return };
if nested {
o.insert("params".into(), Value::Object(params));
} else {
// Top-level form: write back only plug-in keys, never the command's own (`id`, `channel`…).
o.extend(params.into_iter().filter(|(key, _)| !command_key(key)));
}
}
fn plugin_by_id(cmd: &str, p: &Value) -> Result<std::sync::Arc<Plugin>> {
let id = p.get("id").and_then(Value::as_str).ok_or_else(|| bad(cmd, "missing `id` (see plugin.list)"))?;
registry::get(id).ok_or_else(|| err(photocraft_plugins::Error::NotFound(id.into())))
+56
View File
@@ -155,6 +155,62 @@ fn params_are_validated_and_passed() {
Session::new().execute("plugin.remove", json!({"id": "test.cmds.grey"})).unwrap();
}
/// #7524: a reduced live preview divides the plug-in's `"unit": "px"` params by the reduction, in
/// both param shapes, and leaves unitless params (even ones named like built-in pixel params) and
/// the command's own keys alone.
#[test]
fn preview_scales_the_plugin_pixel_params() {
let id = "test.cmds.px";
let params = concat!(
r#"{"amount":{"type":"number","min":0,"max":100,"default":12,"unit":"px"},"#,
r#""radius":{"type":"int","min":2,"max":50,"default":10,"unit":"px"},"#,
r#""size":{"type":"number","min":0,"max":100,"default":50}}"#
);
Session::new().execute("plugin.install", json!({"data": b64(&wat_plugin(id, params, GREY))})).unwrap();
let list = Session::new().execute("plugin.list", json!({})).unwrap();
let me = list["plugins"].as_array().unwrap().iter().find(|p| p["id"] == id).unwrap().clone();
assert_eq!(me["paramsSchema"]["amount"]["unit"], "px");
// Top level (the dialog's shape): px params scaled, `size` and the command's keys untouched.
let mut p = json!({"id": id, "amount": 12, "radius": 10, "size": 40, "channel": 1, "__preview": true});
scale_for_preview(&mut p, 4);
assert_eq!(p, json!({"id": id, "amount": 3.0, "radius": 3, "size": 40, "channel": 1, "__preview": true}));
// Nested: scaled inside `params`, missing ones from their defaults.
let mut p = json!({"id": id, "params": {"size": 40}});
scale_for_preview(&mut p, 2);
assert_eq!(p, json!({"id": id, "params": {"amount": 6.0, "radius": 5, "size": 40}}));
let mut p = json!({"id": id, "params": null});
scale_for_preview(&mut p, 2);
assert_eq!(p["params"], json!({"amount": 6.0, "radius": 5}));
// The scaled params still run.
doc(8).execute("plugin.run", p).unwrap();
// Full resolution, an unknown plug-in or bad shapes change nothing and never panic.
for (mut p, k) in [
(json!({"id": id, "amount": 12}), 1),
(json!({"id": "test.cmds.none", "amount": 12}), 4),
(json!({"amount": 12}), 4),
(json!({"id": id, "params": [1]}), 4),
(json!([1, 2]), 4),
] {
let before = p.clone();
scale_for_preview(&mut p, k);
assert_eq!(p, before);
}
// A value of the wrong type is kept for the run to reject.
let mut p = json!({"id": id, "amount": "lots", "radius": 10});
scale_for_preview(&mut p, 4);
assert_eq!(p, json!({"id": id, "amount": "lots", "radius": 3}));
assert!(doc(8).execute("plugin.run", p).is_err());
// The factor: radius (min 2) 10 allows 4, 5 allows 2; unitless and missing plug-ins don't limit it.
assert_eq!(preview_factor(&json!({"id": id, "radius": 10}), 4), 4);
assert_eq!(preview_factor(&json!({"id": id, "radius": 5, "size": 0.5}), 4), 2);
assert_eq!(preview_factor(&json!({"id": id, "params": {"radius": 3}}), 4), 1);
assert_eq!(preview_factor(&json!({"id": id}), 8), 5, "the default radius");
assert_eq!(preview_factor(&json!({"id": "test.cmds.none", "radius": 3}), 4), 4);
Session::new().execute("plugin.remove", json!({"id": id})).unwrap();
}
#[test]
fn locked_layers_refuse_plugin_filters() {
// Like built-in filters: locked pixels (or Lock All) refuse the run with no undo step.
+1 -1
View File
@@ -26,7 +26,7 @@ pub mod registry;
mod runtime;
pub use apply::format_code;
pub use manifest::{Area, Kind, Manifest, ParamSpec};
pub use manifest::{Area, Kind, Manifest, ParamSpec, Unit};
pub use runtime::{ABI_VERSION, Limits, Plugin};
/// A plug-in error. Every way a module can misbehave ends here, never in a panic.
+181 -5
View File
@@ -30,12 +30,36 @@ pub enum Area {
Canvas,
}
/// What a `number` or `int` parameter measures (`"unit"` in the manifest).
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
pub enum Unit {
/// No unit (the default): a level, factor, count or percentage, the same at any image size.
#[default]
None,
/// A length in document pixels (a radius, distance, shift or size). On a reduced live preview
/// the host divides it by the reduction, so the preview matches what OK applies.
Px,
}
impl Unit {
fn is_none(&self) -> bool {
*self == Unit::None
}
}
/// One declared parameter.
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
#[serde(tag = "type", rename_all = "camelCase")]
pub enum ParamSpec {
/// A real number in `min..=max` (a slider in the dialog).
Number { min: f64, max: f64, default: f64 },
Number {
min: f64,
max: f64,
default: f64,
#[serde(default, skip_serializing_if = "Unit::is_none")]
unit: Unit,
},
/// A whole number in `min..=max`.
Int {
#[serde(default = "int_min")]
@@ -44,6 +68,8 @@ pub enum ParamSpec {
max: i64,
#[serde(default)]
default: i64,
#[serde(default, skip_serializing_if = "Unit::is_none")]
unit: Unit,
},
/// A checkbox.
Bool {
@@ -122,7 +148,7 @@ impl Manifest {
return Err(Error::Manifest(format!("invalid parameter name {k:?}")));
}
match spec {
ParamSpec::Number { min, max, default } if !(min.is_finite() && max.is_finite() && default.is_finite()) || min > max => {
ParamSpec::Number { min, max, default, .. } if !(min.is_finite() && max.is_finite() && default.is_finite()) || min > max => {
return Err(Error::Manifest(format!("parameter `{k}`: bad range")));
}
ParamSpec::Int { min, max, .. } if min > max => return Err(Error::Manifest(format!("parameter `{k}`: bad range"))),
@@ -147,7 +173,7 @@ impl Manifest {
.params
.iter()
.map(|(k, spec)| match spec {
ParamSpec::Number { min, max, default } => format!("\"{k}\":{min}..{max}={default}"),
ParamSpec::Number { min, max, default, .. } => format!("\"{k}\":{min}..{max}={default}"),
ParamSpec::Int { default, .. } => format!("\"{k}\":int={default}"),
ParamSpec::Bool { default } => format!("\"{k}\":bool={default}"),
ParamSpec::Choice { options, default } => {
@@ -179,11 +205,11 @@ impl Manifest {
let v = given.get(k).filter(|v| !v.is_null());
let wrong = |what: &str| Error::Params(format!("`{k}` must be {what}"));
let resolved = match spec {
ParamSpec::Number { min, max, default } => match v {
ParamSpec::Number { min, max, default, .. } => match v {
None => json!((*default).clamp(*min, *max)),
Some(v) => json!(v.as_f64().filter(|f| f.is_finite()).ok_or_else(|| wrong("a number"))?.clamp(*min, *max)),
},
ParamSpec::Int { min, max, default } => match v {
ParamSpec::Int { min, max, default, .. } => match v {
None => json!((*default).clamp(*min, *max)),
Some(v) => {
let f = v.as_f64().filter(|f| f.is_finite()).ok_or_else(|| wrong("a number"))?;
@@ -209,6 +235,60 @@ impl Manifest {
}
Ok(out)
}
/// Whether parameter `key` is a length in pixels (`"unit": "px"`).
pub fn is_pixel_param(&self, key: &str) -> bool {
self.pixel_params().any(|(k, ..)| k == key)
}
/// `(key, min, max, default, is_int)` of each parameter declared `"unit": "px"`.
fn pixel_params(&self) -> impl Iterator<Item = (&str, f64, f64, f64, bool)> {
self.params.iter().filter_map(|(k, spec)| match spec {
ParamSpec::Number { min, max, default, unit: Unit::Px } => Some((k.as_str(), *min, *max, *default, false)),
ParamSpec::Int { min, max, default, unit: Unit::Px } => Some((k.as_str(), *min as f64, *max as f64, *default as f64, true)),
_ => None,
})
}
/// The value [`Manifest::resolve_params`] would use for a pixel parameter: the given number, or
/// the default when missing, within `min..=max`. `None` for a value of the wrong type (which
/// `resolve_params` rejects) or one that isn't finite.
fn pixel_value(params: &Map<String, Value>, key: &str, min: f64, max: f64, default: f64) -> Option<f64> {
let v = match params.get(key).filter(|v| !v.is_null()) {
None => default,
Some(v) => v.as_f64()?,
};
// `max`/`min` rather than `clamp`, which panics on a reversed or NaN range.
v.is_finite().then(|| v.max(min).min(max))
}
/// Proxy factor for a live preview with `params` on a document reduced `k` times: the largest
/// factor up to `k` at which no pixel parameter with a positive minimum, divided by it, drops
/// below that minimum. A clamped value would change the feature size (as for built-in filters,
/// #2063), so such a preview runs less reduced instead. Always at least 1.
pub fn preview_factor(&self, params: &Map<String, Value>, k: u32) -> u32 {
self.pixel_params().fold(k.max(1), |k, (key, min, max, default, _)| match Self::pixel_value(params, key, min, max, default) {
Some(v) if min > 0.0 => k.min((v / min).floor().max(1.0).min(f64::from(k)) as u32),
_ => k,
})
}
/// Rewrites the plug-in's `params` for a live preview on a copy of the document reduced `k`
/// times: each pixel parameter (the given value, or its default when missing) is divided by
/// `k`, rounded for `int`s and kept within its range, so the preview shows the same lengths in
/// document pixels as OK applies (#7524). Unitless parameters, unknown keys and values of the
/// wrong type (which [`Manifest::resolve_params`] rejects) are left as they are.
pub fn scale_for_preview(&self, params: &mut Map<String, Value>, k: u32) {
if k <= 1 {
return;
}
let k = f64::from(k);
for (key, min, max, default, int) in self.pixel_params() {
let Some(v) = Self::pixel_value(params, key, min, max, default) else { continue };
let x = (v / k).max(min).min(max);
params.insert(key.to_string(), if int { json!(x.round().max(min).min(max) as i64) } else { json!(x) });
}
}
}
/// Reads the `params` object keeping its key order (serde_json's `Map` would sort it).
@@ -387,6 +467,102 @@ mod tests {
assert!(Manifest::parse(&vec![b' '; MAX_MANIFEST_BYTES + 1]).is_err());
}
/// Chromatic Aberration-like: a pixel shift, a pixel int, a signed pixel offset and a unitless
/// `size` (a name the built-in filters treat as pixels).
const PIXELS: &str = r#"{"id":"org.example.fringe","name":"Fringe","kind":"filter",
"params":{"amount":{"type":"number","min":0,"max":100,"default":12,"unit":"px"},
"radius":{"type":"int","min":2,"max":50,"default":10,"unit":"px"},
"shift":{"type":"number","min":-40,"max":40,"default":0,"unit":"px"},
"size":{"type":"number","min":0,"max":100,"default":50},
"mix":{"type":"number","min":0,"max":1,"default":1,"unit":"none"}}}"#;
#[test]
fn parses_units_and_keeps_old_manifests_unitless() {
let m = Manifest::parse(PIXELS.as_bytes()).unwrap();
assert!(m.is_pixel_param("amount") && m.is_pixel_param("radius") && m.is_pixel_param("shift"));
assert!(!m.is_pixel_param("size") && !m.is_pixel_param("mix") && !m.is_pixel_param("missing"));
assert!(matches!(m.params[0].1, ParamSpec::Number { unit: Unit::Px, .. }));
assert!(matches!(m.params[1].1, ParamSpec::Int { unit: Unit::Px, .. }));
// No `unit` is unitless, and unitless params serialize as before (no `unit` key).
let old = Manifest::parse(FULL.as_bytes()).unwrap();
assert!(old.params.iter().all(|(k, _)| !old.is_pixel_param(k)));
let schema = serde_json::to_value(&m).unwrap();
assert_eq!(schema["params"]["amount"]["unit"], "px");
assert!(schema["params"]["size"].get("unit").is_none() && schema["params"]["mix"].get("unit").is_none());
assert_eq!(Manifest::parse(serde_json::to_string(&m).unwrap().as_bytes()).unwrap(), m);
// A unit is a `number`/`int` property; the dialog notation is unchanged by it.
assert_eq!(m.params_notation(), r#"{"amount":0..100=12,"radius":int=10,"shift":-40..40=0,"size":0..100=50,"mix":0..1=1}"#);
}
#[test]
fn malformed_units_are_manifest_errors() {
for unit in [r#""pt""#, r#""PX""#, r#""""#, "5", "true", "null", r#"["px"]"#, r#"{"px":1}"#] {
let bad = format!(r#"{{"id":"a","name":"x","kind":"filter","params":{{"r":{{"type":"number","min":0,"max":9,"default":1,"unit":{unit}}}}}}}"#);
assert!(matches!(Manifest::parse(bad.as_bytes()), Err(Error::Manifest(_))), "{bad}");
let bad = bad.replace(r#""type":"number""#, r#""type":"int""#);
assert!(matches!(Manifest::parse(bad.as_bytes()), Err(Error::Manifest(_))), "{bad}");
}
}
#[test]
fn preview_scales_pixel_params_only() {
// #7524: at a reduction of 4, a 12 px shift must be 3 proxy pixels (12 document pixels).
let m = Manifest::parse(PIXELS.as_bytes()).unwrap();
let given = json!({"id": "org.example.fringe", "amount": 12, "radius": 10, "shift": -20, "size": 40, "mix": 0.5});
let mut p = given.as_object().unwrap().clone();
m.scale_for_preview(&mut p, 4);
assert_eq!(
Value::Object(p.clone()),
json!({"id": "org.example.fringe", "amount": 3.0, "radius": 3, "shift": -5.0, "size": 40, "mix": 0.5}),
"px params divided (ints rounded, signs kept); unitless and other keys untouched"
);
// The scaled values are what the plug-in receives.
assert_eq!(m.resolve_params(&Value::Object(p)).unwrap()["amount"], json!(3.0));
// Missing px params get their scaled default, so the default isn't previewed unscaled.
let mut p = Map::new();
m.scale_for_preview(&mut p, 2);
assert_eq!(Value::Object(p), json!({"amount": 6.0, "radius": 5, "shift": 0.0}));
// Never below the minimum, and values are clamped into range first, as `resolve_params` does.
let mut p = json!({"radius": 3, "amount": 1000}).as_object().unwrap().clone();
m.scale_for_preview(&mut p, 4);
assert_eq!(Value::Object(p), json!({"radius": 2, "amount": 25.0, "shift": 0.0}));
// A full-resolution preview changes nothing.
for k in [0, 1] {
let mut p = given.as_object().unwrap().clone();
m.scale_for_preview(&mut p, k);
assert_eq!(Value::Object(p), given);
}
// Wrong types and non-numbers are left for `resolve_params` to reject, never a panic.
let mut p = json!({"amount": "lots", "radius": true, "shift": [1]}).as_object().unwrap().clone();
m.scale_for_preview(&mut p, 4);
assert_eq!(Value::Object(p.clone()), json!({"amount": "lots", "radius": true, "shift": [1]}));
assert!(m.resolve_params(&Value::Object(p)).is_err());
// A plug-in without px params is never touched, even with built-in pixel names.
let old = Manifest::parse(br#"{"id":"a","name":"x","kind":"filter","params":{"radius":{"type":"number","min":0,"max":9,"default":4}}}"#).unwrap();
let mut p = json!({"radius": 8}).as_object().unwrap().clone();
old.scale_for_preview(&mut p, 4);
assert_eq!(Value::Object(p), json!({"radius": 8}));
}
#[test]
fn preview_factor_keeps_pixel_params_above_their_minimum() {
let m = Manifest::parse(PIXELS.as_bytes()).unwrap();
let p = |v: Value| v.as_object().unwrap().clone();
// radius min 2: 10 px allows 4, 5 px allows 2, 3 px only full resolution.
assert_eq!(m.preview_factor(&p(json!({"radius": 10})), 4), 4);
assert_eq!(m.preview_factor(&p(json!({"radius": 5})), 4), 2);
assert_eq!(m.preview_factor(&p(json!({"radius": 3})), 4), 1);
// Missing: the default (10). Params with a zero or negative minimum don't limit it.
assert_eq!(m.preview_factor(&Map::new(), 8), 5);
assert_eq!(m.preview_factor(&p(json!({"radius": 50, "amount": 0, "shift": -40})), 8), 8);
// Never more than the document's factor, never 0; bad values don't limit it.
assert_eq!(m.preview_factor(&p(json!({"radius": 50})), 2), 2);
assert_eq!(m.preview_factor(&p(json!({"radius": 50})), 0), 1);
assert_eq!(m.preview_factor(&p(json!({"radius": "x"})), 4), 4);
let unitless = Manifest::parse(FULL.as_bytes()).unwrap();
assert_eq!(unitless.preview_factor(&p(json!({"amount": 0.01})), 4), 4);
}
fn with_name(name: &str) -> Result<Manifest> {
Manifest::parse(json!({"id": "org.example.tone", "name": name, "kind": "filter"}).to_string().as_bytes())
}
+10 -1
View File
@@ -251,6 +251,10 @@ pub fn preview_factor(command: &str, params: &Value, k: u32) -> u32 {
if k <= 1 || full_resolution_preview(command, params) {
return 1;
}
// A plug-in's pixel lengths are the params its manifest marks `"unit": "px"` (#7524).
if command == photocraft_engine::plugin_cmds::RUN {
return photocraft_engine::plugin_cmds::preview_factor(params, k);
}
let Some(spec) = photocraft_engine::commands::find(command) else { return k };
let k = parse_spec(spec.params).into_iter().fold(k, |k, p| match p.kind {
Kind::Range { min, .. } if min > 0.0 && is_pixel_param_of(command, &p.key) => match params.get(&p.key).and_then(Value::as_f64) {
@@ -1066,7 +1070,12 @@ pub(crate) fn preview_document_for_target(
s.select_layer(id).ok()?;
}
s.active_mut()?.channel_view.target = target;
if run_k > 1
if command == photocraft_engine::plugin_cmds::RUN {
// A plug-in declares its pixel lengths (`"unit": "px"` in its manifest), and only those
// are scaled: a 12 px shift previewed 4 times too wide at k = 4, and a unitless param that
// happens to be called `size` or `radius` must not be divided (#7524).
photocraft_engine::plugin_cmds::scale_for_preview(&mut p, run_k);
} else if run_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();
+9
View File
@@ -84,6 +84,7 @@ exports. All integers are `i32` unless noted; pointers are offsets into the modu
"description": "What it does.",
"params": {
"amount": {"type": "number", "min": 0, "max": 100, "default": 50},
"shift": {"type": "number", "min": 0, "max": 40, "default": 4, "unit": "px"},
"steps": {"type": "int", "min": 1, "max": 16, "default": 4},
"mono": {"type": "bool", "default": false},
"mode": {"type": "choice", "options": ["soft", "hard"], "default": "soft"}
@@ -102,6 +103,14 @@ exports. All integers are `i32` unless noted; pointers are offsets into the modu
- `params`: in dialog order. Names are `[A-Za-z0-9_]`, not starting with `_`, not `id`/`layer`.
The host validates the user's values before calling: missing values take the default, numbers are
clamped to `min..max` (ints rounded), a wrong type or an unknown choice is an error.
- `unit` (optional, on `number` and `int` params): `"px"` marks a length in document pixels (a
radius, distance, shift or size); `"none"`, the default, is everything else (levels, factors,
counts, percentages). Any other value is a manifest error. A filter dialog's live preview on a
large document runs on a reduced copy (Preferences › Performance › Low Resolution Previews), and
the host divides each `px` param by that reduction (ints rounded, kept within `min..max`) so the
preview shows the same lengths OK applies; when a `px` param with a positive `min` would drop below
it, the preview runs less reduced instead. Unitless params reach the preview unchanged, whatever
their name. OK, actions and smart filters always get the values as given.
- `overlap` (0–256): pixels of context the filter needs around each output pixel (a blur radius).
The host pads each band by `overlap` on every side, repeating the edge pixels of the canvas (or of
off-canvas layer content), and keeps only the inner rectangle of the result.