Merge remote-tracking branch 'origin/main' into web-host-bridge

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<Value> {
     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::<Vec<_>>()))
+    // 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::<Vec<_>>()))
 }
 
 pub(super) fn connect(s: &mut Session, p: &Value) -> Result<Value> {
@@ -12,8 +13,13 @@ pub(super) fn connect(s: &mut Session, p: &Value) -> Result<Value> {
         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<Value> {
     };
     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");

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<Point> {
+    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<Point> {
     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<Point> {
     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<Vec<Point>> {
+    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<Shape> = list.clone();
     let view = doc.clone();
     let site = |id: ShapeId, idx: u32| -> Option<Point> {
-        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![];

crates/engine/src/tests.rs

@@ -6510,6 +6510,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();

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<ShapeId> = origs2.iter().map(|o| o.0).collect();
+            let mut moved: Vec<ShapeId> = 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] {