diff --git a/ROADMAP.md b/ROADMAP.md index 962bd5e7..76f27b5c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -132,6 +132,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| | 2026-10-17 | Opening a file caps absurd effect sizes (glow, soft edges, shadows, reflections, on shapes, text and table text) at what Format Shape allows: a glow of 10^18 pt in a .pptx made rendering ask for 34 GB and abort | +| 2026-10-17 | Opening a .deckcraft file whose shapes repeat an id gives the repeats fresh ids (groups, layouts and masters too), so the deck validates and edits by id find one shape | | 2026-10-15 | PPTX trigger sequences keep separate On Click steps, including paragraph builds; synthetic import/edit/save/reopen and headless show regressions | | 2026-10-15 | CLI: .txt and .md export includes table cell text, which it used to skip silently (#814) | | 2026-10-15 | A `.pptx` slide that can't be read is reported, not dropped in silence: `lostSlides` from `file.open`, a message and a repaired copy in the app, a warning from `deckcraft-cli` (#203) | diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index b9185556..e963027d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3779,3 +3779,49 @@ fn opening_a_file_caps_absurd_effect_sizes() { t.execute("file.render", &json!({"slide": i, "scale": 0.1})).unwrap(); } } + +/// A .deckcraft file whose shapes repeat an id (groups included) opens with fresh ids for the +/// repeats: the deck passes `validate`, the first shape keeps its id and none is lost. +#[test] +fn opening_a_file_gives_repeated_shape_ids_fresh_ones() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + for x in [0, 100, 200, 300] { + s.execute("shape.insert", &json!({"preset": "rect", "rect": [x, 50, 80, 80]})).unwrap(); + } + s.execute("edit.selectAll", &json!({})).unwrap(); + s.execute("arrange.group", &json!({})).unwrap(); + s.execute("shape.insert", &json!({"preset": "ellipse", "rect": [400, 300, 80, 80]})).unwrap(); + let first = s.doc().unwrap().shapes()[0].id; + // Every shape, the group's children too, takes the group's id. + s.edit(|doc, sel| { + fn all(shapes: &mut [deckcraft_model::Shape], id: deckcraft_model::ShapeId) { + for sh in shapes { + sh.id = id; + if let deckcraft_model::ShapeKind::Group { children, .. } = &mut sh.kind { + all(children, id); + } + } + } + all(crate::shapes_mut(doc, sel).unwrap(), first); + Ok(()) + }) + .unwrap(); + assert!(!s.doc().unwrap().doc.validate().is_empty(), "the deck repeats ids"); + let bytes = cmd::file::save_bytes(&s.doc().unwrap().doc, "deckcraft").unwrap(); + let mut t = Session::new(); + t.execute("file.openBytes", &json!({"name": "dup.deckcraft", "data": cmd::base64_encode(&bytes)})).unwrap(); + let st = t.doc().unwrap(); + assert_eq!(st.doc.validate(), Vec::::new()); + let slide = st.doc.slides.last().unwrap(); + let mut ids = vec![]; + deckcraft_model::walk(&slide.shapes, &mut |sh, _| ids.push(sh.id)); + assert_eq!(ids.len(), 6, "the group, its four children and the ellipse"); + assert_eq!(ids[0], first, "the first shape keeps its id"); + assert!(ids.iter().all(|id| id.0 <= st.doc.next_id), "new ids come from the deck's counter"); + // Each shape can be selected by its own id now. + let last = *ids.last().unwrap(); + t.execute("slide.go", &json!({"index": st.doc.slides.len() - 1})).unwrap(); + t.execute("selection.select", &json!({"id": last.0})).unwrap(); + assert_eq!(t.doc().unwrap().selected_shapes()[0].id, last); +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index f987a2b7..5701a1ec 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -197,6 +197,7 @@ pub fn repair(p: &mut Presentation) { } } } + reid_duplicate_shapes(p); p.slide_size = sane_slide_size(p.slide_size); // The notes page only lays out notes, but its size divides too. if !(p.notes_size.width.is_finite() && p.notes_size.width > 0.0 && p.notes_size.height.is_finite() && p.notes_size.height > 0.0) { @@ -205,6 +206,57 @@ pub fn repair(p: &mut Presentation) { sane_numbers(p); } +/// Give a shape that repeats an id already used on its slide, layout or master (groups included) +/// a fresh one, so edits, selection and the Selection Pane find one shape per id. The first keeps +/// its id, and with it what animations and connectors point at. An exhausted id space leaves the +/// rest as they are (#407). +fn reid_duplicate_shapes(p: &mut Presentation) { + fn has_duplicates(shapes: &[deckcraft_model::Shape]) -> bool { + let mut seen = std::collections::HashSet::new(); + let mut dup = false; + deckcraft_model::walk(shapes, &mut |sh, _| dup |= !seen.insert(sh.id)); + dup + } + fn reid(shapes: &mut [deckcraft_model::Shape], seen: &mut std::collections::HashSet, next: &mut u32, depth: usize) { + if depth > 64 { + return; + } + for sh in shapes { + if !seen.insert(sh.id) + && let Some(id) = (*next).max(1).checked_add(1) + { + *next = id; + sh.id = deckcraft_model::ShapeId(id); + seen.insert(sh.id); + } + if let deckcraft_model::ShapeKind::Group { children, .. } = &mut sh.kind { + reid(children, seen, next, depth + 1); + } + } + } + let mut next = p.next_id; + let mut fix = |shapes: &mut Vec| { + if has_duplicates(shapes) { + reid(shapes, &mut std::collections::HashSet::new(), &mut next, 0); + } + }; + for s in &mut p.slides { + if has_duplicates(&s.shapes) { + fix(&mut Arc::make_mut(s).shapes); + } + } + for m in p.masters.iter_mut().chain(p.notes_master.iter_mut()).chain(p.handout_master.iter_mut()) { + if has_duplicates(&m.shapes) || m.layouts.iter().any(|l| has_duplicates(&l.shapes)) { + let m = Arc::make_mut(m); + fix(&mut m.shapes); + for l in &mut m.layouts { + fix(&mut l.shapes); + } + } + } + p.next_id = next; +} + /// Smallest and largest slide side, in points: PowerPoint's 1–56 in, as Design › Slide Size allows. pub const SLIDE_SIDE: std::ops::RangeInclusive = 72.0..=4032.0;