Merge branch 'main' into fix/sec-xlsx-richdata-index

wrap_lines split an over-long word by re-measuring and copying the
rest of the word for every piece it cut, and only checked its line
limit once the word was done, so one long word in a narrow text box
cost word length x pieces. Each new word also re-measured the whole
line built so far.

Line and piece widths are now running sums of glyph widths (the same
sum text_width makes), so every character is measured once, and the
line limit is checked while a word is being split. The lines produced
are unchanged; a test compares them with the old wrapper.

crates/core/src/addr.rs

@@ -147,10 +147,10 @@ impl RangeRef {
         RangeRef::new(CellRef::new(0, 0), CellRef::new(MAX_ROWS - 1, MAX_COLS - 1))
     }
     pub fn height(&self) -> u32 {
-        self.end.row.saturating_sub(self.start.row).saturating_add(1)
+        self.end.row - self.start.row + 1
     }
     pub fn width(&self) -> u32 {
-        self.end.col.saturating_sub(self.start.col).saturating_add(1)
+        self.end.col - self.start.col + 1
     }
     pub fn count(&self) -> u64 {
         self.height() as u64 * self.width() as u64
@@ -300,13 +300,6 @@ pub fn split_sheet(reference: &str) -> (Option<String>, &str) {
 mod tests {
     use super::*;
 
-    #[test]
-    fn hostile_range_dimensions_saturate_instead_of_wrapping() {
-        let r = RangeRef { start: CellRef::new(0, 0), end: CellRef::new(u32::MAX, u32::MAX) };
-        assert_eq!((r.height(), r.width()), (u32::MAX, u32::MAX));
-        assert_eq!(r.count(), u64::from(u32::MAX) * u64::from(u32::MAX));
-    }
-
     #[test]
     fn range_parse_rejects_signed_rows() -> Result<(), std::io::Error> {
         assert!(RangeRef::parse("+1:+3").is_none());

crates/engine/src/cmd/data/filter.rs

@@ -184,8 +184,7 @@ pub(crate) fn grow_autofilter(sh: &mut Sheet, at: CellRef) {
 pub(crate) fn apply_filter(wb: &mut Workbook, sheet: usize) {
     let Some(sh) = wb.sheet(sheet) else { return };
     let Some(af) = sh.autofilter.clone() else { return };
-    // An imported range can name rows past the grid; none of those exist to hide.
-    let r = RangeRef::new(af.range.start, CellRef::new(af.range.end.row.min(gridcraft_core::MAX_ROWS - 1), af.range.end.col));
+    let r = af.range;
     let mut hide = Vec::new();
     // Precompute per-criterion top10 threshold and average once, not per row.
     let mut col_stats: std::collections::HashMap<usize, (Option<f64>, f64)> = std::collections::HashMap::new();

crates/engine/src/cmd/data/tools.rs

@@ -5,9 +5,6 @@ use super::{
 
 // ---------------------------------------------------------------- tools
 
-/// Most cells (rows times compared columns) one Remove Duplicates call reads.
-const MAX_COMPARED_CELLS: u64 = 50_000_000;
-
 pub(super) fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
     let (sheet, r) = data_target(s, p, "data.removeDuplicates")?;
     let d = s.doc()?;
@@ -27,10 +24,6 @@ pub(super) fn remove_duplicates(s: &mut Session, p: &Json) -> Result<Json> {
     if cols.is_empty() || cols.iter().any(|c| !(r.start.col..=r.end.col).contains(c)) {
         return Err(bad("data.removeDuplicates", "`columns` must name at least one column of the range"));
     }
-    // Rows are compared one by one, so a sparse sheet's enclosing rectangle is bounded like a list.
-    if r.count() > MAX_COMPARED_CELLS {
-        return Err(bad("data.removeDuplicates", "the range is too large"));
-    }
     let start = r.start.row + header as u32;
     let mut removed = 0;
     edit(s, |cx| {

crates/engine/src/cmd/data/validation.rs

@@ -289,14 +289,11 @@ pub(super) fn circle_invalid(s: &mut Session, _: &Json) -> Result<Json> {
     let sheet = d.wb.active_sheet();
     let Some(sh) = d.wb.sheet(sheet) else { return Err(EngineError::NoDocument) };
     let mut bad_cells = Vec::new();
-    // One scan allowance for the whole command: duplicate or overlapping ranges don't multiply it.
-    let mut budget = 100_000usize;
     for dv in &sh.validations {
         for r in &dv.ranges {
             let r = sh.used_range().and_then(|u| r.intersection(&u));
             let Some(r) = r else { continue };
-            for c in r.iter().take(budget) {
-                budget -= 1;
+            for c in r.iter().take(100_000) {
                 let text = sh.input_text(c);
                 if !text.is_empty() && check_validation(&d.wb, sheet, c, &text).is_some() {
                     bad_cells.push(c.a1());

crates/engine/src/cmd/edit/find.rs

@@ -130,11 +130,7 @@ pub(super) fn replace(s: &mut Session, p: &Json) -> Result<Json> {
         let mut n = 0;
         for si in sheets {
             let cells: Vec<(CellRef, String)> =
-                // A hidden formula's source is not searched: a count or error would reveal its text.
-                cx.wb
-                    .sheet(si)
-                    .map(|sh| sh.cells.iter().filter(|(c, _)| !crate::display::formula_hidden(&cx.wb, sh, *c)).map(|(c, _)| (c, sh.input_text(c))).collect())
-                    .unwrap_or_default();
+                cx.wb.sheet(si).map(|sh| sh.cells.iter().map(|(c, _)| (c, sh.input_text(c))).collect()).unwrap_or_default();
             for (c, text) in cells {
                 if (!all && c != active) || selection.as_ref().is_some_and(|sel| !sel.contains(c)) {
                     continue;

crates/engine/src/cmd/format/sizes.rs

@@ -140,44 +140,46 @@ pub(super) fn autofit_cols(s: &mut Session, p: &Json) -> Result<Json> {
     let measure = s.text_width.clone();
     edit(s, |cx| {
         let Some(sh) = cx.wb.sheet(si) else { return Ok(Json::Null) };
+        let mut widths = Vec::new();
         let mut measured: std::collections::HashMap<StyleId, std::collections::HashMap<String, f32>> = Default::default();
         let mut distinct = 0usize;
-        // Selected columns, then one pass over the populated cells (the store is row-oriented, so a
-        // scan per column would revisit every row for each selected column, empty or not).
-        let cols: std::collections::BTreeSet<u32> = lines.iter().flat_map(|&(a, b)| a..=b.min(a + 16383)).collect();
-        let mut maxes: std::collections::BTreeMap<u32, f32> = Default::default();
-        if let (Some(u), Some(&first), Some(&last)) = (sh.used_range(), cols.first(), cols.last()) {
-            let span = RangeRef::new(CellRef::new(u.start.row, first), CellRef::new(u.end.row, last));
-            for (cell, _) in sh.cells.iter_range(span) {
-                if !cols.contains(&cell.col) || sh.merge_at(cell).is_some_and(|m| !m.is_single()) {
-                    continue;
-                }
-                let id = sh.style_id(cell);
-                let st = cx.wb.styles.get(id);
-                let text = crate::display::cell_text(&cx.wb, sh, cell);
-                for line in text.lines() {
-                    let w = match &measure {
-                        Some(m) => {
-                            let known = measured.entry(id).or_default();
-                            match known.get(line) {
-                                Some(w) => *w,
-                                None => {
-                                    let w = m(line, &st.font, distinct < EXACT_TEXT_WIDTHS);
-                                    known.insert(line.to_owned(), w);
-                                    distinct += 1;
-                                    w
+        let used = sh.used_range();
+        for (a, b) in lines {
+            for c in a..=b.min(a + 16383) {
+                let Some(u) = used else { continue };
+                let col = RangeRef::new(CellRef::new(u.start.row, c), CellRef::new(u.end.row, c));
+                let mut max: f32 = 0.0;
+                for (cell, _) in sh.cells.iter_range(col) {
+                    if sh.merge_at(cell).is_some_and(|m| !m.is_single()) {
+                        continue;
+                    }
+                    let id = sh.style_id(cell);
+                    let st = cx.wb.styles.get(id);
+                    let text = crate::display::cell_text(&cx.wb, sh, cell);
+                    for line in text.lines() {
+                        let w = match &measure {
+                            Some(m) => {
+                                let known = measured.entry(id).or_default();
+                                match known.get(line) {
+                                    Some(w) => *w,
+                                    None => {
+                                        let w = m(line, &st.font, distinct < EXACT_TEXT_WIDTHS);
+                                        known.insert(line.to_owned(), w);
+                                        distinct += 1;
+                                        w
+                                    }
                                 }
                             }
-                        }
-                        None => approx_text_width(line, st.font.size, st.font.bold),
-                    };
-                    let w = w + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell);
-                    let max = maxes.entry(cell.col).or_insert(0.0);
-                    *max = max.max(w);
+                            None => approx_text_width(line, st.font.size, st.font.bold),
+                        };
+                        max = max.max(w + 6.0 + st.align.indent as f32 * 9.0 + sh.filter_button_space(cell));
+                    }
+                }
+                if max > 0.0 {
+                    widths.push((c, max.clamp(8.0, 1785.0)));
                 }
             }
         }
-        let widths: Vec<(u32, f32)> = maxes.into_iter().filter(|&(_, m)| m > 0.0).map(|(c, m)| (c, m.clamp(8.0, 1785.0))).collect();
         let sh = cx.sheet_mut(si)?;
         for (c, w) in widths {
             let e = sh.cols.entry(c).or_default();

crates/engine/src/cmd/print/tests.rs

@@ -695,6 +695,79 @@ fn bands_unit() {
     assert!(w.len() > 2);
 }
 
+/// The wrapper `wrap_lines` replaced, re-measuring strings, as the reference for its lines.
+fn wrap_lines_remeasuring(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
+    let mut out = Vec::new();
+    for para in text.split('\n') {
+        let mut line = String::new();
+        for word in para.split(' ') {
+            let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
+            if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
+                line = cand;
+            } else {
+                out.push(std::mem::take(&mut line));
+                line = word.to_string();
+            }
+            while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
+                let chars: Vec<char> = line.chars().collect();
+                let (mut lo, mut hi) = (1usize, chars.len());
+                while lo < hi {
+                    let mid = lo + (hi - lo).div_ceil(2);
+                    let cut: String = chars[..mid].iter().collect();
+                    if gridcraft_pdf::text_width(font, size, &cut) <= width {
+                        lo = mid;
+                    } else {
+                        hi = mid - 1;
+                    }
+                }
+                if lo == chars.len() {
+                    break;
+                }
+                out.push(chars[..lo].iter().collect());
+                line = chars[lo..].iter().collect();
+            }
+            if out.len() > 2000 {
+                return out;
+            }
+        }
+        out.push(line);
+    }
+    out
+}
+
+#[test]
+fn wrap_lines_matches_the_remeasuring_wrapper() {
+    let texts = [
+        "the quick brown fox jumps",
+        "Supercalifragilistic",
+        "a  b   c\n\nverylongwordthatneedsbreaking short\nend ",
+        "Wide WWWWWWWWWW words, iiiiiiii narrow ones; mixed MMMMiiiiMMMM",
+        "€ ü ß — unicode Ünïcödé text",
+        "",
+        " ",
+    ];
+    for text in texts {
+        for font in [Font::Helvetica, Font::HelveticaBold, Font::Courier] {
+            for (size, width) in [(10.0, 50.0), (10.0, 20.0), (11.0, 1.0), (8.0, 300.0), (12.0, 0.0)] {
+                assert_eq!(wrap_lines(text, font, size, width), wrap_lines_remeasuring(text, font, size, width), "{text:?} {font:?} {size} {width}");
+            }
+        }
+    }
+}
+
+#[test]
+fn wrapping_a_long_word_in_a_narrow_box_stops_at_the_line_limit() {
+    // One character per line: the old wrapper re-measured and copied the rest of the word for
+    // each of the 100,000 lines before checking the limit.
+    let word = "w".repeat(100_000);
+    let lines = wrap_lines(&word, Font::Helvetica, 10.0, 1.0);
+    assert!(lines.len() <= 2002, "{}", lines.len());
+    assert!(lines.iter().all(|l| l == "w"));
+    // A long paragraph in a very wide box is one line.
+    let para = vec!["word"; 50_000].join(" ");
+    assert_eq!(wrap_lines(&para, Font::Helvetica, 10.0, 1.0e9), vec![para.clone()]);
+}
+
 #[test]
 fn center_across_spans_following_empty_center_across_cells() {
     let mut styles = StyleTable::default();

crates/engine/src/cmd/print/text.rs

@@ -3,42 +3,57 @@ use super::{CellRef, Color, Font, HAlign, MAX_COLS, Page, Point, RangeRef, Sheet
 
 const PAD_PX: f32 = 2.0;
 
+/// Most lines [`wrap_lines`] returns (it may return one more, the rest of a paragraph).
+const MAX_WRAP_LINES: usize = 2000;
+
 /// Word-wraps `text` to `width` points.
+///
+/// Widths are kept as running sums of glyph widths (the sum `text_width` makes), so each
+/// character is measured once: re-measuring the line for every word, or the rest of an
+/// over-long word for every piece cut from it, made a long paragraph or word quadratic.
 pub(super) fn wrap_lines(text: &str, font: Font, size: f32, width: f32) -> Vec<String> {
+    let units_of = |s: &str| -> u64 { s.chars().map(|c| u64::from(font.char_width(c))).sum() };
+    let fits = |units: u64| units as f32 * size / 1000.0 <= width;
+    let space = units_of(" ");
     let mut out = Vec::new();
     for para in text.split('\n') {
         let mut line = String::new();
+        let mut line_units: u64 = 0;
         for word in para.split(' ') {
-            let cand = if line.is_empty() { word.to_string() } else { format!("{line} {word}") };
-            if line.is_empty() || gridcraft_pdf::text_width(font, size, &cand) <= width {
-                line = cand;
+            let word_units = units_of(word);
+            if line.is_empty() {
+                line.push_str(word);
+                line_units = word_units;
+            } else if fits(line_units + space + word_units) {
+                line.push(' ');
+                line.push_str(word);
+                line_units += space + word_units;
             } else {
                 out.push(std::mem::take(&mut line));
-                line = word.to_string();
+                line.push_str(word);
+                line_units = word_units;
             }
-            // Break a single over-long word by characters. The cut that fits is found by binary
-            // search (each try measures one candidate prefix), so an over-long word costs
-            // O(n log n) measurements instead of re-measuring the whole cut per character for
-            // every emitted line (`a` × 100k pasted text would be cubic worst case).
-            while gridcraft_pdf::text_width(font, size, &line) > width && line.chars().count() > 1 {
-                let chars: Vec<char> = line.chars().collect();
-                let (mut lo, mut hi) = (1usize, chars.len());
-                while lo < hi {
-                    let mid = lo + (hi - lo).div_ceil(2);
-                    let cut: String = chars[..mid].iter().collect();
-                    if gridcraft_pdf::text_width(font, size, &cut) <= width {
-                        lo = mid;
-                    } else {
-                        hi = mid - 1;
+            // Break a single over-long word by characters: each piece is the longest that fits
+            // (at least one character).
+            if !fits(line_units) && line.chars().nth(1).is_some() {
+                let mut piece = String::new();
+                let mut units: u64 = 0;
+                for c in line.chars() {
+                    let w = u64::from(font.char_width(c));
+                    if !piece.is_empty() && !fits(units + w) {
+                        out.push(std::mem::take(&mut piece));
+                        units = 0;
+                        if out.len() > MAX_WRAP_LINES {
+                            return out;
+                        }
                     }
+                    piece.push(c);
+                    units += w;
                 }
-                if lo == chars.len() {
-                    break;
-                }
-                out.push(chars[..lo].iter().collect());
-                line = chars[lo..].iter().collect();
+                line = piece;
+                line_units = units;
             }
-            if out.len() > 2000 {
+            if out.len() > MAX_WRAP_LINES {
                 return out;
             }
         }

crates/engine/src/cmd/whatif/advanced_filter.rs

@@ -83,9 +83,6 @@ pub(crate) fn criterion_matches(v: &Value, text: &str, crit: &Value) -> bool {
     }
 }
 
-/// Most (list row, criteria row) pairs one Advanced Filter tests.
-const MAX_FILTER_TESTS: u64 = 50_000_000;
-
 pub(super) fn advanced_filter(s: &mut Session, p: &Json) -> Result<Json> {
     const CMD: &str = "data.advancedFilter";
     let d = s.doc()?;
@@ -136,10 +133,6 @@ pub(super) fn advanced_filter(s: &mut Session, p: &Json) -> Result<Json> {
             rows.push(conds);
         }
     }
-    // Each list row is tried against every criteria row, so bound their product too.
-    if u64::from(list.height()) * rows.len() as u64 > MAX_FILTER_TESTS {
-        return Err(bad(CMD, "the list and criteria ranges together are too large"));
-    }
     let matches_row = |row: u32| -> bool {
         if rows.is_empty() {
             return true;

crates/engine/src/cmd/whatif/tests.rs

@@ -297,12 +297,3 @@ fn consolidate_by_position_and_label() {
     assert!(s.execute("data.consolidate", json!({"sources": []})).is_err());
     assert!(s.execute("data.consolidate", json!({"sources": ["A1"], "function": "median"})).is_err());
 }
-
-#[test]
-fn advanced_filter_refuses_a_list_times_criteria_product_over_the_work_limit() {
-    let mut s = s();
-    s.execute("range.setValues", json!({"range": "A1", "values": [["Name"], ["A"]]})).unwrap();
-    s.execute("range.setValues", json!({"range": "H1", "values": [["Name"], ["A"]]})).unwrap();
-    // A sparse million-row list against 9,999 blank criteria rows would be ~10 billion tests.
-    assert!(s.execute("data.advancedFilter", json!({"range": "A2:A1048576", "criteria": "H1:H10000"})).is_err());
-}

crates/engine/src/macros/run.rs

@@ -1280,18 +1280,9 @@ fn content_differs(b: Option<&Cell>, a: Option<&Cell>) -> bool {
 /// Objects added, changed and removed between two versions of a sheet.
 fn objects(b: &Sheet, a: &Sheet) -> (usize, usize, usize) {
     fn count<T: PartialEq>(b: &[T], a: &[T], id: impl Fn(&T) -> u32) -> (usize, usize, usize) {
-        // Objects grouped by id, so each lookup is a map probe instead of a scan of the list.
-        fn group<'a, T>(v: &'a [T], id: &impl Fn(&T) -> u32) -> std::collections::HashMap<u32, Vec<&'a T>> {
-            let mut m: std::collections::HashMap<u32, Vec<&T>> = std::collections::HashMap::new();
-            for x in v {
-                m.entry(id(x)).or_default().push(x);
-            }
-            m
-        }
-        let (gb, ga) = (group(b, &id), group(a, &id));
-        let added = a.iter().filter(|x| !gb.contains_key(&id(x))).count();
-        let removed = b.iter().filter(|x| !ga.contains_key(&id(x))).count();
-        let changed = b.iter().filter(|x| ga.get(&id(x)).is_some_and(|ys| ys.iter().any(|y| *y != *x))).count();
+        let added = a.iter().filter(|x| !b.iter().any(|y| id(y) == id(x))).count();
+        let removed = b.iter().filter(|x| !a.iter().any(|y| id(y) == id(x))).count();
+        let changed = b.iter().filter(|x| a.iter().any(|y| id(y) == id(x) && y != *x)).count();
         (added, changed, removed)
     }
     let parts = [count(&b.charts, &a.charts, |o| o.id), count(&b.images, &a.images, |o| o.id), count(&b.shapes, &a.shapes, |o| o.id)];

crates/engine/src/tests.rs

@@ -5572,14 +5572,3 @@ fn clearing_links_and_notes_keeps_allow_edit_ranges() {
     let sh = s.doc().unwrap().wb.active().unwrap().clone();
     assert!(sh.hyperlinks.is_empty() && sh.comments.is_empty());
 }
-
-#[test]
-fn replace_does_not_search_hidden_formula_source() {
-    let mut s = s();
-    s.execute("cell.set", json!({"cell": "A1", "input": "=SEQUENCE(3)*7"})).unwrap();
-    s.execute("home.formatCells", json!({"range": "A1", "style": {"protection": {"locked": true, "hidden": true}}})).unwrap();
-    s.execute("review.protectSheet", json!({})).unwrap();
-    let hit = s.execute("edit.replace", json!({"what": "SEQUENCE", "with": "X"})).map_err(|e| e.to_string());
-    let miss = s.execute("edit.replace", json!({"what": "NOSUCHTEXT", "with": "X"})).map_err(|e| e.to_string());
-    assert_eq!(hit, miss);
-}

crates/xlsx/src/richdata.rs

@@ -142,13 +142,22 @@ pub fn read(cx: &mut Ctx<'_>, workbook_rels: &[Rel]) -> Result<Vec<Option<Arc<Ce
     let Some(image_rel_list) = read_xml(cx, &image_rels_path, "richValueRels")? else { return Ok(vec![]) };
     let image_rels = cx.pkg.rels(&image_rels_path)?;
     let structures = entries(cx, &structures, "s")?;
+    // Each structure's key layout, worked out once: every value would otherwise walk its
+    // structure's keys again. Only local-image structures matter here.
+    let layouts: Vec<Option<Layout>> = structures.iter().map(|s| Layout::of(s)).collect();
     let image_rel_list = entries(cx, &image_rel_list, "rel")?;
+    // Relationships by id, a repeated id recorded as invalid (`None`), so a value's lookup is
+    // one probe rather than a scan of the whole table for duplicates.
+    let mut rels_by_id: HashMap<&str, Option<&Rel>> = HashMap::new();
+    for rel in &image_rels {
+        rels_by_id.entry(rel.id.as_str()).and_modify(|r| *r = None).or_insert(Some(rel));
+    }
     let values = entries(cx, &data, "rv")?;
     let mut pictures = Vec::with_capacity(values.len());
     let mut cache = HashMap::new();
     let mut total = 0usize;
     for value in values {
-        let picture = read_value(cx, value, &structures, &image_rel_list, &image_rels, &mut cache, &mut total)?;
+        let picture = read_value(cx, value, &layouts, &image_rel_list, &rels_by_id, &mut cache, &mut total)?;
         if picture.is_none() {
             cx.warn("some rich values are unsupported or malformed; their scalar fallbacks were retained (cell pictures may be missing)");
         }
@@ -188,43 +197,56 @@ pub fn read(cx: &mut Ctx<'_>, workbook_rels: &[Rel]) -> Result<Vec<Option<Arc<Ce
         .collect())
 }
 
+/// Where a `_localImage` structure keeps the values a picture needs.
+struct Layout {
+    keys: usize,
+    /// Index of the one `_rvRel:LocalImageIdentifier` key of type `i`, if exactly one.
+    image: Option<usize>,
+    /// Index of the one `Text` key of type `s`, if exactly one.
+    text: Option<usize>,
+}
+
+impl Layout {
+    fn of(structure: &El) -> Option<Layout> {
+        if structure.attr("t") != Some("_localImage") {
+            return None;
+        }
+        let keys: Vec<&El> = structure.kids("k").collect();
+        let one = |name: &str, ty: &str| {
+            let mut matches = keys.iter().enumerate().filter(|(_, k)| k.attr("n") == Some(name));
+            let (i, key) = matches.next()?;
+            (matches.next().is_none() && key.attr("t") == Some(ty)).then_some(i)
+        };
+        Some(Layout { keys: keys.len(), image: one("_rvRel:LocalImageIdentifier", "i"), text: one("Text", "s") })
+    }
+}
+
 fn read_value(
     cx: &mut Ctx<'_>,
     value: &El,
-    structures: &[&El],
+    layouts: &[Option<Layout>],
     image_rel_list: &[&El],
-    image_rels: &[Rel],
+    rels_by_id: &HashMap<&str, Option<&Rel>>,
     cache: &mut HashMap<(String, String), Arc<CellPicture>>,
     total: &mut usize,
 ) -> Result<Option<Arc<CellPicture>>, IoError> {
-    let Some(structure) = value.attr_u32("s").and_then(|n| structures.get(n as usize)) else { return Ok(None) };
-    if structure.attr("t") != Some("_localImage") {
-        return Ok(None);
-    }
-    let keys: Vec<_> = structure.kids("k").collect();
+    let Some(Some(layout)) = value.attr_u32("s").and_then(|n| layouts.get(n as usize)) else { return Ok(None) };
     let values: Vec<_> = value.kids("v").collect();
-    if keys.len() != values.len() {
+    if layout.keys != values.len() {
         return Ok(None);
     }
-    let lookup = |name: &str, ty: &str| {
-        let mut matches = keys.iter().zip(&values).filter(|(k, _)| k.attr("n") == Some(name));
-        let (key, value) = matches.next()?;
-        (matches.next().is_none() && key.attr("t") == Some(ty)).then_some(value.text.as_str())
-    };
-    let Some(rel_id) = lookup("_rvRel:LocalImageIdentifier", "i")
-        .and_then(|v| v.trim().parse::<usize>().ok())
-        .and_then(|n| image_rel_list.get(n))
-        .and_then(|rel| rel.attr("id"))
+    let lookup = |slot: Option<usize>| slot.and_then(|i| values.get(i)).map(|v| v.text.as_str());
+    let Some(rel_id) =
+        lookup(layout.image).and_then(|v| v.trim().parse::<usize>().ok()).and_then(|n| image_rel_list.get(n)).and_then(|rel| rel.attr("id"))
     else {
         return Ok(None);
     };
-    let mut matches = image_rels.iter().filter(|rel| rel.id == rel_id);
-    let Some(rel) = matches.next() else { return Ok(None) };
-    if matches.next().is_some() || rel.kind != "image" || rel.external {
+    let Some(found) = rels_by_id.get(rel_id) else { return Ok(None) };
+    let Some(rel) = found.filter(|rel| rel.kind == "image" && !rel.external) else {
         cx.warn("a cell picture has an external or invalid image relationship; kept its scalar fallback");
         return Ok(None);
-    }
-    let alt = lookup("Text", "s").unwrap_or("").to_string();
+    };
+    let alt = lookup(layout.text).unwrap_or("").to_string();
     let key = (normalize(&rel.target), alt.clone());
     if let Some(picture) = cache.get(&key) {
         return Ok(Some(picture.clone()));

crates/xlsx/src/tests/richdata_tests.rs

@@ -294,3 +294,27 @@ fn picture_metadata_limits_and_relationship_read_errors_do_not_block_workbooks()
         assert!(report.warnings.iter().any(|w| w.contains("cell-picture data could not be read")), "{:?}", report.warnings);
     }
 }
+
+/// Many values over one wide structure, and a repeated relationship id: the structure's layout
+/// is worked out once and relationships are indexed, so the import is linear in the records;
+/// the repeated id is invalid and keeps the scalar fallback.
+#[test]
+fn wide_structures_and_repeated_relationship_ids_are_resolved_through_indexes() {
+    let mut parts = independent_parts(false);
+    // 200,000 keys in one structure and 50,000 short values that name it: rebuilding the key
+    // list per value would be ten billion steps.
+    let padding = r#"<k n="pad" t="s"/>"#.repeat(200_000);
+    replace(&mut parts, "rich/structures.xml", r#"<k n="CalcOrigin" t="i"/>"#, &format!(r#"<k n="CalcOrigin" t="i"/>{padding}"#));
+    let values = r#"<rv s="1"><v/></rv>"#.repeat(50_000);
+    replace(&mut parts, "rich/values.xml", r#"<rv s="1"><v>green &amp; &lt;image&gt;</v><v>1</v><v>5</v></rv>"#, &values);
+    let (wb, report) = read_xlsx(&zip(&parts)).unwrap();
+    assert!(!wb.sheet(0).unwrap().cell_pictures.contains_key(&at("A1")));
+    assert!(report.warnings.iter().any(|w| w.contains("picture")), "{:?}", report.warnings);
+    // The same id twice makes every reference to it invalid.
+    let mut parts = independent_parts(false);
+    let dup = r#"<Relationship Id="opaqueImageId" Type="http://schemas.openxmlformats.org/officeDocument/2006/relationships/image" Target="../media/original.bin"/>"#;
+    replace(&mut parts, "rich/_rels/images.xml.rels", dup, &dup.repeat(2));
+    let (wb, report) = read_xlsx(&zip(&parts)).unwrap();
+    assert!(!wb.sheet(0).unwrap().cell_pictures.contains_key(&at("A1")));
+    assert!(report.warnings.iter().any(|w| w.contains("invalid image relationship")), "{:?}", report.warnings);
+}