From cc6773c95f1f23800c82329ec404f68868c8b22b Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Thu, 22 Oct 2026 21:02:59 +0200 Subject: [PATCH 01/38] Use evaluated field values in the placed text so the PDF text layer shows the slide number (#1153) --- crates/render/src/placed.rs | 21 ++++++++++++++++++--- crates/render/src/tests.rs | 12 ++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/render/src/placed.rs b/crates/render/src/placed.rs index 0164f3f07..f83af46d8 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}; @@ -162,7 +162,22 @@ 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() }); + // 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 body = body.clone(); + for para in &mut body.paragraphs { + let src = para.clone(); + for run in &mut para.runs { + if let RunKind::Field { field } = &run.kind { + let lang = resolve::run(rctx, s, &src, &run.props).lang; + if let Some(v) = fields.field_in(field, lang.as_deref()) { + run.text = v; + run.kind = RunKind::Text; + } + } + } + } + out.texts.push(PlacedText { layout, transform: m, body }); } /// The hyperlink of the model run containing character `at` of paragraph `para`. diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 4ee1c889d..9987bd7c9 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2595,3 +2595,15 @@ fn empty_bullet_is_gray_only_while_editing_and_never_in_exports() { assert!(changed.iter().any(|(px, _)| px[0] == 179 && px[1] == 179 && px[2] == 179)); 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)); +} From 813378cc20aa0d94228bb3f700645e7d546b795d Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:08:53 +0200 Subject: [PATCH 02/38] Keep the PDF text layer aligned with line breaks, use one clock reading for raster and text layer, and test the page text (#1153) --- crates/pdf/src/lib.rs | 9 +++--- crates/pdf/src/tests.rs | 61 ++++++++++++++++++++++++++++++++++--- crates/render/src/lib.rs | 2 +- crates/render/src/placed.rs | 30 ++++++++++-------- crates/render/src/tests.rs | 9 ++++++ 5 files changed, 89 insertions(+), 22 deletions(-) diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index 22dc42d36..cc5d5e56a 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -299,7 +299,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) @@ -440,7 +440,8 @@ impl Exporter<'_> { let k = b.width() / sw; // Raster at the requested dpi relative to the box size on paper. let scale = (b.width() / 72.0 * self.opts.dpi.clamp(36.0, 600.0)) / sw; - 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). @@ -476,7 +477,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; @@ -494,7 +495,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 2bc5c41ac..fd1839377 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -200,8 +200,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"); @@ -223,8 +221,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); @@ -275,7 +282,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/render/src/lib.rs b/crates/render/src/lib.rs index cf124a9f6..722162721 100644 --- a/crates/render/src/lib.rs +++ b/crates/render/src/lib.rs @@ -53,7 +53,7 @@ pub use extent::{Region, TILE, layout_extent, pasteboard_region, slide_extent}; pub use images::{decode as decode_image, gif_frame}; pub use morph_path::PathMorph; pub use morph_text::TextMorph; -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/placed.rs b/crates/render/src/placed.rs index f83af46d8..2081b2400 100644 --- a/crates/render/src/placed.rs +++ b/crates/render/src/placed.rs @@ -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); @@ -164,20 +170,20 @@ fn place_text(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, owner: ShapeId } // 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 body = body.clone(); - for para in &mut body.paragraphs { - let src = para.clone(); - for run in &mut para.runs { - if let RunKind::Field { field } = &run.kind { - let lang = resolve::run(rctx, s, &src, &run.props).lang; - if let Some(v) = fields.field_in(field, lang.as_deref()) { - run.text = v; - run.kind = RunKind::Text; - } + 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; } } } - out.texts.push(PlacedText { layout, transform: m, body }); + 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/tests.rs b/crates/render/src/tests.rs index 9987bd7c9..b9011f999 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -2607,3 +2607,12 @@ fn placed_text_holds_the_field_value_not_the_saved_placeholder() { // 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"); +} From d8936f94af1dd1a9a9c72cb626c5a0062ed19abb Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Tue, 27 Oct 2026 10:58:38 +0100 Subject: [PATCH 03/38] Keep whole character after a backslash in RTF outline import Found while testing crates/format/src/rtf.rs:301-326 (symbol / text, reached via escape) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/format/src/rtf.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/format/src/rtf.rs b/crates/format/src/rtf.rs index 4800aba26..b546a832f 100644 --- a/crates/format/src/rtf.rs +++ b/crates/format/src/rtf.rs @@ -299,7 +299,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 +309,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 +421,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() { From 6303166a1816693d7dfa7b04263cee61daf062fa Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Mon, 19 Oct 2026 02:58:27 -0500 Subject: [PATCH 04/38] [PP1] show.setup showMediaControls survives a pptx save and reopen PowerPoint stores Show media controls as the p14:showMediaCtrls extension inside p:showPr (uri {2FDB2607-...}); absent means shown. Write it when hidden, read it back. Co-Authored-By: Claude Sonnet 5.5 --- crates/pptx/src/read.rs | 1 + crates/pptx/src/write.rs | 8 ++++++++ crates/pptx/tests/it/roundtrip.rs | 9 +++++++++ 3 files changed, 18 insertions(+) diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 917aa9277..2df349cb7 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -1283,6 +1283,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/write.rs b/crates/pptx/src/write.rs index 5d3e60e92..37cf5cd7d 100644 --- a/crates/pptx/src/write.rs +++ b/crates/pptx/src/write.rs @@ -1181,6 +1181,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/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index 370cc9a9f..c61e6e433 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -1969,3 +1969,12 @@ fn default_durations_match_powerpoint() { assert_eq!(a.4, ms, "{effect}"); } } + +/// Show media controls is PowerPoint's `p14:showMediaCtrls` extension inside `p:showPr`. +#[test] +fn hidden_media_controls_survive_save_and_reopen() { + let mut p = Presentation::default(); + p.show.show_media_controls = false; + assert!(!round(&p).show.show_media_controls); + assert!(round(&Presentation::default()).show.show_media_controls); +} From b9eaaeb23b011b7a1e96568a4101f6d0d3cc13b9 Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Mon, 2 Nov 2026 17:03:02 -0600 Subject: [PATCH 05/38] Test: hidden media controls are written as PowerPoint's p14 extension The round trip now also checks the written presProps.xml: inside p:showPr, the extension uri and p14:showMediaCtrls with its namespace declared on the element; the default writes nothing. Co-Authored-By: Claude Opus 5.5 --- crates/pptx/tests/it/roundtrip.rs | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/pptx/tests/it/roundtrip.rs b/crates/pptx/tests/it/roundtrip.rs index c61e6e433..521956a06 100644 --- a/crates/pptx/tests/it/roundtrip.rs +++ b/crates/pptx/tests/it/roundtrip.rs @@ -1970,11 +1970,30 @@ fn default_durations_match_powerpoint() { } } -/// Show media controls is PowerPoint's `p14:showMediaCtrls` extension inside `p:showPr`. +/// 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); - assert!(round(&Presentation::default()).show.show_media_controls); + let default = Presentation::default(); + assert!(!props(&default).contains("showMediaCtrls")); + assert!(round(&default).show.show_media_controls); } From 0107186cee878b399ddbd0eac27899db3cabfd00 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:46:09 -0500 Subject: [PATCH 06/38] Engine: check the temporary file's path against the write root (#1792) --- crates/engine/src/io.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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); + } } From 42108d0ee75ac621d8849938be744be47e06c17c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:53:44 -0500 Subject: [PATCH 07/38] PPTX export: index animation targets, cache build roles, serialize run properties once per text run (#1788) * PPTX export: index animation targets, cache a paragraph's build roles, serialize run properties once per text run * address review: warn once when a by-paragraph build is truncated --- crates/pptx/src/write/dml.rs | 14 +++++++++-- crates/pptx/src/write/timing.rs | 44 ++++++++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 5 deletions(-) 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)); } From b7d57bacab02c2b8d2d69903337ae681f931e58d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:54:06 -0500 Subject: [PATCH 08/38] Text edit: splice inserted paragraphs once and paste runs without rescanning (#1787) --- crates/model/src/edit.rs | 149 +++++++++++++++++++++++++++++++++------ 1 file changed, 128 insertions(+), 21 deletions(-) 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"); From a72a5167d0fddf51a4b915e8a08e0d028ea8b72f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:59:29 -0500 Subject: [PATCH 09/38] Morph: pick the next curve to split from a heap, not a full scan (#1782) --- crates/geom/src/morph.rs | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) 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)] }; From 0233d2906424f024a89041f1f95c28c89eca20d9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 19:59:51 -0500 Subject: [PATCH 10/38] Spelling: bound a word's language and index results by offset (#1767) * engine: bound a word's language and index spelling results by offset * engine: pass the test run's text by value --- crates/engine/src/spell.rs | 50 ++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 8 deletions(-) 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"); From 872cdbf33136545699bdfa280dbbebbb6cbd9557 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:04:49 -0500 Subject: [PATCH 11/38] pptx: take nested math text once and check an object's part for relationships once (#1762) --- crates/pptx/src/read.rs | 5 +++++ crates/pptx/src/read/dml.rs | 31 ++++++++++++++++++++++++++----- crates/pptx/src/read/shapes.rs | 3 ++- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 288e497f8..2056d53d7 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); 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()); From d2a8be197d1e5674f6fcf2193e38f8a97898380a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:05:11 -0500 Subject: [PATCH 12/38] geom: measure each follow band once when mapping warped lines (#1758) --- crates/geom/src/warp.rs | 28 +++++++++++++++++++--------- crates/geom/src/warp/tests.rs | 17 +++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) 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); +} From 65d5c28512a4fb452e6d99d6d265eacfdb29bcc5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:05:32 -0500 Subject: [PATCH 13/38] Text: sort a paragraph's tab stops once and find the next by binary search (#1757) * text: sort a paragraph's tab stops once and find the next by binary search * address review: skip the tab-stop lookup for a NaN position --- crates/text/src/breaking.rs | 41 ++++++++++++++++++++++++++----------- crates/text/src/tests.rs | 10 +++++++++ 2 files changed, 39 insertions(+), 12 deletions(-) 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/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 From c6b7de43bc4e69d8a654156bdc7c8c4c47d5508a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:05:53 -0500 Subject: [PATCH 14/38] engine: use sets for slide-range expansion, added slide selection and repeated-word offsets (#1756) --- crates/engine/src/cmd/file.rs | 13 ++++++++++++- crates/engine/src/cmd/review.rs | 2 +- crates/engine/src/cmd/slide.rs | 3 ++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 8943eb9a3..4f8580c86 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -397,6 +397,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()), @@ -412,7 +413,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); } } @@ -776,6 +777,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..47cf1ff24 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -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; 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); } } From f5511d93b60ab0e1803fca825c97b079d56d2462 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:12:03 -0500 Subject: [PATCH 15/38] Compare: index slide, shape and media lookups and compare each picture pair once (#1752) * engine: index review.compare lookups and compare each picture pair once * engine: keep the compare test's inserted slides empty --- crates/engine/src/compare.rs | 130 ++++++++++++++++++++++++++++++----- 1 file changed, 112 insertions(+), 18 deletions(-) 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 { From 3c3d504eb15243a6517d1a1d8b841082a2edef9b Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:12:24 -0500 Subject: [PATCH 16/38] PPTX: read a notes or comments part for the first slide that names it (#1741) * pptx: read a notes or comments part for the first slide that names it * address review: keep modern-comments precedence for already-owned parts --- crates/pptx/src/read.rs | 37 +++++++++++++++++-------------- crates/pptx/tests/it/comments.rs | 27 ++++++++++++++++++++++ crates/pptx/tests/it/malformed.rs | 31 ++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 17 deletions(-) diff --git a/crates/pptx/src/read.rs b/crates/pptx/src/read.rs index 2056d53d7..4ee3c4d32 100644 --- a/crates/pptx/src/read.rs +++ b/crates/pptx/src/read.rs @@ -668,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)); 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"); +} From 996bb42ff51a5f0d513fde4bd3a25512474009e5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:12:47 -0500 Subject: [PATCH 17/38] pptx: index relationships by id on read and by target on write (#1739) --- crates/pptx/src/opc.rs | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) 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)]); From f5fbadde63b4f34efd292932d4ecbbe8b54fdae8 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:38:10 -0500 Subject: [PATCH 18/38] Render: evict the least recently used animation instead of clearing the GIF cache (#1802) --- crates/render/src/images.rs | 50 ++++++++++++++++++++++++++++++++----- 1 file changed, 44 insertions(+), 6 deletions(-) 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); From 2dfb07f2c478ba1c09922aadee74c1cecb53dcdb Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:38:31 -0500 Subject: [PATCH 19/38] Thumbnails: index section starts once (#1800) --- crates/ui-egui/src/panels/thumbs.rs | 40 ++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) 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 From 9d59fa96d3651c9404f1c11c11e47df1fc3062cd Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:38:53 -0500 Subject: [PATCH 20/38] Text, morph: share a run's gradient between glyph runs, match morph tokens by text (#1784) --- crates/render/src/morph_text.rs | 32 ++++++++++++++++++++++---------- crates/text/src/run.rs | 2 +- crates/text/src/style.rs | 4 ++-- 3 files changed, 25 insertions(+), 13 deletions(-) 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/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), From a790fdbc2080e524b2ea5f7e18eda4cc3e787598 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:39:14 -0500 Subject: [PATCH 21/38] Render: find a glyph's line by bisection for gradient and WordArt text (#1783) --- crates/render/src/text_draw.rs | 112 ++++++++++++++++++++++++++++++++- crates/render/src/warp.rs | 10 +-- 2 files changed, 115 insertions(+), 7 deletions(-) diff --git a/crates/render/src/text_draw.rs b/crates/render/src/text_draw.rs index 3fe96d694..d2400f19f 100644 --- a/crates/render/src/text_draw.rs +++ b/crates/render/src/text_draw.rs @@ -105,6 +105,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 +145,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 +192,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 +389,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() { From 69c91bb2651b80a71b4a3bba55ccba17fd4e9485 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:39:36 -0500 Subject: [PATCH 22/38] Render: index table style, skip list, chart point fills and waterfall bars; count each dash (#1774) * render: look up a table's style once, keep the skip list as a set, count each dash * render: index chart point fills, scatter x presence and waterfall bars once per chart --- crates/render/src/chart.rs | 33 +++++++++++++++++++++++++-------- crates/render/src/placed.rs | 4 +++- crates/render/src/renderer.rs | 28 +++++++++++++++++++++++++++- crates/render/src/stroke.rs | 3 ++- crates/render/src/tests_wild.rs | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 11 deletions(-) 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/placed.rs b/crates/render/src/placed.rs index 4dc9e1082..2af8152be 100644 --- a/crates/render/src/placed.rs +++ b/crates/render/src/placed.rs @@ -131,9 +131,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); } 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_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); } From 23ac4f7efdbd1e538bd9a7869584d4adeb9039b0 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 20:39:57 -0500 Subject: [PATCH 23/38] Fonts: bound font-controlled work in enumeration, outlines and carets (#1701) * Fonts: bound font-controlled work in enumeration, outlines and carets An embedded font could make each lookup do far more than real fonts need: - enumerating a collection read up to 256 faces times 1024 named instances, each then built with its own shaper; now 4096 per file across all faces, with a set for the duplicate-style check instead of a scan; - composite and VARC glyphs could expand one dense outline without limit, and the outline cache only counted entries; outlines now stop at 32768 path elements (drawing nothing past it) and the cache is also bounded by the elements it holds; - the 64-caret limit applied after filtering, so a ligature lookup could read every caret record; it now applies to the records read. * address review: bound thread-local glyph cache by path elements --- crates/fonts/src/fallback.rs | 7 +- crates/fonts/src/fontdb.rs | 159 +++++++++++++++++++++++++++++---- crates/render/src/text_draw.rs | 18 ++-- 3 files changed, 161 insertions(+), 23 deletions(-) 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..c1db0b356 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, @@ -1513,41 +1558,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 +1651,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/render/src/text_draw.rs b/crates/render/src/text_draw.rs index d2400f19f..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 } From 50187449c20d58d8b014a3eb04b967ef3cc687b2 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Wed, 4 Nov 2026 07:52:46 +0100 Subject: [PATCH 24/38] Render tests: import place_slide_with so the date test compiles (#1153) --- crates/render/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index 05306f66c..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; From 51d19baa0cbe35f259bf5e24e38f1f59b2b2ef2c Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 13:56:48 -0500 Subject: [PATCH 25/38] Ogg, xtask: check packet ranges against the source first; stats skips directory links (#1796) --- crates/ogg/src/lib.rs | 5 +++++ crates/ogg/src/tests.rs | 10 ++++++++++ xtask/src/stats.rs | 8 +++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/crates/ogg/src/lib.rs b/crates/ogg/src/lib.rs index 77c081d52..719c96698 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(); diff --git a/crates/ogg/src/tests.rs b/crates/ogg/src/tests.rs index 480c1cda2..8f9673089 100644 --- a/crates/ogg/src/tests.rs +++ b/crates/ogg/src/tests.rs @@ -274,3 +274,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/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); From 07dc2210a1554b8186624e88b383bcc8bc0c9b19 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 14:09:46 -0500 Subject: [PATCH 26/38] Repair: bound the notes page size like a slide (#1832) --- crates/format/src/lib.rs | 5 +++++ crates/format/src/tests.rs | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 6f345e11c..97caafdb3 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); } 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. From 80f784d21011dc71bfe20abe0bbc144e1713fae9 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 14:36:13 -0500 Subject: [PATCH 27/38] Fonts: ignore variation tags the font has no axis for (#1795) --- crates/fonts/src/fontdb.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs index c1db0b356..19f190445 100644 --- a/crates/fonts/src/fontdb.rs +++ b/crates/fonts/src/fontdb.rs @@ -1260,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(':')?; @@ -1272,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)), From 9d36b5931cf93b36605eb5c8173260a2a346ccf5 Mon Sep 17 00:00:00 2001 From: Mex <44814932+MMexicann@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:15:59 +0100 Subject: [PATCH 28/38] Ogg: include recovered tail packets in Opus duration (#1862) * Ogg: include recovered tail packets in Opus duration Keep the existing missing-granule recovery cursor in step with the packet ends it assigns. Add a regression for a partially and entirely unanchored stream; existing valid end-trimming coverage remains unchanged. Validation: rustc --edition=2024 --test crates/ogg/src/lib.rs: all 12 tests pass. Scoped rustfmt check passes. Full cargo --offline xtask ci is blocked before build by the uncached pinned egui checkout. * Ogg: record recovery duration correction --- ROADMAP.md | 4 ++++ crates/ogg/src/lib.rs | 1 + crates/ogg/src/tests.rs | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/ROADMAP.md b/ROADMAP.md index 9315be8a6..37ef74b55 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,6 +2,8 @@ **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 (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) @@ -150,6 +152,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-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 | | 2026-11-02 | Picture compression measures displayed dimensions through nested group transforms; checklist score unchanged | @@ -330,6 +333,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-02 | trivial | File › About replaces Account; product links, credits and appearance retained | diff --git a/crates/ogg/src/lib.rs b/crates/ogg/src/lib.rs index 719c96698..0261ee0e6 100644 --- a/crates/ogg/src/lib.rs +++ b/crates/ogg/src/lib.rs @@ -513,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 8f9673089..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 { From 65ddd8ad517e15032195059c73f54c690e5d2dbf Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Wed, 4 Nov 2026 20:55:43 +0000 Subject: [PATCH 29/38] Apply review fixes - crates/render/src/placed.rs --- crates/render/src/placed.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/render/src/placed.rs b/crates/render/src/placed.rs index bde0fa157..483db41bc 100644 --- a/crates/render/src/placed.rs +++ b/crates/render/src/placed.rs @@ -160,16 +160,6 @@ 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); - // 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(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); - let (x0, x1) = line.span(run.chars.0, run.chars.1).unwrap_or((first, first)); - 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 }); - } // 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(); @@ -185,6 +175,16 @@ fn place_text(out: &mut Placed, rctx: &Ctx, fields: &SlideFields, owner: ShapeId } } } + // 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(&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); + let (x0, x1) = line.span(run.chars.0, run.chars.1).unwrap_or((first, first)); + 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: placed }); } From 31abd2d819094b7e3b4a70746e7f804f4695ab0d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 4 Nov 2026 17:45:40 -0500 Subject: [PATCH 30/38] Pass the Linux signing key to the release workflow (#1868) --- .github/workflows/release.yml | 1 + 1 file changed, 1 insertion(+) 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 }} From fc18a78e65bc7ef125ee65c0a3279bc0793b7eb5 Mon Sep 17 00:00:00 2001 From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:09:50 -0600 Subject: [PATCH 31/38] comment: reject malformed navigation indices (#1547) --- crates/engine/src/cmd/review.rs | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 47cf1ff24..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")); @@ -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"); From 85de6ca5d76356ebaf3367ad319bd2fdbb0c0155 Mon Sep 17 00:00:00 2001 From: ".CooCoo" Date: Wed, 4 Nov 2026 15:11:29 -0800 Subject: [PATCH 32/38] Shape Fill: the gradient help says which kinds rotateWithShape keeps upright (#1843) With rotateWithShape false, the renderer lays linear and radial gradients out upright on the slide, while rectangular and path gradients still turn with the shape. The shape.fill params doc now says so. --- crates/engine/src/cmd/shape.rs | 2 +- crates/engine/src/cmd/shape/appearance.rs | 3 ++- crates/engine/src/tests.rs | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) 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/tests.rs b/crates/engine/src/tests.rs index e37f9d135..12fac5efc 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!({ From f321416b34ded8c85b9d8e6a6fec822af212aa48 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 5 Nov 2026 00:52:05 +0100 Subject: [PATCH 33/38] Make Previous/Next work in the web presenter popup Fixes #1864 Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/show/tests.rs | 6 ++++++ crates/ui-egui/src/show/ui.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index 1dd6b09ef..b948efde2 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1173,6 +1173,12 @@ 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}"); + } 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..226459755 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -94,7 +94,7 @@ 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; } From f49ff09b8ac7282598f4c0cfe8b75329b59304ce Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 02:25:54 +0100 Subject: [PATCH 34/38] Show: All slides opens in the web Presenter View, as on desktop (#1866) --- apps/deckcraft-web/src/presenter.rs | 3 + crates/ui-egui/src/presenter.rs | 100 ++++++++++++++++++++++++++-- crates/ui-egui/src/show.rs | 3 + crates/ui-egui/src/show/tests.rs | 22 +++++- crates/ui-egui/src/show/ui.rs | 29 +++++++- 5 files changed, 149 insertions(+), 8 deletions(-) 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/ui-egui/src/presenter.rs b/crates/ui-egui/src/presenter.rs index 95569778d..0ad7929bc 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. @@ -200,6 +228,11 @@ impl View { } }); ui.separator(); + if snapshot.grid && !snapshot.ended { + // 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 +260,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 +290,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 > slides.len() && !slides.is_empty() { + ui.label(format!("First {} slides", slides.len())); + } + 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 +349,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 +486,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 +506,27 @@ 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 }]); + } + #[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..c1e3f4ab2 100644 --- a/crates/ui-egui/src/show.rs +++ b/crates/ui-egui/src/show.rs @@ -65,6 +65,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 +239,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 b948efde2..01b30ff01 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1132,6 +1132,7 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { opens: usize, closes: usize, commands: Vec, + last: Option, } struct Mock(Rc>); impl Host for Mock { @@ -1145,7 +1146,9 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { 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; } @@ -1179,6 +1182,23 @@ 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, "{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"); + 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); 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 226459755..688d2bb6e 100644 --- a/crates/ui-egui/src/show/ui.rs +++ b/crates/ui-egui/src/show/ui.rs @@ -96,9 +96,18 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { crate::presenter::Command::Key { name, command, shift, alt, ctrl } => { 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 +152,21 @@ 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) { + let size = (160, (160.0 * doc.slide_size.height / doc.slide_size.width.max(1.0)).clamp(1.0, 640.0) as u32); + let image = deckcraft_render::render_slide( + &doc, + i, + &RenderOpts { scale: size.0 as f64 / doc.slide_size.width.max(1.0), 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 +176,11 @@ 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()), + slides: show.presenter_grid.then(|| show.remote_grid.clone()), }); show.remote_sent = now; } From 884e3d3353649870127280eb8bf0bd9c40c5cf48 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 02:37:37 +0100 Subject: [PATCH 35/38] Show: keep web All slides messages small, clear the grid when the popup closes, show it on the end screen (#1866) --- crates/ui-egui/src/presenter.rs | 6 +++--- crates/ui-egui/src/show.rs | 1 + crates/ui-egui/src/show/tests.rs | 13 ++++++++++++- crates/ui-egui/src/show/ui.rs | 23 +++++++++++++++-------- crates/ui-egui/src/show/zoom.rs | 2 +- 5 files changed, 32 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/presenter.rs b/crates/ui-egui/src/presenter.rs index 0ad7929bc..5d99e63c8 100644 --- a/crates/ui-egui/src/presenter.rs +++ b/crates/ui-egui/src/presenter.rs @@ -228,7 +228,7 @@ impl View { } }); ui.separator(); - if snapshot.grid && !snapshot.ended { + 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; @@ -303,8 +303,8 @@ impl View { } ui.heading("All slides"); ui.label("Choose a slide to show it, or press G to return."); - if snapshot.total > slides.len() && !slides.is_empty() { - ui.label(format!("First {} slides", slides.len())); + 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| { diff --git a/crates/ui-egui/src/show.rs b/crates/ui-egui/src/show.rs index c1e3f4ab2..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}; diff --git a/crates/ui-egui/src/show/tests.rs b/crates/ui-egui/src/show/tests.rs index 01b30ff01..a01989ac0 100644 --- a/crates/ui-egui/src/show/tests.rs +++ b/crates/ui-egui/src/show/tests.rs @@ -1133,6 +1133,7 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { closes: usize, commands: Vec, last: Option, + gone: bool, } struct Mock(Rc>); impl Host for Mock { @@ -1141,7 +1142,7 @@ 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) @@ -1194,11 +1195,21 @@ fn remote_presenter_uses_the_audience_state_and_closes_with_the_show() { 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 688d2bb6e..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() { @@ -157,12 +161,11 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { } 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) { - let size = (160, (160.0 * doc.slide_size.height / doc.slide_size.width.max(1.0)).clamp(1.0, 640.0) as u32); - let image = deckcraft_render::render_slide( - &doc, - i, - &RenderOpts { scale: size.0 as f64 / doc.slide_size.width.max(1.0), size: Some(size), ..Default::default() }, - ); + // 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] })); } @@ -178,8 +181,9 @@ pub fn ui(app: &mut SlideApp, ui: &mut Ui) { blank: show.blank_name().map(str::to_owned), 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; @@ -189,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; From efd26de81d0b27d091293d5ad89b2a0f175ac5a4 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 02:48:01 +0100 Subject: [PATCH 36/38] Show: clear the web presenter grid in hidden logic, let Enter and Space pick a focused thumbnail (#1866) --- crates/ui-egui/src/app/playback.rs | 4 ++++ crates/ui-egui/src/presenter.rs | 7 +++++++ 2 files changed, 11 insertions(+) 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/presenter.rs b/crates/ui-egui/src/presenter.rs index 5d99e63c8..a58878830 100644 --- a/crates/ui-egui/src/presenter.rs +++ b/crates/ui-egui/src/presenter.rs @@ -175,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 { @@ -525,6 +529,9 @@ mod tests { 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] From ede154d6be565d47b4b9c608103ed88896569ee7 Mon Sep 17 00:00:00 2001 From: Zach Berry <1112351+zberry@users.noreply.github.com> Date: Sun, 18 Oct 2026 23:07:03 -0500 Subject: [PATCH 37/38] Outlines leave footer, date and slide-number placeholders out: export, RTF and the outline view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An agent run exporting a deck as an outline got the footer text and the slide-number field (‹#›) as body lines under each title. PowerPoint's outline view shows title and body placeholders only, so the text outline, the RTF outline (Save As ▸ Outline/RTF) and the outline view now skip the footer-row placeholders as they already skipped the title. Split out of the frame export PR at review, which asked for all three together. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/tests.rs | 32 +++++++++++++++++++++++++++ crates/format/src/lib.rs | 4 +++- crates/format/src/rtf.rs | 3 ++- crates/ui-egui/src/sorter.rs | 5 +++-- crates/ui-egui/src/sorter/ui_tests.rs | 21 ++++++++++++++++++ 5 files changed, 61 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 12fac5efc..6741f03d5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6509,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/format/src/lib.rs b/crates/format/src/lib.rs index 97caafdb3..4125c2905 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -694,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 b546a832f..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 }; 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); +} From a2d9e71b5e71bfbe2e96499b6dce304118153fe2 Mon Sep 17 00:00:00 2001 From: FredApps <153173854+FredApps@users.noreply.github.com> Date: Thu, 5 Nov 2026 04:33:51 +0100 Subject: [PATCH 38/38] About: credits tables fit without side-scrolling and their columns resize (#1806) * About: credits tables fit without side-scrolling and their columns resize The Contributors table and the Models table were egui Grids inside a two-way scroll area: they opened wider than the dialog, so they scrolled sideways, and Company and Model sat almost touching. Both are now egui_extras tables. Each column opens at the width of its header and widest cell, the spare width goes to the text columns (Name; Company and Model), and every column edge can be dragged. Widening a column grows the table like a spreadsheet, and only then does it scroll sideways; the header stays put while the rows scroll. Counts and dates are right-aligned and every cell is vertically centred. The sorted column stays highlighted and its direction shows on the toolbar's sort-order button, so headers no longer reserve room for an arrow. The opening widths come from fit_widths, a pure function with unit tests that a fitted table fills its room exactly and that only the text columns give way when room is short. The About dialog opens 820 points wide, so every column fits at its natural width. Co-Authored-By: Claude Opus 5.5 * Let every credits column be dragged, and refit changed tables The last column was egui's remainder column, which has no drag handle, so it could only be resized from the edge before it. It is now an ordinary column with its own handle. A table keeps its column widths, so switching to a longer name left the Name column at its old width and clipped the text. The table now goes back to the opening fit whenever that fit changes: a longer widest cell or a resized window. Dragged widths are dropped at that point. fit_widths could overflow the room when one column hit the minimum, because the other columns did not take up its share. Columns now give way in proportion to their width above the minimum. Co-Authored-By: Claude Opus 5.5 * Refit credits tables on what they measure, not the fitted widths Comparing the fitted widths missed the case it was meant for. Name is the only growing column, so a longer name takes its extra width from the spare room and the fit comes out the same: a dragged Name width stayed, clipping the longer name. It could also drop dragged widths by accident. The fit comes from the available width inside the scroll area, which scrolling can nudge, so scrolling a widened table could reset it. The table now compares the natural widths and the clip rect's width, which scrolling does not move. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- Cargo.lock | 41 ++- crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/src/dialogs/about.rs | 2 +- crates/ui-egui/src/dialogs/credits.rs | 371 ++++++++++++++++++++++---- 4 files changed, 357 insertions(+), 58 deletions(-) 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/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/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:?}"); + } + } }