diff --git a/crates/engine/src/cmd/shape/freeform.rs b/crates/engine/src/cmd/shape/freeform.rs index f3be4f4bb..5fd299c9e 100644 --- a/crates/engine/src/cmd/shape/freeform.rs +++ b/crates/engine/src/cmd/shape/freeform.rs @@ -3,8 +3,9 @@ use super::{ColorRef, Fill, Geom, Line, NO_ID_SPACE, Result, Session, Value, bad pub(super) fn sites(s: &mut Session, p: &Value) -> Result { let id = id_param(p, "id", "shape.sites")?.ok_or_else(|| bad("shape.sites", "missing `id`"))?; let st = s.doc()?; - let sh = st.shape(id).ok_or_else(|| bad("shape.sites", "no such shape"))?; - Ok(json!(crate::connect::sites(&st.doc, &st.selection, sh).iter().map(|q| [q.x, q.y]).collect::>())) + // In slide space, as shape.connect uses them, for a shape inside a group too. + let v = crate::connect::sites_of(&st.doc, &st.selection, st.shapes(), id).ok_or_else(|| bad("shape.sites", "no such shape"))?; + Ok(json!(v.iter().map(|q| [q.x, q.y]).collect::>())) } pub(super) fn connect(s: &mut Session, p: &Value) -> Result { @@ -12,8 +13,13 @@ pub(super) fn connect(s: &mut Session, p: &Value) -> Result { return Err(bad("shape.connect", "missing `from` / `to`")); }; let st = s.doc()?; - let site_list = |id| st.shape(id).map(|sh| crate::connect::sites(&st.doc, &st.selection, sh)).unwrap_or_default(); - let (a, b) = (site_list(from), site_list(to)); + // Anywhere in the slide's tree: a shape inside a group takes a connector too (PowerPoint + // glues to grouped shapes), its sites where the group draws them. + let site_list = |id| { + crate::connect::sites_of(&st.doc, &st.selection, st.shapes(), id) + .ok_or_else(|| bad("shape.connect", format!("no shape {} on the current slide", id.0))) + }; + let (a, b) = (site_list(from)?, site_list(to)?); if a.is_empty() || b.is_empty() { return Err(bad("shape.connect", "both shapes need connection sites (lines and groups have none)")); } @@ -35,6 +41,11 @@ pub(super) fn connect(s: &mut Session, p: &Value) -> Result { }; let (p0, p1) = (a[ia], b[ib]); let id = match id_param(p, "id", "shape.connect")? { + // The ends are in slide space and rerouting follows the slide's own connectors, so a line + // inside a group would be drawn in the group's space and never follow its shapes. + Some(id) if !st.shapes().iter().any(|sh| sh.id == id) && st.shape(id).is_some_and(|sh| sh.is_line()) => { + return Err(bad("shape.connect", "`id` names a line inside a group; ungroup it first, or leave `id` out for a new connector")); + } Some(id) => id, None => { let preset = str_param(p, "preset").unwrap_or("straightConnector1"); diff --git a/crates/engine/src/connect.rs b/crates/engine/src/connect.rs index 8cdbecda8..5991ba619 100644 --- a/crates/engine/src/connect.rs +++ b/crates/engine/src/connect.rs @@ -1,14 +1,21 @@ //! Connectors glued to shapes: connection sites, snapping line ends to them, and rerouting attached //! connectors whenever the shapes they connect move, resize or rotate. -use deckcraft_geom::{Point, Xfrm}; +use deckcraft_geom::{Affine, Point, Rect, Xfrm}; use deckcraft_model::{Presentation, Shape, ShapeId, ShapeKind}; use crate::Selection; use crate::cmd::xfrm_of; -/// Connection sites of `s` in slide coordinates (index = site index stored on connectors). +/// Connection sites of a top-level shape `s` in slide coordinates (index = site index stored on +/// connectors). For a shape inside a group use [`sites_of`] or [`sites_in`]. pub fn sites(doc: &Presentation, sel: &Selection, s: &Shape) -> Vec { + sites_in(doc, sel, s, Affine::IDENTITY) +} + +/// Connection sites of `s` in slide coordinates, `to_abs` mapping the space its box lives in (the +/// slide, or the child space of the groups around it) onto the slide. +pub fn sites_in(doc: &Presentation, sel: &Selection, s: &Shape, to_abs: Affine) -> Vec { if s.is_line() || matches!(s.kind, ShapeKind::Group { .. } | ShapeKind::Ink { .. }) { return vec![]; } @@ -17,10 +24,42 @@ pub fn sites(doc: &Presentation, sel: &Selection, s: &Shape) -> Vec { if local.is_empty() { local = vec![Point::new(x.w / 2.0, 0.0), Point::new(0.0, x.h / 2.0), Point::new(x.w / 2.0, x.h), Point::new(x.w, x.h / 2.0)]; } - let a = x.affine(); + let a = to_abs * x.affine(); local.into_iter().map(|p| a * p).collect() } +/// The shape `id` anywhere in `shapes` (a group's member included) with the affine that maps the +/// space its box lives in onto the slide: a connector glues to a grouped shape, as in PowerPoint, +/// and its sites are where the group draws them. +pub fn locate<'a>(doc: &Presentation, sel: &Selection, shapes: &'a [Shape], id: ShapeId) -> Option<(&'a Shape, Affine)> { + fn rec<'a>(doc: &Presentation, sel: &Selection, shapes: &'a [Shape], id: ShapeId, to_abs: Affine, depth: usize) -> Option<(&'a Shape, Affine)> { + if depth > 64 { + return None; + } + for s in shapes { + if s.id == id { + return Some((s, to_abs)); + } + if let ShapeKind::Group { children, child } = &s.kind { + let x = xfrm_of(doc, sel, s); + let ch = Rect::new(child.x, child.y, child.x + child.w, child.y + child.h); + let nested = to_abs * deckcraft_geom::group_child_affine(&x, ch); + if let Some(f) = rec(doc, sel, children, id, nested, depth + 1) { + return Some(f); + } + } + } + None + } + rec(doc, sel, shapes, id, Affine::IDENTITY, 0) +} + +/// Connection sites of shape `id` anywhere in `shapes`, in slide coordinates; `None` when there is +/// no such shape. +pub fn sites_of(doc: &Presentation, sel: &Selection, shapes: &[Shape], id: ShapeId) -> Option> { + locate(doc, sel, shapes, id).map(|(s, a)| sites_in(doc, sel, s, a)) +} + /// The nearest connection site within `radius` of `p` on any visible shape except `exclude`. pub fn nearest_site( doc: &Presentation, @@ -68,8 +107,8 @@ pub fn reroute(doc: &mut Presentation, sel: &Selection) { let snapshot: Vec = list.clone(); let view = doc.clone(); let site = |id: ShapeId, idx: u32| -> Option { - let s = snapshot.iter().find(|s| s.id == id)?; - let v = sites(&view, sel, s); + // Anywhere in the tree: a connector glued to a shape that is then grouped keeps following it. + let v = sites_of(&view, sel, &snapshot, id)?; v.get(idx as usize).or(v.first()).copied() }; let mut updates: Vec<(usize, Xfrm, bool, bool)> = vec![]; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index aa63c5c0e..ca1f93d26 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6872,6 +6872,71 @@ fn save_to_image_path_is_an_error_not_zip_bytes() { let _ = std::fs::remove_dir_all(&dir); } +/// `shape.connect` glues to a shape inside a group, as PowerPoint does: the connector's +/// end is set to the grouped shape and follows it when the group moves. The glue was dropped at +/// once (the end stayed `None`) because rerouting only looked at the slide's top-level shapes, +/// and the sites of a grouped shape were computed in the group's child space. +#[test] +fn shape_connect_glues_to_a_shape_inside_a_group() { + use deckcraft_model::{ShapeId, ShapeKind}; + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let a = s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 10, 100, 50]})).unwrap()["id"].as_u64().unwrap(); + let b = s.execute("shape.insert", &json!({"preset": "rect", "rect": [10, 200, 100, 50]})).unwrap()["id"].as_u64().unwrap(); + let g = s.execute("arrange.group", &json!({"ids": [a, b]})).unwrap()["id"].as_u64().unwrap(); + let c = s.execute("shape.insert", &json!({"preset": "rect", "rect": [400, 10, 100, 50]})).unwrap()["id"].as_u64().unwrap(); + let glue = |s: &Session, id: u64| { + let st = s.doc().unwrap(); + match st.shape(ShapeId(id as u32)).unwrap().kind { + ShapeKind::Connector { start, end } => (start.map(|(i, _)| i.0 as u64), end.map(|(i, _)| i.0 as u64)), + _ => (None, None), + } + }; + let near = |p: deckcraft_geom::Point, x: f64, y: f64| (p.x - x).abs() < 1e-6 && (p.y - y).abs() < 1e-6; + // A new connector into the group. + let r = s.execute("shape.connect", &json!({"from": c, "to": a})).unwrap(); + let k = r["id"].as_u64().unwrap(); + assert_eq!(glue(&s, k), (Some(c), Some(a)), "{r}"); + let (p0, p1) = line_ends(&s, k); + assert!(near(p0, 400.0, 35.0), "from C's left site: {p0:?}"); + assert!(near(p1, 110.0, 35.0), "to A's right site in slide space: {p1:?}"); + s.execute("shape.move", &json!({"ids": [g], "dy": 100})).unwrap(); + assert_eq!(glue(&s, k), (Some(c), Some(a)), "the glue survives the move"); + let (_, p1) = line_ends(&s, k); + assert!(near(p1, 110.0, 135.0), "the connector follows the grouped shape: {p1:?}"); + // Re-gluing an existing line with `id`. + let l = s.execute("shape.insert", &json!({"preset": "straightConnector1", "rect": [600, 400, 50, 50]})).unwrap()["id"].as_u64().unwrap(); + let r = s.execute("shape.connect", &json!({"id": l, "from": c, "to": b})).unwrap(); + assert_eq!(r["id"], l, "{r}"); + assert_eq!(glue(&s, l), (Some(c), Some(b)), "{r}"); + let (_, p1) = line_ends(&s, l); + assert!(near(p1, 110.0, 325.0), "to B's right site (C is mostly to its right) in slide space, B moved with the group: {p1:?}"); + s.execute("shape.move", &json!({"ids": [g], "dx": 50})).unwrap(); + assert_eq!(glue(&s, l), (Some(c), Some(b))); + let (_, p1) = line_ends(&s, l); + assert!(near(p1, 160.0, 325.0), "{p1:?}"); + // shape.sites answers in slide space, as shape.connect uses them: A's right site, A moved + // with the group. + let v = s.execute("shape.sites", &json!({"id": a})).unwrap(); + assert!( + v.as_array().unwrap().iter().any(|q| near(deckcraft_geom::Point::new(q[0].as_f64().unwrap(), q[1].as_f64().unwrap()), 160.0, 135.0)), + "{v}" + ); + // A line inside a group is refused as `id`: its ends would be written in slide space into + // the group's space, and rerouting does not follow grouped connectors. + let m = s.execute("shape.insert", &json!({"preset": "line", "rect": [600, 300, 50, 0]})).unwrap()["id"].as_u64().unwrap(); + let n = s.execute("shape.insert", &json!({"preset": "rect", "rect": [600, 450, 50, 50]})).unwrap()["id"].as_u64().unwrap(); + s.execute("arrange.group", &json!({"ids": [m, n]})).unwrap(); + let before = s.doc().unwrap().doc.clone(); + let e = s.execute("shape.connect", &json!({"id": m, "from": c, "to": b})).unwrap_err().to_string(); + assert!(e.contains("inside a group"), "{e}"); + assert_eq!(s.doc().unwrap().doc, before, "nothing changed"); + // A grouped shape that is not a line is refused as not a line, not as a grouped line. + let e = s.execute("shape.connect", &json!({"id": n, "from": c, "to": b})).unwrap_err().to_string(); + assert!(e.contains("not a line"), "{e}"); + assert_eq!(s.doc().unwrap().doc, before, "nothing changed"); +} + #[test] fn flip_is_refused_on_a_smartart_graphic_and_leaves_it_a_graphic() { let mut s = session(); diff --git a/crates/engine/src/tools/drag.rs b/crates/engine/src/tools/drag.rs index dcee9cf7d..a1abcb93c 100644 --- a/crates/engine/src/tools/drag.rs +++ b/crates/engine/src/tools/drag.rs @@ -236,7 +236,12 @@ impl Session { } } // A connector dragged away from shapes that stay put comes unglued at those ends. - let moved: Vec = origs2.iter().map(|o| o.0).collect(); + let mut moved: Vec = vec![]; + for (id, _) in &origs2 { + if let Some(sh) = deckcraft_model::find_shape(list, *id) { + deckcraft_model::walk(std::slice::from_ref(sh), &mut |s, _| moved.push(s.id)); + } + } for sh in list.iter_mut().filter(|s| moved.contains(&s.id)) { if let ShapeKind::Connector { start, end } = &mut sh.kind { for c in [start, end] {