diff --git a/Cargo.lock b/Cargo.lock index 2e7f72a5f..9c60fb50a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1008,7 +1008,7 @@ checksum = "0c87e182de0887fd5361989c677c4e8f5000cd9491d6d563161a8f3a5519fc7f" [[package]] name = "deckcraft" -version = "0.14.1" +version = "0.15.0" dependencies = [ "arboard", "chrono", @@ -1030,7 +1030,7 @@ dependencies = [ [[package]] name = "deckcraft-anim" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-geom", "deckcraft-model", @@ -1039,7 +1039,7 @@ dependencies = [ [[package]] name = "deckcraft-av1" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1048,14 +1048,14 @@ dependencies = [ [[package]] name = "deckcraft-bitstream" -version = "0.14.1" +version = "0.15.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-cli" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-engine", "deckcraft-mcp", @@ -1065,14 +1065,14 @@ dependencies = [ [[package]] name = "deckcraft-color" -version = "0.14.1" +version = "0.15.0" dependencies = [ "serde", ] [[package]] name = "deckcraft-engine" -version = "0.14.1" +version = "0.15.0" dependencies = [ "chrono", "deckcraft-anim", @@ -1098,7 +1098,7 @@ dependencies = [ [[package]] name = "deckcraft-fonts" -version = "0.14.1" +version = "0.15.0" dependencies = [ "epaint_default_fonts", "harfrust 0.13.3", @@ -1110,7 +1110,7 @@ dependencies = [ [[package]] name = "deckcraft-format" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-model", "serde", @@ -1121,7 +1121,7 @@ dependencies = [ [[package]] name = "deckcraft-geom" -version = "0.14.1" +version = "0.15.0" dependencies = [ "kurbo", "serde", @@ -1129,7 +1129,7 @@ dependencies = [ [[package]] name = "deckcraft-h264" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1138,7 +1138,7 @@ dependencies = [ [[package]] name = "deckcraft-hevc" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1147,21 +1147,21 @@ dependencies = [ [[package]] name = "deckcraft-isobmff" -version = "0.14.1" +version = "0.15.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-matroska" -version = "0.14.1" +version = "0.15.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-mcp" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-engine", "deckcraft-render", @@ -1171,7 +1171,7 @@ dependencies = [ [[package]] name = "deckcraft-media" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-av1", "deckcraft-h264", @@ -1189,7 +1189,7 @@ dependencies = [ [[package]] name = "deckcraft-metafile" -version = "0.14.1" +version = "0.15.0" dependencies = [ "image", "kurbo", @@ -1198,7 +1198,7 @@ dependencies = [ [[package]] name = "deckcraft-model" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-color", "deckcraft-geom", @@ -1210,15 +1210,15 @@ dependencies = [ [[package]] name = "deckcraft-ogg" -version = "0.14.1" +version = "0.15.0" [[package]] name = "deckcraft-opus" -version = "0.14.1" +version = "0.15.0" [[package]] name = "deckcraft-pdf" -version = "0.14.1" +version = "0.15.0" dependencies = [ "craft-pdf", "deckcraft-fonts", @@ -1236,7 +1236,7 @@ dependencies = [ [[package]] name = "deckcraft-pptx" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-color", "deckcraft-engine", @@ -1252,7 +1252,7 @@ dependencies = [ [[package]] name = "deckcraft-render" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-anim", "deckcraft-color", @@ -1271,7 +1271,7 @@ dependencies = [ [[package]] name = "deckcraft-text" -version = "0.14.1" +version = "0.15.0" dependencies = [ "chrono", "deckcraft-color", @@ -1285,7 +1285,7 @@ dependencies = [ [[package]] name = "deckcraft-ui-egui" -version = "0.14.1" +version = "0.15.0" dependencies = [ "craft-ui", "craft-update", @@ -1310,7 +1310,7 @@ dependencies = [ [[package]] name = "deckcraft-vp9" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1319,7 +1319,7 @@ dependencies = [ [[package]] name = "deckcraft-web" -version = "0.14.1" +version = "0.15.0" dependencies = [ "deckcraft-engine", "deckcraft-ui-egui", @@ -6269,7 +6269,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee" [[package]] name = "xtask" -version = "0.14.1" +version = "0.15.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 7cd456fdf..ab2b74291 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/deckcraft", "apps/deckcraft-cli"] [workspace.package] -version = "0.14.1" +version = "0.15.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" diff --git a/ROADMAP.md b/ROADMAP.md index 42ccd19dc..1ad200c39 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,9 @@ **Stage: alpha** · next: beta, ~20 points and ~120–210 h away -> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-11-01 · **Change:** major (fix for a workflow-breaking regression: Ctrl+C/X/V use the system clipboard again, broken for all text and objects from 0.11.1 to 0.14.1; pasted and duplicated placeholders keep their look; stage and scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-02 · **Change:** trivial (File › About replaces Account; product links, credits and appearance retained; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-11-01 · **Change:** trivial (progress log: fix for a workflow-breaking regression: Ctrl+C/X/V use the system clipboard again, broken for all text and objects from 0.11.1 to 0.15.0; pasted and duplicated placeholders keep their look; stage and scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) +> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-11-01 · **Change:** trivial (locked No Fill and no-outline click-through regression coverage added; No Fill hit-testing follows resolved fill instead of the presence of a style (#1621); scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-30 · **Change:** trivial (progress log: read-only `slide.actions` lists shape and table text actions; parity 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) @@ -148,9 +150,12 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-11-02 | File › About replaces the unused Account page, with version, project links, contributor/model credits and appearance preferences | +| 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-11-01 | Ctrl+V of a picture copied in a browser (Copy Image) pastes the picture, not a text box with its link and alt text; the link still pastes as text when no picture came with it | | 2026-11-01 | Copy, Cut, Paste and Duplicate of a title, body or other placeholder on a slide keep its look: the copy is a regular shape that takes the size, font, bullets, indents, autofit and box its layout and master gave it, instead of 18 pt unbulleted text | | 2026-11-01 | Clipboard: Ctrl+C, Ctrl+X and Ctrl+V use the system clipboard again, so text and pictures paste from other apps and in-app copies paste back. Since the egui 0.37 migration (#1350) the desktop build had a clipboard that only worked inside the app. On native Wayland, text goes through egui's Wayland clipboard; HTML stays on X11, macOS and Windows | +| 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 | | 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 | @@ -321,7 +326,10 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| -| 2026-11-01 | major | Workflow-breaking regression fixed: the desktop build uses the system clipboard again (eframe `clipboard` feature) | +| 2026-11-02 | trivial | File › About replaces Account; product links, credits and appearance retained | +| 2026-11-02 | trivial | Progress log: tiled picture fills keep their size in scaled groups. Scores and milestone names unchanged | +| 2026-11-01 | trivial | Workflow-breaking regression fixed: the desktop build uses the system clipboard again (eframe `clipboard` feature) | +| 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 | | 2026-10-29 | trivial | Progress log: the Picture Styles row leaves room for Crop and Size (#1232). Scores and milestone names unchanged | diff --git a/crates/anim/src/morph.rs b/crates/anim/src/morph.rs index a869ebdb3..be41aeacb 100644 --- a/crates/anim/src/morph.rs +++ b/crates/anim/src/morph.rs @@ -1,5 +1,7 @@ //! Morph: matching shapes between two slides, interpolating their boxes and building the frames. +use std::collections::{HashMap, HashSet, VecDeque}; + use deckcraft_model::resolve::{self, Ctx}; use deckcraft_model::{ColorRef, Fill, Geom, Presentation, Rgba, Shape, ShapeId, ShapeKind, Slide, Xfrm, walk}; @@ -15,48 +17,66 @@ fn text_of(s: &Shape) -> String { /// kind and preset geometry if neither carries text. A text box with neither a name nor its text on /// the other slide stays unpaired and fades in place, as in PowerPoint (#119). Each shape is used /// once; top-level shapes only (groups move whole). +/// +/// Each pass compares a key of the two shapes, so the shapes of `b` are indexed by key once per +/// pass: matching costs the sum of the two shape lists, not their product. pub fn morph_pairs(a: &Slide, b: &Slide) -> Vec<(ShapeId, ShapeId)> { let mut pairs: Vec<(ShapeId, ShapeId)> = Vec::new(); let mut used_a = vec![false; a.shapes.len()]; let mut used_b = vec![false; b.shapes.len()]; - let passes: [&dyn Fn(&Shape, &Shape) -> bool; 4] = [ - &|x, y| x.name.starts_with("!!") && x.name == y.name, - &|x, y| !x.name.is_empty() && x.name == y.name, - &|x, y| { - if x.kind_name() != y.kind_name() { - return false; + // Each shape's text, worked out once rather than inside every comparison. + let text_a: Vec = a.shapes.iter().map(text_of).collect(); + let text_b: Vec = b.shapes.iter().map(text_of).collect(); + let forced = |s: &Shape, _: &str| (s.name.starts_with("!!")).then(|| MorphKey::Name(s.name.clone())); + let named = |s: &Shape, _: &str| (!s.name.is_empty()).then(|| MorphKey::Name(s.name.clone())); + let same_text = |s: &Shape, t: &str| (!t.trim().is_empty()).then(|| MorphKey::Text(s.kind_name(), t.to_string())); + let same_geom = |s: &Shape, t: &str| match s.geom.preset_name() { + Some(g) if t.trim().is_empty() => Some(MorphKey::Geom(s.kind_name(), g.to_string())), + _ => None, + }; + let passes: [&dyn Fn(&Shape, &str) -> Option; 4] = [&forced, &named, &same_text, &same_geom]; + for key in passes { + // Unused shapes of `b` by key, in slide order: the front of each queue is the first unused + // match, which is the one a search through `b` would find. + let mut index: HashMap> = HashMap::new(); + for (j, sb) in b.shapes.iter().enumerate() { + if used_b.get(j).copied().unwrap_or(true) { + continue; } - let tx = text_of(x); - !tx.trim().is_empty() && tx == text_of(y) - }, - &|x, y| { - x.kind_name() == y.kind_name() - && x.geom.preset_name().is_some() - && x.geom.preset_name() == y.geom.preset_name() - && text_of(x).trim().is_empty() - && text_of(y).trim().is_empty() - }, - ]; - for pass in passes { + if let Some(k) = key(sb, text_b.get(j).map_or("", String::as_str)) { + index.entry(k).or_default().push_back(j); + } + } + if index.is_empty() { + continue; + } for (i, sa) in a.shapes.iter().enumerate() { if used_a.get(i).copied().unwrap_or(true) { continue; } - let found = b.shapes.iter().enumerate().find(|(j, sb)| !used_b.get(*j).copied().unwrap_or(true) && pass(sa, sb)); - if let Some((j, sb)) = found { - if let Some(u) = used_a.get_mut(i) { - *u = true; - } - if let Some(u) = used_b.get_mut(j) { - *u = true; - } - pairs.push((sa.id, sb.id)); + let Some(k) = key(sa, text_a.get(i).map_or("", String::as_str)) else { continue }; + let Some(j) = index.get_mut(&k).and_then(VecDeque::pop_front) else { continue }; + let Some(sb) = b.shapes.get(j) else { continue }; + if let Some(u) = used_a.get_mut(i) { + *u = true; } + if let Some(u) = used_b.get_mut(j) { + *u = true; + } + pairs.push((sa.id, sb.id)); } } pairs } +/// What one [`morph_pairs`] pass matches shapes on. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +enum MorphKey { + Name(String), + Text(&'static str, String), + Geom(&'static str, String), +} + fn lerp(a: f64, b: f64, t: f64) -> f64 { let v = a + (b - a) * t; if v.is_finite() { @@ -256,13 +276,23 @@ pub fn morph_frame(pres: &Presentation, a: &Slide, b: &Slide, t: f64, hidden_b: f.shapes.push((sh, old)); id }; + // Lookups by id, built once (the first shape or pair with an id wins, as a search would). + let paired_a: HashSet = pairs.iter().map(|(ia, _)| *ia).collect(); + let mut partner: HashMap = HashMap::with_capacity(pairs.len()); + for (ia, ib) in &pairs { + partner.entry(*ib).or_insert(*ia); + } + let mut a_by_id: HashMap = HashMap::with_capacity(a.shapes.len()); + for x in &a.shapes { + a_by_id.entry(x.id).or_insert(x); + } for sa in &a.shapes { - if !pairs.iter().any(|(ia, _)| *ia == sa.id) { + if !paired_a.contains(&sa.id) { push(sa.clone(), true, 1.0 - s); } } for sb in &b.shapes { - let pair = pairs.iter().find(|(_, ib)| *ib == sb.id).and_then(|(ia, _)| a.shapes.iter().find(|x| x.id == *ia)); + let pair = partner.get(&sb.id).and_then(|ia| a_by_id.get(ia).copied()); let Some(sa) = pair else { if !hidden_b(sb.id) { push(sb.clone(), false, s); diff --git a/crates/anim/src/tests.rs b/crates/anim/src/tests.rs index c98ac22b8..385520b6d 100644 --- a/crates/anim/src/tests.rs +++ b/crates/anim/src/tests.rs @@ -1155,3 +1155,58 @@ fn many_by_paragraph_animations_on_one_shape_share_paragraph_targets() { assert_eq!(tl.effects().len(), 6_000); assert_eq!(tl.paragraph_targets(ShapeId(1)), vec![0, 1, 2]); } + +#[test] +fn morph_pairs_take_the_first_unused_match_in_slide_order() { + let mut a = Slide::default(); + let mut b = Slide::default(); + a.shapes = vec![shape(1, "Same"), shape(2, "Same"), shape(3, "Other")]; + b.shapes = vec![shape(11, "Same"), shape(12, "Same"), shape(13, "Same")]; + // By name first, in slide order; the shape of `a` left over pairs by kind and geometry with + // the shape of `b` left over. + assert_eq!(morph_pairs(&a, &b), [(ShapeId(1), ShapeId(11)), (ShapeId(2), ShapeId(12)), (ShapeId(3), ShapeId(13))]); +} + +#[test] +fn morph_of_large_slides_stays_linear() { + let n = 20_000u32; + let text = |i: u32, side: &str| Some(TextBody::from_text(&format!("{side} {i}"))); + let mut a = Slide::default(); + let mut b = Slide::default(); + // Text no shape on the other slide shares: every pass looks for a partner and finds none. + a.shapes = (0..n).map(|i| Shape { text: text(i, "a"), ..shape(i + 1, "") }).collect(); + b.shapes = (0..n).map(|i| Shape { text: text(i, "b"), ..shape(n + i + 1, "") }).collect(); + // One named pair at opposite ends. + a.shapes[0].name = "Logo".into(); + b.shapes[n as usize - 1].name = "Logo".into(); + // No wall-clock bound (it would flake on a loaded runner): at this size a quadratic pairing + // takes minutes, so a regression shows as a test that hangs rather than a failed assertion. + assert_eq!(morph_pairs(&a, &b), [(ShapeId(1), ShapeId(2 * n))]); + let p = deckcraft_model::defaults::new_presentation(None); + let f = morph_frame(&p, &a, &b, 0.5, &|_| false); + // Every shape fades in place; the named pair differs in text, so it cross-fades as two. + assert_eq!(f.shapes.len(), 2 * n as usize); +} + +#[test] +fn paragraph_states_of_a_large_build_stay_linear() { + let n = 20_000usize; + let mut s = slide_with(vec![]); + let mut body = TextBody::from_text(&vec!["line"; n].join("\n")); + // One first-level paragraph owning every other paragraph. + for p in body.paragraphs.iter_mut().skip(1) { + p.level = 1; + } + s.shapes[0].text = Some(body); + let mut a = anim(1, AnimClass::Entrance, "fade", AnimStart::OnClick); + a.text_build = TextBuild::ByParagraph; + s.animations = vec![a]; + let tl = Timeline::new(&s, W, H); + // No wall-clock bound, as above: a per-paragraph scan of the slide would take minutes here. + assert_eq!(tl.paragraph_targets(ShapeId(1)).len(), n); + for p in 0..n { + assert!(!tl.state(ShapeId(1), Some(p), 0, 0.0).visible); + assert!(tl.state(ShapeId(1), Some(p), 1, 0.0).visible); + } + assert_eq!(tl.animated_shapes(), [ShapeId(1)]); +} diff --git a/crates/anim/src/timeline.rs b/crates/anim/src/timeline.rs index c995f2eaa..5784f4ea8 100644 --- a/crates/anim/src/timeline.rs +++ b/crates/anim/src/timeline.rs @@ -103,6 +103,41 @@ pub struct Timeline { /// By-paragraph builds animate first-level paragraphs; deeper paragraphs follow their parent: /// (shape, member paragraph, owner paragraph). groups: Vec<(ShapeId, usize, usize)>, + /// Lookups built with the timeline, so evaluating a shape or paragraph costs its own effects + /// rather than a scan of every effect, group and colour on the slide. + index: TimelineIndex, +} + +#[derive(Clone, Debug, Default)] +struct TimelineIndex { + /// Effects (indices into `effects`, in order) by target shape and paragraph. + by_target: HashMap<(ShapeId, Option), Vec>, + /// Effects (indices into `effects`, in order) by target shape. + by_shape: HashMap>, + /// Owner paragraph of each member paragraph (the first group recorded wins). + owner: HashMap<(ShapeId, usize), usize>, + /// Groups (member, owner) by shape, in order. + groups_by_shape: HashMap>, + /// Base colour by shape (the first recorded wins). + color: HashMap, +} + +impl TimelineIndex { + fn build(effects: &[Effect], groups: &[(ShapeId, usize, usize)], colors: &[(ShapeId, Rgba)]) -> TimelineIndex { + let mut ix = TimelineIndex::default(); + for (i, e) in effects.iter().enumerate() { + ix.by_target.entry((e.timing.shape, e.timing.paragraph)).or_default().push(i); + ix.by_shape.entry(e.timing.shape).or_default().push(i); + } + for &(shape, member, owner) in groups { + ix.owner.entry((shape, member)).or_insert(owner); + ix.groups_by_shape.entry(shape).or_default().push((member, owner)); + } + for &(shape, c) in colors { + ix.color.entry(shape).or_insert(c); + } + ix + } } enum Status { @@ -324,6 +359,7 @@ impl Timeline { }; tl.colors.push((id, c)); } + tl.index = TimelineIndex::build(&tl.effects, &tl.groups, &tl.colors); tl } @@ -413,8 +449,12 @@ impl Timeline { /// Paragraph indices of `shape` animated separately (by-paragraph builds). pub fn paragraph_targets(&self, shape: ShapeId) -> Vec { - let mut v: Vec = self.effects.iter().filter(|e| e.timing.shape == shape).filter_map(|e| e.timing.paragraph).collect(); - v.extend(self.groups.iter().filter(|g| g.0 == shape && v.contains(&g.2)).map(|g| g.1).collect::>()); + let mine = self.index.by_shape.get(&shape).map_or(&[][..], Vec::as_slice); + let mut v: Vec = mine.iter().filter_map(|&i| self.effects.get(i)).filter_map(|e| e.timing.paragraph).collect(); + let owners: HashSet = v.iter().copied().collect(); + if let Some(groups) = self.index.groups_by_shape.get(&shape) { + v.extend(groups.iter().filter(|(_, owner)| owners.contains(owner)).map(|(member, _)| *member)); + } v.sort_unstable(); v.dedup(); v @@ -422,13 +462,8 @@ impl Timeline { /// Shapes with any effect. pub fn animated_shapes(&self) -> Vec { - let mut v: Vec = Vec::new(); - for e in &self.effects { - if !v.contains(&e.timing.shape) { - v.push(e.timing.shape); - } - } - v + let mut seen: HashSet = HashSet::new(); + self.effects.iter().map(|e| e.timing.shape).filter(|s| seen.insert(*s)).collect() } /// State of `shape` (or of one of its paragraphs, `para`) when `step` main steps have been @@ -456,9 +491,9 @@ impl Timeline { triggers: &[TriggerPlay], stamps: &[StepStamp], ) -> AnimState { - let para = para.map(|p| self.groups.iter().find(|g| g.0 == shape && g.1 == p).map(|g| g.2).unwrap_or(p)); - let mine = self.effects.iter().filter(|e| e.timing.shape == shape && e.timing.paragraph == para); - let base_color = self.colors.iter().find(|(s, _)| *s == shape).map(|(_, c)| *c).unwrap_or(Rgba::rgb(0x15, 0x60, 0x82)); + let para = para.map(|p| self.index.owner.get(&(shape, p)).copied().unwrap_or(p)); + let mine = self.index.by_target.get(&(shape, para)).map_or(&[][..], Vec::as_slice).iter().filter_map(|&i| self.effects.get(i)); + let base_color = self.index.color.get(&shape).copied().unwrap_or(Rgba::rgb(0x15, 0x60, 0x82)); let mut st = AnimState::default(); // Initial visibility comes from the first effect in the Animation Pane order. let mut ordered: Vec<&Effect> = mine.collect(); diff --git a/crates/engine/src/cmd/design/master_view.rs b/crates/engine/src/cmd/design/master_view.rs index 5dfe04964..c03bfa574 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/cmd/design/slide_background.rs b/crates/engine/src/cmd/design/slide_background.rs index 0de78e7e3..494b53be8 100644 --- a/crates/engine/src/cmd/design/slide_background.rs +++ b/crates/engine/src/cmd/design/slide_background.rs @@ -1,12 +1,14 @@ use super::{ Arc, BTreeSet, Background, Fill, NO_ID_SPACE, Presentation, Result, Session, Target, Value, bad, bool_or, bool_param, color_param, color_value, - f64_param, ok, usize_param, with_param, + f64_param, ok, opt_usize_param, usize_param, with_param, }; pub(super) fn background(s: &mut Session, p: &Value) -> Result { if bool_or(p, "copy", false) { return copy_slide_background(s); } + // Check the index before anything is edited: a picture is added to the media first. + let index = opt_usize_param(p, "index", "design.background")?; let props = p.get("pictureProps").filter(|v| v.is_object()).cloned(); let transparency = f64_param(p, "transparency"); let reset = bool_or(p, "reset", false); @@ -62,7 +64,6 @@ pub(super) fn background(s: &mut Session, p: &Value) -> Result { )); }; let all = bool_or(p, "all", false); - let index = usize_param(p, "index"); if matches!(op, BgOp::Adjust) { let next = { let st = s.doc()?; diff --git a/crates/engine/src/cmd/design/tests.rs b/crates/engine/src/cmd/design/tests.rs index 9765fcc8f..82eed6feb 100644 --- a/crates/engine/src/cmd/design/tests.rs +++ b/crates/engine/src/cmd/design/tests.rs @@ -485,3 +485,16 @@ fn background_all_clears_layout_overrides_even_if_slides_and_master_match() { s.execute("design.background", &json!({"color": "#AA0000", "all": true})).unwrap(); assert_eq!(undo_len(&s), n + 1, "now it is truly a no-op"); } + +#[test] +fn background_rejects_invalid_index() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({})).unwrap(); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + let before = s.doc().unwrap().doc.slides[0].background.clone(); + for bad in [json!(-1), json!(1.5)] { + assert!(s.execute("design.background", &json!({"color": "#0000C8", "index": bad})).is_err()); + assert!(s.execute("design.background", &json!({"transparency": 0.5, "index": bad})).is_err()); + assert_eq!(s.doc().unwrap().doc.slides[0].background, before); + } +} diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 0d52e6358..8943eb9a3 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" | "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())), } } @@ -359,7 +360,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 = 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"))); + } + 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/cmd/format/paragraph.rs b/crates/engine/src/cmd/format/paragraph.rs index f036068f6..6d575ff1a 100644 --- a/crates/engine/src/cmd/format/paragraph.rs +++ b/crates/engine/src/cmd/format/paragraph.rs @@ -55,6 +55,7 @@ pub(super) fn bullets(s: &mut Session, p: &Value) -> Result { } if size.is_some() { para.props.bullet_size = size; + para.props.bullet_size_points = None; para.props.bullet_size_follow_text = None; } if para.props.indent.is_none_or(|i| i >= 0.0) && para.props.margin_left.is_none_or(|m| m == 0.0) { @@ -85,6 +86,7 @@ pub(super) fn numbering(s: &mut Session, p: &Value) -> Result { } if size.is_some() { para.props.bullet_size = size; + para.props.bullet_size_points = None; para.props.bullet_size_follow_text = None; } para.props.margin_left = Some(27.0 + 36.0 * para.level as f64); diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 9565cdf7b..08ed9be64 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -103,7 +103,7 @@ pub fn specs() -> Vec { "Shape Fill", ["Shape Format", "Shape Styles"], None, - "{color? | none?: true | background?: true (the slide background shows through; shapes and text boxes only) | gradient?: {stops: [[pos, color]], angle?, kind?: linear|radial} | picture?: base64 | pattern?: {preset, fg, bg} | transparency?: 0..1 | reset?: true, ids?}; pictureProps?: {tile?: bool, offsetX?, offsetY?: pt, scaleX?, scaleY?: 0..10, align?: tl|t|tr|l|ctr|r|bl|b|br, mirror?: none|x|y|xy, stretch?: [l, t, r, b] fractions, rotateWithShape?: bool} adjusts a picture fill; a group also formats its members", + "{color? | none?: true | background?: true (the slide background shows through; shapes and text boxes only) | gradient?: {stops: [[pos 0..1, color]], kind?: linear|radial|rectangular|path, angle?: deg (linear), focus?: [l, t, r, b] fractions (radial, rectangular and path: where the first stop sits), rotateWithShape?: bool} | picture?: base64 | pattern?: {preset, fg, bg} | transparency?: 0..1 | reset?: true, ids?}; pictureProps?: {tile?: bool, offsetX?, offsetY?: pt, scaleX?, scaleY?: 0..10, align?: tl|t|tr|l|ctr|r|bl|b|br, mirror?: none|x|y|xy, stretch?: [l, t, r, b] fractions, rotateWithShape?: bool} adjusts a picture fill; a group also formats its members", has_selection, fill ), diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7d21bb4b8..38f4de8d2 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); + } } diff --git a/crates/engine/src/spell.rs b/crates/engine/src/spell.rs index 93852e3f8..2dda5c6f4 100644 --- a/crates/engine/src/spell.rs +++ b/crates/engine/src/spell.rs @@ -709,7 +709,8 @@ pub fn paragraph_words(p: &deckcraft_model::Paragraph, default: &str) -> Vec Vec a && !chars[b - 1].0.is_alphanumeric() { - b -= 1; + // Trim back to the last letter or digit, keeping only the marks that sit on it + // (a mark on a trailing apostrophe goes with the apostrophe). + while b > a { + let k = (a..b).rev().find(|&k| !deckcraft_model::words::is_combining_mark(chars[k].0)).unwrap_or(a); + if chars[k].0.is_alphanumeric() { + break; + } + b = k; } let text: String = chars[a..b].iter().map(|c| c.0).collect(); if text.is_empty() || text.chars().any(|c| c.is_ascii_digit()) || !text.chars().any(char::is_alphabetic) { @@ -819,6 +826,20 @@ SFX D Y 1\nSFX D 0 ed/S .\n"; Dictionary::parse(AFF.as_bytes(), DIC.as_bytes()) } + #[test] + fn nfd_word_not_split() { + let p = deckcraft_model::Paragraph::new("Das na\u{ef}ve Caf\u{e9} und cafe\u{301} na\u{308}ive"); + let w: Vec = paragraph_words(&p, "en-US").into_iter().map(|w| w.text).collect(); + assert_eq!(w, ["Das", "na\u{ef}ve", "Caf\u{e9}", "und", "cafe\u{301}", "na\u{308}ive"]); + } + + #[test] + fn a_mark_on_trailing_punctuation_is_trimmed_with_it() { + let p = deckcraft_model::Paragraph::new("word'\u{301} cafe\u{301}' x"); + let w: Vec = paragraph_words(&p, "en-US").into_iter().map(|w| w.text).collect(); + assert_eq!(w, ["word", "cafe\u{301}", "x"]); + } + #[test] fn curly_apostrophe_matches_ascii_entry() { let d = Dictionary::parse(AFF.as_bytes(), "2\ndon't\nwork\n".as_bytes()); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 26fb7b9cf..beeedb62b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2595,6 +2595,23 @@ 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}"); + } + // 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] fn shape_line_sets_compound_and_arrow_sizes() { // The Format Shape pane's Line page: compound type, arrowheads and their sizes. @@ -3391,6 +3408,7 @@ fn numbering_takes_color_and_size() { props.bullet_font_follow_text = Some(true); props.bullet_color_follow_text = Some(true); props.bullet_size_follow_text = Some(true); + props.bullet_size_points = Some(12.0); } s.execute("format.numbering", &json!({"on": true, "scheme": "romanUcPeriod", "start": 3, "color": "accent2", "size": 150})).unwrap(); let para = |s: &Session| { @@ -3404,9 +3422,11 @@ fn numbering_takes_color_and_size() { assert_eq!(pp.bullet_font_follow_text, Some(true)); assert_eq!(pp.bullet_color_follow_text, None); assert_eq!(pp.bullet_size_follow_text, None); + assert_eq!(pp.bullet_size_points, None); assert_eq!(pp.bullet_color, Some(deckcraft_model::ColorRef::scheme(deckcraft_color::SchemeSlot::Accent2))); s.execute("format.bullets", &json!({"on": true, "char": "★", "size": 9000})).unwrap(); assert_eq!(para(&s).bullet_size, Some(4.0)); + assert_eq!(para(&s).bullet_size_points, None); assert_eq!(para(&s).bullet_font_follow_text, None); } @@ -4479,6 +4499,38 @@ fn a_params_doc_spells_out_each_parameter() { ); } +/// `shape.fill`'s params doc names each key and kind its `gradient` takes. +#[test] +fn shape_fill_documents_each_gradient_key_and_kind() { + use deckcraft_model::style::GradientShape; + let params = cmd::parse_params(find_command("shape.fill").unwrap().params).unwrap(); + let about = params.iter().find(|p| p.name == "gradient").unwrap().about; + assert_eq!(cmd::param_keys(about).unwrap(), ["stops", "kind", "angle", "focus", "rotateWithShape"]); + for kind in ["linear", "radial", "rectangular", "path"] { + assert!(about.contains(kind), "{kind}: {about}"); + let g = cmd::shape::gradient_param(&json!({ + "stops": [[0, "#ffffff"], [1, "#000000"]], + "kind": kind, + "angle": 30, + "focus": [0, 0, 1, 1], + "rotateWithShape": false + })) + .unwrap(); + assert!(!g.rotate_with_shape, "{kind}"); + match g.shape { + GradientShape::Linear { angle, .. } => assert_eq!((kind, angle), ("linear", 30.0)), + GradientShape::Path { path, focus } => { + let expected = match kind { + "radial" => "circle", + "rectangular" => "rect", + _ => "shape", + }; + assert_eq!((path.as_str(), focus), (expected, [0.0, 0.0, 1.0, 1.0]), "{kind}"); + } + } + } +} + #[test] fn what_a_command_acts_on_names_only_keys_it_takes() { for c in command_specs() { @@ -5757,6 +5809,170 @@ fn edit_points_conversion_is_one_undo_step() { assert!(preset(&s), "the only step was the conversion"); } +/// #1621: build overlapping objects with an ordinary quick style on the foreground shape. +fn no_fill_overlap(s: &mut Session, text: bool) -> (ShapeId, ShapeId) { + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + let under = s + .execute( + if text { "insert.textBox" } else { "shape.insert" }, + &json!({"preset": "rect", "rect": [100, 300, 300, 60], "text": if text { "UNDERLYING TEXT" } else { "" }}), + ) + .unwrap()["id"] + .as_u64() + .unwrap(); + let over = s.execute("shape.insert", &json!({"preset": "rect", "rect": [80, 270, 350, 140]})).unwrap()["id"].as_u64().unwrap(); + (ShapeId(under as u32), ShapeId(over as u32)) +} + +/// No Fill passes interior clicks through while preserving outline and actual-fill targets. +#[test] +fn no_fill_shape_allows_selecting_and_editing_underlying_text() { + let mut s = session(); + let (under, over) = no_fill_overlap(&mut s, true); + s.execute("shape.fill", &json!({"none": true})).unwrap(); + assert!(s.doc().unwrap().shape(over).unwrap().style.is_some(), "the ordinary quick style remains"); + s.execute("edit.deselect", &json!({})).unwrap(); + let p = deckcraft_geom::Point::new(200.0, 310.0); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, .. }) if id == under)); + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(tev(kind, p.x, p.y)).unwrap(); + } + assert_eq!(s.doc().unwrap().selection.shapes, vec![under]); + assert_eq!(text_sel(&s).unwrap().shape, under); + // The transparent foreground also leaves an already-active text editor's caret alone. + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(tev(kind, p.x + 20.0, p.y)).unwrap(); + } + assert_eq!(text_sel(&s).unwrap().shape, under); + s.execute("edit.deselect", &json!({})).unwrap(); + // The rectangle's outline is still a target, and selecting it does not remove its style. + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(tev(kind, 80.0, 330.0)).unwrap(); + } + assert_eq!(s.doc().unwrap().selection.shapes, vec![over]); + assert!(s.doc().unwrap().shape(over).unwrap().style.is_some()); + // A real fill still catches clicks, even with 100% transparency (distinct from No Fill). + for transparency in [0.0, 1.0] { + s.execute("edit.select", &json!({"ids": [over.0]})).unwrap(); + s.execute("shape.fill", &json!({"color": "#336699", "transparency": transparency})).unwrap(); + s.execute("edit.deselect", &json!({})).unwrap(); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, .. }) if id == over)); + } +} + +/// Rotation and pointer tolerance must preserve both pass-through interiors and outline hits. +#[test] +fn no_fill_shape_hit_testing_respects_rotation_and_pointer_tolerance() { + let mut s = session(); + let (under, over) = no_fill_overlap(&mut s, false); + s.execute("shape.fill", &json!({"none": true})).unwrap(); + s.execute("shape.rotate", &json!({"deg": 30})).unwrap(); + let x = s.doc().unwrap().shape(over).unwrap().xfrm.unwrap(); + let p = x.affine() * deckcraft_geom::Point::new(x.w / 2.0, x.h / 2.0); + let edge = x.affine() * deckcraft_geom::Point::new(0.0, x.h / 4.0); + for tol in [0.5, 3.0, 8.0] { + s.execute("edit.deselect", &json!({})).unwrap(); + assert!(matches!(s.hit_test(p, tol), Some(tools::Hit::Shape { id, .. }) if id == under)); + assert!(matches!(s.hit_test(edge, tol), Some(tools::Hit::Shape { id, .. }) if id == over)); + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(PointerEvent { tol, ..tev(kind, p.x, p.y) }).unwrap(); + } + assert_eq!(s.doc().unwrap().selection.shapes, vec![under]); + } +} + +/// Style, theme and absent fills follow rendering, with explicit fill taking precedence. +#[test] +fn no_fill_hit_testing_resolves_style_and_theme_fills() { + use std::sync::Arc; + for source in ["style", "theme", "absent"] { + let mut s = session(); + let (under, over) = no_fill_overlap(&mut s, true); + { + let st = s.doc_mut().unwrap(); + let doc = Arc::make_mut(&mut st.doc); + if source == "theme" { + Arc::make_mut(&mut doc.masters[0]).theme.format.fills[0] = model::Fill::None; + } + let sh = Arc::make_mut(&mut doc.slides[st.selection.slide]).shapes.iter_mut().find(|sh| sh.id == over).unwrap(); + assert!(sh.fill.is_none(), "the shape has no explicit fill override"); + if source == "absent" { + sh.style = None; + } else { + sh.style.as_mut().unwrap().fill_ref.0 = if source == "style" { 0 } else { 1 }; + } + } + s.execute("edit.deselect", &json!({})).unwrap(); + let p = deckcraft_geom::Point::new(200.0, 310.0); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, .. }) if id == under), "{source}"); + // An explicit fill takes precedence over the no-fill style/theme. + s.execute("edit.select", &json!({"ids": [over.0]})).unwrap(); + s.execute("shape.fill", &json!({"color": "#336699"})).unwrap(); + s.execute("edit.deselect", &json!({})).unwrap(); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, .. }) if id == over), "{source}"); + } +} + +/// Text on a foreground No Fill shape retains its selection and editing target. +#[test] +fn no_fill_text_shapes_keep_their_interior_text_target() { + let mut s = session(); + let (_, over) = no_fill_overlap(&mut s, true); + s.execute("text.set", &json!({"id": over.0, "text": "FOREGROUND TEXT"})).unwrap(); + s.execute("shape.fill", &json!({"none": true})).unwrap(); + s.execute("edit.deselect", &json!({})).unwrap(); + let p = deckcraft_geom::Point::new(200.0, 310.0); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, text: true }) if id == over)); + s.pointer(tev(PointerKind::DoubleClick, p.x, p.y)).unwrap(); + assert_eq!(text_sel(&s).unwrap().shape, over); +} + +/// A locked No Fill shape remains selectable on its outline, but dragging cannot move it. +#[test] +fn no_fill_locked_shape_keeps_outline_selection_without_moving() { + let mut s = session(); + let (under, over) = no_fill_overlap(&mut s, true); + s.execute("shape.fill", &json!({"none": true})).unwrap(); + s.execute("shape.lock", &json!({"locked": true})).unwrap(); + let before = s.doc().unwrap().shape(over).unwrap().xfrm; + s.execute("edit.deselect", &json!({})).unwrap(); + let inside = deckcraft_geom::Point::new(200.0, 310.0); + assert!(matches!(s.hit_test(inside, 3.0), Some(tools::Hit::Shape { id, .. }) if id == under)); + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(tev(kind, inside.x, inside.y)).unwrap(); + } + assert_eq!(text_sel(&s).unwrap().shape, under); + s.execute("edit.deselect", &json!({})).unwrap(); + let edge = deckcraft_geom::Point::new(80.0, 330.0); + assert!(matches!(s.hit_test(edge, 3.0), Some(tools::Hit::Shape { id, .. }) if id == over)); + s.pointer(tev(PointerKind::Down, edge.x, edge.y)).unwrap(); + s.pointer(tev(PointerKind::Drag, edge.x + 40.0, edge.y + 20.0)).unwrap(); + s.pointer(tev(PointerKind::Up, edge.x + 40.0, edge.y + 20.0)).unwrap(); + let st = s.doc().unwrap(); + assert_eq!(st.selection.shapes, vec![over]); + assert_eq!(st.shape(over).unwrap().xfrm, before); + assert!(st.shape(over).unwrap().locked); +} + +/// Removing both fill and outline leaves the empty interior transparent to clicks, locked or not. +#[test] +fn no_fill_shape_without_text_or_outline_passes_interior_clicks() { + for locked in [false, true] { + let mut s = session(); + let (under, _) = no_fill_overlap(&mut s, false); + s.execute("shape.fill", &json!({"none": true})).unwrap(); + s.execute("shape.line", &json!({"none": true})).unwrap(); + s.execute("shape.lock", &json!({"locked": locked})).unwrap(); + s.execute("edit.deselect", &json!({})).unwrap(); + let p = deckcraft_geom::Point::new(200.0, 310.0); + assert!(matches!(s.hit_test(p, 3.0), Some(tools::Hit::Shape { id, .. }) if id == under)); + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(tev(kind, p.x, p.y)).unwrap(); + } + assert_eq!(s.doc().unwrap().selection.shapes, vec![under]); + } +} + /// #931: while the text of a placeholder is being edited, a shape in front of it still takes a /// click (as in PowerPoint); a click on the text where nothing is in front still moves the caret. #[test] @@ -5830,6 +6046,204 @@ fn box_of(s: &mut Session, id: u64) -> serde_json::Value { s.execute("shape.inspect", &json!({"id": id})).unwrap()["box"].clone() } +/// Two selected boxes with different sizes and aligned right edges, on a blank slide. +fn resize_selection_scene() -> (Session, [u64; 2]) { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.prefs.smart_guides = false; + s.prefs.snap_to_grid = false; + let ids = [[100, 100, 100, 80], [120, 300, 80, 100]] + .map(|rect| s.execute("shape.insert", &json!({"preset": "rect", "rect": rect, "text": "Resize test"})).unwrap()["id"].as_u64().unwrap()); + s.execute("edit.select", &json!({"ids": ids})).unwrap(); + (s, ids) +} + +/// Simulate file-loaded shapes whose stored boxes have not yet been fitted to their text. +fn pending_resize_autofit(s: &mut Session, ids: &[u64]) { + s.edit(|doc, sel| { + let shapes = crate::shapes_mut(doc, sel).unwrap(); + for id in ids { + let sh = deckcraft_model::find_shape_mut(shapes, ShapeId(*id as u32)).unwrap(); + sh.text.as_mut().unwrap().body.autofit = Some(deckcraft_model::text::AutoFit::Shape); + } + Ok(()) + }) + .unwrap(); +} + +/// Resizing multiple selected shapes scales them proportionally around their anchors in one undo step. +#[test] +fn resizing_selected_shapes_is_one_undo_step() { + let (mut s, [a, b]) = resize_selection_scene(); + let before = s.doc().unwrap().doc.clone(); + let steps = s.doc().unwrap().history.undo.len(); + let down = drag_gesture(&mut s, (200.0, 180.0), (250.0, 210.0)); + assert_eq!(down["hit"]["kind"], "resize"); + assert_eq!(down["hit"]["id"], a); + for (id, expected) in [(a, [100.0, 100.0, 150.0, 110.0]), (b, [120.0, 300.0, 120.0, 137.5])] { + let actual = box_of(&mut s, id); + for (key, value) in ["x", "y", "w", "h"].into_iter().zip(expected) { + assert_eq!(actual[key], value, "{id}: {key}"); + } + } + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1); + let after = s.doc().unwrap().doc.clone(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, before, "one undo restores both shapes"); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, after, "one redo resizes both shapes again"); +} + +/// Resizing selected shapes refits their text boxes inside the same undo transaction. +#[test] +fn resizing_selected_shapes_refits_text_in_the_same_undo_step() { + let (mut s, ids) = resize_selection_scene(); + pending_resize_autofit(&mut s, &ids); + let before = s.doc().unwrap().doc.clone(); + let steps = s.doc().unwrap().history.undo.len(); + // An east-handle drag changes width only; a height change must come from text fitting. + drag_gesture(&mut s, (200.0, 140.0), (250.0, 140.0)); + for (id, old_height) in ids.into_iter().zip([80.0, 100.0]) { + assert_ne!(box_of(&mut s, id)["h"], old_height, "both shapes' text is refitted"); + } + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1); + let after = s.doc().unwrap().doc.clone(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, before); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, after, "redo includes the final text fit"); +} + +/// Clicking a resize handle without dragging neither modifies shapes nor triggers text autofit. +#[test] +fn clicking_a_resize_handle_does_not_change_or_refit_selected_shapes() { + let (mut s, ids) = resize_selection_scene(); + pending_resize_autofit(&mut s, &ids); + let before = s.doc().unwrap().doc.clone(); + let steps = s.doc().unwrap().history.undo.len(); + let revision = s.doc().unwrap().revision; + for kind in [PointerKind::Down, PointerKind::Up] { + s.pointer(PointerEvent { kind, x: 200.0, y: 180.0, tol: 3.0, ..Default::default() }).unwrap(); + } + assert_eq!(s.doc().unwrap().doc, before); + assert_eq!(s.doc().unwrap().history.undo.len(), steps, "a click creates no undo entry"); + assert_eq!(s.doc().unwrap().revision, revision); + assert!(std::sync::Arc::ptr_eq(&s.doc().unwrap().doc, &before), "a click does not dirty the document"); + // Ensure this fixture would catch an accidental refit on release. + for id in ids { + cmd::text::fit_text_box(&mut s, ShapeId(id as u32)).unwrap(); + } + assert_ne!(s.doc().unwrap().doc, before); +} + +/// Locked shapes in a multi-selection are excluded from both resizing and post-resize text fitting. +#[test] +fn resizing_selected_shapes_skips_locked_shapes_and_their_text_fit() { + let (mut s, [a, b]) = resize_selection_scene(); + pending_resize_autofit(&mut s, &[b]); + s.execute("shape.lock", &json!({"ids": [b], "locked": true})).unwrap(); + s.execute("edit.select", &json!({"ids": [a, b]})).unwrap(); + let locked = s.doc().unwrap().shape(ShapeId(b as u32)).unwrap().clone(); + let before = s.doc().unwrap().doc.clone(); + let steps = s.doc().unwrap().history.undo.len(); + drag_gesture(&mut s, (200.0, 180.0), (250.0, 210.0)); + assert_eq!(box_of(&mut s, a)["w"], 150.0); + assert_eq!(box_of(&mut s, a)["h"], 110.0); + assert_eq!(s.doc().unwrap().shape(ShapeId(b as u32)).unwrap(), &locked, "the locked shape is neither resized nor refitted"); + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, before); +} + +/// Resizing selected shapes excludes other selected resize targets from smart-guide snapping. +#[test] +fn resizing_selected_shapes_does_not_snap_to_other_resize_targets() { + for reverse in [false, true] { + let (mut s, [a, b]) = resize_selection_scene(); + s.prefs.smart_guides = true; + let ids = if reverse { [b, a] } else { [a, b] }; + s.execute("edit.select", &json!({"ids": ids})).unwrap(); + // Within snap tolerance of the other selected shape's original right edge. + drag_gesture(&mut s, (200.0, 140.0), (203.0, 140.0)); + assert_eq!(box_of(&mut s, a)["w"], 103.0, "first shape must not snap back"); + assert_eq!(box_of(&mut s, b)["w"], 82.4, "selection order must not affect snapping"); + } +} + +/// Resizing selected shapes snaps the active shape against unselected stationary shapes and scales others proportionally. +#[test] +fn resizing_selected_shapes_still_snaps_to_stationary_shapes() { + let (mut s, [a, b]) = resize_selection_scene(); + let fixed = s.execute("shape.insert", &json!({"preset": "rect", "rect": [207, 430, 100, 80]})).unwrap()["id"].as_u64().unwrap(); + let fixed_before = box_of(&mut s, fixed); + s.execute("edit.select", &json!({"ids": [a, b]})).unwrap(); + s.prefs.smart_guides = true; + drag_gesture(&mut s, (200.0, 140.0), (203.0, 140.0)); + assert_eq!(box_of(&mut s, a)["w"], 107.0); + assert!((box_of(&mut s, b)["w"].as_f64().unwrap() - 85.6).abs() < 1e-9); + assert_eq!(box_of(&mut s, fixed), fixed_before); +} + +/// Secondary pictures resize uniformly around their anchor, preserving independent flips +/// and keeping the complete multi-selection resize in one undo step. +#[test] +fn resizing_selected_shapes_resizes_secondary_pictures() { + // Pointer destination, Alt, expected rectangle, expected picture, expected flips. + let cases = [ + ((180.0, 200.0), false, [100.0, 100.0, 80.0, 100.0], [300.0, 300.0, 80.0, 80.0], [false, false]), + ((200.0, 180.0), false, [100.0, 100.0, 100.0, 80.0], [300.0, 300.0, 80.0, 80.0], [false, false]), + ((220.0, 200.0), false, [100.0, 100.0, 120.0, 100.0], [300.0, 300.0, 120.0, 120.0], [false, false]), + ((200.0, 220.0), false, [100.0, 100.0, 100.0, 120.0], [300.0, 300.0, 120.0, 120.0], [false, false]), + ((180.0, 200.0), true, [120.0, 100.0, 60.0, 100.0], [320.0, 320.0, 60.0, 60.0], [false, false]), + ((200.0, 180.0), true, [100.0, 120.0, 100.0, 60.0], [320.0, 320.0, 60.0, 60.0], [false, false]), + ((80.0, 200.0), false, [80.0, 100.0, 20.0, 100.0], [280.0, 300.0, 20.0, 20.0], [true, false]), + ((200.0, 80.0), false, [100.0, 80.0, 100.0, 20.0], [300.0, 280.0, 20.0, 20.0], [false, true]), + ]; + + for (to, alt, expected_rect, expected_picture, flips) in cases { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "blank"})).unwrap(); + s.prefs.smart_guides = false; + s.prefs.snap_to_grid = false; + + let rect = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 100, 100]})).unwrap()["id"].as_u64().unwrap(); + let picture = ramp_picture(&mut s, 1, [300, 300, 100, 100]).as_u64().unwrap(); + s.execute("edit.select", &json!({"ids": [rect, picture]})).unwrap(); + + let before = s.doc().unwrap().doc.clone(); + let steps = s.doc().unwrap().history.undo.len(); + let mods = Mods { alt, ..Default::default() }; + let ev = |kind, (x, y): (f64, f64)| PointerEvent { kind, x, y, mods, tol: 3.0, pressure: None }; + + let down = s.pointer(ev(PointerKind::Down, (200.0, 200.0))).unwrap(); + assert_eq!(down["hit"]["kind"], "resize"); + assert_eq!(down["hit"]["id"], rect); + assert_eq!(down["hit"]["handle"], 4); + + s.pointer(ev(PointerKind::Drag, ((200.0 + to.0) / 2.0, (200.0 + to.1) / 2.0))).unwrap(); + s.pointer(ev(PointerKind::Drag, to)).unwrap(); + s.pointer(ev(PointerKind::Up, to)).unwrap(); + + for (id, expected) in [(rect, expected_rect), (picture, expected_picture)] { + let actual = box_of(&mut s, id); + for (key, value) in ["x", "y", "w", "h"].into_iter().zip(expected) { + let got = actual[key].as_f64().unwrap(); + assert!((got - value).abs() < 1e-9, "{to:?}, alt={alt}, shape={id}, {key}: expected {value}, got {got}"); + } + + let xfrm = s.doc().unwrap().shape(ShapeId(id as u32)).unwrap().xfrm.unwrap(); + assert_eq!([xfrm.flip_h, xfrm.flip_v], flips, "{to:?}, alt={alt}, shape={id}"); + } + + assert_eq!(s.doc().unwrap().history.undo.len(), steps + 1); + let after = s.doc().unwrap().doc.clone(); + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, before); + s.execute("edit.redo", &json!({})).unwrap(); + assert_eq!(s.doc().unwrap().doc, after); + } +} + /// #961: dragging the east handle snaps the dragged (right) edge, even when the fixed left edge /// already lines up with another shape. The nearest line used to be the fixed edge's, at distance 0. #[test] @@ -6249,3 +6663,30 @@ 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()); +} + +#[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); +} diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index 78c6ed35c..82f53dfc3 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -150,8 +150,10 @@ enum Drag { }, Resize { id: ShapeId, - handle: u8, orig: Xfrm, + ids: Vec, + handle: u8, + origs: Vec<(ShapeId, Xfrm)>, start: Point, }, LineEnd { diff --git a/crates/engine/src/tools/drag.rs b/crates/engine/src/tools/drag.rs index 21aac4f38..dcee9cf7d 100644 --- a/crates/engine/src/tools/drag.rs +++ b/crates/engine/src/tools/drag.rs @@ -13,6 +13,7 @@ struct MoveDrag { } impl Session { + /// Update the active drag interaction using the pointer position and modifiers. pub(super) fn drag(&mut self, p: Point, mods: Mods, tol: f64) -> Result { let Some(drag) = self.tool.drag.clone() else { return Ok(Value::Null) }; match drag { @@ -26,10 +27,48 @@ impl Session { Drag::Move { ids, start, origs, moved, duplicate, enter_text } => { return self.drag_move(MoveDrag { ids, start, origs, moved, duplicate, enter_text }, p, mods, tol); } - Drag::Resize { id, handle, orig, start } => { - let nx = resize_box(&orig, handle, p - start, mods.shift || self.is_picture(id), mods.alt); - let nx = self.snap_resize(nx, &orig, handle, id, tol); - self.set_xfrm(id, nx)?; + Drag::Resize { id, orig, ids, handle, origs, start } => { + let keep_aspect = mods.shift || self.is_picture(id); + let nx = resize_box(&orig, handle, p - start, keep_aspect, mods.alt); + let nx = self.snap_resize(nx, handle, &ids, tol); + let sx = if orig.w > 1e-9 { (nx.w / orig.w) * if nx.flip_h != orig.flip_h { -1.0 } else { 1.0 } - 1.0 } else { 0.0 }; + let sy = if orig.h > 1e-9 { (nx.h / orig.h) * if nx.flip_v != orig.flip_v { -1.0 } else { 1.0 } - 1.0 } else { 0.0 }; + let div = if mods.alt { 2.0 } else { 1.0 }; + let hx = match handle { + 0 | 6 | 7 => -1.0, + 2..=4 => 1.0, + _ => 0.0, + }; + let hy = match handle { + 0..=2 => -1.0, + 4..=6 => 1.0, + _ => 0.0, + }; + for (tid, o) in &origs { + let target_nx = if *tid == id { + nx + } else { + let picture = self.is_picture(*tid); + let (sx, sy) = if picture && matches!(handle, 0 | 2 | 4 | 6) { + // Use the axis with the larger relative change so an unchanged + // dimension cannot cancel shrinking. Preserve each axis's flip. + let scale_x = 1.0 + sx; + let scale_y = 1.0 + sy; + let scale = if sx.abs() >= sy.abs() { scale_x.abs() } else { scale_y.abs() }; + (scale.copysign(scale_x) - 1.0, scale.copysign(scale_y) - 1.0) + } else { + (sx, sy) + }; + + let lp = Affine::rotate(o.rot.to_radians()) + * Point::new( + hx * sx * o.w / div * if o.flip_h { -1.0 } else { 1.0 }, + hy * sy * o.h / div * if o.flip_v { -1.0 } else { 1.0 }, + ); + resize_box(o, handle, Vec2::new(lp.x, lp.y), picture, mods.alt) + }; + self.set_xfrm(*tid, target_nx)?; + } } Drag::LineEnd { id, end, orig } => { let a = orig.affine(); diff --git a/crates/engine/src/tools/geometry.rs b/crates/engine/src/tools/geometry.rs index 536a4ead2..cbbd01275 100644 --- a/crates/engine/src/tools/geometry.rs +++ b/crates/engine/src/tools/geometry.rs @@ -228,7 +228,7 @@ impl Session { } /// Snap the edges that resize handle `handle` drags in the resized box `x` to the smart guides. - pub(super) fn snap_resize(&mut self, x: Xfrm, _orig: &Xfrm, handle: u8, id: ShapeId, tol: f64) -> Xfrm { + pub(super) fn snap_resize(&mut self, x: Xfrm, handle: u8, exclude: &[ShapeId], tol: f64) -> Xfrm { if x.rot != 0.0 { self.tool.guides.clear(); return x; @@ -241,7 +241,7 @@ impl Session { let (top, bottom) = (matches!(handle, 0..=2), matches!(handle, 4..=6)); let (moves_left, moves_right) = if x.flip_h { (right, left) } else { (left, right) }; let (moves_top, moves_bottom) = if x.flip_v { (bottom, top) } else { (top, bottom) }; - let (off, guides) = self.snap_edges(r, &[id], tol * 1.5, true, [moves_left, false, moves_right], [moves_top, false, moves_bottom]); + let (off, guides) = self.snap_edges(r, exclude, tol * 1.5, true, [moves_left, false, moves_right], [moves_top, false, moves_bottom]); let mut nx = x; if moves_left { nx.x += off.x; diff --git a/crates/engine/src/tools/hit.rs b/crates/engine/src/tools/hit.rs index 6a0da529f..247ab1e79 100644 --- a/crates/engine/src/tools/hit.rs +++ b/crates/engine/src/tools/hit.rs @@ -216,6 +216,7 @@ pub(super) fn in_table_cell(st: &crate::DocState, id: ShapeId, p: Point) -> bool }) } +/// Resolve a shape's target at slide point `p`, using its effective fill for interior hits. pub(super) fn hit_shape(st: &crate::DocState, sh: &Shape, p: Point, tol: f64) -> Option { let x = xfrm_of(&st.doc, &st.selection, sh); let inv = x.affine().inverse(); @@ -267,8 +268,11 @@ pub(super) fn hit_shape(st: &crate::DocState, sh: &Shape, p: Point, tol: f64) -> return None; } let has_text = matches!(sh.kind, ShapeKind::Shape) && (sh.text.as_ref().is_some_and(|t| !t.is_empty()) || sh.ph.is_some() || sh.text_box); - // Empty interiors of unfilled, text-less shapes don't catch clicks. - let filled = !matches!(sh.fill, Some(deckcraft_model::Fill::None)) || sh.style.is_some(); + // Match rendering: an explicit No Fill overrides the style, and style/theme fills + // can themselves be No Fill. Empty, text-less interiors then let clicks through. + let fill = + cmd::text::ctx_for(&st.doc, &st.selection).map(|ctx| deckcraft_model::resolve::fill(&ctx, sh).0).unwrap_or_else(|| sh.fill.clone()); + let filled = !matches!(fill, None | Some(deckcraft_model::Fill::None)); let geo = deckcraft_render::shape_geometry(sh, x.w, x.h); let in_geo = { use deckcraft_geom::KurboShape; diff --git a/crates/engine/src/tools/pointer.rs b/crates/engine/src/tools/pointer.rs index cb71cfed0..d20ae9315 100644 --- a/crates/engine/src/tools/pointer.rs +++ b/crates/engine/src/tools/pointer.rs @@ -220,6 +220,7 @@ impl Session { } } + /// Handle a pointer press and start the applicable tool interaction. fn down(&mut self, p: Point, mods: Mods, tol: f64) -> Result { if let Some(Drag::Freeform { mode, mut points, .. }) = self.tool.drag.clone() { // Next click of a freeform: close on the start point, else add a vertex. @@ -303,8 +304,12 @@ impl Session { Some(Hit::TableRow { id, row }) => self.begin_table_border(id, false, row, p)?, Some(Hit::Resize { id, handle }) => { let orig = st.shape(id).map(|s| xfrm_of(&st.doc, &st.selection, s)).unwrap_or_default(); + let ids: Vec = if st.selection.shapes.contains(&id) { st.selection.shapes.clone() } else { vec![id] }; + let origs: Vec<(ShapeId, Xfrm)> = + ids.iter().filter_map(|i| st.shape(*i).filter(|s| !s.locked).map(|s| (*i, xfrm_of(&st.doc, &st.selection, s)))).collect(); + let ids: Vec = origs.iter().map(|(id, _)| *id).collect(); self.begin("Size"); - self.tool.drag = Some(Drag::Resize { id, handle, orig, start: p }); + self.tool.drag = Some(Drag::Resize { id, orig, ids, handle, origs, start: p }); } Some(Hit::LineEnd { id, end }) => { let orig = st.shape(id).map(|s| xfrm_of(&st.doc, &st.selection, s)).unwrap_or_default(); @@ -436,6 +441,7 @@ impl Session { Ok(json!({"hit": serde_json::to_value(&hit).unwrap_or_default()})) } + /// Handle a pointer release and finalize the active tool interaction. fn up(&mut self, p: Point, mods: Mods) -> Result { let drag = self.tool.drag.take(); self.tool.guides.clear(); @@ -558,10 +564,22 @@ impl Session { out = self.add_ink(points)?; } } - Some(Drag::Resize { id, .. }) => { + Some(Drag::Resize { origs, .. }) => { + let changed: Vec = origs + .iter() + .filter_map(|(id, orig)| { + let current = self.active().and_then(|st| st.shape(*id).map(|s| xfrm_of(&st.doc, &st.selection, s)))?; + + (current != *orig).then_some(*id) + }) + .collect(); + if !changed.is_empty() { + for id in changed { + let _ = cmd::text::fit_text_box(self, id); + self.refit_after_resize(id)?; + } + } self.commit(); - let _ = cmd::text::fit_text_box(self, id); - self.refit_after_resize(id)?; } Some(Drag::TextSelect { before, .. }) => { out = self.paint_if_selection_changed(before, out)?; diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index 043b8f2dd..2e2f35299 100644 --- a/crates/fonts/src/fontdb.rs +++ b/crates/fonts/src/fontdb.rs @@ -30,12 +30,15 @@ pub fn bundled() -> Vec<&'static [u8]> { /// Open substitutes for families documents often ask for but we can't ship (metric-compatible /// where one exists), tried in order after the family itself. pub fn substitutes(family: &str) -> &'static [&'static str] { - match family.to_ascii_lowercase().as_str() { + let f = family.to_ascii_lowercase(); + if is_monospace_family(&f) { + return &["Liberation Mono", "Courier New", "Menlo", "Courier", "DejaVu Sans Mono"]; + } + match f.as_str() { "calibri" | "calibri light" => &["Carlito", "Liberation Sans", "Arial", "Inter"], "cambria" | "cambria math" => &["Caladea", "Liberation Serif", "Times New Roman", "Merriweather"], "arial" | "helvetica" | "helvetica neue" | "arial nova" => &["Liberation Sans", "Arial", "Helvetica Neue", "Helvetica", "Inter"], "times new roman" | "times" => &["Liberation Serif", "Times New Roman", "Times", "Merriweather"], - "courier new" | "courier" | "consolas" | "menlo" => &["Liberation Mono", "Courier New", "Menlo", "Courier"], "aptos" | "aptos display" | "aptos narrow" | "segoe ui" | "segoe ui light" | "segoe ui semibold" | "tahoma" | "verdana" => { &["Inter", "Helvetica Neue", "Arial", "Liberation Sans"] } @@ -52,12 +55,77 @@ pub fn substitutes(family: &str) -> &'static [&'static str] { } } +/// Families that are monospaced: when not installed, documents keep a monospaced substitute +/// ([`substitutes`]) so code blocks and columns stay aligned instead of falling back to +/// proportional Calibri. +pub fn is_monospace_family(family: &str) -> bool { + let f = family.to_ascii_lowercase(); + let s = f.trim(); + if matches!( + s, + "courier new" + | "courier" + | "cousine" + | "consolas" + | "menlo" + | "monaco" + | "source code pro" + | "jetbrains mono" + | "cascadia code" + | "cascadia code pl" + | "cascadia mono" + | "cascadia mono pl" + | "fira code" + | "fira mono" + | "inconsolata" + | "dejavu sans mono" + | "sf mono" + | "roboto mono" + | "hack" + | "ubuntu mono" + | "pt mono" + | "liberation mono" + | "andale mono" + | "lucida console" + | "lucida sans typewriter" + | "fixedsys" + | "terminal" + | "monospace" + | "iosevka" + | "monolisa" + | "envy code r" + | "hasklig" + ) { + return true; + } + s.starts_with("courier") + || s.starts_with("consolas") + || s.starts_with("inconsolata") + || s.starts_with("iosevka") + || s.ends_with(" mono") + || s.ends_with(" code") + || s.ends_with(" console") + || s.ends_with(" typewriter") + || s.ends_with(" monospace") + || s.split_whitespace().any(|w| w == "mono" || w == "terminal" || w == "monospace") +} + /// Families whose open substitutes ([`substitutes`]) keep their metrics, so text drawn with /// them breaks into the same lines. fn has_metric_twin(family: &str) -> bool { matches!( family.to_ascii_lowercase().as_str(), - "calibri" | "calibri light" | "cambria" | "cambria math" | "arial" | "helvetica" | "times new roman" | "times" | "courier new" | "courier" + "calibri" + | "calibri light" + | "cambria" + | "cambria math" + | "arial" + | "helvetica" + | "times new roman" + | "times" + | "courier new" + | "courier" + | "cousine" ) } @@ -1080,6 +1148,7 @@ impl FontDb { /// when it is installed or bundled, or a metric-compatible stand-in (Carlito for Calibri, /// Liberation Sans for Arial…), or the family PowerPoint maps it to (Liberation Sans as Arial, /// see `office_twin`); any other missing family is drawn in Calibri (or Carlito). + /// Monospaced families keep a monospaced substitute instead of falling back to Calibri. /// DeckCraft's own default family keeps [`FontDb::face`]'s fallbacks. pub fn document_face(&self, family: &str, style: &str) -> Arc { if family.eq_ignore_ascii_case(FALLBACK_FAMILY) || has_metric_twin(family) || is_office_cloud_font(family) || self.has_family(family) { @@ -1088,6 +1157,9 @@ impl FontDb { if let Some(twin) = office_twin(family) { return self.face(twin, style); } + if is_monospace_family(family) { + return self.face(family, style); + } self.face(MISSING_FONT_FAMILY, style) } diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index 92d2cb44d..0bb0edb50 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -15,7 +15,8 @@ mod shape; pub use fallback::{bundled_families, covers_cluster, is_emoji_cluster, ligature_carets}; pub use fontdb::{ - FALLBACK_FAMILY, FaceRef, FontDb, FontFace, LAST_RESORT_FAMILY, MISSING_FONT_FAMILY, base_style, bundled, substitutes, system_font_dirs, + FALLBACK_FAMILY, FaceRef, FontDb, FontFace, LAST_RESORT_FAMILY, MISSING_FONT_FAMILY, base_style, bundled, is_monospace_family, substitutes, + system_font_dirs, }; pub use harfrust::Feature; use harfrust::Tag; diff --git a/crates/fonts/src/shape.rs b/crates/fonts/src/shape.rs index 5c88055ac..91ceef3dc 100644 --- a/crates/fonts/src/shape.rs +++ b/crates/fonts/src/shape.rs @@ -37,56 +37,146 @@ pub struct ShapeParams<'a> { pub language: Option<&'a str>, } +/// Most bytes of text given to the shaper at once. The shaper lets one call grow its buffers to +/// 256 glyphs per character (a font's substitutions decide how many), so a long item is shaped in +/// pieces of this size, each with the rest of the paragraph as context. +const MAX_SHAPE_CHUNK: usize = 1024; +/// Most glyphs kept per character of a piece: a piece the font expands further is laid out one +/// glyph per character from the cmap instead. +const MAX_GLYPHS_PER_CHAR: usize = 32; + /// Shape `para[range]` with `face`, the rest of `para` as context. Glyphs are in visual order /// (left to right) and clusters are byte offsets into `para`. `map` substitutes characters (All /// Caps) without moving clusters. An empty or out-of-bounds range shapes nothing. pub fn shape_range(face: &FontFace, para: &str, range: Range, params: &ShapeParams, map: impl Fn(char) -> char) -> Vec { - let (Some(pre), Some(text), Some(post)) = (para.get(..range.start), para.get(range.clone()), para.get(range.end..)) else { + let (Some(_), Some(text), Some(_)) = (para.get(..range.start), para.get(range.clone()), para.get(range.end..)) else { return Vec::new(); }; - let mut out = Vec::with_capacity(text.len()); + let chunks = chunks(text, MAX_SHAPE_CHUNK); + let mut pieces: Vec> = Vec::with_capacity(chunks.len()); let shaped = face.hb().map(|hb| { let shaper = face.shaper.shaper(&hb).instance(face.instance.as_ref()).build(); - let mut buf = UnicodeBuffer::new(); - buf.set_pre_context(pre); - buf.set_post_context(post); - for (i, c) in text.char_indices() { - buf.add(map(c), (range.start + i) as u32); - } - buf.guess_segment_properties(); - if let Some(s) = params.script.and_then(|t| Script::from_iso15924_tag(Tag::new(&t))) { - buf.set_script(s); - } - if let Some(l) = params.language.or_else(|| params.script.and_then(crate::script::default_language)).and_then(|l| l.parse::().ok()) - { - buf.set_language(l); - } - buf.set_direction(if params.rtl { Direction::RightToLeft } else { Direction::LeftToRight }); - let gb = shaper.shape(buf, ShapeOptions::new().features(params.features)); - for (info, pos) in gb.glyph_infos().iter().zip(gb.glyph_positions()) { - out.push(ShapedGlyph { - gid: info.glyph_id, - cluster: info.cluster as usize, - x_advance: pos.x_advance, - x_offset: pos.x_offset, - y_offset: pos.y_offset, - }); + // Pieces of one item share the script the whole item would be guessed to have. + let item_script = (chunks.len() > 1 && params.script.is_none()).then(|| { + let mut probe = UnicodeBuffer::new(); + for c in text.chars() { + probe.add(map(c), 0); + } + probe.guess_segment_properties(); + probe.script() + }); + for c in &chunks { + let r = (range.start + c.start)..(range.start + c.end); + let (Some(pre), Some(piece), Some(post)) = (para.get(..r.start), para.get(r.clone()), para.get(r.end..)) else { continue }; + let mut buf = UnicodeBuffer::new(); + buf.set_pre_context(pre); + buf.set_post_context(post); + for (i, c) in piece.char_indices() { + buf.add(map(c), (r.start + i) as u32); + } + buf.guess_segment_properties(); + if let Some(s) = params.script.and_then(|t| Script::from_iso15924_tag(Tag::new(&t))) { + buf.set_script(s); + } else if let Some(s) = item_script { + buf.set_script(s); + } + if let Some(l) = + params.language.or_else(|| params.script.and_then(crate::script::default_language)).and_then(|l| l.parse::().ok()) + { + buf.set_language(l); + } + buf.set_direction(if params.rtl { Direction::RightToLeft } else { Direction::LeftToRight }); + let gb = shaper.shape(buf, ShapeOptions::new().features(params.features)); + let budget = piece.chars().count().saturating_mul(MAX_GLYPHS_PER_CHAR); + if gb.len() > budget { + // Font-controlled expansion past anything real text needs: keep it bounded. + pieces.push(cmap_glyphs(face, para, r, params.rtl, &map)); + continue; + } + pieces.push( + gb.glyph_infos() + .iter() + .zip(gb.glyph_positions()) + .map(|(info, pos)| ShapedGlyph { + gid: info.glyph_id, + cluster: info.cluster as usize, + x_advance: pos.x_advance, + x_offset: pos.x_offset, + y_offset: pos.y_offset, + }) + .collect(), + ); } }); - if shaped.is_none() - && let Some(f) = face.skrifa() - { + if shaped.is_none() { // No shaper for this face: one glyph per character from the cmap, in visual order. - let cmap = f.charmap(); - let gm = f.glyph_metrics(Size::unscaled(), face.location()); - for (i, c) in text.char_indices() { - let g = cmap.map(map(c)).unwrap_or_default(); - let adv = gm.advance_width(g).unwrap_or(face.upem as f32 * 0.5); - out.push(ShapedGlyph { gid: g.to_u32(), cluster: range.start + i, x_advance: adv.round() as i32, x_offset: 0, y_offset: 0 }); - } - if params.rtl { - out.reverse(); + return cmap_glyphs(face, para, range, params.rtl, &map); + } + // Each piece is in visual order; right to left, the later pieces come first. + if params.rtl { + pieces.reverse(); + } + let mut out = Vec::with_capacity(pieces.iter().map(Vec::len).sum()); + for p in pieces { + out.extend(p); + } + out +} + +/// One glyph per character of `para[range]` from the cmap, in visual order. +fn cmap_glyphs(face: &FontFace, para: &str, range: Range, rtl: bool, map: &impl Fn(char) -> char) -> Vec { + let (Some(text), Some(f)) = (para.get(range.clone()), face.skrifa()) else { return Vec::new() }; + let cmap = f.charmap(); + let gm = f.glyph_metrics(Size::unscaled(), face.location()); + let mut out = Vec::with_capacity(text.len()); + for (i, c) in text.char_indices() { + let g = cmap.map(map(c)).unwrap_or_default(); + let adv = gm.advance_width(g).unwrap_or(face.upem as f32 * 0.5); + out.push(ShapedGlyph { gid: g.to_u32(), cluster: range.start + i, x_advance: adv.round() as i32, x_offset: 0, y_offset: 0 }); + } + if rtl { + out.reverse(); + } + out +} + +/// `text` cut into byte ranges of at most `max` bytes (or one character, if longer), at the last +/// space of a piece where there is one, else before a character that starts a cluster (not a +/// combining mark or joiner, not after a joiner). Text that fits is one piece. +fn chunks(text: &str, max: usize) -> Vec> { + use unicode_bidi::BidiClass::{BN, NSM}; + let mut out = Vec::new(); + let mut start = 0; + while text.len() - start > max { + let window = text.get(start..).unwrap_or_default(); + let mut space = None; + let mut boundary = None; + let mut any = None; + let mut prev: Option = None; + for (i, c) in window.char_indices() { + if i > max { + break; + } + if i > 0 { + any = Some(i); + let joined = prev.is_some_and(|p| matches!(unicode_bidi::bidi_class(p), BN)); + if !joined && !matches!(unicode_bidi::bidi_class(c), NSM | BN) { + boundary = Some(i); + if prev.is_some_and(char::is_whitespace) { + space = Some(i); + } + } + } + prev = Some(c); } + // A piece always moves forward by at least one character. + let first = window.chars().next().map_or(window.len(), char::len_utf8); + let cut = space.filter(|&i| i >= max / 2).or(boundary).or(any).unwrap_or(first).max(first); + out.push(start..start + cut); + start += cut; + } + if start < text.len() || out.is_empty() { + out.push(start..text.len()); } out } @@ -121,4 +211,37 @@ mod tests { // Not a char boundary. assert!(shape_range(&face, "سنة", 1..2, &ShapeParams::default(), |c| c).is_empty()); } + + #[test] + fn long_items_are_shaped_in_bounded_pieces() { + for text in ["word ".repeat(2_000), "e\u{301}".repeat(2_000), "x".repeat(5_000), "\u{1f469}\u{200d}\u{1f4bb} ".repeat(500)] { + let pieces = chunks(&text, MAX_SHAPE_CHUNK); + assert_eq!(pieces.first().map(|r| r.start), Some(0)); + assert_eq!(pieces.last().map(|r| r.end), Some(text.len())); + for w in pieces.windows(2) { + assert_eq!(w[0].end, w[1].start); + } + for r in &pieces { + assert!(!r.is_empty() && r.len() <= MAX_SHAPE_CHUNK); + // Never before a combining mark, never after a joiner. + let rest = &text[r.end..]; + assert!(!rest.starts_with('\u{301}')); + assert!(!text[..r.end].ends_with('\u{200d}') || rest.is_empty()); + } + } + assert_eq!(chunks("short", MAX_SHAPE_CHUNK), vec![0..5]); + assert_eq!(chunks("", MAX_SHAPE_CHUNK), vec![0..0]); + } + + #[test] + fn long_items_keep_every_cluster_in_visual_order() { + let face = FontDb::global().face(DEFAULT_FAMILY, "Regular"); + let text = "ab cd ".repeat(1_000); + let g = shape_range(&face, &text, 0..text.len(), &ShapeParams::default(), |c| c); + assert_eq!(g.len(), text.len()); + assert!(g.windows(2).all(|w| w[0].cluster < w[1].cluster)); + let rtl = shape_range(&face, &text, 0..text.len(), &ShapeParams { rtl: true, ..Default::default() }, |c| c); + assert_eq!(rtl.len(), text.len()); + assert!(rtl.windows(2).all(|w| w[0].cluster > w[1].cluster)); + } } diff --git a/crates/fonts/src/tests.rs b/crates/fonts/src/tests.rs index a6fca4c4d..596415d51 100644 --- a/crates/fonts/src/tests.rs +++ b/crates/fonts/src/tests.rs @@ -216,9 +216,13 @@ fn missing_document_fonts_are_drawn_in_calibri() { assert_eq!(f.family, db.face(MISSING_FONT_FAMILY, "Bold").family, "{missing:?}"); } // PowerPoint's own twins and Windows' substitutes, when the font itself is missing. - for (missing, twin) in - [("Liberation Sans", "Arial"), ("Liberation Serif", "Times New Roman"), ("Arial CE", "Arial"), ("Times New Roman CYR", "Times New Roman")] - { + for (missing, twin) in [ + ("Liberation Sans", "Arial"), + ("Liberation Serif", "Times New Roman"), + ("Arial CE", "Arial"), + ("Times New Roman CYR", "Times New Roman"), + ("Courier New CE", "Courier New"), + ] { if !db.has_family(missing) { assert_eq!(db.document_face(missing, "Regular").family, db.face(twin, "Regular").family, "{missing}"); } @@ -237,6 +241,73 @@ fn office_fonts_have_substitutes() { assert!(substitutes("Times New Roman").contains(&"Liberation Serif")); assert!(substitutes("Aptos").contains(&"Inter")); assert!(substitutes("zzz").is_empty()); + + for mono in [ + "Courier New", + "Cousine", + "Consolas", + "Menlo", + "Monaco", + "Source Code Pro", + "JetBrains Mono", + "Cascadia Code", + "Cascadia Code PL", + "Cascadia Mono", + "Fira Code", + "Inconsolata", + "DejaVu Sans Mono", + "Roboto Mono", + "Hack", + "Ubuntu Mono", + "Custom Monospace", + "Fantasque Sans Mono", + ] { + assert!(substitutes(mono).contains(&"Liberation Mono"), "{mono} should have Liberation Mono substitute"); + assert!(is_monospace_family(mono), "{mono} should be recognized as monospace"); + } + + for prop in ["Calibri", "Arial", "Times New Roman", "Inter", "Monotype Corsiva", "Monoton", "Monotype Baskerville", "Code Pro", "zzz"] { + assert!(!is_monospace_family(prop), "{prop} should not be recognized as monospace"); + } + + // Explicit checks requested in review: Consolas, Courier New, Fira Code, Monotype Corsiva, Monoton, + // and ensuring proportional families maintain their unchanged substitutes. + assert!(is_monospace_family("Consolas")); + assert!(is_monospace_family("Courier New")); + assert!(is_monospace_family("Fira Code")); + assert!(!is_monospace_family("Monotype Corsiva")); + assert!(!is_monospace_family("Monoton")); + + assert_eq!(substitutes("Calibri"), &["Carlito", "Liberation Sans", "Arial", "Inter"]); + assert_eq!(substitutes("Arial"), &["Liberation Sans", "Arial", "Helvetica Neue", "Helvetica", "Inter"]); + assert_eq!(substitutes("Times New Roman"), &["Liberation Serif", "Times New Roman", "Times", "Merriweather"]); + assert_eq!(substitutes("Georgia"), &["Merriweather", "Liberation Serif", "Georgia"]); + assert!(substitutes("Monotype Corsiva").is_empty()); + assert!(substitutes("Monoton").is_empty()); +} + +#[test] +fn missing_monospace_fonts_keep_monospace_fallback() { + let db = FontDb::global(); + let mono_sub = db.face("Unavailable JetBrains Mono", "Regular").family.clone(); + let calibri_target = db.face(MISSING_FONT_FAMILY, "Regular").family.clone(); + + assert!(is_monospace_family(&mono_sub), "substitute {mono_sub} must be monospace"); + + // Cousine is metric-compatible with Courier New / Liberation Mono and must not fall back to Calibri. + if !db.has_family("Cousine") { + assert_eq!(db.document_face("Cousine", "Regular").family, mono_sub); + } + + // Monospace fonts missing from the system must resolve to a monospace substitute. + for mono in ["Unavailable JetBrains Mono", "Custom App Code", "Unknown Space Mono", "Mock Terminal Font"] { + let f = db.document_face(mono, "Regular"); + assert_eq!(f.family, mono_sub, "{mono}"); + assert!(is_monospace_family(&f.family), "{mono} must resolve to a monospace family"); + if mono_sub != calibri_target { + assert_ne!(f.family, calibri_target, "{mono} must not fall back to Calibri"); + } + } } #[test] diff --git a/crates/geom/src/lib.rs b/crates/geom/src/lib.rs index 3be72e03f..a08bed987 100644 --- a/crates/geom/src/lib.rs +++ b/crates/geom/src/lib.rs @@ -71,7 +71,10 @@ impl Unit { /// `2.08"` style text for a length in points. pub fn format(self, pt: f64) -> String { let v = pt / self.points_per_unit(); - let s = format!("{v:.2}"); + let mut s = format!("{v:.2}"); + if s.starts_with('-') && s[1..].bytes().all(|b| b == b'0' || b == b'.') { + s.remove(0); + } format!("{s}{}", self.suffix()) } /// Parses `2.5`, `2.5"`, `2.5 in`, `6 cm`, `30 pt`, `40px`, `5mm` into points. A bare number uses `self`. @@ -153,8 +156,8 @@ impl Xfrm { } /// Rotation normalised to 0..360. pub fn rot_norm(&self) -> f64 { - let r = self.rot % 360.0; - if r < 0.0 { r + 360.0 } else { r } + let r = self.rot.rem_euclid(360.0); + if r >= 360.0 { 0.0 } else { r } } } @@ -222,6 +225,14 @@ pub fn preset_flatten(path: &BezPath, f: &mut dyn FnMut(PathEl)) { mod tests { use super::*; + #[test] + fn format_has_no_negative_zero() { + assert_eq!(Unit::Points.format(-0.001), "0.00 pt"); + assert_eq!(Unit::Inches.format(-0.001), "0.00\""); + assert!(!Unit::Centimeters.format(-0.01).starts_with('-')); + assert_eq!(Unit::Points.format(-1.5), "-1.50 pt"); + } + #[test] fn emu_roundtrip_is_exact() { for emu in [0i64, 1, 12_699, 12_700, 914_400, 12_192_000, 6_858_000, -5, 123_456_789] { @@ -266,6 +277,14 @@ mod tests { assert!((p.x - 300.0).abs() < 1e-9 && (p.y - 200.0).abs() < 1e-9); } + #[test] + fn rot_norm_stays_below_360() { + for rot in [-1e-15, -1e-13, -f64::EPSILON, -0.0, 360.0, 720.0, -360.0] { + let n = Xfrm { rot, ..Default::default() }.rot_norm(); + assert!((0.0..360.0).contains(&n), "rot {rot} -> {n}"); + } + } + #[test] fn segment_distance() { assert!((dist_to_segment(Point::new(5.0, 5.0), Point::new(0.0, 0.0), Point::new(10.0, 0.0)) - 5.0).abs() < 1e-9); diff --git a/crates/h264/src/slicedec.rs b/crates/h264/src/slicedec.rs index 17fb8ba7a..058ccfe16 100644 --- a/crates/h264/src/slicedec.rs +++ b/crates/h264/src/slicedec.rs @@ -566,6 +566,19 @@ impl<'a> SliceDecoder<'a> { (refs, mvs, false) } + /// Temporal direct mvs (8.4.1.2.3) from the co-located mv and `DistScaleFactor` (clamped + /// to ±1024 in `SliceDecoder`): mvL0 = clip_mv((mvCol · dsf + 128) >> 8), mvL1 = + /// clip_mv(mvL0 − mvCol), both clipped to the spec's motion-vector range −2^15..2^15−1. + /// The i32 maths cannot overflow; a truncating `as i16` used to wrap mvL0 past the range, + /// after which the mvL1 subtraction overflowed a debug build. mvL1 is scaled from the + /// unclamped mvL0 with only the results clipped, as the sibling SoundCraft decoder's + /// review established (soundcraft#994): dsf 512, mvCol 32767 keeps mvL1 at 32767. + fn temporal_direct_mvs(dsf: i32, mv_col: [i16; 2]) -> ([i16; 2], [i16; 2]) { + let clamp = |v: i32| v.clamp(-(1 << 15), (1 << 15) - 1) as i16; + let m0 = mv_col.map(|c| (dsf * i32::from(c) + 128) >> 8); + (m0.map(clamp), [clamp(m0[0] - i32::from(mv_col[0])), clamp(m0[1] - i32::from(mv_col[1]))]) + } + /// Derive direct-mode motion for the 8x8 blocks in `b8_mask` of the current MB. fn derive_direct(&mut self, b8_mask: u8) -> Result<()> { let spatial = self.sh.direct_spatial_mv_pred; @@ -600,13 +613,7 @@ impl<'a> SliceDecoder<'a> { } }; let (dsf, ltr) = self.tdirect.get(ref0 as usize).copied().unwrap_or((256, true)); - let (mv0, mv1) = if ltr { - (mv_col, [0, 0]) - } else { - let s = |c: i16| ((dsf * c as i32 + 128) >> 8) as i16; - let m0 = [s(mv_col[0]), s(mv_col[1])]; - (m0, [m0[0] - mv_col[0], m0[1] - mv_col[1]]) - }; + let (mv0, mv1) = if ltr { (mv_col, [0, 0]) } else { Self::temporal_direct_mvs(dsf, mv_col) }; ([ref0, 0], [mv0, mv1]) }; self.set_motion(x, y, unit, unit, refs, mvs); @@ -1481,3 +1488,33 @@ impl<'a> SliceDecoder<'a> { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::SliceDecoder; + + /// Temporal direct mvs stay inside the spec's motion-vector range −2^15..2^15−1 with a + /// large `DistScaleFactor` and a co-located mv at an i16 extreme: the scale used to + /// truncate past the range with `as i16`, and mvL0 − mvCol then overflowed a debug build + /// ("attempt to subtract with overflow"). `dsf` 256 is the identity scale. + #[test] + fn temporal_direct_mvs_at_i16_extremes_stay_in_the_spec_range() { + for dsf in [-1024i32, -1, 256, 1023] { + for col in [[i16::MIN, i16::MIN], [i16::MAX, i16::MAX], [i16::MIN, i16::MAX]] { + let (mv0, mv1) = SliceDecoder::temporal_direct_mvs(dsf, col); + for mv in [mv0, mv1] { + for c in mv { + assert!((-(1 << 15)..(1 << 15)).contains(&i32::from(c)), "dsf {dsf}, col {col:?}: {mv:?}"); + } + } + if dsf == 256 { + assert_eq!((mv0, mv1), (col, [0, 0]), "dsf 256 is the identity scale"); + } + } + } + // soundcraft#994: mvL1 is scaled from the unclamped mvL0 with only the results clipped; + // deriving it from the clipped mvL0 gave [0, 0] here. + let (mv0, mv1) = SliceDecoder::temporal_direct_mvs(512, [i16::MAX, 0]); + assert_eq!((mv0, mv1), ([i16::MAX, 0], [i16::MAX, 0]), "mvL1 keeps mvL0's off-range part"); + } +} diff --git a/crates/model/src/edit.rs b/crates/model/src/edit.rs index d635c052e..3e21e3bf8 100644 --- a/crates/model/src/edit.rs +++ b/crates/model/src/edit.rs @@ -469,6 +469,9 @@ pub fn change_case(s: &str, mode: &str) -> String { /// piece is the changed form of the matching input piece, even where a character changes length. pub fn change_case_parts(parts: &[&str], mode: &str) -> Vec { let mut start = true; + // All characters of all pieces, so word-final sigma sees neighbours across piece boundaries. + let all: Vec = parts.iter().flat_map(|s| s.chars()).collect(); + let mut base = 0; parts .iter() .map(|s| match mode { @@ -479,10 +482,14 @@ pub fn change_case_parts(parts: &[&str], mode: &str) -> Vec { // `title` restarts after whitespace, sentence case after sentence punctuation. let title = mode == "title"; let mut out = String::new(); - for c in s.chars() { + for (j, c) in s.chars().enumerate() { + let i = base + j; if start && c.is_alphabetic() { out.extend(c.to_uppercase()); start = false; + } else if c == 'Σ' && i > 0 && all[i - 1].is_alphabetic() && !all.get(i + 1).is_some_and(|n| n.is_alphabetic()) { + // Word-final capital sigma lowercases to ς, as `str::to_lowercase` does. + out.push('ς'); } else { out.extend(c.to_lowercase()); } @@ -490,6 +497,7 @@ pub fn change_case_parts(parts: &[&str], mode: &str) -> Vec { start = true; } } + base += s.chars().count(); out } }) @@ -504,6 +512,20 @@ mod tests { TextBody::from_text(t) } + #[test] + fn title_and_sentence_final_sigma() { + assert_eq!(change_case("ΟΔΟΣ", "title"), "Οδος"); + assert_eq!(change_case("ΟΔΟΣ ΜΟΥ.", "sentence"), "Οδος μου."); + assert_eq!(change_case("ΟΔΟΣ", "lower"), "οδος"); + } + + #[test] + fn title_sigma_across_runs_is_not_final() { + assert_eq!(change_case_parts(&["ΟΔΟΣ", "ΑΣ"], "title"), ["Οδοσ", "ας"]); + assert_eq!(change_case_parts(&["ΟΔΟ", "ΣΑ"], "title"), ["Οδο", "σα"]); + assert_eq!(change_case_parts(&["ΟΔΟΣ", " ΜΟΥ"], "title"), ["Οδος", " Μου"]); + } + #[test] fn insert_and_split() { let mut b = body("Hello"); diff --git a/crates/model/src/style.rs b/crates/model/src/style.rs index e2d70b994..3b81d3ae5 100644 --- a/crates/model/src/style.rs +++ b/crates/model/src/style.rs @@ -55,10 +55,12 @@ impl ColorRef { }; deckcraft_color::apply(base, &self.mods) } - /// Alpha (0–1) from an `alpha` transform, if any. + /// Alpha (0–1) from the `alpha`, `alphaMod` and `alphaOff` transforms, applied in order. pub fn alpha(&self) -> f64 { self.mods.iter().fold(1.0, |a, m| match m { ColorTransform::Alpha(v) => (*v as f64 / 100_000.0).clamp(0.0, 1.0), + ColorTransform::AlphaMod(v) => (a * *v as f64 / 100_000.0).clamp(0.0, 1.0), + ColorTransform::AlphaOff(v) => (a + *v as f64 / 100_000.0).clamp(0.0, 1.0), _ => a, }) } diff --git a/crates/model/src/tests.rs b/crates/model/src/tests.rs index 1fd29aeff..4ae6fc0fd 100644 --- a/crates/model/src/tests.rs +++ b/crates/model/src/tests.rs @@ -327,3 +327,15 @@ fn only_web_and_mail_links_are_safe_to_open() { assert!(!safe_url(url), "{url}"); } } + +#[test] +fn colour_alpha_follows_alpha_mod_and_off() { + use deckcraft_color::{ColorTransform, Rgba}; + let base = || ColorRef::rgb(Rgba::rgb(1, 2, 3)); + let mod_half = base().with(ColorTransform::AlphaMod(50000)); + assert!((mod_half.alpha() - 0.5).abs() < 1e-9, "{}", mod_half.alpha()); + let off = base().with(ColorTransform::AlphaOff(-50000)); + assert!((off.alpha() - 0.5).abs() < 1e-9, "{}", off.alpha()); + let both = base().with(ColorTransform::Alpha(40000)).with(ColorTransform::AlphaMod(50000)); + assert!((both.alpha() - 0.2).abs() < 1e-9, "{}", both.alpha()); +} diff --git a/crates/model/src/text.rs b/crates/model/src/text.rs index 362f43939..ca992c38a 100644 --- a/crates/model/src/text.rs +++ b/crates/model/src/text.rs @@ -242,6 +242,8 @@ pub struct ParaProps { pub bullet_color_follow_text: 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 bullet_size_follow_text: Option, pub tabs: Option>, pub default_tab: Option, @@ -286,11 +288,13 @@ impl ParaProps { self.bullet_color.clone_from(&p.bullet_color); } } - if self.bullet_size_follow_text == Some(false) || self.bullet_size.is_some() { + 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_follow_text.is_none() { - if p.bullet_size_follow_text == Some(true) { + if self.bullet_size.is_none() && self.bullet_size_points.is_none() && self.bullet_size_follow_text.is_none() { + if p.bullet_size_points.is_some() { + self.bullet_size_points = p.bullet_size_points; + } else if p.bullet_size_follow_text == Some(true) { self.bullet_size_follow_text = Some(true); } else { self.bullet_size = p.bullet_size; @@ -310,6 +314,33 @@ 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)); + + let parent = ParaProps { bullet_size_points: Some(12.0), ..Default::default() }; + let mut child = ParaProps { bullet_size_follow_text: Some(true), ..Default::default() }; + child.inherit(&parent); + assert_eq!(child.bullet_size, None); + assert_eq!(child.bullet_size_points, None); + assert_eq!(child.bullet_size_follow_text, Some(true)); + } +} + #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub enum Caps { diff --git a/crates/model/src/words.rs b/crates/model/src/words.rs index 54b23659f..bcf662e22 100644 --- a/crates/model/src/words.rs +++ b/crates/model/src/words.rs @@ -90,11 +90,16 @@ fn base_class(c: char) -> Class { c if c.is_alphanumeric() => Class::Word, c if is_symbol(c) => Class::Symbol, // A combining mark with no base (or any other mark) stays with letters. - '\u{300}'..='\u{36f}' | '\u{1ab0}'..='\u{1aff}' | '\u{1dc0}'..='\u{1dff}' | '\u{20d0}'..='\u{20ff}' | '\u{fe20}'..='\u{fe2f}' => Class::Word, + c if is_combining_mark(c) => Class::Word, _ => Class::Punct, } } +/// Whether `c` is a combining mark (a decomposed accent), which belongs to the letter before it. +pub fn is_combining_mark(c: char) -> bool { + matches!(c, '\u{300}'..='\u{36f}' | '\u{1ab0}'..='\u{1aff}' | '\u{1dc0}'..='\u{1dff}' | '\u{20d0}'..='\u{20ff}' | '\u{fe20}'..='\u{fe2f}') +} + fn clusters(text: &str) -> Vec { let mut out: Vec = Vec::new(); let mut firsts: Vec = Vec::new(); diff --git a/crates/pptx/src/lib.rs b/crates/pptx/src/lib.rs index 1b1dc5b46..be08c680f 100644 --- a/crates/pptx/src/lib.rs +++ b/crates/pptx/src/lib.rs @@ -14,6 +14,7 @@ mod maps; mod opc; mod png; pub(crate) mod read; +pub use read::MAX_TAG_BYTES; mod tables; mod write; mod xml; diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 917aa9277..288e497f8 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -40,6 +40,12 @@ pub struct Imp<'a> { pub kept_sidecar_bytes: usize, /// Cached kept sidecars by normalized chart part name. pub kept_charts: HashMap, + /// Add-in tags parts, parsed once each, by normalized part name. + pub tags_by_part: HashMap>, + /// Bytes of tags copied into the model so far (see [`MAX_TAG_BYTES`]). + pub tag_bytes: usize, + /// The tag budget ran out and the warning was given. + pub tags_capped: bool, } impl Imp<'_> { @@ -196,16 +202,64 @@ fn choice_has_model3d(el: &El, rels: &Rels) -> bool { rec(el, rels, 0) } +/// Bytes of tags one import copies into the model, every tags part and reference together: a +/// small compressed tags part listed by many shapes, or many times by one, must not grow without +/// bound (#1164). The package budgets (`opc::MAX_XML_PART`, `MAX_TOTAL`, #1283) bound what is +/// parsed; this bounds what is copied from it. +pub const MAX_TAG_BYTES: usize = 64 << 20; +/// What each copied pair costs besides its text, so empty pairs count too. +const TAG_PAIR_COST: usize = std::mem::size_of::<(String, String)>(); + /// Add-in tags named by `holder`'s `p:custDataLst`: the name/value pairs of each tags part. -pub fn tags(imp: &Imp, part: &Part, holder: &El) -> Vec<(String, String)> { +/// Each part is parsed once per import and counted once per holder. Once [`MAX_TAG_BYTES`] is +/// reached, no further tag parts are parsed or cached (#1164). +pub fn tags(imp: &mut Imp, part: &Part, holder: &El) -> Vec<(String, String)> { + if imp.tags_capped { + return vec![]; + } let Some(list) = holder.child("custDataLst") else { return vec![] }; let mut out = Vec::new(); + let mut seen = HashSet::new(); for t in list.children_named("tags").take(100) { - let Some(doc) = t.attr("r:id").and_then(|id| part.rels.get(id)).and_then(|r| imp.pkg.xml(&r.target)) else { continue }; - for tag in doc.root.children_named("tag").take(10_000) { - if let Some(name) = tag.attr("name") { - out.push((name.to_string(), tag.attr("val").unwrap_or("").to_string())); + if imp.tags_capped { + return out; + } + let Some(target) = t.attr("r:id").and_then(|id| part.rels.get(id)).map(|r| opc::norm(&r.target)) else { continue }; + if !seen.insert(target.clone()) { + continue; + } + let parsed = match imp.tags_by_part.get(&target) { + Some(p) => p.clone(), + None => { + if imp.tags_capped { + return out; + } + let p: Arc<[(String, String)]> = imp + .pkg + .xml(&target) + .map(|doc| { + doc.root + .children_named("tag") + .take(10_000) + .filter_map(|tag| Some((tag.attr("name")?.to_string(), tag.attr("val").unwrap_or("").to_string()))) + .collect() + }) + .unwrap_or_default(); + imp.tags_by_part.insert(target, p.clone()); + p } + }; + for (name, val) in parsed.iter() { + let n = imp.tag_bytes.saturating_add(TAG_PAIR_COST + name.len() + val.len()); + if n > MAX_TAG_BYTES { + if !imp.tags_capped { + log::warn!("pptx: add-in tags over {MAX_TAG_BYTES} bytes; the rest are dropped"); + imp.tags_capped = true; + } + return out; + } + imp.tag_bytes = n; + out.push((name.clone(), val.clone())); } } out @@ -389,6 +443,9 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< vml_previews: HashMap::new(), kept_sidecar_bytes: 0, kept_charts: HashMap::new(), + tags_by_part: HashMap::new(), + tag_bytes: 0, + tags_capped: false, }; let part = Part::new(&pkg, &theme_path, &doc); let theme = theme::read_theme(&mut imp, &part, &doc.root); @@ -427,6 +484,9 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE vml_previews: HashMap::new(), kept_sidecar_bytes: 0, kept_charts: HashMap::new(), + tags_by_part: HashMap::new(), + tag_bytes: 0, + tags_capped: false, }; // Slide ids first so hyperlinks anywhere can point at slides. let slide_ids = register_slides(&mut imp, &docs, &slide_entries, &mut lost_slides); @@ -441,7 +501,7 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE let layout_by_part = read_masters(&mut imp, &mut p, &master_paths, &docs); let file_slide_ids = read_slides(&mut imp, &mut p, &pres_part, &docs, &slide_ids, &layout_by_part); read_sections(&mut p, &pres_part, pres_el, &file_slide_ids); - p.tags = tags(&imp, &pres_part, pres_el); + p.tags = tags(&mut imp, &pres_part, pres_el); read_custom_shows(&mut p, &imp, &pres_part, pres_el); read_guides_and_show(&mut p, &imp, &pres_part); read_notes_master(&mut imp, &mut p, &pres_part); diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index a2ed6abe7..300773a57 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -23,6 +23,16 @@ 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) +} + +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) @@ -454,9 +464,22 @@ pub fn ppr(imp: &mut Imp, part: &Part, p: &El) -> (Option, ParaProps, Option "buFont" => pp.bullet_font = e.attr("typeface").map(String::from), "buFontTx" => pp.bullet_font_follow_text = Some(true), "buClr" => pp.bullet_color = color(e), + "buSzPct" => { + pp.bullet_size_points = None; + pp.bullet_size_follow_text = None; + pp.bullet_size = bullet_size_percent(e.attr("val")).map(|v| v.clamp(0.25, 4.0)); + } + "buSzPts" => { + pp.bullet_size = None; + pp.bullet_size_follow_text = None; + pp.bullet_size_points = bullet_size_points(e.attr("val")); + } "buClrTx" => pp.bullet_color_follow_text = Some(true), - "buSzPct" => pp.bullet_size = pct(e, "val").map(|v| v.clamp(0.25, 4.0)), - "buSzTx" => pp.bullet_size_follow_text = Some(true), + "buSzTx" => { + pp.bullet_size = None; + pp.bullet_size_points = None; + pp.bullet_size_follow_text = Some(true); + } "tabLst" => { pp.tabs = Some( e.children_named("tab") @@ -692,6 +715,21 @@ 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 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)); diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 320810958..84b30c837 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -543,6 +543,7 @@ pub fn ppr(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, level: Option, p: || p.bullet_color.is_some() || p.bullet_color_follow_text.is_some() || p.bullet_size.is_some() + || p.bullet_size_points.is_some() || p.bullet_size_follow_text.is_some() || p.tabs.is_some() || def.is_some(); @@ -567,8 +568,10 @@ 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 p.bullet_size.is_none() && p.bullet_size_follow_text == Some(true) { + if p.bullet_size.is_none() && p.bullet_size_points.is_none() && p.bullet_size_follow_text == Some(true) { w.empty0("a:buSzTx"); + } else 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))); } diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index f651fc5c6..1ed97f546 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -490,6 +490,33 @@ 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: [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() + }) + .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; + 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] fn line_miter_limit_survives_two_pptx_round_trips() { let mut p = Presentation::default(); diff --git a/crates/pptx/tests/it/tags.rs b/crates/pptx/tests/it/tags.rs index e8903b442..8cc0849be 100644 --- a/crates/pptx/tests/it/tags.rs +++ b/crates/pptx/tests/it/tags.rs @@ -36,3 +36,111 @@ fn tags_survive_round_trip() { assert_eq!(first.tags, vec![t("THINKCELLSHAPEDONOTDELETE", "t!abc")]); assert!(back.slides.iter().skip(1).all(|s| s.tags.is_empty())); } + +/// #1164: the same tags part listed over and over is read once, and the tags one import copies +/// into the model are capped however often a shape lists them. +#[test] +fn a_tags_part_listed_many_times_is_copied_once_and_capped() { + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + for _ in 0..6 { + s.execute("shape.insert", &serde_json::json!({"preset": "rect", "rect": [10, 10, 50, 50]})).expect("shape"); + } + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + // A quarter of the budget with its name and pair cost: the fifth shape listing it goes over. + let pair = std::mem::size_of::<(String, String)>(); + slide.shapes[0].tags = vec![t("BIG", &"x".repeat(deckcraft_pptx::MAX_TAG_BYTES / 4 - 3 - pair))]; + let bytes = deckcraft_pptx::export(&p).expect("export"); + + // Every shape on the first slide lists that one tags part 100 times. + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip"); + let mut xml = String::new(); + zip.by_name("ppt/slides/slide1.xml").expect("slide").read_to_string(&mut xml).expect("read"); + let start = xml.find("").expect("tags listed"); + let end = xml[start..].find("").expect("list ends") + start + "".len(); + let list = &xml[start..end]; + let inner = list.trim_start_matches("").trim_end_matches(""); + let many = format!("{}", inner.repeat(100)); + let xml = xml.replace(list, "").replace("", "").replace("", &format!("{many}")); + assert!(xml.matches(&many).count() >= 5, "enough shapes to go over the budget"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + for i in 0..zip.len() { + let mut f = zip.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut data = vec![]; + f.read_to_end(&mut data).expect("read"); + out.start_file(&name, zip::write::SimpleFileOptions::default()).expect("start"); + std::io::Write::write_all(&mut out, if name == "ppt/slides/slide1.xml" { xml.as_bytes() } else { &data }).expect("write"); + } + let bytes = out.finish().expect("finish").into_inner(); + + let back = deckcraft_pptx::import(&bytes).expect("import"); + let tagged: Vec<_> = back.slides[0].shapes.iter().filter(|x| !x.tags.is_empty()).collect(); + assert_eq!(tagged.len(), 4, "four copies fit the budget"); + assert!(tagged.iter().all(|x| x.tags.len() == 1), "each shape keeps the part's tag once, not 100 times"); +} + +/// #1164: empty tags cost something too, or a part of 10,000 empty pairs listed by every shape +/// would be copied without ever reaching the budget. +#[test] +fn empty_tags_count_against_the_budget() { + let pair = std::mem::size_of::<(String, String)>(); + let per_shape = 10_000 * pair; + let shapes = deckcraft_pptx::MAX_TAG_BYTES / per_shape + 2; + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + let template = slide.shapes[0].clone(); + slide.shapes.clear(); + for i in 0..shapes { + let mut sh = template.clone(); + sh.id = deckcraft_model::ShapeId(10_000 + i as u32); + sh.ph = None; + sh.tags = vec![t("", ""); 10_000]; + slide.shapes.push(sh); + } + let bytes = deckcraft_pptx::export(&p).expect("export"); + let back = deckcraft_pptx::import(&bytes).expect("import"); + let pairs: usize = back.slides[0].shapes.iter().map(|x| x.tags.len()).sum(); + assert!(pairs * pair <= deckcraft_pptx::MAX_TAG_BYTES, "{pairs} pairs copied"); + assert!(pairs < shapes * 10_000, "the budget stopped the copying"); +} + +/// #1164: once the budget is exhausted, further tags parts are not parsed or cached into +/// `tags_by_part`. +#[test] +fn new_tag_parts_are_not_parsed_or_cached_after_the_budget_is_exhausted() { + let mut s = Session::new(); + deckcraft_engine::sample::open_sample(&mut s).expect("sample"); + s.execute("shape.insert", &serde_json::json!({"preset": "rect", "rect": [10, 10, 50, 50]})).expect("shape"); + let mut p = s.doc().expect("doc").doc.as_ref().clone(); + let slide = Arc::make_mut(&mut p.slides[0]); + assert!(slide.shapes.len() >= 2); + // The first shape's tag fits the budget, and a second pair pushes it over the cap. + let pair = std::mem::size_of::<(String, String)>(); + slide.shapes[0].tags = vec![t("BIG", &"x".repeat(deckcraft_pptx::MAX_TAG_BYTES - 3 - pair)), t("OVER", "1")]; + // The second shape has its own separate tag part. + slide.shapes[1].tags = vec![t("SHAPE2_TAG", "dropped")]; + let bytes = deckcraft_pptx::export(&p).expect("export"); + + // Corrupt the second shape's tag part (`tag2.xml`) with invalid XML in the zip. + // If it were parsed, XML loading would fail. Because the budget was capped, it is never read. + let mut zip = zip::ZipArchive::new(Cursor::new(bytes)).expect("zip"); + let mut out = zip::ZipWriter::new(Cursor::new(Vec::new())); + for i in 0..zip.len() { + let mut f = zip.by_index(i).expect("entry"); + let name = f.name().to_string(); + let mut data = vec![]; + f.read_to_end(&mut data).expect("read"); + out.start_file(&name, zip::write::SimpleFileOptions::default()).expect("start"); + let payload = if name == "ppt/tags/tag2.xml" { b" 1e-6 && tile_h > 1e-6 && tile_w.is_finite() && tile_h.is_finite()) { return; } @@ -260,6 +266,31 @@ 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) }; + 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(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(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 84c5e5a8f..e7cd5ff35 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. @@ -428,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 @@ -541,6 +550,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 +576,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); } @@ -655,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. @@ -736,6 +753,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,13 +762,14 @@ 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); 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 @@ -761,7 +780,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 +806,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); } } } @@ -1125,12 +1144,15 @@ impl Renderer { // see-through as its fill (none for an outline-only shape) … None => { if let Some(a) = fill_alpha { - let mut filled = BezPath::new(); - for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None) { - filled.extend(sp.path.iter()); - } ctx.set_paint(color(c.with_alpha(255), a)); - ctx.fill_path(&filled); + // Each subpath under its own fill rule, as the shape itself is + // drawn: one merged path filled NonZero fills the hole of an + // even-odd shape (a donut, a frame) (#1596). + for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None) { + ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero }); + ctx.fill_path(&sp.path); + } + ctx.set_fill_rule(peniko::Fill::NonZero); } } } @@ -1161,7 +1183,19 @@ impl Renderer { ctx.stroke_path(&silhouette); // A shape without a fill (an outline, a frame) glows along its line only (#225). if fill_alpha.is_some() || content.is_some() { - ctx.fill_path(&silhouette); + // Per subpath under its own fill rule, as in the shadow above: the + // merged silhouette filled NonZero fills an even-odd hole (#1596). + // When no subpath takes part the silhouette is the outline fallback. + let mut filled_any = false; + for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None || fill.is_none()) { + ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero }); + ctx.fill_path(&sp.path); + filled_any = true; + } + if !filled_any { + ctx.fill_path(&silhouette); + } + ctx.set_fill_rule(peniko::Fill::NonZero); } ctx.pop_layer(); } @@ -1180,7 +1214,20 @@ impl Renderer { } None => { if let Some(fl) = fill { - paint::fill_path(ctx, rctx, fl, ph, &silhouette, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres); + // Per subpath under its own fill rule, as in the shadow above: + // the merged silhouette filled NonZero fills an even-odd hole + // (#1596). The outline fallback covers shapes with no + // participating subpath. + let mut filled_any = false; + for sp in geo.paths.iter().filter(|sp| sp.fill != FillMode::None || fill.is_none()) { + ctx.set_fill_rule(if sp.even_odd { peniko::Fill::EvenOdd } else { peniko::Fill::NonZero }); + paint::fill_path(ctx, rctx, fl, ph, &sp.path, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres); + filled_any = true; + } + if !filled_any { + paint::fill_path(ctx, rctx, fl, ph, &silhouette, Rect::new(0.0, 0.0, w, h), mm, FillMode::Norm, 1.0, rctx.pres); + } + ctx.set_fill_rule(peniko::Fill::NonZero); } } } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index f01ee1cf8..2de2daabe 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -515,6 +515,56 @@ fn glow_of_a_shape_with_no_fill_and_no_line_is_not_drawn() { } } +#[test] +fn shadow_and_glow_keep_an_even_odd_shapes_hole() { + // #1596: a donut is one even-odd subpath — its hole exists only under the + // even-odd rule. The effects were cast from a merged silhouette filled + // NonZero, so the shadow and the glow filled the hole solid. The hole's + // centre must stay the slide's white in both, while the effects still draw. + let donut = |effects: deckcraft_model::Effects| { + let mut sh = Shape { + id: ShapeId(612), + xfrm: Some(Xfrm::new(50.0, 50.0, 200.0, 200.0)), + geom: Geom::preset("donut"), + fill: Some(Fill::solid(ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 160, 0)))), + ..Default::default() + }; + sh.effects = Some(effects); + render_slide(&deck_with(vec![sh]), 0, &RenderOpts { scale: 1.0, ..Default::default() }) + }; + let img = donut(deckcraft_model::Effects { + outer_shadow: Some(deckcraft_model::style::Shadow { + color: ColorRef::rgb(deckcraft_color::Rgba::BLACK), + blur: 8.0, + dist: 0.0, + dir: 45.0, + inner: false, + sx: 1.0, + sy: 1.0, + kx: 0.0, + ky: 0.0, + align: String::new(), + rotate_with_shape: false, + }), + ..Default::default() + }); + let hole = img.pixel(150, 150); + assert!(hole[0] > 245 && hole[1] > 245 && hole[2] > 245, "shadow in the hole: {hole:?}"); + // The shadow still falls around the shape (just below its bottom edge). + let below = img.pixel(150, 251); + assert!(below[0] < 200 && below[0] == below[2], "no shadow below: {below:?}"); + + let img = donut(deckcraft_model::Effects { + glow: Some(deckcraft_model::style::Glow { color: ColorRef::rgb(deckcraft_color::Rgba::rgb(255, 0, 0)), radius: 8.0 }), + ..Default::default() + }); + let hole = img.pixel(150, 150); + assert!(hole[0] > 245 && hole[1] > 245 && hole[2] > 245, "glow in the hole: {hole:?}"); + // The ring itself is the shape's orange. + let ring = img.pixel(150, 75); + assert!(ring[0] > 200 && ring[1] > 100 && ring[2] < 100, "ring: {ring:?}"); +} + #[test] fn placeholder_prompts_only_in_edit_view() { let p = Presentation::default(); diff --git a/crates/render/src/tests_effect_holes.rs b/crates/render/src/tests_effect_holes.rs new file mode 100644 index 000000000..fdddf60fd --- /dev/null +++ b/crates/render/src/tests_effect_holes.rs @@ -0,0 +1,196 @@ +//! Contributor-original pixel goldens for #1596 and the review of #1626. +//! The fixed samples cover background, hole/centre, ring, visible halos and reflected coverage. + +use super::{Fill, Image, RenderOpts, Rgba, render_slide}; +use deckcraft_model::style::{ColorRef, Effects, Glow, Line, Reflection, Shadow}; +use deckcraft_model::{Geom, Presentation, Shape, ShapeId, Xfrm}; + +const PROBES: [(u32, u32); 9] = [ + (20, 20), // Clear background. + (130, 130), // Clear donut hole; filled ellipse centre. + (130, 70), // Original ring/ellipse fill. + (130, 47), // Visible shadow/glow above the outer edge. + (130, 213), // Visible shadow/glow below the outer edge. + (130, 93), // Shadow/glow along the inner rim, inside the hole. + (130, 230), // Reflected upper ring. + (130, 300), // Reflected hole/ellipse centre. + (70, 300), // Reflected ring at the same fade as the hole. +]; + +#[derive(Clone, Copy, Debug)] +enum Effect { + Shadow, + Glow, + Reflection, +} + +fn scene(shape: &str, effect: Effect) -> Presentation { + let mut p = Presentation::default(); + let mut effects = Effects::default(); + match effect { + Effect::Shadow => { + effects.outer_shadow = Some(Shadow { + color: ColorRef::rgb(Rgba::BLACK), + blur: 8.0, + dist: 0.0, + dir: 45.0, + inner: false, + sx: 1.0, + sy: 1.0, + kx: 0.0, + ky: 0.0, + align: String::new(), + rotate_with_shape: false, + }) + } + Effect::Glow => effects.glow = Some(Glow { color: ColorRef::rgb(Rgba::rgb(255, 0, 0)), radius: 8.0 }), + Effect::Reflection => effects.reflection = Some(Reflection { blur: 0.0, start_alpha: 1.0, end_alpha: 0.0, end_pos: 1.0, dist: 10.0 }), + } + let slide = std::sync::Arc::make_mut(&mut p.slides[0]); + slide.background = Some(deckcraft_model::Background::Fill { fill: Fill::solid(ColorRef::rgb(Rgba::WHITE)) }); + slide.shapes = vec![Shape { + id: ShapeId(1596), + xfrm: Some(Xfrm::new(50.0, 50.0, 160.0, 160.0)), + geom: Geom::preset(shape), + fill: Some(Fill::solid(ColorRef::rgb(Rgba::rgb(32, 96, 224)))), + line: Some(Line::none()), + effects: Some(effects), + ..Default::default() + }]; + p +} + +fn draw(shape: &str, effect: Effect) -> Image { + let p = scene(shape, effect); + let image = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); + if let Ok(dir) = std::env::var("DECKCRAFT_EFFECT_HOLES_QA") { + // Overwrite, so a QA directory can be reused across runs. + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(std::path::Path::new(&dir).join(format!("{shape}-{effect:?}.png")), image.to_png()).unwrap(); + } + image +} + +// Fixed contributor-original RGBA goldens; tolerate one final quantization step. +// The nonwhite halo/reflection samples also fail if an effect is disabled outright. +fn assert_golden(shape: &str, effect: Effect, expected: [[u8; 4]; 9]) { + let image = draw(shape, effect); + for ((x, y), want) in PROBES.into_iter().zip(expected) { + let got = image.pixel(x, y); + assert!(got.iter().zip(want).all(|(a, b)| (*a as i32 - b as i32).abs() <= 1), "{shape} {effect:?} at {x},{y}: {got:?} vs golden {want:?}"); + } +} + +#[test] +fn donut_shadow_matches_golden() { + assert_golden( + "donut", + Effect::Shadow, + [ + [255, 255, 255, 255], + [255, 255, 255, 255], + [32, 96, 224, 255], + [214, 214, 214, 255], + [233, 233, 233, 255], + [227, 227, 227, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + ], + ); +} + +#[test] +fn donut_glow_matches_golden() { + assert_golden( + "donut", + Effect::Glow, + [ + [255, 255, 255, 255], + [255, 255, 255, 255], + [32, 96, 224, 255], + [255, 49, 49, 255], + [255, 111, 111, 255], + [255, 96, 96, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + ], + ); +} + +#[test] +fn donut_reflection_matches_golden() { + assert_golden( + "donut", + Effect::Reflection, + [ + [255, 255, 255, 255], + [255, 255, 255, 255], + [32, 96, 224, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [130, 166, 238, 255], + [255, 255, 255, 255], + [189, 208, 246, 255], + ], + ); +} + +#[test] +fn ellipse_shadow_matches_golden() { + assert_golden( + "ellipse", + Effect::Shadow, + [ + [255, 255, 255, 255], + [32, 96, 224, 255], + [32, 96, 224, 255], + [214, 214, 214, 255], + [233, 233, 233, 255], + [32, 96, 224, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + ], + ); +} + +#[test] +fn ellipse_glow_matches_golden() { + assert_golden( + "ellipse", + Effect::Glow, + [ + [255, 255, 255, 255], + [32, 96, 224, 255], + [32, 96, 224, 255], + [255, 49, 49, 255], + [255, 111, 111, 255], + [32, 96, 224, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + ], + ); +} + +#[test] +fn ellipse_reflection_matches_golden() { + assert_golden( + "ellipse", + Effect::Reflection, + [ + [255, 255, 255, 255], + [32, 96, 224, 255], + [32, 96, 224, 255], + [255, 255, 255, 255], + [255, 255, 255, 255], + [32, 96, 224, 255], + [130, 166, 238, 255], + [189, 208, 246, 255], + [189, 208, 246, 255], + ], + ); +} diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index ae78f64b1..987781f15 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -454,6 +454,198 @@ 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:?}" + ); + } + } +} + +/// 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 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}; + // `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); diff --git a/crates/text/src/layout.rs b/crates/text/src/layout.rs index 05cc4beb2..c0422815e 100644 --- a/crates/text/src/layout.rs +++ b/crates/text/src/layout.rs @@ -93,12 +93,24 @@ impl BoxGeom { } else { text_rect }; - let inner = Rect::new( - r.x0 + bp.inset_l.unwrap_or(7.2), - r.y0 + bp.inset_t.unwrap_or(3.6), - (r.x1 - bp.inset_r.unwrap_or(7.2)).max(r.x0 + bp.inset_l.unwrap_or(7.2)), - (r.y1 - bp.inset_b.unwrap_or(3.6)).max(r.y0 + bp.inset_t.unwrap_or(3.6)), - ); + // Insets wider than the shape scale down proportionally to fit inside it (#1632). + let (mut ins_l, mut ins_r) = (bp.inset_l.unwrap_or(7.2), bp.inset_r.unwrap_or(7.2)); + let (mut ins_t, mut ins_b) = (bp.inset_t.unwrap_or(3.6), bp.inset_b.unwrap_or(3.6)); + let w = r.width().max(0.0); + let total_x = ins_l + ins_r; + if ins_l >= 0.0 && ins_r >= 0.0 && total_x > w && total_x > 0.0 { + let s = w / total_x; + ins_l *= s; + ins_r *= s; + } + let h = r.height().max(0.0); + let total_y = ins_t + ins_b; + if ins_t >= 0.0 && ins_b >= 0.0 && total_y > h && total_y > 0.0 { + let s = h / total_y; + ins_t *= s; + ins_b *= s; + } + let inner = Rect::new(r.x0 + ins_l, r.y0 + ins_t, (r.x1 - ins_r).max(r.x0 + ins_l), (r.y1 - ins_b).max(r.y0 + ins_t)); // Text that follows a WordArt arch or circle is set on one line per paragraph. let follow = bp.warp.as_deref().is_some_and(deckcraft_geom::warp::is_follow); let wrap = bp.wrap.unwrap_or(true) && !follow && !matches!(bp.autofit, Some(AutoFit::Shape) if shape.text_box && !bp.wrap.unwrap_or(true)); diff --git a/crates/text/src/shape.rs b/crates/text/src/shape.rs index 2be8424f7..d5558001d 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))); diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 656dfdb66..7dbdbe4f0 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1946,3 +1946,39 @@ fn a_paragraph_with_its_own_start_at_restarts_the_count_there() { // A later paragraph with a new start begins a fresh run. check(&[1, 1, 7, 7], &[1, 2, 7, 8]); } + +/// #1632: when insets are wider than the shape, they scale down proportionally so the text +/// rectangle stays inside the shape bounds and unwrapped text does not project outside it. +#[test] +fn insets_wider_than_the_shape_scale_down_within_the_shape_bounds() { + let (p, mut sh) = setup("Collaboration Hub", 14.17, 14.17); + sh.ph = None; + let bp = &mut sh.text.as_mut().unwrap().body; + bp.wrap = Some(false); + bp.inset_l = Some(127.5); + bp.inset_r = Some(11.33); + bp.inset_t = Some(11.33); + bp.inset_b = Some(11.33); + sh.text.as_mut().unwrap().paragraphs[0].props = deckcraft_model::text::ParaProps { align: Some(Align::Center), ..Default::default() }; + let l = lay(&p, &sh, 14.17, 14.17); + assert_eq!(l.lines.len(), 1); + let line = &l.lines[0]; + let mid = (line.caret_x[0] + *line.caret_x.last().unwrap()) / 2.0; + // The text center must remain inside the 14.17 pt wide shape, not pushed ~127.5 pt to the right. + assert!((0.0..=14.17).contains(&mid), "line center {mid} should be within [0, 14.17]"); +} + +/// #1632: signed insets (e.g. negative left inset with positive right inset) are preserved when +/// their sum fits within the shape bounds. +#[test] +fn signed_insets_are_preserved_when_they_fit() { + let (p, mut sh) = setup("Text", 100.0, 50.0); + sh.ph = None; + let bp = &mut sh.text.as_mut().unwrap().body; + bp.inset_l = Some(-10.0); + bp.inset_r = Some(10.0); + sh.text.as_mut().unwrap().paragraphs[0].props = deckcraft_model::text::ParaProps { align: Some(Align::Left), ..Default::default() }; + let l = lay(&p, &sh, 100.0, 50.0); + assert_eq!(l.lines.len(), 1); + assert_eq!(l.inner.x0, -10.0); +} diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 1278e7d8a..0fec08a52 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -2,7 +2,7 @@ //! bar until Esc or the back arrow returns to the presentation. With nothing open it is the start //! screen: PowerPoint's start screen and File › Home are the same page. //! -//! A side menu on the left picks a page (Home, New, Open, Info, Save As, Print, Export, Account); +//! A side menu on the left picks a page (Home, New, Open, Info, Save As, Print, Export, About); //! Save, Close and Options act at once. Every action runs an existing command through //! [`SlideApp::run`] or one of the app's file helpers ([`SlideApp::export_as`], //! [`SlideApp::print_pdf`], [`SlideApp::save_as_dialog`]); the state here is UI-only and never @@ -24,7 +24,7 @@ mod print; pub use files::split_path; use files::{home, new_page, open_page}; -use info::{PROPERTIES, account, commit_props, info, save_as}; +use info::{PROPERTIES, about, commit_props, info, save_as}; pub use info::{SAVE_TYPES, format_bytes}; use print::{export, print}; pub use print::{print_page_size, print_params, print_slides}; @@ -47,11 +47,11 @@ pub enum Page { SaveAs, Print, Export, - Account, + About, } impl Page { - pub const ALL: [Page; 8] = [Page::Home, Page::New, Page::Open, Page::Info, Page::SaveAs, Page::Print, Page::Export, Page::Account]; + pub const ALL: [Page; 8] = [Page::Home, Page::New, Page::Open, Page::Info, Page::SaveAs, Page::Print, Page::Export, Page::About]; /// The id `app.backstage {page}` takes. pub fn id(self) -> &'static str { @@ -63,12 +63,16 @@ impl Page { Page::SaveAs => "saveAs", Page::Print => "print", Page::Export => "export", - Page::Account => "account", + Page::About => "about", } } pub fn parse(id: &str) -> Option { let id = id.trim(); + // Keep existing control-channel callers working after the visible page rename. + if id.eq_ignore_ascii_case("account") { + return Some(Page::About); + } Page::ALL.into_iter().find(|p| p.id().eq_ignore_ascii_case(id)) } @@ -82,7 +86,7 @@ impl Page { Page::SaveAs => "Save As", Page::Print => "Print", Page::Export => "Export", - Page::Account => "Account", + Page::About => "About", } } @@ -95,7 +99,7 @@ impl Page { Page::SaveAs => Icon::SaveAs, Page::Print => Icon::Print, Page::Export => Icon::Export, - Page::Account => Icon::Account, + Page::About => Icon::Help, } } @@ -140,7 +144,7 @@ pub fn entries(has_doc: bool) -> (Vec