From f3cd0509cbf6a731ecab4bc77a7a307a242932e6 Mon Sep 17 00:00:00 2001 From: Piettro Castro Date: Wed, 14 Oct 2026 00:50:01 -0500 Subject: [PATCH] fix(engine): read the documented gradient shorthand on every route (#252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{"type":"linearGradient","stops":["#hex",…]}` was only expanded by the MCP tools, so `figcraft-cli run` and the control channel's `engine.execute` refused it ("`fills` must be paints or colours"). The expansion now lives in the engine (`params::normalize_paint`, used by `paint_of`), so node.set, node.create props, fill.add/set, strokes and paint styles all take it; MCP calls the engine's copy instead of its own. Tests: engine (every paint command, MCP-equal defaults, junk still refused), control channel `engine.execute`, and `figcraft-cli run` end to end. Co-Authored-By: Claude Opus 5.5 --- apps/figcraft-cli/tests/cli.rs | 21 ++++ crates/engine/src/cmd/props.rs | 2 +- crates/engine/src/gradient_shorthand_tests.rs | 107 ++++++++++++++++++ crates/engine/src/lib.rs | 2 + crates/engine/src/params.rs | 74 +++++++++++- crates/mcp/src/tools.rs | 68 +---------- .../ui-egui/src/gradient_shorthand_tests.rs | 21 ++++ crates/ui-egui/src/lib.rs | 2 + docs/control-protocol.md | 4 +- docs/mcp.md | 8 +- 10 files changed, 238 insertions(+), 71 deletions(-) create mode 100644 crates/engine/src/gradient_shorthand_tests.rs create mode 100644 crates/ui-egui/src/gradient_shorthand_tests.rs diff --git a/apps/figcraft-cli/tests/cli.rs b/apps/figcraft-cli/tests/cli.rs index 390e6414..9dd52203 100644 --- a/apps/figcraft-cli/tests/cli.rs +++ b/apps/figcraft-cli/tests/cli.rs @@ -78,6 +78,27 @@ fn run_export_info_convert() { std::fs::remove_dir_all(&d).ok(); } +#[test] +fn run_takes_the_documented_gradient_shorthand() { + // #252: only MCP used to accept it; `run` said "`fills` must be paints or colours". + let d = tmp("gradient"); + let script = d.join("s.json"); + std::fs::write( + &script, + r##"[{"command":"shape.rectangle","params":{"x":0,"y":0,"width":200,"height":100}}, + {"command":"node.set","params":{"ids":[2],"fills":[{"type":"linearGradient","stops":["#ff0000","#0000ff"],"from":[0,0.5],"to":[1,0.5]}]}}]"##, + ) + .unwrap(); + let file = d.join("g.figcraft"); + let out = bin().arg("run").arg(&script).arg("--out").arg(&file).output().unwrap(); + assert!(out.status.success(), "{}{}", String::from_utf8_lossy(&out.stdout), String::from_utf8_lossy(&out.stderr)); + let json = d.join("g.json"); + assert!(bin().arg("convert").arg(&file).arg(&json).status().unwrap().success()); + let doc = std::fs::read_to_string(&json).unwrap(); + assert!(doc.contains("\"kind\":\"linear\"") || doc.contains("\"kind\": \"linear\""), "{doc}"); + std::fs::remove_dir_all(&d).ok(); +} + #[test] fn mcp_over_stdio() { let mut child = bin().arg("mcp").stdin(Stdio::piped()).stdout(Stdio::piped()).stderr(Stdio::null()).spawn().unwrap(); diff --git a/crates/engine/src/cmd/props.rs b/crates/engine/src/cmd/props.rs index 257d1a12..dd2daa82 100644 --- a/crates/engine/src/cmd/props.rs +++ b/crates/engine/src/cmd/props.rs @@ -13,7 +13,7 @@ pub const STROKE_SIDES: [&str; 4] = ["strokeTopWeight", "strokeRightWeight", "st /// Keys `node.set` understands (documented for agents). pub const KEYS: &str = "name, visible, locked, opacity (0-1), blend, x, y (relative to the parent frame; groups don't count), width, height, rotation (degrees), \ -fills (paints or colours), fill (colour), strokes, stroke (colour), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \ +fills (paints or colours; gradients as {type: linearGradient|radialGradient|angularGradient|diamondGradient, stops: [\"#hex\", ...], from?, to?} too), fill (colour), strokes, stroke (colour), strokeWeight, strokeAlign (inside|center|outside), strokeSides [t,r,b,l], strokeTopWeight / strokeRightWeight / strokeBottomWeight / strokeLeftWeight (one side; turns on individual strokes), \ strokeCap / startCap / endCap, strokeJoin, miterAngle (degrees: sharper joins bevel; or miterLimit as a ratio), dashes [..], widthProfile (preset uniform|wedge|taper|eye|rounded|pinch, [[t, scale], ...] or null), brush (preset id, {preset, spacing, sizeJitter, rotationJitter, scatter, seed} or null), cornerRadius (n or [tl,tr,br,bl]), cornerSmoothing (0-1), effects, \ clip, isMask, layout (null|horizontal|vertical|grid|{...}), gap, crossGap, padding (n or [t,r,b,l]), paddingTop/Right/Bottom/Left, \ mainAlign (min|center|max, or the auto gaps spaceBetween|spaceAround|spaceEvenly), crossAlign (min|center|max|baseline: first text baselines, horizontal layouts), contentAlign (wrapped lines: auto = packed where crossAlign says | spaceBetween), wrap (rows of a horizontal layout, columns of a vertical one), strokesIncluded (the frame's inside stroke takes up space), reverseZ (canvas stacking: true = first on top), \ diff --git a/crates/engine/src/gradient_shorthand_tests.rs b/crates/engine/src/gradient_shorthand_tests.rs new file mode 100644 index 00000000..3ea51a89 --- /dev/null +++ b/crates/engine/src/gradient_shorthand_tests.rs @@ -0,0 +1,107 @@ +//! The documented gradient shorthand (`{"type":"linearGradient","stops":["#hex",…]}`) works +//! through every route into the engine, not only MCP (#252): `figcraft-cli run` and the control +//! channel send params straight to `Session::execute`. + +use figcraft_color::Paint; +use figcraft_doc::NodeId; +use serde_json::{Value, json}; + +use crate::Session; +use crate::params::{id_of, paint_of}; + +/// The red→blue left-to-right gradient as written in the docs. +fn short() -> Value { + json!({"type": "linearGradient", "stops": ["#ff0000", "#0000ff"], "from": [0, 0.5], "to": [1, 0.5]}) +} + +/// The same gradient in the engine's own form. +fn long() -> Value { + json!({"type": "gradient", "kind": "linear", + "stops": [{"position": 0, "color": {"r": 1, "g": 0, "b": 0, "a": 1}}, + {"position": 1, "color": {"r": 0, "g": 0, "b": 1, "a": 1}}], + "from": [0, 0.5], "to": [1, 0.5], "width": [0, 1.5]}) +} + +fn rect(s: &mut Session) -> NodeId { + id_of(&s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()["id"]).unwrap() +} + +fn fills(s: &Session, id: NodeId) -> Vec { + s.doc().unwrap().node(id).unwrap().fills.clone() +} + +#[test] +fn the_shorthand_reads_as_the_long_form() { + assert_eq!(paint_of(&short()).expect("the documented shorthand is a paint"), paint_of(&long()).unwrap()); +} + +#[test] +fn node_set_takes_the_shorthand() { + let mut s = Session::new(); + let r = rect(&mut s); + s.execute("node.set", &json!({"ids": [r], "fills": [short()]})).unwrap(); + s.execute("node.set", &json!({"ids": [r], "strokes": [short()]})).unwrap(); + let want = paint_of(&long()).unwrap(); + assert_eq!(fills(&s, r), vec![want.clone()]); + assert_eq!(s.doc().unwrap().node(r).unwrap().strokes, vec![want]); +} + +#[test] +fn node_create_fill_add_and_fill_set_take_the_shorthand() { + let want = paint_of(&long()).unwrap(); + let mut s = Session::new(); + let v = s.execute("node.create", &json!({"type": "rectangle", "width": 200, "height": 100, "props": {"fills": [short()]}})).unwrap(); + let r = id_of(&v["id"]).unwrap(); + assert_eq!(fills(&s, r), vec![want.clone()]); + s.execute("fill.add", &json!({"ids": [r], "paint": short()})).unwrap(); + assert_eq!(fills(&s, r), vec![want.clone(), want.clone()]); + let r2 = rect(&mut s); + s.execute("fill.set", &json!({"ids": [r2], "index": 0, "paint": short()})).unwrap(); + assert_eq!(fills(&s, r2), vec![want]); +} + +#[test] +fn shorthand_defaults_match_the_mcp_server() { + // No handles: top to bottom, with a square width handle. + let p = paint_of(&json!({"type": "linearGradient", "stops": ["#000000", "#ffffff"]})).unwrap(); + let v = serde_json::to_value(&p).unwrap(); + assert_eq!( + (v["kind"].clone(), v["from"].clone(), v["to"].clone(), v["width"].clone()), + (json!("linear"), json!([0.5, 0.0]), json!([0.5, 1.0]), json!([-0.5, 0.0])) + ); + // Plain colour stops are spread evenly. + let p = paint_of(&json!({"type": "radialGradient", "stops": ["#000000", "#808080", "#ffffff"]})).unwrap(); + let v = serde_json::to_value(&p).unwrap(); + assert_eq!(v["kind"], "radial"); + let pos: Vec = v["stops"].as_array().unwrap().iter().map(|s| s["position"].as_f64().unwrap()).collect(); + assert_eq!(pos, vec![0.0, 0.5, 1.0]); + for kind in ["angular", "diamond"] { + let p = paint_of(&json!({"type": format!("{kind}Gradient"), "stops": ["#000000", "#ffffff"]})).unwrap(); + assert_eq!(serde_json::to_value(&p).unwrap()["kind"], kind); + } + // A CSS colour inside a solid paint or a positioned stop is read too. + assert_eq!(paint_of(&json!({"type": "solid", "color": "#00ff00"})), paint_of(&json!("#00ff00"))); + let p = paint_of(&json!({"type": "gradient", "stops": [{"position": 0, "color": "#ff0000"}, {"position": 1, "color": "#0000ff"}], "from": [0, 0.5], "to": [1, 0.5]})).unwrap(); + assert_eq!(p, paint_of(&long()).unwrap()); +} + +#[test] +fn junk_is_still_refused() { + let mut s = Session::new(); + let r = rect(&mut s); + for junk in [ + json!({"type": "linearGradient", "stops": "red"}), + json!({"type": "linearGradient", "stops": [42]}), + json!({"type": "fooGradient", "stops": ["#ff0000"]}), + json!({"type": "linearGradient", "stops": ["#ff0000"], "from": "left"}), + ] { + let e = s.execute("node.set", &json!({"ids": [r], "fills": [junk.clone()]})).unwrap_err().to_string(); + assert!(e.contains("`fills` must be paints or colours"), "{junk}: {e}"); + } + // Deep nesting doesn't overflow the stack. + let mut deep = json!("#ff0000"); + for _ in 0..200 { + deep = json!({"type": "linearGradient", "stops": [{"position": 0, "color": deep}]}); + } + assert!(paint_of(&deep).is_none()); +} diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 3bc49a90..ac0ef938 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -1467,6 +1467,8 @@ mod fx_behaviour_tests; #[cfg(test)] mod gesture_preview_tests; #[cfg(test)] +mod gradient_shorthand_tests; +#[cfg(test)] mod hide_publishing_tests; #[cfg(test)] mod history_behaviour_tests; diff --git a/crates/engine/src/params.rs b/crates/engine/src/params.rs index 4eca1444..bbaa42a4 100644 --- a/crates/engine/src/params.rs +++ b/crates/engine/src/params.rs @@ -3,7 +3,7 @@ use figcraft_color::{BlendMode, Color, Paint}; use figcraft_doc::NodeId; -use serde_json::Value; +use serde_json::{Value, json}; use crate::{EngineError, Result, Session, bad}; @@ -135,12 +135,80 @@ pub fn color(p: &Value, key: &str) -> Option { get(p, key).and_then(color_of) } -/// A paint from JSON: a colour string/object (solid) or a full paint object. +/// Read the friendlier paint/effect JSON that the docs and MCP tools describe as the engine's own +/// form: any `"color"` given as a CSS string ("#RRGGBB[AA]", "rgb(…)") becomes {r,g,b,a}, +/// `{"type":"linearGradient"}` becomes `{"type":"gradient","kind":"linear"}`, gradients get default +/// handles (top to bottom) when `from`/`to`/`width` are missing, and stops given as plain colours +/// are spread evenly. Anything else is left for serde to refuse. +pub fn normalize_paint(v: &mut Value) { + normalize_paintish(v, 0); +} + +fn normalize_paintish(v: &mut Value, depth: usize) { + if depth > 16 { + return; + } + match v { + Value::Array(a) => { + for x in a { + normalize_paintish(x, depth + 1); + } + } + Value::Object(o) => { + if let Some(c) = o.get("color").filter(|c| c.is_string()).and_then(color_of) { + o.insert("color".into(), serde_json::to_value(c).unwrap_or(Value::Null)); + } + let is_gradient = o.get("type").and_then(Value::as_str).is_some_and(|t| t.to_ascii_lowercase().contains("gradient")); + if is_gradient { + // Accept {"type":"linearGradient"} as well as {"type":"gradient","kind":"linear"}. + if let Some(t) = o.get("type").and_then(Value::as_str).map(str::to_ascii_lowercase) + && t != "gradient" + { + let kind = t.trim_end_matches("gradient").to_string(); + o.insert("type".into(), json!("gradient")); + o.entry("kind").or_insert(json!(if kind.is_empty() { "linear".to_string() } else { kind })); + } + o.entry("kind").or_insert(json!("linear")); + let linear = o.get("kind").and_then(Value::as_str) == Some("linear"); + let (from, to, width) = if linear { ([0.5, 0.0], [0.5, 1.0], [-0.5, 0.0]) } else { ([0.5, 0.5], [0.5, 1.0], [1.0, 0.5]) }; + o.entry("from").or_insert(json!(from)); + o.entry("to").or_insert(json!(to)); + // A linear gradient's width handle goes square to its axis (else its bands skew, + // or vanish when the handle lies on the axis). + let pt = |k: &str| o.get(k).and_then(Value::as_array).and_then(|a| Some((a.first()?.as_f64()?, a.get(1)?.as_f64()?))); + if linear + && !o.contains_key("width") + && let (Some(f), Some(t)) = (pt("from"), pt("to")) + { + o.insert("width".into(), json!([f.0 - (t.1 - f.1), f.1 + (t.0 - f.0)])); + } + o.entry("width").or_insert(json!(width)); + if let Some(Value::Array(stops)) = o.get_mut("stops") { + let n = stops.len(); + for (i, s) in stops.iter_mut().enumerate() { + if let Some(c) = color_of(s).filter(|_| s.is_string()) { + let pos = if n > 1 { i as f64 / (n - 1) as f64 } else { 0.0 }; + *s = json!({"position": pos, "color": c}); + } else { + normalize_paintish(s, depth + 1); + } + } + } + } + } + _ => {} + } +} + +/// A paint from JSON: a colour string/object (solid), a full paint object or its shorthand +/// ([`normalize_paint`]). pub fn paint_of(v: &Value) -> Option { if let Some(c) = color_of(v) { return Some(Paint::solid(c)); } - let mut p: Paint = serde_json::from_value(v.clone()).ok()?; + let mut v = v.clone(); + normalize_paint(&mut v); + let mut p: Paint = serde_json::from_value(v).ok()?; p.opacity = if p.opacity.is_finite() { p.opacity.clamp(0.0, 1.0) } else { 1.0 }; Some(p) } diff --git a/crates/mcp/src/tools.rs b/crates/mcp/src/tools.rs index b8d3f3ad..9f4d5b8f 100644 --- a/crates/mcp/src/tools.rs +++ b/crates/mcp/src/tools.rs @@ -3,6 +3,7 @@ //! behave the same. use base64::Engine as _; +use figcraft_engine::params::normalize_paint; use serde_json::{Map, Value, json}; use crate::backend::{Backend, decode_data, page_children}; @@ -369,74 +370,17 @@ fn bounds_of(b: &mut dyn Backend, ids: &[u64]) -> Option<(f64, f64, f64, f64)> { acc } -/// Make paint/effect JSON friendlier than the engine's strict serde: any `"color"` given as a CSS -/// string ("#RRGGBB[AA]", "rgb(…)") becomes the engine's {r,g,b,a} object, gradient paints get -/// default handles (top-to-bottom) when `from`/`to`/`width` are missing, and gradient stops given -/// as plain colours are spread evenly. +/// Make paint/effect JSON friendlier than the engine's strict serde (see +/// [`figcraft_engine::params::normalize_paint`]). The engine reads paints this way itself; effects +/// still need it here. pub fn normalize_props(props: &mut Map) { for k in ["fills", "strokes", "effects"] { if let Some(v) = props.get_mut(k) { - normalize_paintish(v, 0); + normalize_paint(v); } } } -fn normalize_paintish(v: &mut Value, depth: usize) { - if depth > 16 { - return; - } - match v { - Value::Array(a) => { - for x in a { - normalize_paintish(x, depth + 1); - } - } - Value::Object(o) => { - if let Some(c) = o.get("color").filter(|c| c.is_string()).and_then(figcraft_engine::params::color_of) { - o.insert("color".into(), serde_json::to_value(c).unwrap_or(Value::Null)); - } - let is_gradient = o.get("type").and_then(Value::as_str).is_some_and(|t| t.to_ascii_lowercase().contains("gradient")); - if is_gradient { - // Accept {"type":"linearGradient"} as well as {"type":"gradient","kind":"linear"}. - if let Some(t) = o.get("type").and_then(Value::as_str).map(str::to_ascii_lowercase) - && t != "gradient" - { - let kind = t.trim_end_matches("gradient").to_string(); - o.insert("type".into(), json!("gradient")); - o.entry("kind").or_insert(json!(if kind.is_empty() { "linear".to_string() } else { kind })); - } - o.entry("kind").or_insert(json!("linear")); - let linear = o.get("kind").and_then(Value::as_str) == Some("linear"); - let (from, to, width) = if linear { ([0.5, 0.0], [0.5, 1.0], [-0.5, 0.0]) } else { ([0.5, 0.5], [0.5, 1.0], [1.0, 0.5]) }; - o.entry("from").or_insert(json!(from)); - o.entry("to").or_insert(json!(to)); - // A linear gradient's width handle goes square to its axis (else its bands skew, - // or vanish when the handle lies on the axis). - let pt = |k: &str| o.get(k).and_then(Value::as_array).and_then(|a| Some((a.first()?.as_f64()?, a.get(1)?.as_f64()?))); - if linear - && !o.contains_key("width") - && let (Some(f), Some(t)) = (pt("from"), pt("to")) - { - o.insert("width".into(), json!([f.0 - (t.1 - f.1), f.1 + (t.0 - f.0)])); - } - o.entry("width").or_insert(json!(width)); - if let Some(Value::Array(stops)) = o.get_mut("stops") { - let n = stops.len(); - for (i, s) in stops.iter_mut().enumerate() { - if let Some(c) = figcraft_engine::params::color_of(s).filter(|_| s.is_string()) { - let pos = if n > 1 { i as f64 / (n - 1) as f64 } else { 0.0 }; - *s = json!({"position": pos, "color": c}); - } else { - normalize_paintish(s, depth + 1); - } - } - } - } - } - _ => {} - } -} - /// [`normalize_props`] for command params: `props`, flat node.set keys, `paint`, `effect`. fn friendly_params(p: &mut Value) { let Some(o) = p.as_object_mut() else { return }; @@ -446,7 +390,7 @@ fn friendly_params(p: &mut Value) { normalize_props(o); for k in ["paint", "effect"] { if let Some(v) = o.get_mut(k) { - normalize_paintish(v, 0); + normalize_paint(v); } } } diff --git a/crates/ui-egui/src/gradient_shorthand_tests.rs b/crates/ui-egui/src/gradient_shorthand_tests.rs new file mode 100644 index 00000000..db368f8e --- /dev/null +++ b/crates/ui-egui/src/gradient_shorthand_tests.rs @@ -0,0 +1,21 @@ +//! The control channel takes the documented gradient shorthand, as MCP does (#252). + +use figcraft_engine::Session; +use serde_json::json; + +use crate::layers_drag_tests::Ui; + +#[test] +fn engine_execute_takes_the_gradient_shorthand() { + let mut u = Ui::new(Session::new()); + let exec = |u: &mut Ui, command: &str, params: serde_json::Value| { + crate::control::handle(&mut u.app, &u.ctx.clone(), "engine.execute", &json!({"command": command, "params": params})) + }; + let r = exec(&mut u, "shape.rectangle", json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()["id"].clone(); + let short = json!({"type": "linearGradient", "stops": ["#ff0000", "#0000ff"], "from": [0, 0.5], "to": [1, 0.5]}); + exec(&mut u, "node.set", json!({"ids": [r], "fills": [short]})).unwrap(); + let n = exec(&mut u, "node.get", json!({"id": r, "depth": 0})).unwrap(); + let f = &n["fills"][0]; + assert_eq!((f["type"].clone(), f["kind"].clone(), f["width"].clone()), (json!("gradient"), json!("linear"), json!([0.0, 1.5])), "{n}"); + assert_eq!(f["stops"].as_array().map(Vec::len), Some(2), "{n}"); +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 613f18f9..85fa092d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -84,6 +84,8 @@ mod frame_label_tests; #[cfg(test)] mod gap_handle_tests; #[cfg(test)] +mod gradient_shorthand_tests; +#[cfg(test)] mod home_recents_tests; #[cfg(test)] mod home_tests; diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 9fc9937d..3a4d10ae 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -51,7 +51,9 @@ unknown id is an error. Run any command (the same ids as the menus, shortcuts, Actions palette, CLI and MCP). Programmatic calls never open dialogs: dialog requests are dropped. Returns the command's result (creation -commands return `{"id": N}`). +commands return `{"id": N}`). Paints (`fills`, `strokes`, `paint`) take a colour, the full paint +JSON or the gradient shorthand `{"type":"linearGradient","stops":["#hex","#hex"],"from"?,"to"?}` +(as in `docs/mcp.md`). ```json {"id":1,"method":"engine.execute","params":{"command":"shape.rectangle","params":{"x":10,"y":10,"width":100,"height":50}}} diff --git a/docs/mcp.md b/docs/mcp.md index 2b66657c..11f9c056 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -128,10 +128,10 @@ Or in a project's `.mcp.json`: resolved `gridCell`. - **Every menu action is a command** (`list_commands`), and most commands act on `ids` when given, else on the selection. New layers become the selection. -- Colours are `"#RRGGBB"` or `"#RRGGBBAA"`. Inside fills, strokes and effects passed through the MCP - tools, `color` may also be a hex string and gradients can be written - `{"type": "linearGradient", "stops": ["#7048E8", "#1098AD"]}`; the server converts them to the - engine's paint JSON. +- Colours are `"#RRGGBB"` or `"#RRGGBBAA"`. Inside fills and strokes, `color` may also be a hex + string and gradients can be written `{"type": "linearGradient", "stops": ["#7048E8", "#1098AD"]}`; + the engine reads them as its paint JSON, so `run_command`, `figcraft-cli run` and the control + channel take them too. Effects passed through the MCP tools may use hex `color` strings as well. - **Blend modes**: `blend` on a layer (`node.set`) or a paint (`fill.set` / `stroke.set`) is a menu label or CSS keyword (`"Normal"`, `"pass through"`, `"multiply"`, `"plusDarker"`, ...; `normal` is Normal). Groups and frames start as pass-through: their children blend with what is