From 009314662cedbb44debf0323d96c61581806fabe Mon Sep 17 00:00:00 2001 From: BFlat Date: Sat, 10 Oct 2026 13:17:09 +0700 Subject: [PATCH] M3: variable bindings follow paint and effect lists (#73) A bound fill or stroke colour now sets only the first paint's colour, keeping its opacity, visibility and blend mode and the rest of the list (it used to replace every fill with one solid paint). Effect bindings (`effects//`) follow their effect when an effect is removed or moved with the new `effect.move {ids?, from, to}`; removing a bound effect or the first fill/stroke drops its bindings instead of passing them to a neighbour. `effect.remove` rejects negative indices. Co-authored-by: Claude Opus 5.5 --- ROADMAP.md | 2 +- crates/engine/src/binding_lists_tests.rs | 99 ++++++++++++++++++++++++ crates/engine/src/cmd/edit.rs | 36 ++++++++- crates/engine/src/cmd/styles.rs | 60 +++++++++++++- crates/engine/src/lib.rs | 2 + 5 files changed, 196 insertions(+), 3 deletions(-) create mode 100644 crates/engine/src/binding_lists_tests.rs diff --git a/ROADMAP.md b/ROADMAP.md index 73e866f..355caea 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -235,7 +235,7 @@ Scores by behaviour, 0–100%. Hours are what remains (one agent). | Auto layout & constraints | 9 | 72% | visual QA of nested edge cases, arrow-key moves across nested auto layout frames | 2–4 | | Text | 8 | 67% | right-to-left layer names in the panels, text style picker depth, axis hover preview | 5–8 | | Components & variants | 9 | 76% | QA with larger real-world libraries | 1 | -| Styles & variables | 5 | 84% | bindings that follow effects when they are reordered, grid and component property bindings | 1–2 | +| Styles & variables | 5 | 86% | grid and component property bindings, reordering paints and effects in the panel | 1–2 | | Prototyping | 6 | 84% | building conditionals in the interaction editor, hotspot hints, comments and inspect in the presenter | 3–6 | | Import/export | 6 | 66% | .fig import (best effort), variable-mode lookups in prototype expressions, PDF print options (bleed, CMYK) | 10–20 | | Dev mode / inspect | 2 | 78% | component code with properties as parameters, visual QA | 2–3 | diff --git a/crates/engine/src/binding_lists_tests.rs b/crates/engine/src/binding_lists_tests.rs new file mode 100644 index 0000000..8e83c86 --- /dev/null +++ b/crates/engine/src/binding_lists_tests.rs @@ -0,0 +1,99 @@ +//! Variable bindings on paint and effect lists: a bound fill colour changes only the first fill, +//! effect bindings follow their effects when the list is reordered or shrinks, and removing the +//! first fill takes its binding with it. + +use figcraft_color::{Color, PaintKind}; +use figcraft_doc::{EffectKind, NodeId}; +use serde_json::json; + +use crate::Session; + +fn id(v: serde_json::Value) -> NodeId { + NodeId(v["id"].as_u64().unwrap()) +} + +fn ex(s: &mut Session, cmd: &str, p: serde_json::Value) { + s.execute(cmd, &p).unwrap_or_else(|e| panic!("{cmd}: {e}")); +} + +fn setup() -> (Session, NodeId) { + let mut s = Session::new(); + let r = id(s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap()); + ex(&mut s, "variable.createCollection", json!({"name": "T"})); + for (name, kind, value) in [("brand", "color", json!("#FF0000")), ("small", "number", json!(2)), ("large", "number", json!(30))] { + ex(&mut s, "variable.create", json!({"collection": "T", "name": name, "type": kind, "value": value})); + } + (s, r) +} + +fn blur(s: &Session, r: NodeId, i: usize) -> f64 { + match s.doc().unwrap().node(r).unwrap().effects.get(i).map(|e| &e.kind) { + Some(EffectKind::LayerBlur { radius } | EffectKind::BackgroundBlur { radius }) => *radius, + k => panic!("{k:?}"), + } +} + +fn bound(s: &Session, r: NodeId) -> Vec<(String, String)> { + s.doc().unwrap().node(r).unwrap().bound.iter().map(|(k, v)| (k.clone(), v.to_string())).collect() +} + +#[test] +fn a_bound_fill_colour_keeps_the_other_fills() { + let (mut s, r) = setup(); + ex(&mut s, "fill.add", json!({"ids": [r], "color": "#00FF00"})); + ex(&mut s, "fill.set", json!({"ids": [r], "index": 0, "color": "#0000FF", "opacity": 0.5})); + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "fill", "variable": "brand"})); + let fills = s.doc().unwrap().node(r).unwrap().fills.clone(); + assert_eq!(fills.len(), 2, "the second fill stays"); + assert_eq!(fills[0].kind, PaintKind::Solid { color: Color::from_rgb8_const(0xFF, 0, 0) }); + assert_eq!(fills[0].opacity, 0.5, "the bound paint keeps its opacity"); + assert_eq!(fills[1].kind, PaintKind::Solid { color: Color::from_rgb8_const(0, 0xFF, 0) }); + // Removing the bound (first) fill takes the binding with it. + ex(&mut s, "fill.remove", json!({"ids": [r], "index": 0})); + assert!(bound(&s, r).is_empty()); + ex(&mut s, "variable.set", json!({"variable": "brand", "value": "#123456"})); + let fills = s.doc().unwrap().node(r).unwrap().fills.clone(); + assert_eq!(fills.len(), 1); + assert_eq!(fills[0].kind, PaintKind::Solid { color: Color::from_rgb8_const(0, 0xFF, 0) }, "the remaining fill isn't recoloured"); +} + +#[test] +fn effect_bindings_follow_their_effects() { + let (mut s, r) = setup(); + for _ in 0..3 { + ex(&mut s, "effect.add", json!({"ids": [r], "type": "layerBlur"})); + } + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "effects/0/radius", "variable": "small"})); + ex(&mut s, "variable.bind", json!({"ids": [r], "field": "effects/2/radius", "variable": "large"})); + assert_eq!((blur(&s, r, 0), blur(&s, r, 2)), (2.0, 30.0)); + // Move the "small" effect to the top: its binding goes with it. + ex(&mut s, "effect.move", json!({"ids": [r], "from": 0, "to": 2})); + assert_eq!((blur(&s, r, 1), blur(&s, r, 2)), (30.0, 2.0)); + ex(&mut s, "variable.set", json!({"variable": "small", "value": 5})); + assert_eq!((blur(&s, r, 1), blur(&s, r, 2)), (30.0, 5.0)); + // Removing the effect under the "large" one shifts its binding down. + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 0})); + ex(&mut s, "variable.set", json!({"variable": "large", "value": 40})); + assert_eq!((blur(&s, r, 0), blur(&s, r, 1)), (40.0, 5.0)); + // Removing a bound effect drops its binding. + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 1})); + let keys: Vec = bound(&s, r).into_iter().map(|(k, _)| k).collect(); + assert_eq!(keys, vec!["effects/0/radius".to_string()]); + // Undo restores the list and its bindings together. + s.undo().unwrap(); + assert_eq!(bound(&s, r).len(), 2); +} + +#[test] +fn effect_moves_and_removals_reject_junk() { + let (mut s, r) = setup(); + ex(&mut s, "effect.add", json!({"ids": [r], "type": "layerBlur"})); + for bad in [json!({"ids": [r], "from": -1, "to": 0}), json!({"ids": [r], "from": 0}), json!({"ids": [r], "from": "x", "to": 0})] { + assert!(s.execute("effect.move", &bad).is_err(), "{bad}"); + } + assert!(s.execute("effect.remove", &json!({"ids": [r], "index": -1})).is_err()); + // Out-of-range indices change nothing. + ex(&mut s, "effect.move", json!({"ids": [r], "from": 0, "to": 5})); + ex(&mut s, "effect.remove", json!({"ids": [r], "index": 7})); + assert_eq!(s.doc().unwrap().node(r).unwrap().effects.len(), 1); +} diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index bedf76d..b581db5 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -206,15 +206,42 @@ pub fn specs() -> Vec { effect_add ), spec!("effect.remove", "Remove effect", "{ids?, index}", true, |s, p| { - let i = req_num("effect.remove", p, "index")? as usize; + let i = req_index("effect.remove", p, "index")?; for id in req_targets("effect.remove", s, p)? { let n = s.doc_mut()?.get_mut(id)?; if i < n.effects.len() { n.effects.remove(i); + // Its bindings go with it; later effects' move down one. + super::styles::remap_effect_bindings(n, |k| match k.cmp(&i) { + std::cmp::Ordering::Less => Some(k), + std::cmp::Ordering::Equal => None, + std::cmp::Ordering::Greater => Some(k - 1), + }); } } ok() }), + spec!("effect.move", "Move effect", "{ids?, from, to} (indices in the effect list, bottom first)", true, |s, p| { + let from = req_index("effect.move", p, "from")?; + let to = req_index("effect.move", p, "to")?; + for id in req_targets("effect.move", s, p)? { + let n = s.doc_mut()?.get_mut(id)?; + if from >= n.effects.len() || to >= n.effects.len() || from == to { + continue; + } + let e = n.effects.remove(from); + n.effects.insert(to, e); + // Bindings follow their effects. + super::styles::remap_effect_bindings(n, |k| { + if k == from { + return Some(to); + } + let k = if k > from { k - 1 } else { k }; + Some(if k >= to { k + 1 } else { k }) + }); + } + ok() + }), spec!("effect.set", "Set effect", "{ids?, index, effect {..}}", true, |s, p| { let i = req_num("effect.set", p, "index")? as usize; let e: Effect = parse("effect.set", p, "effect")?.ok_or_else(|| bad("effect.set", "`effect` required"))?; @@ -984,6 +1011,13 @@ fn paint_list(s: &mut Session, p: &Value, fills: bool, add: bool) -> Result = Vec::new(); + let mut paint_sets: Vec<(bool, Color)> = Vec::new(); for (field, var) in binds { let Some(val) = doc.variables.resolve(&var, &modes) else { continue }; + // The first fill's or stroke's colour (the rest of the list stays). + let paint = match field.as_str() { + "fill" | "fills" | "fills/0" | "fills/0/color" => Some(true), + "stroke" | "strokes/0/color" => Some(false), + _ => None, + }; + if let (Some(fills), VariableValue::Color(c)) = (paint, &val) { + paint_sets.push((fills, *c)); + continue; + } // `effects//`: one effect's value. if let Some((i, prop)) = field.strip_prefix("effects/").and_then(|r| r.split_once('/')) && let Ok(i) = i.parse::() @@ -681,12 +692,59 @@ pub fn apply_bindings(doc: &mut Document) { sets.insert(key.to_string(), jv); } let _ = super::props::apply("variables", doc, id, &sets); + if !paint_sets.is_empty() { + apply_paint_bindings(doc, id, &paint_sets); + } if !effect_sets.is_empty() { apply_effect_bindings(doc, id, &effect_sets); } } } +/// Write bound colours into a node's first fill (`true`) or stroke, keeping the rest of the list and +/// the paint's opacity, visibility and blend mode (only what changed, so unchanged layers stay +/// shared). +fn apply_paint_bindings(doc: &mut Document, id: NodeId, sets: &[(bool, Color)]) { + let Some(n) = doc.node(id) else { return }; + let (mut fills, mut strokes) = (n.fills.clone(), n.strokes.clone()); + for (is_fill, c) in sets { + let list = if *is_fill { &mut fills } else { &mut strokes }; + match list.first_mut() { + Some(p) => p.kind = PaintKind::Solid { color: *c }, + None => list.push(Paint::solid(*c)), + } + } + if doc.node(id).is_some_and(|n| n.fills != fills || n.strokes != strokes) + && let Ok(n) = doc.get_mut(id) + { + n.fills = fills; + n.strokes = strokes; + } +} + +/// Keep a node's bindings on its effects' fields (`effects//`) with their effects +/// when the list changes: `map` gives an effect's new index, or None when it was removed. +pub(crate) fn remap_effect_bindings(n: &mut Node, map: impl Fn(usize) -> Option) { + let moved: Vec<(String, Option)> = n + .bound + .keys() + .filter_map(|k| { + let (i, field) = k.strip_prefix("effects/")?.split_once('/')?; + let i = i.parse::().ok()?; + Some((k.clone(), map(i).map(|j| format!("effects/{j}/{field}")))) + }) + .collect(); + let mut kept = Vec::new(); + for (old, new) in moved { + if let Some(var) = n.bound.remove(&old) + && let Some(new) = new + { + kept.push((new, var)); + } + } + n.bound.extend(kept); +} + /// Write bound values into a node's effects (only what changed, so unchanged layers stay shared). fn apply_effect_bindings(doc: &mut Document, id: NodeId, sets: &[(usize, String, VariableValue)]) { let Some(mut effects) = doc.node(id).map(|n| n.effects.clone()) else { return }; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7bdc9dc..8ee5acb 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -904,6 +904,8 @@ mod al_nesting_tests; #[cfg(test)] mod autosave_tests; #[cfg(test)] +mod binding_lists_tests; +#[cfg(test)] mod devinspect_tests; #[cfg(test)] mod effects_tests;