From c98efffc0638581c2ccfb3a5afde765719fcf7dc Mon Sep 17 00:00:00 2001 From: Martin Ball Date: Thu, 22 Oct 2026 18:09:47 +0100 Subject: [PATCH] #3692: empty polygon groups in metafiles reserve nothing Finding 33. geom::subpaths reserved pts + counts ops up front, so a poly-polygon record with a long list of zero counts reserved room for ops it never emits, and the canvas charged only ops.len() against MAX_BYTES. subpaths now sizes the vector exactly, and Canvas::push charges what each item holds (capacity) for paths, bitmaps and text. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/metafile/src/canvas.rs | 9 ++++++--- crates/metafile/src/geom.rs | 12 +++++++++++- crates/metafile/src/tests.rs | 15 +++++++++++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/metafile/src/canvas.rs b/crates/metafile/src/canvas.rs index 03a8f3453..bb6ecd316 100644 --- a/crates/metafile/src/canvas.rs +++ b/crates/metafile/src/canvas.rs @@ -284,9 +284,12 @@ impl Canvas { fn push(&mut self, raw: Raw) { let cost = match &raw { - Raw::Path { ops, .. } => ops.len() * std::mem::size_of::(), - Raw::Bitmap { img, .. } => img.px.len(), - Raw::Text(t) => t.text.len() + t.font.len() + t.dx.len() * std::mem::size_of::(), + // What is held, capacity and all. + Raw::Path { ops, .. } => ops.capacity().saturating_mul(std::mem::size_of::()), + Raw::Bitmap { img, .. } => img.px.capacity(), + Raw::Text(t) => { + t.text.capacity().saturating_add(t.font.capacity()).saturating_add(t.dx.capacity().saturating_mul(std::mem::size_of::())) + } }; if self.items.len() >= MAX_ITEMS || self.used.saturating_add(cost) > MAX_BYTES { self.full = true; diff --git a/crates/metafile/src/geom.rs b/crates/metafile/src/geom.rs index 0547de18b..0c2b772cf 100644 --- a/crates/metafile/src/geom.rs +++ b/crates/metafile/src/geom.rs @@ -99,7 +99,17 @@ fn arc(cx: f64, cy: f64, rx: f64, ry: f64, a0: f64, sweep: f64) -> Vec { /// Consecutive subpaths: `counts[i]` points each, taken in order from `pts`. Counts that run past the /// point list are cut short rather than failing the whole record. pub(crate) fn subpaths(pts: &[P], counts: &[usize], close: bool) -> Vec { - let mut ops = Vec::with_capacity(pts.len() + counts.len()); + // The exact size first: empty groups add no ops, so a long count list of zeros (file data) + // mustn't reserve room the canvas budget never sees. + let (mut n, mut at) = (0usize, 0usize); + for &c in counts { + let end = at.saturating_add(c).min(pts.len()); + if end > at { + n = n.saturating_add(end - at + usize::from(close)); + } + at = end; + } + let mut ops = Vec::with_capacity(n); let mut at = 0usize; for &c in counts { let end = at.saturating_add(c).min(pts.len()); diff --git a/crates/metafile/src/tests.rs b/crates/metafile/src/tests.rs index eaa5b1674..a71481ffa 100644 --- a/crates/metafile/src/tests.rs +++ b/crates/metafile/src/tests.rs @@ -1053,3 +1053,18 @@ fn plain_wmf_target_comes_from_a_window_that_was_read() { let (x0, y0, x1, y1) = rect_extent(&p.items[0]); assert!(close(x0, 0.0) && close(y0, 0.0) && close(x1, 200.0) && close(y1, 100.0), "{x0} {y0} {x1} {y1}"); } + +/// A poly-polygon's group counts are file data: a long list of empty groups adds no ops, so it +/// mustn't reserve room for them either (the canvas charges what a path holds, capacity included). +#[test] +fn empty_poly_groups_reserve_nothing() { + let pts = [(0.0, 0.0), (10.0, 0.0), (10.0, 10.0)]; + let mut counts = vec![0usize; 100_000]; + counts.push(3); + let ops = crate::geom::subpaths(&pts, &counts, true); + assert_eq!(ops.len(), 4, "M L L Z"); + assert!(ops.capacity() <= 8, "capacity {}", ops.capacity()); + // Counts past the points are cut short, and the size still matches. + let ops = crate::geom::subpaths(&pts, &[2, 5, 4], false); + assert_eq!((ops.len(), ops.capacity()), (3, 3)); +}