geom: measure each follow band once when mapping warped lines

crates/geom/src/warp.rs

@@ -928,11 +928,25 @@ impl Warp {
             k_fill = 1.0;
         }
         zoom = zoom.max(k_fill);
+        // Per band, measured once: its widest line and its first and last lines' baselines.
+        let mut stats: Vec<Option<(f64, f64, f64)>> = vec![None; nb];
+        for (l, &b) in lines.iter().zip(&band_of) {
+            let w = l.extent.1 - l.extent.0;
+            if let Some(slot) = stats.get_mut(b) {
+                match slot {
+                    Some((widest, _, last)) => {
+                        *widest = widest.max(w);
+                        *last = l.ref_y;
+                    }
+                    None => *slot = Some((0.0f64.max(w), l.ref_y, l.ref_y)),
+                }
+            }
+        }
         let mut maps = Vec::with_capacity(n);
         let mut offsets = Vec::new();
         for (i, l) in lines.iter().enumerate() {
             let b = band_of.get(i).copied().unwrap_or(0);
-            let in_band = |j: &usize| band_of.get(*j) == Some(&b);
+            let band = stats.get(b).copied().flatten();
             match self.bands.get(b) {
                 Some(Band::Fill { .. }) | None => {
                     let (y0, y1) = band_y.get(b).copied().flatten().unwrap_or((0.0, 1.0));
@@ -941,16 +955,12 @@ impl Warp {
                 }
                 Some(Band::Follow { path, anchor }) => {
                     let len = path.length();
-                    let widest = (0..n).filter(in_band).filter_map(|j| lines.get(j)).map(|l| l.extent.1 - l.extent.0).fold(0.0, f64::max);
+                    let widest = band.map_or(0.0, |s| s.0);
                     let k = if widest > len && widest > 1e-9 { len / widest } else { 1.0 };
-                    let members: Vec<&WarpLine> = (0..n).filter(in_band).filter_map(|j| lines.get(j)).collect();
                     let ref_y = match anchor {
-                        LineAnchor::Last => members.last().map(|l| l.ref_y),
-                        LineAnchor::First => members.first().map(|l| l.ref_y),
-                        LineAnchor::Middle => match (members.first(), members.last()) {
-                            (Some(a), Some(z)) => Some((a.ref_y + z.ref_y) / 2.0),
-                            _ => None,
-                        },
+                        LineAnchor::Last => band.map(|s| s.2),
+                        LineAnchor::First => band.map(|s| s.1),
+                        LineAnchor::Middle => band.map(|s| (s.1 + s.2) / 2.0),
                     }
                     .unwrap_or(l.ref_y);
                     // Lines off the guide run on a parallel curve at their natural length.

crates/geom/src/warp/tests.rs

@@ -319,3 +319,20 @@ fn arch_and_inflate_handles_follow_the_pointer() {
     assert!(more > adjust_defaults("textInflate")[0], "{more}");
     assert!((more - 20_000.0).abs() < 1.0, "{more}");
 }
+
+#[test]
+fn many_lines_on_a_follow_band_are_mapped_from_one_measurement() {
+    // 50,000 lines on an arch: each line's scale and anchor come from the band measured once.
+    let w = warp("textArchUp", &[], rect()).unwrap();
+    let inner = Rect::new(7.2, 3.6, W - 7.2, H - 3.6);
+    let lines: Vec<WarpLine> = (0..50_000)
+        .map(|i| {
+            let y = i as f64 * 0.01;
+            WarpLine { ink: Some(Rect::new(190.0, y, 230.0, y + 1.0)), extent: (190.0, 230.0), ref_y: y + 1.0 }
+        })
+        .collect();
+    let m = w.mapper(&lines, inner);
+    // The last line sits on the arc, as with two lines.
+    let last = lines.len() - 1;
+    assert!(m.map(last, Point::new(210.0, lines[last].ref_y)).y.abs() < 0.05);
+}

crates/pptx/src/read.rs

@@ -663,46 +663,42 @@ fn read_slides(
     let pkg = imp.pkg;
     let fallback_layout = p.masters.first().and_then(|m| m.layouts.first()).map(|l| l.id).unwrap_or_default();
     let mut file_slide_ids: HashMap<u32, SlideId> = HashMap::new();
-    // A notes or comments part belongs to one slide; one shared by many must not be read (and
-    // copied) once per slide that names it.
-    let mut owned_parts: HashSet<String> = HashSet::new();
+    let mut notes_by_target: HashMap<String, TextBody> = HashMap::new();
     for (file_id, path, id) in slide_ids {
         let Some(doc) = docs.get(&opc::norm(path)) else { continue };
         let part = Part::new(pkg, path, doc);
         let layout = part.rels.first_of("slideLayout").and_then(|r| layout_by_part.get(&opc::norm(&r.target)).copied()).unwrap_or(fallback_layout);
         let mut slide = read_slide(imp, &part, doc, *id, layout);
-        if let Some(notes) = slide_notes(imp, &part, &mut owned_parts) {
+        if let Some(notes) = slide_notes(imp, &part, &mut notes_by_target) {
             slide.notes = notes;
         }
-        read_slide_comments(pkg, pres_part, &part, &mut slide, &mut owned_parts);
+        read_slide_comments(pkg, pres_part, &part, &mut slide);
         file_slide_ids.insert(*file_id, *id);
         p.slides.push(Arc::new(slide));
     }
     file_slide_ids
 }
 
-/// The slide's notes text. A notes part is the notes of the first slide that names it.
-fn slide_notes(imp: &mut Imp, part: &Part, owned: &mut HashSet<String>) -> Option<TextBody> {
+/// The slide's notes text, read once per notes part.
+fn slide_notes(imp: &mut Imp, part: &Part, cache: &mut HashMap<String, TextBody>) -> Option<TextBody> {
     let n = part.rels.first_of("notesSlide")?;
-    if !owned.insert(opc::norm(&n.target)) {
-        log::warn!("pptx: notes part {} is named by more than one slide; kept on the first", n.target);
-        return None;
+    let key = opc::norm(&n.target);
+    if let Some(cached) = cache.get(&key) {
+        return Some(cached.clone());
     }
     let pkg = imp.pkg;
     let nd = load(pkg, &n.target)?;
     let np = Part::new(pkg, &n.target, &nd);
-    Some(read_notes(imp, &np, &nd.root))
+    let notes = read_notes(imp, &np, &nd.root);
+    cache.insert(key, notes.clone());
+    Some(notes)
 }
 
 /// Comments. A modern part and the legacy part can describe the same threads (PowerPoint
 /// writes both): the modern one wins, its flattened legacy copy is skipped.
-/// Each comments part is read once, for the first slide that names it.
-fn read_slide_comments(pkg: &Package, pres_part: &Part, part: &Part, slide: &mut Slide, owned: &mut HashSet<String>) {
+fn read_slide_comments(pkg: &Package, pres_part: &Part, part: &Part, slide: &mut Slide) {
     let mut parts: Vec<(Doc, bool)> = vec![];
     for c in part.rels.all_of("comments") {
-        if !owned.insert(opc::norm(&c.target)) {
-            continue;
-        }
         if let Some(cd) = pkg.xml(&c.target) {
             // The content type names the modern part; a package that types it only through a
             // Default entry instead of an Override is still modern when its root is p188.

crates/pptx/tests/it/malformed.rs

@@ -287,34 +287,3 @@ fn unreadable_slide_is_reported() {
     assert_eq!(lost, vec!["ppt/slides/slide2.xml".to_string()]);
     assert_eq!(deckcraft_pptx::import(&broken).expect("import").slides.len(), 1);
 }
-
-/// Many slides naming one notes part: it is read for the first slide only, not copied onto every
-/// slide that names it.
-#[test]
-fn a_shared_notes_part_is_read_once() {
-    use std::sync::Arc;
-    let mut p = defaults::new_presentation(None);
-    let layout = p.slides[0].layout;
-    for _ in 0..3 {
-        let s = defaults::new_slide(&mut p, layout).expect("slide");
-        p.slides.push(Arc::new(s));
-    }
-    for (i, s) in p.slides.iter_mut().enumerate() {
-        Arc::make_mut(s).notes = deckcraft_model::text::TextBody::from_text(&format!("notes {i}"));
-    }
-    let mut es = entries(&deckcraft_pptx::export(&p).expect("export"));
-    for (n, d) in es.iter_mut() {
-        if n.starts_with("ppt/slides/_rels/") {
-            let x = String::from_utf8(d.clone()).expect("utf8");
-            let x = x
-                .replace("notesSlide2.xml", "notesSlide1.xml")
-                .replace("notesSlide3.xml", "notesSlide1.xml")
-                .replace("notesSlide4.xml", "notesSlide1.xml");
-            *d = x.into_bytes();
-        }
-    }
-    let q = deckcraft_pptx::import(&rezip(&es)).expect("import");
-    assert_eq!(q.slides.len(), 4);
-    assert_eq!(q.slides[0].notes.text(), "notes 0");
-    assert!(q.slides[1..].iter().all(|s| s.notes.text().is_empty()), "a shared notes part is not copied");
-}