Opening documents: index lookups in Document::check and the format-1 style upgrade (#2218)

* fix(doc): index items, threads and styles when opening a document

* Apply review fixes

- crates/doc/src/tests.rs

* address review: borrow style names in pin_para_default

---------

Co-authored-by: aiongg <67543387+aiongg@users.noreply.github.com>
Co-authored-by: autocraft-bot[bot] <354215646+autocraft-bot[bot]@users.noreply.github.com>
This commit is contained in:
authored and GitHub committed 2026-11-11 16:20:34 -05:00
1 parent 15bea9fa7a
commit eba00a0dec
5 files changed
+78 -10

No files matched your search

+19 -5
View File
@@ -1136,6 +1136,11 @@ impl Document {
}
}
let mut ids = std::collections::HashSet::new();
// Every item's story (text frames), by id, and every (story, frame) of the threads: built
// once, so checking each reference both ways is a lookup, not a search of the document.
let mut frame_story: std::collections::HashMap<ItemId, Option<StoryId>> = std::collections::HashMap::new();
let threads: std::collections::HashSet<(StoryId, ItemId)> =
self.stories.iter().flat_map(|(sid, st)| st.frames.iter().map(move |f| (*sid, *f))).collect();
for sp in self.spreads.iter().chain(self.parents.iter()) {
for it in &sp.items {
let mut dup = None;
@@ -1143,20 +1148,30 @@ impl Document {
if !ids.insert(i.id.0) {
dup = Some(i.id);
}
frame_story.insert(i.id, i.text_frame().map(|t| t.story));
});
if let Some(d) = dup {
return Err(DocError::Invalid(format!("duplicate item id {d}")));
}
}
}
let embedded = self.anchored_items();
// An anchored item's story, by id (the first of an id, as a search would find it).
let mut anchored: std::collections::HashMap<ItemId, Option<StoryId>> = std::collections::HashMap::new();
for (_, item) in &embedded {
anchored.entry(item.id).or_insert_with(|| item.text_frame().map(|t| t.story));
}
for (sid, st) in &self.stories {
st.check().map_err(DocError::Invalid)?;
if st.id != *sid {
return Err(DocError::Invalid(format!("story key {sid} != id {}", st.id)));
}
for f in &st.frames {
let item = self.item(*f).or_else(|| self.anchored_item(*f)).ok_or(DocError::NoItem(*f))?;
match item.text_frame().map(|t| t.story) {
let story = match frame_story.get(f) {
Some(story) => *story,
None => anchored.get(f).copied().ok_or(DocError::NoItem(*f))?,
};
match story {
Some(s) if s == *sid => {}
other => return Err(DocError::Invalid(format!("frame {f} in story {sid} points at {other:?}"))),
}
@@ -1168,7 +1183,7 @@ impl Document {
let mut bad = None;
it.walk(&mut |i| {
if let Some(t) = i.text_frame()
&& !self.stories.get(&t.story).is_some_and(|s| s.frames.contains(&i.id))
&& !threads.contains(&(t.story, i.id))
{
bad = Some(i.id);
}
@@ -1178,14 +1193,13 @@ impl Document {
}
}
}
let embedded = self.anchored_items();
let mut edges: std::collections::HashMap<StoryId, Vec<StoryId>> = std::collections::HashMap::new();
for (owner, item) in embedded {
if !ids.insert(item.id.0) {
return Err(DocError::Invalid(format!("duplicate item id {}", item.id)));
}
if let Some(tf) = item.text_frame() {
if !self.stories.get(&tf.story).is_some_and(|s| s.frames.contains(&item.id)) {
if !threads.contains(&(tf.story, item.id)) {
return Err(DocError::Invalid(format!("embedded text frame {} not in its story's thread", item.id)));
}
edges.entry(owner).or_default().push(tf.story);
+17 -2
View File
@@ -775,14 +775,29 @@ impl Styles {
/// The paragraph style chain from root to `name` (cycle-safe): resolution starts at its first.
pub fn para_chain(&self, name: &str) -> Vec<&ParagraphStyle> {
Self::para_chain_by(name, |n| self.para(n))
}
/// The paragraph styles by name, the first of each name (as [`Self::para`] finds it), for
/// callers that look up every style: one map instead of a search per style.
pub fn para_index(&self) -> std::collections::HashMap<&str, &ParagraphStyle> {
let mut index = std::collections::HashMap::new();
for s in &self.paragraph {
index.entry(s.name.as_str()).or_insert(s);
}
index
}
/// [`Self::para_chain`] with styles found through `find` (a [`Self::para_index`]).
pub fn para_chain_by<'a>(name: &str, find: impl Fn(&str) -> Option<&'a ParagraphStyle>) -> Vec<&'a ParagraphStyle> {
let mut chain = Vec::new();
let mut cur = self.para(name);
let mut cur = find(name);
while let Some(s) = cur {
if chain.len() >= MAX_DEPTH || chain.iter().any(|c: &&ParagraphStyle| c.name == s.name) {
break;
}
chain.push(s);
cur = s.based_on.as_deref().and_then(|b| self.para(b));
cur = s.based_on.as_deref().and_then(&find);
}
chain.reverse();
chain
+19
View File
@@ -370,6 +370,25 @@ fn placed_pdf_without_a_crop_choice_loads_with_the_crop_box() {
assert_eq!(Asset { pdf_box: Some([0.5, 0.5, 0.5, 0.9]), ..back }.shown_box(), None);
}
/// Checking a document looks each frame up once: 20,000 text frames check in well under a
/// second (each lookup searched every item before).
#[test]
fn check_is_linear_in_the_frames() {
let mut d = Document::new(&NewDocument::default());
let lid = d.default_layer();
for i in 0..20_000 {
let y = f64::from(i % 100);
d.add_text_frame(SpreadRef::Doc(0), Rect::new(0.0, y, 10.0, y + 1.0), lid, "x", ParaFormat::default()).unwrap();
}
let t = std::time::Instant::now();
d.check().unwrap();
assert!(t.elapsed() < std::time::Duration::from_secs(10), "{:?}", t.elapsed());
// A frame its story names but the document doesn't have is still refused.
let sid = *d.stories.keys().next().unwrap();
Arc::make_mut(d.stories.get_mut(&sid).unwrap()).frames.push(ItemId(u64::MAX));
assert!(matches!(d.check(), Err(DocError::NoItem(ItemId(u64::MAX)))));
}
#[test]
fn adjust_text_ranges_tracks_and_prunes_hyperlinks_and_placeholders() {
let mut d = doc();