diff --git a/Cargo.lock b/Cargo.lock index 11cb01f13..862599989 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1028,7 +1028,7 @@ checksum = "0c87e182de0887fd5361989c677c4e8f5000cd9491d6d563161a8f3a5519fc7f" [[package]] name = "deckcraft" -version = "0.17.0" +version = "0.18.0" dependencies = [ "arboard", "chrono", @@ -1050,7 +1050,7 @@ dependencies = [ [[package]] name = "deckcraft-anim" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-geom", "deckcraft-model", @@ -1059,7 +1059,7 @@ dependencies = [ [[package]] name = "deckcraft-av1" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1068,14 +1068,14 @@ dependencies = [ [[package]] name = "deckcraft-bitstream" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-cli" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-engine", "deckcraft-mcp", @@ -1085,14 +1085,14 @@ dependencies = [ [[package]] name = "deckcraft-color" -version = "0.17.0" +version = "0.18.0" dependencies = [ "serde", ] [[package]] name = "deckcraft-engine" -version = "0.17.0" +version = "0.18.0" dependencies = [ "chrono", "deckcraft-anim", @@ -1118,7 +1118,7 @@ dependencies = [ [[package]] name = "deckcraft-fonts" -version = "0.17.0" +version = "0.18.0" dependencies = [ "epaint_default_fonts", "harfrust 0.13.3", @@ -1130,7 +1130,7 @@ dependencies = [ [[package]] name = "deckcraft-format" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-model", "serde", @@ -1141,7 +1141,7 @@ dependencies = [ [[package]] name = "deckcraft-geom" -version = "0.17.0" +version = "0.18.0" dependencies = [ "kurbo", "serde", @@ -1149,7 +1149,7 @@ dependencies = [ [[package]] name = "deckcraft-h264" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1158,7 +1158,7 @@ dependencies = [ [[package]] name = "deckcraft-hevc" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1167,21 +1167,21 @@ dependencies = [ [[package]] name = "deckcraft-isobmff" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-matroska" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-mcp" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-engine", "deckcraft-render", @@ -1191,7 +1191,7 @@ dependencies = [ [[package]] name = "deckcraft-media" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-av1", "deckcraft-h264", @@ -1209,7 +1209,7 @@ dependencies = [ [[package]] name = "deckcraft-metafile" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo", @@ -1218,7 +1218,7 @@ dependencies = [ [[package]] name = "deckcraft-model" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-color", "deckcraft-geom", @@ -1230,15 +1230,15 @@ dependencies = [ [[package]] name = "deckcraft-ogg" -version = "0.17.0" +version = "0.18.0" [[package]] name = "deckcraft-opus" -version = "0.17.0" +version = "0.18.0" [[package]] name = "deckcraft-pdf" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-pdf", "deckcraft-fonts", @@ -1256,7 +1256,7 @@ dependencies = [ [[package]] name = "deckcraft-pptx" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-color", "deckcraft-engine", @@ -1272,7 +1272,7 @@ dependencies = [ [[package]] name = "deckcraft-render" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-anim", "deckcraft-color", @@ -1291,7 +1291,7 @@ dependencies = [ [[package]] name = "deckcraft-text" -version = "0.17.0" +version = "0.18.0" dependencies = [ "chrono", "deckcraft-color", @@ -1305,7 +1305,7 @@ dependencies = [ [[package]] name = "deckcraft-ui-egui" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-ui", "craft-update", @@ -1331,7 +1331,7 @@ dependencies = [ [[package]] name = "deckcraft-vp9" -version = "0.17.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1340,7 +1340,7 @@ dependencies = [ [[package]] name = "deckcraft-web" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-web-api", "craft-web-host", @@ -6325,7 +6325,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee" [[package]] name = "xtask" -version = "0.17.0" +version = "0.18.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 3ddc9f06a..c1e005125 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/deckcraft", "apps/deckcraft-cli"] [workspace.package] -version = "0.17.0" +version = "0.18.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" diff --git a/apps/deckcraft/src/logging.rs b/apps/deckcraft/src/logging.rs index f2cbae0d2..fed5d503e 100644 --- a/apps/deckcraft/src/logging.rs +++ b/apps/deckcraft/src/logging.rs @@ -148,7 +148,19 @@ pub fn rotate(dir: &Path) -> std::io::Result<(PathBuf, File)> { /// One formatted log line: ` [] : \n`. pub fn format_line(ts: &str, level: log::Level, thread: &str, target: &str, message: &str) -> String { let message = message.strip_suffix('\n').unwrap_or(message); - format!("{ts} {level:<5} [{thread}] {target}: {message}\n") + // Messages can embed document-controlled text (a part name, a relationship id): control + // characters other than tab are escaped and each continuation line is marked, so such text + // cannot start a line that looks like a record of its own. + let mut text = String::with_capacity(message.len()); + for c in message.chars() { + match c { + '\n' => text.push_str("\n | "), + '\t' => text.push(c), + c if c.is_control() => text.extend(c.escape_default()), + c => text.push(c), + } + } + format!("{ts} {level:<5} [{thread}] {target}: {text}\n") } /// Where formatted lines go besides standard error: kept in memory until a file is attached, @@ -446,8 +458,13 @@ mod tests { ), "2026-10-08T07:59:17.728Z WARN [main] deckcraft_pptx::opc: pptx: skipping oversized part ppt/media/image1.png\n" ); - // A multi-line message keeps its lines; the record still ends in exactly one newline. - assert!(format_line("t", log::Level::Error, "w", "x", "a\nb\n").ends_with("x: a\nb\n")); + // A multi-line message keeps its lines, each continuation marked; the record still ends in + // exactly one newline. + assert!(format_line("t", log::Level::Error, "w", "x", "a\nb\n").ends_with("x: a\n | b\n")); + // Injected line breaks and escape sequences cannot forge a record. + let line = format_line("t", log::Level::Warn, "w", "x", "part a\r\n2026 ERROR [m] y: forged\x1b[2J"); + assert_eq!(line.matches('\n').count(), 2); + assert!(!line.contains('\r') && !line.contains('\x1b')); } #[test] diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 6f20c4143..e3a50f689 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -466,22 +466,26 @@ fn ctx_xfrm(doc: &deckcraft_model::Presentation, sel: &crate::Selection, shape: /// Run `f` on each target shape (copy-on-write), materialising inherited boxes first. pub(crate) fn edit_shapes(s: &mut Session, p: &Value, cmd: &str, f: impl Fn(&mut deckcraft_model::Shape) -> Result<()>) -> Result { - let ids = targets(s, p)?; + let mut ids = targets(s, p)?; if ids.is_empty() { return Err(bad(cmd, "no shapes selected")); } + // A shape named twice is edited once. + let mut seen = std::collections::HashSet::new(); + ids.retain(|id| seen.insert(*id)); s.edit(|doc, sel| { - let boxes: Vec<(ShapeId, Xfrm)> = { - let st_shapes: Vec = - ids.iter().filter_map(|id| current_shapes(doc, sel).and_then(|v| deckcraft_model::find_shape(v, *id)).cloned()).collect(); - st_shapes.iter().map(|sh| (sh.id, xfrm_of(doc, sel, sh))).collect() - }; + // The boxes are read from the shapes in place: no copy of a shape (and its text) per id. + let boxes: std::collections::HashMap = ids + .iter() + .filter_map(|id| current_shapes(doc, sel).and_then(|v| deckcraft_model::find_shape(v, *id))) + .map(|sh| (sh.id, xfrm_of(doc, sel, sh))) + .collect(); let shapes = crate::shapes_mut(doc, sel).ok_or_else(|| bad(cmd, "no slide"))?; let mut n = 0; for id in &ids { if let Some(sh) = deckcraft_model::find_shape_mut(shapes, *id) { if sh.xfrm.is_none() { - sh.xfrm = boxes.iter().find(|(i, _)| i == id).map(|(_, x)| *x); + sh.xfrm = boxes.get(id).copied(); } f(sh)?; n += 1; diff --git a/crates/engine/src/cmd/selection_tests.rs b/crates/engine/src/cmd/selection_tests.rs index e4e2494b4..07bed2d97 100644 --- a/crates/engine/src/cmd/selection_tests.rs +++ b/crates/engine/src/cmd/selection_tests.rs @@ -135,6 +135,14 @@ fn eye_show_all_and_hide_all() { assert!(!flags(&s, a).0 && flags(&s, g).0 && flags(&s, e).0); } +#[test] +fn lock_group_with_its_child_targeted_leaves_child_locked() { + let (mut s, ids) = deck(); + let (d, g) = (ids[3], ids[7]); + s.execute("shape.lock", &json!({"ids": [g, d, d]})).unwrap(); + assert!(flags(&s, g).1 && flags(&s, d).1); +} + #[test] fn lock_cascades_through_groups_and_undoes() { let (mut s, ids) = deck(); diff --git a/crates/engine/src/cmd/shape.rs b/crates/engine/src/cmd/shape.rs index 78b6204bb..b7b3d3f05 100644 --- a/crates/engine/src/cmd/shape.rs +++ b/crates/engine/src/cmd/shape.rs @@ -374,6 +374,25 @@ fn visible(s: &mut Session, p: &Value) -> Result { /// (a child can still be unlocked on its own afterwards), as PowerPoint does. fn lock(s: &mut Session, p: &Value) -> Result { let v = bool_param(p, "locked"); + // A targeted shape inside another targeted group is reached through the group: editing it again + // would toggle it back. + let mut ids = super::targets(s, p)?; + if ids.len() > 1 { + let doc = s.doc()?; + // Walk each distinct group once, however often `ids` repeats it. + let mut once = std::collections::HashSet::new(); + ids.retain(|id| once.insert(*id)); + let mut inside = std::collections::HashSet::new(); + for id in &ids { + if let Some(g) = doc.shape(*id) { + deckcraft_model::walk(g.children(), &mut |c, _| { + inside.insert(c.id); + }); + } + } + ids.retain(|id| !inside.contains(id)); + } + let p = &json!({"ids": ids, "locked": p.get("locked")}); edit_shapes(s, p, "shape.lock", |sh| { let to = v.unwrap_or(!sh.locked); set_locked(sh, to, 0); diff --git a/crates/engine/src/cmd/shape/freeform.rs b/crates/engine/src/cmd/shape/freeform.rs index 5fd299c9e..f6886c75f 100644 --- a/crates/engine/src/cmd/shape/freeform.rs +++ b/crates/engine/src/cmd/shape/freeform.rs @@ -104,7 +104,7 @@ pub(super) fn freeform(s: &mut Session, p: &Value) -> Result { a.iter() .filter_map(|v| Some(deckcraft_geom::Point::new(v.get(0)?.as_f64()?, v.get(1)?.as_f64()?))) .filter(|q| q.x.is_finite() && q.y.is_finite()) - .take(20_000) + .take(crate::tools::MAX_GESTURE_POINTS) .collect() }) .unwrap_or_default(); diff --git a/crates/engine/src/fonts.rs b/crates/engine/src/fonts.rs index e208f72c7..3d99d3f57 100644 --- a/crates/engine/src/fonts.rs +++ b/crates/engine/src/fonts.rs @@ -1,6 +1,8 @@ //! Fonts a presentation uses that this machine doesn't have, so opening it can say so (its text //! draws in a stand-in; see `FontDb::document_face`). +use std::collections::HashSet; + use deckcraft_fonts::FontDb; use deckcraft_model::resolve::{self, Ctx}; use deckcraft_model::text::{Bullet, TextBody}; @@ -12,8 +14,13 @@ use deckcraft_model::{Presentation, Shape, ShapeKind}; /// font to load and are left for the round trip. pub fn install_embedded_fonts(p: &Presentation) { let db = FontDb::global(); + // Faces can name one media item many times; each is copied and enumerated once. + let mut seen = HashSet::new(); for font in &p.embedded_fonts { for id in font.faces().into_iter().flatten() { + if !seen.insert(id) { + continue; + } let Some(item) = p.media(id) else { continue }; let Some(sfnt) = deckcraft_fonts::embedded_sfnt(&item.data) else { continue }; db.add_font(sfnt.to_vec()); @@ -27,12 +34,8 @@ pub fn install_embedded_fonts(p: &Presentation) { /// and can't be installed, so they aren't reported. pub fn missing_fonts(p: &Presentation) -> Vec { let db = FontDb::global(); - let mut out: Vec = Vec::new(); - for family in used_fonts(p) { - if !family.starts_with('.') && !db.has_family(&family) && !out.iter().any(|f| f.eq_ignore_ascii_case(&family)) { - out.push(family); - } - } + // used_fonts is already free of case-insensitive duplicates. + let mut out: Vec = used_fonts(p).into_iter().filter(|f| !f.starts_with('.') && !db.has_family(f)).collect(); out.sort_by_key(|f| f.to_lowercase()); out } @@ -40,9 +43,10 @@ pub fn missing_fonts(p: &Presentation) -> Vec { /// The families the slides' text runs and bullets are drawn in. pub fn used_fonts(p: &Presentation) -> Vec { let mut out: Vec = Vec::new(); + let mut seen: HashSet = HashSet::new(); let mut add = |f: String| { let f = f.trim().to_string(); - if !f.is_empty() && !out.iter().any(|o| o.eq_ignore_ascii_case(&f)) { + if !f.is_empty() && seen.insert(f.to_ascii_lowercase()) { out.push(f); } }; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index d8ebb5a23..754a64f1e 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -824,10 +824,32 @@ fn clamp_selection(st: &mut DocState) { /// Default for [`History::byte_limit`]. pub const UNDO_BYTE_BUDGET: usize = 256 << 20; +/// A rough size of a text body in memory: its paragraphs and runs, at their inline struct sizes. +fn text_cost(t: &deckcraft_model::text::TextBody) -> usize { + use deckcraft_model::text::{Paragraph, Run}; + let (para, run) = (std::mem::size_of::(), std::mem::size_of::()); + t.paragraphs.iter().map(|p| para + p.runs.iter().map(|r| run + r.text.len()).sum::()).sum() +} + +/// A rough size of comments in memory, replies included. +fn comments_cost(cs: &[deckcraft_model::Comment], depth: usize) -> usize { + let kids = |c: &deckcraft_model::Comment| if depth < 64 { comments_cost(&c.replies, depth + 1) } else { 0 }; + cs.iter().map(|c| 256 + c.author.len() + c.initials.len() + c.text.len() + c.date.len() + kids(c)).sum() +} + +/// A rough size of a slide's own data besides its shapes: notes, comments, tags and kept XML, +/// all copied with the slide when an edit changes it. +fn slide_extra_cost(s: &deckcraft_model::Slide) -> usize { + text_cost(&s.notes) + + comments_cost(&s.comments, 0) + + s.name.len() + + s.tags.iter().map(|(k, v)| 48 + k.len() + v.len()).sum::() + + s.raw_ext.as_ref().map_or(0, String::len) +} + /// A rough size of a shape in memory: the struct, its text and kept XML, its children. fn shape_cost(sh: &deckcraft_model::Shape, depth: usize) -> usize { - let text: usize = - sh.text.as_ref().map_or(0, |t| t.paragraphs.iter().map(|p| 96 + p.runs.iter().map(|r| 128 + r.text.len()).sum::()).sum()); + let text: usize = sh.text.as_ref().map_or(0, text_cost); let kids: usize = if depth < 64 { sh.children().iter().map(|c| shape_cost(c, depth + 1)).sum() } else { 0 }; 768 + text + sh.raw_ext.as_ref().map_or(0, String::len) + kids } @@ -845,7 +867,7 @@ pub fn undo_cost(doc: &Presentation, newer: &Presentation) -> usize { let media: HashSet<*const Vec> = newer.media.iter().map(|m| Arc::as_ptr(&m.data)).collect(); let mut n = 64 * doc.slides.len(); for s in doc.slides.iter().filter(|s| !slides.contains(&Arc::as_ptr(s))) { - n += 512 + shapes_cost(&s.shapes); + n += 512 + shapes_cost(&s.shapes) + slide_extra_cost(s); } for m in doc.masters.iter().filter(|m| !masters.contains(&Arc::as_ptr(m))) { n += 4096 + shapes_cost(&m.shapes) + m.layouts.iter().map(|l| 512 + shapes_cost(&l.shapes)).sum::(); diff --git a/crates/engine/src/spell.rs b/crates/engine/src/spell.rs index 0eeaa6757..3323151cb 100644 --- a/crates/engine/src/spell.rs +++ b/crates/engine/src/spell.rs @@ -171,7 +171,9 @@ impl Dictionary { ["COMPOUNDWORDMAX", n, ..] => d.compound_max = n.parse().unwrap_or(0), ["REP", n] => { for _ in 0..n.parse::().unwrap_or(0) { - let r: Vec<&str> = lines.next().unwrap_or("").split_whitespace().collect(); + // The count is the file's claim: stop when the records run out. + let Some(line) = lines.next() else { break }; + let r: Vec<&str> = line.split_whitespace().collect(); if let [_, a, b, ..] = r.as_slice() { d.rep.push((a.replace('_', " "), b.replace('_', " "))); } @@ -182,7 +184,8 @@ impl Dictionary { let Some(flag) = one_flag(Some(flag), mode) else { continue }; let cross = cross.eq_ignore_ascii_case("y"); for _ in 0..n.parse::().unwrap_or(0) { - let r: Vec<&str> = lines.next().unwrap_or("").split_whitespace().collect(); + let Some(line) = lines.next() else { break }; + let r: Vec<&str> = line.split_whitespace().collect(); let [_, _, strip, add, rest @ ..] = r.as_slice() else { continue }; let (add, cont) = match add.split_once('/') { Some((a, c)) => (a, parse_flags(c, mode)), @@ -304,7 +307,12 @@ impl Dictionary { /// `w` made of dictionary parts allowed in compounds (`COMPOUNDFLAG`, or the begin, middle and /// end flags), each at least `COMPOUNDMIN` characters long. - fn compounded(&self, w: &[char], index: usize) -> bool { + fn compounded(&self, w: &[char], index: usize, failed: &mut HashSet<(usize, usize)>) -> bool { + // `w` is always a tail of the checked word, so its length names it: a (tail, part index) + // that failed once fails again however the earlier parts were split. + if failed.contains(&(w.len(), index)) { + return false; + } if self.compound.is_none() && self.compound_end.is_none() { return false; } @@ -322,10 +330,11 @@ impl Dictionary { if [self.compound, self.compound_end].into_iter().flatten().any(|f| self.compound_part(rest, f, true)) { return true; } - if self.compounded(rest, index + 1) { + if self.compounded(rest, index + 1, failed) { return true; } } + failed.insert((w.len(), index)); false } @@ -352,7 +361,7 @@ impl Dictionary { return true; } } - self.compounded(&w, 0) + self.compounded(&w, 0, &mut HashSet::new()) } /// Is `word` spelt correctly? Capitalised and upper-case forms of a lower-case word pass. @@ -576,6 +585,15 @@ fn cache() -> &'static Cache { CACHE.get_or_init(Default::default) } +/// Language strings remembered by [`dictionary`]; a string past it is resolved again on each call. +const MAX_CACHED_LANGS: usize = 256; + +/// Parsed dictionaries by their affix file, so aliases of one language share it. +fn by_files() -> &'static Mutex>> { + static M: OnceLock>>> = OnceLock::new(); + M.get_or_init(Default::default) +} + /// The dictionary for `lang` (a BCP-47 tag such as `en-US`), loaded once per process. pub fn dictionary(lang: &str) -> Option> { let cache = cache(); @@ -586,10 +604,23 @@ pub fn dictionary(lang: &str) -> Option> { return d.clone(); } let loaded = find_in(&search_dirs(), lang).and_then(|(a, d)| { - let (a, d) = (std::fs::read(a).ok()?, std::fs::read(d).ok()?); - Some(Arc::new(Dictionary::parse(&a, &d))) + // Tags that resolve to the same files share one parsed dictionary. + if let Ok(m) = by_files().lock() + && let Some(dict) = m.get(&a) + { + return Some(dict.clone()); + } + let (bytes_a, bytes_d) = (std::fs::read(&a).ok()?, std::fs::read(d).ok()?); + let dict = Arc::new(Dictionary::parse(&bytes_a, &bytes_d)); + // A concurrent loader of the same files may have inserted first: share its copy. + if let Ok(mut m) = by_files().lock() { + return Some(m.entry(a).or_insert(dict).clone()); + } + Some(dict) }); - if let Ok(mut c) = cache.lock() { + if let Ok(mut c) = cache.lock() + && c.len() < MAX_CACHED_LANGS + { c.insert(key, loaded.clone()); } loaded @@ -897,6 +928,14 @@ SFX D Y 1\nSFX D 0 ed/S .\n"; assert!(d.suggest("workingslides", 5).contains(&"working slides".to_string())); } + /// An affix header can claim billions of records; parsing stops where the file does. + #[test] + fn a_header_count_past_the_records_ends_with_them() { + let aff = b"SFX S Y 4000000000\nSFX S 0 s .\nREP 4000000000\nREP a b\n"; + let d = Dictionary::parse(aff, b"1\ncat/S\n".as_slice()); + assert!(d.check("cats")); + } + /// #1175: compound splitting is quadratic in the word's length, so a word longer than /// `MAX_CHECK_CHARS` is not checked; shorter ones still are. #[test] @@ -931,6 +970,17 @@ SFX D Y 1\nSFX D 0 ed/S .\n"; } } + #[test] + fn overlapping_compound_roots_with_a_failing_ending_stay_cheap() { + // Roots a..aaaaaaaa overlap, so many splits of "aaa…ab" reach the same (tail, part) state; unmemoized that is millions of searches. + let d = Dictionary::parse( + b"COMPOUNDFLAG X\nCOMPOUNDMIN 1\n".as_slice(), + b"8\na/X\naa/X\naaa/X\naaaa/X\naaaaa/X\naaaaaa/X\naaaaaaa/X\naaaaaaaa/X\n".as_slice(), + ); + assert!(!d.check(&format!("{}b", "a".repeat(99)))); + assert!(d.check(&"a".repeat(20))); + } + #[test] fn runs_give_words_their_language() { use deckcraft_model::{Paragraph, Run, RunProps}; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 5d2310328..4f40b2dfb 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3745,6 +3745,25 @@ fn undo_history_is_capped_by_memory_as_well_as_steps() { assert_eq!(s.doc().unwrap().history.undo.len(), 120); } +/// A slide's notes count toward the undo budget: small edits to a slide with large notes keep a +/// copy of those notes per step, and must not slip past the budget as if the slide were empty. +#[test] +fn undo_history_counts_slide_notes() { + let mut s = Session::with_new(); + let notes = "n".repeat(1 << 20); + s.edit(|doc, _| { + std::sync::Arc::make_mut(&mut doc.slides[0]).notes = deckcraft_model::text::TextBody::from_text(¬es); + Ok(()) + }) + .unwrap(); + s.doc_mut().unwrap().history = History { limit: 500, byte_limit: 8 << 20, ..Default::default() }; + for i in 0..40 { + s.execute("slide.rename", &json!({"name": format!("v{i}"), "index": 0})).unwrap(); + } + let n = s.doc().unwrap().history.undo.len(); + assert!((1..=8).contains(&n), "{n} steps of 1 MB notes kept under an 8 MB budget"); +} + #[test] fn picture_styles_replace_each_other_in_one_step() { const PNG: &[u8] = &[ @@ -5433,6 +5452,38 @@ fn pen_pressure_is_kept_and_a_mouse_draws_at_full_width() { assert_eq!(got, vec![vec![0.2, 1.0, 0.4], vec![1.0, 1.0]]); } +#[test] +fn a_long_ink_stroke_is_extended_in_place_and_capped() { + let mut s = session(); + s.set_tool(tools::ToolKind::Ink { mode: "pen".into(), color: deckcraft_model::Rgba::rgb(20, 20, 20), width: 4.0 }); + let ev = |kind, x: f64| tools::PointerEvent { kind, x, y: 80.0, mods: Mods::default(), tol: 3.0, pressure: Some(0.5) }; + s.pointer(ev(PointerKind::Down, 0.0)).unwrap(); + let n = tools::MAX_GESTURE_POINTS + 5_000; + for i in 1..n { + s.pointer(ev(PointerKind::Drag, (i % 500) as f64)).unwrap(); + } + assert_eq!(s.tool.ink_preview.len(), tools::MAX_GESTURE_POINTS); + s.pointer(ev(PointerKind::Up, 0.0)).unwrap(); + let kept: usize = ink_pressures(&s).iter().map(Vec::len).sum(); + assert!((2..=tools::MAX_GESTURE_POINTS).contains(&kept), "{kept}"); +} + +#[test] +fn double_clicking_a_capped_freeform_keeps_its_vertices() { + let mut s = session(); + let ev = |kind, x: f64, y: f64| tools::PointerEvent { kind, x, y, mods: Default::default(), tol: 3.0, pressure: None }; + s.set_tool(tools::ToolKind::Shape { preset: "freeform".into() }); + s.pointer(ev(tools::PointerKind::Down, 100.0, 100.0)).unwrap(); + // Densely clicked vertices, each within the double-click tolerance of the last. + for i in 0..tools::MAX_GESTURE_POINTS + 10 { + s.pointer(ev(tools::PointerKind::Down, 300.0 + i as f64 * 0.001, 100.0)).unwrap(); + } + s.pointer(ev(tools::PointerKind::DoubleClick, 300.0, 100.0)).unwrap(); + let st = s.doc().unwrap(); + let deckcraft_model::Geom::Custom { paths } = &st.shapes().last().unwrap().geom else { panic!("no freeform") }; + assert_eq!(paths[0].d.matches(" L ").count(), tools::MAX_GESTURE_POINTS - 1); +} + fn ink_pressures(s: &Session) -> Vec> { let st = s.doc().unwrap(); let Some(sh) = st.shapes().iter().rev().find(|sh| matches!(sh.kind, deckcraft_model::ShapeKind::Ink { .. })) else { @@ -6870,6 +6921,39 @@ fn master_body_placeholder_format_keeps_per_level_formatting() { assert_eq!(shown(&s), vec![Some(28.0), Some(24.0)]); } +/// A shape named many times in one edit is edited once and not copied per mention. +#[test] +fn repeated_target_ids_edit_the_shape_once() { + let mut s = session(); + let r = s.execute("shape.insert", &json!({"preset": "rect", "rect": [100, 100, 300, 100], "text": "x".repeat(100_000)})).unwrap(); + let id = r["id"].as_u64().unwrap(); + let edited = s.execute("shape.fill", &json!({"ids": vec![id; 50_000], "color": "#ff0000"})).unwrap(); + assert_eq!(edited, json!(1)); + let v = s.execute("shape.inspect", &json!({"id": id})).unwrap(); + assert_eq!(v["shape"]["fill"]["color"]["base"]["rgb"], json!({"a": 255, "b": 0, "g": 0, "r": 255})); +} + +#[test] +fn many_distinct_fonts_are_listed_once_each_without_a_quadratic_scan() { + // 30,000 runs in 15,000 families, each written twice in different case. + let mut p = deckcraft_model::Presentation::default(); + let slide = std::sync::Arc::make_mut(&mut p.slides[0]); + let mut body = deckcraft_model::text::TextBody::from_text("x"); + let run = body.paragraphs[0].runs[0].clone(); + body.paragraphs[0].runs = (0..30_000) + .map(|i| { + let mut r = run.clone(); + let name = format!("Qqq Font {}", i / 2); + r.props.font = Some(if i % 2 == 0 { name } else { name.to_uppercase() }); + r + }) + .collect(); + slide.shapes[0].text = Some(body); + let used = crate::fonts::used_fonts(&p); + assert_eq!(used.iter().filter(|f| f.starts_with("Qqq Font ")).count(), 15_000); + assert_eq!(crate::fonts::missing_fonts(&p).iter().filter(|f| f.to_ascii_lowercase().starts_with("qqq font ")).count(), 15_000); +} + #[test] fn slide_master_view_rejects_invalid_indices() { let mut s = Session::with_new(); diff --git a/crates/engine/src/tools.rs b/crates/engine/src/tools.rs index 82f53dfc3..2b7f78da9 100644 --- a/crates/engine/src/tools.rs +++ b/crates/engine/src/tools.rs @@ -17,6 +17,7 @@ mod geometry; mod hit; mod pointer; +pub(crate) use drag::MAX_GESTURE_POINTS; pub use hit::{line_xfrm, resize_box, table_rows, text_pos}; #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] @@ -275,6 +276,8 @@ pub struct ToolState { pub path_preview: Vec, pub path_smooth: bool, pub path_closing: bool, + /// Whether the last freeform press added a vertex (it adds none at the point cap). + pub press_added_vertex: bool, /// The shape in Edit Points: its vertices are handles instead of the box's. pub edit_points: Option, /// The vertex (path, index) whose control handles show in Edit Points. diff --git a/crates/engine/src/tools/click.rs b/crates/engine/src/tools/click.rs index 802eb8c5e..10bc03d98 100644 --- a/crates/engine/src/tools/click.rs +++ b/crates/engine/src/tools/click.rs @@ -4,8 +4,12 @@ use super::{Drag, Hit, Point, Result, Session, ShapeId, ShapeKind, TextSel, Tool impl Session { pub(super) fn double_click(&mut self, p: Point, tol: f64) -> Result { if let Some(Drag::Freeform { mode, mut points, .. }) = self.tool.drag.clone() { - // The double-click's own click added a duplicate vertex. - while points.len() > 2 && points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol) { + // The double-click's own click added a duplicate vertex (none at the point cap, where + // the existing vertices are all kept). + while self.tool.press_added_vertex + && points.len() > 2 + && points.last().zip(points.get(points.len() - 2)).is_some_and(|(a, b)| (*a - *b).hypot() <= tol) + { points.pop(); } self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false }); diff --git a/crates/engine/src/tools/drag.rs b/crates/engine/src/tools/drag.rs index a1abcb93c..41ade3266 100644 --- a/crates/engine/src/tools/drag.rs +++ b/crates/engine/src/tools/drag.rs @@ -12,9 +12,16 @@ struct MoveDrag { enter_text: Option<(ShapeId, Point)>, } +/// Samples one ink or freeform gesture keeps; later pointer moves extend it no further. +pub(crate) const MAX_GESTURE_POINTS: usize = 100_000; + impl Session { /// Update the active drag interaction using the pointer position and modifiers. pub(super) fn drag(&mut self, p: Point, mods: Mods, tol: f64) -> Result { + if matches!(self.tool.drag, Some(Drag::Ink { .. })) { + self.extend_ink(p); + return Ok(Value::Null); + } let Some(drag) = self.tool.drag.clone() else { return Ok(Value::Null) }; match drag { Drag::Guide { index } => { @@ -181,7 +188,7 @@ impl Session { } Drag::Freeform { mode, mut points, pressed } => { // Dragging scribbles (except for curves, which only take clicked points). - if pressed && mode != "curve" && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) { + if pressed && mode != "curve" && points.len() < MAX_GESTURE_POINTS && points.last().is_none_or(|l| (*l - p).hypot() >= 1.5) { points.push(p); } let mut v = points.clone(); @@ -191,15 +198,24 @@ impl Session { self.tool.path_preview = v; self.tool.drag = Some(Drag::Freeform { mode, points, pressed }); } - Drag::Ink { mut points } => { - points.push(self.ink_sample(p)); - self.tool.ink_preview = points.clone(); - self.tool.drag = Some(Drag::Ink { points }); - } + // Handled above. + Drag::Ink { .. } => {} } Ok(Value::Null) } + /// Ink grows by one sample per move: extend it (and its preview) in place rather than + /// copying the whole stroke twice per event, up to [`MAX_GESTURE_POINTS`]. + fn extend_ink(&mut self, p: Point) { + let sample = self.ink_sample(p); + if let Some(Drag::Ink { points }) = self.tool.drag.as_mut() + && points.len() < MAX_GESTURE_POINTS + { + points.push(sample); + self.tool.ink_preview.push(sample); + } + } + fn drag_move(&mut self, drag: MoveDrag, p: Point, mods: Mods, tol: f64) -> Result { let MoveDrag { ids, start, origs, moved, duplicate, enter_text } = drag; let mut d = p - start; diff --git a/crates/engine/src/tools/pointer.rs b/crates/engine/src/tools/pointer.rs index d20ae9315..e289e5c70 100644 --- a/crates/engine/src/tools/pointer.rs +++ b/crates/engine/src/tools/pointer.rs @@ -228,7 +228,10 @@ impl Session { self.tool.drag = Some(Drag::Freeform { mode, points, pressed: false }); return self.finish_freeform(true); } - points.push(p); + self.tool.press_added_vertex = points.len() < super::MAX_GESTURE_POINTS; + if self.tool.press_added_vertex { + points.push(p); + } self.tool.path_preview = points.clone(); self.tool.drag = Some(Drag::Freeform { mode, points, pressed: true }); return Ok(Value::Null); diff --git a/crates/h264/src/cabac_mb.rs b/crates/h264/src/cabac_mb.rs index 78c257e9b..368750292 100644 --- a/crates/h264/src/cabac_mb.rs +++ b/crates/h264/src/cabac_mb.rs @@ -48,7 +48,7 @@ impl SliceDecoder<'_> { let intra_slice = self.sh.slice_type.is_intra(); loop { ensure!(addr < total, "macroblock address {addr} out of range"); - self.start_mb(addr); + self.start_mb(addr)?; let skip = !intra_slice && self.decode_skip_flag(&mut c); if skip { self.decode_skip()?; diff --git a/crates/h264/src/decoder.rs b/crates/h264/src/decoder.rs index a3244d473..455701f92 100644 --- a/crates/h264/src/decoder.rs +++ b/crates/h264/src/decoder.rs @@ -15,7 +15,7 @@ use crate::slicedec::{PicState, SliceDecoder}; use crate::transform::LevelScale; use crate::{ColorInfo, Picture}; use deckcraft_bitstream::{BitReader, annexb_nals, length_prefixed_nals, unescape_rbsp}; -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use std::sync::{Arc, Mutex}; /// One slice of a picture job. @@ -90,6 +90,10 @@ pub struct Decoder { ls_cache: Vec<(Arc, Arc)>, /// Pictures leaving the DPB, converted once their decoding job has finished. out_queue: VecDeque, + /// Queued entries per frame id and their summed bytes, kept in step with `out_queue` so the + /// budget check does not rescan it. + queued_ids: HashMap, + queued_bytes: u64, shared: Arc, #[cfg(feature = "threads")] pool: Option, @@ -175,6 +179,8 @@ impl Decoder { active_sps: None, ls_cache: Vec::new(), out_queue: VecDeque::new(), + queued_ids: HashMap::new(), + queued_bytes: 0, shared: Arc::new(Shared::default()), #[cfg(feature = "threads")] pool, @@ -283,7 +289,7 @@ impl Decoder { if !o.frame.is_published(o.frame.mb_h() - 1) && self.out_queue.len() <= max_queue { break; } - let Some(o) = self.out_queue.pop_front() else { break }; + let Some(o) = self.pop_out() else { break }; pics.push(make_picture(&o)); } Ok(pics) @@ -295,7 +301,7 @@ impl Decoder { let mut outs = Vec::new(); self.dpb.flush(&mut outs); self.emit(outs); - let pics = self.out_queue.drain(..).map(|o| make_picture(&o)).collect(); + let pics = std::iter::from_fn(|| self.pop_out()).map(|o| make_picture(&o)).collect(); while let Some(f) = self.in_flight.pop_front() { f.wait_complete(); } @@ -451,13 +457,24 @@ impl Decoder { chroma_planes: if chroma == 0 { 0 } else { 2 }, bytes_per_sample: 1, }; - let pictures = (sps.max_dpb_frames() + 1 + self.max_in_flight) as u64; - ensure!( - shape.fits(pictures, self.memory_budget), - "{}x{} pictures with {pictures} in the decoder exceed the memory budget", - sps.width(), - sps.height() - ); + // Pictures already held count at their own size, not the incoming SPS's: after a + // resolution change, larger pictures of the old size can still be in the DPB or waiting + // for output (one decode call may carry many access units). A buffer shared with a DPB + // entry is counted once. + let pic = shape.bytes().unwrap_or(u64::MAX); + let held = |frames: &mut dyn Iterator>| frames.fold(0u64, |n, f| n.saturating_add(f.bytes())); + // Queued bytes minus the queue entries that share a DPB frame (each distinct DPB frame once). + let mut ids: Vec<(u32, u64)> = self.dpb.entries.iter().map(|e| (e.frame.id, e.frame.bytes())).collect(); + ids.sort_unstable(); + ids.dedup(); + let shared = ids.iter().fold(0u64, |n, (id, b)| n.saturating_add(self.queued_ids.get(id).copied().unwrap_or(0).saturating_mul(*b))); + let queued = self.queued_bytes.saturating_sub(shared); + let in_dpb = held(&mut self.dpb.entries.iter().map(|e| &e.frame)); + let dpb = sps.max_dpb_frames().max(self.dpb.entries.len()) as u64; + // The DPB (the larger of its new slots and what it holds now), the picture being decoded, + // and what is in flight or queued. + let bytes = pic.saturating_mul(dpb).max(in_dpb).saturating_add(pic).saturating_add(pic.saturating_mul(self.max_in_flight as u64).max(queued)); + ensure!(bytes <= self.memory_budget, "{}x{} pictures with {bytes} bytes in the decoder exceed the memory budget", sps.width(), sps.height()); Ok(()) } @@ -562,8 +579,24 @@ impl Decoder { } fn emit(&mut self, outs: Vec) { + for o in &outs { + *self.queued_ids.entry(o.frame.id).or_insert(0) += 1; + self.queued_bytes = self.queued_bytes.saturating_add(o.frame.bytes()); + } self.out_queue.extend(outs); } + + fn pop_out(&mut self) -> Option { + let o = self.out_queue.pop_front()?; + if let Some(n) = self.queued_ids.get_mut(&o.frame.id) { + *n -= 1; + if *n == 0 { + self.queued_ids.remove(&o.frame.id); + } + } + self.queued_bytes = self.queued_bytes.saturating_sub(o.frame.bytes()); + Some(o) + } } /// Decode all slices of one picture and publish its rows. diff --git a/crates/h264/src/dpb.rs b/crates/h264/src/dpb.rs index 46e8f060a..940b1a4c5 100644 --- a/crates/h264/src/dpb.rs +++ b/crates/h264/src/dpb.rs @@ -139,8 +139,9 @@ impl Dpb { let mut unused = (prev_ref_frame_num + 1) % max_frame_num; // Only the last `max_num_ref_frames` "non-existing" frames can survive the sliding window, and // inserting more than that evicts every earlier short-term reference. Skip straight to that tail - // (keeping the POC state walk for every missing frame_num) so a hostile gap of up to 65535 - // frames costs at most `max_num_ref_frames` frame allocations instead of one per missing frame. + // (advancing the POC state across the skipped run in constant time) so a hostile gap of up to + // 65535 frames costs at most `max_num_ref_frames` frame allocations and POC updates instead of + // one per missing frame. let gap = frame_num.wrapping_add(max_frame_num).wrapping_sub(unused) % max_frame_num; let keep = u32::try_from(max_num_ref_frames.max(1)).unwrap_or(u32::MAX); let skip = gap.saturating_sub(keep); @@ -149,10 +150,17 @@ impl Dpb { e.mark = RefMark::Unused; } self.prune(); - for _ in 0..skip { - poc_state(unused); - unused = (unused + 1) % max_frame_num; + // The POC state only advances FrameNumOffset where frame_num decreases, so walking the + // skipped run is the same as visiting its first value, the wrap (if the run crosses + // it) and its last value: constant work however long the gap. + let last = (unused + (skip - 1) % max_frame_num) % max_frame_num; + poc_state(unused); + if last < unused { + poc_state(max_frame_num - 1); + poc_state(0); } + poc_state(last); + unused = (last + 1) % max_frame_num; } let mut guard = 0; while unused != frame_num && guard < max_frame_num { @@ -438,14 +446,15 @@ impl Dpb { } } } else { - while self.entries.len() >= self.capacity { + // Evict until there is room (frame_num gap concealment can leave the DPB over-full), so + // the store never holds more pictures than the memory budget was checked for. + while self.entries.len() >= self.capacity.max(1) { if !self.bump(out) { if let Some(pos) = self.entries.iter().position(|e| !e.needed_for_output) { self.entries.remove(pos); } else { self.entries.remove(0); } - break; } } } @@ -546,6 +555,37 @@ mod tests { (made, walk, left) } + /// (FrameNumOffset, prev frame_num) after `walk`, by the rule of `PocState::update_gap_frame`. + fn poc_after(walk: &[u32], prev: u32, max: u32) -> (u64, u32) { + walk.iter().fold((0, prev), |(off, prev), &f| (if prev > f { off + u64::from(max) } else { off }, f)) + } + + #[test] + fn gap_walk_matches_every_frame_num_for_any_skip() { + for (prev, cur) in [(1, 9), (10, 3), (14, 0), (15, 14), (0, 15), (7, 6)] { + for refs in 1..4 { + let mut dpb = Dpb::new(); + let (_, walk, _) = fill(&mut dpb, prev, cur, 16, refs); + let every: Vec = (1..16).map(|i| (prev + i) % 16).take_while(|&f| f != cur).collect(); + assert_eq!(poc_after(&walk, prev, 16), poc_after(&every, prev, 16), "{prev} -> {cur}, {refs} refs"); + assert_eq!(walk.last(), every.last(), "{prev} -> {cur}, {refs} refs"); + } + } + } + + #[test] + fn reference_store_evicts_down_to_capacity() { + let mut dpb = Dpb::new(); + dpb.capacity = 2; + // Gap concealment can leave more non-output references than the capacity. + for i in 0..5 { + short(&mut dpb, i, i); + } + let mut out = Vec::new(); + assert!(!dpb.bump_for_space(RefMark::Short, 0, &mut out)); + assert!(dpb.entries.len() < 2, "{} pictures left before storing into a 2-picture DPB", dpb.entries.len()); + } + #[test] fn huge_gap_makes_only_the_frames_that_stay_referenced() { let mut dpb = Dpb::new(); @@ -553,8 +593,9 @@ mod tests { short(&mut dpb, 2, 1); let (made, walk, left) = fill(&mut dpb, 1, 65535, 65536, 4); assert_eq!(made, 4); - // The POC state still walks every missing frame_num, in order. - assert_eq!(walk.len(), 65533); + // The POC state ends where walking every missing frame_num would leave it, in a few steps. + assert_eq!(poc_after(&walk, 1, 65536), poc_after(&(2..65535).collect::>(), 1, 65536)); + assert!(walk.len() <= 8, "{} POC updates", walk.len()); assert_eq!(walk.first(), Some(&2)); assert_eq!(walk.last(), Some(&65534)); assert_eq!(left, vec![(65531, true), (65532, true), (65533, true), (65534, true)]); @@ -566,7 +607,9 @@ mod tests { short(&mut dpb, 1, 65000); let (made, walk, left) = fill(&mut dpb, 65000, 3, 65536, 2); assert_eq!(made, 2); - assert_eq!(walk.len(), 538); + let every: Vec = (65001..65536).chain(0..3).collect(); + assert_eq!(poc_after(&walk, 65000, 65536), poc_after(&every, 65000, 65536)); + assert!(walk.len() <= 6, "{} POC updates", walk.len()); assert_eq!(walk.first(), Some(&65001)); assert_eq!(walk.last(), Some(&2)); assert_eq!(left, vec![(1, true), (2, true)]); diff --git a/crates/h264/src/picture.rs b/crates/h264/src/picture.rs index 39add6a08..83331cb95 100644 --- a/crates/h264/src/picture.rs +++ b/crates/h264/src/picture.rs @@ -160,6 +160,14 @@ impl Frame { self.rows.len() } + /// Sample bytes this picture holds (luma and both chroma planes), as the decoder's memory + /// budget counts a picture. + pub fn bytes(&self) -> u64 { + (self.width as u64) + .saturating_mul(self.height as u64) + .saturating_add((self.cwidth as u64).saturating_mul(self.cheight as u64).saturating_mul(2)) + } + /// Publish MB row `r` (ignored if already published). pub fn publish(&self, r: usize, row: FrameRow) { let _ = self.rows[r].set(row); diff --git a/crates/h264/src/slicedec.rs b/crates/h264/src/slicedec.rs index 058ccfe16..649739986 100644 --- a/crates/h264/src/slicedec.rs +++ b/crates/h264/src/slicedec.rs @@ -346,8 +346,11 @@ impl<'a> SliceDecoder<'a> { // Neighbour helpers // ------------------------------------------------------------------------------------------ - /// Set up the current macroblock address and neighbour availability. - pub fn start_mb(&mut self, addr: usize) { + /// Set up the current macroblock address and neighbour availability. A macroblock an earlier + /// slice of this picture already decoded is refused: overlapping slices would otherwise redo + /// the picture's reconstruction once per slice. + pub fn start_mb(&mut self, addr: usize) -> Result<()> { + ensure!(self.pic.mbs[addr].slice_num == u32::MAX, "macroblock {addr} overlaps an earlier slice"); self.mb_addr = addr; self.mb_x = addr % self.mb_w; self.mb_y = addr / self.mb_w; @@ -364,6 +367,7 @@ impl<'a> SliceDecoder<'a> { ]; self.mv_done = 0; self.pic.mbs[addr] = MbState { slice_num: sn, ..MbState::default() }; + Ok(()) } #[inline] @@ -1467,7 +1471,7 @@ impl<'a> SliceDecoder<'a> { let run = r.read_ue()? as usize; ensure!(addr + run <= total, "mb_skip_run out of range"); for _ in 0..run { - self.start_mb(addr); + self.start_mb(addr)?; self.decode_skip()?; self.pic.mb_done(addr); addr += 1; @@ -1477,7 +1481,7 @@ impl<'a> SliceDecoder<'a> { } ensure!(addr < total, "macroblock address {addr} out of range"); } - self.start_mb(addr); + self.start_mb(addr)?; self.decode_mb_cavlc(r)?; self.pic.mb_done(addr); addr += 1; diff --git a/crates/h264/src/synth_tests.rs b/crates/h264/src/synth_tests.rs index 76609dbde..199f5e69d 100644 --- a/crates/h264/src/synth_tests.rs +++ b/crates/h264/src/synth_tests.rs @@ -113,6 +113,7 @@ fn avcc_rejects_a_valid_picture_slice_without_starting_a_picture() { poc_lsb: Some(0), poc_delta: None, wt: None, + first_mb: 0, }, false, ); @@ -153,6 +154,8 @@ struct Pic { /// Explicit weights (luma, cb, cr) as (weight, offset) for refIdx 0 of list 0 and list 1; /// log2 denominators are 5 (luma) and 3 (chroma). wt: Option<[[(i32, i32); 3]; 2]>, + /// first_mb_in_slice (`mbs` then lists the slice's macroblocks from there). + first_mb: u32, } impl Pic { @@ -165,7 +168,7 @@ impl Pic { } fn slice_header(w: &mut BitWriter, p: &Pic, cabac: bool) { - w.write_ue(0); // first_mb + w.write_ue(p.first_mb); w.write_ue(if p.b { 6 } else if p.is_p() { @@ -660,3 +663,63 @@ fn frame_num_gap_is_filled_when_the_sps_allows_gaps() { assert_eq!(out.len(), 2); assert_eq!(dec.stats().frame_num_gaps, 1); } + +/// An IDR picture followed by `pictures` reference P pictures of P_Skip macroblocks. +fn skip_stream(pictures: u32) -> (Vec, Vec>) { + let mut seed = 11u32; + let a: Vec>> = (0..NMB).map(|_| Some(random_mb(&mut seed))).collect(); + let mut head = sps(66, 2, false, PocType::T2); + head.extend(pps(false, false)); + head.extend(slice_nal(&Pic { idr: true, frame_num: 0, ref_idc: 3, mbs: a, ..Default::default() }, false)); + let ps = (1..=pictures).map(|k| slice_nal(&Pic { frame_num: k % 16, ref_idc: 2, mbs: vec![None; NMB], ..Default::default() }, false)).collect(); + (head, ps) +} + +#[test] +fn overlapping_slices_are_not_reconstructed_twice() { + let (head, ps) = skip_stream(1); + let mut once = Decoder::with_threads(1); + let mut out = once.decode(&[head.clone(), ps[0].clone()].concat(), 0).unwrap(); + out.extend(once.flush()); + assert!(once.take_error().is_none()); + assert_eq!(out.len(), 2); + // A second slice of the same picture starting at macroblock 1 covers those macroblocks again. + let again = slice_nal(&Pic { frame_num: 1, ref_idc: 2, mbs: vec![None; NMB - 1], first_mb: 1, ..Default::default() }, false); + let mut twice = Decoder::with_threads(1); + let result = twice.decode(&[head, ps[0].clone(), again].concat(), 0); + twice.flush(); + let err = result.err().or_else(|| twice.take_error()).expect("overlapping slice refused"); + assert!(err.to_string().contains("overlaps"), "{err}"); +} + +#[test] +fn output_queue_counts_against_the_memory_budget() { + let (head, ps) = skip_stream(40); + let sps_nal = sps(66, 2, false, PocType::T2); + let parsed = crate::params::Sps::parse(&deckcraft_bitstream::unescape_rbsp(&sps_nal[5..])).unwrap(); + let one = (NMB * 256 + 2 * NMB * 64) as u64; + let budget = (parsed.max_dpb_frames() + 1 + 3) as u64 * one; + // One access unit per call: output drains as it goes, and the stream fits. + let mut dec = Decoder::with_threads(1); + dec.set_memory_budget(budget); + let mut out = dec.decode(&head, 0).unwrap(); + for (i, p) in ps.iter().enumerate() { + out.extend(dec.decode(p, i as i64 + 1).unwrap()); + } + out.extend(dec.flush()); + assert!(dec.take_error().is_none()); + assert_eq!(out.len(), 41); + // Every access unit in one call: the pictures held for output stop fitting the budget. + let mut dec = Decoder::with_threads(1); + dec.set_memory_budget(budget); + let err = dec.decode(&[head, ps.concat()].concat(), 0).expect_err("output queue is over budget"); + assert!(err.to_string().contains("memory budget"), "{err}"); +} + +/// A held picture is charged at its own size (luma and both 4:2:0 chroma planes), the same +/// count the budget uses for the incoming SPS's pictures. +#[test] +fn a_held_frame_is_charged_at_its_own_size() { + let f = crate::picture::Frame::new(0, 0, 2, 3); + assert_eq!(f.bytes(), (2 * 3 * 256 + 2 * 2 * 3 * 64) as u64); +} diff --git a/crates/media/src/audio.rs b/crates/media/src/audio.rs index 5f623a3e2..09f9efa37 100644 --- a/crates/media/src/audio.rs +++ b/crates/media/src/audio.rs @@ -81,7 +81,9 @@ pub fn decode(bytes: &Bytes) -> Result { Some(r) => r, None => contained(|| decode_symphonia(bytes, "mkv")), }, - Container::Mp4 if !crate::mp4guard::tables_fit(bytes) => Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into())), + Container::Mp4 | Container::Unknown | Container::Mp3 if !crate::mp4guard::probe_fits(bytes) => { + Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into())) + } c => contained(|| decode_symphonia(bytes, c.name())), } } diff --git a/crates/media/src/mp4guard.rs b/crates/media/src/mp4guard.rs index ff8e86138..56047979f 100644 --- a/crates/media/src/mp4guard.rs +++ b/crates/media/src/mp4guard.rs @@ -12,6 +12,29 @@ pub(crate) fn tables_fit(b: &[u8]) -> bool { boxes_fit(b, b.len() as u64, 0) } +/// Bytes symphonia's probe scans for a format marker before giving up. +const PROBE_SEARCH_LIMIT: usize = 1024 * 1024; + +/// Whether symphonia can be given `b`, whatever the sniffer made of it: its probe takes the first +/// `ftyp` within its search window as the start of an MP4 (the size field 4 bytes before it), so +/// the table check must start where that parser will, not at offset 0. +pub(crate) fn probe_fits(b: &[u8]) -> bool { + if crate::Container::sniff(b) == crate::Container::Mp4 && !tables_fit(b) { + return false; + } + // The probe reads leading ID3v2 tags as metadata and searches again after the last one. + let mut rest = b; + while rest.starts_with(b"ID3") { + let size = rest.get(6..10).map(|s| s.iter().fold(0usize, |a, &x| (a << 7) | (x & 0x7f) as usize)).unwrap_or(0); + rest = rest.get(10 + size..).unwrap_or(&[]); + } + let head = &rest[..rest.len().min(PROBE_SEARCH_LIMIT + 16)]; + match head.windows(4).position(|w| w == b"ftyp") { + Some(p) => tables_fit(&rest[p.saturating_sub(4)..]), + None => true, + } +} + fn be32(b: &[u8], at: usize) -> Option { b.get(at..at.checked_add(4)?).map(|s| u32::from_be_bytes([s[0], s[1], s[2], s[3]]) as u64) } @@ -74,3 +97,29 @@ fn table_fits(kind: &[u8], body: &[u8], file_len: u64) -> bool { }; be32(body, 4).is_some_and(|count| count <= body.len().saturating_sub(8) as u64 / entry) } + +#[cfg(test)] +mod tests { + use super::*; + + /// An MP4 whose `stsc` count asks for far more entries than the file holds. + fn mp4_with_huge_stsc() -> Vec { + let stsc = [&24u32.to_be_bytes()[..], b"stsc", &[0, 0, 0, 0], &u32::MAX.to_be_bytes(), &[0; 8]].concat(); + let stbl = [&(stsc.len() as u32 + 8).to_be_bytes()[..], b"stbl", &stsc].concat(); + let moov = [&(stbl.len() as u32 + 8).to_be_bytes()[..], b"moov", &stbl].concat(); + [&16u32.to_be_bytes()[..], b"ftyp", b"isom", &[0; 4], &moov].concat() + } + + #[test] + fn a_prefixed_mp4_is_checked_where_symphonia_starts_it() { + let mp4 = mp4_with_huge_stsc(); + assert!(!tables_fit(&mp4)); + assert!(!probe_fits(&mp4)); + let prefixed = [&b"junk before the file "[..], &mp4].concat(); + assert_eq!(crate::Container::sniff(&prefixed), crate::Container::Unknown); + assert!(tables_fit(&prefixed), "offset 0 sees no MP4 boxes"); + assert!(!probe_fits(&prefixed)); + let far = [&vec![0u8; PROBE_SEARCH_LIMIT + 16][..], &mp4].concat(); + assert!(probe_fits(&far), "past the probe window symphonia never reads it as an MP4"); + } +} diff --git a/crates/media/src/probe.rs b/crates/media/src/probe.rs index 97d9a751f..4e8e845c5 100644 --- a/crates/media/src/probe.rs +++ b/crates/media/src/probe.rs @@ -71,7 +71,8 @@ pub fn probe_headers(bytes: &Bytes) -> Result { }; if info.audio.is_none() && info.video.is_none() || info.audio.as_ref().is_some_and(|a| a.sample_rate == 0) { // Plain audio files (and anything our demuxers didn't describe): symphonia's probe. - if c == Container::Mp4 && !crate::mp4guard::tables_fit(bytes) { + // Symphonia opens an MP4 wherever its probe finds an `ftyp` (after leading ID3v2 tags too); other known containers (a WAV's audio bytes can spell an `ftyp`) skip the check. + if matches!(c, Container::Mp4 | Container::Unknown | Container::Mp3) && !crate::mp4guard::probe_fits(bytes) { return Err(MediaError::Corrupt("the MP4's sample tables do not fit the file".into())); } let (codec, rate, channels, frames) = std::panic::catch_unwind(|| crate::audio::probe_symphonia(bytes, c.name())) diff --git a/crates/metafile/src/cliprect.rs b/crates/metafile/src/cliprect.rs index fd9dfc14a..ef6bb7c88 100644 --- a/crates/metafile/src/cliprect.rs +++ b/crates/metafile/src/cliprect.rs @@ -11,6 +11,14 @@ use crate::geom::{Op, P}; /// Line segments per Bézier when a crossing shape is flattened. const CURVE_STEPS: usize = 16; +/// Point-against-edge tests one clipped item may cost (all its subpaths together); an item over +/// it is dropped, as a single polygon over it already was. +const MAX_CLIP_WORK: usize = 1_000_000; + +/// The same allowance for intersecting two clip regions, a budget of its own: two convex +/// 1,000-gons cost about 1.5 million tests once the cut polygon has grown. +pub(crate) const MAX_AREA_CLIP_WORK: usize = 4 * MAX_CLIP_WORK; + /// The bounding box `[l, t, r, b]` of the points in `ops` (control points included). pub(crate) fn ops_bbox(ops: &[Op]) -> Option<[f64; 4]> { let mut b: Option<[f64; 4]> = None; @@ -98,16 +106,18 @@ fn is_rect(clip: &[P]) -> bool { pts_bbox(clip).is_some_and(|b| clip.len() == 4 && clip.iter().all(|p| (p.0 == b[0] || p.0 == b[2]) && (p.1 == b[1] || p.1 == b[3]))) } -/// Sutherland–Hodgman: `poly` cut by convex `clip`. -pub(crate) fn clip_polygon(poly: &[P], clip: &[P]) -> Vec

{ - if poly.len().saturating_mul(clip.len()) > 1_000_000 { - return Vec::new(); - } +/// Sutherland–Hodgman: `poly` cut by convex `clip`, charging each point visited against each clip +/// edge to `budget`: the polygon +/// can gain a vertex at every edge, so its cost isn't known from its input size alone. `None` +/// once the budget is spent. +pub(crate) fn clip_polygon_within(poly: &[P], clip: &[P], budget: &mut usize) -> Option> { let mut poly = poly.to_vec(); for i in 0..clip.len() { - if poly.len().saturating_mul(clip.len() - i) > 1_000_000 { - return Vec::new(); + // The work actually done: this edge visits the polygon as it now stands. + if poly.len() > *budget { + return None; } + *budget -= poly.len(); let (a, b) = (clip[i], clip[(i + 1) % clip.len()]); let keep = |p: P| cross(a, b, p) >= 0.0; let cut = |p: P, q: P| { @@ -133,7 +143,7 @@ pub(crate) fn clip_polygon(poly: &[P], clip: &[P]) -> Vec

{ break; } } - poly + Some(poly) } fn disjoint(b: [f64; 4], c: [f64; 4]) -> bool { @@ -158,15 +168,17 @@ pub(crate) fn clip_raw(raw: Raw, clip: &[P]) -> Vec { return Vec::new(); } let subs = flatten(&ops); + // One allowance for the fill and the stroke together; the item goes once it is spent. + let mut budget = MAX_CLIP_WORK; let mut out = Vec::new(); if fill.is_some() { - let f = clip_fill(&subs, clip); + let Some(f) = clip_fill(&subs, clip, &mut budget) else { return Vec::new() }; if !f.is_empty() { out.push(Raw::Path { ops: f, fill, stroke: None, even_odd, region: region.clone() }); } } if stroke.is_some() { - let s = clip_stroke(&subs, clip); + let Some(s) = clip_stroke(&subs, clip, &mut budget) else { return Vec::new() }; if !s.is_empty() { out.push(Raw::Path { ops: s, fill: None, stroke, even_odd, region }); } @@ -181,7 +193,8 @@ pub(crate) fn clip_raw(raw: Raw, clip: &[P]) -> Vec { if disjoint(b, cb) { return Vec::new(); } - let f = clip_fill(&flatten(&ops), clip); + let subs = flatten(&ops); + let f = clip_fill(&subs, clip, &mut { MAX_CLIP_WORK }).unwrap_or_default(); if f.is_empty() { Vec::new() } else { vec![Raw::Pattern { ops: f, even_odd, tile, region }] } } Raw::Bitmap { rect, img, region } => crop(rect, img, clip).map(|(rect, img)| vec![Raw::Bitmap { rect, img, region }]).unwrap_or_default(), @@ -230,20 +243,23 @@ pub(crate) fn flatten(ops: &[Op]) -> Vec<(Vec

, bool)> { subs } -fn clip_fill(subs: &[(Vec

, bool)], clip: &[P]) -> Vec { +/// The clipped fill of every subpath, its clipping work charged to `budget` with that of the +/// stroke: `None` (the whole item goes) once it is spent. +fn clip_fill(subs: &[(Vec

, bool)], clip: &[P], budget: &mut usize) -> Option> { let mut ops = Vec::new(); for (pts, _) in subs { - let poly = clip_polygon(pts, clip); + let poly = clip_polygon_within(pts, clip, budget)?; if poly.len() >= 3 { ops.push(Op::M(poly[0])); ops.extend(poly[1..].iter().map(|p| Op::L(*p))); ops.push(Op::Z); } } - ops + Some(ops) } -fn clip_stroke(subs: &[(Vec

, bool)], clip: &[P]) -> Vec { +/// The clipped outline of every subpath; `None` once `budget` is spent. +fn clip_stroke(subs: &[(Vec

, bool)], clip: &[P], budget: &mut usize) -> Option> { let mut ops = Vec::new(); for (pts, closed) in subs { let mut pts = pts.clone(); @@ -252,7 +268,7 @@ fn clip_stroke(subs: &[(Vec

, bool)], clip: &[P]) -> Vec { } let mut last: Option

= None; for w in pts.windows(2) { - let Some((a, b)) = segment(w[0], w[1], clip) else { + let Some((a, b)) = segment(w[0], w[1], clip, budget)? else { last = None; continue; }; @@ -263,21 +279,23 @@ fn clip_stroke(subs: &[(Vec

, bool)], clip: &[P]) -> Vec { last = Some(b); } } - ops + Some(ops) } -/// Cyrus–Beck: the part of segment `a`–`b` inside convex `clip`. -fn segment(a: P, b: P, clip: &[P]) -> Option<(P, P)> { +/// Cyrus–Beck: the part of segment `a`–`b` inside convex `clip`, one unit of `budget` per clip edge +/// visited; the outer `None` is a spent budget. +fn segment(a: P, b: P, clip: &[P], budget: &mut usize) -> Option> { let d = (b.0 - a.0, b.1 - a.1); let (mut t0, mut t1) = (0.0f64, 1.0f64); for i in 0..clip.len() { + *budget = budget.checked_sub(1)?; let (e0, e1) = (clip[i], clip[(i + 1) % clip.len()]); // Inside is cross(e0, e1, p) >= 0, linear in t along the segment. let f0 = cross(e0, e1, a); let df = (e1.0 - e0.0) * d.1 - (e1.1 - e0.1) * d.0; if df == 0.0 { if f0 < 0.0 { - return None; + return Some(None); } } else { let t = -f0 / df; @@ -288,7 +306,7 @@ fn segment(a: P, b: P, clip: &[P]) -> Option<(P, P)> { } } } - (t0 <= t1).then_some(((a.0 + d.0 * t0, a.1 + d.1 * t0), (a.0 + d.0 * t1, a.1 + d.1 * t1))) + Some((t0 <= t1).then_some(((a.0 + d.0 * t0, a.1 + d.1 * t0), (a.0 + d.0 * t1, a.1 + d.1 * t1)))) } /// Crops a bitmap drawn into `rect` = `[x, y, w, h]` (device, positive extents) to the clip diff --git a/crates/metafile/src/emfplus.rs b/crates/metafile/src/emfplus.rs index cf5fd97e9..d0f56f862 100644 --- a/crates/metafile/src/emfplus.rs +++ b/crates/metafile/src/emfplus.rs @@ -28,7 +28,7 @@ use std::sync::Arc; use crate::bytes::Bytes; use crate::canvas::Raw; use crate::canvas::{BS_NULL, BS_SOLID, Brush, Canvas, LogFont, PS_NULL, Pen, RawTile, Xform, hatch_tile, mul}; -use crate::cliprect::{clip_polygon, convex, flatten, ops_bbox, pts_bbox, rect_poly}; +use crate::cliprect::{MAX_AREA_CLIP_WORK, clip_polygon_within, convex, flatten, ops_bbox, pts_bbox, rect_poly}; use crate::dib::Rgba; use crate::geom::{Op, P}; use crate::text::{ETO_CLIPPED, Font}; @@ -1170,7 +1170,13 @@ fn area_of_ops(ops: &[Op]) -> Vec

{ fn area_intersect(a: Area, b: Area) -> Area { match (a, b) { (None, x) | (x, None) => x, - (Some(a), Some(b)) if a.len() >= 3 && b.len() >= 3 => Some(convex(&clip_polygon(&a, &b)).unwrap_or_default()), + (Some(a), Some(b)) if a.len() >= 3 && b.len() >= 3 => { + // Out of work: keep the area so far rather than call the region empty. + match clip_polygon_within(&a, &b, &mut { MAX_AREA_CLIP_WORK }) { + Some(p) => Some(convex(&p).unwrap_or_default()), + None => Some(a), + } + } _ => Some(Vec::new()), } } diff --git a/crates/metafile/src/tests.rs b/crates/metafile/src/tests.rs index 29fab81ec..766beddab 100644 --- a/crates/metafile/src/tests.rs +++ b/crates/metafile/src/tests.rs @@ -2071,6 +2071,87 @@ fn emfplus_wrap_line_linear_performance() { fn clip_polygon_work_bound() { let poly: Vec<(f64, f64)> = (0..2000).map(|i| (i as f64, i as f64)).collect(); let clip: Vec<(f64, f64)> = (0..2000).map(|i| (i as f64, i as f64)).collect(); - let clipped = crate::cliprect::clip_polygon(&poly, &clip); - assert!(clipped.is_empty(), "excessive polygon clip product rejected"); + assert!(crate::cliprect::clip_polygon_within(&poly, &clip, &mut 1_000_000).is_none(), "excessive polygon clip product rejected"); +} + +#[test] +fn clip_polygon_charges_actual_work_not_projected_growth() { + // Two convex 1,000-gons: the product is exactly the budget, and a cut that adds a vertex must + // not make a later step look over budget. + let gon = |cx: f64| -> Vec<(f64, f64)> { + (0..1000).map(|i| (i as f64 / 1000.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (cx + 100.0 * c, 100.0 * s)).collect() + }; + let clipped = crate::cliprect::clip_polygon_within(&gon(0.0), &gon(30.0), &mut 4_000_000).unwrap(); + assert!(clipped.len() > 100, "the intersection is kept: {}", clipped.len()); +} + +#[test] +fn clip_work_is_shared_by_fill_and_stroke() { + use crate::canvas::Raw; + use crate::geom::Op; + // One zigzag subpath crossing a 512-gon's edge: about 510k point-edge tests per pass, so a + // fill alone fits the allowance but a fill plus a stroke would spend it twice. + let clip: Vec<(f64, f64)> = + (0..512).map(|i| (i as f64 / 512.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (100.0 + 100.0 * c, 100.0 + 100.0 * s)).collect(); + let mut ops = vec![Op::M((0.0, 95.0))]; + ops.extend((1..1000).map(|i| Op::L((i as f64 * 0.19, if i % 2 == 0 { 95.0 } else { 300.0 })))); + ops.push(Op::Z); + let raw = |fill, stroke| Raw::Path { ops: ops.clone(), fill, stroke, even_odd: false, region: None }; + let black = [0, 0, 0, 255]; + assert!(!crate::cliprect::clip_raw(raw(Some(black), None), &clip).is_empty(), "a fill alone is within the allowance"); + assert!(crate::cliprect::clip_raw(raw(Some(black), Some((black, 1.0))), &clip).is_empty(), "fill and stroke share one allowance"); +} + +#[test] +fn clip_work_is_bounded_across_subpaths() { + use crate::canvas::Raw; + use crate::geom::Op; + // 512-gon clip; 4,000 small triangles crossing its edge: each under the per-polygon cap, together + // far over it, for both the fill and the stroke. + let clip: Vec<(f64, f64)> = + (0..512).map(|i| (i as f64 / 512.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (100.0 + 100.0 * c, 100.0 + 100.0 * s)).collect(); + let tri = |x: f64| [Op::M((x, 95.0)), Op::L((x + 1.0, 95.0)), Op::L((x, 300.0)), Op::Z]; + let many: Vec = (0..4000).flat_map(|i| tri(i as f64 % 200.0)).collect(); + let raw = |ops| Raw::Path { ops, fill: Some([0, 0, 0, 255]), stroke: Some(([0, 0, 0, 255], 1.0)), even_odd: false, region: None }; + assert!(crate::cliprect::clip_raw(raw(many), &clip).is_empty(), "the whole item's clipping work is bounded"); + assert_eq!(crate::cliprect::clip_raw(raw(tri(100.0).to_vec()), &clip).len(), 2, "a small crossing shape is still clipped"); +} + +/// Clipping a fill can add a vertex at every clip edge, so a polygon's work isn't its input size: +/// shapes that pass the up-front estimate but grow while clipped are charged as they grow, against +/// one budget for the whole item. +#[test] +fn growing_fill_clips_share_one_budget() { + use crate::canvas::Raw; + use crate::geom::Op; + let clip: Vec<(f64, f64)> = + (0..512).map(|i| (i as f64 / 512.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (100.0 + 100.0 * c, 100.0 + 100.0 * s)).collect(); + // A triangle around the whole circle: clipped, it becomes the 512-gon, about 130,000 steps. + let tri = [Op::M((-1000.0, -1000.0)), Op::L((1000.0, -1000.0)), Op::L((100.0, 1000.0)), Op::Z]; + let raw = |n: usize| Raw::Path { ops: tri.repeat(n), fill: Some([0, 0, 0, 255]), stroke: None, even_odd: false, region: None }; + // 400 of them pass the estimate (400 x 4 x 512 is under a million) but cost about 52 million. + assert!(crate::cliprect::clip_raw(raw(400), &clip).is_empty(), "the growing work is charged"); + assert_eq!(crate::cliprect::clip_raw(raw(2), &clip).len(), 1, "a couple of them are still clipped"); +} + +#[test] +fn stroke_work_is_charged_and_outside_items_are_cheap() { + use crate::canvas::Raw; + use crate::geom::Op; + let clip: Vec<(f64, f64)> = + (0..512).map(|i| (i as f64 / 512.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (100.0 + 100.0 * c, 100.0 + 100.0 * s)).collect(); + // A stroke-only zigzag of 4,000 segments across the edge: 4,000 x 512 tests, over the allowance. + let mut ops = vec![Op::M((0.0, 95.0))]; + ops.extend((1..4000).map(|i| Op::L((i as f64 * 0.05, if i % 2 == 0 { 95.0 } else { 300.0 })))); + let raw = Raw::Path { ops, fill: None, stroke: Some(([0, 0, 0, 255], 1.0)), even_odd: false, region: None }; + assert!(crate::cliprect::clip_raw(raw, &clip).is_empty(), "stroke work is bounded"); +} + +#[test] +fn region_intersection_over_budget_keeps_the_previous_area() { + let gon = |cx: f64| -> Vec<(f64, f64)> { + (0..1000).map(|i| (i as f64 / 1000.0 * std::f64::consts::TAU).sin_cos()).map(|(s, c)| (cx + 100.0 * c, 100.0 * s)).collect() + }; + assert!(crate::cliprect::clip_polygon_within(&gon(0.0), &gon(30.0), &mut 10).is_none()); + assert!(crate::cliprect::clip_polygon_within(&gon(0.0), &gon(30.0), &mut crate::cliprect::MAX_AREA_CLIP_WORK.clone()).is_some()); } diff --git a/xtask/src/assets.rs b/xtask/src/assets.rs index a7169ee66..da7371b8e 100644 --- a/xtask/src/assets.rs +++ b/xtask/src/assets.rs @@ -61,16 +61,23 @@ pub fn missing(files: &[String], assets_md: &str) -> Vec { files.iter().filter(|f| is_asset(f) && !patterns.iter().any(|p| glob(p, f))).cloned().collect() } -/// `*` matches any run of characters except `/`; everything else literally. +/// `*` matches any run of characters except `/`; everything else literally. Matched with a +/// table over the pattern, so a pattern with many `*` costs pattern length times name length +/// rather than exploring every way to split the name. fn glob(pat: &str, s: &str) -> bool { - match pat.split_once('*') { - None => pat == s, - Some((head, tail)) => { - let Some(rest) = s.strip_prefix(head) else { return false }; - let ends = rest.char_indices().map(|(i, _)| i).chain(std::iter::once(rest.len())); - ends.take_while(|&i| !rest[..i].contains('/')).any(|i| glob(tail, &rest[i..])) + let t: Vec = s.chars().collect(); + // `row[k]`: the pattern so far matches the first k characters of the name. + let mut row = vec![false; t.len() + 1]; + row[0] = true; + for pc in pat.chars() { + let mut next = vec![false; t.len() + 1]; + next[0] = pc == '*' && row[0]; + for k in 1..=t.len() { + next[k] = if pc == '*' { row[k] || (next[k - 1] && t[k - 1] != '/') } else { row[k - 1] && t[k - 1] == pc }; } + row = next; } + row[t.len()] } /// Splits `git ls-files -z` output into real file names (no display quoting of Unicode or special characters). @@ -120,6 +127,15 @@ mod tests { assert_eq!(missing(&["assets/é.png".into()], "`assets/*.svg`"), vec!["assets/é.png".to_string()]); } + #[test] + fn many_wildcards_on_a_failing_name_finish_at_once() { + let pat = format!("{}b", "a*".repeat(40)); + assert!(!glob(&pat, &"a".repeat(60))); + assert!(glob(&pat, &format!("{}b", "a".repeat(60)))); + assert!(!glob("a*b", "a/b")); + assert!(glob("a*b", "ab")); + } + #[test] fn ls_files_output_is_nul_separated_and_unquoted() { let names = parse_ls_files("assets/zażółć.png\0docs/images/a b\nc.png\0".as_bytes());