From d453bee54f4dc0810e07b97ead32fa5c6c19b7d9 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:57:51 -0500 Subject: [PATCH 01/19] D604: readable animation effect labels --- ROADMAP.md | 3 + crates/ui-egui/src/ribbon/animations.rs | 89 ++++++++++++++++++++++++- docs/animation-parity.md | 5 +- docs/gaps.md | 4 +- 4 files changed, 96 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index a9936925..5a31b3bd 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,7 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (progress log: animation Effect Options show readable labels; stage and scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-27 · **Change:** trivial (progress log: Set Up Slide Show ▸ Keep mouse cursor visible, #1445; stage and scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-26 · **Change:** trivial (progress log: dragging several selected slides moves them all; previous: trivial, progress log: Edit Points handle drags with Shift, Ctrl/Cmd or Alt/Option set the point type; previous: trivial, progress log: scroll bars on the zoomed slide pane, #1157; previous: minor, native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: ~60 landed items in the progress log, among them the Font, Paragraph, Photo Album, theme colour and font and Date and Time dialogs, presenter and show controls, the SmartArt Text Pane and PPTX corpus slices; breadth 84% after `cargo xtask parity`, ready stays ~55%, stage stays alpha; duplicated rows and headers folded; previous: minor, hours to ~95% per audience added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-24 · **Change:** trivial (progress log: Edit Points handle drags with Shift, Ctrl/Cmd or Alt/Option set the point type; previous: trivial, progress log: scroll bars on the zoomed slide pane, #1157; previous: minor, native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: ~60 landed items in the progress log, among them the Font, Paragraph, Photo Album, theme colour and font and Date and Time dialogs, presenter and show controls, the SmartArt Text Pane and PPTX corpus slices; breadth 84% after `cargo xtask parity`, ready stays ~55%, stage stays alpha; duplicated rows and headers folded; previous: minor, hours to ~95% per audience added) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) @@ -146,6 +147,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-10-30 | Animation ▸ Effect Options shows readable labels for directions, split/shape variants, wheel spokes and other option values; stored codes are unchanged | | 2026-10-29 | Slide Show: going back to the previous slide plays the transition of the slide being left, reversed, as PowerPoint does, instead of the previous slide's own entrance transition (#1531); a jump (Go to Slide, links) still plays the target slide's transition | | 2026-10-27 | Set Up Slide Show ▸ Keep mouse cursor visible: a full-screen show no longer hides a still pointer, so the choices of a branching deck stay visible and clickable; the setting is kept with the user's preferences (`ui.json`, `showKeepCursor`), off by default (#1445) | | 2026-10-25 | A photo in a picture placeholder is cut into the shape its layout gives the placeholder, as PowerPoint draws it: a layout's round picture placeholder keeps a slide's photo in a circle with the layout's white border, where it came out as a rectangle (the reader only passed `p:sp` placeholders to `inherit_placeholder_geometry`, not `p:pic`) | @@ -314,6 +316,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-10-30 | trivial | Progress log: animation Effect Options show readable labels. Scores and milestone names unchanged | | 2026-10-29 | trivial | Progress log: going back in the slide show reverses the left slide's transition (#1531). Scores and milestone names unchanged | | 2026-10-29 | trivial | Progress log: the Picture Styles row leaves room for Crop and Size (#1232). Scores and milestone names unchanged | | 2026-10-25 | trivial | Progress log: a picture in a picture placeholder takes the layout's shape. Scores and milestone names unchanged | diff --git a/crates/ui-egui/src/ribbon/animations.rs b/crates/ui-egui/src/ribbon/animations.rs index 67f0bbd0..6d35afdc 100644 --- a/crates/ui-egui/src/ribbon/animations.rs +++ b/crates/ui-egui/src/ribbon/animations.rs @@ -1,4 +1,3 @@ -use super::transitions::option_label; use super::{ Align2, AnimClass, CornerRadius, Icon, Rect, Sense, SlideApp, Tokens, Ui, big, big_button, big_button_ex, dropdown, enabled, group, icons, json, pos2, rows, run, theme, vec2, widgets, @@ -81,12 +80,13 @@ pub(super) fn animations(app: &mut SlideApp, ui: &mut Ui) { } }); let effect_rows = cur.as_ref().map(|c| animation_effect_rows(&c.effect, c.class, Some(c.option.as_str()))).unwrap_or_default(); + let effect = cur.as_ref().map(|c| c.effect.as_str()).unwrap_or_default(); let build = cur.as_ref().map(|c| text_build_id(c.text_build)); let build_rows = marked_options(&["asOne", "byParagraph"], build); let r = big_button(ui, Icon::EffectOptions, "Effect\nOptions", !effect_rows.is_empty()); egui::Popup::menu(&r).show(|ui| { for &(o, on) in &effect_rows { - if ui.selectable_label(on, option_label(o)).clicked() { + if ui.selectable_label(on, animation_option_label(effect, o)).clicked() { run(app, "animation.options", json!({"option": o})); ui.close(); } @@ -216,6 +216,67 @@ pub(super) fn animation_effect_rows(effect: &str, class: AnimClass, current: Opt marked_options(opts, current) } +fn animation_option_label(effect: &str, option: &str) -> String { + let direction = match option { + "b" => Some("From Bottom"), + "lb" => Some("From Bottom-Left"), + "l" => Some("From Left"), + "lt" => Some("From Top-Left"), + "t" => Some("From Top"), + "rt" => Some("From Top-Right"), + "r" => Some("From Right"), + "rb" => Some("From Bottom-Right"), + _ => None, + }; + if matches!(effect, "fly" | "flyOut" | "wipe" | "wipeOut" | "peek" | "peekOut") + && let Some(label) = direction + { + return label.into(); + } + + match (effect, option) { + ("float" | "floatOut", "u") => "Up".into(), + ("float" | "floatOut", "d") => "Down".into(), + ("split" | "splitOut", "horzIn") => "Horizontal In".into(), + ("split" | "splitOut", "horzOut") => "Horizontal Out".into(), + ("split" | "splitOut", "vertIn") => "Vertical In".into(), + ("split" | "splitOut", "vertOut") => "Vertical Out".into(), + ("shape" | "shapeOut", o) if o.ends_with("In") => format!("{} In", title_first(shape_option_name(o))), + ("shape" | "shapeOut", o) if o.ends_with("Out") => format!("{} Out", title_first(shape_option_name(o))), + ("wheel" | "wheelOut", "1") => "1 Spoke".into(), + ("wheel" | "wheelOut", n) => format!("{n} Spokes"), + ("box" | "boxOut" | "circle" | "diamond", "in") => format!("{} In", title_first(effect.trim_end_matches("Out"))), + ("box" | "boxOut" | "circle" | "diamond", "out") => format!("{} Out", title_first(effect.trim_end_matches("Out"))), + ("zoom" | "zoomOut", "in") => "Zoom In".into(), + ("zoom" | "zoomOut", "out") => "Zoom Out".into(), + (_, "lu") => "From Top-Left".into(), + (_, "ru") => "From Top-Right".into(), + (_, "ld") => "From Bottom-Left".into(), + (_, "rd") => "From Bottom-Right".into(), + (_, "horz") => "Horizontal".into(), + (_, "vert") => "Vertical".into(), + (_, "across") => "Across".into(), + (_, "down") => "Down".into(), + (_, "up") => "Up".into(), + (_, "right") => "Right".into(), + (_, "left") => "Left".into(), + (_, "clockwise") => "Clockwise".into(), + (_, "counterClockwise") => "Counterclockwise".into(), + (_, "in") => "In".into(), + (_, "out") => "Out".into(), + (_, other) => super::transitions::option_label(other), + } +} + +fn shape_option_name(option: &str) -> &str { + option.strip_suffix("In").or_else(|| option.strip_suffix("Out")).unwrap_or(option) +} + +fn title_first(value: &str) -> String { + let mut chars = value.chars(); + chars.next().map(|c| c.to_uppercase().collect::() + chars.as_str()).unwrap_or_default() +} + pub(super) fn text_build_id(build: deckcraft_model::anim::TextBuild) -> &'static str { match build { deckcraft_model::anim::TextBuild::ByParagraph => "byParagraph", @@ -224,3 +285,27 @@ pub(super) fn text_build_id(build: deckcraft_model::anim::TextBuild) -> &'static deckcraft_model::anim::TextBuild::AsOne => "asOne", } } + +#[cfg(test)] +mod animation_option_label_tests { + use super::animation_option_label; + use deckcraft_model::anim::ANIMATIONS; + + #[test] + fn labels_every_animation_gallery_option_without_changing_its_code() { + for &(effect, _, _, _, _, options) in ANIMATIONS { + for &option in options { + let label = animation_option_label(effect, option); + assert!(!label.is_empty(), "{effect}/{option}"); + assert_ne!(label, option, "raw option shown for {effect}/{option}"); + } + } + assert_eq!(animation_option_label("fly", "lb"), "From Bottom-Left"); + assert_eq!(animation_option_label("flyOut", "rt"), "From Top-Right"); + assert_eq!(animation_option_label("wipe", "b"), "From Bottom"); + assert_eq!(animation_option_label("wheel", "1"), "1 Spoke"); + assert_eq!(animation_option_label("split", "vertOut"), "Vertical Out"); + assert_eq!(animation_option_label("shape", "circleIn"), "Circle In"); + assert_eq!(animation_option_label("diamond", "out"), "Diamond Out"); + } +} diff --git a/docs/animation-parity.md b/docs/animation-parity.md index b7f840a9..ffca88c9 100644 --- a/docs/animation-parity.md +++ b/docs/animation-parity.md @@ -1,6 +1,6 @@ # Transitions and animations: parity with PowerPoint -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-29 · **Change:** minor (going back in the show reverses the left slide's transition, #1531; previous: minor, Animation Pane ▸ Play From plays from the selected effect; previous: community PR wave 2026-10-14: interactive-deck triggers, canvas preview, the Animation Pane timeline, repeat and bounce end round-trip; duplicated rows and headers folded; previous: major, transitions measured frame by frame against PowerPoint for Windows' video export) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (animation Effect Options translate stored codes into readable labels; previous: minor, going back in the show reverses the left slide's transition, #1531; previous: minor, Animation Pane ▸ Play From plays from the selected effect; previous: community PR wave 2026-10-14: interactive-deck triggers, canvas preview, the Animation Pane timeline, repeat and bounce end round-trip; duplicated rows and headers folded; previous: major, transitions measured frame by frame against PowerPoint for Windows' video export) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) Transitions and animations are what make a deck a presentation, and the part of PowerPoint people notice first when it's missing. Scores roll up into [target-app-parity.md](target-app-parity.md) @@ -65,7 +65,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Animation feature | PowerPoint | DeckCraft | |---|---|---| | Start On Click / With Previous / After Previous, duration, delay | yes | yes | -| Effect Options (direction, sequence by paragraph) | yes | yes | +| Effect Options (direction, sequence by paragraph) | yes | yes; every gallery option has a readable label while retaining its stored code | | Effect dialog: sound, after-animation dim/hide, animate text by word/letter, smooth start/end, bounce end, repeat, rewind | yes | partly; repeat, bounce end and rewind round-trip | | By-paragraph text builds, sub-bullets follow parents | yes | yes; builds edited in PowerPoint (own start per paragraph, other effects in between, by 2nd-level paragraphs) import as one effect per paragraph and keep their clicks (#844) | | Triggers (on click of a shape) | yes | yes; played in the show: a step per click, the sequence starts over after its last step, a click completes a playing step. Separate click steps, mixed starts and paragraph builds survive PPTX save (`trigger_steps.rs`) | @@ -94,6 +94,7 @@ In, Wipe, Zoom, Float In, Grow/Shrink, Pulse, Spin, motion lines): weighted, ~85 | Date | Change | Summary | |---|---|---| +| 2026-10-30 | trivial | Animation Effect Options show readable, effect-aware labels for all gallery option codes; stored values and command parameters stay unchanged | | 2026-10-29 | minor | Going back in the show plays the transition of the slide being left, reversed, instead of the previous slide's entrance transition (#1531); new row in the transition features. Scores unchanged | | 2026-10-22 | minor | Animation Pane ▸ Play From: with an effect selected, the preview starts at that effect on the play-all clock; the row says yes and the estimate drops to ~14–26 h | | 2026-10-15 | trivial | PPTX interactive sequences preserve later On Click effects and one click per paragraph; no change to playback or feature scores | diff --git a/docs/gaps.md b/docs/gaps.md index c1746a17..885a64d1 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,5 +1,6 @@ # Where DeckCraft falls short of PowerPoint +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (animation Effect Options now show readable labels; Animation Pane gap remains) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-28 · **Change:** trivial (AutoFormat lists run independently of Replace text as you type; previous: minor, Animation Pane ▸ Play From; previous: minor, native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: soak tests, PPTX corpus, comment threads (#70), embedded fonts, SmartArt Text Pane, Animation Pane timeline, Hunspell spelling, thesaurus, AutoCorrect, 14 new dialogs, pen pressure and trackpad gestures; duplicated entries and headers folded; previous: minor, 3-D bevel, camera and face lighting rendered) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-20 · **Change:** trivial (empty chart number-format sections hide values; previous: minor, native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: soak tests, PPTX corpus, comment threads (#70), embedded fonts, SmartArt Text Pane, Animation Pane timeline, Hunspell spelling, thesaurus, AutoCorrect, 14 new dialogs, pen pressure and trackpad gestures; duplicated entries and headers folded; previous: minor, 3-D bevel, camera and face lighting rendered) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-27 · **Last updated:** 2026-10-27 · **Change:** minor (docs refresh against merged PRs: #67 and #77 landed, #75 closed unmerged, #53 closed, Windows image paste is #1414; one header line; previous: minor, Animation Pane ▸ Play From; previous: minor, native chart payloads stored in bounded ZIP members; previous: major, community PR wave 2026-10-14: soak tests, PPTX corpus, comment threads (#70), embedded fonts, SmartArt Text Pane, Animation Pane timeline, Hunspell spelling, thesaurus, AutoCorrect, 14 new dialogs, pen pressure and trackpad gestures; duplicated entries and headers folded; previous: minor, 3-D bevel, camera and face lighting rendered) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) @@ -28,7 +29,7 @@ revision history. | 7 | Fonts: only Regular, Bold and Italic faces (#69) | Feature | B | 3–5 | [ui-parity.md](ui-parity.md) | | 8 | Image paste fails on Windows (#1414); Linux not re-checked | Feature | B | 2–4 | [ui-parity.md](ui-parity.md) | | 9 | Presenter view isn't placed on the second display | Hardware | B | 3–5 | [slideshow-parity.md](slideshow-parity.md) | -| 10 | Animation Pane: 64 of ~190 effects, effect options (timeline and Play From done) | Feature | B (pane) | 7–13 | [animation-parity.md](animation-parity.md) | +| 10 | Animation Pane: 64 of ~190 effects, effect options (readable labels, timeline and Play From done) | Feature | B (pane) | 7–13 | [animation-parity.md](animation-parity.md) | | 11 | Charts: data grid editing, elements, `chartex` types | Feature | B (data, elements) | 12–20 | [parity-checklist.md](parity-checklist.md) | | 12 | Format Shape pane thinner than PowerPoint's | UI/UX | B | 6–10 | [ui-parity.md](ui-parity.md) | | 13 | No native macOS menu bar | UI/UX | B | 3–5 | [ui-parity.md](ui-parity.md) | @@ -370,6 +371,7 @@ Detail in [localization-parity.md](localization-parity.md). | Date | Change | Summary | |---|---|---| +| 2026-10-30 | trivial | Animation effect options display readable labels for every gallery value; the broader Animation Pane gap remains | | 2026-10-28 | trivial | Automatic lists now run independently of Replace text as you type; proofing gap description updated | | 2026-10-27 | minor | Refreshed against merged PRs: removed the Morph alignment entry (#67 merged 2026-10-14); #77 (animation rewind, PR #83) is fixed, leaving #74 in the round-trip gap; #75 was closed unmerged, so #60 stays open with no fix PR; #53 is closed and the image-paste gap is now Windows #1414; Reuse Slides (#1103) and Compare reject (#850) noted in the smaller rows; the duplicate header line is gone | | 2026-10-25 | trivial | New entries: copying text and cell ranges (no media, no table), a control-server timing test that may fail under load; Proofing notes that automatic lists follow Replace text as you type and that Backspace puts the typed lead back | From b1900e526d02ab4b05c88de0782a0b8953f99be4 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:42:28 -0500 Subject: [PATCH 02/19] Preserve point-sized bullets in PPTX --- crates/model/src/text.rs | 3 +++ crates/pptx/src/read/dml.rs | 4 ++++ crates/pptx/src/write/dml.rs | 4 ++++ crates/pptx/tests/it/roundtrip.rs | 24 ++++++++++++++++++++++++ crates/text/src/shape.rs | 4 ++-- 5 files changed, 37 insertions(+), 2 deletions(-) diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 17d606de..525f1df3 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -240,6 +240,8 @@ pub struct ParaProps { pub bullet_color: Option, /// Bullet size as a fraction of the text size. pub bullet_size: Option, + /// Absolute bullet font size in points, independent of the paragraph's run size. + pub bullet_size_points: Option, pub tabs: Option>, pub default_tab: Option, pub rtl: Option, @@ -270,6 +272,7 @@ impl ParaProps { self.bullet_color.clone_from(&p.bullet_color); } self.bullet_size = self.bullet_size.or(p.bullet_size); + self.bullet_size_points = self.bullet_size_points.or(p.bullet_size_points); if self.tabs.is_none() { self.tabs.clone_from(&p.tabs); } diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 767b7dc0..7a3cc217 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -433,6 +433,10 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option "buFont" => pp.bullet_font = e.attr("typeface").map(String::from), "buClr" => pp.bullet_color = color(e), "buSzPct" => pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)), + "buSzPts" => { + pp.bullet_size_points = + e.attr("val").and_then(|v| v.parse::().ok()).filter(|v| v.is_finite() && *v > 0.0 && *v <= 40_000.0).map(|v| v / 100.0); + } "tabLst" => { pp.tabs = Some( e.children_named("tab") diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 518f8c4a..70b085e2 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -535,6 +535,7 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: || p.bullet_font.is_some() || p.bullet_color.is_some() || p.bullet_size.is_some() + || p.bullet_size_points.is_some() || p.tabs.is_some() || def.is_some(); if !has_kids { @@ -559,6 +560,9 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: if let Some(s) = p.bullet_size { w.val("a:buSzPct", pct(s.clamp(0.25, 4.0))); } + if let Some(s) = p.bullet_size_points { + w.val("a:buSzPts", ((s.clamp(0.01, 400.0) * 100.0).round() as u32).to_string()); + } if let Some(f) = &p.bullet_font { w.empty("a:buFont", A::new().a("typeface", f)); } diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index 370cc9a9..7936234c 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -488,6 +488,30 @@ fn shapes_fills_lines_effects_round_trip() { assert_eq!(by_name(&q.slides[1], "Group").id, by_name(&p.slides[1], "Group").id); } +#[test] +fn bullet_size_in_points_survives_two_pptx_round_trips() { + let mut p = Presentation::default(); + let mut textbox = shape(&mut p, "Point-sized bullet", 20.0, 20.0, 400.0, 100.0); + textbox.text = Some(TextBody { + paragraphs: vec![Paragraph { + props: deckcraft_model::text::ParaProps { + bullet: Some(Bullet::Char { char: "•".into() }), + bullet_size_points: Some(12.0), + ..Default::default() + }, + runs: vec![Run::with("Bullet text", RunProps::default())], + ..Default::default() + }], + ..Default::default() + }); + Arc::make_mut(&mut p.slides[0]).shapes.push(textbox); + + let q = round(&round(&p)); + let props = &by_name(&q.slides[0], "Point-sized bullet").text.as_ref().expect("text").paragraphs[0].props; + assert_eq!(props.bullet_size_points, Some(12.0)); + assert_eq!(props.bullet_size, None); +} + #[test] fn pictures_media_and_ink_round_trip() { let p = rich_deck(); diff --git a/crates/text/src/shape.rs b/crates/text/src/shape.rs index 2be8424f..d5558001 100644 --- a/crates/text/src/shape.rs +++ b/crates/text/src/shape.rs @@ -76,7 +76,7 @@ pub(crate) fn shape_para( if let Some(c) = &props.bullet_color { rp.fill = Some(Fill::solid(c.clone())); } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0))); rp.underline = None; rp.strike = None; rp.highlight = None; @@ -105,7 +105,7 @@ pub(crate) fn shape_para( if let Some(c) = &props.bullet_color { rp.fill = Some(Fill::solid(c.clone())); } - rp.size = Some(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0)); + rp.size = Some(props.bullet_size_points.unwrap_or(lead.size.unwrap_or(18.0) * props.bullet_size.unwrap_or(1.0))); rp.underline = None; let mut st = make_style(ctx, &rp, scale, opts.prompt_color); let label = autonum(scheme, start_at.saturating_sub(1).saturating_add(number.unwrap_or(1))); From 917d4b96a9ceb2063a2739a098375fad1395f3bc Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:07:54 -0500 Subject: [PATCH 03/19] Honor bullet size schema bounds and inheritance --- crates/model/src/text.rs | 29 +++++++++++++++++++++++++++-- crates/pptx/src/read/dml.rs | 8 ++++++-- crates/pptx/src/write/dml.rs | 7 +++---- crates/pptx/tests/it/roundtrip.rs | 25 ++++++++++++++----------- 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 525f1df3..8d01cdcd 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -271,8 +271,13 @@ impl ParaProps { if self.bullet_color.is_none() { self.bullet_color.clone_from(&p.bullet_color); } - self.bullet_size = self.bullet_size.or(p.bullet_size); - self.bullet_size_points = self.bullet_size_points.or(p.bullet_size_points); + if self.bullet_size.is_none() && self.bullet_size_points.is_none() { + if p.bullet_size_points.is_some() { + self.bullet_size_points = p.bullet_size_points; + } else { + self.bullet_size = p.bullet_size; + } + } if self.tabs.is_none() { self.tabs.clone_from(&p.tabs); } @@ -287,6 +292,26 @@ impl ParaProps { } } +#[cfg(test)] +mod bullet_size_tests { + use super::ParaProps; + + #[test] + fn bullet_size_inheritance_keeps_percentage_and_points_as_one_mode() { + let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() }; + let mut child = ParaProps { bullet_size: Some(2.0), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, Some(2.0)); + assert_eq!(child.bullet_size_points, None); + + let parent = ParaProps { bullet_size: Some(1.5), ..Default::default() }; + let mut child = ParaProps { bullet_size_points: Some(14.0), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, None); + assert_eq!(child.bullet_size_points, Some(14.0)); + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum Caps { diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 7a3cc217..5076c376 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -432,10 +432,14 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option } "buFont" => pp.bullet_font = e.attr("typeface").map(String::from), "buClr" => pp.bullet_color = color(e), - "buSzPct" => pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)), + "buSzPct" => { + pp.bullet_size_points = None; + pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)); + } "buSzPts" => { + pp.bullet_size = None; pp.bullet_size_points = - e.attr("val").and_then(|v| v.parse::().ok()).filter(|v| v.is_finite() && *v > 0.0 && *v <= 40_000.0).map(|v| v / 100.0); + e.attr("val").and_then(|v| v.parse::().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0); } "tabLst" => { pp.tabs = Some( diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 70b085e2..9efb990f 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -557,12 +557,11 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: color(w, c); w.close("a:buClr"); } - if let Some(s) = p.bullet_size { + if let Some(s) = p.bullet_size_points.filter(|s| s.is_finite() && (1.0..=4000.0).contains(s)) { + w.val("a:buSzPts", ((s * 100.0).round() as u32).to_string()); + } else if let Some(s) = p.bullet_size { w.val("a:buSzPct", pct(s.clamp(0.25, 4.0))); } - if let Some(s) = p.bullet_size_points { - w.val("a:buSzPts", ((s.clamp(0.01, 400.0) * 100.0).round() as u32).to_string()); - } if let Some(f) = &p.bullet_font { w.empty("a:buFont", A::new().a("typeface", f)); } diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index 7936234c..065eb849 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -493,23 +493,26 @@ fn bullet_size_in_points_survives_two_pptx_round_trips() { let mut p = Presentation::default(); let mut textbox = shape(&mut p, "Point-sized bullet", 20.0, 20.0, 400.0, 100.0); textbox.text = Some(TextBody { - paragraphs: vec![Paragraph { - props: deckcraft_model::text::ParaProps { - bullet: Some(Bullet::Char { char: "•".into() }), - bullet_size_points: Some(12.0), + paragraphs: [12.0, 500.0, 4000.0] + .into_iter() + .map(|size| Paragraph { + props: deckcraft_model::text::ParaProps { + bullet: Some(Bullet::Char { char: "•".into() }), + bullet_size_points: Some(size), + ..Default::default() + }, + runs: vec![Run::with("Bullet text", RunProps::default())], ..Default::default() - }, - runs: vec![Run::with("Bullet text", RunProps::default())], - ..Default::default() - }], + }) + .collect(), ..Default::default() }); Arc::make_mut(&mut p.slides[0]).shapes.push(textbox); let q = round(&round(&p)); - let props = &by_name(&q.slides[0], "Point-sized bullet").text.as_ref().expect("text").paragraphs[0].props; - assert_eq!(props.bullet_size_points, Some(12.0)); - assert_eq!(props.bullet_size, None); + let props = &by_name(&q.slides[0], "Point-sized bullet").text.as_ref().expect("text").paragraphs; + assert_eq!(props.iter().map(|p| p.props.bullet_size_points).collect::>(), vec![Some(12.0), Some(500.0), Some(4000.0)]); + assert!(props.iter().all(|p| p.props.bullet_size.is_none())); } #[test] From 98138f563b357921ebc2f6fe6ff7ffd57d50aae7 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Fri, 30 Oct 2026 23:24:07 -0500 Subject: [PATCH 04/19] Trim whitespace in bullet point sizes --- crates/pptx/src/read/dml.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 5076c376..11949be5 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -23,6 +23,10 @@ pub fn pct(e: &El, name: &str) -> Option { e.f64(name).map(|v| v / 100_000.0) } +fn bullet_size_points(value: Option<&str>) -> Option { + value.and_then(|v| v.trim().parse::().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0) +} + /// 1/60 000 degree → degrees. pub fn ang(e: &El, name: &str) -> Option { e.f64(name).map(|v| v / 60_000.0) @@ -438,8 +442,7 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option } "buSzPts" => { pp.bullet_size = None; - pp.bullet_size_points = - e.attr("val").and_then(|v| v.parse::().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0); + pp.bullet_size_points = bullet_size_points(e.attr("val")); } "tabLst" => { pp.tabs = Some( @@ -676,6 +679,13 @@ mod tests { assert_eq!(color(&d.root).unwrap().base, ColorBase::Rgb { rgb: Rgba::rgb(255, 0, 0) }); } + #[test] + fn bullet_point_size_trims_schema_whitespace() { + let d = parse(br#""#).unwrap(); + assert_eq!(bullet_size_points(d.root.attr("val")), Some(14.0)); + assert_eq!(bullet_size_points(Some("1400")), Some(14.0)); + } + #[test] fn body_props() { let d = parse(br#""#).unwrap(); From ea9dad8ef988362f4a365d5cf44f1d525b00ad5b Mon Sep 17 00:00:00 2001 From: Naseq Date: Mon, 2 Nov 2026 15:04:05 +0430 Subject: [PATCH 05/19] Transitions: play transition preview from previous slide when Preview is clicked (#1654) - When Preview is clicked on the Transitions ribbon, initialize Show with start_transition_preview from the prior slide into the selected slide (or from black for slide 0). - If tr.old is unset for a non-part transition with tr.from, render the previous slide texture on the first frame so the transition blends cleanly. - Indicate active playback on the Preview button while previewing. - Regression tests for transitions from previous slide and from black on slide 0. --- crates/ui-egui/src/ribbon/transitions.rs | 13 +++++---- crates/ui-egui/src/show.rs | 21 +++++++++++++++ crates/ui-egui/src/show/tests.rs | 34 ++++++++++++++++++++++++ crates/ui-egui/src/show/ui.rs | 10 +++++++ 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/ribbon/transitions.rs b/crates/ui-egui/src/ribbon/transitions.rs index 3f43ebd9..6986d828 100644 --- a/crates/ui-egui/src/ribbon/transitions.rs +++ b/crates/ui-egui/src/ribbon/transitions.rs @@ -12,11 +12,14 @@ pub(super) fn transitions(app: &mut SlideApp, ui: &mut Ui) { let cur = app.session.active().and_then(|d| d.current_slide().and_then(|s| s.transition.clone())); let kind = cur.as_ref().map(|t| t.kind.clone()).unwrap_or_else(|| "none".into()); group(ui, |ui| { - if big_button(ui, Icon::Preview, "Preview", true).clicked() { - let from = app.session.active().map(|d| d.selection.slide).unwrap_or(0); - app.start_show(from, true); - if let Some(s) = app.show.as_mut() { - s.preview_only = true; + let playing = app.show.as_ref().is_some_and(|s| s.preview_only); + let (resp, _) = widgets::big_button_ex(ui, Icon::Preview, "Preview", true, false, playing); + if resp.on_hover_text("Play this slide's transition").clicked() { + let to = app.session.active().map(|d| d.selection.slide).unwrap_or(0); + app.start_show(to, true); + let now = ui.input(|i| i.time); + if let (Some(s), Some(d)) = (app.show.as_mut(), app.session.active()) { + s.start_transition_preview(&d.doc, now); } } }); diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index 1896e3be..7bd2edf7 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -261,6 +261,27 @@ impl Show { } } + /// Play this slide's transition from the previous slide (or black for slide 0) as a preview (#1654). + pub fn start_transition_preview(&mut self, doc: &Presentation, now: f64) { + let i = self.state.slide; + let from = i.checked_sub(1); + let (kind, option, dur) = doc + .slides + .get(i) + .and_then(|s| s.transition.as_ref()) + .map(|t| (t.kind.clone(), t.option.clone(), t.duration_ms as f64 / 1000.0)) + .unwrap_or(("none".into(), String::new(), 0.0)); + self.trans = if kind != "none" && dur > 0.0 && !doc.show.without_animation { + Some(Transition { old: None, from, morph: None, parts: None, start: now, kind, option, dur, reverse: false }) + } else { + None + }; + self.preview_only = true; + self.slide_start = now; + self.step_start = now; + self.step_end = now; + } + /// Identity of the show state a queued window-point event belongs to. pub(crate) fn input_state(&self) -> ShowInputState { let bits = |v: f32| if v.is_finite() { v.to_bits() } else { 0 }; diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index a1069a5b..5e39bd18 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1238,3 +1238,37 @@ fn a_preview_of_an_edited_presentation_is_rendered_again() { let tex = harness.state().1.next_tex.as_ref().map(|(d, k, _)| (std::sync::Arc::ptr_eq(d, &edited), *k)); assert_eq!(tex, Some((true, 1)), "the preview comes from the edited presentation"); } + +/// #1654: Preview on the Transitions ribbon plays the current slide's transition from the +/// previous slide into the current slide, and exits when finished. +#[test] +fn transition_preview_plays_from_the_previous_slide() { + let mut s = deckcraft_engine::Session::with_new(); + s.execute("slide.new", &json!({})).unwrap(); + s.execute("transition.set", &json!({"kind": "push", "index": 1, "duration": 1.0})).unwrap(); + let doc = s.active().unwrap().doc.clone(); + let mut show = Show::new(&doc, 1, true, false); + show.start_transition_preview(&doc, 10.0); + assert!(show.preview_only); + let tr = show.trans.as_ref().expect("transition preview is playing"); + assert_eq!(tr.kind, "push"); + assert_eq!(tr.from, Some(0), "enters from slide 0 into slide 1"); + assert_eq!(tr.dur, 1.0); + assert!(!tr.reverse); + assert_eq!(tr.start, 10.0); +} + +/// #1654: Transition preview for slide 0 plays with from = None (transitioning from black). +#[test] +fn transition_preview_for_first_slide_transitions_from_black() { + let mut s = deckcraft_engine::Session::with_new(); + s.execute("transition.set", &json!({"kind": "fade", "index": 0, "duration": 500})).unwrap(); + let doc = s.active().unwrap().doc.clone(); + let mut show = Show::new(&doc, 0, true, false); + show.start_transition_preview(&doc, 5.0); + assert!(show.preview_only); + let tr = show.trans.as_ref().expect("fade transition preview is playing"); + assert_eq!(tr.kind, "fade"); + assert_eq!(tr.from, None, "slide 0 transitions from black (None)"); + assert_eq!(tr.dur, 0.5); +} diff --git a/crates/ui-egui/src/show/ui.rs b/crates/ui-egui/src/show/ui.rs index 71107f94..321f8116 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -388,6 +388,16 @@ fn draw_transition(show: &mut Show, f: &Fr, sl: &Sl, painter: &egui::Painter, te if tr.parts.is_none() && deckcraft_anim::uses_slide_parts(&tr.kind, &tr.option) { tr.parts = Some(part_textures(ctx, doc, tr.from, idx, size)); } + if tr.old.is_none() + && let Some(prev) = tr.from + && !deckcraft_anim::uses_slide_parts(&tr.kind, &tr.option) + { + let threads = + if cfg!(target_arch = "wasm32") { 0 } else { std::thread::available_parallelism().map(|n| n.get().min(8) as u16).unwrap_or(0) }; + let opts = RenderOpts { scale: size.0 as f64 / doc.slide_size.width.max(1.0), size: Some(size), threads, ..Default::default() }; + let img = deckcraft_render::render_slide(doc, prev, &opts); + tr.old = Some(crate::textures::upload(ctx, "trans-old", crate::textures::to_color_image(&img, false))); + } let aspect = doc.slide_size.width / doc.slide_size.height.max(1.0); let part = |k: usize| tr.parts.as_ref().and_then(|p| p.get(k).cloned().flatten()); let parts = [part(0), part(1), part(2), part(3)]; From 4bed9b6c370350cc9e76625e7e3a7d409631beee Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:50:54 +0100 Subject: [PATCH 06/19] Reject unsupported handout perPage values in file.export Found while testing crates/engine/src/cmd/file.rs:362 (pdf_options) with crates/pdf/src/lib.rs:364-371 (handout_boxes) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file.rs | 8 +++++++- crates/engine/src/tests.rs | 13 +++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 0d52e635..dc9f1a44 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -359,7 +359,13 @@ pub fn pdf_options(p: &Value, count: usize) -> Result let mut o = deckcraft_pdf::PdfOptions { layout: deckcraft_pdf::PageLayout::Slides, ..Default::default() }; o.layout = match str_param(p, "layout").unwrap_or("slides") { "notes" => deckcraft_pdf::PageLayout::Notes, - "handouts" => deckcraft_pdf::PageLayout::Handouts { per_page: usize_param(p, "perPage").unwrap_or(6).clamp(1, 9) as u8 }, + "handouts" => { + let per_page = usize_param(p, "perPage").unwrap_or(6); + if ![1, 2, 3, 4, 6, 9].contains(&per_page) { + return Err(bad("file.export", format!("perPage {per_page} isn't one of 1, 2, 3, 4, 6, 9"))); + } + deckcraft_pdf::PageLayout::Handouts { per_page: per_page as u8 } + } _ => deckcraft_pdf::PageLayout::Slides, }; o.dpi = f64_or(p, "dpi", o.dpi).clamp(36.0, 600.0); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 68de1341..e8038c9a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2410,6 +2410,19 @@ fn pdf_options_take_paper_and_orientation() { assert!(cmd::file::pdf_options(&json!({"orientation": "sideways"}), 1).is_err()); } +/// Handout `perPage` accepts only 1, 2, 3, 4, 6 and 9; anything else is an error, not a 9-up page. +#[test] +fn pdf_options_reject_unsupported_per_page() { + for n in [1, 2, 3, 4, 6, 9] { + let o = cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": n}), 1).unwrap(); + assert!(matches!(o.layout, deckcraft_pdf::PageLayout::Handouts { per_page } if per_page as usize == n)); + } + for n in [0, 5, 7, 8, 10] { + let e = cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": n}), 1).unwrap_err(); + assert!(e.to_string().contains("1, 2, 3, 4, 6, 9"), "perPage {n}: {e}"); + } +} + #[test] fn shape_line_sets_compound_and_arrow_sizes() { // The Format Shape pane's Line page: compound type, arrowheads and their sizes. From e0992c132568a26259d686c84c2c804deae201ee Mon Sep 17 00:00:00 2001 From: Naseq Date: Mon, 2 Nov 2026 15:25:31 +0430 Subject: [PATCH 07/19] Transitions: render cached previous slide with completed animation state - Use render_frame with timeline.steps() and t = 0.0 so the previous slide is rendered in its final completed state during transition preview. - Pass duration: 1000 integer ms in test to explicitly test the requested duration. --- crates/ui-egui/src/show/tests.rs | 2 +- crates/ui-egui/src/show/ui.rs | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index 5e39bd18..c7c34b17 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1245,7 +1245,7 @@ fn a_preview_of_an_edited_presentation_is_rendered_again() { fn transition_preview_plays_from_the_previous_slide() { let mut s = deckcraft_engine::Session::with_new(); s.execute("slide.new", &json!({})).unwrap(); - s.execute("transition.set", &json!({"kind": "push", "index": 1, "duration": 1.0})).unwrap(); + s.execute("transition.set", &json!({"kind": "push", "index": 1, "duration": 1000})).unwrap(); let doc = s.active().unwrap().doc.clone(); let mut show = Show::new(&doc, 1, true, false); show.start_transition_preview(&doc, 10.0); diff --git a/crates/ui-egui/src/show/ui.rs b/crates/ui-egui/src/show/ui.rs index 321f8116..b02a181b 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -10,7 +10,7 @@ use super::zoom::{ }; use super::{ Align2, BackKey, Click, Color32, CornerRadius, Key, Layer, LinkJump, Pos2, Rect, RenderOpts, Sense, ShapeId, ShapeState, Show, ShowAction, - SlideApp, Source, Stroke, TextureHandle, Ui, json, pos2, show_fills_screen, state_for, theme, vec2, viewport_visible, + ShowState, SlideApp, Source, Stroke, TextureHandle, Ui, json, pos2, show_fills_screen, state_for, theme, vec2, viewport_visible, }; use deckcraft_anim::{StepStamp, TriggerPlay}; use deckcraft_model::Presentation; @@ -394,9 +394,10 @@ fn draw_transition(show: &mut Show, f: &Fr, sl: &Sl, painter: &egui::Painter, te { let threads = if cfg!(target_arch = "wasm32") { 0 } else { std::thread::available_parallelism().map(|n| n.get().min(8) as u16).unwrap_or(0) }; - let opts = RenderOpts { scale: size.0 as f64 / doc.slide_size.width.max(1.0), size: Some(size), threads, ..Default::default() }; - let img = deckcraft_render::render_slide(doc, prev, &opts); - tr.old = Some(crate::textures::upload(ctx, "trans-old", crate::textures::to_color_image(&img, false))); + let timeline = ShowState::timeline(doc, prev); + let step = timeline.steps(); + let ci = render_frame(doc, prev, timeline, step, 0.0, size, vec![], &[], threads, &[], &[], None); + tr.old = Some(crate::textures::upload(ctx, "trans-old", ci)); } let aspect = doc.slide_size.width / doc.slide_size.height.max(1.0); let part = |k: usize| tr.parts.as_ref().and_then(|p| p.get(k).cloned().flatten()); From 72ac32715ffd5c94de9e6d6b1cfb9bce7ccb65c4 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 2 Nov 2026 12:21:37 +0100 Subject: [PATCH 08/19] Reject invalid master and layout indices in view.slideMaster Found while testing crates/engine/src/cmd/design/master_view.rs:8 (master_view; `master` read via usize_param, `layout` at :14) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/design/master_view.rs | 6 +++--- crates/engine/src/tests.rs | 11 +++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/design/master_view.rs b/crates/engine/src/cmd/design/master_view.rs index 5dfe0496..c03bfa57 100644 --- a/crates/engine/src/cmd/design/master_view.rs +++ b/crates/engine/src/cmd/design/master_view.rs @@ -1,17 +1,17 @@ use super::{ Arc, Fill, NO_ID_SPACE, PhType, Result, Run, RunKind, Session, Shape, ShapeId, Size, Target, TextBody, Value, bad, bool_param, json, - master_index, ok, opt_usize_param, rect_param, str_param, usize_param, + master_index, ok, opt_usize_param, rect_param, str_param, }; pub(super) fn master_view(s: &mut Session, p: &Value) -> Result { let st = s.doc()?; - let mi = usize_param(p, "master").unwrap_or_else(|| master_index(&st.doc, &st.selection)); + let mi = opt_usize_param(p, "master", "view.slideMaster")?.unwrap_or_else(|| master_index(&st.doc, &st.selection)); // A master thumbnail sends `{master}` and means the master itself. `{master, layout}` selects // that layout. With neither key, follow the current slide's layout. let layout = if p.get("master").is_some() && p.get("layout").is_none() { None } else { - usize_param(p, "layout") + opt_usize_param(p, "layout", "view.slideMaster")? .or_else(|| st.current_slide().and_then(|sl| st.doc.masters.get(mi).and_then(|m| m.layouts.iter().position(|l| l.id == sl.layout)))) }; let target = match layout { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 68de1341..140a88de 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6458,3 +6458,14 @@ fn master_body_placeholder_format_keeps_per_level_formatting() { assert_eq!((level(&s, 0).run.size, level(&s, 1).run.size), (Some(28.0), Some(24.0))); assert_eq!(shown(&s), vec![Some(28.0), Some(24.0)]); } + +#[test] +fn slide_master_view_rejects_invalid_indices() { + let mut s = Session::with_new(); + for p in [json!({"master": -1}), json!({"master": 0.5}), json!({"master": 0, "layout": -1}), json!({"master": 0, "layout": 1.5})] { + let r = s.execute("view.slideMaster", &p); + assert!(r.is_err(), "invalid index accepted for {p}: {r:?}"); + } + assert!(s.execute("view.slideMaster", &json!({})).is_ok()); + assert!(s.execute("view.slideMaster", &json!({"master": 0})).is_ok()); +} From 77cd617596da9052fb833ed65401aa0afbd4f118 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:37:19 +0100 Subject: [PATCH 09/19] Use a short temp file name in write_file for long file names Found while testing crates/engine/src/io.rs:49 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/io.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7d21bb4b..38f4de8d 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -41,12 +41,13 @@ pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> { Ok(canonical) => canonical, Err(_) => std::path::PathBuf::from(path), }; - let target_str = target_path.to_str().unwrap_or(path); let existing_perms = std::fs::metadata(&target_path).ok().map(|m| m.permissions()); let mut attempts = 0; let (tmp, mut file) = loop { - let tmp = format!("{target_str}.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); + // A short fixed-size name: appending to the target's name could exceed the file name limit. + let tmp_name = format!(".deckcraft.{}.{}.saving", std::process::id(), COUNTER.fetch_add(1, Ordering::Relaxed)); + let tmp = target_path.with_file_name(tmp_name); match std::fs::OpenOptions::new().write(true).create_new(true).open(&tmp) { Ok(f) => break (tmp, f), Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists && attempts < 16 => attempts += 1, @@ -120,4 +121,18 @@ mod tests { let _ = std::fs::remove_file(&test_file); let _ = std::fs::remove_dir(&tmp_dir); } + + #[test] + fn write_file_overwrites_file_with_name_near_the_length_limit() { + let dir = std::env::temp_dir().join("deckcraft_io_long_name_test"); + let _ = std::fs::create_dir_all(&dir); + let file = dir.join(format!("{}.pptx", "a".repeat(250))); + std::fs::write(&file, b"old").unwrap(); + let path_str = file.to_str().unwrap(); + + write_file(path_str, b"new").unwrap(); + assert_eq!(read_file(path_str).unwrap(), b"new"); + + let _ = std::fs::remove_file(&file); + } } From 62ada3ab9a00f9fc5724ded9814745b69b3d80ec Mon Sep 17 00:00:00 2001 From: Leonardo Date: Mon, 2 Nov 2026 16:29:41 -0300 Subject: [PATCH 10/19] fix(render): tiled picture fills keep their size in scaled groups and upright tiles A tiled picture fill (a:blipFill with a:tile) was sized in the shape's own units. Inside a group whose child space differs from its box (old decks write child spaces ~1800 times smaller) one tile covered hundreds of boxes, so the fill showed a single flat texel instead of the texture. PowerPoint's own PNG export of a synthetic probe deck shows the tile keeps its size, offset, anchor and mirroring in slide points whatever the group's scale, uniform or stretched unevenly, nested or flipped. The tile is now converted from points to shape units per axis (Renderer::per_point_xy, paint:: tiled_per_point), and snapped to whole device pixels per axis. Tiles with rotWithShape="0" are drawn in device pixels and were sized in pixels, ignoring the zoom (even outside groups); they are now sized to pixels first. Tests: a tile in groups of seven scales (uniform and uneven, wide and tall tile), and an upright tile in a turned shape at zoom 1 and 2. Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 2 + crates/render/src/paint.rs | 31 ++++++++-- crates/render/src/renderer.rs | 21 +++++-- crates/render/src/tests_wild.rs | 100 ++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+), 8 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index ecf27f31..2b472291 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -148,6 +148,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped, and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | | 2026-10-31 | No Fill shape interiors allow selection and editing of underlying objects, even when a quick style remains; hit-testing uses the renderer’s fill resolution. Outlines, rotation, text targets and actual fills keep their hit behaviour (#1621) | | 2026-10-30 | Print-preview tests use the URL emitted by egui and its exact file path; previews are explicitly removed before the macOS process exits | | 2026-10-30 | `slide.actions` lists shape click and hover actions and text-run hyperlinks (including table cells and grouped shapes) for audit; listing never runs an action. The UI pane is a follow-up | @@ -319,6 +320,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-11-02 | trivial | Progress log: tiled picture fills keep their size in scaled groups. Scores and milestone names unchanged | | 2026-10-31 | trivial | No Fill hit-testing follows resolved fill instead of the presence of a style (#1621); scores unchanged | | 2026-10-30 | trivial | Progress log: read-only slide action inspection; parity scores unchanged | | 2026-10-29 | trivial | Progress log: going back in the slide show reverses the left slide's transition (#1531). Scores and milestone names unchanged | diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index ee572fd9..3c4b1db5 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -91,7 +91,10 @@ pub fn fill_path( let on_path = m * path.clone(); ctx.set_transform(Affine::IDENTITY); ctx.push_clip_layer(&on_path); - picture(ctx, pres, &resolved(rctx, pf, ph), on_slide, Affine::IDENTITY, None); + // Drawn in device pixels, so a tile (given in shape units) is sized to pixels first. + let tiled = tiled_per_point(fill, (c[0].hypot(c[1]), c[2].hypot(c[3]))); + let upright = if let Fill::Picture(p) = tiled.as_ref() { p } else { pf }; + picture(ctx, pres, &resolved(rctx, upright, ph), on_slide, Affine::IDENTITY, None); ctx.pop_layer(); return; } @@ -226,9 +229,12 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b } let (nw, nh) = images::natural_size(&item.data).unwrap_or((iw, ih)); // PowerPoint draws the tile at a whole number of output pixels. - let k = px_scale.max(1e-9); - let snap = |v: f64| (v * k).round().max(1.0) / k; - let (tile_w, tile_h) = (snap(nw * 0.75 * sx), snap(nh * 0.75 * sy)); + // Per axis: a group can stretch its child space unevenly, and a pixel is then a different + // number of shape units across and down. + let c = m.as_coeffs(); + let (kx, ky) = (c[0].hypot(c[1]).max(1e-9), c[2].hypot(c[3]).max(1e-9)); + let snap = |v: f64, k: f64| (v * k).round().max(1.0) / k; + let (tile_w, tile_h) = (snap(nw * 0.75 * sx, kx), snap(nh * 0.75 * sy, ky)); if !(tile_w > 1e-6 && tile_h > 1e-6 && tile_w.is_finite() && tile_h.is_finite()) { return; } @@ -260,6 +266,23 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b draw_image(ctx, &image, iw, ih, dest, m, alpha); } +/// `fill`, with a tiled picture's tile size and offset moved from points to shape units, given +/// the shape units to the point along x and y. A group's child coordinates scale its shapes' +/// boxes (a group can stretch them unevenly), but a tile keeps its size on the slide as PowerPoint +/// draws it, so in a group with a legacy child space the tile would otherwise cover hundreds of +/// boxes and the fill show one flat texel. Anything but a tiled picture, or a unit scale, is +/// returned as it is. +pub fn tiled_per_point(fill: &Fill, per_point: (f64, f64)) -> std::borrow::Cow<'_, Fill> { + let Fill::Picture(pf) = fill else { return std::borrow::Cow::Borrowed(fill) }; + let PictureMode::Tile { tx, ty, sx, sy, flip, align } = &pf.mode else { return std::borrow::Cow::Borrowed(fill) }; + let (kx, ky) = per_point; + if !(kx.is_finite() && ky.is_finite() && kx > 1e-12 && ky > 1e-12) || ((kx - 1.0).abs() < 1e-9 && (ky - 1.0).abs() < 1e-9) { + return std::borrow::Cow::Borrowed(fill); + } + let mode = PictureMode::Tile { tx: tx * kx, ty: ty * ky, sx: sx * kx, sy: sy * ky, flip: flip.clone(), align: align.clone() }; + std::borrow::Cow::Owned(Fill::Picture(PictureFill { mode, ..pf.clone() })) +} + /// Where the first tile of a tiled picture fill goes in `bounds`: its corner, edge middle or /// centre named by `align` (`tl`, `t`, `tr`, `l`, `ctr`, `r`, `bl`, `b`, `br`) on the same point /// of the box, then moved by the offset (points). diff --git a/crates/render/src/renderer.rs b/crates/render/src/renderer.rs index c25f6bab..916f508e 100644 --- a/crates/render/src/renderer.rs +++ b/crates/render/src/renderer.rs @@ -346,6 +346,14 @@ impl Renderer { if k.is_finite() && k > 1e-12 { 1.0 / k } else { 1.0 } } + /// [`Renderer::per_point`] along the shape's own x and y axes: a group can stretch its child + /// space unevenly, and a tiled fill keeps its size in points on the slide on both axes. + fn per_point_xy(&self, m: Affine, depth: usize) -> (f64, f64) { + let c = self.line_space(m, depth).1.as_coeffs(); + let inv = |s: f64| if s.is_finite() && s > 1e-12 { 1.0 / s } else { 1.0 }; + (inv(c[0].hypot(c[1])), inv(c[2].hypot(c[3]))) + } + /// The size and transform a shape's text is set in: inside a group whose child coordinates /// scale the shape, the shape's size in slide points and `m` with that scale taken out, so /// insets and font sizes stay in points; otherwise `w`, `h` and `m` themselves. @@ -541,6 +549,7 @@ impl Renderer { geo: &Geometry, (w, h): (f64, f64), m: Affine, + per_point: (f64, f64), fade: &dyn Fn(usize) -> deckcraft_geom::morph::Fade, ) { for (i, sp) in geo.paths.iter().enumerate() { @@ -566,7 +575,10 @@ impl Renderer { paint::fill_path(ctx, rctx, gf, None, &path, *gbox, *gm, sp.fill, fade(i).fill, f.pres); } } - _ => paint::fill_path(ctx, rctx, fl, fill_ph, &sp.path, Rect::new(0.0, 0.0, w, h), m, sp.fill, fade(i).fill, f.pres), + _ => { + let fl = paint::tiled_per_point(fl, per_point); + paint::fill_path(ctx, rctx, &fl, fill_ph, &sp.path, Rect::new(0.0, 0.0, w, h), m, sp.fill, fade(i).fill, f.pres) + } } ctx.set_fill_rule(peniko::Fill::NonZero); } @@ -736,6 +748,7 @@ impl Renderer { let fill = &d.fill; let outline = &d.outline; let fade = |i: usize| d.fade(i); + let pt_xy = self.per_point_xy(m, d.depth); // Fill. match &s.kind { ShapeKind::Picture { fill: pf } => { @@ -744,7 +757,7 @@ impl Renderer { if s.fill.as_ref().is_some_and(|f| !f.is_none()) && let Some(fl) = &fill { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } ctx.set_transform(m); let bounds = Rect::new(0.0, 0.0, w, h); @@ -761,7 +774,7 @@ impl Renderer { if s.fill.as_ref().is_some_and(|f| !f.is_none()) && let Some(fl) = &fill { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } ctx.set_transform(m); let pf = deckcraft_model::style::PictureFill { media: *media, ..Default::default() }; @@ -787,7 +800,7 @@ impl Renderer { if let Some(fl) = &fill && !(empty_ph && f.opts.edit && s.fill.is_none() && s.line.is_none() && s.style.is_none()) { - self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, &fade); + self.fill_geometry(ctx, f, rctx, fl, fill_ph, geo, (w, h), m, pt_xy, &fade); } } } diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index ae78f64b..96a546f0 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -454,6 +454,106 @@ fn tiled_pictures_flip_align_and_skip_non_positive_scales() { assert!(close(img.pixel(115, 115), WHITE, 5) && close(img.pixel(300, 150), WHITE, 5)); } +#[allow(clippy::unwrap_used)] +fn red_over_blue() -> Vec { + let img = image::RgbImage::from_fn(4, 8, |_, y| if y < 4 { image::Rgb([255, 0, 0]) } else { image::Rgb([0, 0, 255]) }); + let mut out = std::io::Cursor::new(vec![]); + image::DynamicImage::ImageRgb8(img).write_to(&mut out, image::ImageFormat::Png).unwrap(); + out.into_inner() +} + +#[test] +fn a_tile_keeps_its_size_in_a_group_with_scaled_child_coordinates() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // A group's child space scales its shapes' boxes, not the tile: the same tiles show whether + // the group's child space is 1600 times smaller (old decks write them so), 4 times larger, + // the slide's own or stretched unevenly (PowerPoint draws all of them alike, measured on its + // own export of a probe deck). Before, one tile covered hundreds of boxes and the fill was one + // flat texel, and an uneven group stretched the tile. + // Wide: 8x4 px = 6x3 pt, ten times = 60x30 pt tiles, red then blue across. + // Tall: 4x8 px = 3x6 pt, ten times = 30x60 pt tiles, red over blue down. + let blue = Rgba::rgb(0, 0, 255); + let scales = [(1.0, 1.0), (1600.0, 1600.0), (0.25, 0.25), (2.0, 0.5), (0.5, 2.0), (1800.0, 1.0), (1.0, 1800.0)]; + for (kx, ky) in scales { + for tall in [false, true] { + let mut p = Presentation::default(); + let png = if tall { red_over_blue() } else { half_red_half_blue() }; + let media = p.add_media("t.png", "image/png", png).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let member = Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, 240.0 / kx, 60.0 / ky)), + fill: Some(Fill::Picture(PictureFill { media, mode, ..Default::default() })), + line: no_line(), + ..Default::default() + }; + let group = Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 60.0)), + kind: ShapeKind::Group { children: vec![member], child: Xfrm::new(0.0, 0.0, 240.0 / kx, 60.0 / ky) }, + ..Default::default() + }; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![group]; + let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); + // Along the tile's long side: first half red, second blue, then red again. + let (a, b, c) = if tall { ((115, 115), (115, 145), (115, 175)) } else { ((115, 115), (145, 115), (175, 115)) }; + let (pa, pb, pc) = (img.pixel(a.0, a.1), img.pixel(b.0, b.1), img.pixel(c.0, c.1)); + assert!( + close(pa, RED, 30) && close(pb, blue, 30) && (tall || close(pc, RED, 30)) && (!tall || close(pc, WHITE, 5) || close(pc, RED, 30)), + "child space {kx} x {ky}, tall={tall}: {pa:?} {pb:?} {pc:?}" + ); + } + } +} + +#[test] +fn an_upright_tile_in_a_turned_shape_keeps_its_size_in_points_at_any_zoom() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // `rotWithShape="0"` draws the tiles on the slide's axes, in device pixels: they were sized in + // pixels, so zooming (or a group's child scale) changed their size on the slide. PowerPoint's + // export of the same shapes shows 60x30 pt tiles at every zoom. + let draw = |scale: f64, group: f64| { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let fill = Fill::Picture(PictureFill { media, mode, upright: true, ..Default::default() }); + let shape = Shape { + id: ShapeId(22), + xfrm: Some(Xfrm { rot: 20.0, ..Xfrm::new(0.0, 0.0, 240.0 / group, 120.0 / group) }), + fill: Some(fill), + line: no_line(), + ..Default::default() + }; + let shape = if group == 1.0 { + Shape { xfrm: Some(Xfrm { rot: 20.0, ..Xfrm::new(100.0, 100.0, 240.0, 120.0) }), ..shape } + } else { + Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 120.0)), + kind: ShapeKind::Group { children: vec![shape], child: Xfrm::new(0.0, 0.0, 240.0 / group, 120.0 / group) }, + ..Default::default() + } + }; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![shape]; + render_slide(&p, 0, &RenderOpts { scale, ..Default::default() }) + }; + for group in [1.0, 0.25] { + let (one, two) = (draw(1.0, group), draw(2.0, group)); + let (mut same, mut total) = (0, 0); + for y in (120..220).step_by(7) { + for x in (120..320).step_by(9) { + let (a, b) = (one.pixel(x, y), two.pixel(2 * x, 2 * y)); + if close(a, WHITE, 5) && close(b, WHITE, 5) { + continue; // outside the turned rectangle + } + total += 1; + same += usize::from(close(a, Rgba::rgb(b[0], b[1], b[2]), 40)); + } + } + assert!(total > 40 && same * 10 >= total * 9, "group scale {group}: {same} of {total} samples agree between zoom 1 and 2"); + } +} + #[test] fn a_tiles_first_copy_sits_on_the_point_its_alignment_names() { let b = Rect::new(0.0, 0.0, 100.0, 50.0); From 5cb6ba8403a0d1598c59c83e9fd5c83a227a29ee Mon Sep 17 00:00:00 2001 From: Luis <43929895+Luis-0508@users.noreply.github.com> Date: Mon, 2 Nov 2026 21:03:34 +0100 Subject: [PATCH 11/19] Comments: the pane keeps its width and resizes again (fixes #1431) A comment card's reply row reserved a fixed 70 px for its Resolve and delete buttons, which need more. The row overflowed the card, the pane grew by the overflow every frame until it stuck at its widest, and dragging its edge no longer changed it. The buttons are now laid out first, from the right, and the reply field takes the width they leave. --- crates/ui-egui/src/panels/comments.rs | 47 +++++++++++++++------------ crates/ui-egui/src/tests.rs | 44 +++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 21 deletions(-) diff --git a/crates/ui-egui/src/panels/comments.rs b/crates/ui-egui/src/panels/comments.rs index edf0e8b8..5acb7140 100644 --- a/crates/ui-egui/src/panels/comments.rs +++ b/crates/ui-egui/src/panels/comments.rs @@ -130,29 +130,34 @@ fn comment_card(app: &mut SlideApp, ui: &mut Ui, slide: usize, index: usize, c: for r in &c.replies { ui.label(egui::RichText::new(format!("↳ {}: {}", r.author, r.text)).color(t.text_dim)); } + // The buttons are laid out first, from the right, so the reply field takes exactly the + // width they leave. A fixed allowance overflowed the card and widened the pane every + // frame until it stuck at its widest (#1431). ui.horizontal(|ui| { - let id = ui.scope_id().with(("reply", slide, index)); - let mut txt: String = ui.data_mut(|d| d.get_temp(id).unwrap_or_default()); - let r = ui.add(egui::TextEdit::singleline(&mut txt).hint_text("Reply…").desired_width(ui.available_width() - 70.0)); - if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) && !txt.trim().is_empty() { - focus_comment(app, slide, index); - run(app, "comment.reply", json!({"index": index, "text": txt})); - txt.clear(); - } - ui.data_mut(|d| d.insert_temp(id, txt)); - if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { - focus_comment(app, slide, index); - run(app, "comment.resolve", json!({"index": index, "resolved": !c.resolved})); - } - if ui.small_button("🗑").clicked() { - let _ = app.run("slide.go", json!({"index": slide})); - let _ = app.run("comment.delete", json!({"index": index})); - match app.ui.comment_focus { - Some((fs, fi)) if fs == slide && fi == index => app.ui.comment_focus = None, - Some((fs, fi)) if fs == slide && fi > index => app.ui.comment_focus = Some((fs, fi - 1)), - _ => {} + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + if ui.small_button("🗑").clicked() { + let _ = app.run("slide.go", json!({"index": slide})); + let _ = app.run("comment.delete", json!({"index": index})); + match app.ui.comment_focus { + Some((fs, fi)) if fs == slide && fi == index => app.ui.comment_focus = None, + Some((fs, fi)) if fs == slide && fi > index => app.ui.comment_focus = Some((fs, fi - 1)), + _ => {} + } } - } + if ui.small_button(if c.resolved { "Reopen" } else { "Resolve" }).clicked() { + focus_comment(app, slide, index); + run(app, "comment.resolve", json!({"index": index, "resolved": !c.resolved})); + } + let id = ui.scope_id().with(("reply", slide, index)); + let mut txt: String = ui.data_mut(|d| d.get_temp(id).unwrap_or_default()); + let r = ui.add(egui::TextEdit::singleline(&mut txt).hint_text("Reply…").desired_width(ui.available_width())); + if r.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)) && !txt.trim().is_empty() { + focus_comment(app, slide, index); + run(app, "comment.reply", json!({"index": index, "text": txt})); + txt.clear(); + } + ui.data_mut(|d| d.insert_temp(id, txt)); + }); }); }); if focused { diff --git a/crates/ui-egui/src/tests.rs b/crates/ui-egui/src/tests.rs index 29fa755f..5221544a 100644 --- a/crates/ui-egui/src/tests.rs +++ b/crates/ui-egui/src/tests.rs @@ -2361,3 +2361,47 @@ fn overdue_visible_logic_does_not_reanchor_a_later_hidden_show() { let _ = ctx.run_logic(&raw_at(12.0, true, false), |ctx| app.tick_hidden_show(ctx, visible_anchor + std::time::Duration::from_secs(3))); assert_eq!(app.show.as_ref().unwrap().state.slide, 1, "time from the full visible frame is preserved through the logic-only pass"); } + +/// #1431: a comment card's reply row was wider than the pane, so the Comments pane grew every +/// frame until it stuck at its widest and dragging its edge did nothing. +#[test] +fn comments_pane_keeps_its_width_and_resizes() { + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SlideApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SlideApp::new(Session::with_new(), Services::default()), + ); + h.run_steps(3); + h.state_mut().run("comment.add", json!({"text": "Check the figures on this slide"})).unwrap(); + h.state_mut().run("comment.add", json!({"text": "Resolved one"})).unwrap(); + h.state_mut().run("comment.resolve", json!({"index": 1, "resolved": true})).unwrap(); + h.state_mut().run("view.pane", json!({"pane": "comments"})).unwrap(); + h.run_steps(30); + let width = h.state().ui.pane_width; + assert!(width < 320.0, "the pane keeps its default width, not {width}"); + // The cards stay compact: the second one starts near the top, not below a stretched first. + let second = h.get_by_label("Resolved one").rect(); + assert!(second.top() < 450.0, "the second comment sits near the top, at {second:?}"); + + // Drag the pane's left edge: right narrows it, left widens it. + let drag = |h: &mut egui_kittest::Harness<'_, SlideApp>, dx: f32| { + let start = egui::pos2(1200.0 - h.state().ui.pane_width - 1.0, 500.0); + let end = start + egui::vec2(dx, 0.0); + h.event(egui::Event::PointerMoved(start)); + h.run_steps(1); + h.event(egui::Event::PointerButton { pos: start, button: egui::PointerButton::Primary, pressed: true, modifiers: egui::Modifiers::NONE }); + for i in 1..=6 { + h.event(egui::Event::PointerMoved(start + (end - start) * (i as f32 / 6.0))); + h.run_steps(1); + } + h.event(egui::Event::PointerButton { pos: end, button: egui::PointerButton::Primary, pressed: false, modifiers: egui::Modifiers::NONE }); + h.run_steps(10); + h.state().ui.pane_width + }; + let narrow = drag(&mut h, 50.0); + assert!(narrow < width - 40.0, "dragging right narrows the pane: {width} -> {narrow}"); + let wide = drag(&mut h, -150.0); + assert!(wide > narrow + 140.0, "dragging left widens the pane: {narrow} -> {wide}"); +} From 9e59b5f2238f386a44c0324e3dbb8690d19c7bd3 Mon Sep 17 00:00:00 2001 From: Leonardo Date: Mon, 2 Nov 2026 17:31:10 -0300 Subject: [PATCH 12/19] fix(render): size a group's own tile and a picture shape's own tile in points too Review of #1693 found two more places that drew a tiled picture fill in the shape's own (child-space) units inside a scaled group. A group nested in a scaled group stored its own tiled fill unconverted, so the members that take it (grpFill) got oversized tiles. The owning group now converts the fill with per_point_xy when it stores it, so inheriting members, directly or through a chain of grpFill groups, get the converted tile. A picture shape (p:pic) whose blip fill is a tile passed its own fill straight to paint::picture. It now goes through paint::tiled_picture_per_point, which tiled_per_point also uses. Tests: a group's tile passed to its members and a picture shape's own tile, each in outer groups scaled 1, 0.25, 1600 and 2x0.5; both fail without the change. Docs: the Picture and texture fill checklist row is re-assessed (remains P) with its status line and revision history, and the ROADMAP progress log row mentions both cases. Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 2 +- crates/render/src/paint.rs | 14 ++++++-- crates/render/src/renderer.rs | 6 ++-- crates/render/src/tests_wild.rs | 62 +++++++++++++++++++++++++++++++++ docs/parity-checklist.md | 5 ++- 5 files changed, 82 insertions(+), 7 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 2b472291..3a1889a8 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -148,7 +148,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| -| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped, and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | +| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped (also a group's own tile fill passed to its members, and a picture shape's own tile), and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | | 2026-10-31 | No Fill shape interiors allow selection and editing of underlying objects, even when a quick style remains; hit-testing uses the renderer’s fill resolution. Outlines, rotation, text targets and actual fills keep their hit behaviour (#1621) | | 2026-10-30 | Print-preview tests use the URL emitted by egui and its exact file path; previews are explicitly removed before the macOS process exits | | 2026-10-30 | `slide.actions` lists shape click and hover actions and text-run hyperlinks (including table cells and grouped shapes) for audit; listing never runs an action. The UI pane is a follow-up | diff --git a/crates/render/src/paint.rs b/crates/render/src/paint.rs index 3c4b1db5..74d11123 100644 --- a/crates/render/src/paint.rs +++ b/crates/render/src/paint.rs @@ -274,13 +274,21 @@ pub fn picture(ctx: &mut RenderContext, pres: &Presentation, pf: &PictureFill, b /// returned as it is. pub fn tiled_per_point(fill: &Fill, per_point: (f64, f64)) -> std::borrow::Cow<'_, Fill> { let Fill::Picture(pf) = fill else { return std::borrow::Cow::Borrowed(fill) }; - let PictureMode::Tile { tx, ty, sx, sy, flip, align } = &pf.mode else { return std::borrow::Cow::Borrowed(fill) }; + match tiled_picture_per_point(pf, per_point) { + std::borrow::Cow::Borrowed(_) => std::borrow::Cow::Borrowed(fill), + std::borrow::Cow::Owned(p) => std::borrow::Cow::Owned(Fill::Picture(p)), + } +} + +/// [`tiled_per_point`] for a picture fill itself (a picture shape's own fill is not a [`Fill`]). +pub fn tiled_picture_per_point(pf: &PictureFill, per_point: (f64, f64)) -> std::borrow::Cow<'_, PictureFill> { + let PictureMode::Tile { tx, ty, sx, sy, flip, align } = &pf.mode else { return std::borrow::Cow::Borrowed(pf) }; let (kx, ky) = per_point; if !(kx.is_finite() && ky.is_finite() && kx > 1e-12 && ky > 1e-12) || ((kx - 1.0).abs() < 1e-9 && (ky - 1.0).abs() < 1e-9) { - return std::borrow::Cow::Borrowed(fill); + return std::borrow::Cow::Borrowed(pf); } let mode = PictureMode::Tile { tx: tx * kx, ty: ty * ky, sx: sx * kx, sy: sy * ky, flip: flip.clone(), align: align.clone() }; - std::borrow::Cow::Owned(Fill::Picture(PictureFill { mode, ..pf.clone() })) + std::borrow::Cow::Owned(PictureFill { mode, ..pf.clone() }) } /// Where the first tile of a tiled picture fill goes in `bounds`: its corner, edge middle or diff --git a/crates/render/src/renderer.rs b/crates/render/src/renderer.rs index 916f508e..699a938f 100644 --- a/crates/render/src/renderer.rs +++ b/crates/render/src/renderer.rs @@ -436,7 +436,8 @@ impl Renderer { // A group's own fill (a group filled with its parent's passes that on). let own = match &s.fill { Some(Fill::Group) | None => self.group_fill.clone(), - Some(fl) => Some((fl.clone(), m, Rect::new(0.0, 0.0, x.w, x.h))), + // A tile is given in points; this group's own space may be scaled by its parents. + Some(fl) => Some((paint::tiled_per_point(fl, self.per_point_xy(m, depth)).into_owned(), m, Rect::new(0.0, 0.0, x.w, x.h))), }; let outer = std::mem::replace(&mut self.group_fill, own); // A group's outer shadow is cast by everything in it, as PowerPoint draws it: the @@ -763,7 +764,8 @@ impl Renderer { let bounds = Rect::new(0.0, 0.0, w, h); ctx.push_clip_layer(outline); let t = f.opts.gif_times.iter().find(|g| g.0 == s.id).map(|g| g.1); - paint::picture(ctx, f.pres, &paint::resolved(rctx, pf, None), bounds, m, t); + let pf = paint::tiled_picture_per_point(pf, pt_xy); + paint::picture(ctx, f.pres, &paint::resolved(rctx, &pf, None), bounds, m, t); ctx.pop_layer(); } // An object DeckCraft doesn't model (an embedded OLE object, an unknown graphic): its diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index 96a546f0..c81e481c 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -506,6 +506,68 @@ fn a_tile_keeps_its_size_in_a_group_with_scaled_child_coordinates() { } } +/// A slide with `shape` (built for a box of `w` x `h` child units) in a group at (100, 100) that is +/// 240 x 60 pt on the slide and scaled `kx` x `ky` against its child space, drawn at 1 px per point. +fn in_a_scaled_group(kx: f64, ky: f64, p: Presentation, shape: impl FnOnce(f64, f64) -> Shape) -> Image { + let (w, h) = (240.0 / kx, 60.0 / ky); + let group = Shape { + id: ShapeId(21), + xfrm: Some(Xfrm::new(100.0, 100.0, 240.0, 60.0)), + kind: ShapeKind::Group { children: vec![shape(w, h)], child: Xfrm::new(0.0, 0.0, w, h) }, + ..Default::default() + }; + let mut p = p; + std::sync::Arc::make_mut(&mut p.slides[0]).shapes = vec![group]; + render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }) +} + +#[test] +fn a_tile_a_group_passes_to_its_members_keeps_its_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // The group that owns the tiled fill sits inside another group whose child space is scaled: the + // members that take it (`grpFill`) get tiles of 60 x 30 pt, as with no outer group. + let blue = Rgba::rgb(0, 0, 255); + for (kx, ky) in [(1.0, 1.0), (0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let img = in_a_scaled_group(kx, ky, p, |w, h| { + let member = + Shape { id: ShapeId(23), xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), fill: Some(Fill::Group), line: no_line(), ..Default::default() }; + Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + fill: Some(Fill::Picture(PictureFill { media, mode, ..Default::default() })), + kind: ShapeKind::Group { children: vec![member], child: Xfrm::new(0.0, 0.0, w, h) }, + ..Default::default() + } + }); + let (a, b, c) = (img.pixel(115, 115), img.pixel(145, 115), img.pixel(175, 115)); + assert!(close(a, RED, 30) && close(b, blue, 30) && close(c, RED, 30), "outer group {kx} x {ky}: {a:?} {b:?} {c:?}"); + } +} + +#[test] +fn a_picture_shapes_own_tile_keeps_its_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode}; + // A picture (`p:pic`) whose blip fill is a tile rather than a stretch. + let blue = Rgba::rgb(0, 0, 255); + for (kx, ky) in [(1.0, 1.0), (0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let img = in_a_scaled_group(kx, ky, p, |w, h| Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + kind: ShapeKind::Picture { fill: PictureFill { media, mode, ..Default::default() } }, + line: no_line(), + ..Default::default() + }); + let (a, b, c) = (img.pixel(115, 115), img.pixel(145, 115), img.pixel(175, 115)); + assert!(close(a, RED, 30) && close(b, blue, 30) && close(c, RED, 30), "group {kx} x {ky}: {a:?} {b:?} {c:?}"); + } +} + #[test] fn an_upright_tile_in_a_turned_shape_keeps_its_size_in_points_at_any_zoom() { use deckcraft_model::style::{PictureFill, PictureMode}; diff --git a/docs/parity-checklist.md b/docs/parity-checklist.md index 606f1228..8af3745e 100644 --- a/docs/parity-checklist.md +++ b/docs/parity-checklist.md @@ -2,7 +2,9 @@ > **Generated:** 2026-10-14 (bullets/numbering remain D after empty-line marker review; summary by `cargo xtask parity`; rows re-scored by the 2026-10-14 community PR wave, duplicate Thesaurus, AutoCorrect, Artistic effects and Corrections rows folded; Preview in editor re-scored 2026-10-13; earlier rows re-scored 2026-10-10 against PowerPoint for Mac 16.113.4; video/animated GIF row re-scored 2026-10-11, #171) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) -> **Last updated:** 2026-11-01 · Locked-shape and no-outline regressions added for #1621; fill row re-assessed: remains D; No Fill hit-testing now follows resolved appearance. Scores unchanged. +> **Last updated:** 2026-11-02 · Tiled picture fills keep their size in scaled groups and upright tiles (#1693); Picture and texture fill row re-assessed: remains P, since only how tiles are sized changed. Scores unchanged. +> +> **Previously (2026-11-01):** Locked-shape and no-outline regressions added for #1621; fill row re-assessed: remains D; No Fill hit-testing now follows resolved appearance. Scores unchanged. The summary between ` **Weighted breadth parity: 84%** over 191 features. @@ -240,4 +242,5 @@ Open P0 items: Contextual tabs (Shape Format, Picture Format, Table Design/Layou | Date | Change | Summary | |---|---|---| +| 2026-11-02 | trivial | Tiled picture fills keep their size in scaled groups and upright tiles (#1693); Picture and texture fill row remains P and summary scores are unchanged | | 2026-10-31 | trivial | No Fill interior hit-testing follows resolved fill (#1621); fill row remains D and summary scores are unchanged | From e90e7f6c03afbf7a3694e88521aff6be5df53ba0 Mon Sep 17 00:00:00 2001 From: Leonardo Date: Mon, 2 Nov 2026 17:46:33 -0300 Subject: [PATCH 13/19] fix(render): size the tile of a picture's shadow and reflection in points too Review of #1693 found that cast_effects built the picture content that a shadow, a glow and a reflection are drawn from without converting a tiled fill to points, so inside a scaled group the reflection of a tiled picture showed child-space tiles while its face showed the right ones. The content now goes through paint::tiled_picture_per_point with per_point_xy, as the face does. Test: a tiled picture's reflection in groups scaled 0.25, 1600 and 2x0.5 shows the same colours as in an unscaled group; it fails without the change. Co-Authored-By: Claude Sonnet 5.5 --- ROADMAP.md | 2 +- crates/render/src/renderer.rs | 8 ++++++-- crates/render/src/tests_wild.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 3a1889a8..050c3958 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -148,7 +148,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| -| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped (also a group's own tile fill passed to its members, and a picture shape's own tile), and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | +| 2026-11-02 | Tiled picture fills in groups: a tile (`a:blipFill` with `a:tile`) keeps its size, offset and anchor in slide points whatever the group's child-coordinate scale, uniform or stretched unevenly, nested or flipped (also a group's own tile fill passed to its members, and a picture shape's own tile, with its shadow and reflection), and tiles with `rotWithShape="0"` keep their size in points at any zoom, as PowerPoint draws them (measured on its own PNG export of a synthetic probe deck: every grouped case within ~3/255 of the ungrouped control's own difference from PowerPoint, which is ~13/255 from resampling). Before, a group whose child space was ~1800 times smaller than its box showed one flat texel of the texture (a Venn diagram with paper-textured circles and a hatched intersection) | | 2026-10-31 | No Fill shape interiors allow selection and editing of underlying objects, even when a quick style remains; hit-testing uses the renderer’s fill resolution. Outlines, rotation, text targets and actual fills keep their hit behaviour (#1621) | | 2026-10-30 | Print-preview tests use the URL emitted by egui and its exact file path; previews are explicitly removed before the macOS process exits | | 2026-10-30 | `slide.actions` lists shape click and hover actions and text-run hyperlinks (including table cells and grouped shapes) for audit; listing never runs an action. The UI pane is a follow-up | diff --git a/crates/render/src/renderer.rs b/crates/render/src/renderer.rs index 699a938f..e7cd5ff3 100644 --- a/crates/render/src/renderer.rs +++ b/crates/render/src/renderer.rs @@ -668,9 +668,13 @@ impl Renderer { let line = &d.line; // Effects below the shape. if let Some(e) = &effects { - // A picture (or picture fill) casts its shadow from its pixels, not its frame. + // A picture (or picture fill) casts its shadow from its pixels, not its frame. A tile + // is sized in points on the slide, as the face's is, and so is its shadow and reflection. let content = match (&s.kind, &fill) { - (ShapeKind::Picture { fill: pf }, _) | (_, Some(Fill::Picture(pf))) => Some(paint::resolved(rctx, pf, None).into_owned()), + (ShapeKind::Picture { fill: pf }, _) | (_, Some(Fill::Picture(pf))) => { + let pf = paint::tiled_picture_per_point(pf, self.per_point_xy(m, depth)); + Some(paint::resolved(rctx, &pf, None).into_owned()) + } _ => None, }; // What casts the shadow: the fill as see-through as it is, and the outline. diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index c81e481c..987781f1 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -568,6 +568,36 @@ fn a_picture_shapes_own_tile_keeps_its_size_inside_a_scaled_group() { } } +#[test] +fn a_tiled_pictures_reflection_keeps_the_tile_size_inside_a_scaled_group() { + use deckcraft_model::style::{PictureFill, PictureMode, Reflection}; + // The reflection mirrors the picture's pixels, so its tiles must be the face's, 60 x 30 pt, + // whatever the group's child scale: just below the box it shows the same colours. + let reflected = |kx: f64, ky: f64| { + let mut p = Presentation::default(); + let media = p.add_media("t.png", "image/png", half_red_half_blue()).expect("media"); + let mode = PictureMode::Tile { tx: 0.0, ty: 0.0, sx: 10.0, sy: 10.0, flip: "none".into(), align: "tl".into() }; + let rf = Reflection { blur: 0.0, start_alpha: 1.0, end_alpha: 0.0, end_pos: 0.5, dist: 0.0 }; + let img = in_a_scaled_group(kx, ky, p, |w, h| Shape { + id: ShapeId(22), + xfrm: Some(Xfrm::new(0.0, 0.0, w, h)), + kind: ShapeKind::Picture { fill: PictureFill { media, mode, ..Default::default() } }, + line: no_line(), + effects: Some(deckcraft_model::Effects { reflection: Some(rf), ..Default::default() }), + ..Default::default() + }); + [img.pixel(115, 163), img.pixel(145, 163), img.pixel(175, 163)] + }; + let plain = reflected(1.0, 1.0); + assert!(plain[0][0] > plain[0][2] && plain[1][2] > plain[1][0] && plain[2][0] > plain[2][2], "red, blue, red in the reflection: {plain:?}"); + for (kx, ky) in [(0.25, 0.25), (1600.0, 1600.0), (2.0, 0.5)] { + let got = reflected(kx, ky); + for (g, e) in got.iter().zip(&plain) { + assert!(close(*g, Rgba::rgb(e[0], e[1], e[2]), 40), "child scale {kx} x {ky}: {got:?} against {plain:?}"); + } + } +} + #[test] fn an_upright_tile_in_a_turned_shape_keeps_its_size_in_points_at_any_zoom() { use deckcraft_model::style::{PictureFill, PictureMode}; From f2803102c1abb1cc65681f7a21522b3ba7a125a9 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:41:17 -0600 Subject: [PATCH 14/19] Correct animation option labels --- crates/ui-egui/src/ribbon/animations.rs | 37 +++++++++++++++++++------ 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/ribbon/animations.rs b/crates/ui-egui/src/ribbon/animations.rs index 6d35afdc..d4380160 100644 --- a/crates/ui-egui/src/ribbon/animations.rs +++ b/crates/ui-egui/src/ribbon/animations.rs @@ -228,9 +228,12 @@ fn animation_option_label(effect: &str, option: &str) -> String { "rb" => Some("From Bottom-Right"), _ => None, }; - if matches!(effect, "fly" | "flyOut" | "wipe" | "wipeOut" | "peek" | "peekOut") + if matches!(effect, "fly" | "flyOut" | "wipe" | "wipeOut" | "peek" | "peekOut" | "crawl" | "crawlOut") && let Some(label) = direction { + if matches!(effect, "flyOut" | "peekOut" | "crawlOut") { + return label.replacen("From", "To", 1); + } return label.into(); } @@ -245,14 +248,14 @@ fn animation_option_label(effect: &str, option: &str) -> String { ("shape" | "shapeOut", o) if o.ends_with("Out") => format!("{} Out", title_first(shape_option_name(o))), ("wheel" | "wheelOut", "1") => "1 Spoke".into(), ("wheel" | "wheelOut", n) => format!("{n} Spokes"), - ("box" | "boxOut" | "circle" | "diamond", "in") => format!("{} In", title_first(effect.trim_end_matches("Out"))), - ("box" | "boxOut" | "circle" | "diamond", "out") => format!("{} Out", title_first(effect.trim_end_matches("Out"))), + ("box" | "boxOut" | "circle" | "diamond" | "diamondOut", "in") => format!("{} In", title_first(effect.trim_end_matches("Out"))), + ("box" | "boxOut" | "circle" | "diamond" | "diamondOut", "out") => format!("{} Out", title_first(effect.trim_end_matches("Out"))), ("zoom" | "zoomOut", "in") => "Zoom In".into(), ("zoom" | "zoomOut", "out") => "Zoom Out".into(), - (_, "lu") => "From Top-Left".into(), - (_, "ru") => "From Top-Right".into(), - (_, "ld") => "From Bottom-Left".into(), - (_, "rd") => "From Bottom-Right".into(), + ("strips" | "stripsOut", "lu") => "Left Up".into(), + ("strips" | "stripsOut", "ru") => "Right Up".into(), + ("strips" | "stripsOut", "ld") => "Left Down".into(), + ("strips" | "stripsOut", "rd") => "Right Down".into(), (_, "horz") => "Horizontal".into(), (_, "vert") => "Vertical".into(), (_, "across") => "Across".into(), @@ -301,11 +304,29 @@ mod animation_option_label_tests { } } assert_eq!(animation_option_label("fly", "lb"), "From Bottom-Left"); - assert_eq!(animation_option_label("flyOut", "rt"), "From Top-Right"); + assert_eq!(animation_option_label("flyOut", "rt"), "To Top-Right"); + assert_eq!(animation_option_label("flyOut", "b"), "To Bottom"); + assert_eq!(animation_option_label("peekOut", "l"), "To Left"); assert_eq!(animation_option_label("wipe", "b"), "From Bottom"); assert_eq!(animation_option_label("wheel", "1"), "1 Spoke"); assert_eq!(animation_option_label("split", "vertOut"), "Vertical Out"); assert_eq!(animation_option_label("shape", "circleIn"), "Circle In"); assert_eq!(animation_option_label("diamond", "out"), "Diamond Out"); + assert_eq!(animation_option_label("diamondOut", "in"), "Diamond In"); + assert_eq!(animation_option_label("diamondOut", "out"), "Diamond Out"); + assert_eq!(animation_option_label("strips", "lu"), "Left Up"); + assert_eq!(animation_option_label("strips", "ru"), "Right Up"); + assert_eq!(animation_option_label("strips", "ld"), "Left Down"); + assert_eq!(animation_option_label("strips", "rd"), "Right Down"); + assert_eq!(animation_option_label("crawl", "b"), "From Bottom"); + assert_eq!(animation_option_label("crawl", "l"), "From Left"); + assert_eq!(animation_option_label("crawl", "t"), "From Top"); + assert_eq!(animation_option_label("crawl", "r"), "From Right"); + assert_eq!(animation_option_label("crawlOut", "l"), "To Left"); + assert_eq!(animation_option_label("crawlOut", "r"), "To Right"); + assert_eq!(animation_option_label("plus", "in"), "In"); + assert_eq!(animation_option_label("plus", "out"), "Out"); + assert_eq!(animation_option_label("changeFontSize", "increase"), "Increase"); + assert_eq!(animation_option_label("changeFontSize", "decrease"), "Decrease"); } } From 3cfd4c5e6e14bcbda200cb962bc439878aec8d95 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Mon, 2 Nov 2026 15:47:58 -0600 Subject: [PATCH 15/19] Parse percentage-form bullet sizes --- crates/pptx/src/read/dml.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 37250201..300773a5 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -27,6 +27,12 @@ fn bullet_size_points(value: Option<&str>) -> Option { value.and_then(|v| v.trim().parse::().ok()).filter(|v| (100..=400_000).contains(v)).map(|v| v as f64 / 100.0) } +fn bullet_size_percent(value: Option<&str>) -> Option { + let value = value?.trim(); + let (value, scale) = if let Some(value) = value.strip_suffix('%') { (value.trim(), 100.0) } else { (value, 100_000.0) }; + value.parse::().ok().filter(|v| v.is_finite()).map(|v| v / scale) +} + /// 1/60 000 degree → degrees. pub fn ang(e: &El, name: &str) -> Option { e.f64(name).map(|v| v / 60_000.0) @@ -461,7 +467,7 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option "buSzPct" => { pp.bullet_size_points = None; pp.bullet_size_follow_text = None; - pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)); + pp.bullet_size = bullet_size_percent(e.attr("val")).map(|v| v.clamp(0.25, 4.0)); } "buSzPts" => { pp.bullet_size = None; @@ -716,6 +722,14 @@ mod tests { assert_eq!(bullet_size_points(Some("1400")), Some(14.0)); } + #[test] + fn bullet_percentage_size_accepts_numeric_and_percent_forms() { + assert_eq!(bullet_size_percent(Some("100000")), Some(1.0)); + assert_eq!(bullet_size_percent(Some("100%")), Some(1.0)); + assert_eq!(bullet_size_percent(Some(" 125% ")), Some(1.25)); + assert_eq!(bullet_size_percent(Some("invalid")), None); + } + #[test] fn miter_limit_accepts_both_percentage_forms_with_schema_bounds() { assert_eq!(parse_miter_limit("200000"), Some(200_000)); From 58c24883c7a1ae98b0829c4fc9af4b7a326b78c6 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 3 Nov 2026 01:14:15 +0100 Subject: [PATCH 16/19] Reject png/jpeg targets in file.save instead of writing zip bytes Found while testing crates/engine/src/cmd/file.rs:489 (save, with save_bytes at :341-349 and format_for_path at :440) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/file.rs | 1 + crates/engine/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 0d52e635..53009f51 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -345,6 +345,7 @@ pub fn save_bytes(doc: &Presentation, format: &str) -> Result> { "rtf" => Ok(deckcraft_format::rtf::slides_to_rtf(doc).into_bytes()), "pdf" => deckcraft_pdf::export(doc, &Default::default()).map_err(|e| EngineError::Other(e.to_string())), "gif" => gif_bytes(doc, &Value::Null), + "png" | "jpeg" => Err(EngineError::Other("saving a presentation as a single image isn't supported; use file.export".into())), _ => deckcraft_format::save(doc).map_err(|e| EngineError::Other(e.to_string())), } } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 68de1341..12b13e68 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6458,3 +6458,19 @@ fn master_body_placeholder_format_keeps_per_level_formatting() { assert_eq!((level(&s, 0).run.size, level(&s, 1).run.size), (Some(28.0), Some(24.0))); assert_eq!(shown(&s), vec![Some(28.0), Some(24.0)]); } + +#[test] +fn save_to_image_path_is_an_error_not_zip_bytes() { + let dir = std::env::temp_dir().join(format!("deckcraft-save-img-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + for (cmd, name) in [("file.save", "a.png"), ("file.saveAs", "a.jpg"), ("file.save", "a.jpeg")] { + let mut s = Session::new(); + sample::open_sample(&mut s).unwrap(); + let path = dir.join(name).to_string_lossy().to_string(); + let before = s.doc().unwrap().path.clone(); + assert!(s.execute(cmd, &json!({"path": path})).is_err(), "{cmd} {name} should fail"); + assert!(!std::path::Path::new(&path).exists(), "{name} must not be written"); + assert_eq!(s.doc().unwrap().path, before); + } + let _ = std::fs::remove_dir_all(&dir); +} From ba3b57c756d57f21874e9a3ed075cbd3ce195b52 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 2 Nov 2026 23:07:24 -0500 Subject: [PATCH 17/19] file.export: a perPage that isn't a count is refused, not read as 6 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 2 +- crates/engine/src/tests.rs | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index dc9f1a44..7857535c 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -360,7 +360,7 @@ pub fn pdf_options(p: &Value, count: usize) -> Result o.layout = match str_param(p, "layout").unwrap_or("slides") { "notes" => deckcraft_pdf::PageLayout::Notes, "handouts" => { - let per_page = usize_param(p, "perPage").unwrap_or(6); + let per_page = opt_usize_param(p, "perPage", "file.export")?.unwrap_or(6); if ![1, 2, 3, 4, 6, 9].contains(&per_page) { return Err(bad("file.export", format!("perPage {per_page} isn't one of 1, 2, 3, 4, 6, 9"))); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e8038c9a..1c8e7a8a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2421,6 +2421,10 @@ fn pdf_options_reject_unsupported_per_page() { let e = cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": n}), 1).unwrap_err(); assert!(e.to_string().contains("1, 2, 3, 4, 6, 9"), "perPage {n}: {e}"); } + // Not a count at all: refused too, instead of falling back to 6. + for v in [json!(-1), json!(5.5), json!("6")] { + assert!(cmd::file::pdf_options(&json!({"layout": "handouts", "perPage": v}), 1).is_err(), "perPage {v}"); + } } #[test] From fb3b857f53d3d8e371d0b06cc073d8751f7b6091 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 2 Nov 2026 23:07:29 -0500 Subject: [PATCH 18/19] file.save: an explicit jpg format is refused too Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 53009f51..06ed36e3 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -345,7 +345,7 @@ pub fn save_bytes(doc: &Presentation, format: &str) -> Result> { "rtf" => Ok(deckcraft_format::rtf::slides_to_rtf(doc).into_bytes()), "pdf" => deckcraft_pdf::export(doc, &Default::default()).map_err(|e| EngineError::Other(e.to_string())), "gif" => gif_bytes(doc, &Value::Null), - "png" | "jpeg" => Err(EngineError::Other("saving a presentation as a single image isn't supported; use file.export".into())), + "png" | "jpeg" | "jpg" => Err(EngineError::Other("saving a presentation as a single image isn't supported; use file.export".into())), _ => deckcraft_format::save(doc).map_err(|e| EngineError::Other(e.to_string())), } } From 97050a353b91a63c2800bcb186d90398b6cecf58 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Mon, 2 Nov 2026 23:07:34 -0500 Subject: [PATCH 19/19] Bullets: an own bullet size still clears an inherited follow-text flag (as #1589 did) Co-Authored-By: Claude Opus 5.5 --- crates/model/src/text.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 25c29074..ca992c38 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -288,7 +288,7 @@ impl ParaProps { self.bullet_color.clone_from(&p.bullet_color); } } - if self.bullet_size_follow_text == Some(false) { + if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() || self.bullet_size_points.is_some() { self.bullet_size_follow_text = None; } if self.bullet_size.is_none() && self.bullet_size_points.is_none() && self.bullet_size_follow_text.is_none() {