From e8910cb253ed190dae9fc3d4353dc7abfd0b5193 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 24 Oct 2026 02:34:00 -0700 Subject: [PATCH] Paste: a table, bullet or slide from another deck keeps its own pictures and sounds Copy listed the media of pictures, media clips, 3-D models and shape fills only, and Paste rewrote the ids of pictures, media clips, opaque previews and shape fills only. Picture fills in table cells and table backgrounds, picture-filled text runs and picture bullets were left off the clipboard. In another deck they pointed at a missing picture, or at that deck's own picture when it used the same media id. A copied slide lost its transition and animation sounds in the same way. Paste Special > Picture gave clashing media fresh ids but drew the shapes with the old ids, so a clashing id drew the open deck's picture. Review > Compare > Accept Revision of a compared deck's slide copied the same partial set, and missed the slide background as well. Copy, Paste, Paste Special > Picture and Accept Revision now find and rewrite media ids with the media_refs walkers that slide.reuse uses. --- crates/engine/src/cmd/edit.rs | 71 ++++--------------------- crates/engine/src/compare.rs | 80 +++++++++++++++++++--------- crates/engine/src/tests.rs | 98 ++++++++++++++++++++++++++++++++++- 3 files changed, 162 insertions(+), 87 deletions(-) diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index fa13825f..0a4702a8 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -69,30 +69,6 @@ fn slides_scope(s: &Session, p: &Value) -> bool { || s.active().is_some_and(|d| d.selection.shapes.is_empty() && d.selection.text.is_none() && !d.selection.slides.is_empty()) } -pub(crate) fn media_of_shapes(shapes: &[Shape], out: &mut Vec) { - deckcraft_model::walk(shapes, &mut |s, _| { - match &s.kind { - deckcraft_model::ShapeKind::Picture { fill } => out.push(fill.media), - deckcraft_model::ShapeKind::Media(m) => { - out.push(m.media); - if let Some(p) = m.poster { - out.push(p); - } - } - deckcraft_model::ShapeKind::Model3d { media, poster } => { - out.push(*media); - if let Some(p) = poster { - out.push(*p); - } - } - _ => {} - } - if let Some(deckcraft_model::Fill::Picture(pf)) = &s.fill { - out.push(pf.media); - } - }); -} - fn copy_inner(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; // Text selection. @@ -113,10 +89,7 @@ fn copy_inner(s: &mut Session, p: &Value) -> Result { let slides: Vec = ids.iter().filter_map(|id| st.doc.slide(*id).cloned()).collect(); 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); - } + mids.extend(crate::media_refs::slide_media(sl)); } 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"); @@ -136,8 +109,7 @@ fn copy_inner(s: &mut Session, p: &Value) -> Result { }) }) .collect(); - let mut mids = vec![]; - media_of_shapes(&shapes, &mut mids); + let mids = crate::media_refs::shapes_media(&shapes); let media = st.doc.media.iter().filter(|m| mids.contains(&m.id)).cloned().collect(); let plain = shapes.iter().filter_map(|x| x.text.as_ref().map(|t| t.text())).collect::>().join("\n"); let source_theme = st.current_slide().and_then(|sl| st.doc.master_for(sl)).map(|(m, _)| (m.scheme(), m.theme.fonts.clone())); @@ -250,32 +222,13 @@ fn walk_mut(shapes: &mut [Shape], f: &mut dyn FnMut(&mut Shape)) { 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| { +/// Rewrite a media id the clipboard used (`map`: clipboard id to destination id). +fn remap_media(map: &HashMap) -> impl FnMut(&mut MediaId) + '_ { + move |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 @@ -328,12 +281,7 @@ fn paste(s: &mut Session, p: &Value) -> Result { sl.id = SlideId(doc.alloc_id().ok_or_else(|| bad("edit.paste", NO_ID_SPACE))?); let map = reid("edit.paste", 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; - } + crate::media_refs::visit_mut::slide(&mut sl, &mut remap_media(&media)); 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(); } @@ -357,7 +305,7 @@ fn paste(s: &mut Session, p: &Value) -> Result { let media = bring_media("edit.paste", doc, &clip.media)?; let mut shapes = clip.shapes.clone(); reid("edit.paste", doc, &mut shapes)?; - remap_media(&mut shapes, &media); + crate::media_refs::visit_mut::shapes(&mut shapes, &mut remap_media(&media)); for sh in &mut shapes { if let Some(x) = sh.xfrm.as_mut() { x.x += off; @@ -504,7 +452,8 @@ fn paste_picture(s: &mut Session) -> Result { } let st = s.doc()?; let mut doc = (*st.doc).clone(); - bring_media("edit.pasteSpecial", &mut doc, &clip.media)?; + let media = bring_media("edit.pasteSpecial", &mut doc, &clip.media)?; + crate::media_refs::visit_mut::shapes(&mut clip.shapes, &mut remap_media(&media)); let mut slide = st.current_slide().cloned().ok_or_else(|| bad("edit.pasteSpecial", "no slide"))?; let xs: Vec = clip.shapes.iter().filter_map(|x| x.xfrm).collect(); let b = super::arrange::union_bounds(&xs).ok_or_else(|| bad("edit.pasteSpecial", "nothing to draw"))?; diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index 93469f06..bcdcf7eb 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -7,7 +7,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::Arc; -use deckcraft_model::{Fill, MediaId, Presentation, Shape, ShapeId, ShapeKind, Slide, SlideId}; +use deckcraft_model::{MediaId, Presentation, Shape, ShapeId, ShapeKind, Slide, SlideId}; use serde::Serialize; /// The revised presentation a document is being compared with. @@ -233,10 +233,8 @@ fn max_shape_id(shapes: &[Shape], m: &mut u32) { pub fn adopt(doc: &mut Presentation, rev: &Presentation, slide: &Slide, replacing: Option) -> crate::Result { let mut sl = slide.clone(); // Media. - let mut mids = vec![]; - crate::cmd::edit::media_of_shapes(&sl.shapes, &mut mids); let mut media_map: HashMap = HashMap::new(); - for id in mids { + for id in crate::media_refs::slide_media(&sl) { let Some(m) = rev.media(id) else { continue }; match doc.media(id) { Some(x) if x.data == m.data => {} @@ -262,7 +260,6 @@ pub fn adopt(doc: &mut Presentation, rev: &Presentation, slide: &Slide, replacin shapes: &mut [Shape], taken: &HashSet, ids: &mut HashMap, - media: &HashMap, depth: usize, ) -> crate::Result<()> { if depth > 64 { @@ -274,31 +271,18 @@ pub fn adopt(doc: &mut Presentation, rev: &Presentation, slide: &Slide, replacin ids.insert(s.id, new); s.id = new; } - let remap = |m: &mut MediaId| { - if let Some(n) = media.get(m) { - *m = *n; - } - }; - match &mut s.kind { - ShapeKind::Picture { fill } => remap(&mut fill.media), - ShapeKind::Media(m) => { - remap(&mut m.media); - if let Some(p) = m.poster.as_mut() { - remap(p); - } - } - _ => {} - } - if let Some(Fill::Picture(pf)) = &mut s.fill { - remap(&mut pf.media); - } if let Some(ch) = s.children_mut() { - rec(doc, ch, taken, ids, media, depth + 1)?; + rec(doc, ch, taken, ids, depth + 1)?; } } Ok(()) } - rec(doc, &mut sl.shapes, &taken, &mut id_map, &media_map, 0)?; + rec(doc, &mut sl.shapes, &taken, &mut id_map, 0)?; + crate::media_refs::visit_mut::slide(&mut sl, &mut |m: &mut MediaId| { + if let Some(n) = media_map.get(m) { + *m = *n; + } + }); for a in &mut sl.animations { if let Some(n) = id_map.get(&a.shape) { a.shape = *n; @@ -412,6 +396,52 @@ mod tests { assert_eq!(s.doc().unwrap().doc.slides.len(), n, "comparing alone changes nothing"); } + /// A deck with one slide: a table whose cell has a picture fill, a picture background and a + /// transition sound. `tag` starts the bytes of each. + fn media_deck(tag: &str) -> deckcraft_model::Presentation { + use std::sync::Arc; + + use deckcraft_model::style::PictureFill; + use deckcraft_model::{Background, Fill, Shape, ShapeId, ShapeKind, Table, Transition, defaults}; + + let mut p = defaults::new_presentation(None); + let cell = p.add_media("cell.png", "image/png", format!("{tag} cell").into_bytes()).expect("media"); + let back = p.add_media("back.png", "image/png", format!("{tag} background").into_bytes()).expect("media"); + let sound = p.add_media("sound.wav", "audio/wav", format!("{tag} sound").into_bytes()).expect("media"); + let mut t = Table::new(1, 1, 100.0, 40.0); + t.rows[0].cells[0].fill = Some(Fill::Picture(PictureFill { media: cell, ..Default::default() })); + let id = ShapeId(p.alloc_id().expect("id")); + let mut s = (*p.slides.first().cloned().unwrap_or_default()).clone(); + s.shapes.push(Shape { id, kind: ShapeKind::Table(t), ..Default::default() }); + s.background = Some(Background::Fill { fill: Fill::Picture(PictureFill { media: back, ..Default::default() }) }); + s.transition = Some(Transition { kind: "fade".into(), sound: Some(sound), ..Default::default() }); + p.slides = vec![Arc::new(s)]; + p + } + + #[test] + fn an_accepted_slide_keeps_its_cell_picture_background_and_sound() { + use deckcraft_model::{Background, Fill, MediaId, ShapeKind}; + + // Both decks hold the same media ids with different bytes. + let mut doc = media_deck("open"); + let rev = media_deck("revised"); + let sl = super::adopt(&mut doc, &rev, &rev.slides[0], None).unwrap(); + let bytes = |id: MediaId| doc.media(id).unwrap().data.as_ref().clone(); + let table = sl.shapes.iter().find_map(|x| if let ShapeKind::Table(t) = &x.kind { Some(t) } else { None }).unwrap(); + let Some(Fill::Picture(cell)) = &table.rows[0].cells[0].fill else { panic!("no cell picture") }; + let Some(Background::Fill { fill: Fill::Picture(back) }) = &sl.background else { panic!("no background picture") }; + let sound = sl.transition.as_ref().and_then(|t| t.sound).unwrap(); + assert_eq!( + [bytes(cell.media), bytes(back.media), bytes(sound)], + ["revised cell", "revised background", "revised sound"].map(|x| x.as_bytes().to_vec()) + ); + // The open deck's own slide is untouched. + let own = &doc.slides[0]; + let Some(Background::Fill { fill: Fill::Picture(back) }) = &own.background else { panic!("no background picture") }; + assert_eq!(bytes(back.media), b"open background"); + } + #[test] fn accept_one_then_all_makes_the_decks_equal_and_undoes() { let (mut s, rev, n) = decks(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 35b0ab39..e33a7513 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1722,7 +1722,8 @@ mod copy_remap { use std::sync::Arc; use deckcraft_model::style::PictureFill; - use deckcraft_model::{Animation, Shape, ShapeId, ShapeKind, Xfrm}; + use deckcraft_model::text::Bullet; + use deckcraft_model::{Animation, Fill, MediaId, Shape, ShapeId, ShapeKind, Table, TextBody, Transition, Xfrm}; use serde_json::json; use crate::*; @@ -1781,6 +1782,101 @@ mod copy_remap { assert_eq!(picture_bytes(&b, &shape(&b, 0, pb)), b"deck b picture"); } + /// A 4 by 4 PNG of one color. + fn solid_png(rgb: [u8; 3]) -> Vec { + deckcraft_render::Image { width: 4, height: 4, pixels: [rgb[0], rgb[1], rgb[2], 255].repeat(16) }.to_png() + } + + #[test] + fn paste_as_picture_into_a_deck_with_a_colliding_media_id_draws_the_copied_picture() { + let mut a = Session::with_new(); + let mut b = Session::with_new(); + let (pa, ..) = populate(&mut a, &solid_png([255, 0, 0])); + populate(&mut b, &solid_png([0, 0, 255])); + 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.pasteSpecial", &json!({"as": "picture"})).unwrap(); + let id = ShapeId(r["id"].as_u64().or_else(|| r["ids"][0].as_u64()).unwrap() as u32); + let img = deckcraft_render::decode_image(&picture_bytes(&b, &shape(&b, 0, id))).unwrap(); + // The middle of the drawing is the middle of the copied picture. + assert_eq!(img.get_pixel(img.width() / 2, img.height() / 2).0, [255, 0, 0, 255]); + } + + /// Slide 0 of `s` gets a table whose first cell, background and second cell's picture bullet + /// each use a picture of their own, and a transition sound. `tag` starts the bytes of each. + /// Returns the table. + fn picture_table(s: &mut Session, tag: &str) -> ShapeId { + let st = s.doc_mut().unwrap(); + let doc = Arc::make_mut(&mut st.doc); + let cell = doc.add_media("cell.png", "image/png", format!("{tag} cell").into_bytes()).expect("media"); + let back = doc.add_media("back.png", "image/png", format!("{tag} background").into_bytes()).expect("media"); + let bullet = doc.add_media("bullet.png", "image/png", format!("{tag} bullet").into_bytes()).expect("media"); + let sound = doc.add_media("sound.wav", "audio/wav", format!("{tag} sound").into_bytes()).expect("media"); + let mut t = Table::new(1, 2, 200.0, 40.0); + t.background = Some(Fill::Picture(PictureFill { media: back, ..Default::default() })); + t.rows[0].cells[0].fill = Some(Fill::Picture(PictureFill { media: cell, ..Default::default() })); + t.rows[0].cells[1].text = TextBody::from_text("item"); + t.rows[0].cells[1].text.paragraphs[0].props.bullet = Some(Bullet::Picture { media: bullet }); + let id = ShapeId(doc.alloc_id().expect("id")); + let slide = Arc::make_mut(&mut doc.slides[0]); + slide.shapes.push(Shape { id, kind: ShapeKind::Table(t), xfrm: Some(Xfrm::new(10.0, 10.0, 200.0, 40.0)), ..Default::default() }); + slide.transition = Some(Transition { kind: "fade".into(), sound: Some(sound), ..Default::default() }); + id + } + + /// The bytes behind a table's first cell fill, background and second cell's bullet. + fn table_pictures(s: &Session, sh: &Shape) -> [Vec; 3] { + let ShapeKind::Table(t) = &sh.kind else { panic!("not a table") }; + let doc = &s.doc().unwrap().doc; + let bytes = |id: MediaId| doc.media(id).unwrap().data.as_ref().clone(); + let Some(Fill::Picture(cell)) = &t.rows[0].cells[0].fill else { panic!("no cell picture") }; + let Some(Fill::Picture(back)) = &t.background else { panic!("no table picture") }; + let Some(Bullet::Picture { media }) = &t.rows[0].cells[1].text.paragraphs[0].props.bullet else { panic!("no picture bullet") }; + [bytes(cell.media), bytes(back.media), bytes(*media)] + } + + fn deck_pictures(tag: &str) -> [Vec; 3] { + ["cell", "background", "bullet"].map(|w| format!("{tag} {w}").into_bytes()) + } + + #[test] + fn a_table_pasted_into_another_deck_keeps_its_cell_background_and_bullet_pictures() { + let mut a = Session::with_new(); + let mut b = Session::with_new(); + let ta = picture_table(&mut a, "a"); + let tb = picture_table(&mut b, "b"); + select(&mut a, &[ta]); + 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!(table_pictures(&b, &shape(&b, 0, new)), deck_pictures("a")); + // Deck b's own table is untouched. + assert_eq!(table_pictures(&b, &shape(&b, 0, tb)), deck_pictures("b")); + } + + #[test] + fn a_slide_pasted_into_another_deck_keeps_its_table_pictures_and_transition_sound() { + let mut a = Session::with_new(); + let mut b = Session::with_new(); + picture_table(&mut a, "a"); + let tb = picture_table(&mut b, "b"); + a.execute("edit.copy", &json!({"scope": "slides"})).unwrap(); + b.clipboard = a.clipboard.clone(); + b.execute("edit.paste", &json!({"scope": "slides"})).unwrap(); + let d = b.doc().unwrap(); + let pasted = &d.doc.slides[1]; + let table = pasted.shapes.iter().find(|x| matches!(x.kind, ShapeKind::Table(_))).cloned().unwrap(); + assert_eq!(table_pictures(&b, &table), deck_pictures("a")); + let sound = pasted.transition.as_ref().and_then(|t| t.sound).unwrap(); + assert_eq!(d.doc.media(sound).unwrap().data.as_ref(), b"a sound"); + // Deck b's own slide is untouched. + assert_eq!(table_pictures(&b, &shape(&b, 0, tb)), deck_pictures("b")); + } + #[test] fn copied_connectors_follow_copied_ends_and_detach_from_the_rest() { let mut s = Session::with_new();