Extents: cache block bounds per depth so a deep visit can't hide a shallow reference (fixes #706) (#778)

This commit is contained in:
Marco Arnulfo
2026-10-13 04:35:20 -04:00
committed by GitHub
parent bca698a588
commit 565cadaa51
2 changed files with 40 additions and 6 deletions
+8 -6
View File
@@ -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<String, Bounds2>) -> 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<String,
// insertion point. Attributes below still count.
let Some(blk) = d.block(&ins.block) else { return Bounds2::EMPTY };
let m = ins.transform(blk.base.xy());
let inner = match blocks.get(&blk.name) {
let inner = match blocks.get(&(blk.name.as_str(), depth)) {
Some(b) => *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
}
};
+32
View File
@@ -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<EntityKind>| {
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");