diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 743aa8362..1e065c53b 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -559,6 +559,16 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { let (new_ids, made) = s.edit(|doc, sel| { let list = super::current_shapes(doc, sel).cloned().unwrap_or_default(); let mut copies: Vec = list.iter().filter(|x| ids.contains(&x.id)).cloned().collect(); + // A shape inside a group is copied into that group, with the map from the group's space onto + // the slide so that its copy is offset on the slide as a top-level copy is. + let top = copies.len(); + let mut to_slide = vec![None; top]; + for id in grouped_targets(&list, &ids) { + if let Some((m, a)) = crate::connect::locate(doc, sel, &list, id) { + copies.push(m.clone()); + to_slide.push(Some(a)); + } + } let sources: Vec = copies.iter().map(|c| c.id).collect(); let boxes: Vec<_> = copies.iter().map(|c| xfrm_of(doc, sel, c)).collect(); detach_on_slide(doc, sel, &mut copies); @@ -570,10 +580,12 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { }) .and_then(|(_, _, pairs)| { let (src, copy) = pairs.first()?; - let a = xfrm_of(doc, sel, list.iter().find(|x| x.id == *src)?); - let b = xfrm_of(doc, sel, list.iter().find(|x| x.id == *copy)?); - let d = (b.x - a.x, b.y - a.y); - (d.0.is_finite() && d.1.is_finite()).then_some(d) + // Measured on the slide, so the copy of a shape in a group repeats its drawn move. + let (a, to_a) = crate::connect::locate(doc, sel, &list, *src)?; + let (b, to_b) = crate::connect::locate(doc, sel, &list, *copy)?; + let (a, b) = (xfrm_of(doc, sel, a), xfrm_of(doc, sel, b)); + let d = to_b * deckcraft_geom::Point::new(b.x, b.y) - to_a * deckcraft_geom::Point::new(a.x, a.y); + (d.x.is_finite() && d.y.is_finite()).then_some((d.x, d.y)) }) .unwrap_or((off, off)); reid("edit.duplicate", doc, &mut copies)?; @@ -588,7 +600,8 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { rename_automatic(c, &mut taken, count); count += 1; }); - for (c, b) in copies.iter_mut().zip(boxes) { + for ((c, b), to) in copies.iter_mut().zip(boxes).zip(&to_slide) { + let (dx, dy) = to.map_or((dx, dy), |a| offset_in(a, dx, dy)); let mut x = b; x.x += dx; x.y += dy; @@ -599,7 +612,12 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { } let new_ids: Vec = copies.iter().map(|c| c.id).collect(); let shapes = crate::shapes_mut(doc, sel).ok_or_else(|| bad("edit.duplicate", "no slide"))?; - shapes.extend(copies); + let mut copies = copies.into_iter(); + shapes.extend(copies.by_ref().take(top)); + // A member's copy goes last among its group's children, as a top-level copy goes last on the slide. + for (c, src) in copies.zip(sources.iter().skip(top)) { + deckcraft_model::sibling_list_mut(shapes, *src).ok_or_else(|| bad("edit.duplicate", "no such shape"))?.push(c); + } sel.shapes = new_ids.clone(); let made = (sel.slide, sel.target, sources.into_iter().zip(new_ids.iter().copied()).collect()); Ok((new_ids, made)) @@ -608,6 +626,34 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { Ok(json!({"ids": new_ids})) } +/// The shapes in `ids` that are inside groups, in drawing order, without those in a group that is +/// in `ids` too: they are copied along with it. +fn grouped_targets(list: &[Shape], ids: &[ShapeId]) -> Vec { + fn rec(shapes: &[Shape], ids: &[ShapeId], nested: bool, out: &mut Vec, depth: usize) { + if depth > 64 { + return; + } + for sh in shapes { + if !ids.contains(&sh.id) { + rec(sh.children(), ids, true, out, depth + 1); + } else if nested { + out.push(sh.id); + } + } + } + let mut out = vec![]; + rec(list, ids, false, &mut out, 0); + out +} + +/// The offset in the space `to_slide` maps onto the slide that moves a shape by (`dx`, `dy`) on +/// the slide. A group with no width or height draws nothing, and the offset stays as it is. +fn offset_in(to_slide: deckcraft_geom::Affine, dx: f64, dy: f64) -> (f64, f64) { + let back = to_slide.inverse(); + let v = back * deckcraft_geom::Point::new(dx, dy) - back * deckcraft_geom::Point::ZERO; + if v.x.is_finite() && v.y.is_finite() { (v.x, v.y) } else { (dx, dy) } +} + fn delete(s: &mut Session, p: &Value) -> Result { if s.doc()?.selection.text.is_some() && ids_param(p, "ids").is_none() { return super::text::delete_dir(s, "forward"); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 5d2310328..640c586dc 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6543,6 +6543,65 @@ fn duplicating_a_moved_copy_repeats_the_move() { assert_eq!(s.doc().unwrap().doc.validate(), Vec::::new()); } +/// Duplicating a shape inside a group puts the copy in that group, last among its children, and +/// selects it. The copy is offset on the slide as a top-level copy is, however the group is moved, +/// stretched or turned, and duplicating a moved copy repeats the move (#966). A shape whose group is +/// duplicated as well goes along with the group's copy. Before, Duplicate looked only at the slide's +/// top level: it answered `{ids: []}` and changed nothing. +#[test] +fn duplicating_a_shape_in_a_group_copies_it_into_the_group() { + fn rect(s: &mut Session, x: f64) -> u64 { + s.execute("shape.insert", &json!({"preset": "rect", "rect": [x, 100, 100, 100]})).unwrap()["id"].as_u64().unwrap() + } + let sid = |id: u64| ShapeId(u32::try_from(id).unwrap()); + // How far shape `to` is drawn from shape `from` on the slide, the same for each of its sites. + let offset = |s: &Session, from: u64, to: u64| { + let st = s.doc().unwrap(); + let sites = |id| connect::sites_of(&st.doc, &st.selection, st.shapes(), sid(id)).unwrap(); + let (p, q) = (sites(from), sites(to)); + let d = q[0] - p[0]; + assert!(p.iter().zip(&q).all(|(p, q)| (*q - *p - d).hypot() < 1e-6), "{p:?} {q:?}"); + (d.x, d.y) + }; + let grouped = |edit: &str, p: Value| { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let (a, b) = (rect(&mut s, 100.0), rect(&mut s, 300.0)); + let g = s.execute("arrange.group", &json!({"ids": [a, b]})).unwrap()["id"].as_u64().unwrap(); + let mut p = p; + p["ids"] = json!([g]); + s.execute(edit, &p).unwrap(); + (s, a, b, g) + }; + for (edit, p) in [("shape.move", json!({"dx": 200})), ("shape.resize", json!({"w": 600})), ("shape.rotate", json!({"deg": 90}))] { + let (mut s, a, b, g) = grouped(edit, p); + let off = s.prefs.duplicate_offset; + let r = s.execute("edit.duplicate", &json!({"ids": [a]})).unwrap(); + assert_eq!(r["ids"].as_array().map(Vec::len), Some(1), "{edit}: {r}"); + let c = r["ids"][0].as_u64().unwrap(); + assert_eq!(s.doc().unwrap().selection.shapes, [sid(c)]); + let st = s.doc().unwrap(); + let order: Vec = deckcraft_model::find_shape(st.shapes(), sid(g)).unwrap().children().iter().map(|x| x.id).collect(); + assert_eq!(order, [sid(a), sid(b), sid(c)], "{edit}"); + let (dx, dy) = offset(&s, a, c); + assert!((dx - off).abs() < 1e-6 && (dy - off).abs() < 1e-6, "{edit}: the copy is drawn ({dx}, {dy}) from the original"); + // Moved, then duplicated again: the next copy is drawn as far from it again. + s.execute("shape.move", &json!({"ids": [c], "dx": 30})).unwrap(); + let (mx, my) = offset(&s, a, c); + let d = s.execute("edit.duplicate", &json!({"ids": [c]})).unwrap()["ids"][0].as_u64().unwrap(); + let (nx, ny) = offset(&s, c, d); + assert!((nx - mx).abs() < 1e-6 && (ny - my).abs() < 1e-6, "{edit}: moved ({mx}, {my}), then ({nx}, {ny})"); + assert_eq!(s.doc().unwrap().doc.validate(), Vec::::new()); + } + let (mut s, a, _, g) = grouped("shape.move", json!({"dx": 200})); + let r = s.execute("edit.duplicate", &json!({"ids": [a, g]})).unwrap(); + assert_eq!(r["ids"].as_array().map(Vec::len), Some(1), "{r}"); + let st = s.doc().unwrap(); + let count = |id: u64| deckcraft_model::find_shape(st.shapes(), sid(id)).unwrap().children().len(); + assert_eq!((count(g), count(r["ids"][0].as_u64().unwrap())), (2, 2)); + assert_eq!(st.shapes().len(), 2); +} + fn chart_of(s: &Session, id: u64) -> deckcraft_model::Chart { match &s.doc().unwrap().shape(deckcraft_model::ShapeId(id as u32)).unwrap().kind { deckcraft_model::ShapeKind::Chart(c) => (**c).clone(),