From 15bea9fa7afd5b64db4bcb22db3d15b36372d0d8 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:20:09 -0500 Subject: [PATCH] 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 | 28 +++++++++++++++++++++++++++- crates/render/src/pdf_layers.rs | 3 ++- crates/textimport/src/export.rs | 26 ++++++++++++++++++-------- crates/textimport/src/tagged.rs | 11 +++++++---- 4 files changed, 54 insertions(+), 14 deletions(-) diff --git a/crates/epub/src/css.rs b/crates/epub/src/css.rs index f066d6b3d..1af0be208 100644 --- a/crates/epub/src/css.rs +++ b/crates/epub/src/css.rs @@ -7,14 +7,18 @@ use super::*; fn class_names<'a>(names: impl Iterator) -> 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 = 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 { } 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 = (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()); + } +} diff --git a/crates/render/src/pdf_layers.rs b/crates/render/src/pdf_layers.rs index bf3a7509d..f428b522e 100644 --- a/crates/render/src/pdf_layers.rs +++ b/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::>(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::>(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, std::collections::HashSet) = + (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. diff --git a/crates/textimport/src/export.rs b/crates/textimport/src/export.rs index 7d614f640..2493f7f98 100644 --- a/crates/textimport/src/export.rs +++ b/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 = Vec::new(); + // Each font's number in the table (`fonts` keeps the table's order). + let mut font_index: std::collections::HashMap = std::collections::HashMap::new(); + let mut add_font = |fonts: &mut Vec, 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 ¶s { 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") { diff --git a/crates/textimport/src/tagged.rs b/crates/textimport/src/tagged.rs index 3fa209126..c3668614d 100644 --- a/crates/textimport/src/tagged.rs +++ b/crates/textimport/src/tagged.rs @@ -143,6 +143,8 @@ pub fn import(bytes: &[u8]) -> Result { let mut fmt = CharFormat::default(); let mut para_styles: Vec = Vec::new(); let mut char_styles: Vec = 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 { "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 { "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 { "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); } }