Tagged Text, RTF, EPUB classes and PDF layers: lookups by set or map instead of list searches (#2212)

* fix: index style, font, class and layer name lookups in exporters and importers

* Apply review fixes

- crates/epub/src/css.rs

---------

Co-authored-by: aiongg <67543387+aiongg@users.noreply.github.com>

crates/epub/src/css.rs

@@ -7,14 +7,18 @@ use super::*;
 fn class_names<'a>(names: impl Iterator<Item = &'a str>) -> BTreeMap<&'a str, String> {
     let mut out = BTreeMap::new();
     let mut used = std::collections::BTreeSet::new();
+    // The last suffix given for each slug: the next name that slugs alike carries on from it
+    // (still checked against every class, since a name can itself end in `-2`).
+    let mut last: std::collections::HashMap<String, usize> = std::collections::HashMap::new();
     for name in names {
         let base = slug(name);
         let mut class = base.clone();
-        let mut n = 1;
+        let mut n = last.get(&base).copied().unwrap_or(1);
         while !used.insert(class.clone()) {
             n += 1;
             class = format!("{base}-{n}");
         }
+        last.insert(base, n);
         out.insert(name, class);
     }
     out
@@ -251,3 +255,25 @@ fn class_selector(tag: &str, class: &str) -> Option<String> {
     }
     Some(selector)
 }
+
+#[cfg(test)]
+mod class_name_tests {
+    use super::class_names;
+
+    /// Names that slug alike are numbered in order, each carrying on from the last number given,
+    /// and a name that already ends in a number never takes another's class.
+    #[test]
+    fn colliding_names_get_unique_classes_in_one_pass() {
+        // 20,000 distinct names that all slug to `body-text` (any non-ASCII letter is a `-`).
+        let names: Vec<String> = (0..20_000u32).filter_map(|i| char::from_u32(0x4E00 + i)).map(|c| format!("Body{c}Text")).collect();
+        let mut all: Vec<&str> = vec!["Body Text", "body-text-3"];
+        all.extend(names.iter().map(String::as_str));
+        let classes = class_names(all.iter().copied());
+        assert_eq!(classes["Body Text"], "body-text");
+        assert_eq!(classes["body-text-3"], "body-text-3");
+        assert_eq!(classes[names[0].as_str()], "body-text-2");
+        assert_eq!(classes[names[1].as_str()], "body-text-4", "-3 is the class of the name that is it");
+        let distinct: std::collections::HashSet<&String> = classes.values().collect();
+        assert_eq!(distinct.len(), classes.len());
+    }
+}

crates/render/src/pdf_layers.rs

