diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 09b477f38..b156fa50d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -45,3 +45,4 @@ jobs: AZURE_SIGNING_ENDPOINT: ${{ secrets.AZURE_SIGNING_ENDPOINT }} AZURE_SIGNING_ACCOUNT: ${{ secrets.AZURE_SIGNING_ACCOUNT }} AZURE_CERT_PROFILE: ${{ secrets.AZURE_CERT_PROFILE }} + LINUX_SIGNING_KEY: ${{ secrets.LINUX_SIGNING_KEY }} diff --git a/Cargo.lock b/Cargo.lock index c86df3540..417823304 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1275,6 +1275,7 @@ dependencies = [ "deckcraft-render", "deckcraft-text", "egui", + "egui_extras", "egui_kittest", "log", "serde", @@ -1520,6 +1521,19 @@ dependencies = [ "winit", ] +[[package]] +name = "egui_extras" +version = "0.37.0" +source = "git+https://github.com/storytold/egui.git?rev=6d518fc799681571963842f95b55a6b68431b15a#6d518fc799681571963842f95b55a6b68431b15a" +dependencies = [ + "ahash", + "egui", + "enum-map", + "itertools", + "log", + "profiling", +] + [[package]] name = "egui_glow" version = "0.37.0" @@ -1587,6 +1601,26 @@ version = "1.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "66b7e2430c6dff6a955451e2cfc438f09cea1965a9d6f87f7e3b90decc014099" +[[package]] +name = "enum-map" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d4532960034c15ba2f55e9eee631040ecc473c32e6c4b200828c759840f13a06" +dependencies = [ + "enum-map-derive", +] + +[[package]] +name = "enum-map-derive" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7127272f64b3e098f7d81f350d672081b879bff896d219c42370dc858b1af9ff" +dependencies = [ + "proc-macro2", + "quote", + "syn 3.0.6", +] + [[package]] name = "enumflags2" version = "0.7.12" @@ -3487,7 +3521,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d8fae84b431384b68627d0f9b3b1245fcf9f46f6c0e3dc902e9dce64edd1967" dependencies = [ "libc", - "windows-sys 0.45.0", + "windows-sys 0.61.2", ] [[package]] @@ -6511,11 +6545,6 @@ name = "egui_demo_lib" version = "0.37.0" source = "git+https://github.com/storytold/egui.git?rev=6d518fc799681571963842f95b55a6b68431b15a#6d518fc799681571963842f95b55a6b68431b15a" -[[patch.unused]] -name = "egui_extras" -version = "0.37.0" -source = "git+https://github.com/storytold/egui.git?rev=6d518fc799681571963842f95b55a6b68431b15a#6d518fc799681571963842f95b55a6b68431b15a" - [[patch.unused]] name = "egui_inspection" version = "0.37.0" diff --git a/ROADMAP.md b/ROADMAP.md index c254c0d1f..d172a2402 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-11-04 · **Last updated:** 2026-11-04 · **Change:** trivial (Ogg Opus recovery includes trailing packets without granules in its reported duration; parity scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) > **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (progress log: web `?host=parent` bridge opens and saves through a same-origin framing page; scores unchanged) · **Target:** Microsoft PowerPoint for Mac 16.113 (Microsoft 365) + > **Last reviewed:** 2026-11-02 · **Last updated:** 2026-11-03 · **Change:** trivial (community PR wave 10: PPTX and native saves keep table cell centred anchoring, text overflow, compatible line spacing, normalised character height, Thai distributed alignment, RTL table direction, alternate run language and reflection transforms; imported effects placed by parent time-node delays; Reuse Slides fits the destination size and brings custom table styles; deleted slides leave custom shows; merged shapes drop their animations; Compress Pictures measures grouped pictures at displayed size; parity 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 (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) @@ -151,6 +153,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | What landed | |---|---| +| 2026-11-04 | Ogg Opus recovery keeps trailing packets without granules in the total duration, matching their recovered positions; valid end trimming stays covered | | 2026-11-03 | Web: the `?host=parent` bridge (as in GridCraft, WordCraft and PdfCraft) lets a same-origin page that frames DeckCraft send a presentation and store its saves; Save writes an opened .pptx back as .pptx under its own name, as on the desktop | | 2026-11-02 | Native and PPTX saves retain optional table right-to-left direction, including explicit false and omission; two-save round-trip tests keep labeled columns in order | | 2026-11-02 | Text runs preserve alternate-language metadata independently from the main language through native and PPTX saves | @@ -332,6 +335,7 @@ app's JSON control channel (`deckcraft --control PORT`). | Date | Change | Summary | |---|---|---| +| 2026-11-04 | trivial | Ogg Opus recovery duration includes its recovered tail; parity scores unchanged | | 2026-11-02 | trivial | Progress log: run alternate-language metadata round-trips through native and PPTX formats; product parity scores unchanged | | 2026-11-03 | trivial | Reuse Slides brings referenced custom table styles along; scores unchanged | | 2026-11-03 | trivial | Progress log: web `?host=parent` bridge to a framing page; scores unchanged | diff --git a/apps/deckcraft-web/src/presenter.rs b/apps/deckcraft-web/src/presenter.rs index 1ec9a298c..40cafe849 100644 --- a/apps/deckcraft-web/src/presenter.rs +++ b/apps/deckcraft-web/src/presenter.rs @@ -251,6 +251,9 @@ impl Popup { if value.get("nextChanged").and_then(|v| v.as_bool()) == Some(false) { data.next = previous.next.clone(); } + if data.slides.is_none() { + data.slides = previous.slides.clone(); + } } *target.borrow_mut() = Some(data); *received.borrow_mut() = js_sys::Date::now(); diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index e750977d9..a1c781f96 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -404,6 +404,7 @@ pub fn pdf_options(p: &Value, count: usize) -> Result pub fn parse_slide_range(text: &str, count: usize) -> Result> { let invalid = || bad("file.export", format!("“{text}” isn't a valid slide range: use numbers and ranges such as 1-3, 5")); let mut out: Vec = vec![]; + let mut seen: std::collections::HashSet = std::collections::HashSet::new(); for part in text.split([',', ';']).map(str::trim).filter(|p| !p.is_empty()) { let (a, b) = match part.split_once('-') { Some((a, b)) => (a.trim(), b.trim()), @@ -419,7 +420,7 @@ pub fn parse_slide_range(text: &str, count: usize) -> Result> { return Err(bad("file.export", format!("the presentation has {count} slides; “{part}” is outside them"))); } for i in lo - 1..hi { - if !out.contains(&i) { + if seen.insert(i) { out.push(i); } } @@ -786,6 +787,16 @@ mod tests { use super::{parse_slide_range, write_file}; + #[test] + fn broad_and_repeated_slide_ranges_expand_once() { + // 200 copies of a 100,000-slide range: each index once, in order, without a scan per index. + let text = vec!["1-100000"; 200].join(","); + let v = parse_slide_range(&text, 100_000).unwrap(); + assert_eq!(v.len(), 100_000); + assert!(v.windows(2).all(|w| w[0] < w[1])); + assert_eq!(parse_slide_range("3, 1-3, 2", 3).unwrap(), vec![2, 0, 1]); + } + #[test] fn slide_range_rejects_plus_sign() { assert!(parse_slide_range("+2", 3).is_err()); diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 92637741f..351ca29dc 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -219,7 +219,7 @@ fn neighbour(s: &mut Session, p: &Value, forward: bool) -> Result { let cmd = if forward { "comment.next" } else { "comment.prev" }; let (threads, slide, index) = { let st = s.doc()?; - (comment_threads(&st.doc), usize_param(p, "slide").unwrap_or(st.selection.slide), usize_param(p, "index")) + (comment_threads(&st.doc), usize_param(p, "slide").unwrap_or(st.selection.slide), super::opt_usize_param(p, "index", cmd)?) }; if threads.is_empty() { return Err(bad(cmd, "no comments")); @@ -367,7 +367,7 @@ fn spelling(s: &mut Session, p: &Value) -> Result { } out.push(v); } - let rep: Vec = crate::spell::repeated(para, &default).iter().map(|w| w.start).collect(); + let rep: std::collections::HashSet = crate::spell::repeated(para, &default).iter().map(|w| w.start).collect(); for w in crate::spell::paragraph_words(para, &default) { if ignore.contains(&w.text.to_lowercase()) || rep.contains(&w.start) { continue; @@ -862,6 +862,40 @@ mod tests { assert_eq!(here["text"], "on first"); } + #[test] + fn malformed_comment_navigation_index_does_not_change_selection() { + let mut s = Session::with_new(); + s.execute("comment.add", &json!({"text": "first"})).unwrap(); + s.execute("shape.insert", &json!({"preset": "rect", "rect": [10.0, 10.0, 40.0, 40.0]})).unwrap(); + s.execute("slide.new", &json!({})).unwrap(); + s.execute("comment.add", &json!({"text": "second"})).unwrap(); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + let shape = s.doc().unwrap().doc.slides.first().and_then(|slide| slide.shapes.first()).map(|shape| shape.id).unwrap(); + s.doc_mut().unwrap().selection.shapes = vec![shape]; + + for (command, value) in [ + ("comment.next", json!(-1)), + ("comment.next", json!(1.5)), + ("comment.next", json!("bad")), + ("comment.prev", json!(-1)), + ("comment.prev", json!(1.5)), + ("comment.prev", json!("bad")), + ] { + let before = s.doc().unwrap().selection.clone(); + let error = s.execute(command, &json!({"index": value})).unwrap_err(); + assert!(error.to_string().contains("valid index"), "{command} {value}: {error}"); + assert_eq!(s.doc().unwrap().selection, before, "{command} {value}"); + } + + let zero = s.execute("comment.next", &json!({"slide": 0, "index": 0})).unwrap(); + assert_eq!(zero["slide"], 1); + assert_eq!(zero["index"], 0); + s.execute("slide.go", &json!({"index": 0})).unwrap(); + let omitted = s.execute("comment.next", &json!({})).unwrap(); + assert_eq!(omitted["slide"], 0); + assert_eq!(omitted["index"], 0); + } + #[test] fn timestamps_are_iso_utc() { assert_eq!(iso_utc(0), "1970-01-01T00:00:00Z"); diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 08ed9be64..a447a20b9 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 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", + "{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 (false keeps linear and radial gradients upright on the slide; rectangular and path gradients turn with the shape when drawn)} | 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/cmd/shape/appearance.rs b/crates/engine/src/cmd/shape/appearance.rs index 4189deaa1..c515d2b2c 100644 --- a/crates/engine/src/cmd/shape/appearance.rs +++ b/crates/engine/src/cmd/shape/appearance.rs @@ -74,7 +74,8 @@ pub(super) fn change(s: &mut Session, p: &Value) -> Result { /// `{stops: [[pos 0–1, color], …], kind?: linear|radial|rectangular|path, angle?: deg (linear), /// focus?: [l, t, r, b] fractions (radial/rectangular/path: where the first stop sits), -/// rotateWithShape?}`. +/// rotateWithShape?}`. The renderer keeps only linear and radial gradients upright when +/// `rotateWithShape` is false; rectangular and path gradients turn with the shape. pub(crate) fn gradient_param(v: &Value) -> Option { let stops: Vec = v .get("stops")? diff --git a/crates/engine/src/cmd/slide.rs b/crates/engine/src/cmd/slide.rs index 198f1bc6c..f15cf32fa 100644 --- a/crates/engine/src/cmd/slide.rs +++ b/crates/engine/src/cmd/slide.rs @@ -408,8 +408,9 @@ fn select_slides(s: &mut Session, p: &Value) -> Result { }; s.select(|doc, sel| { if add { + let mut have: std::collections::HashSet<_> = sel.slides.iter().copied().collect(); for i in &ids { - if !sel.slides.contains(i) { + if have.insert(*i) { sel.slides.push(*i); } } diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index bf8ccf4ac..cb89d08f5 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -121,9 +121,10 @@ pub fn diff(cur: &Presentation, rev: &Presentation, by_position: bool) -> Vec = pairs.iter().map(|p| p.0).collect(); let rev_matched: HashSet = pairs.iter().map(|p| p.1).collect(); let mut out = vec![]; + let mut media = Media::new(cur, rev); for (i, j) in &pairs { let (Some(a), Some(b)) = (cur.slides.get(*i), rev.slides.get(*j)) else { continue }; - let details = slide_details(cur, a, rev, b); + let details = slide_details(cur, a, rev, b, &mut media); if !details.is_empty() { out.push(Change { kind: ChangeKind::Changed, slide: *i, number: i.saturating_add(1), only_in: None, revised: Some(*j), details }); } @@ -140,10 +141,18 @@ pub fn diff(cur: &Presentation, rev: &Presentation, by_position: bool) -> Vec = HashMap::new(); + for p in &pairs { + rev_to_cur.entry(p.1).or_insert(p.0); + } + // After the open slide that matches the nearest matched revised slide before it. + let mut at = 0; for (j, s) in rev.slides.iter().enumerate() { + if let Some(i) = rev_to_cur.get(&j) { + at = i + 1; + } if !rev_matched.contains(&j) { - // After the open slide that matches the revised slide before it. - let at = (0..j).rev().find_map(|k| pairs.iter().find(|p| p.1 == k).map(|p| p.0 + 1)).unwrap_or(0); out.push(Change { kind: ChangeKind::Inserted, slide: at, @@ -174,8 +183,34 @@ fn shape_label(s: &Shape) -> String { if s.name.trim().is_empty() { format!("Shape {}", s.id.0) } else { s.name.clone() } } -fn media_bytes(p: &Presentation, id: MediaId) -> Option<&[u8]> { - p.media(id).map(|m| m.data.as_slice()) +/// Both decks' media bytes by id, and which picture pairs were already compared: a deck can +/// show one picture many times, and its bytes are compared once per pair. +struct Media<'a> { + cur: HashMap, + rev: HashMap, + same: HashMap<(MediaId, MediaId), bool>, +} + +impl<'a> Media<'a> { + fn new(cur: &'a Presentation, rev: &'a Presentation) -> Self { + // The first item with an id wins, as in `Presentation::media`. + let index = |p: &'a Presentation| { + let mut m = HashMap::new(); + for x in &p.media { + m.entry(x.id).or_insert(x.data.as_slice()); + } + m + }; + Media { cur: index(cur), rev: index(rev), same: HashMap::new() } + } + /// Whether the pictures (by media id, `None` for a shape that is not a picture) differ. + fn differ(&mut self, a: Option, b: Option) -> bool { + let (ba, bb) = (a.and_then(|x| self.cur.get(&x)), b.and_then(|y| self.rev.get(&y))); + match (a, b) { + (Some(x), Some(y)) if ba.is_some() && bb.is_some() => !*self.same.entry((x, y)).or_insert_with(|| ba == bb), + _ => ba.is_some() || bb.is_some(), + } + } } /// A shape without its ids (shape and media), for comparing across decks. @@ -204,7 +239,7 @@ fn turn(ctx: Option<&Ctx>, s: &Shape) -> (f64, bool, bool) { drawn_box(ctx, s).map_or((0.0, false, false), |x| (x.rot, x.flip_h, x.flip_v)) } -fn shape_details(cp: &Presentation, ca: Option<&Ctx>, a: &Shape, rp: &Presentation, cb: Option<&Ctx>, b: &Shape, out: &mut Vec) { +fn shape_details(ca: Option<&Ctx>, a: &Shape, cb: Option<&Ctx>, b: &Shape, media: &mut Media, out: &mut Vec) { let label = shape_label(b); let text = |s: &Shape| s.text.as_ref().map(|t| t.text()).unwrap_or_default(); let mut what = vec![]; @@ -234,11 +269,11 @@ fn shape_details(cp: &Presentation, ca: Option<&Ctx>, a: &Shape, rp: &Presentati if a.fill != b.fill || a.line != b.line || a.style != b.style || a.effects != b.effects { what.push("formatting"); } - let pic = |p: &Presentation, s: &Shape| match &s.kind { - ShapeKind::Picture { fill } => media_bytes(p, fill.media).map(|b| b.to_vec()), + let pic = |s: &Shape| match &s.kind { + ShapeKind::Picture { fill } => Some(fill.media), _ => None, }; - if pic(cp, a) != pic(rp, b) { + if media.differ(pic(a), pic(b)) { what.push("picture"); } // Anything the checks above do not cover (alt text, hidden, a link…) is reported too, also @@ -265,21 +300,32 @@ fn shape_details(cp: &Presentation, ca: Option<&Ctx>, a: &Shape, rp: &Presentati } } -fn slide_details(cp: &Presentation, a: &Slide, rp: &Presentation, b: &Slide) -> Vec { +fn slide_details(cp: &Presentation, a: &Slide, rp: &Presentation, b: &Slide, media: &mut Media) -> Vec { let mut out = vec![]; let mut used: HashSet = HashSet::new(); let (ca, cb) = (Ctx::for_slide(cp, a), Ctx::for_slide(rp, b)); + // The revised shapes by id (first wins) and by name (in order, with a cursor past used ones). + let mut by_id: HashMap = HashMap::new(); + let mut by_name: HashMap<&str, (Vec, usize)> = HashMap::new(); + for (k, x) in b.shapes.iter().enumerate() { + by_id.entry(x.id).or_insert(k); + if !x.name.is_empty() { + by_name.entry(x.name.as_str()).or_default().0.push(k); + } + } for sa in &a.shapes { - let k = b - .shapes - .iter() - .position(|x| x.id == sa.id) - .or_else(|| b.shapes.iter().enumerate().position(|(k, x)| !used.contains(&k) && x.name == sa.name && !x.name.is_empty())); + let k = by_id.get(&sa.id).copied().or_else(|| { + let (ks, next) = by_name.get_mut(sa.name.as_str())?; + while ks.get(*next).is_some_and(|k| used.contains(k)) { + *next += 1; + } + ks.get(*next).copied() + }); match k { Some(k) if !used.contains(&k) => { used.insert(k); if let Some(sb) = b.shapes.get(k) { - shape_details(cp, ca.as_ref(), sa, rp, cb.as_ref(), sb, &mut out); + shape_details(ca.as_ref(), sa, cb.as_ref(), sb, media, &mut out); } } _ => out.push(format!("{} deleted (only in the deck)", shape_label(sa))), @@ -334,19 +380,34 @@ pub fn adopt(doc: &mut Presentation, rev: &Presentation, slide: &Slide, replacin let mut sl = slide.clone(); // Media. let mut media_map: HashMap = HashMap::new(); + // Both decks' media by id (the first item with an id wins, as in `Presentation::media`), + // looked up once per reference instead of searched. + let mut rev_at: HashMap = HashMap::new(); + for (i, m) in rev.media.iter().enumerate() { + rev_at.entry(m.id).or_insert(i); + } + let mut doc_at: HashMap = HashMap::new(); + for (i, m) in doc.media.iter().enumerate() { + doc_at.entry(m.id).or_insert(i); + } for id in crate::media_refs::slide_media(&sl) { - let Some(m) = rev.media(id) else { continue }; - match doc.media(id) { + let Some(m) = rev_at.get(&id).and_then(|&i| rev.media.get(i)) else { continue }; + match doc_at.get(&id).and_then(|&i| doc.media.get(i)) { Some(x) if x.data == m.data => {} None => { // Keeps the revision's id, so later ids must start past it. + doc_at.insert(id, doc.media.len()); doc.media.push(m.clone()); doc.next_id = doc.next_id.max(id.0); } Some(_) => { + let before = doc.media.len(); let new = doc .add_media(&m.name, &m.content_type, m.data.as_ref().clone()) .ok_or_else(|| crate::cmd::bad("review.accept", crate::cmd::NO_ID_SPACE))?; + if doc.media.len() > before { + doc_at.entry(new).or_insert(before); + } media_map.insert(id, new); } } @@ -519,6 +580,39 @@ mod tests { assert_eq!(s.doc().unwrap().doc.slides.len(), n, "comparing alone changes nothing"); } + #[test] + fn many_inserted_slides_and_same_named_pictures_compare_by_index() { + use std::sync::Arc; + + use deckcraft_model::style::PictureFill; + use deckcraft_model::{Shape, ShapeId, ShapeKind, SlideId, defaults}; + + // One slide of 3,000 pictures of one image, all named "Box"; the revision has the same + // pictures under other ids (matched by name, in order) and 3,000 slides inserted after it. + let mut cur = defaults::new_presentation(None); + let img = cur.add_media("a.png", "image/png", vec![7; 4096]).unwrap(); + let mut rev = cur.clone(); + let pic = |id: u32| Shape { + id: ShapeId(id), + name: "Box".into(), + xfrm: Some(deckcraft_model::Xfrm::new(10.0, 10.0, 50.0, 50.0)), + kind: ShapeKind::Picture { fill: PictureFill { media: img, ..Default::default() } }, + ..Default::default() + }; + Arc::make_mut(&mut cur.slides[0]).shapes = (0..3000).map(|k| pic(100_000 + k)).collect(); + Arc::make_mut(&mut rev.slides[0]).shapes = (0..3000).map(|k| pic(200_000 + k)).collect(); + let mut base = (*rev.slides[0]).clone(); + base.shapes.clear(); + for k in 0..3000 { + let mut s = base.clone(); + s.id = SlideId(300_000 + k); + rev.slides.push(Arc::new(s)); + } + let changes = super::diff(&cur, &rev, false); + assert_eq!(changes.len(), 3000, "only the inserted slides differ"); + assert!(changes.iter().all(|c| c.kind == super::ChangeKind::Inserted && c.slide == 1)); + } + /// A deck with one slide: a table whose cell has a picture fill, a picture background and a /// transition sound. `tag` starts the bytes of each. fn media_deck(tag: &str) -> deckcraft_model::Presentation { diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 38f4de8d2..9a98f62a0 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -48,6 +48,8 @@ pub fn write_file(path: &str, bytes: &[u8]) -> Result<(), String> { // 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); + // The sibling must pass the same check: when `path` is the write root itself it lands outside. + crate::file_access::check_write(&tmp.to_string_lossy())?; 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, @@ -135,4 +137,16 @@ mod tests { let _ = std::fs::remove_file(&file); } + + #[test] + fn write_file_to_the_write_root_itself_creates_no_sibling_outside_it() { + let base = std::env::temp_dir().join(format!("deckcraft_io_root_test_{}", std::process::id())); + let root = base.join("root"); + std::fs::create_dir_all(&root).unwrap(); + let roots = crate::file_access::AutomationRoots::new(&[], std::slice::from_ref(&root)).unwrap().unwrap(); + let res = crate::file_access::confine(Some(&roots), || write_file(root.to_str().unwrap(), b"x")); + assert!(res.unwrap_err().contains("outside the write root")); + assert_eq!(std::fs::read_dir(&base).unwrap().count(), 1, "only the root is there"); + let _ = std::fs::remove_dir_all(&base); + } } diff --git a/crates/engine/src/spell.rs b/crates/engine/src/spell.rs index 2dda5c6f4..0eeaa6757 100644 --- a/crates/engine/src/spell.rs +++ b/crates/engine/src/spell.rs @@ -700,15 +700,29 @@ pub struct Word { pub repeated: Option, } +/// Longest language tag a word is checked in. BCP 47 tags in documents are a few bytes; a longer +/// value from a file is no language we have a dictionary for, and copying it into every word of +/// a paragraph would multiply it. +const MAX_LANG_LEN: usize = 64; + /// The words of `p`, each with its run's language (`lang` on the run, else `default`). Words /// with digits are skipped; apostrophes inside a word belong to it; hyphenated words are checked -/// part by part. +/// part by part. A language over [`MAX_LANG_LEN`] counts as none. pub fn paragraph_words(p: &deckcraft_model::Paragraph, default: &str) -> Vec { let mut chars: Vec<(char, usize)> = vec![]; for (ri, r) in p.runs.iter().enumerate() { chars.extend(r.content().chars().map(|c| (c, ri))); } - let lang_of = |ri: usize| p.runs.get(ri).and_then(|r| r.props.lang.clone()).filter(|l| !l.is_empty()).unwrap_or_else(|| default.to_string()); + let default = if default.len() > MAX_LANG_LEN { "" } else { default }; + let lang_of = |ri: usize| { + p.runs + .get(ri) + .and_then(|r| r.props.lang.as_deref()) + .filter(|l| !l.is_empty()) + .map(|l| if l.len() > MAX_LANG_LEN { "" } else { l }) + .unwrap_or(default) + .to_string() + }; // Combining marks (decomposed accents) belong to the letter before them. let inside = |c: char| c.is_alphanumeric() || deckcraft_model::words::is_combining_mark(c) || c == '\'' || c == '’' || c == '-'; let mut out = vec![]; @@ -791,15 +805,18 @@ pub fn misspelt(p: &deckcraft_model::Paragraph, default: &str, ignore: &[String] .chain(repeated(p, default)) .collect::>() .into_iter() - .fold(vec![], |mut v: Vec, w| { + .fold((vec![], HashMap::new()), |(mut v, mut at): (Vec, HashMap), w| { // A repeated misspelt word is reported once, as repeated. - if let Some(i) = v.iter().position(|x| x.start == w.start) { - v[i] = w; - } else { - v.push(w); + match at.get(&w.start).and_then(|&i| v.get_mut(i)) { + Some(x) => *x = w, + None => { + at.insert(w.start, v.len()); + v.push(w); + } } - v + (v, at) }) + .0 } /// Use `d` for `lang` from now on in this process (a dictionary the app loaded itself). @@ -953,6 +970,23 @@ SFX D Y 1\nSFX D 0 ed/S .\n"; assert_eq!(w, vec!["check", "done"]); } + #[test] + fn a_long_paragraph_with_a_huge_language_is_checked_in_linear_work() { + use deckcraft_model::{Paragraph, Run, RunProps}; + // 100,000 copies of one word under a 1 MB language value: every word after the first is + // a repeat, and no word carries a copy of the value. + let lang = "x".repeat(1 << 20); + let p = + Paragraph { runs: vec![Run::with("the ".repeat(100_000), RunProps { lang: Some(lang), ..Default::default() })], ..Default::default() }; + let words = paragraph_words(&p, "en-US"); + assert_eq!(words.len(), 100_000); + assert!(words.iter().all(|w| w.lang.is_empty())); + let bad = misspelt(&p, "en-US", &[]); + assert_eq!(bad.len(), 99_999); + assert!(bad.iter().all(|w| w.repeated.is_some())); + assert!(paragraph_words(&Paragraph::new("word"), &"y".repeat(100)).iter().all(|w| w.lang.is_empty())); + } + #[test] fn long_and_numeric_flags_and_latin1() { let d = Dictionary::parse(b"FLAG long\nSFX Aa Y 1\nSFX Aa 0 s .\n", b"1\ncat/Aa\n"); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e37f9d135..6741f03d5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4321,6 +4321,9 @@ fn shape_fill_documents_each_gradient_key_and_kind() { 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"]); + // The renderer keeps only linear and radial gradients upright (`paint::fill_path`). + let rotate = cmd::parse_params(about).unwrap().into_iter().find(|p| p.name == "rotateWithShape").unwrap().about; + assert!(rotate.contains("rectangular and path gradients turn with the shape"), "{rotate}"); for kind in ["linear", "radial", "rectangular", "path"] { assert!(about.contains(kind), "{kind}: {about}"); let g = cmd::shape::gradient_param(&json!({ @@ -6506,3 +6509,35 @@ fn save_to_image_path_is_an_error_not_zip_bytes() { } let _ = std::fs::remove_dir_all(&dir); } + +/// The outline exports (PowerPoint's outline view) holds titles and body text only: footer, +/// date and slide-number placeholders are not body lines. +#[test] +fn outline_export_skips_footer_date_and_slide_number() { + let mut s = session(); + s.execute("slide.new", &json!({"layout": "titleAndContent", "title": "Agenda", "body": "First point\nSecond point"})).unwrap(); + s.execute( + "design.headerFooter", + &json!({"footer": true, "footerText": "Company Confidential", "slideNumber": true, "date": true, "dateText": "1 May 2026"}), + ) + .unwrap(); + let dir = std::env::temp_dir().join(format!("deckcraft-outline-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("deck.md"); + s.execute("file.export", &json!({"path": path.to_string_lossy(), "format": "outline"})).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + let _ = std::fs::remove_dir_all(&dir); + assert!(text.contains("Agenda") && text.contains("First point") && text.contains("Second point"), "{text:?}"); + assert!(!text.contains("Company Confidential"), "footer text is not a body line: {text:?}"); + assert!(!text.contains("‹#›"), "the slide-number field is not a body line: {text:?}"); + assert!(!text.contains("1 May 2026"), "the date is not a body line: {text:?}"); + // The RTF outline (Save As ▸ Outline/RTF) too. + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("deck.rtf"); + s.execute("file.export", &json!({"path": path.to_string_lossy(), "format": "rtf"})).unwrap(); + let rtf = std::fs::read_to_string(&path).unwrap(); + let _ = std::fs::remove_dir_all(&dir); + assert!(rtf.contains("Agenda") && rtf.contains("First point"), "{rtf:?}"); + assert!(!rtf.contains("Company Confidential") && !rtf.contains("1 May 2026"), "{rtf:?}"); +} diff --git a/crates/fonts/src/fallback.rs b/crates/fonts/src/fallback.rs index de4b2b0a6..8d54453f8 100644 --- a/crates/fonts/src/fallback.rs +++ b/crates/fonts/src/fallback.rs @@ -227,6 +227,9 @@ impl FontDb { } } +/// Most caret records read for one ligature glyph. +const MAX_LIGATURE_CARETS: usize = 64; + /// Caret positions inside a ligature glyph from GDEF `LigCaretList`, in font units from the /// glyph's origin (sorted ascending). Empty when the font has none for `gid`. pub fn ligature_carets(face: &FontFace, gid: u32) -> Vec { @@ -236,9 +239,12 @@ pub fn ligature_carets(face: &FontFace, gid: u32) -> Vec { let Ok(cov) = list.coverage() else { return Vec::new() }; let Some(idx) = cov.get(skrifa::GlyphId::new(gid)) else { return Vec::new() }; let Ok(lig) = list.lig_glyphs().get(idx as usize) else { return Vec::new() }; + // The limit applies to the records read, not to the carets kept: a font can list thousands of + // records it then has to be read through for every ligature laid out. let mut out: Vec = lig .caret_values() .iter() + .take(MAX_LIGATURE_CARETS) .filter_map(|cv| cv.ok()) .filter_map(|cv| match cv { skrifa::raw::tables::gdef::CaretValue::Format1(f) => Some(f.coordinate() as i32), @@ -246,7 +252,6 @@ pub fn ligature_carets(face: &FontFace, gid: u32) -> Vec { // Format 2 is a contour point index; outlines aren't consulted here. skrifa::raw::tables::gdef::CaretValue::Format2(_) => None, }) - .take(64) .collect(); out.sort_unstable(); out diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index 2e2f35299..19f190445 100644 --- a/crates/fonts/src/fontdb.rs +++ b/crates/fonts/src/fontdb.rs @@ -368,7 +368,7 @@ struct CatalogEntry { /// Process-wide font database. pub struct FontDb { faces: RwLock>>, - outlines: Mutex>>, + outlines: Mutex, #[cfg(not(target_arch = "wasm32"))] catalog: RwLock>, /// The folders the system font scan reads (the platform's font folders for [`FontDb::global`]). @@ -439,6 +439,38 @@ const SYSTEM_FALLBACKS: &[&str] = &[ static NEXT_ID: AtomicU32 = AtomicU32::new(1); const OUTLINE_CACHE_MAX: usize = 50_000; +/// Most path elements one glyph outline may expand to. Composite and `VARC` glyphs can draw a +/// shared outline many times over; a glyph past this draws nothing rather than holding megabytes. +const MAX_OUTLINE_ELEMENTS: usize = 1 << 15; +/// Most path elements the outline cache holds across all glyphs (about 230 MB at most) before it +/// starts over: distinct glyph ids can each expand one dense outline. +const OUTLINE_CACHE_MAX_ELEMENTS: usize = 1 << 22; + +/// Glyph outlines by (face, glyph), with the path elements they hold. +#[derive(Default)] +struct OutlineCache { + paths: HashMap<(u32, u32), Arc>, + elements: usize, +} + +impl OutlineCache { + fn get(&self, key: &(u32, u32)) -> Option> { + self.paths.get(key).cloned() + } + + fn insert(&mut self, key: (u32, u32), path: Arc) { + let n = path.elements().len(); + if let Some(old) = self.paths.remove(&key) { + self.elements = self.elements.saturating_sub(old.elements().len()); + } + if self.paths.len() >= OUTLINE_CACHE_MAX || self.elements.saturating_add(n) > OUTLINE_CACHE_MAX_ELEMENTS { + self.paths.clear(); + self.elements = 0; + } + self.paths.insert(key, path); + self.elements = self.elements.saturating_add(n); + } +} fn name(font: &skrifa::FontRef<'_>, ids: &[StringId]) -> Option { ids.iter().find_map(|id| font.localized_strings(*id).english_or_first().map(|s| s.to_string()).filter(|s| !s.is_empty())) @@ -694,6 +726,10 @@ const MAX_COLLECTION_FACES: u32 = 256; const MAX_COLLECTION_ENTRIES_SCANNED: u32 = 16_384; /// Maximum named instances read from a single face. const MAX_NAMED_INSTANCES_PER_FACE: usize = 1024; +/// Maximum faces (named instances counted separately) enumerated from one font file, across all +/// of a collection's faces: the per-collection and per-face limits multiply, and each face found +/// is then built with its own shaper. +const MAX_ENUMERATED_FACES: usize = 4096; fn distinct_face_indices(entries: impl IntoIterator) -> Vec { let mut offsets = std::collections::HashSet::new(); @@ -730,21 +766,30 @@ fn enumerate_faces(data: &[u8]) -> Vec { Err(_) => Vec::new(), }; let mut out = Vec::new(); + // Named instances read and plain faces kept, across the whole file. + let mut budget = MAX_ENUMERATED_FACES; for i in indices { + if budget == 0 { + break; + } let Ok(f) = skrifa::FontRef::from_index(data, i) else { continue }; let Some((family, style)) = face_names(&f) else { continue }; let axes = f.axes(); let mut named = Vec::new(); + let mut seen = std::collections::HashSet::new(); let instances = f.named_instances(); for ni in instances.iter().take(capped_named_instance_count(instances.len())) { + let Some(left) = budget.checked_sub(1) else { break }; + budget = left; let Some(style) = name(&f, &[ni.subfamily_name_id()]) else { continue }; - if named.iter().any(|(_, s, _): &(u32, String, _)| s.eq_ignore_ascii_case(&style)) { + if !seen.insert(style.to_ascii_lowercase()) { continue; } let coords: Vec<([u8; 4], f32)> = axes.iter().zip(ni.user_coords()).map(|(a, v)| (a.tag().to_be_bytes(), v)).collect(); named.push((i, style, coords)); } if named.is_empty() { + budget = budget.saturating_sub(1); out.push((i, family, style, Vec::new())); } else { out.extend(named.into_iter().map(|(i, s, c)| (i, family.clone(), s, c))); @@ -905,7 +950,7 @@ impl FontDb { } Self { faces: RwLock::new(faces), - outlines: Mutex::new(HashMap::new()), + outlines: Mutex::new(OutlineCache::default()), #[cfg(not(target_arch = "wasm32"))] catalog: RwLock::new(Vec::new()), font_dirs, @@ -1215,9 +1260,8 @@ impl FontDb { fn instance(&self, family: &str, style: &str) -> Option> { let open = style.find('{')?; let base = self.find(family, style[..open].trim())?; - if base.skrifa().is_none_or(|f| f.axes().is_empty()) { - return Some(base); - } + let Some(font) = base.skrifa().filter(|f| !f.axes().is_empty()) else { return Some(base) }; + let axis_tags: Vec<[u8; 4]> = font.axes().iter().map(|a| a.tag().to_be_bytes()).collect(); let mut coords = base.coords.clone(); for kv in style[open + 1..].trim_end_matches('}').split(',') { let (k, v) = kv.split_once(':')?; @@ -1227,6 +1271,11 @@ impl FontDb { return None; } let tag = [k[0], k[1], k[2], k[3]]; + // A tag the font has no axis for changes nothing; keeping it would grow the list with + // every distinct tag in the style text. + if !axis_tags.contains(&tag) { + continue; + } match coords.iter_mut().find(|(t, _)| *t == tag) { Some(c) => c.1 = v, None => coords.push((tag, v)), @@ -1513,41 +1562,72 @@ impl FontDb { pub fn outline(&self, face: &FontFace, gid: u32) -> Arc { let key = (face.id, gid); if let Some(p) = self.outlines.lock().unwrap_or_else(std::sync::PoisonError::into_inner).get(&key) { - return p.clone(); + return p; } - let mut pen = FlipPen(BezPath::new()); + let mut pen = FlipPen::new(MAX_OUTLINE_ELEMENTS); if let Some(f) = face.skrifa() && let Some(g) = f.outline_glyphs().get(GlyphId::new(gid)) { let _ = g.draw(DrawSettings::unhinted(Size::unscaled(), face.location()), &mut pen); } - let p = Arc::new(pen.0); - let mut cache = self.outlines.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - if cache.len() >= OUTLINE_CACHE_MAX { - cache.clear(); - } - cache.insert(key, p.clone()); + let p = Arc::new(pen.finish()); + self.outlines.lock().unwrap_or_else(std::sync::PoisonError::into_inner).insert(key, p.clone()); p } } -struct FlipPen(BezPath); +/// Collects a glyph outline, y flipped, up to `limit` path elements. Past the limit it ignores +/// what follows and [`FlipPen::finish`] gives an empty path: a cut-off outline would fill wrongly. +struct FlipPen { + path: BezPath, + limit: usize, + over: bool, +} + +impl FlipPen { + fn new(limit: usize) -> Self { + FlipPen { path: BezPath::new(), limit, over: false } + } + + /// Room for one more element? + fn room(&mut self) -> bool { + if !self.over && self.path.elements().len() >= self.limit { + self.over = true; + self.path = BezPath::new(); + } + !self.over + } + + fn finish(self) -> BezPath { + if self.over { BezPath::new() } else { self.path } + } +} impl OutlinePen for FlipPen { fn move_to(&mut self, x: f32, y: f32) { - self.0.move_to((x as f64, -y as f64)); + if self.room() { + self.path.move_to((x as f64, -y as f64)); + } } fn line_to(&mut self, x: f32, y: f32) { - self.0.line_to((x as f64, -y as f64)); + if self.room() { + self.path.line_to((x as f64, -y as f64)); + } } fn quad_to(&mut self, cx0: f32, cy0: f32, x: f32, y: f32) { - self.0.quad_to((cx0 as f64, -cy0 as f64), (x as f64, -y as f64)); + if self.room() { + self.path.quad_to((cx0 as f64, -cy0 as f64), (x as f64, -y as f64)); + } } fn curve_to(&mut self, cx0: f32, cy0: f32, cx1: f32, cy1: f32, x: f32, y: f32) { - self.0.curve_to((cx0 as f64, -cy0 as f64), (cx1 as f64, -cy1 as f64), (x as f64, -y as f64)); + if self.room() { + self.path.curve_to((cx0 as f64, -cy0 as f64), (cx1 as f64, -cy1 as f64), (x as f64, -y as f64)); + } } fn close(&mut self) { - self.0.close_path(); + if self.room() { + self.path.close_path(); + } } } @@ -1575,6 +1655,55 @@ mod font_dir_tests { assert_eq!(capped_named_instance_count(MAX_NAMED_INSTANCES_PER_FACE + 1), MAX_NAMED_INSTANCES_PER_FACE); } + #[test] + fn faces_of_one_file_are_bounded_across_the_collection() { + // The per-collection and per-face limits together would allow far more. + assert!((MAX_COLLECTION_FACES as usize) * MAX_NAMED_INSTANCES_PER_FACE > 16 * MAX_ENUMERATED_FACES); + for data in bundled() { + assert!(enumerate_faces(data).len() <= MAX_ENUMERATED_FACES); + } + } + + #[test] + fn an_outline_past_the_element_limit_draws_nothing() { + let mut pen = FlipPen::new(8); + pen.move_to(0.0, 0.0); + pen.line_to(1.0, 1.0); + pen.close(); + assert_eq!(pen.finish().elements().len(), 3); + // A shared outline drawn over and over (composite and VARC glyphs). + let mut pen = FlipPen::new(8); + for i in 0..1_000_000 { + pen.move_to(0.0, 0.0); + pen.line_to(i as f32, 1.0); + pen.close(); + } + assert_eq!(pen.path.elements().len(), 0); + assert!(pen.finish().elements().is_empty()); + } + + #[test] + fn outline_cache_is_bounded_by_elements() { + let mut path = BezPath::new(); + path.move_to((0.0, 0.0)); + for i in 1..MAX_OUTLINE_ELEMENTS { + path.line_to((i as f64, 0.0)); + } + let path = Arc::new(path); + let mut cache = OutlineCache::default(); + let glyphs = 4 * OUTLINE_CACHE_MAX_ELEMENTS / MAX_OUTLINE_ELEMENTS; + for gid in 0..glyphs as u32 { + cache.insert((1, gid), path.clone()); + assert!(cache.elements <= OUTLINE_CACHE_MAX_ELEMENTS); + assert_eq!(cache.elements, cache.paths.values().map(|p| p.elements().len()).sum::()); + } + assert!(cache.get(&(1, glyphs as u32 - 1)).is_some()); + // Replacing an entry does not count it twice. + let before = cache.elements; + cache.insert((1, glyphs as u32 - 1), path.clone()); + assert_eq!(cache.elements, before); + } + #[test] fn xdg_data_dirs_contribute_their_fonts_folders() { let home = Path::new("/home/ann"); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 6f345e11c..4125c2905 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -247,6 +247,11 @@ pub fn repair(p: &mut Presentation) { if !(p.notes_size.width.is_finite() && p.notes_size.width > 0.0 && p.notes_size.height.is_finite() && p.notes_size.height > 0.0) { p.notes_size = Presentation::default().notes_size; } + // A notes page is bounded like a slide: canvas and handout drawing scale with it. + p.notes_size = deckcraft_model::Size::new( + p.notes_size.width.clamp(*SLIDE_SIDE.start(), *SLIDE_SIDE.end()), + p.notes_size.height.clamp(*SLIDE_SIDE.start(), *SLIDE_SIDE.end()), + ); sane_numbers(p); } @@ -689,7 +694,9 @@ pub fn slides_to_outline(p: &Presentation) -> String { out.push_str(&s.title().replace('\n', " ")); out.push('\n'); for sh in &s.shapes { - if sh.ph_type().is_some_and(|k| k.is_title()) { + // The title is the slide's line; footer, date and slide-number placeholders are not + // body text (PowerPoint's outline view shows title and body placeholders only). + if sh.ph_type().is_some_and(|k| k.is_title() || k.is_footer_kind()) { continue; } if let Some(t) = &sh.text { diff --git a/crates/format/src/rtf.rs b/crates/format/src/rtf.rs index 4800aba26..4e7e29583 100644 --- a/crates/format/src/rtf.rs +++ b/crates/format/src/rtf.rs @@ -71,7 +71,8 @@ pub fn slides_to_rtf(p: &Presentation) -> String { } out.push_str("\\b0\\par\n"); for sh in &s.shapes { - if sh.ph_type().is_some_and(|k| k.is_title()) { + // As in the text outline: footer, date and slide-number placeholders are not body text. + if sh.ph_type().is_some_and(|k| k.is_title() || k.is_footer_kind()) { continue; } let Some(t) = &sh.text else { continue }; @@ -299,7 +300,8 @@ impl Outline<'_> { /// A one-character control symbol (`\\`, `\{`, `\~`, ...). fn symbol(&mut self, n: u8) { - self.i += 1; + let c = self.rtf.get(self.i..).and_then(|r| r.chars().next()); + self.i = advance(self.rtf, self.i, 1); if self.st.skip { return; } @@ -308,6 +310,7 @@ impl Outline<'_> { b'~' => self.line.push('\u{a0}'), b'_' => self.line.push('\u{2011}'), b'\n' | b'\r' => self.flush(), + _ if !n.is_ascii() => self.line.extend(c), _ => {} } } @@ -419,6 +422,11 @@ mod tests { assert_eq!(rtf_to_outline(r"{\rtf1 x\u-70000?y\par}"), "x\u{ee90}y\n"); } + #[test] + fn multibyte_control_symbol_keeps_character_and_following_text() { + assert_eq!(rtf_to_outline("{\\rtf1 A\\éB\\par}"), "AéB\n"); + } + /// `\\_` is RTF's non-breaking hyphen: a visible character, not markup. #[test] fn non_breaking_hyphen_is_kept() { diff --git a/crates/format/src/tests.rs b/crates/format/src/tests.rs index aa692d03f..ccda612f0 100644 --- a/crates/format/src/tests.rs +++ b/crates/format/src/tests.rs @@ -451,6 +451,13 @@ fn degenerate_slide_sizes_are_repaired() { } } +#[test] +fn repair_bounds_the_notes_page_size() { + let mut p = Presentation { notes_size: deckcraft_model::Size::new(1e12, 0.01), ..Default::default() }; + repair(&mut p); + assert_eq!((p.notes_size.width, p.notes_size.height), (4032.0, 72.0)); +} + /// A file whose slide size is null, a string or missing a side (written by hand or by /// another tool; NaN is also saved as null) opens with the default size rather than failing /// as damaged. diff --git a/crates/geom/src/morph.rs b/crates/geom/src/morph.rs index 411da5f93..154db5e87 100644 --- a/crates/geom/src/morph.rs +++ b/crates/geom/src/morph.rs @@ -126,6 +126,21 @@ fn split_curve(c: &[Point], n: usize, out: &mut Vec) { out.extend([p1, p2, p3]); } +/// A piece length ordered totally, so it can sit in a heap. +#[derive(PartialEq)] +struct Share(f64); +impl Eq for Share {} +impl PartialOrd for Share { + fn partial_cmp(&self, o: &Self) -> Option { + Some(self.cmp(o)) + } +} +impl Ord for Share { + fn cmp(&self, o: &Self) -> std::cmp::Ordering { + self.0.total_cmp(&o.0) + } +} + /// Brings a contour to `n` curves. Unlike manim (which splits every curve equally often), the extra /// splits go to the longest pieces, so a long side gets more points than a short one. fn refine(c: &Contour, n: usize) -> Contour { @@ -136,11 +151,15 @@ fn refine(c: &Contour, n: usize) -> Contour { let curves: Vec<&[Point]> = (0..have).filter_map(|k| c.pts.get(3 * k..3 * k + 4)).collect(); let len: Vec = curves.iter().map(|q| curve_length(q) + 1e-9).collect(); let mut parts = vec![1usize; curves.len()]; + // The curve with the longest piece gets the next split (the lowest index on a tie); a heap keeps + // this O(n log n) instead of rescanning every curve for each split. + let mut heap: std::collections::BinaryHeap<(Share, std::cmp::Reverse)> = + len.iter().enumerate().map(|(k, l)| (Share(*l), std::cmp::Reverse(k))).collect(); for _ in 0..n - have { - let share = |k: usize| len.get(k).copied().unwrap_or(0.0) / parts.get(k).copied().unwrap_or(1) as f64; - let best = (1..curves.len()).fold(0, |b, k| if share(k) > share(b) { k } else { b }); + let Some((_, std::cmp::Reverse(best))) = heap.pop() else { break }; if let Some(p) = parts.get_mut(best) { *p += 1; + heap.push((Share(len.get(best).copied().unwrap_or(0.0) / *p as f64), std::cmp::Reverse(best))); } } let mut out = Vec::with_capacity(1 + 3 * n); @@ -420,6 +439,16 @@ mod tests { assert_eq!(contours(&to_path(&cs)), cs); } + #[test] + fn refine_many_curves_finishes_at_the_asked_count() { + let mut c = Contour { closed: false, pts: vec![Point::ZERO] }; + for i in 1..=20_000 { + line_to(&mut c.pts, Point::new(f64::from(i), f64::from(i % 7))); + } + let r = refine(&c, 40_000); + assert_eq!(r.curves(), 40_000); + } + #[test] fn refine_splits_the_longest_curves_and_keeps_the_shape() { let c = Contour { closed: false, pts: vec![Point::ZERO, Point::new(1.0, 0.0), Point::new(2.0, 0.0), Point::new(3.0, 0.0)] }; diff --git a/crates/geom/src/warp.rs b/crates/geom/src/warp.rs index d3035d512..81376d7d7 100644 --- a/crates/geom/src/warp.rs +++ b/crates/geom/src/warp.rs @@ -928,11 +928,25 @@ impl Warp { k_fill = 1.0; } zoom = zoom.max(k_fill); + // Per band, measured once: its widest line and its first and last lines' baselines. + let mut stats: Vec> = vec![None; nb]; + for (l, &b) in lines.iter().zip(&band_of) { + let w = l.extent.1 - l.extent.0; + if let Some(slot) = stats.get_mut(b) { + match slot { + Some((widest, _, last)) => { + *widest = widest.max(w); + *last = l.ref_y; + } + None => *slot = Some((0.0f64.max(w), l.ref_y, l.ref_y)), + } + } + } let mut maps = Vec::with_capacity(n); let mut offsets = Vec::new(); for (i, l) in lines.iter().enumerate() { let b = band_of.get(i).copied().unwrap_or(0); - let in_band = |j: &usize| band_of.get(*j) == Some(&b); + let band = stats.get(b).copied().flatten(); match self.bands.get(b) { Some(Band::Fill { .. }) | None => { let (y0, y1) = band_y.get(b).copied().flatten().unwrap_or((0.0, 1.0)); @@ -941,16 +955,12 @@ impl Warp { } Some(Band::Follow { path, anchor }) => { let len = path.length(); - let widest = (0..n).filter(in_band).filter_map(|j| lines.get(j)).map(|l| l.extent.1 - l.extent.0).fold(0.0, f64::max); + let widest = band.map_or(0.0, |s| s.0); let k = if widest > len && widest > 1e-9 { len / widest } else { 1.0 }; - let members: Vec<&WarpLine> = (0..n).filter(in_band).filter_map(|j| lines.get(j)).collect(); let ref_y = match anchor { - LineAnchor::Last => members.last().map(|l| l.ref_y), - LineAnchor::First => members.first().map(|l| l.ref_y), - LineAnchor::Middle => match (members.first(), members.last()) { - (Some(a), Some(z)) => Some((a.ref_y + z.ref_y) / 2.0), - _ => None, - }, + LineAnchor::Last => band.map(|s| s.2), + LineAnchor::First => band.map(|s| s.1), + LineAnchor::Middle => band.map(|s| (s.1 + s.2) / 2.0), } .unwrap_or(l.ref_y); // Lines off the guide run on a parallel curve at their natural length. diff --git a/crates/geom/src/warp/tests.rs b/crates/geom/src/warp/tests.rs index c85b2b0ae..c4aa1bbd0 100644 --- a/crates/geom/src/warp/tests.rs +++ b/crates/geom/src/warp/tests.rs @@ -319,3 +319,20 @@ fn arch_and_inflate_handles_follow_the_pointer() { assert!(more > adjust_defaults("textInflate")[0], "{more}"); assert!((more - 20_000.0).abs() < 1.0, "{more}"); } + +#[test] +fn many_lines_on_a_follow_band_are_mapped_from_one_measurement() { + // 50,000 lines on an arch: each line's scale and anchor come from the band measured once. + let w = warp("textArchUp", &[], rect()).unwrap(); + let inner = Rect::new(7.2, 3.6, W - 7.2, H - 3.6); + let lines: Vec = (0..50_000) + .map(|i| { + let y = i as f64 * 0.01; + WarpLine { ink: Some(Rect::new(190.0, y, 230.0, y + 1.0)), extent: (190.0, 230.0), ref_y: y + 1.0 } + }) + .collect(); + let m = w.mapper(&lines, inner); + // The last line sits on the arc, as with two lines. + let last = lines.len() - 1; + assert!(m.map(last, Point::new(210.0, lines[last].ref_y)).y.abs() < 0.05); +} diff --git a/crates/model/src/edit.rs b/crates/model/src/edit.rs index 3e21e3bf8..8f79d9c4c 100644 --- a/crates/model/src/edit.rs +++ b/crates/model/src/edit.rs @@ -78,45 +78,57 @@ pub fn insert(body: &mut TextBody, pos: Pos, text: &str, props: Option if body.paragraphs.is_empty() { body.paragraphs.push(Paragraph::default()); } - let (mut pi, mut ch) = clamp(body, pos); - let props = props.unwrap_or_else(|| props_at(body, (pi, ch))); + let (pi0, mut ch) = clamp(body, pos); + let props = props.unwrap_or_else(|| props_at(body, (pi0, ch))); let text = text.replace("\r\n", "\n").replace('\r', "\n"); + // Work on the paragraph being edited apart from the body, so each new paragraph is pushed to a + // list and the body's vector is spliced once instead of shifted for every `\n`. + let Some(slot) = body.paragraphs.get_mut(pi0) else { return (pi0, ch) }; + let mut cur = std::mem::take(slot); + let mut done: Vec = Vec::new(); let mut first = true; for part in text.split('\n') { if !first { - // Split paragraph pi at ch. - let Some(p) = body.paragraphs.get_mut(pi) else { break }; - let idx = split_at(p, ch); - let tail: Vec = p.runs.drain(idx..).collect(); - let newp = - Paragraph { level: p.level, props: p.props.clone(), runs: tail, def_props: p.def_props.clone(), end_props: p.end_props.clone() }; - p.end_props = props.clone(); - body.paragraphs.insert(pi + 1, newp); - pi += 1; + // Split the current paragraph at ch. + let idx = split_at(&mut cur, ch); + let tail: Vec = cur.runs.drain(idx..).collect(); + let newp = Paragraph { + level: cur.level, + props: cur.props.clone(), + runs: tail, + def_props: cur.def_props.clone(), + end_props: cur.end_props.clone(), + }; + cur.end_props = props.clone(); + done.push(std::mem::replace(&mut cur, newp)); ch = 0; } first = false; + // The run index just after the last insertion: the next one lands there with no rescan. + let mut next: Option = None; for (k, seg) in part.split('\u{b}').enumerate() { - let Some(p) = body.paragraphs.get_mut(pi) else { break }; if k > 0 { - let idx = split_at(p, ch); - p.runs.insert(idx, Run { text: String::new(), props: props.clone(), kind: RunKind::Break }); + let idx = next.unwrap_or_else(|| split_at(&mut cur, ch)); + cur.runs.insert(idx, Run { text: String::new(), props: props.clone(), kind: RunKind::Break }); ch += 1; + next = Some(idx + 1); } if seg.is_empty() { continue; } - let idx = split_at(p, ch); - p.runs.insert(idx, Run::with(seg, props.clone())); + let idx = next.unwrap_or_else(|| split_at(&mut cur, ch)); + cur.runs.insert(idx, Run::with(seg, props.clone())); ch += seg.chars().count(); + next = Some(idx + 1); } - if let Some(p) = body.paragraphs.get_mut(pi) { - p.normalize(); - if p.runs.is_empty() { - p.end_props = props.clone(); - } + cur.normalize(); + if cur.runs.is_empty() { + cur.end_props = props.clone(); } } + let pi = pi0 + done.len(); + done.push(cur); + body.paragraphs.splice(pi0..=pi0, done); (pi, ch) } @@ -252,8 +264,16 @@ pub fn slice(body: &TextBody, a: Pos, b: Pos) -> TextBody { /// Insert a formatted body at `pos` (paste): first paragraph merges into the current one. pub fn insert_body(body: &mut TextBody, pos: Pos, src: &TextBody) -> Pos { let mut caret = clamp(body, pos); + // Plain text runs go straight in at the run index after the previous one and the paragraph is + // normalized once, not rescanned and merged for every source run. + let mut next: Option = None; + let mut dirty = false; for (k, sp) in src.paragraphs.iter().enumerate() { if k > 0 { + if let (true, Some(p)) = (dirty, body.paragraphs.get_mut(caret.0)) { + p.normalize(); + } + (next, dirty) = (None, false); caret = insert(body, caret, "\n", None); if let Some(p) = body.paragraphs.get_mut(caret.0) { p.level = sp.level; @@ -261,6 +281,22 @@ pub fn insert_body(body: &mut TextBody, pos: Pos, src: &TextBody) -> Pos { } } for r in &sp.runs { + if r.kind == RunKind::Text + && !r.text.is_empty() + && !r.text.contains(['\n', '\r', '\u{b}']) + && let Some(p) = body.paragraphs.get_mut(caret.0) + { + let idx = next.unwrap_or_else(|| split_at(p, caret.1)); + p.runs.insert(idx, Run::with(&r.text, r.props.clone())); + caret.1 += r.text.chars().count(); + (next, dirty) = (Some(idx + 1), true); + continue; + } + // Anything else takes the general path, after the pending runs are merged. + if let (true, Some(p)) = (dirty, body.paragraphs.get_mut(caret.0)) { + p.normalize(); + } + (next, dirty) = (None, false); match r.kind { RunKind::Text => caret = insert(body, caret, &r.text, Some(r.props.clone())), RunKind::Break => caret = insert(body, caret, "\u{b}", Some(r.props.clone())), @@ -274,6 +310,9 @@ pub fn insert_body(body: &mut TextBody, pos: Pos, src: &TextBody) -> Pos { } } } + if let (true, Some(p)) = (dirty, body.paragraphs.get_mut(caret.0)) { + p.normalize(); + } caret } @@ -568,6 +607,74 @@ mod tests { assert_eq!(caret, (0, 5)); } + /// The paste as it ran before the fast path: one `insert` (and normalize) per source run. + fn insert_body_by_runs(body: &mut TextBody, pos: Pos, src: &TextBody) -> Pos { + let mut caret = clamp(body, pos); + for (k, sp) in src.paragraphs.iter().enumerate() { + if k > 0 { + caret = insert(body, caret, "\n", None); + if let Some(p) = body.paragraphs.get_mut(caret.0) { + p.level = sp.level; + p.props = sp.props.clone(); + } + } + for r in &sp.runs { + match r.kind { + RunKind::Text => caret = insert(body, caret, &r.text, Some(r.props.clone())), + RunKind::Break => caret = insert(body, caret, "\u{b}", Some(r.props.clone())), + _ => { + if let Some(p) = body.paragraphs.get_mut(caret.0) { + let idx = split_at(p, caret.1); + p.runs.insert(idx, r.clone()); + caret.1 += r.char_len(); + } + } + } + } + } + caret + } + + #[test] + fn paste_matches_inserting_run_by_run() { + let bold = |t: &str| Run::with(t, RunProps { bold: Some(true), ..Default::default() }); + let mut src = body(""); + src.paragraphs = vec![ + Paragraph { + runs: vec![Run::with("ab", RunProps::default()), Run::with("cd", RunProps::default()), bold("ef"), bold("g")], + ..Default::default() + }, + Paragraph { + runs: vec![ + bold("h"), + Run { text: String::new(), props: RunProps::default(), kind: RunKind::Break }, + Run::with("i", RunProps::default()), + ], + level: 1, + ..Default::default() + }, + Paragraph { runs: vec![Run::with("jk", RunProps::default())], ..Default::default() }, + ]; + for at in [0, 1, 3, 6] { + let mut dest = body("XYZabc"); + format(&mut dest, (0, 2), (0, 4), &|p| p.italic = Some(true)); + let mut want = dest.clone(); + let want_caret = insert_body_by_runs(&mut want, (0, at), &src); + let caret = insert_body(&mut dest, (0, at), &src); + assert_eq!(caret, want_caret, "caret at {at}"); + assert_eq!(format!("{:?}", dest.paragraphs), format!("{:?}", want.paragraphs), "paste at {at}"); + } + } + + #[test] + fn multiline_insert_splits_into_paragraphs() { + let mut b = body("abcd"); + let c = insert(&mut b, (0, 2), "x\ny\u{b}z\n\nw", None); + assert_eq!(c, (3, 1)); + let texts: Vec = b.paragraphs.iter().map(Paragraph::text).collect(); + assert_eq!(texts, ["abx", "y\u{b}z", "", "wcd"]); + } + #[test] fn words_and_moves() { let b = body("Hello brave world"); diff --git a/crates/ogg/src/lib.rs b/crates/ogg/src/lib.rs index 77c081d52..0261ee0e6 100644 --- a/crates/ogg/src/lib.rs +++ b/crates/ogg/src/lib.rs @@ -415,6 +415,11 @@ impl CaptureScanner { } fn read_parts(src: &(impl ByteSource + ?Sized), parts: &[(u64, u32)]) -> Result> { + // Ranges come from a scan of some source; check them against this one before sizing a buffer. + let len = src.len(); + if parts.iter().any(|&(o, n)| o.checked_add(u64::from(n)).is_none_or(|end| end > len)) { + return Err(Error::Invalid("packet range lies outside the source".into())); + } let mut v = Vec::with_capacity(parts.iter().map(|p| p.1 as usize).sum()); for &(o, n) in parts { let a = v.len(); @@ -508,6 +513,7 @@ impl OpusTiming { c = timing_add(c, i64::from(durations[k]))?; ends[k] = Some(c); } + cursor = Some(c); break; } let Some(g) = stream.packets[j].granule else { break }; diff --git a/crates/ogg/src/tests.rs b/crates/ogg/src/tests.rs index 480c1cda2..728e618bd 100644 --- a/crates/ogg/src/tests.rs +++ b/crates/ogg/src/tests.rs @@ -183,6 +183,27 @@ fn opus_toc_durations() { assert_eq!(opus_packet_samples([0xF8, 0], 0), None); } +#[test] +fn opus_timing_includes_packets_after_the_last_known_granule() { + let f = opus_file(6, 312, 0); + let o = open(&f).unwrap(); + let mut stream = o.streams[0].clone(); + // Missing granules on complete-packet pages are non-conforming. The existing recovery + // fallback still assigns their packet positions; total must include the same samples. + stream.packets.last_mut().unwrap().granule = None; + let timing = OpusTiming::of(&stream, 312).unwrap(); + assert_eq!(timing.starts, vec![-312, 648, 1608, 2568, 3528, 4488]); + assert_eq!(timing.total, 6 * 960 - 312); + + // With no usable granule, every complete packet still contributes its TOC duration. + for packet in &mut stream.packets { + packet.granule = None; + } + let timing = OpusTiming::of(&stream, 312).unwrap(); + assert_eq!(timing.starts, vec![-312, 648, 1608, 2568, 3528, 4488]); + assert_eq!(timing.total, 6 * 960 - 312); +} + #[test] fn opus_timing_rejects_a_granule_that_overflows() { let packet = |granule: i64| Stream { @@ -274,3 +295,13 @@ fn a_budget_too_small_to_search_is_spent() { let f = opus_file(6, 312, 0); assert_eq!(scanner.find(&f, 0, f.len() as u64).unwrap(), None); } + +#[test] +fn read_packet_checks_its_ranges_against_the_source_before_allocating() { + let f = opus_file(10, 312, 500); + let o = open(&f).unwrap(); + let s = o.stream_of(Codec::Opus).unwrap(); + // The last packet lies past the end of a source cut short: an error, not a read of missing bytes. + let short = &f[..f.len() / 2]; + assert!(matches!(o.read_packet(&short, s, 9), Err(Error::Invalid(_)))); +} diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index 6baeda314..79758aaad 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -307,7 +307,7 @@ pub fn master_text_pieces(pres: &Presentation, m: &deckcraft_model::Master, pape .body .paragraphs .get(run.para) - .map(|p| p.runs.iter().map(|r| r.text.as_str()).collect::()) + .map(|p| p.runs.iter().map(|r| r.content()).collect::()) .unwrap_or_default() .chars() .skip(run.chars.0) @@ -522,7 +522,8 @@ impl Exporter<'_> { let k = b.width() / sw; // Raster at the requested dpi relative to the box size on paper. let scale = raster_scale(b.width(), (sw, sh), self.opts.dpi); - let ropts = RenderOpts { scale, threads: 4, ..Default::default() }; + // One clock reading for the raster and the text layer, so a date field cannot differ between them. + let ropts = RenderOpts { scale, threads: 4, now: deckcraft_text::datetime::DateTime::now_local(), ..Default::default() }; // Two rasters per slide: the backdrop (background, master and layout graphics) is the same // on every slide that shares its layout, so the writer stores it once, and the slide's own // shapes draw over it (#845). @@ -563,7 +564,7 @@ impl Exporter<'_> { } if self.opts.text_layer { let view = Affine::translate((b.x0, b.y0)) * Affine::scale(k); - for pt in deckcraft_render::place_slide(pres, i).texts { + for pt in deckcraft_render::place_slide_with(pres, i, &ropts).texts { self.text_layer(s, &pt, view, false)?; } let _ = sh; @@ -581,7 +582,7 @@ impl Exporter<'_> { .body .paragraphs .get(run.para) - .map(|p| p.runs.iter().map(|r| r.text.as_str()).collect::()) + .map(|p| p.runs.iter().map(|r| r.content()).collect::()) .unwrap_or_default() .chars() .skip(run.chars.0) diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index b34e963e1..c34cab664 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -269,8 +269,6 @@ fn unembeddable_font_returns_error_instead_of_a_pdf_missing_text() { #[cfg_attr(not(target_arch = "wasm32"), test)] fn supported_unicode_is_shaped_and_recovered_from_the_page_text_layer() { use deckcraft_model::{Shape, ShapeId, Xfrm}; - use hayro_syntax::content::ops::TypedInstruction; - use hayro_syntax::object::{Dict, Object, Stream}; let text = "Café Ελληνικά Привет"; let face = FontDb::global().face("Ubuntu", "Regular"); @@ -292,8 +290,17 @@ fn supported_unicode_is_shaped_and_recovered_from_the_page_text_layer() { assert!(runs.iter().all(|run| run.face.family == "Ubuntu" && !run.glyphs.is_empty() && run.glyphs.iter().all(|glyph| glyph.0 != 0))); let bytes = export(&p, &opts(PageLayout::Slides)).unwrap(); - let pdf = hayro_syntax::Pdf::new(bytes.clone()).unwrap(); - let page = &pdf.pages()[0]; + assert_eq!(page_text(&bytes, 0), text); +} + +/// The text of page `index`, read the way a PDF reader does: the content stream's text-showing +/// operators decoded through the page font's ToUnicode map. +fn page_text(bytes: &[u8], index: usize) -> String { + use hayro_syntax::content::ops::TypedInstruction; + use hayro_syntax::object::{Dict, Object, Stream}; + + let pdf = hayro_syntax::Pdf::new(bytes.to_vec()).unwrap(); + let page = &pdf.pages()[index]; let resources = page.raw().get::>(b"Resources").unwrap(); let fonts = resources.get::>(b"Font").unwrap(); assert_eq!(fonts.len(), 1); @@ -344,7 +351,51 @@ fn supported_unicode_is_shaped_and_recovered_from_the_page_text_layer() { _ => {} } } - assert_eq!(extracted, text); + extracted +} + +fn ubuntu_runs(runs: Vec) -> TextBody { + let paragraph = deckcraft_model::text::Paragraph { runs, ..Default::default() }; + let mut body = TextBody { paragraphs: vec![paragraph], ..Default::default() }; + for run in &mut body.paragraphs[0].runs { + run.props.font = Some("Ubuntu".into()); + run.props.size = Some(20.0); + } + body +} + +fn deck_with_body(n: usize, body: TextBody) -> Presentation { + use deckcraft_model::{Shape, ShapeId, Xfrm}; + let mut p = deck(n); + for slide in &mut p.slides { + Arc::make_mut(slide).shapes = + vec![Shape { id: ShapeId(9001), xfrm: Some(Xfrm::new(20.0, 20.0, 920.0, 100.0)), text: Some(body.clone()), ..Default::default() }]; + } + p +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn slide_number_field_is_recovered_from_the_page_text_layer() { + use deckcraft_model::text::{Run, RunKind}; + let mut field = Run::new("<#>"); + field.kind = RunKind::Field { field: "slidenum".into() }; + let p = deck_with_body(3, ubuntu_runs(vec![Run::new("Page "), field])); + let bytes = export(&p, &opts(PageLayout::Slides)).unwrap(); + for (i, want) in ["Page 1", "Page 2", "Page 3"].into_iter().enumerate() { + assert_eq!(page_text(&bytes, i), want); + } +} + +#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test::wasm_bindgen_test)] +#[cfg_attr(not(target_arch = "wasm32"), test)] +fn line_break_keeps_the_text_layer_aligned_with_the_layout() { + use deckcraft_model::text::{Run, RunKind}; + let mut brk = Run::new(""); + brk.kind = RunKind::Break; + let p = deck_with_body(1, ubuntu_runs(vec![Run::new("one"), brk, Run::new("two")])); + let bytes = export(&p, &opts(PageLayout::Slides)).unwrap(); + assert_eq!(page_text(&bytes, 0), "onetwo"); } /// Notes pages and handouts print on the chosen paper, landscape included, with every slide box diff --git a/crates/pptx/src/opc.rs b/crates/pptx/src/opc.rs index 20a53bcb0..459a7334c 100644 --- a/crates/pptx/src/opc.rs +++ b/crates/pptx/src/opc.rs @@ -171,6 +171,7 @@ fn rels_of(doc: &xml::Doc, part: &str) -> Rels { let external = r.attr("TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("external")); let target_raw = target.to_string(); let target = if external { target_raw.clone() } else { resolve(&base, target) }; + out.by_id.entry(id.to_string()).or_insert(out.items.len()); out.items.push(Rel { id: id.to_string(), kind: ty.rsplit('/').next().unwrap_or(ty).to_string(), @@ -211,11 +212,13 @@ pub struct Rel { #[derive(Clone, Debug, Default)] pub struct Rels { pub items: Vec, + /// Id → index of its first relationship in `items`. + by_id: HashMap, } impl Rels { pub fn get(&self, id: &str) -> Option<&Rel> { - self.items.iter().find(|r| r.id == id) + self.by_id.get(id).and_then(|&i| self.items.get(i)) } pub fn target(&self, id: &str) -> Option<&str> { self.get(id).map(|r| r.target.as_str()) @@ -339,23 +342,29 @@ pub fn relative(from: &str, to: &str) -> String { #[derive(Default)] pub struct RelsOut { pub items: Vec<(String, String, String, bool)>, + /// (type, target, external) → id of the relationship [`RelsOut::add`] made for it. + added: HashMap<(String, String, bool), String>, } impl RelsOut { /// Add (or reuse) a relationship; `kind` is a full type URI or a suffix of the standard base. pub fn add(&mut self, kind: &str, target: &str, external: bool) -> String { let ty = if kind.starts_with("http") { kind.to_string() } else { format!("{RT_BASE}{kind}") }; - if let Some((id, ..)) = self.items.iter().find(|(_, t, tg, e)| *t == ty && tg == target && *e == external) { + let key = (ty, target.to_string(), external); + if let Some(id) = self.added.get(&key) { return id.clone(); } let id = format!("rId{}", self.items.len() + 1); - self.items.push((id.clone(), ty, target.to_string(), external)); + self.items.push((id.clone(), key.0.clone(), key.1.clone(), external)); + self.added.insert(key, id.clone()); id } /// Always add a new relationship even if an identical one exists. pub fn add_new(&mut self, kind: &str, target: &str, external: bool) -> String { let ty = if kind.starts_with("http") { kind.to_string() } else { format!("{RT_BASE}{kind}") }; let id = format!("rId{}", self.items.len() + 1); + // `add` reuses the first relationship for a target, whichever call made it. + self.added.entry((ty.clone(), target.to_string(), external)).or_insert_with(|| id.clone()); self.items.push((id.clone(), ty, target.to_string(), external)); id } @@ -500,6 +509,27 @@ mod tests { assert!(p.get("e").is_some()); } + #[test] + fn many_relationships_are_found_by_index_and_first_id_wins() { + let n = 50_000; + let mut x = String::from(r#""#); + for i in 0..n { + x.push_str(&format!(r#""#)); + } + x.push_str(&format!(r#""#)); + let rels = rels_from_bytes(x.as_bytes(), "ppt/slides/slide1.xml"); + assert_eq!(rels.target("rId0"), Some("ppt/media/image0.png"), "the first of a repeated id wins"); + assert!((0..n).all(|i| rels.target(&format!("rId{i}")) == Some(&*format!("ppt/media/image{i}.png")))); + + let mut out = RelsOut::default(); + let ids: Vec = (0..n).map(|i| out.add("hyperlink", &format!("https://example.com/{i}"), true)).collect(); + assert_eq!(out.add("hyperlink", "https://example.com/7", true), ids[7], "an identical target reuses its id"); + assert_eq!(out.add("hyperlink", "https://example.com/7", false), format!("rId{}", n + 1), "internal is distinct"); + let fresh = out.add_new("image", "../media/a.png", false); + assert_ne!(out.add_new("image", "../media/a.png", false), fresh); + assert_eq!(out.add("image", "../media/a.png", false), fresh, "add reuses the first add_new made"); + } + #[test] fn backslash_zip_entry_names_are_found() { let z = zip_of(&[("PPT\\Slides\\Slide1.xml", 3)]); diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 288e497f8..47f6229c3 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -46,6 +46,9 @@ pub struct Imp<'a> { pub tag_bytes: usize, /// The tag budget ran out and the warning was given. pub tags_capped: bool, + /// Whether a part has relationships of its own, by normalized part name: looked up once + /// however many embedded objects point at the part. + pub has_rels: HashMap, } impl Imp<'_> { @@ -446,6 +449,7 @@ pub fn import_theme(bytes: &[u8]) -> Result<(deckcraft_model::theme::Theme, Vec< tags_by_part: HashMap::new(), tag_bytes: 0, tags_capped: false, + has_rels: HashMap::new(), }; let part = Part::new(&pkg, &theme_path, &doc); let theme = theme::read_theme(&mut imp, &part, &doc.root); @@ -487,6 +491,7 @@ pub fn import_checked(bytes: &[u8]) -> Result<(Presentation, Vec), PptxE tags_by_part: HashMap::new(), tag_bytes: 0, tags_capped: false, + has_rels: HashMap::new(), }; // Slide ids first so hyperlinks anywhere can point at slides. let slide_ids = register_slides(&mut imp, &docs, &slide_entries, &mut lost_slides); @@ -663,52 +668,55 @@ fn read_slides( let pkg = imp.pkg; let fallback_layout = p.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id).unwrap_or_default(); let mut file_slide_ids: HashMap = HashMap::new(); - let mut notes_by_target: HashMap = HashMap::new(); + // A notes or comments part belongs to one slide; one shared by many must not be read (and + // copied) once per slide that names it. + let mut owned_parts: HashSet = HashSet::new(); for (file_id, path, id) in slide_ids { let Some(doc) = docs.get(&opc::norm(path)) else { continue }; let part = Part::new(pkg, path, doc); let layout = part.rels.first_of("slideLayout").and_then(|r| layout_by_part.get(&opc::norm(&r.target)).copied()).unwrap_or(fallback_layout); let mut slide = read_slide(imp, &part, doc, *id, layout); - if let Some(notes) = slide_notes(imp, &part, &mut notes_by_target) { + if let Some(notes) = slide_notes(imp, &part, &mut owned_parts) { slide.notes = notes; } - read_slide_comments(pkg, pres_part, &part, &mut slide); + read_slide_comments(pkg, pres_part, &part, &mut slide, &mut owned_parts); file_slide_ids.insert(*file_id, *id); p.slides.push(Arc::new(slide)); } file_slide_ids } -/// The slide's notes text, read once per notes part. -fn slide_notes(imp: &mut Imp, part: &Part, cache: &mut HashMap) -> Option { +/// The slide's notes text. A notes part is the notes of the first slide that names it. +fn slide_notes(imp: &mut Imp, part: &Part, owned: &mut HashSet) -> Option { let n = part.rels.first_of("notesSlide")?; - let key = opc::norm(&n.target); - if let Some(cached) = cache.get(&key) { - return Some(cached.clone()); + if !owned.insert(opc::norm(&n.target)) { + log::warn!("pptx: notes part {} is named by more than one slide; kept on the first", n.target); + return None; } let pkg = imp.pkg; let nd = load(pkg, &n.target)?; let np = Part::new(pkg, &n.target, &nd); - let notes = read_notes(imp, &np, &nd.root); - cache.insert(key, notes.clone()); - Some(notes) + Some(read_notes(imp, &np, &nd.root)) } /// Comments. A modern part and the legacy part can describe the same threads (PowerPoint /// writes both): the modern one wins, its flattened legacy copy is skipped. -fn read_slide_comments(pkg: &Package, pres_part: &Part, part: &Part, slide: &mut Slide) { - let mut parts: Vec<(Doc, bool)> = vec![]; +/// Each comments part is read once, for the first slide that names it. +fn read_slide_comments(pkg: &Package, pres_part: &Part, part: &Part, slide: &mut Slide, owned: &mut HashSet) { + let mut parts: Vec<(Doc, bool, bool)> = vec![]; for c in part.rels.all_of("comments") { + // An already-owned part is not re-read, but its kind still decides modern precedence. + let fresh = owned.insert(opc::norm(&c.target)); if let Some(cd) = pkg.xml(&c.target) { // The content type names the modern part; a package that types it only through a // Default entry instead of an Override is still modern when its root is p188. let modern = pkg.content_type(&c.target).as_deref() == Some(crate::write::CT_COMMENTS_MODERN) || cd.root.prefix() == "p188"; - parts.push((cd, modern)); + parts.push((cd, modern, fresh)); } } - let has_modern = parts.iter().any(|(_, m)| *m); - for (cd, modern) in parts { - if has_modern && !modern { + let has_modern = parts.iter().any(|(_, m, _)| *m); + for (cd, modern, fresh) in parts { + if !fresh || (has_modern && !modern) { continue; } slide.comments.extend(read_comments(pkg, pres_part, &cd.root, modern)); @@ -1343,6 +1351,7 @@ fn read_show(sp: &El, shows: &HashMap) -> ShowSettings { without_narration: sp.bool("showNarration") == Some(false), without_animation: sp.bool("showAnimation") == Some(false), use_timings: sp.bool("useTimings").unwrap_or(true), + show_media_controls: sp.find("showMediaCtrls").and_then(|e| e.bool("val")).unwrap_or(true), ..Default::default() }; if sp.child("browse").is_some() { diff --git a/crates/pptx/src/read/dml.rs b/crates/pptx/src/read/dml.rs index 2d82fa36a..c8902e4c7 100644 --- a/crates/pptx/src/read/dml.rs +++ b/crates/pptx/src/read/dml.rs @@ -664,12 +664,22 @@ fn push_text_runs(para: &mut Paragraph, props: RunProps, text: &str) { /// Appends an Office Math (a14:m) run: its plain text and its OMML. fn push_math_run(para: &mut Paragraph, part: &Part, e: &El) { if para.runs.len() < MAX_PARAGRAPH_RUNS { - let mut text = String::new(); - let mut ts = vec![]; - e.find_all("t", &mut ts); - for t in ts { - text.push_str(&t.text()); + // The text of the outermost `t` elements: a `t` nested in another is already part of + // the outer one's text, and taking it again would copy it once per nesting level. + fn outer_t(e: &El, depth: usize, text: &mut String) { + if depth > crate::xml::MAX_DEPTH { + return; + } + for c in e.elements() { + if c.is("t") { + text.push_str(&c.text()); + } else { + outer_t(c, depth + 1, text); + } + } } + let mut text = String::new(); + outer_t(e, 0, &mut text); let omml = e.elements().map(|c| part.keep_inner(c)).collect::(); para.runs.push(Run { text, props: RunProps::default(), kind: RunKind::Math { omml } }); } @@ -714,6 +724,17 @@ mod tests { use super::*; use crate::xml::parse; + #[test] + fn nested_math_text_is_taken_once() { + // 200 `m:t` nested in each other around one word: the word, not 200 copies of it. + let x = format!(r#"{}word{}"#, "".repeat(200), "".repeat(200)); + let d = parse(x.as_bytes()).unwrap(); + let part = Part { name: "ppt/slides/slide1.xml".into(), rels: Default::default(), ns: vec![], inner_ns: vec![] }; + let mut para = Paragraph::default(); + push_math_run(&mut para, &part, &d.root); + assert_eq!(para.runs[0].text, "word"); + } + #[test] fn colors_and_transforms() { let d = parse( diff --git a/crates/pptx/src/read/shapes.rs b/crates/pptx/src/read/shapes.rs index b3e3d48e6..3230db100 100644 --- a/crates/pptx/src/read/shapes.rs +++ b/crates/pptx/src/read/shapes.rs @@ -475,7 +475,8 @@ fn frame(imp: &mut Imp, part: &Part, ctx: &mut IdCtx, e: &El, depth: usize) -> S .into_iter() .map(|id| { let rel = part.rels.get(&id).filter(|r| !r.external)?; - if !imp.pkg.rels(&rel.target).items.is_empty() { + let pkg = imp.pkg; + if *imp.has_rels.entry(crate::opc::norm(&rel.target)).or_insert_with(|| !pkg.rels(&rel.target).items.is_empty()) { return None; } let (kind, target) = (rel.ty.clone(), rel.target.clone()); diff --git a/crates/pptx/src/write.rs b/crates/pptx/src/write.rs index 3d80adef1..e4c0858c9 100644 --- a/crates/pptx/src/write.rs +++ b/crates/pptx/src/write.rs @@ -1206,6 +1206,14 @@ fn pres_props_xml(p: &Presentation) -> Vec { w.open0("p:penClr"); w.val("a:srgbClr", s.pen_color.hex()); w.close("p:penClr"); + if !s.show_media_controls { + // "Show media controls" is PowerPoint's p14 extension; absent means shown. + w.open0("p:extLst"); + w.open("p:ext", A::new().a("uri", "{2FDB2607-1784-4EEB-B798-7EB5836EED8A}")); + w.empty("p14:showMediaCtrls", A::new().a("xmlns:p14", NS_P14).a("val", 0)); + w.close("p:ext"); + w.close("p:extLst"); + } w.close("p:showPr"); w.close("p:presentationPr"); w.finish() diff --git a/crates/pptx/src/write/dml.rs b/crates/pptx/src/write/dml.rs index 85041596b..8acc648d0 100644 --- a/crates/pptx/src/write/dml.rs +++ b/crates/pptx/src/write/dml.rs @@ -762,17 +762,27 @@ pub fn text_body(w: &mut W, x: &mut Exp, o: &mut Out, tag: &str, tb: &TextBody, } fn run_text(w: &mut W, x: &mut Exp, o: &mut Out, text: &str, props: &RunProps) { + // The run's properties are the same for every piece and line break: serialize them once. + let mut rpr_xml: Option = None; + let mut put_rpr = |w: &mut W| match &rpr_xml { + Some(xml) => w.s.push_str(xml), + None => { + let start = w.s.len(); + rpr(w, x, o, "a:rPr", props); + rpr_xml = Some(w.s[start..].to_string()); + } + }; for (i, piece) in text.split(['\n', '\u{b}']).enumerate() { if i > 0 { w.open0("a:br"); - rpr(w, x, o, "a:rPr", props); + put_rpr(w); w.close("a:br"); } if piece.is_empty() && i > 0 { continue; } w.open0("a:r"); - rpr(w, x, o, "a:rPr", props); + put_rpr(w); w.elt("a:t", piece); w.close("a:r"); } diff --git a/crates/pptx/src/write/timing.rs b/crates/pptx/src/write/timing.rs index f2f3f2736..6376e3ca0 100644 --- a/crates/pptx/src/write/timing.rs +++ b/crates/pptx/src/write/timing.rs @@ -1,7 +1,7 @@ //! Transitions and the animation timing tree. use deckcraft_model::anim::{AnimClass, AnimStart, Animation, BuildRole, TextBuild, Transition, build_filled, paragraph_build_roles}; -use deckcraft_model::{Shape, ShapeId, ShapeKind, find_shape}; +use deckcraft_model::{Shape, ShapeId, ShapeKind}; use super::shapes::IdMap; use super::{Exp, Out}; @@ -577,6 +577,21 @@ fn click_pars(w: &mut W, x: &mut Exp, ids: &mut Ids, clicks: &Groups, interactiv } } +/// Most effects one slide's timing writes; animations past it are left out. +const MAX_EFFECTS: usize = 100_000; + +/// The shapes of the tree by id, the first in document order for a repeated id (as `find_shape` +/// finds it), so each animation resolves its shape without a tree walk. +fn shape_index<'a>(shapes: &'a [Shape], depth: usize, out: &mut std::collections::HashMap) { + if depth > 64 { + return; + } + for s in shapes { + out.entry(s.id).or_insert(s); + shape_index(s.children(), depth + 1, out); + } +} + /// Write `p:timing` for a slide's animations. pub fn timing(w: &mut W, x: &mut Exp, anims: &[Animation], shapes: &[Shape], idmap: &IdMap) { let mut grp_count: std::collections::HashMap = Default::default(); @@ -585,16 +600,31 @@ pub fn timing(w: &mut W, x: &mut Exp, anims: &[Animation], shapes: &[Shape], idm let mut bld: Vec<(u32, u32, bool)> = vec![]; // The last paragraph effect written: (shape, class, trigger, grpId). let mut prev_para: Option<(ShapeId, AnimClass, Option, u32)> = None; + let mut by_id = std::collections::HashMap::new(); + shape_index(shapes, 0, &mut by_id); + let mut roles_of: std::collections::HashMap> = Default::default(); + let mut effect_count = 0usize; + let mut warned = false; for a in anims.iter().take(10_000) { + if effect_count >= MAX_EFFECTS { + if !warned { + log::warn!("pptx: more than {MAX_EFFECTS} animation effects; the rest are left out"); + } + break; + } let Some(spid) = idmap.get(a.shape) else { log::warn!("pptx: animation on missing shape {} skipped", a.shape); continue; }; - let Some(shape) = find_shape(shapes, a.shape) else { continue }; + let Some(&shape) = by_id.get(&a.shape) else { continue }; let text_sp = matches!(shape.kind, ShapeKind::Shape) && shape.text.is_some(); let is_text_sp = matches!(shape.kind, ShapeKind::Shape) && shape.text.as_ref().is_some_and(|t| !t.is_empty()); let text = shape.text.as_ref().filter(|_| a.text_build == TextBuild::ByParagraph && is_text_sp); - let roles = text.map(paragraph_build_roles).unwrap_or_default(); + let no_roles = Vec::new(); + let roles = match text { + Some(t) => roles_of.entry(a.shape).or_insert_with(|| paragraph_build_roles(t)), + None => &no_roles, + }; let by_para = roles.contains(&BuildRole::Top); // PowerPoint keeps a shape's consecutive paragraph effects in one build (`grpId`) marked // `build="p"`; the reader turns them back into a by-paragraph build only if that plays them @@ -632,11 +662,19 @@ pub fn timing(w: &mut W, x: &mut Exp, anims: &[Animation], shapes: &[Shape], idm _ => continue, }; let Ok(pi) = u32::try_from(i) else { break }; + if effect_count + effs.len() >= MAX_EFFECTS { + if !warned { + log::warn!("pptx: more than {MAX_EFFECTS} animation effects; the rest are left out"); + warned = true; + } + break; + } effs.push(Eff { a, spid, start, paragraph: Some(pi), grp }); } } else { effs.push(Eff { a, spid, start: a.start, paragraph: a.paragraph, grp }); } + effect_count += effs.len(); if text_sp && !joins { bld.push((spid, grp, by_para || para_effect)); } diff --git a/crates/pptx/tests/it/comments.rs b/crates/pptx/tests/it/comments.rs index 9713b0f42..9b0e85bde 100644 --- a/crates/pptx/tests/it/comments.rs +++ b/crates/pptx/tests/it/comments.rs @@ -146,6 +146,33 @@ fn comments_on_several_slides_round_trip() { assert_eq!(q.slides[2].comments[0].text, "Slide three"); } +/// A slide that names a modern part another slide already owns, plus its own legacy part, must +/// not import the legacy copy: modern precedence holds even when the modern part is not re-read. +#[test] +fn owned_modern_part_still_suppresses_later_legacy() { + let mut p = deck(); + let layout = p.slides[0].layout; + let s2 = defaults::new_slide(&mut p, layout).expect("slide"); + p.slides.push(Arc::new(s2)); + Arc::make_mut(&mut p.slides[1]).comments = vec![Comment { author: "Cara".into(), text: "Slide two".into(), ..Default::default() }]; + let bytes = deckcraft_pptx::export(&p).expect("export"); + let rels2 = part(&bytes, "ppt/slides/_rels/slide2.xml.rels").replace("modernComment2.xml", "modernComment1.xml"); + let mut z = zip::ZipArchive::new(std::io::Cursor::new(&bytes)).expect("zip"); + let mut out = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + for i in 0..z.len() { + let mut f = z.by_index(i).expect("entry"); + out.start_file(f.name(), zip::write::SimpleFileOptions::default()).expect("start"); + if f.name() == "ppt/slides/_rels/slide2.xml.rels" { + std::io::Write::write_all(&mut out, rels2.as_bytes()).expect("write"); + } else { + std::io::copy(&mut f, &mut out).expect("copy"); + } + } + let q = deckcraft_pptx::import(&out.finish().expect("finish").into_inner()).expect("import"); + assert_eq!(q.slides[0].comments.len(), 2); + assert!(q.slides[1].comments.is_empty(), "{:?}", q.slides[1].comments); +} + /// A legacy-only package (what older tools read and write) still imports: 1/8 pt positions, /// flattened replies. #[test] diff --git a/crates/pptx/tests/it/malformed.rs b/crates/pptx/tests/it/malformed.rs index 774a5ee92..a07493486 100644 --- a/crates/pptx/tests/it/malformed.rs +++ b/crates/pptx/tests/it/malformed.rs @@ -287,3 +287,34 @@ fn unreadable_slide_is_reported() { assert_eq!(lost, vec!["ppt/slides/slide2.xml".to_string()]); assert_eq!(deckcraft_pptx::import(&broken).expect("import").slides.len(), 1); } + +/// Many slides naming one notes part: it is read for the first slide only, not copied onto every +/// slide that names it. +#[test] +fn a_shared_notes_part_is_read_once() { + use std::sync::Arc; + let mut p = defaults::new_presentation(None); + let layout = p.slides[0].layout; + for _ in 0..3 { + let s = defaults::new_slide(&mut p, layout).expect("slide"); + p.slides.push(Arc::new(s)); + } + for (i, s) in p.slides.iter_mut().enumerate() { + Arc::make_mut(s).notes = deckcraft_model::text::TextBody::from_text(&format!("notes {i}")); + } + let mut es = entries(&deckcraft_pptx::export(&p).expect("export")); + for (n, d) in es.iter_mut() { + if n.starts_with("ppt/slides/_rels/") { + let x = String::from_utf8(d.clone()).expect("utf8"); + let x = x + .replace("notesSlide2.xml", "notesSlide1.xml") + .replace("notesSlide3.xml", "notesSlide1.xml") + .replace("notesSlide4.xml", "notesSlide1.xml"); + *d = x.into_bytes(); + } + } + let q = deckcraft_pptx::import(&rezip(&es)).expect("import"); + assert_eq!(q.slides.len(), 4); + assert_eq!(q.slides[0].notes.text(), "notes 0"); + assert!(q.slides[1..].iter().all(|s| s.notes.text().is_empty()), "a shared notes part is not copied"); +} diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index ee8f2bc5d..f44b0c152 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -2258,6 +2258,34 @@ fn default_durations_match_powerpoint() { } } +/// Show media controls is PowerPoint's `p14:showMediaCtrls` extension inside `p:showPr`: off is +/// written there, under its extension uri and with the p14 namespace declared on the element; +/// on (the default) writes nothing. +#[test] +fn hidden_media_controls_survive_save_and_reopen() { + let props = |p: &Presentation| { + let bytes = deckcraft_pptx::export(p).expect("export"); + let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).expect("zip"); + let mut x = String::new(); + std::io::Read::read_to_string(&mut zip.by_name("ppt/presProps.xml").expect("presProps"), &mut x).expect("read"); + x + }; + let mut p = Presentation::default(); + p.show.show_media_controls = false; + let xml = props(&p); + let show = xml.split("").next()).expect("p:showPr"); + assert!( + show.contains( + "" + ), + "{xml}" + ); + assert!(!round(&p).show.show_media_controls); + let default = Presentation::default(); + assert!(!props(&default).contains("showMediaCtrls")); + assert!(round(&default).show.show_media_controls); +} + #[test] fn reflection_angles_and_skews_out_of_range_are_written_inside_the_schema() { let mut p = Presentation::default(); diff --git a/crates/render/src/chart.rs b/crates/render/src/chart.rs index d13f65223..53794987a 100644 --- a/crates/render/src/chart.rs +++ b/crates/render/src/chart.rs @@ -1,5 +1,7 @@ //! Charts: title, plot area with axes and gridlines, series marks, legend. +use std::collections::HashMap; + use deckcraft_color::{ColorTransform, Rgba, SchemeSlot}; use deckcraft_model::Shape; use deckcraft_model::chart::{Chart, ChartType, TrendlineType, axis_scale}; @@ -85,9 +87,9 @@ fn bar_outline(ctx: &Ctx, s: &deckcraft_model::chart::Series) -> Option<(Rgba, f /// Paint of one bar: a `` fill wins, and a single-series chart that varies colours by point uses one accent per point /// (not waterfalls, which are never read from a ``). Pattern fills draw as patterns and /// `a:noFill` leaves the bar empty, as in PowerPoint. -fn bar_paint(ctx: &Ctx, c: &Chart, si: usize, pi: usize, s: &deckcraft_model::chart::Series) -> Paint { - if let Some((_, f)) = s.point_fills.iter().find(|(k, _)| *k == pi) { - return paint_of(ctx, Some(f), series_fill(ctx, c, si, Some(s))); +fn bar_paint(ctx: &Ctx, c: &Chart, si: usize, pi: usize, s: &deckcraft_model::chart::Series, fills: Option<&HashMap>) -> Paint { + if let Some(f) = fills.and_then(|m| m.get(&pi)) { + return paint_of(ctx, Some(*f), series_fill(ctx, c, si, Some(s))); } if c.vary_colors && c.series.len() == 1 && s.fill.is_none() && c.kind != ChartType::Waterfall { return Paint::Color(series_color(ctx, c, pi)); @@ -883,6 +885,8 @@ fn draw_lines(ctx: &mut RenderContext, draw: &mut LineDrawing<'_, '_>) { let col = if area_kind { series_fill(rctx, c, si, Some(s)) } else { series_line(rctx, c, si, s) }; let series_stats = s.error_bars.as_ref().map(|_| deckcraft_model::chart::SeriesStats::of(&s.values)); let mut pts = vec![]; + // Whether the series has any x value, found once rather than per point. + let any_x = s.x_values.iter().flatten().next().is_some(); for i in 0..ncat { let Some(v) = s.values.get(i).copied().flatten() else { continue }; let v = if stacked { @@ -896,11 +900,11 @@ fn draw_lines(ctx: &mut RenderContext, draw: &mut LineDrawing<'_, '_>) { }; let x = match kind { ChartType::Scatter | ChartType::Bubble => { - match (s.x_values.get(i).copied().flatten().and_then(to_x), s.x_values.iter().flatten().next()) { + match (s.x_values.get(i).copied().flatten().and_then(to_x), any_x) { (Some(x), _) => x, // No x values at all: the points are numbered 1, 2, 3... - (None, None) => to_x(i as f64 + 1.0).unwrap_or(plot.x0 + band * (slot(i) + 0.5)), - (None, Some(_)) => continue, + (None, false) => to_x(i as f64 + 1.0).unwrap_or(plot.x0 + band * (slot(i) + 0.5)), + (None, true) => continue, } } _ => plot.x0 + band * (slot(i) + 0.5), @@ -970,6 +974,19 @@ fn draw_bars(ctx: &mut RenderContext, draw: &BarDrawing<'_, '_>) -> Vec<(f64, f6 let group = bar + step * (n - 1.0); // Error bars need each series' mean and spread: worked out once, not per point. let stats: Vec = bars.iter().map(|(_, s)| deckcraft_model::chart::SeriesStats::of(&s.values)).collect(); + // Each series' per-point fills by point (the first for a point wins), not searched per bar. + let point_fills: Vec> = bars + .iter() + .map(|(_, s)| { + let mut m = HashMap::new(); + for (k, f) in &s.point_fills { + m.entry(*k).or_insert(f); + } + m + }) + .collect(); + // A waterfall's running bars, worked out once for all its categories. + let waterfall_bars = if waterfall { c.waterfall_bars() } else { Vec::new() }; for i in 0..ncat { let mut pos_acc = 0.0; let mut neg_acc = 0.0; @@ -983,7 +1000,7 @@ fn draw_bars(ctx: &mut RenderContext, draw: &BarDrawing<'_, '_>) -> Vec<(f64, f6 let v = s.values.get(i).copied().flatten().unwrap_or(0.0) / total; let start = band * slot(i) + (band - group) / 2.0 + if stacked { 0.0 } else { step * bi as f64 }; let (a, b) = if waterfall { - c.waterfall_bars().get(i).copied().unwrap_or_default() + waterfall_bars.get(i).copied().unwrap_or_default() } else if stacked { if v >= 0.0 { let r = (pos_acc, pos_acc + v); @@ -1015,7 +1032,7 @@ fn draw_bars(ctx: &mut RenderContext, draw: &BarDrawing<'_, '_>) -> Vec<(f64, f6 }; Paint::Color(series_color(rctx, c, slot)) } else { - bar_paint(rctx, c, *si, i, s) + bar_paint(rctx, c, *si, i, s, point_fills.get(bi)) }; fill_with(ctx, &paint, r.abs(), m); if !waterfall && let Some((oc, ow)) = bar_outline(rctx, s).filter(|o| o.1 > 0.0) { diff --git a/crates/render/src/images.rs b/crates/render/src/images.rs index 0e86be8ce..59a274b14 100644 --- a/crates/render/src/images.rs +++ b/crates/render/src/images.rs @@ -37,7 +37,8 @@ struct Cache { bad_order: VecDeque, bad_bytes: usize, /// GIFs by bytes address and adjustments (`None`: not animated); the bytes stay alive with them. - anims: HashMap<(usize, u64), (Arc>, Option>)>, + /// Decoded animations by (bytes, adjustments), each with the clock of its last use. + anims: HashMap<(usize, u64), (Arc>, Option>, u64)>, } /// Longest side, total decoded size and frame count kept for one animated GIF; the largest canvas @@ -432,16 +433,33 @@ fn anim(bytes: &Arc>, adj: &PictureAdjust) -> Option> { return None; } let key = (Arc::as_ptr(bytes) as usize, adjust_key(adj)); - if let Some(a) = with(|c| c.anims.get(&key).map(|e| e.1.clone())) { + let hit = with(|c| { + c.clock += 1; + let clock = c.clock; + c.anims.get_mut(&key).map(|e| { + e.2 = clock; + e.1.clone() + }) + }); + if let Some(a) = hit { return a; } let a = decode_anim(bytes, adj).map(Arc::new); with(|c| { - let held: usize = c.anims.values().filter_map(|e| e.1.as_ref()).map(|a| a.bytes()).sum(); - if held + a.as_ref().map_or(0, |a| a.bytes()) > GIF_CACHE_BUDGET || c.anims.len() >= 64 { - c.anims.clear(); + // Make room by dropping the least recently used animations, not every one of them, so a + // slide whose animations overflow the cache does not decode all of them again each frame. + let new = a.as_ref().map_or(0, |a| a.bytes()); + loop { + let held: usize = c.anims.values().filter_map(|e| e.1.as_ref()).map(|a| a.bytes()).sum(); + if held + new <= GIF_CACHE_BUDGET && c.anims.len() < 64 { + break; + } + let Some(oldest) = c.anims.iter().min_by_key(|(_, e)| e.2).map(|(k, _)| *k) else { break }; + c.anims.remove(&oldest); } - c.anims.insert(key, (bytes.clone(), a.clone())); + c.clock += 1; + let clock = c.clock; + c.anims.insert(key, (bytes.clone(), a.clone(), clock)); }); a } @@ -535,6 +553,26 @@ mod tests { out.into_inner() } + /// A two-frame GIF whose first pixel is `n`, so each one has its own bytes. + fn gif(n: u8) -> Arc> { + use image::codecs::gif::GifEncoder; + let frame = |c: u8| image::Frame::new(image::RgbaImage::from_pixel(2, 2, image::Rgba([n, c, 0, 255]))); + let mut out = Vec::new(); + GifEncoder::new(&mut out).encode_frames([frame(0), frame(255)]).expect("encode"); + Arc::new(out) + } + + #[test] + fn a_full_animation_cache_drops_the_oldest_entry_not_all_of_them() { + let adj = PictureAdjust::default(); + let gifs: Vec<_> = (0..70).map(gif).collect(); + for g in &gifs { + assert!(anim(g, &adj).is_some()); + } + let (len, newest) = with(|c| (c.anims.len(), c.anims.contains_key(&(Arc::as_ptr(&gifs[69]) as usize, adjust_key(&adj))))); + assert!(newest && len >= 32, "{len} cached"); + } + #[test] fn rgba_conversion_is_refused_past_the_cap() { let png = grey_png(100, 50); diff --git a/crates/render/src/lib.rs b/crates/render/src/lib.rs index 15b89ec57..0f15b970e 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -54,7 +54,7 @@ pub use images::{decode as decode_image, gif_frame}; pub use morph_path::PathMorph; pub use morph_text::TextMorph; pub use pattern::{is_fg as pattern_is_fg, presets as pattern_presets, tile as pattern_tile}; -pub use placed::{Placed, PlacedLink, PlacedShape, PlacedText, place_master_texts, place_slide, text_outline}; +pub use placed::{Placed, PlacedLink, PlacedShape, PlacedText, place_master_texts, place_slide, place_slide_with, text_outline}; pub use table::cell_body as table_cell_body; pub use table::cell_rects as table_cell_rects; pub use table::row_heights as table_row_heights; diff --git a/crates/render/src/morph_text.rs b/crates/render/src/morph_text.rs index 391ec0ba3..19c83cdb2 100644 --- a/crates/render/src/morph_text.rs +++ b/crates/render/src/morph_text.rs @@ -76,16 +76,12 @@ fn tokens(l: &TextLayout, chars: bool) -> (Vec, Vec<(usize, usize)>) { /// For each token of `b`, the token of `a` with the same text, in reading order, each used once. fn matches(a: &[Token], b: &[Token]) -> Vec> { - let mut used = vec![false; a.len()]; - b.iter() - .map(|tb| { - let i = a.iter().enumerate().position(|(i, ta)| !used.get(i).copied().unwrap_or(true) && ta.key == tb.key)?; - if let Some(u) = used.get_mut(i) { - *u = true; - } - Some(i) - }) - .collect() + // Unused tokens of `a` by text, in reading order, so each lookup is a pop rather than a scan. + let mut free: std::collections::HashMap<&str, std::collections::VecDeque> = std::collections::HashMap::new(); + for (i, ta) in a.iter().enumerate() { + free.entry(ta.key.as_str()).or_default().push_back(i); + } + b.iter().map(|tb| free.get_mut(tb.key.as_str())?.pop_front()).collect() } fn layout(ctx: &Ctx, s: &Shape, x: Xfrm, fields: &dyn Fields) -> Option { @@ -228,3 +224,19 @@ pub(crate) fn draw( pub(crate) fn token_keys(l: &TextLayout, chars: bool) -> Vec { tokens(l, chars).0.into_iter().map(|t| t.key).collect() } + +#[cfg(test)] +mod match_tests { + use super::*; + + fn tok(k: &str) -> Token { + Token { key: k.into(), glyphs: Vec::new(), origin: Point::ZERO, run: 0 } + } + + #[test] + fn each_source_token_is_used_once_in_reading_order() { + let a: Vec = ["a", "b", "a"].map(tok).into(); + let b: Vec = ["a", "a", "a", "b", "c"].map(tok).into(); + assert_eq!(matches(&a, &b), vec![Some(0), Some(2), None, Some(1), None]); + } +} diff --git a/crates/render/src/placed.rs b/crates/render/src/placed.rs index 4dc9e1082..483db41bc 100644 --- a/crates/render/src/placed.rs +++ b/crates/render/src/placed.rs @@ -2,9 +2,9 @@ //! that put real text over a raster (PDF text layer, links) and for text extraction. use deckcraft_model::resolve::{self, Ctx}; -use deckcraft_model::text::{Hyperlink, TextBody}; +use deckcraft_model::text::{Hyperlink, RunKind, TextBody}; use deckcraft_model::{Presentation, Shape, ShapeId, ShapeKind}; -use deckcraft_text::{Opts, TextLayout}; +use deckcraft_text::{Fields, Opts, TextLayout}; use kurbo::{Affine, Rect}; use crate::{SlideFields, shape_geometry, table}; @@ -52,10 +52,16 @@ pub struct Placed { /// Text and links of slide `index` (master and layout graphics first, as rendered). pub fn place_slide(pres: &Presentation, index: usize) -> Placed { + place_slide_with(pres, index, &crate::RenderOpts::default()) +} + +/// [`place_slide`] with the render options the slide was drawn with, so date fields take the same +/// date and time as the raster (`opts.now`). +pub fn place_slide_with(pres: &Presentation, index: usize, opts: &crate::RenderOpts) -> Placed { let mut out = Placed::default(); let Some(slide) = pres.slides.get(index) else { return out }; let Some(rctx) = Ctx::for_slide(pres, slide) else { return out }; - let fields = SlideFields::new(pres.slide_number(index), pres.header_footer.footer_text.clone(), &crate::RenderOpts::default()); + let fields = SlideFields::new(pres.slide_number(index), pres.header_footer.footer_text.clone(), opts); let (show_layout, show_master) = resolve::show_master_shapes(slide, rctx.layout); if show_master { let mctx = Ctx::for_master(pres, rctx.master); @@ -131,9 +137,11 @@ fn place(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, s: &Shape, parent: } ShapeKind::Table(t) => { // Cell text, laid out exactly as the table draws it. + // The table's style is looked up once, not once per cell. + let def = table::resolve_style(rctx, t); for (r, c, rect) in table::cell_rects(t, &table::row_heights(rctx, t, fields)) { let Some(cell) = t.cell(r, c).filter(|cell| !cell.text.is_empty()) else { continue }; - let body = table::cell_body(rctx, t, r, c, cell); + let body = table::cell_body_with_def(rctx, def, t, r, c, cell); let tmp = Shape::table_cell(body.clone()); place_text(out, rctx, fields, s.id, &tmp, &body, rect, m); } @@ -152,9 +160,24 @@ fn place(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, s: &Shape, parent: fn place_text(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, owner: ShapeId, s: &Shape, body: &TextBody, tr: Rect, m: Affine) { let layout = deckcraft_text::layout(rctx, s, body, &Opts { rect: tr, fields, prompt_color: None, no_shrink: false }); let m = crate::text_transform(m, tr, layout.rotation); + // Field runs carry their saved text (`‹#›`); the layout drew their values, so the body that + // goes with it (character offsets, text layer) must hold the same values. + let mut placed = body.clone(); + for (pi, para) in body.paragraphs.iter().enumerate() { + for (ri, run) in para.runs.iter().enumerate() { + let RunKind::Field { field } = &run.kind else { continue }; + let lang = resolve::run(rctx, s, para, &run.props).lang; + if let Some(v) = fields.field_in(field, lang.as_deref()) + && let Some(out_run) = placed.paragraphs.get_mut(pi).and_then(|p| p.runs.get_mut(ri)) + { + out_run.text = v; + out_run.kind = RunKind::Text; + } + } + } // Text hyperlinks: one box per line segment of a linked run. for run in layout.runs.iter().filter(|r| r.link && !r.glyphs.is_empty()) { - let Some(link) = run_link(body, run.para, run.chars.0) else { continue }; + let Some(link) = run_link(&placed, run.para, run.chars.0) else { continue }; let Some(line) = layout.lines.iter().find(|l| l.para == run.para && l.start <= run.chars.0 && run.chars.0 <= l.end) else { continue }; // The run's visual extent (right-to-left runs run from right to left). let first = run.glyphs.first().map(|g| g.1).unwrap_or(0.0); @@ -162,7 +185,7 @@ fn place_text(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, owner: ShapeId let r = Rect::new(x0, line.top, x1, line.bottom); out.links.push(PlacedLink { rect: m.transform_rect_bbox(r), link, shape: owner, text: true }); } - out.texts.push(PlacedText { layout, transform: m, body: body.clone() }); + out.texts.push(PlacedText { layout, transform: m, body: placed }); } /// The hyperlink of the model run containing character `at` of paragraph `para`. diff --git a/crates/render/src/renderer.rs b/crates/render/src/renderer.rs index e7cd5ff35..0b66b30e6 100644 --- a/crates/render/src/renderer.rs +++ b/crates/render/src/renderer.rs @@ -33,6 +33,10 @@ pub struct Renderer { /// Visits spent on outer-shadow silhouette passes, kept apart so they cannot use up the budget /// of the normal drawing that follows. shadow_work: usize, + /// `RenderOpts::skip` as a set, with the slice (address, length) it was built from: the + /// slice can be as long as the slide, and is tested for every shape drawn. Forgotten when a + /// frame starts, so another frame's slice at the same address is never mistaken for it. + skip: Option<(usize, usize, std::collections::HashSet)>, } impl Default for Renderer { @@ -48,6 +52,7 @@ impl Default for Renderer { in_shadow_pass: false, render_work: 0, shadow_work: 0, + skip: None, } } } @@ -163,6 +168,7 @@ impl Renderer { pub(crate) fn slide(&mut self, pres: &Presentation, slide: &Slide, index: usize, opts: &RenderOpts) -> Image { self.render_work = 0; self.shadow_work = 0; + self.skip = None; self.in_shadow_pass = false; let (mut ctx, w, h, view) = Self::canvas(pres, opts, needs_filters(pres, slide)); let f = Frame { pres, opts, fields: Self::fields(pres, index, opts) }; @@ -178,6 +184,7 @@ impl Renderer { pub(crate) fn slide_part(&mut self, pres: &Presentation, slide: &Slide, index: usize, part: SlidePart, opts: &RenderOpts) -> Image { self.render_work = 0; self.shadow_work = 0; + self.skip = None; self.in_shadow_pass = false; let (mut ctx, w, h, view) = Self::canvas(pres, opts, needs_filters(pres, slide)); let f = Frame { pres, opts, fields: Self::fields(pres, index, opts) }; @@ -212,6 +219,7 @@ impl Renderer { let mix = if mix.is_finite() { mix.clamp(0.0, 1.0) } else { 1.0 }; self.render_work = 0; self.shadow_work = 0; + self.skip = None; self.in_shadow_pass = false; let (mut ctx, w, h, view) = Self::canvas(pres, opts, needs_filters(pres, old.0) || needs_filters(pres, new.0)); let (Some(co), Some(cn)) = (Ctx::for_slide(pres, old.0), Ctx::for_slide(pres, new.0)) else { return self.finish(ctx, w, h) }; @@ -260,6 +268,7 @@ impl Renderer { pub(crate) fn layout(&mut self, pres: &Presentation, mi: usize, li: Option, opts: &RenderOpts) -> Image { self.render_work = 0; self.shadow_work = 0; + self.skip = None; self.in_shadow_pass = false; let (w, h) = output_size(pres, opts); let mut ctx = RenderContext::new_with(w, h, vello_cpu::RenderSettings { num_threads: 0, ..Default::default() }); @@ -393,8 +402,25 @@ impl Renderer { geo } + /// Whether `id` is in `skip`, looked up in a set built once per frame and slice. + fn skipped(&mut self, skip: &[ShapeId], id: ShapeId) -> bool { + if skip.is_empty() { + return false; + } + let key = (skip.as_ptr() as usize, skip.len()); + match &self.skip { + Some((p, n, set)) if (*p, *n) == key => set.contains(&id), + _ => { + let set: std::collections::HashSet = skip.iter().copied().collect(); + let hit = set.contains(&id); + self.skip = Some((key.0, key.1, set)); + hit + } + } + } + fn shape_at(&mut self, ctx: &mut RenderContext, f: &Frame, rctx: &Ctx, s: &Shape, parent: Affine, depth: usize) { - if depth > 32 || (s.hidden && !f.opts.show_hidden) || f.opts.skip.contains(&s.id) { + if depth > 32 || (s.hidden && !f.opts.show_hidden) || self.skipped(f.opts.skip, s.id) { return; } let work = if self.in_shadow_pass { &mut self.shadow_work } else { &mut self.render_work }; diff --git a/crates/render/src/stroke.rs b/crates/render/src/stroke.rs index 8310cf0c9..fc1403f6b 100644 --- a/crates/render/src/stroke.rs +++ b/crates/render/src/stroke.rs @@ -191,7 +191,8 @@ pub(crate) fn limit_dash_count(path: &BezPath, pat: Vec) -> Vec { return pat; } let len: f64 = path.segments().map(|s| s.arclen(1e-2)).sum(); - let dashes = len / period; + // Every dash of the pattern counts, not each repeat of it: a custom pattern can hold many. + let dashes = len / period * pat.len().div_ceil(2) as f64; if !len.is_finite() { return vec![]; } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 2de2daabe..540a8500a 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1,7 +1,7 @@ use super::{ Ctx, Fields, Fill, Image, Opts, PathMorph, Point, RENDERER, Rect, Region, RenderOpts, Renderer, Rgba, ShapeState, SlideFields, TextMorph, crop_dest, dpi_for_scale, gif_frame, image_dpi, images, morph_path, morph_text, needs_filters, parse_path, pasteboard_region, place_slide, - render_blend, render_shapes, render_slide, render_slide_drawn, slide_extent, text_transform, + place_slide_with, render_blend, render_shapes, render_slide, render_slide_drawn, slide_extent, text_transform, }; use deckcraft_model::chart::{ChartType, Trendline, TrendlineType}; use deckcraft_model::style::ColorRef; @@ -2971,6 +2971,27 @@ fn empty_bullet_is_gray_only_while_editing_and_never_in_exports() { assert!(changed.iter().all(|(px, _)| px[0] >= 179 && px[0] == px[1] && px[1] == px[2])); } +#[test] +fn placed_text_holds_the_field_value_not_the_saved_placeholder() { + let mut p = deck_with(vec![field_shape(Some("slidenum"), "<#>")]); + let second = p.slides[0].clone(); + p.slides.push(second); + let placed = place_slide(&p, 1); + assert_eq!(placed.texts.len(), 1); + assert_eq!(placed.texts[0].body.text(), "2"); + // The layout was drawn from the same value, so the offsets agree with the body. + assert_eq!(placed.texts[0].layout.runs.iter().map(|r| r.chars.1).max(), Some(1)); +} + +#[test] +fn placed_text_takes_its_date_from_the_given_render_options() { + use deckcraft_text::datetime::DateTime; + let p = deck_with(vec![field_shape(Some("datetime1"), "1/1/2000")]); + let at = |now: DateTime| place_slide_with(&p, 0, &RenderOpts { now: Some(now), ..Default::default() }); + assert_eq!(at(DateTime::new(2026, 10, 9, 20, 5, 7)).texts[0].body.text(), "10/9/2026"); + assert_eq!(at(DateTime::new(2027, 3, 4, 0, 0, 0)).texts[0].body.text(), "3/4/2027"); +} + /// A chart that has no text colour of its own takes PowerPoint's default, the theme's text colour /// at 65 % plus 35 %: light on Nocturne's dark slides (it used to be a fixed grey, near-invisible /// there), 595959 grey on Harbor's light ones. diff --git a/crates/render/src/tests_wild.rs b/crates/render/src/tests_wild.rs index fb25c7f0b..2da29cb37 100644 --- a/crates/render/src/tests_wild.rs +++ b/crates/render/src/tests_wild.rs @@ -990,4 +990,37 @@ fn a_tiny_dash_on_a_huge_shape_draws_without_running_out_of_memory() { p.line_to((4.0e6, 0.0)); let pat = limit_dash_count(&p, vec![1.0, 1.0]); assert!((4.0e6 / (pat[0] + pat[1]) - MAX_DASHES).abs() < 1.0, "{pat:?}"); + // A custom pattern of 1,000 dashes counts each of them: the limit is on dashes drawn. + let pat = limit_dash_count(&p, vec![1.0; 2000]); + let period: f64 = pat.iter().sum(); + assert!((4.0e6 / period * 1000.0 - MAX_DASHES).abs() < 1.0, "{}", period); +} + +/// A bar chart with 20,000 points, each with its own fill, and a sparse scatter of the same size: +/// the per-point fills and the scatter's "has any x" are looked up, not searched per point. +#[test] +fn many_point_fills_and_sparse_scatter_x_render() { + use deckcraft_model::ShapeKind; + use deckcraft_model::chart::{Chart, ChartType}; + use deckcraft_model::style::Fill; + let n = 20_000; + let chart = |kind: ChartType| { + let mut c = Chart::sample(kind); + c.series.truncate(1); + c.categories = (0..n).map(|i| i.to_string()).collect(); + c.series[0].values = (0..n).map(|i| Some(i as f64)).collect(); + c + }; + let mut bars = chart(ChartType::Column); + bars.series[0].point_fills = (0..n).rev().map(|i| (i, Fill::solid(ColorRef::rgb(Rgba::rgb(200, 0, 0))))).collect(); + let mut scatter = chart(ChartType::Scatter); + scatter.series[0].x_values = (0..n).map(|i| (i + 1 == n).then_some(1.0)).collect(); + let shape = |id: u32, c: Chart| Shape { + id: ShapeId(id), + xfrm: Some(Xfrm::new(10.0, 10.0, 400.0, 300.0)), + kind: ShapeKind::Chart(Box::new(c)), + ..Default::default() + }; + let img = render(vec![shape(2, bars), shape(3, scatter)]); + assert!(img.width > 0); } diff --git a/crates/render/src/text_draw.rs b/crates/render/src/text_draw.rs index 3fe96d694..d9788a4a9 100644 --- a/crates/render/src/text_draw.rs +++ b/crates/render/src/text_draw.rs @@ -35,22 +35,30 @@ pub(crate) fn prompt_body(rctx: &Ctx, s: &Shape, kind: PhType) -> TextBody { t } +/// Most path elements this thread's glyph cache retains. Its entries are shared with the font +/// cache, but they stay alive here after that cache evicts them, so they are bounded by elements. +const GLYPH_CACHE_MAX_ELEMENTS: usize = 1 << 22; + thread_local! { - static GLYPH_CACHE: std::cell::RefCell>> = std::cell::RefCell::new(HashMap::new()); + /// The cached outlines, and the path elements they hold. + static GLYPH_CACHE: std::cell::RefCell<(HashMap<(u32, u32), Arc>, usize)> = std::cell::RefCell::new((HashMap::new(), 0)); } pub(crate) fn glyph(face: &deckcraft_fonts::FontFace, gid: u32) -> Arc { let key = (face.id(), gid); - if let Some(p) = GLYPH_CACHE.with(|c| c.borrow().get(&key).cloned()) { + if let Some(p) = GLYPH_CACHE.with(|c| c.borrow().0.get(&key).cloned()) { return p; } let p = FontDb::global().outline(face, gid); GLYPH_CACHE.with(|c| { let mut c = c.borrow_mut(); - if c.len() > 200_000 { - c.clear(); + let n = p.elements().len(); + if c.0.len() > 200_000 || c.1.saturating_add(n) > GLYPH_CACHE_MAX_ELEMENTS { + c.0.clear(); + c.1 = 0; } - c.insert(key, p.clone()); + c.1 += n; + c.0.insert(key, p.clone()); }); p } @@ -105,6 +113,7 @@ pub fn draw_layout(ctx: &mut RenderContext, l: &deckcraft_text::TextLayout, tr: pub fn draw_layout_with(ctx: &mut RenderContext, l: &deckcraft_text::TextLayout, tr: Rect, m: Affine, paras: &[(usize, ParaState)]) { let para = |i: usize| paras.iter().find(|(p, _)| *p == i).map(|(_, s)| *s).unwrap_or_default(); let m = text_transform(m, tr, l.rotation); + let lines_of = LineIndex::new(l); let deco = |ctx: &mut RenderContext, d: &deckcraft_text::Deco| { let ps = para(d.para); if !ps.visible || ps.opacity <= 0.001 { @@ -144,7 +153,7 @@ pub fn draw_layout_with(ctx: &mut RenderContext, l: &deckcraft_text::TextLayout, draw_glyph(ctx, run, *gid, at, ps.opacity); continue; }; - let li = line_at(l, run.para, *y); + let li = lines_of.line_at(run.para, *y); if !grads.iter().any(|(i, _)| *i == li) { let scheme = deckcraft_color::ColorScheme { name: String::new(), colors: [Rgba::BLACK; 12] }; let stops = g.stops.iter().map(|s| (s.pos, s.color.resolve(&scheme, None))).collect(); @@ -191,8 +200,78 @@ pub(crate) fn fill_deco(ctx: &mut RenderContext, d: &deckcraft_text::Deco) { } } -/// The line of paragraph `para` holding a glyph on baseline `y` (the nearest baseline). -fn line_at(l: &deckcraft_text::TextLayout, para: usize, y: f64) -> usize { +/// Finds the line of a paragraph holding a glyph on baseline `y` without scanning the paragraph's +/// lines for every glyph. A paragraph whose lines are ordered top to bottom (every layout the text +/// crate produces) is searched by bisection; any other falls back to a scan with the same answer. +pub(crate) struct LineIndex<'a> { + l: &'a deckcraft_text::TextLayout, + /// Per paragraph: its lines' tops, bottoms and baselines all rise with the line index. + ordered: Vec, +} + +impl<'a> LineIndex<'a> { + pub(crate) fn new(l: &'a deckcraft_text::TextLayout) -> Self { + let ordered = l + .para_lines + .iter() + .map(|&(first, count)| { + let end = first.saturating_add(count).min(l.lines.len()); + l.lines.get(first..end).is_some_and(|ls| { + ls.iter().all(|a| a.top <= a.bottom) + && ls.windows(2).all(|w| w[0].top <= w[1].top && w[0].bottom <= w[1].bottom && w[0].baseline <= w[1].baseline) + }) + }) + .collect(); + LineIndex { l, ordered } + } + + /// The nearest line baseline to `y` among `lines` (the first of equals), as an index into `lines`. + fn nearest(lines: &[deckcraft_text::LineInfo], y: f64) -> Option { + let i = lines.partition_point(|li| li.baseline < y); + let best = match (i.checked_sub(1), lines.get(i)) { + (Some(p), Some(n)) => { + if (lines[p].baseline - y).abs() <= (n.baseline - y).abs() { + p + } else { + i + } + } + (Some(p), None) => p, + (None, Some(_)) => i, + (None, None) => return None, + }; + let b = lines[best].baseline; + Some(lines.partition_point(|li| li.baseline < b)) + } + + /// The paragraph's lines when they are ordered, with the index of the first. + fn ordered_lines(&self, para: usize) -> Option<(usize, &[deckcraft_text::LineInfo])> { + let &(first, count) = self.l.para_lines.get(para)?; + let end = first.saturating_add(count).min(self.l.lines.len()); + (self.ordered[para] && first < end).then(|| (first, &self.l.lines[first..end])) + } + + /// The line of paragraph `para` holding a glyph on baseline `y` (the nearest baseline). + pub(crate) fn line_at(&self, para: usize, y: f64) -> usize { + if let Some((first, ls)) = self.ordered_lines(para) { + return Self::nearest(ls, y).map_or(0, |i| first + i); + } + line_at_scan(self.l, para, y) + } + + /// The first line of paragraph `para` whose box holds `y`, else the nearest baseline's line. + pub(crate) fn line_of(&self, para: usize, y: f64) -> Option { + let Some((first, ls)) = self.ordered_lines(para) else { return crate::warp::line_of_scan(self.l, para, y) }; + let i = ls.partition_point(|li| li.bottom + 0.01 < y); + if ls.get(i).is_some_and(|li| y >= li.top - 0.01) { + return Some(first + i); + } + Self::nearest(ls, y).map(|i| first + i) + } +} + +/// The line of paragraph `para` holding a glyph on baseline `y` (the nearest baseline), by scanning. +fn line_at_scan(l: &deckcraft_text::TextLayout, para: usize, y: f64) -> usize { if let Some(&(first, count)) = l.para_lines.get(para) { if count == 0 { return 0; @@ -318,3 +397,38 @@ fn draw_glyph_with( ctx.stroke_path(&path); } } + +#[cfg(test)] +mod line_index_tests { + use super::*; + use deckcraft_text::{LineInfo, TextLayout}; + + fn layout(ordered: bool) -> TextLayout { + let line = |para, top: f64| LineInfo { para, top, baseline: top + 8.0, bottom: top + 10.0, ..Default::default() }; + let mut lines: Vec = (0..40).map(|i| line(usize::from(i >= 25), f64::from(i) * 10.0)).collect(); + if !ordered { + lines.swap(3, 7); + } + TextLayout { lines, para_lines: vec![(0, 25), (25, 15)], ..Default::default() } + } + + #[test] + fn bisection_matches_the_scan() { + for ordered in [true, false] { + let l = layout(ordered); + let idx = LineIndex::new(l_ref(&l)); + assert_eq!(idx.ordered, vec![ordered, true]); + for para in 0..2 { + for k in -20..450 { + let y = f64::from(k) * 0.9; + assert_eq!(idx.line_at(para, y), line_at_scan(&l, para, y), "line_at {para} {y}"); + assert_eq!(idx.line_of(para, y), crate::warp::line_of_scan(&l, para, y), "line_of {para} {y}"); + } + } + } + } + + fn l_ref(l: &TextLayout) -> &TextLayout { + l + } +} diff --git a/crates/render/src/warp.rs b/crates/render/src/warp.rs index 58caa2ead..72fcbbbe5 100644 --- a/crates/render/src/warp.rs +++ b/crates/render/src/warp.rs @@ -9,13 +9,14 @@ use deckcraft_text::{GlyphRun, TextLayout}; use kurbo::{Affine, BezPath, PathEl, Point, Rect, Shape as _}; use vello_cpu::{RenderContext, peniko}; +use crate::text_draw::LineIndex; use crate::{ParaState, color, glyph, scale_of, text_transform}; /// Most points one warped glyph may have (a hostile size or zoom can't explode the work). const MAX_POINTS: usize = 40_000; -/// The line of `l` a glyph of paragraph `para` on baseline `y` belongs to. -fn line_of(l: &TextLayout, para: usize, y: f64) -> Option { +/// The line of `l` a glyph of paragraph `para` on baseline `y` belongs to, by scanning. +pub(crate) fn line_of_scan(l: &TextLayout, para: usize, y: f64) -> Option { if let Some(&(first, count)) = l.para_lines.get(para) { if count == 0 { return None; @@ -110,6 +111,7 @@ pub(crate) fn draw(ctx: &mut RenderContext, l: &TextLayout, tr: Rect, m: Affine, let para = |i: usize| paras.iter().find(|(p, _)| *p == i).map(|(_, s)| *s).unwrap_or_default(); let m = text_transform(m, tr, l.rotation); let n = l.lines.len(); + let lines_of = LineIndex::new(l); let mut ink: Vec> = vec![None; n]; let mut off: Vec> = vec![None; n]; let mut items: Vec = Vec::new(); @@ -117,7 +119,7 @@ pub(crate) fn draw(ctx: &mut RenderContext, l: &TextLayout, tr: Rect, m: Affine, let k = run.size / run.face.upem.max(1.0); let skew = if run.fake_italic { Affine::new([1.0, 0.0, -0.2, 1.0, 0.0, 0.0]) } else { Affine::IDENTITY }; for (gid, x, y) in &run.glyphs { - let Some(li) = line_of(l, run.para, *y) else { continue }; + let Some(li) = lines_of.line_of(run.para, *y) else { continue }; if let Some(o) = off.get_mut(li) { *o = Some(o.map_or(ref_offset(run), |v| v.max(ref_offset(run)))); } @@ -156,7 +158,7 @@ pub(crate) fn draw(ctx: &mut RenderContext, l: &TextLayout, tr: Rect, m: Affine, return; } let y = (d.rect.y0 + d.rect.y1) / 2.0; - let Some(li) = line_of(l, d.para, y) else { return }; + let Some(li) = lines_of.line_of(d.para, y) else { return }; ctx.set_transform(m * Affine::translate(ps.offset)); ctx.set_paint(color(d.color, ps.opacity)); for r in d.pieces() { diff --git a/crates/text/src/breaking.rs b/crates/text/src/breaking.rs index bb764b64c..9a6028876 100644 --- a/crates/text/src/breaking.rs +++ b/crates/text/src/breaking.rs @@ -260,12 +260,15 @@ pub(crate) struct Tab { align: TabAlign, } +/// The paragraph's tab stops in position order (a stop at no finite position is left out), so +/// [`tab_advance`] finds the next one by binary search instead of a scan per tab. pub(crate) fn tabs_of(props: &ParaProps) -> Vec { - props + let mut tabs: Vec = props .tabs .as_ref() .map(|t| { t.iter() + .filter(|s| s.pos.is_finite()) .map(|s| Tab { pos: s.pos, align: match s.align.as_str() { @@ -277,7 +280,9 @@ pub(crate) fn tabs_of(props: &ParaProps) -> Vec { }) .collect() }) - .unwrap_or_default() + .unwrap_or_default(); + tabs.sort_by(|a, b| a.pos.total_cmp(&b.pos)); + tabs } /// Width of the text after the tab at cell `i` up to the next tab or line break, and the width of @@ -301,18 +306,30 @@ pub(crate) fn tab_segment(cells: &[Cell], i: usize) -> (f64, f64) { /// (`seg`: its width, and its width before the decimal point) as the stop aligns it but never /// moving back, as PowerPoint does; past the last stop, to the next default stop. pub(crate) fn tab_advance(x: f64, stops: &[Tab], default_tab: f64, seg: (f64, f64)) -> f64 { - for s in stops { - if s.pos > x + 0.5 { - let to = s.pos - x; - return match s.align { - TabAlign::Left => to, - TabAlign::Right => (to - seg.0).max(0.0), - TabAlign::Center => (to - seg.0 / 2.0).max(0.0), - TabAlign::Decimal => (to - seg.1).max(0.0), - }; - } + // `stops` is in position order (`tabs_of`): the next stop is the first past x. + // A NaN `x` (a NaN spacing upstream) skips the lookup and takes the finite default-stop path. + if let Some(s) = stops.get(stops.partition_point(|s| s.pos <= x + 0.5)).filter(|_| !x.is_nan()) { + let to = s.pos - x; + return match s.align { + TabAlign::Left => to, + TabAlign::Right => (to - seg.0).max(0.0), + TabAlign::Center => (to - seg.0 / 2.0).max(0.0), + TabAlign::Decimal => (to - seg.1).max(0.0), + }; } let d = if default_tab > 1.0 { default_tab } else { 72.0 }; let next = ((x / d).floor() + 1.0) * d; (next - x).max(1.0) } + +#[cfg(test)] +mod tab_tests { + use super::*; + + #[test] + fn a_nan_position_takes_the_finite_default_stop() { + let stops = [Tab { pos: 100.0, align: TabAlign::Left }]; + assert!(tab_advance(f64::NAN, &stops, 72.0, (0.0, 0.0)).is_finite()); + assert_eq!(tab_advance(10.0, &stops, 72.0, (0.0, 0.0)), 90.0); + } +} diff --git a/crates/text/src/run.rs b/crates/text/src/run.rs index d7e9dbeea..56e6ead7c 100644 --- a/crates/text/src/run.rs +++ b/crates/text/src/run.rs @@ -22,7 +22,7 @@ pub struct GlyphRun { pub link: bool, /// Opacity of the fill on top of `color`'s own alpha (0 for no fill). pub alpha: f64, - pub gradient: Option, + pub gradient: Option>, /// Shadow and glow behind the glyphs (`a:rPr/a:effectLst`). pub effects: Option, /// East Asian vertical text (`eaVert`): each glyph stands upright, turned a quarter turn diff --git a/crates/text/src/style.rs b/crates/text/src/style.rs index e1289c520..b61359faf 100644 --- a/crates/text/src/style.rs +++ b/crates/text/src/style.rs @@ -46,7 +46,7 @@ pub(crate) struct Style { pub(crate) highlight: Option, pub(crate) outline: Option<(Rgba, f64)>, pub(crate) link: bool, - pub(crate) gradient: Option, + pub(crate) gradient: Option>, pub(crate) effects: Option, /// "Regular", "Bold"… for the slot and fallback faces. pub(crate) style_name: &'static str, @@ -136,7 +136,7 @@ pub(crate) fn make_style(ctx: &Ctx, r: &RunProps, scale: f64, prompt: Option (Rgba::TRANSPARENT, 0.0, None), _ => (resolve::text_color(ctx, r), 1.0, None), diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 7dbdbe4f0..a747af045 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -1416,6 +1416,16 @@ fn tab_stops_align_the_text_after_them() { assert!((l.lines[0].edges[4].0 - x0(&l) - 72.0).abs() < 0.01); } +#[test] +fn many_tabs_over_many_stops_lay_out_by_position() { + // 20,000 stops given out of order, and 2,000 tabs: each tab finds the next stop by position. + let mut stops: Vec<(f64, &str)> = (1..=20_000).rev().map(|k| (k as f64 * 0.05 + 30.0, "l")).collect(); + stops.push((f64::NAN, "l")); + let text = "a\t".repeat(2_000); + let l = tabbed(&text, &stops, 1.0e6); + assert!((l.lines[0].edges[2].0 - l.inner.x0 - 30.05).abs() < 0.01, "{:?}", &l.lines[0].edges[..3]); +} + #[test] fn a_tab_past_the_end_moves_to_the_next_line() { // PowerPoint: "abcdefghij\tk\tl\tm…" in a narrow box puts the tab that doesn't fit on the next diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 88b9f770b..166eba220 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -19,6 +19,7 @@ deckcraft-anim = { workspace = true } deckcraft-engine = { workspace = true } deckcraft-media = { workspace = true } egui = { workspace = true, features = ["default_fonts", "monochrome_emoji_fonts"] } +egui_extras = { workspace = true } craft-ui = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/ui-egui/src/app/playback.rs b/crates/ui-egui/src/app/playback.rs index 73e765ff7..c959829cd 100644 --- a/crates/ui-egui/src/app/playback.rs +++ b/crates/ui-egui/src/app/playback.rs @@ -144,6 +144,10 @@ impl SlideApp { let visible = show::viewport_visible(ctx); let Some(doc) = self.session.active().map(|d| d.doc.clone()) else { return }; let Some(show) = self.show.as_mut() else { return }; + if show.presenter_grid && self.services.presenter.as_ref().is_some_and(|host| !host.is_open()) { + // A web popup closed with its grid open must not hold auto-advance while this tab is hidden (#1866). + show.presenter_grid = false; + } if visible { // A visible logic-only pass (`run_logic`) carries the last full frame's input time. // Anchoring that stale time to `now` would move the hidden clock backwards if the diff --git a/crates/ui-egui/src/dialogs/about.rs b/crates/ui-egui/src/dialogs/about.rs index 03c6629b2..059543b61 100644 --- a/crates/ui-egui/src/dialogs/about.rs +++ b/crates/ui-egui/src/dialogs/about.rs @@ -1,7 +1,7 @@ use super::{Color32, CornerRadius, Rect, Sense, SlideApp, Stroke, Tokens, Ui, pos2, theme, vec2}; /// Room the About credits need for their table; the About tab keeps its compact size. -pub(super) const CREDITS_SIZE: egui::Vec2 = vec2(640.0, 380.0); +pub(super) const CREDITS_SIZE: egui::Vec2 = vec2(820.0, 380.0); /// The Contributors / Models body in a block of its own fixed size, below the tab row. Sizing the /// dialog's `ui` instead (`set_min_size` / `set_max_height`) squeezed the tab row and the first diff --git a/crates/ui-egui/src/dialogs/credits.rs b/crates/ui-egui/src/dialogs/credits.rs index d1bf5b30e..b9b32e56e 100644 --- a/crates/ui-egui/src/dialogs/credits.rs +++ b/crates/ui-egui/src/dialogs/credits.rs @@ -246,12 +246,14 @@ pub fn contributors_ui(ui: &mut egui::Ui) { let list = sorted(CONTRIBUTORS, v.names, v.key, v.ascending); ui.label(RichText::new(format!("{} contributors · {} commits", list.len(), group(TOTAL_COMMITS))).small().weak()); ui.separator(); - egui::ScrollArea::both().auto_shrink([false, false]).show(ui, |ui| { - if list.is_empty() { - ui.label("No contributor data was built into this copy."); - } else if v.table { - table(ui, &list, &mut v); - } else { + if list.is_empty() { + ui.label("No contributor data was built into this copy."); + } else if v.table { + // The table scrolls vertically itself, under a fixed header; it only scrolls sideways + // once its columns are dragged wider than the dialog. + egui::ScrollArea::horizontal().auto_shrink([false, false]).show(ui, |ui| table(ui, &list, &mut v)); + } else { + egui::ScrollArea::vertical().auto_shrink([false, false]).show(ui, |ui| { ui.horizontal_wrapped(|ui| { for (i, c) in list.iter().enumerate() { if i > 0 { @@ -260,39 +262,153 @@ pub fn contributors_ui(ui: &mut egui::Ui) { ui.hyperlink_to(c.name(v.names), format!("https://github.com/{}", c.login)).on_hover_text(c.summary()); } }); - } - }); + }); + } ui.data_mut(|d| d.insert_temp(id, v)); } -fn table(ui: &mut egui::Ui, list: &[&Contributor], v: &mut View) { - egui::Grid::new("credits_table").striped(true).num_columns(SortKey::ALL.len()).show(ui, |ui| { - for k in SortKey::ALL { - let arrow = if v.key == k { if v.ascending { " ▲" } else { " ▼" } } else { "" }; - if ui.button(RichText::new(format!("{}{arrow}", k.label().1)).strong()).on_hover_text(k.label().0).clicked() { - if v.key == k { - v.ascending = !v.ascending; - } else { - v.key = k; - v.ascending = k.default_ascending(); - } +/// How the contributor table's spare width is shared out: all of it to Name. +const CONTRIBUTOR_GROW: [f32; 10] = [1.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]; + +/// How the model table's spare width is shared out: to Company and Model, the text columns. +const MODEL_GROW: [f32; 6] = [1.0, 2.0, 0.0, 0.0, 0.0, 0.0]; + +/// The narrowest a column can be dragged. +const MIN_COLUMN: f32 = 28.0; + +/// The width a column needs for its header and its widest cell. +fn natural(ui: &egui::Ui, header: RichText, cells: impl Iterator) -> f32 { + let measure = |text: egui::WidgetText| text.into_galley(ui, Some(egui::TextWrapMode::Extend), f32::INFINITY, egui::TextStyle::Body).size().x; + let widest = cells.map(|c| measure(c.into())).fold(measure(header.into()), f32::max); + widest + 2.0 * ui.spacing().button_padding.x +} + +/// Opening column widths: each column's natural width, with the spare room shared out by `grow`, +/// so the columns fill `room` exactly. When there is too little room, only the growing columns +/// give way, each in proportion to its width above [`MIN_COLUMN`], and none goes below that. +fn fit_widths(natural: &[f32], grow: &[f32], room: f32) -> Vec { + let spare = room - natural.iter().sum::(); + let weight = grow.iter().sum::(); + let slack = |w: f32| (w - MIN_COLUMN).max(0.0); + let flexible: f32 = natural.iter().zip(grow).filter(|(_, g)| **g > 0.0).map(|(w, _)| slack(*w)).sum(); + natural + .iter() + .zip(grow) + .map(|(w, g)| { + if *g > 0.0 && spare >= 0.0 { + w + spare * g / weight + } else if *g > 0.0 && flexible > 0.0 { + w - (-spare).min(flexible) * slack(*w) / flexible + } else { + *w } - } - ui.end_row(); - for c in list { - ui.hyperlink_to(c.name(v.names), format!("https://github.com/{}", c.login)).on_hover_text(c.summary()); - ui.label(group(c.prs)); - ui.label(group(c.commits)); - ui.label(group(c.lines_added)); - ui.label(group(c.lines_deleted)); - ui.label(signed(c.lines_delta())); - ui.label(group(c.binary_added)); - ui.label(group(c.binary_deleted)); - ui.label(day(c.first_commit)); - ui.label(day(c.last_commit)); - ui.end_row(); - } - }); + }) + .collect() +} + +/// A striped table whose columns open at [`fit_widths`], so it fills the width and nothing scrolls +/// sideways. Every column, the last included, can be dragged wider or narrower at its right edge: +/// only a table dragged wider than the window scrolls sideways. Text too long for its column is +/// clipped. When a column's natural width or the window changes, the columns go back to a new fit, +/// dropping any dragged widths. +fn fitted_table<'a>(ui: &'a mut egui::Ui, id: &str, natural: &[f32], grow: &[f32]) -> egui_extras::TableBuilder<'a> { + // Every cell is followed by the item gap, and the vertical scrollbar keeps its margins. + let s = ui.spacing(); + let gaps = s.item_spacing.x * natural.len() as f32; + let bar = s.scroll.bar_width + s.scroll.bar_inner_margin + s.scroll.bar_outer_margin; + let widths = fit_widths(natural, grow, (ui.available_width() - gaps - bar).max(0.0)); + // The table keeps its own column widths, so a change in what they were fitted to has to clear + // them. A longer name can leave the fit itself unchanged, and scrolling can nudge the available + // width, so this compares the natural widths and the clip rect, which scrolling never moves. + let fit = egui::Id::unique(("credits_fit", id)); + let measured = (natural.to_vec(), ui.clip_rect().width()); + let refit = ui.data_mut(|d| d.get_temp::<(Vec, f32)>(fit)).as_ref() != Some(&measured); + ui.data_mut(|d| d.insert_temp(fit, measured)); + let mut t = + egui_extras::TableBuilder::new(ui).id_salt(id).striped(true).resizable(true).auto_shrink([false, false]).max_scroll_height(f32::INFINITY); + if refit { + t.reset(); + } + for w in widths { + t = t.column(egui_extras::Column::initial(w).at_least(MIN_COLUMN).clip(true)); + } + t +} + +/// Add a cell's widget right-aligned (counts, dates) or left-aligned (names), vertically centred. +fn aligned(ui: &mut egui::Ui, right: bool, add: impl FnOnce(&mut egui::Ui) -> R) -> R { + let layout = if right { egui::Layout::right_to_left(egui::Align::Center) } else { egui::Layout::left_to_right(egui::Align::Center) }; + ui.with_layout(layout, add).inner +} + +fn row_height(ui: &egui::Ui) -> f32 { + ui.text_style_height(&egui::TextStyle::Body) + 6.0 +} + +/// One contributor's table cells after the name, in `SortKey::ALL` order. +fn cells(c: &Contributor) -> [String; 9] { + [ + group(c.prs), + group(c.commits), + group(c.lines_added), + group(c.lines_deleted), + signed(c.lines_delta()), + group(c.binary_added), + group(c.binary_deleted), + day(c.first_commit).to_string(), + day(c.last_commit).to_string(), + ] +} + +/// A contributor-table header. The sorted column is highlighted; its direction shows on the +/// toolbar's sort-order button, so no arrow widens the column. +fn header_text(k: SortKey) -> RichText { + RichText::new(k.label().1).strong() +} + +fn table(ui: &mut egui::Ui, list: &[&Contributor], v: &mut View) { + let h = row_height(ui); + let rows: Vec<[String; 9]> = list.iter().map(|c| cells(c)).collect(); + let widths: Vec = SortKey::ALL + .iter() + .enumerate() + .map(|(i, &k)| match i.checked_sub(1) { + None => natural(ui, header_text(k), list.iter().map(|c| c.name(v.names))), + Some(col) => natural(ui, header_text(k), rows.iter().filter_map(|r| r.get(col).cloned())), + }) + .collect(); + fitted_table(ui, "credits_table", &widths, &CONTRIBUTOR_GROW) + .header(h + 4.0, |mut header| { + for k in SortKey::ALL { + header.col(|ui| { + let r = aligned(ui, k != SortKey::Name, |ui| ui.selectable_label(v.key == k, header_text(k))).on_hover_text(k.label().0); + + if r.clicked() { + if v.key == k { + v.ascending = !v.ascending; + } else { + v.key = k; + v.ascending = k.default_ascending(); + } + } + }); + } + }) + .body(|mut body| { + for (c, cells) in list.iter().zip(rows) { + body.row(h, |mut row| { + row.col(|ui| { + aligned(ui, false, |ui| ui.hyperlink_to(c.name(v.names), format!("https://github.com/{}", c.login))) + .on_hover_text(c.summary()); + }); + for text in cells { + row.col(|ui| { + aligned(ui, true, |ui| ui.label(text)); + }); + } + }); + } + }); } /// About ▸ Models: AI models credited in Co-Authored-By trailers. @@ -302,22 +418,48 @@ pub fn models_ui(ui: &mut egui::Ui) { return; } let assisted: u64 = MODELS.iter().map(|m| m.commits).max().unwrap_or(0).max(1); - egui::ScrollArea::both().auto_shrink([false, false]).show(ui, |ui| { - egui::Grid::new("credits_models").striped(true).num_columns(6).show(ui, |ui| { - for h in ["Company", "Model", "Version", "Commits", "% of all commits", "Lines +/−"] { - ui.label(RichText::new(h).strong()); - } - ui.end_row(); - for m in MODELS { - ui.label(m.company); - ui.label(m.model); - ui.label(m.version); - ui.label(group(m.commits)); - ui.label(format!("{:.0}%", 100.0 * m.commits as f64 / TOTAL_COMMITS.max(assisted) as f64)); - ui.label(format!("+{} / −{}", group(m.lines_added), group(m.lines_deleted))); - ui.end_row(); - } - }); + let h = row_height(ui); + let titles = ["Company", "Model", "Version", "Commits", "% of all commits", "Lines +/−"]; + // Company, Model and Version read as text; the counts line up on the right. + let right = [false, false, false, true, true, true]; + let rows: Vec<[String; 6]> = MODELS + .iter() + .map(|m| { + [ + m.company.to_string(), + m.model.to_string(), + m.version.to_string(), + group(m.commits), + format!("{:.0}%", 100.0 * m.commits as f64 / TOTAL_COMMITS.max(assisted) as f64), + format!("+{} / −{}", group(m.lines_added), group(m.lines_deleted)), + ] + }) + .collect(); + let widths: Vec = titles + .iter() + .enumerate() + .map(|(i, title)| natural(ui, RichText::new(*title).strong(), rows.iter().filter_map(|r| r.get(i).cloned()))) + .collect(); + egui::ScrollArea::horizontal().id_salt("credits_models").auto_shrink([false, false]).show(ui, |ui| { + fitted_table(ui, "credits_models", &widths, &MODEL_GROW) + .header(h + 4.0, |mut header| { + for (title, r) in titles.into_iter().zip(right) { + header.col(|ui| { + aligned(ui, r, |ui| ui.label(RichText::new(title).strong())); + }); + } + }) + .body(|mut body| { + for cells in rows { + body.row(h, |mut row| { + for (text, r) in cells.into_iter().zip(right) { + row.col(|ui| { + aligned(ui, r, |ui| ui.label(text)); + }); + } + }); + } + }); }); } @@ -397,4 +539,131 @@ mod tests { assert_eq!(day("2026-10-07T18:45:23Z"), "2026-10-07"); assert_eq!(day(""), "—"); } + + #[test] + fn columns_fill_the_room_and_spare_width_goes_to_growing_columns() { + assert_eq!(fit_widths(&[100.0, 40.0, 60.0], &[1.0, 0.0, 0.0], 300.0), [200.0, 40.0, 60.0]); + assert_eq!(fit_widths(&[100.0, 40.0, 60.0], &[1.0, 2.0, 0.0], 230.0), [110.0, 60.0, 60.0]); + let w = fit_widths(&[123.0, 45.5, 67.25], &[1.0, 0.0, 2.0], 512.0); + assert!((w.iter().sum::() - 512.0).abs() < 0.01, "a fitted table neither scrolls nor leaves a gap: {w:?}"); + } + + #[test] + fn too_little_room_shrinks_only_growing_columns() { + assert_eq!(fit_widths(&[100.0, 40.0, 60.0], &[1.0, 0.0, 0.0], 150.0), [50.0, 40.0, 60.0]); + assert_eq!(fit_widths(&[100.0, 40.0, 60.0], &[1.0, 0.0, 0.0], 0.0), [MIN_COLUMN, 40.0, 60.0]); + assert_eq!(fit_widths(&[100.0, 40.0], &[0.0, 0.0], 50.0), [100.0, 40.0]); + } + + #[test] + fn a_column_at_the_minimum_leaves_the_shortfall_to_the_others() { + let w = fit_widths(&[100.0, 300.0, 50.0], &[1.0, 1.0, 0.0], 140.0); + assert!((w.iter().sum::() - 140.0).abs() < 0.01, "the columns still fit the room: {w:?}"); + assert!(w.iter().all(|c| *c >= MIN_COLUMN), "no column goes below the minimum: {w:?}"); + assert_eq!(fit_widths(&[100.0, 300.0, 50.0], &[1.0, 1.0, 0.0], 0.0), [MIN_COLUMN, MIN_COLUMN, 50.0]); + } + + const WINDOW: f32 = 640.5; + const NATURAL: [f32; 3] = [120.0, 80.0, 80.0]; + const GROW: [f32; 3] = [1.0, 0.0, 0.0]; + + /// One frame of a table long enough to scroll; the rects of its header cells. + fn table_frame(ctx: &egui::Context, width: f32, natural: &[f32], events: Vec) -> Vec { + scrolled_frame(ctx, width, natural, 0.0, events) + } + + /// [`table_frame`], scrolled `offset` points sideways. + fn scrolled_frame(ctx: &egui::Context, width: f32, natural: &[f32], offset: f32, events: Vec) -> Vec { + let screen_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(width, 300.0))); + let raw = egui::RawInput { screen_rect, events, ..Default::default() }; + let mut cells = Vec::new(); + let out = ctx.run_ui(raw, |ui| { + cells.clear(); + egui::ScrollArea::horizontal().auto_shrink([false, false]).horizontal_scroll_offset(offset).show(ui, |ui| { + fitted_table(ui, "test", natural, &GROW) + .header(20.0, |mut header| { + for _ in natural { + header.col(|ui| cells.push(ui.max_rect())); + } + }) + .body(|mut body| { + for _ in 0..40 { + body.row(20.0, |mut row| { + for _ in natural { + row.col(|ui| { + ui.label("x"); + }); + } + }); + } + }); + }); + }); + out.drop_without_applying_deltas(); + cells + } + + /// A new context with the table open and settled, and the rects of its header cells. + fn opened_table() -> (egui::Context, Vec) { + let ctx = egui::Context::default(); + table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + let cells = table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + (ctx, cells) + } + + /// Drags a column's right edge, which runs midway through the gap after `cell`, then lets go. + fn drag_edge(ctx: &egui::Context, cell: egui::Rect, by: f32) { + use egui::{Event, Modifiers, PointerButton}; + let start = egui::pos2(cell.right() + 4.0, cell.center().y); + let halfway = start + egui::vec2(by / 2.0, 0.0); + let end = start + egui::vec2(by, 0.0); + let moved = Event::PointerMoved; + let button = |pos, pressed| Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }; + for event in [moved(start), button(start, true), moved(halfway), moved(end), button(end, false)] { + table_frame(ctx, WINDOW, &NATURAL, vec![event]); + } + } + + #[test] + fn every_column_can_be_dragged_at_its_right_edge_and_stays_there() { + let (_, opened) = opened_table(); + for (dragged, cell) in opened.iter().enumerate() { + let (ctx, _) = opened_table(); + drag_edge(&ctx, *cell, -30.0); + table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + let after = table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + for (column, (was, now)) in opened.iter().zip(&after).enumerate() { + let want = if column == dragged { was.width() - 30.0 } else { was.width() }; + assert!((now.width() - want).abs() < 1.0, "dragging column {dragged}: column {column} is {now:?}, was {was:?}"); + } + } + } + + #[test] + fn a_changed_fit_replaces_the_column_widths() { + let (ctx, opened) = opened_table(); + drag_edge(&ctx, opened[0], -30.0); + let longer_name = table_frame(&ctx, WINDOW, &[200.0, 80.0, 80.0], Vec::new()); + assert!((longer_name[0].width() - opened[0].width()).abs() < 1.0, "a longer name drops a dragged width: {longer_name:?}"); + let wider = table_frame(&ctx, WINDOW, &[120.0, 200.0, 80.0], Vec::new()); + let widths: Vec = wider.iter().map(|c| c.width().round()).collect(); + assert!(widths.ends_with(&[200.0, 80.0]), "a longer cell widens its column: {widths:?}"); + assert!(wider[2].right() < WINDOW, "the table still fits the window: {wider:?}"); + let narrower = table_frame(&ctx, 400.0, &NATURAL, Vec::new()); + assert!(narrower[2].right() < 400.0, "a narrower window refits the table: {narrower:?}"); + } + + #[test] + fn scrolling_sideways_keeps_dragged_widths() { + let (ctx, opened) = opened_table(); + drag_edge(&ctx, opened[0], 100.0); + table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + let dragged = table_frame(&ctx, WINDOW, &NATURAL, Vec::new()); + assert!((dragged[0].width() - opened[0].width() - 100.0).abs() < 1.0, "the drag took: {dragged:?}"); + for offset in [13.3, 27.77, 41.1, 0.6] { + scrolled_frame(&ctx, WINDOW, &NATURAL, offset, Vec::new()); + let scrolled = scrolled_frame(&ctx, WINDOW, &NATURAL, offset, Vec::new()); + assert!((scrolled[0].width() - dragged[0].width()).abs() < 1.0, "scrolled {offset}: {scrolled:?}"); + } + } } diff --git a/crates/ui-egui/src/panels/thumbs.rs b/crates/ui-egui/src/panels/thumbs.rs index 15bf9d373..946391c35 100644 --- a/crates/ui-egui/src/panels/thumbs.rs +++ b/crates/ui-egui/src/panels/thumbs.rs @@ -112,6 +112,21 @@ pub fn show(app: &mut SlideApp, ui: &mut Ui) { } } +/// Slide index -> the first section starting there, built once instead of searched per slide. +fn section_starts(doc: &deckcraft_model::Presentation) -> std::collections::HashMap { + let mut index_of = std::collections::HashMap::new(); + for (i, s) in doc.slides.iter().enumerate() { + index_of.entry(s.id).or_insert(i); + } + let mut starts = std::collections::HashMap::new(); + for (si, s) in doc.sections.iter().enumerate() { + if let Some(&i) = s.slides.first().and_then(|id| index_of.get(id)) { + starts.entry(i).or_insert(si); + } + } + starts +} + /// Render the scrollable thumbnail list, scrolling the current slide into view when it changes. fn list(app: &mut SlideApp, ui: &mut Ui) { let t = Tokens::get(ui.ctx()); @@ -135,11 +150,10 @@ fn list(app: &mut SlideApp, ui: &mut Ui) { let ppp = ui.ctx().pixels_per_point(); let mut budget = 3u32; let mut rects = vec![]; - let section_starts: Vec<(usize, usize)> = - doc.sections.iter().enumerate().filter_map(|(si, s)| s.slides.first().and_then(|id| doc.slide_index(*id)).map(|i| (i, si))).collect(); + let section_starts = section_starts(&doc); let mut need_more = false; for i in 0..doc.slides.len() { - if let Some((_, si)) = section_starts.iter().find(|(start, _)| *start == i) + if let Some(si) = section_starts.get(&i) && let Some(sec) = doc.sections.get(*si) { ui.add_space(4.0); @@ -434,6 +448,26 @@ mod tests { h } + #[test] + fn section_starts_map_each_first_slide_to_its_first_section() { + let mut s = Session::with_new(); + for _ in 0..3 { + s.execute("slide.new", &json!({})).unwrap(); + } + let mut doc = (*s.active().unwrap().doc).clone(); + let ids: Vec<_> = doc.slides.iter().map(|sl| sl.id).collect(); + doc.sections = vec![ + deckcraft_model::Section { name: "a".into(), slides: vec![ids[0], ids[1]] }, + deckcraft_model::Section { name: "dup".into(), slides: vec![ids[0]] }, + deckcraft_model::Section { name: "c".into(), slides: vec![ids[2]] }, + deckcraft_model::Section { name: "empty".into(), slides: vec![] }, + ]; + let starts = section_starts(&doc); + assert_eq!(starts.get(&0), Some(&0), "the first section starting at a slide wins"); + assert_eq!(starts.get(&2), Some(&2)); + assert_eq!(starts.len(), 2); + } + /// Vertical scroll offset of the thumbnail pane, as recorded each frame by `show`. fn thumbs_scroll(h: &Harness<'_, SlideApp>) -> f32 { h.state().ui.thumbs_scroll diff --git a/crates/ui-egui/src/presenter.rs b/crates/ui-egui/src/presenter.rs index 95569778d..a58878830 100644 --- a/crates/ui-egui/src/presenter.rs +++ b/crates/ui-egui/src/presenter.rs @@ -49,10 +49,15 @@ pub struct Snapshot { pub paused: bool, pub notes_px: f32, pub blank: Option, + /// All slides is open in the presenter window (#1866); the audience keeps the current slide. pub grid: bool, pub zoom_armed: bool, pub current: Option, pub next: Option, + /// The All slides thumbnails, in slide order, while `grid` is on. `None` in a sent delta + /// means "unchanged since the last snapshot the window received". + #[serde(default)] + pub slides: Option>, } /// Images acknowledged by a successful transport send, never merely attempted images. @@ -60,6 +65,7 @@ pub struct Snapshot { pub struct DeltaCache { current: Option, next: Option, + slides: Option>, token: String, } impl DeltaCache { @@ -75,6 +81,12 @@ impl DeltaCache { let next_changed = fresh || self.next != snapshot.next; let current = snapshot.current.clone(); let next = snapshot.next.clone(); + let slides = snapshot.slides.clone(); + if !fresh && self.slides == slides { + snapshot.slides = None; + } else if snapshot.slides.is_none() { + snapshot.slides = Some(Vec::new()); + } if !current_changed { snapshot.current = None; } @@ -84,14 +96,28 @@ impl DeltaCache { send(snapshot, current_changed, next_changed)?; self.current = current; self.next = next; + self.slides = slides; self.token = token.to_owned(); Ok(()) } } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum Command { - Key { name: String, command: bool, shift: bool, alt: bool, ctrl: bool }, - Zoom { x: f32, y: f32 }, + Key { + name: String, + command: bool, + shift: bool, + alt: bool, + ctrl: bool, + }, + Zoom { + x: f32, + y: f32, + }, + /// A slide chosen in the presenter window's All slides grid (#1866). + Goto { + slide: usize, + }, End, } /// Implemented by the browser host. Native hosts retain the existing egui viewport. @@ -112,6 +138,8 @@ pub struct View { next: Option, previous_current: Option, previous_next: Option, + grid_source: Option>, + grid: Vec>, } impl View { /// Install the same interface and dynamic fallback fonts as SlideApp before layout. @@ -147,6 +175,10 @@ impl View { .collect::>() }); for (key, modifiers) in keys { + if snapshot.grid && matches!(key, egui::Key::Space | egui::Key::Enter) { + // A focused thumbnail takes these to choose its slide; the show ignores them while the grid is open. + continue; + } // Consume once before widgets see Space/Enter, avoiding a second button activation. ui.input_mut(|i| i.consume_key(modifiers, key)); commands.push(Command::Key { @@ -200,6 +232,11 @@ impl View { } }); ui.separator(); + if snapshot.grid { + // All slides opens here, as in the desktop Presenter View; the audience keeps the current slide. + self.grid_ui(ui, snapshot, &mut commands); + return commands; + } ui.columns(2, |columns| { let left = &mut columns[0]; if snapshot.ended { @@ -227,9 +264,6 @@ impl View { if snapshot.zoom_armed { left.label("Click the preview to zoom."); } - if snapshot.grid { - left.label("Choose a slide in the audience window, or press G to return."); - } let right = &mut columns[1]; right.label("Next"); if let Some(texture) = &self.next { @@ -260,6 +294,43 @@ impl View { commands } } +impl View { + fn grid_ui(&mut self, ui: &mut egui::Ui, snapshot: &Snapshot, commands: &mut Vec) { + let slides = snapshot.slides.as_deref().unwrap_or_default(); + if self.grid_source.as_deref() != Some(slides) { + self.grid_source = Some(slides.to_vec()); + self.grid = slides + .iter() + .enumerate() + .map(|(i, p)| p.image().map(|image| ui.ctx().load_texture(format!("remote-grid-{i}"), image, egui::TextureOptions::LINEAR))) + .collect(); + } + ui.heading("All slides"); + ui.label("Choose a slide to show it, or press G to return."); + if snapshot.total > crate::show::GRID_CAP { + ui.label(format!("First {} slides", crate::show::GRID_CAP)); + } + egui::ScrollArea::vertical().show(ui, |ui| { + ui.horizontal_wrapped(|ui| { + for (i, texture) in self.grid.iter().enumerate() { + let size = egui::vec2(160.0, 90.0); + let label = format!("{}", i + 1); + let label = if i == snapshot.slide { egui::RichText::new(label).strong() } else { egui::RichText::new(label) }; + let response = match texture { + Some(t) => ui.add(egui::Button::image_and_text( + egui::Image::new(t).fit_to_exact_size(fit_preview(t.size_vec2(), size.x, size.y)), + label, + )), + None => ui.add_sized(size, egui::Button::new(label)), + }; + if response.clicked() { + commands.push(Command::Goto { slide: i }); + } + } + }); + }); + } +} fn fit_preview(size: egui::Vec2, width: f32, height: f32) -> egui::Vec2 { size * (width.max(1.0) / size.x.max(1.0)).min(height.max(1.0) / size.y.max(1.0)) } @@ -282,6 +353,7 @@ mod tests { zoom_armed: false, current: Some(Preview { width: 1, height: 1, rgba: vec![255; 4] }), next: Some(Preview { width: 1, height: 1, rgba: vec![0; 4] }), + slides: None, } } @@ -418,6 +490,7 @@ mod tests { zoom_armed: false, current: None, next: None, + slides: None, }; let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_ui_state( |ui, (view, snapshot, sent): &mut (View, Snapshot, Vec)| { @@ -437,6 +510,30 @@ mod tests { assert!(matches!(h.state().2.last(), Some(Command::Key { name, .. }) if name == egui::Key::Space.name())); } + #[test] + fn all_slides_opens_in_the_popup_and_picks_a_slide() { + use egui_kittest::kittest::Queryable; + let thumb = Preview { width: 16, height: 9, rgba: vec![200; 16 * 9 * 4] }; + let mut snapshot = snapshot(); + snapshot.total = 3; + snapshot.grid = true; + snapshot.slides = Some(vec![thumb.clone(), thumb.clone(), thumb]); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1000.0, 700.0)).build_ui_state( + |ui, (view, snapshot, sent): &mut (View, Snapshot, Vec)| { + sent.extend(view.ui(ui, snapshot)); + }, + (View::default(), snapshot, Vec::new()), + ); + h.run_steps(2); + assert!(h.query_by_label("Notes").is_none(), "the grid replaces the presenter panes, as on desktop (#1866)"); + h.get_by_label("3").click(); + h.run_steps(2); + assert_eq!(h.state().2, vec![Command::Goto { slide: 2 }]); + h.key_press(egui::Key::Enter); + h.run_steps(2); + assert!(!h.state().2.iter().any(|c| matches!(c, Command::Key { .. })), "Enter is left to the focused thumbnail"); + } + #[test] fn previews_are_bounded_and_bad_dimensions_are_rejected() { let image = egui::ColorImage::filled([1280, 720], egui::Color32::RED); diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index 0cb2b458b..db7983c2b 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -18,6 +18,7 @@ mod presenter; mod render; mod ui; mod zoom; +pub(crate) use zoom::GRID_CAP; #[cfg(test)] use media::{on_screen, pointer_on_media_bar, shown_slide_rect, slide_media_bars}; @@ -65,6 +66,8 @@ pub struct Show { preview_spawns: usize, remote_preview: Option, remote_next: Option<(usize, crate::presenter::Preview)>, + /// All slides thumbnails sent to the web presenter window while its grid is open (#1866). + remote_grid: Vec, remote_sent: f64, remote_opened: bool, /// The next slide's first frame, rendered in the background while this one is shown so @@ -237,6 +240,7 @@ impl Show { preview_spawns: 0, remote_preview: None, remote_next: None, + remote_grid: vec![], remote_sent: f64::NEG_INFINITY, remote_opened: false, prefetch: None, diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index 1dd6b09ef..a01989ac0 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1132,6 +1132,8 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { opens: usize, closes: usize, commands: Vec, + last: Option, + gone: bool, } struct Mock(Rc>); impl Host for Mock { @@ -1140,12 +1142,14 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { Ok(()) } fn is_open(&self) -> bool { - true + !self.0.borrow().gone } fn commands(&mut self) -> Vec { std::mem::take(&mut self.0.borrow_mut().commands) } - fn update(&mut self, _: Snapshot) {} + fn update(&mut self, snapshot: Snapshot) { + self.0.borrow_mut().last = Some(snapshot); + } fn close(&mut self) { self.0.borrow_mut().closes += 1; } @@ -1173,6 +1177,39 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { h.run_steps(2); assert_eq!(h.state().show.as_ref().unwrap().state.slide, slide); } + // #1864: the web popup's Previous/Next buttons send the DOM key names. + for (name, slide) in [("ArrowRight", 1), ("ArrowLeft", 0)] { + bridge.borrow_mut().commands.push(Command::Key { name: name.into(), command: false, shift: false, alt: false, ctrl: false }); + h.run_steps(2); + assert_eq!(h.state().show.as_ref().unwrap().state.slide, slide, "{name}"); + } + // #1866: All slides opens in the web Presenter View, as on desktop; the audience keeps its slide. + bridge.borrow_mut().commands.push(Command::Key { name: "G".into(), command: false, shift: false, alt: false, ctrl: false }); + h.run_steps(2); + let show = h.state().show.as_ref().unwrap(); + assert!(show.presenter_grid && !show.grid, "G from the popup opens its own grid, not the audience's"); + for _ in 0..20 { + h.step(); + std::thread::sleep(std::time::Duration::from_millis(15)); + } + let last = bridge.borrow().last.clone().unwrap(); + assert!(last.grid); + assert_eq!(last.slides.as_ref().map(Vec::len), Some(3), "the popup gets a thumbnail per slide"); + assert!(last.slides.iter().flatten().all(|p| p.width.max(p.height) <= 96), "thumbnails stay small enough for one message"); + assert!(last.current.is_none() && last.next.is_none(), "the grid replaces both previews"); + bridge.borrow_mut().commands.push(Command::Goto { slide: 2 }); + h.run_steps(2); + let show = h.state().show.as_ref().unwrap(); + assert!(!show.presenter_grid && !show.grid); + assert_eq!(show.state.slide, 2); + // Closing the popup with its grid open must not hold auto-advance for the rest of the show. + bridge.borrow_mut().commands.push(Command::Key { name: "G".into(), command: false, shift: false, alt: false, ctrl: false }); + h.run_steps(2); + assert!(h.state().show.as_ref().unwrap().presenter_grid); + bridge.borrow_mut().gone = true; + h.run_steps(2); + assert!(!h.state().show.as_ref().unwrap().presenter_grid); + bridge.borrow_mut().gone = false; bridge.borrow_mut().commands.push(Command::End); h.run_steps(2); assert!(h.state().show.is_none()); diff --git a/crates/ui-egui/src/show/ui.rs b/crates/ui-egui/src/show/ui.rs index 072714a31..d048fb93c 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -80,6 +80,10 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { && !show.reading && let Some(host) = app.services.presenter.as_mut() { + if show.remote_opened && show.presenter_grid && !host.is_open() { + // A grid left open in the closed popup would hold auto-advance for the rest of the show. + show.presenter_grid = false; + } if !show.remote_opened { show.remote_opened = true; if let Err(message) = host.open() { @@ -94,11 +98,20 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { show.zoom_arm = false; } crate::presenter::Command::Key { name, command, shift, alt, ctrl } => { - if let Some(key) = egui::Key::ALL.iter().copied().find(|key| key.name() == name) { + if let Some(key) = egui::Key::from_name(&name) { let modifiers = egui::Modifiers { command, shift, alt, ctrl, mac_cmd: command && !ctrl }; - out.keep &= handle_keys(&mut show, &doc, now, vec![(key, modifiers)], false).0; + // The popup is the Presenter View: G opens All slides there, not on the audience screen (#1866). + out.keep &= handle_keys(&mut show, &doc, now, vec![(key, modifiers)], true).0; } } + crate::presenter::Command::Goto { slide } if show.presenter_grid && slide < doc.slides.len() => { + show.presenter_grid = false; + show.blank = None; + show.zoom = None; + show.zoom_arm = false; + let action = show.state.goto(&doc, slide); + out.keep &= show.apply(action, &doc, now); + } _ => {} } } @@ -143,6 +156,20 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { ); show.remote_next = crate::presenter::Preview::from_image(&crate::textures::to_color_image(&image, false)).map(|image| (n, image)); } + if !show.presenter_grid { + show.remote_grid.clear(); + } else if show.remote_grid.len() < grid_indices(doc.slides.len()).len() { + // A few thumbnails per update keep the audience window responsive on the web. + for i in grid_indices(doc.slides.len()).into_iter().skip(show.remote_grid.len()).take(4) { + // At most 96 px on the long side: 48 thumbnails stay well under the popup's message limit. + let (w, h) = (doc.slide_size.width.max(1.0), doc.slide_size.height.max(1.0)); + let scale = REMOTE_THUMB / w.max(h); + let size = (((w * scale).round() as u32).max(1), ((h * scale).round() as u32).max(1)); + let image = deckcraft_render::render_slide(&doc, i, &RenderOpts { scale, size: Some(size), ..Default::default() }); + let preview = crate::presenter::Preview::from_image(&crate::textures::to_color_image(&image, false)); + show.remote_grid.push(preview.unwrap_or(crate::presenter::Preview { width: 1, height: 1, rgba: vec![0; 4] })); + } + } host.update(crate::presenter::Snapshot { slide: show.state.slide, total: doc.slides.len(), @@ -152,10 +179,12 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { paused: show.timer.paused(), notes_px: show.notes_px, blank: show.blank_name().map(str::to_owned), - grid: show.grid, + grid: show.presenter_grid, zoom_armed: show.zoom_arm, - current: show.remote_preview.clone(), - next: show.remote_next.as_ref().filter(|(n, _)| Some(*n) == next).map(|(_, image)| image.clone()), + // The grid replaces both previews in the popup; leaving them out keeps one message small. + current: show.remote_preview.clone().filter(|_| !show.presenter_grid), + next: show.remote_next.as_ref().filter(|(n, _)| Some(*n) == next && !show.presenter_grid).map(|(_, image)| image.clone()), + slides: show.presenter_grid.then(|| show.remote_grid.clone()), }); show.remote_sent = now; } @@ -164,6 +193,9 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { finish(app, show, &f, out.keep); } +/// Longest side of an All slides thumbnail sent to the web presenter window (#1866). +const REMOTE_THUMB: f64 = 96.0; + fn paint_hidden(ui: &Ui, doc: &Presentation, show: &Show) { if show.started { let full = ui.max_rect(); diff --git a/crates/ui-egui/src/show/zoom.rs b/crates/ui-egui/src/show/zoom.rs index 9c45fd4a2..da6104905 100644 --- a/crates/ui-egui/src/show/zoom.rs +++ b/crates/ui-egui/src/show/zoom.rs @@ -182,7 +182,7 @@ const WHEEL_NOTCHES_PER_DOUBLING: f32 = 4.0; /// PowerPoint's slide show magnifier goes up to 400%. const WHEEL_ZOOM_MAX: f32 = 4.0; -pub(super) const GRID_CAP: usize = 48; +pub(crate) const GRID_CAP: usize = 48; const TOOL_BAR_W: f32 = 520.0; diff --git a/crates/ui-egui/src/sorter.rs b/crates/ui-egui/src/sorter.rs index 6c9626380..5205a835d 100644 --- a/crates/ui-egui/src/sorter.rs +++ b/crates/ui-egui/src/sorter.rs @@ -476,7 +476,7 @@ fn outline_estimate(s: &deckcraft_model::Slide) -> f32 { let lines: usize = s .shapes .iter() - .filter(|x| !x.ph_type().is_some_and(|k| k.is_title())) + .filter(|x| !x.ph_type().is_some_and(|k| k.is_title() || k.is_footer_kind())) .filter_map(|x| x.text.as_ref()) .filter(|tb| !tb.is_empty()) .map(|tb| tb.paragraphs.len().max(1)) @@ -512,7 +512,8 @@ fn outline_slide(app: &mut SlideApp, ui: &mut Ui, t: &Tokens, i: usize, s: &deck ui.data_mut(|d| d.insert_temp(id, v)); } }); - for sh in s.shapes.iter().filter(|x| !x.ph_type().is_some_and(|k| k.is_title())) { + // Footer, date and slide-number placeholders are not outline text, as in PowerPoint. + for sh in s.shapes.iter().filter(|x| !x.ph_type().is_some_and(|k| k.is_title() || k.is_footer_kind())) { let Some(tb) = &sh.text else { continue }; if tb.is_empty() { continue; diff --git a/crates/ui-egui/src/sorter/ui_tests.rs b/crates/ui-egui/src/sorter/ui_tests.rs index 966974616..07b42d02f 100644 --- a/crates/ui-egui/src/sorter/ui_tests.rs +++ b/crates/ui-egui/src/sorter/ui_tests.rs @@ -259,3 +259,24 @@ fn shift_and_ctrl_shift_clicks_select_ranges() { click(&mut h, 4, Modifiers::COMMAND | Modifiers::SHIFT); assert_eq!(selected(&h), vec![0, 2, 3]); } + +/// The outline view lists a slide's title and body text, not its footer, date and slide-number +/// placeholders, as PowerPoint's does: turning them on leaves the slide's outline block as it was. +#[test] +fn the_outline_view_leaves_footer_date_and_slide_number_out() { + let mut s = Session::with_new(); + s.execute("slide.new", &json!({"layout": "titleAndContent", "title": "Agenda", "body": "First point\nSecond point"})).unwrap(); + let slide = |s: &Session| { + let st = s.doc().unwrap(); + st.doc.slides[st.selection.slide].clone() + }; + let plain = super::outline_estimate(&slide(&s)); + s.execute( + "design.headerFooter", + &json!({"footer": true, "footerText": "Company Confidential", "slideNumber": true, "date": true, "dateText": "1 May 2026"}), + ) + .unwrap(); + let with_footer = slide(&s); + assert!(with_footer.shapes.iter().any(|x| x.ph_type().is_some_and(|k| k.is_footer_kind())), "the footers are on the slide"); + assert_eq!(super::outline_estimate(&with_footer), plain); +} diff --git a/xtask/src/stats.rs b/xtask/src/stats.rs index f40b1300a..20cd5725f 100644 --- a/xtask/src/stats.rs +++ b/xtask/src/stats.rs @@ -18,7 +18,13 @@ fn walk(dir: &Path, c: &mut Counts) { let p = e.path(); let name = e.file_name(); let name = name.to_string_lossy(); - if p.is_dir() { + // `file_type` does not follow links, so a directory link is not walked (and aliases are + // not counted twice). + let Ok(kind) = e.file_type() else { continue }; + if kind.is_symlink() { + continue; + } + if kind.is_dir() { // Nested fuzz workspaces and build output are not part of the crate. if name != "target" && name != "fuzz" && !name.starts_with('.') { walk(&p, c);