diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 2191f20db..26d3ba512 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -477,10 +477,12 @@ pub(crate) fn ok() -> Result { } /// Leaves whose appearance a paint command changes: groups and layers expand to their contents, -/// and compound paths own their children's appearance. +/// and compound paths own their children's appearance, so a member stands for its compound path +/// (#3692), which is listed once. pub(crate) fn leaf_targets(s: &Session, ids: &[NodeId]) -> Result> { use vectorcraft_doc::NodeKind; let d = &s.doc()?.doc; + let compound = |id: NodeId| d.parent_of(id).filter(|p| matches!(d.node(*p).map(|n| &n.kind), Some(NodeKind::Compound { .. }))); let mut out = vec![]; for id in ids { let Some(n) = d.node(*id) else { continue }; @@ -490,11 +492,13 @@ pub(crate) fn leaf_targets(s: &Session, ids: &[NodeId]) -> Result> { out.push(c.id) } }), - _ => out.push(*id), + _ => out.push(compound(*id).unwrap_or(*id)), } } let comp: Vec = out.iter().filter(|id| matches!(d.node(**id).map(|n| &n.kind), Some(NodeKind::Compound { .. }))).copied().collect(); out.retain(|id| !comp.iter().any(|c| d.parent_of(*id) == Some(*c))); + let mut seen = std::collections::HashSet::new(); + out.retain(|id| seen.insert(*id)); Ok(out) } diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f5854d3e8..bcd8f58a3 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2321,6 +2321,8 @@ mod tests_colormgmt; #[cfg(test)] mod tests_colorthemes; #[cfg(test)] +mod tests_compoundpaint; +#[cfg(test)] mod tests_containers; #[cfg(test)] mod tests_css; diff --git a/crates/engine/src/tests_cmdsplit.rs b/crates/engine/src/tests_cmdsplit.rs index 12e7f56d6..767b82b4c 100644 --- a/crates/engine/src/tests_cmdsplit.rs +++ b/crates/engine/src/tests_cmdsplit.rs @@ -77,6 +77,12 @@ fn leaf_targets_expand_groups_and_stop_at_compound_paths() { s.execute("select.set", &json!({"ids": [c.0, d.0]})).unwrap(); let comp = id_of(&s.execute("object.compoundPath.make", &json!({})).unwrap()); assert_eq!(leaf_targets(&s, &[comp]).unwrap(), vec![comp]); + // A member stands for its compound path, listed once (#3692). + let members: Vec = s.doc().unwrap().doc.node(comp).unwrap().children().into_iter().flatten().map(|m| m.id).collect(); + assert_eq!(members.len(), 2); + assert_eq!(leaf_targets(&s, &[members[0]]).unwrap(), vec![comp]); + assert_eq!(leaf_targets(&s, &[members[0], comp, members[1]]).unwrap(), vec![comp]); + assert_eq!(leaf_targets(&s, &[members[1], members[0]]).unwrap(), vec![comp]); // `paint_targets` reads `ids`, then `id`, then the selection. assert_eq!(paint_targets(&s, &json!({"ids": [g.0]})).unwrap(), vec![a, b]); assert_eq!(paint_targets(&s, &json!({"id": comp.0})).unwrap(), vec![comp]); diff --git a/crates/engine/src/tests_compoundpaint.rs b/crates/engine/src/tests_compoundpaint.rs new file mode 100644 index 000000000..8c81ef2ce --- /dev/null +++ b/crates/engine/src/tests_compoundpaint.rs @@ -0,0 +1,89 @@ +//! Fill and stroke given to a path of a compound path paint the compound path (#3692): the +//! compound holds the paint, so a member's own appearance is never drawn. + +use serde_json::{Value, json}; +use vectorcraft_color::Paint; + +use super::*; + +fn run(s: &mut Session, id: &str, p: Value) -> Value { + s.execute(id, &p).unwrap() +} + +fn rect(s: &mut Session, x: f64) -> NodeId { + NodeId(run(s, "shape.rectangle", json!({"x": x, "y": 0, "width": 40, "height": 40}))["id"].as_u64().unwrap()) +} + +/// A compound path of two black rectangles and a loose black rectangle → (compound, members, loose). +fn compound() -> (Session, NodeId, Vec, NodeId) { + let mut s = Session::new(); + run(&mut s, "file.new", json!({"width": 200, "height": 100})); + let (a, b) = (rect(&mut s, 0.0), rect(&mut s, 10.0)); + run(&mut s, "select.set", json!({"ids": [a.0, b.0]})); + run(&mut s, "object.compoundPath.make", json!({})); + let compound = s.doc().unwrap().selection.objects[0]; + let loose = rect(&mut s, 100.0); + run(&mut s, "paint.setFill", json!({"color": "#000000", "ids": [compound.0, loose.0]})); + run(&mut s, "paint.setStroke", json!({"color": "#000000", "ids": [compound.0, loose.0]})); + let doc = &s.doc().unwrap().doc; + let members: Vec = doc.node(compound).unwrap().children().into_iter().flatten().map(|m| m.id).collect(); + assert_eq!(members.len(), 2); + (s, compound, members, loose) +} + +/// The hex of `id`'s own top fill (`fill`) or stroke, `None` when it has none or it isn't solid. +fn paint(s: &Session, id: NodeId, fill: bool) -> Option { + match s.doc().unwrap().doc.node(id).unwrap().appearance.paint_at(None, fill)? { + Paint::Solid { color, .. } => Some(color.to_hex()), + _ => None, + } +} + +fn solid(hex: &str) -> Option { + Some(hex.to_string()) +} + +#[test] +fn a_fill_given_to_a_member_id_paints_its_compound() { + let (mut s, compound, members, loose) = compound(); + let member_paint = paint(&s, members[0], true); + run(&mut s, "paint.setFill", json!({"color": "#ff00ff", "ids": [members[0].0, members[1].0, loose.0]})); + assert_eq!(paint(&s, compound, true), solid("#ff00ff"), "the compound path takes the fill"); + assert_eq!(paint(&s, loose, true), solid("#ff00ff")); + assert_eq!(paint(&s, members[0], true), member_paint, "the member's own appearance is left alone"); + assert_eq!(paint(&s, compound, false), solid("#000000"), "the stroke stays"); + run(&mut s, "edit.undo", json!({})); + assert_eq!(paint(&s, compound, true), solid("#000000"), "one undo step"); +} + +#[test] +fn a_stroke_given_to_a_selected_member_paints_its_compound() { + let (mut s, compound, members, _) = compound(); + run(&mut s, "select.set", json!({"ids": [members[1].0]})); + run(&mut s, "paint.setStroke", json!({"color": "#00ff00"})); + assert_eq!(paint(&s, compound, false), solid("#00ff00")); + run(&mut s, "paint.setFill", json!({"color": "#0000ff"})); + assert_eq!(paint(&s, compound, true), solid("#0000ff")); +} + +#[test] +fn the_members_of_one_compound_are_one_target() { + let (mut s, compound, members, _) = compound(); + run(&mut s, "paint.setFill", json!({"color": "#ff0000", "ids": [compound.0]})); + // Swapped once per id, the two members would swap the compound path back. + run(&mut s, "paint.swap", json!({"ids": [members[0].0, members[1].0]})); + assert_eq!(paint(&s, compound, true), solid("#000000"), "swapped once, not once per member"); + assert_eq!(paint(&s, compound, false), solid("#ff0000")); +} + +#[test] +fn a_group_with_a_compound_still_paints_the_compound_once() { + let (mut s, compound, _, loose) = compound(); + run(&mut s, "select.set", json!({"ids": [compound.0, loose.0]})); + let group = NodeId(run(&mut s, "object.group", json!({}))["id"].as_u64().unwrap()); + run(&mut s, "paint.setFill", json!({"color": "#ff0000", "ids": [group.0]})); + run(&mut s, "paint.swap", json!({"ids": [group.0]})); + assert_eq!(paint(&s, compound, true), solid("#000000")); + assert_eq!(paint(&s, compound, false), solid("#ff0000")); + assert_eq!(paint(&s, loose, false), solid("#ff0000")); +}