@@ -19,7 +19,8 @@ fn ocgs(pdf: &Pdf) -> Vec<(ObjRef, String, bool)> {
         cfg.as_ref().and_then(|c| c.get::<Array<'_>>(key)).map(|a| a.raw_iter().filter_map(|o| o.as_obj_ref()).collect()).unwrap_or_default()
     };
     let base_off = cfg.as_ref().and_then(|c| c.get::<Name<'_>>(b"BaseState")).is_some_and(|n| n.as_ref() == b"OFF");
-    let (on, off) = (ids_in(b"ON"), ids_in(b"OFF"));
+    let (on, off): (std::collections::HashSet<ObjRef>, std::collections::HashSet<ObjRef>) =
+        (ids_in(b"ON").into_iter().collect(), ids_in(b"OFF").into_iter().collect());
     list.raw_iter()
         .filter_map(|o| o.as_obj_ref())
         // A layer listed again is one layer: it gets one name, not one per entry.

crates/textimport/src/export.rs

@@ -87,13 +87,24 @@ pub fn rtf(doc: &Document, story: &Story) -> String {
     let styles = &doc.styles;
     // Resolve every run first so the font table can be written up front.
     let ranges = story.para_ranges();
+    // The story's runs once; each paragraph takes those reaching it by binary search.
+    let all_runs: Vec<_> = story.runs().collect();
     let mut paras = Vec::with_capacity(ranges.len());
     let mut fonts: Vec<String> = Vec::new();
+    // Each font's number in the table (`fonts` keeps the table's order).
+    let mut font_index: std::collections::HashMap<String, usize> = std::collections::HashMap::new();
+    let mut add_font = |fonts: &mut Vec<String>, name: &str| {
+        if !font_index.contains_key(name) {
+            font_index.insert(name.to_string(), fonts.len());
+            fonts.push(name.to_string());
+        }
+    };
     for (i, r) in ranges.iter().enumerate() {
         let pf = story.paras.get(i).cloned().unwrap_or_default();
         let (pp, base) = styles.resolve_para(&pf);
         let mut runs = Vec::new();
-        for (rr, f) in story.runs() {
+        let first = all_runs.partition_point(|(rr, _)| rr.end <= r.start);
+        for (rr, f) in all_runs.iter().skip(first).take_while(|(rr, _)| rr.start < r.end) {
             let (a, b) = (rr.start.max(r.start), rr.end.min(r.end));
             if a >= b {
                 continue;
@@ -103,13 +114,11 @@ pub fn rtf(doc: &Document, story: &Story) -> String {
             if doc.conditions_hide(&cp.conditions) || cp.change == designcraft_doc::ChangeMark::Deleted {
                 continue;
             }
-            if !fonts.contains(&cp.font_family) {
-                fonts.push(cp.font_family.clone());
-            }
+            add_font(&mut fonts, &cp.font_family);
             runs.push((a..b, cp));
         }
-        if runs.is_empty() && !fonts.contains(&base.font_family) {
-            fonts.push(base.font_family.clone());
+        if runs.is_empty() {
+            add_font(&mut fonts, &base.font_family);
         }
         paras.push((pp, base, pf.table, runs));
     }
@@ -122,6 +131,7 @@ pub fn rtf(doc: &Document, story: &Story) -> String {
         out.push_str(";}");
     }
     out.push_str("}\n");
+    let font_index: std::collections::HashMap<&str, usize> = fonts.iter().enumerate().map(|(i, f)| (f.as_str(), i)).collect();
     for (pp, base, table, runs) in &paras {
         let q = match pp.align {
             Align::Center => "\\qc",
@@ -139,11 +149,11 @@ pub fn rtf(doc: &Document, story: &Story) -> String {
             twips(pp.space_after)
         );
         if runs.is_empty() {
-            let f = fonts.iter().position(|x| *x == base.font_family).unwrap_or(0);
+            let f = font_index.get(base.font_family.as_str()).copied().unwrap_or(0);
             let _ = write!(out, "\\plain\\f{f}\\fs{} ", (base.size * 2.0).round() as i64);
         }
         for (r, cp) in runs {
-            let f = fonts.iter().position(|x| *x == cp.font_family).unwrap_or(0);
+            let f = font_index.get(cp.font_family.as_str()).copied().unwrap_or(0);
             let st = cp.font_style.to_ascii_lowercase();
             let _ = write!(out, "{{\\plain\\f{f}\\fs{}", (cp.size * 2.0).round() as i64);
             if st.contains("bold") || st.contains("black") || st.contains("heavy") || st.contains("semibold") {

crates/textimport/src/tagged.rs

@@ -143,6 +143,8 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
     let mut fmt = CharFormat::default();
     let mut para_styles: Vec<String> = Vec::new();
     let mut char_styles: Vec<String> = Vec::new();
+    // The names in each list, for membership (the lists keep the order the file names them).
+    let (mut para_seen, mut char_seen) = (std::collections::HashSet::new(), std::collections::HashSet::new());
     let mut warnings = Vec::new();
     let push = |paras: &mut Vec<(String, Vec<(String, CharFormat)>)>, c: char, fmt: &CharFormat| {
         let Some((_, runs)) = paras.last_mut() else { return };
@@ -198,7 +200,7 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
                     "ParaStyle" => {
                         started = true;
                         let v = unescape_value(value);
-                        if !v.is_empty() && !para_styles.contains(&v) {
+                        if !v.is_empty() && para_seen.insert(v.clone()) {
                             para_styles.push(v.clone());
                         }
                         if let Some(p) = paras.last_mut() {
@@ -208,7 +210,7 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
                     "CharStyle" => {
                         let v = unescape_value(value);
                         fmt.style = if v.is_empty() { NO_CHAR_STYLE.into() } else { v.clone() };
-                        if !v.is_empty() && !char_styles.contains(&v) {
+                        if !v.is_empty() && char_seen.insert(v.clone()) {
                             char_styles.push(v);
                         }
                     }
@@ -219,8 +221,9 @@ pub fn import(bytes: &[u8]) -> Result<Imported, ImportError> {
                     "cSize" => fmt.over.size = value.trim().parse().ok().and_then(designcraft_doc::attrs::imported_type_size),
                     "DefineParaStyle" | "DefineCharStyle" => {
                         let nm = crate::capped_name(value.split('=').next().unwrap_or("").to_string());
-                        let list = if name == "DefineParaStyle" { &mut para_styles } else { &mut char_styles };
-                        if !nm.is_empty() && !list.contains(&nm) {
+                        let (list, seen) =
+                            if name == "DefineParaStyle" { (&mut para_styles, &mut para_seen) } else { (&mut char_styles, &mut char_seen) };
+                        if !nm.is_empty() && seen.insert(nm.clone()) {
                             list.push(nm);
                         }
                     }