diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 907aa3364..073423629 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,6 +26,10 @@ jobs: # Same craft-fonts commit as release.yml. CRAFT_FONTS_REF: 37827adc41fd165f9a9234ac40f94dbad353aa69 steps: + - name: Harden runner + uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4 + with: + egress-policy: audit - name: Check out craft-fonts uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: diff --git a/.github/workflows/packaging-lint.yml b/.github/workflows/packaging-lint.yml index ea61d8430..1771a35fe 100644 --- a/.github/workflows/packaging-lint.yml +++ b/.github/workflows/packaging-lint.yml @@ -27,6 +27,10 @@ jobs: name: Windows installation policy (no app build) runs-on: windows-latest steps: + - name: Harden runner + uses: step-security/harden-runner@1d88384fd173af6ecb012dfd0264a306b711241a # v2.22.4 + with: + egress-policy: audit - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false diff --git a/Cargo.lock b/Cargo.lock index 4a32c8257..4cf447655 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -847,12 +847,12 @@ dependencies = [ [[package]] name = "craft-fonts-build" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" [[package]] name = "craft-i18n" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "fixed_decimal", "icu_decimal", @@ -875,7 +875,7 @@ dependencies = [ [[package]] name = "craft-media" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "thiserror 2.0.21", ] @@ -883,7 +883,7 @@ dependencies = [ [[package]] name = "craft-pdf" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "krilla", "thiserror 2.0.21", @@ -892,7 +892,7 @@ dependencies = [ [[package]] name = "craft-ui" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "craft-fonts-build", "craft-i18n", @@ -912,7 +912,7 @@ dependencies = [ [[package]] name = "craft-update" version = "0.1.0" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "chrono", "dunce", @@ -922,6 +922,7 @@ dependencies = [ "image", "log", "plist", + "rustix 1.1.5", "semver", "serde", "serde_json", @@ -1028,7 +1029,7 @@ checksum = "0c87e182de0887fd5361989c677c4e8f5000cd9491d6d563161a8f3a5519fc7f" [[package]] name = "deckcraft" -version = "0.15.0" +version = "0.18.0" dependencies = [ "arboard", "chrono", @@ -1050,7 +1051,7 @@ dependencies = [ [[package]] name = "deckcraft-anim" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-geom", "deckcraft-model", @@ -1059,7 +1060,7 @@ dependencies = [ [[package]] name = "deckcraft-av1" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1068,14 +1069,14 @@ dependencies = [ [[package]] name = "deckcraft-bitstream" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-cli" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-engine", "deckcraft-mcp", @@ -1085,14 +1086,14 @@ dependencies = [ [[package]] name = "deckcraft-color" -version = "0.15.0" +version = "0.18.0" dependencies = [ "serde", ] [[package]] name = "deckcraft-engine" -version = "0.15.0" +version = "0.18.0" dependencies = [ "chrono", "deckcraft-anim", @@ -1118,7 +1119,7 @@ dependencies = [ [[package]] name = "deckcraft-fonts" -version = "0.15.0" +version = "0.18.0" dependencies = [ "epaint_default_fonts", "harfrust 0.13.3", @@ -1130,7 +1131,7 @@ dependencies = [ [[package]] name = "deckcraft-format" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-model", "serde", @@ -1141,7 +1142,7 @@ dependencies = [ [[package]] name = "deckcraft-geom" -version = "0.15.0" +version = "0.18.0" dependencies = [ "kurbo", "serde", @@ -1149,7 +1150,7 @@ dependencies = [ [[package]] name = "deckcraft-h264" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1158,7 +1159,7 @@ dependencies = [ [[package]] name = "deckcraft-hevc" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1167,21 +1168,21 @@ dependencies = [ [[package]] name = "deckcraft-isobmff" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-matroska" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-media", ] [[package]] name = "deckcraft-mcp" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-engine", "deckcraft-render", @@ -1191,7 +1192,7 @@ dependencies = [ [[package]] name = "deckcraft-media" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-av1", "deckcraft-h264", @@ -1209,7 +1210,7 @@ dependencies = [ [[package]] name = "deckcraft-metafile" -version = "0.15.0" +version = "0.18.0" dependencies = [ "image", "kurbo", @@ -1218,7 +1219,7 @@ dependencies = [ [[package]] name = "deckcraft-model" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-color", "deckcraft-geom", @@ -1230,15 +1231,15 @@ dependencies = [ [[package]] name = "deckcraft-ogg" -version = "0.15.0" +version = "0.18.0" [[package]] name = "deckcraft-opus" -version = "0.15.0" +version = "0.18.0" [[package]] name = "deckcraft-pdf" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-pdf", "deckcraft-fonts", @@ -1256,7 +1257,7 @@ dependencies = [ [[package]] name = "deckcraft-pptx" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-color", "deckcraft-engine", @@ -1272,7 +1273,7 @@ dependencies = [ [[package]] name = "deckcraft-render" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-anim", "deckcraft-color", @@ -1291,7 +1292,7 @@ dependencies = [ [[package]] name = "deckcraft-text" -version = "0.15.0" +version = "0.18.0" dependencies = [ "chrono", "deckcraft-color", @@ -1305,7 +1306,7 @@ dependencies = [ [[package]] name = "deckcraft-ui-egui" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-ui", "craft-update", @@ -1331,7 +1332,7 @@ dependencies = [ [[package]] name = "deckcraft-vp9" -version = "0.15.0" +version = "0.18.0" dependencies = [ "deckcraft-bitstream", "rayon", @@ -1340,7 +1341,7 @@ dependencies = [ [[package]] name = "deckcraft-web" -version = "0.15.0" +version = "0.18.0" dependencies = [ "craft-web-api", "craft-web-host", @@ -2791,7 +2792,7 @@ dependencies = [ [[package]] name = "krilla" version = "0.8.2" -source = "git+https://github.com/storytold/craft-libs?rev=1b34eba41e62138fc2438d2734523b302840b45d#1b34eba41e62138fc2438d2734523b302840b45d" +source = "git+https://github.com/storytold/craft-libs?rev=6e03f65bf45181db77ba6229716e6624db9eb25b#6e03f65bf45181db77ba6229716e6624db9eb25b" dependencies = [ "base64 0.22.1", "bumpalo", @@ -6325,7 +6326,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee" [[package]] name = "xtask" -version = "0.15.0" +version = "0.18.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index e11a5bbd5..c12ae375f 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.15.0" +version = "0.18.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" @@ -59,11 +59,11 @@ harfrust = "0.13" vello_cpu = "0.2" vello_common = { version = "0.2", default-features = false } image = { version = "0.25", default-features = false, features = ["png", "jpeg", "webp", "gif", "tiff", "bmp"] } -craft-media = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", version = "=0.1.0", default-features = false } -craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", version = "=0.1.0", default-features = false, features = ["write", "write-simple-text"] } +craft-media = { git = "https://github.com/storytold/craft-libs", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", version = "=0.1.0", default-features = false } +craft-pdf = { git = "https://github.com/storytold/craft-libs", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", version = "=0.1.0", default-features = false, features = ["write", "write-simple-text"] } # craft-update: app-specific pretend versions apply to startup and manual checks. -craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "1b34eba41e62138fc2438d2734523b302840b45d", features = ["egui"] } -craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", version = "=0.1.0" } +craft-update = { git = "https://github.com/storytold/craft-libs", version = "=0.1.0", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", features = ["egui"] } +craft-ui = { git = "https://github.com/storytold/craft-libs", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", version = "=0.1.0" } i_overlay = "9" quick-xml = "0.41" zip = { version = "3", default-features = false, features = ["deflate"] } @@ -109,7 +109,7 @@ codegen-units = 16 debug = 0 [patch.crates-io] -krilla = { git = "https://github.com/storytold/craft-libs", rev = "1b34eba41e62138fc2438d2734523b302840b45d", version = "=0.8.2" } +krilla = { git = "https://github.com/storytold/craft-libs", rev = "6e03f65bf45181db77ba6229716e6624db9eb25b", version = "=0.8.2" } ecolor = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } emath = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } epaint = { git = "https://github.com/storytold/egui.git", rev = "6d518fc799681571963842f95b55a6b68431b15a" } 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/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 ee76ff89c..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)), @@ -582,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(); @@ -592,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 @@ -903,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] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8b1841675..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] = &[ @@ -6902,6 +6921,18 @@ 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. 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()); diff --git a/xtask/src/pdf_deps.rs b/xtask/src/pdf_deps.rs index fb8e79d9a..314db1e54 100644 --- a/xtask/src/pdf_deps.rs +++ b/xtask/src/pdf_deps.rs @@ -7,7 +7,7 @@ use std::process::Command; use serde_json::Value; const SHARED_REPO: &str = "https://github.com/storytold/craft-libs"; -const SHARED_REV: &str = "1b34eba41e62138fc2438d2734523b302840b45d"; +const SHARED_REV: &str = "6e03f65bf45181db77ba6229716e6624db9eb25b"; const REGISTRY: &str = "registry+https://github.com/rust-lang/crates.io-index"; pub fn run(root: &Path) -> Result<(), String> {