From 565cadaa518c27a92ee1a202f070cf865a0025fd Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Tue, 13 Oct 2026 10:35:20 +0200 Subject: [PATCH] Extents: cache block bounds per depth so a deep visit can't hide a shallow reference (fixes #706) (#778) --- crates/doc/src/extents.rs | 14 ++++++++------ crates/doc/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/crates/doc/src/extents.rs b/crates/doc/src/extents.rs index 6a3f4299..833e5dfb 100644 --- a/crates/doc/src/extents.rs +++ b/crates/doc/src/extents.rs @@ -88,10 +88,12 @@ pub fn entity_bounds(d: &Drawing, e: &Entity, depth: usize) -> Bounds2 { } /// [`entity_bounds`] with the bounds of block contents (in block coordinates) already computed, -/// by block name: every further reference to a block reuses them instead of expanding the block -/// again, so nested or self-referencing blocks cost one pass over each definition instead of one -/// per reference (which grows exponentially with the nesting depth). -fn bounds_in(d: &Drawing, e: &Entity, depth: usize, blocks: &mut HashMap) -> Bounds2 { +/// by block name and depth: every further reference to a block at that depth reuses them instead of +/// expanding the block again, so nested or self-referencing blocks cost at most one pass over each +/// definition per depth instead of one per reference (which grows exponentially with the nesting +/// depth). The depth is part of the key because [`MAX_BLOCK_DEPTH`] cuts a deep visit short: its +/// bounds must not stand in for a shallower reference to the same block (#706). +fn bounds_in<'a>(d: &'a Drawing, e: &Entity, depth: usize, blocks: &mut HashMap<(&'a str, usize), Bounds2>) -> Bounds2 { match &e.kind { EntityKind::Table(t) if t.is_broken() => t.bounds(), EntityKind::Text(t) if blank(&t.value) => Bounds2::EMPTY, @@ -108,14 +110,14 @@ fn bounds_in(d: &Drawing, e: &Entity, depth: usize, blocks: &mut HashMap *b, None => { let mut inner = Bounds2::EMPTY; for be in blk.entities.iter().filter(|be| in_block_visible(d, be)) { inner = inner.union(&bounds_in(d, be, depth + 1, blocks)); } - blocks.insert(blk.name.clone(), inner); + blocks.insert((blk.name.as_str(), depth), inner); inner } }; diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index 867b051e..816eedf8 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -269,6 +269,38 @@ fn extents_pair(d: &Drawing) -> (Vec2, Vec2) { (b.min, b.max) } +/// #706: block A reaches B once through a chain of 14 blocks (B at depth 15, its line's block E at +/// 16, past the limit) and once directly (B at depth 1). `children` picks which of the two A holds. +fn deep_and_shallow(children: &[&str]) -> Drawing { + let mut d = Drawing::new_imperial(); + let mut add = |name: &str, kids: Vec| { + let mut blk = Block::new(name); + for (i, k) in kids.into_iter().enumerate() { + blk.entities.push(Entity::new(Handle(i as u64 + 1), k)); + } + d.blocks.insert(name.into(), std::sync::Arc::new(blk)); + }; + add("E", vec![line((1000.0, 1000.0), (2000.0, 1000.0))]); + add("B", vec![plain_insert("E")]); + for i in 1..=14 { + let next = if i == 14 { "B".to_string() } else { format!("C{}", i + 1) }; + add(&format!("C{i}"), vec![plain_insert(&next)]); + } + add("A", children.iter().map(|c| plain_insert(c)).collect()); + d.add(&Space::Model, Common::default(), plain_insert("A")).unwrap(); + d +} + +#[test] +fn deep_block_visit_does_not_hide_a_shallow_reference() { + let line = (Vec2::new(1000.0, 1000.0), Vec2::new(2000.0, 1000.0)); + // Control: past the limit, the deep path alone contributes nothing. + assert!(deep_and_shallow(&["C1"]).extents(&Space::Model).is_empty()); + assert_eq!(extents_pair(&deep_and_shallow(&["B"])), line); + assert_eq!(extents_pair(&deep_and_shallow(&["B", "C1"])), line); + assert_eq!(extents_pair(&deep_and_shallow(&["C1", "B"])), line); +} + #[test] fn block_entity_with_invisible_flag_is_ignored_in_insert_extents() { let mut far = far_line("0");