From 272ab5f96f83c9dc4794518f097a6b8ae2524f59 Mon Sep 17 00:00:00 2001 From: Nikita Rakitin Date: Tue, 13 Oct 2026 20:47:27 +0300 Subject: [PATCH] Edit: copies remap media, connector ends, animation targets and triggers (#406) Paste added clashing media under a fresh id but left the pasted shapes on the old id, reid did not touch connector ends despite its comment, pasted slides kept animations on the original shape ids, and slide.duplicate remapped animation targets but not triggers. - reid returns the old-to-new shape map; connector ends on a copied shape follow the copy, ends on a shape outside the copied set are detached. - remap_animations points targets and triggers at the copies (an animation whose target wasn't copied is dropped, an uncopied trigger cleared); used by slide paste and slide.duplicate. - bring_media returns the media ids it had to change and paste rewrites the shapes (and a pasted slide's picture background) to use them; slide copy now also carries a picture background's media. Co-authored-by: Cursor --- crates/engine/src/cmd/edit.rs | 120 +++++++++++++++++++++++++----- crates/engine/src/cmd/slide.rs | 21 +----- crates/engine/src/tests.rs | 131 +++++++++++++++++++++++++++++++++ 3 files changed, 236 insertions(+), 36 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 30437b15..3b886242 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -1,5 +1,6 @@ //! Edit menu: undo/redo, clipboard, duplicate, delete, select all. +use std::collections::HashMap; use std::sync::Arc; use deckcraft_model::{MediaId, Shape, ShapeId, Slide, SlideId}; @@ -87,6 +88,9 @@ fn copy_inner(s: &mut Session, p: &Value) -> Result { let mut mids = vec![]; for sl in &slides { media_of_shapes(&sl.shapes, &mut mids); + if let Some(deckcraft_model::Background::Fill { fill: deckcraft_model::Fill::Picture(pf) }) = &sl.background { + mids.push(pf.media); + } } let media = st.doc.media.iter().filter(|m| mids.contains(&m.id)).cloned().collect(); let plain = slides.iter().map(|x| x.title()).collect::>().join("\n"); @@ -125,34 +129,106 @@ fn cut(s: &mut Session, p: &Value) -> Result { Ok(json!({"text": t})) } -/// Give pasted shapes fresh ids (groups included) and map connector ends. -pub(crate) fn reid(doc: &mut deckcraft_model::Presentation, shapes: &mut [Shape]) { - fn rec(doc: &mut deckcraft_model::Presentation, shapes: &mut [Shape], depth: usize) { +/// Give pasted shapes fresh ids (groups included) and return the old-to-new map. Connector ends +/// attached to a shape that was copied too follow its copy; ends attached to a shape outside the +/// copied set are detached (the connector keeps its geometry), so a copy never stays glued to +/// the original or to an unrelated shape that happens to share an id in another deck. +pub(crate) fn reid(doc: &mut deckcraft_model::Presentation, shapes: &mut [Shape]) -> HashMap { + fn rec(doc: &mut deckcraft_model::Presentation, shapes: &mut [Shape], map: &mut HashMap, depth: usize) { if depth > 64 { return; } for s in shapes { - s.id = ShapeId(doc.alloc_id()); + let id = ShapeId(doc.alloc_id()); + map.insert(s.id, id); + s.id = id; if let Some(ch) = s.children_mut() { - rec(doc, ch, depth + 1); + rec(doc, ch, map, depth + 1); } } } - rec(doc, shapes, 0); + let mut map = HashMap::new(); + rec(doc, shapes, &mut map, 0); + walk_mut(shapes, &mut |s| { + if let deckcraft_model::ShapeKind::Connector { start, end } = &mut s.kind { + for e in [start, end] { + *e = e.and_then(|(id, site)| map.get(&id).map(|n| (*n, site))); + } + } + }); + map } -fn bring_media(doc: &mut deckcraft_model::Presentation, media: &[deckcraft_model::MediaItem]) { - for m in media { - if doc.media(m.id).is_none_or(|x| x.data != m.data) { - if doc.media(m.id).is_none() { - doc.media.push(m.clone()); - } else { - // Id clash with different bytes: unusual (other deck); keep ours, add theirs as new. - let id = doc.add_media(&m.name, &m.content_type, m.data.as_ref().clone()); - let _ = id; +/// Point copied animations at the copies: targets and triggers in `map` follow it; an animation +/// whose target wasn't copied is dropped, and a trigger that wasn't copied is cleared. +pub(crate) fn remap_animations(anims: &mut Vec, map: &HashMap) { + anims.retain_mut(|a| { + let Some(n) = map.get(&a.shape) else { return false }; + a.shape = *n; + a.trigger = a.trigger.and_then(|t| map.get(&t).copied()); + true + }); +} + +/// Visit every shape mutably, depth-first (groups included, depth-bounded like `walk`). +fn walk_mut(shapes: &mut [Shape], f: &mut dyn FnMut(&mut Shape)) { + fn rec(shapes: &mut [Shape], f: &mut dyn FnMut(&mut Shape), depth: usize) { + if depth > 64 { + return; + } + for s in shapes { + f(s); + if let Some(ch) = s.children_mut() { + rec(ch, f, depth + 1); } } } + rec(shapes, f, 0); +} + +/// Rewrite the media ids shapes use (`map`: clipboard id to destination id). +fn remap_media(shapes: &mut [Shape], map: &HashMap) { + if map.is_empty() { + return; + } + let m = |id: &mut MediaId| { + if let Some(n) = map.get(id) { + *id = *n; + } + }; + walk_mut(shapes, &mut |s| { + match &mut s.kind { + deckcraft_model::ShapeKind::Picture { fill } => m(&mut fill.media), + deckcraft_model::ShapeKind::Media(c) => { + m(&mut c.media); + if let Some(p) = c.poster.as_mut() { + m(p); + } + } + deckcraft_model::ShapeKind::Opaque { preview: Some(p), .. } => m(p), + _ => {} + } + if let Some(deckcraft_model::Fill::Picture(pf)) = &mut s.fill { + m(&mut pf.media); + } + }); +} + +/// Add the clipboard's media to `doc` and return the ids that changed: an id already used in +/// `doc` by other bytes (another deck) gets a fresh id, and the pasted shapes must use it. +fn bring_media(doc: &mut deckcraft_model::Presentation, media: &[deckcraft_model::MediaItem]) -> HashMap { + let mut map = HashMap::new(); + for m in media { + match doc.media(m.id) { + None => doc.media.push(m.clone()), + Some(x) if x.data == m.data => {} + Some(_) => { + let id = doc.add_media(&m.name, &m.content_type, m.data.as_ref().clone()); + map.insert(m.id, id); + } + } + } + map } fn paste(s: &mut Session, p: &Value) -> Result { @@ -174,12 +250,19 @@ fn paste(s: &mut Session, p: &Value) -> Result { if !clip.slides.is_empty() { let at = s.doc()?.selection.slide + 1; return s.edit(|doc, sel| { - bring_media(doc, &clip.media); + let media = bring_media(doc, &clip.media); let mut new_ids = vec![]; for (k, sl) in clip.slides.iter().enumerate() { let mut sl = sl.clone(); sl.id = SlideId(doc.alloc_id()); - reid(doc, &mut sl.shapes); + let map = reid(doc, &mut sl.shapes); + remap_animations(&mut sl.animations, &map); + remap_media(&mut sl.shapes, &media); + if let Some(deckcraft_model::Background::Fill { fill: deckcraft_model::Fill::Picture(pf) }) = &mut sl.background + && let Some(n) = media.get(&pf.media) + { + pf.media = *n; + } if doc.layout(sl.layout).is_none() { sl.layout = doc.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id).unwrap_or_default(); } @@ -200,9 +283,10 @@ fn paste(s: &mut Session, p: &Value) -> Result { let same_slide = s.doc().ok().is_some_and(|d| clip.shapes.iter().any(|c| d.shape(c.id).is_some())); let off = if same_slide { off } else { 0.0 }; return s.edit(|doc, sel| { - bring_media(doc, &clip.media); + let media = bring_media(doc, &clip.media); let mut shapes = clip.shapes.clone(); reid(doc, &mut shapes); + remap_media(&mut shapes, &media); for sh in &mut shapes { if let Some(x) = sh.xfrm.as_mut() { x.x += off; diff --git a/crates/engine/src/cmd/slide.rs b/crates/engine/src/cmd/slide.rs index 3d8ad344..bc116a4f 100644 --- a/crates/engine/src/cmd/slide.rs +++ b/crates/engine/src/cmd/slide.rs @@ -166,24 +166,9 @@ fn duplicate(s: &mut Session, p: &Value) -> Result { let Some(src) = doc.slides.get(*i).map(|x| x.as_ref().clone()) else { continue }; let mut copy = src; copy.id = SlideId(doc.alloc_id()); - super::edit::reid(doc, &mut copy.shapes); - // Animations follow the new shape ids by position. - let old_ids: Vec<_> = doc - .slides - .get(*i) - .map(|x| { - let mut v = vec![]; - deckcraft_model::walk(&x.shapes, &mut |sh, _| v.push(sh.id)); - v - }) - .unwrap_or_default(); - let mut new_shape_ids = vec![]; - deckcraft_model::walk(©.shapes, &mut |sh, _| new_shape_ids.push(sh.id)); - for a in &mut copy.animations { - if let Some(pos) = old_ids.iter().position(|x| *x == a.shape) { - a.shape = new_shape_ids.get(pos).copied().unwrap_or(a.shape); - } - } + // Animations follow the new shape ids, triggers included. + let map = super::edit::reid(doc, &mut copy.shapes); + super::edit::remap_animations(&mut copy.animations, &map); let section = doc.section_of(src_id(doc, *i)); let at = (last + 1 + k).min(doc.slides.len()); new_ids.push(copy.id); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e83f72ef..b9ebd1c6 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -835,3 +835,134 @@ fn slide_ranges_parse_like_powerpoints_print_box() { let opts = cmd::file::pdf_options(&json!({"slides": [0, 7, 1]}), 2).unwrap(); assert_eq!(opts.slides, Some(vec![0, 1]), "indices past the end are dropped"); } + +mod copy_remap { + use std::sync::Arc; + + use deckcraft_model::style::PictureFill; + use deckcraft_model::{Animation, Shape, ShapeId, ShapeKind, Xfrm}; + use serde_json::json; + + use crate::*; + + /// Slide 0 of `s` gets a picture of `bytes`, a box, and a connector from the picture to the box. + /// Returns (picture, box, connector). + fn populate(s: &mut Session, bytes: &[u8]) -> (ShapeId, ShapeId, ShapeId) { + let st = s.doc_mut().unwrap(); + let doc = Arc::make_mut(&mut st.doc); + let media = doc.add_media("p.png", "image/png", bytes.to_vec()); + let (p, b, c) = (ShapeId(doc.alloc_id()), ShapeId(doc.alloc_id()), ShapeId(doc.alloc_id())); + let x = Some(Xfrm::new(10.0, 10.0, 50.0, 50.0)); + let slide = Arc::make_mut(&mut doc.slides[0]); + slide.shapes.push(Shape { + id: p, + kind: ShapeKind::Picture { fill: PictureFill { media, ..Default::default() } }, + xfrm: x, + ..Default::default() + }); + slide.shapes.push(Shape { id: b, xfrm: x, ..Default::default() }); + slide.shapes.push(Shape { id: c, kind: ShapeKind::Connector { start: Some((p, 1)), end: Some((b, 3)) }, xfrm: x, ..Default::default() }); + (p, b, c) + } + + fn select(s: &mut Session, ids: &[ShapeId]) { + let sel = &mut s.doc_mut().unwrap().selection; + sel.slides.clear(); + sel.shapes = ids.to_vec(); + } + + fn shape(s: &Session, slide: usize, id: ShapeId) -> Shape { + let d = s.doc().unwrap(); + d.doc.slides[slide].shapes.iter().find(|x| x.id == id).cloned().unwrap() + } + + fn picture_bytes(s: &Session, sh: &Shape) -> Vec { + let ShapeKind::Picture { fill } = &sh.kind else { panic!("not a picture") }; + s.doc().unwrap().doc.media(fill.media).unwrap().data.as_ref().clone() + } + + #[test] + fn paste_into_a_deck_with_a_colliding_media_id_shows_the_copied_picture() { + let mut a = Session::with_new(); + let mut b = Session::with_new(); + let (pa, ..) = populate(&mut a, b"deck a picture"); + let (pb, ..) = populate(&mut b, b"deck b picture"); + assert_eq!(picture_bytes(&a, &shape(&a, 0, pa)), b"deck a picture"); + select(&mut a, &[pa]); + a.execute("edit.copy", &json!({})).unwrap(); + b.clipboard = a.clipboard.clone(); + b.doc_mut().unwrap().selection.shapes.clear(); + let r = b.execute("edit.paste", &json!({})).unwrap(); + let new = ShapeId(r["ids"][0].as_u64().unwrap() as u32); + assert_eq!(picture_bytes(&b, &shape(&b, 0, new)), b"deck a picture"); + // Deck b's own picture is untouched. + assert_eq!(picture_bytes(&b, &shape(&b, 0, pb)), b"deck b picture"); + } + + #[test] + fn copied_connectors_follow_copied_ends_and_detach_from_the_rest() { + let mut s = Session::with_new(); + let (p, b, c) = populate(&mut s, b"x"); + // Both ends copied: the copy connects the copies. + let r = s.execute("edit.duplicate", &json!({"ids": [p.0, b.0, c.0]})).unwrap(); + let ids: Vec = r["ids"].as_array().unwrap().iter().map(|v| ShapeId(v.as_u64().unwrap() as u32)).collect(); + let ShapeKind::Connector { start, end } = shape(&s, 0, ids[2]).kind else { panic!() }; + assert_eq!((start, end), (Some((ids[0], 1)), Some((ids[1], 3)))); + // Only the connector copied: it doesn't stay glued to the originals. + select(&mut s, &[c]); + s.execute("edit.copy", &json!({})).unwrap(); + s.doc_mut().unwrap().selection.shapes.clear(); + let r = s.execute("edit.paste", &json!({})).unwrap(); + let only = ShapeId(r["ids"][0].as_u64().unwrap() as u32); + let ShapeKind::Connector { start, end } = shape(&s, 0, only).kind else { panic!() }; + assert_eq!((start, end), (None, None)); + // The original keeps its ends. + let ShapeKind::Connector { start, .. } = shape(&s, 0, c).kind else { panic!() }; + assert_eq!(start, Some((p, 1))); + } + + fn triggered(s: &mut Session, target: ShapeId, trigger: ShapeId) { + let st = s.doc_mut().unwrap(); + let doc = Arc::make_mut(&mut st.doc); + Arc::make_mut(&mut doc.slides[0]).animations.push(Animation { + shape: target, + trigger: Some(trigger), + effect: "fade".into(), + ..Default::default() + }); + } + + fn resolves(s: &Session, slide: usize) -> (ShapeId, ShapeId) { + let d = s.doc().unwrap(); + let sl = &d.doc.slides[slide]; + let a = &sl.animations[0]; + let t = a.trigger.unwrap(); + assert!(sl.shapes.iter().any(|x| x.id == a.shape) && sl.shapes.iter().any(|x| x.id == t), "{a:?}"); + (a.shape, t) + } + + #[test] + fn duplicated_and_pasted_slides_retarget_animations_and_triggers() { + let mut s = Session::with_new(); + let (p, b, _) = populate(&mut s, b"x"); + triggered(&mut s, p, b); + s.execute("slide.duplicate", &json!({"index": 0})).unwrap(); + let (t, tr) = resolves(&s, 1); + assert!(t != p && tr != b); + // Pasted slide. + s.execute("edit.copy", &json!({"scope": "slides"})).unwrap(); + s.execute("edit.paste", &json!({"scope": "slides"})).unwrap(); + let n = s.doc().unwrap().doc.slides.len(); + let (t2, tr2) = resolves(&s, n - 1); + assert!(t2 != t && tr2 != tr); + // Save and reload keeps them resolving. + let bytes = crate::cmd::file::save_bytes(&s.doc().unwrap().doc, "deckcraft").unwrap(); + let back = deckcraft_format::load(&bytes).unwrap(); + for sl in &back.slides { + for a in &sl.animations { + assert!(sl.shapes.iter().any(|x| x.id == a.shape)); + assert!(a.trigger.is_none_or(|t| sl.shapes.iter().any(|x| x.id == t))); + } + } + } +}