Bound block expansion in drawing and extents: nested, self-referencing or arrayed blocks no longer hang (fixes #255) (#267)

* Bound block expansion in drawing and extents (fixes #255)

MAX_BLOCK_DEPTH bounds only the depth of nested block references, so a
small DXF with a self-referencing block, blocks that each insert the next
several times, or one large MINSERT expanded to billions of copies and
hung the app (drawing, PNG/SVG/PDF export, extents, saving).

- render: a per-display-list budget (MAX_BLOCK_EXPANSION, 2 million
  block copies and entities drawn inside blocks); past it the remaining
  block contents are not drawn.
- doc: entity_bounds computes each block's content bounds once per call
  and reuses them for every further reference.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Move Builder::expand below the drawing primitives

Pure code motion so this branch merges cleanly with fix/272-aci7-only-flips
(#272), which changes the Builder::polyline signature next to it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

crates/doc/src/extents.rs

@@ -1,5 +1,7 @@
 //! Approximate entity bounds (used for extents, zoom and selection pre-filtering).
 
+use std::collections::HashMap;
+
 use cadcraft_geom::{Arc, Bounds2, EPS, Polyline, Vec2, bulge_to_arc};
 
 use crate::{Drawing, Entity, EntityKind, LwPolyline, Prim};
@@ -33,6 +35,14 @@ fn text_box(t: &crate::Text, len: usize) -> Bounds2 {
 }
 
 pub fn entity_bounds(d: &Drawing, e: &Entity, depth: usize) -> Bounds2 {
+    bounds_in(d, e, depth, &mut HashMap::new())
+}
+
+/// [`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 {
     match &e.kind {
         EntityKind::Text(t) => text_box(t, t.value.chars().count()),
         EntityKind::AttDef(a) => text_box(&a.text, a.tag.chars().count()),
@@ -62,10 +72,17 @@ pub fn entity_bounds(d: &Drawing, e: &Entity, depth: usize) -> Bounds2 {
             }
             let Some(blk) = d.block(&ins.block) else { return Bounds2::from_points([ins.insert.xy()]) };
             let m = ins.transform(blk.base.xy());
-            let mut inner = Bounds2::EMPTY;
-            for be in blk.entities.iter() {
-                inner = inner.union(&entity_bounds(d, be, depth + 1));
-            }
+            let inner = match blocks.get(&blk.name) {
+                Some(b) => *b,
+                None => {
+                    let mut inner = Bounds2::EMPTY;
+                    for be in blk.entities.iter() {
+                        inner = inner.union(&bounds_in(d, be, depth + 1, blocks));
+                    }
+                    blocks.insert(blk.name.clone(), inner);
+                    inner
+                }
+            };
             let mut b =
                 if inner.is_empty() { Bounds2::from_points([ins.insert.xy()]) } else { Bounds2::from_points(inner.corners().map(|c| m.apply(c))) };
             if ins.cols > 1 || ins.rows > 1 {
@@ -106,7 +123,7 @@ pub fn entity_bounds(d: &Drawing, e: &Entity, depth: usize) -> Bounds2 {
                 && depth < MAX_BLOCK_DEPTH
             {
                 for be in blk.entities.iter() {
-                    b = b.union(&entity_bounds(d, be, depth + 1));
+                    b = b.union(&bounds_in(d, be, depth + 1, blocks));
                 }
             }
             b

crates/io/tests/block_expansion.rs

@@ -0,0 +1,65 @@
+//! Nested, self-referencing and arrayed block references in a hostile DXF must not hang drawing
+//! or extents (issue #255): the expansion is bounded, not only its depth.
+
+use cadcraft_doc::{Drawing, EntityKind, Space};
+use cadcraft_render::{MAX_BLOCK_EXPANSION, Options, build};
+
+fn read(text: &str) -> Drawing {
+    cadcraft_io::read(text.as_bytes(), "x.dxf").unwrap()
+}
+
+/// Block `A` holds a line and `refs` references to itself; model space holds one `A`.
+fn self_referencing(refs: usize) -> String {
+    let mut s = String::from("0\nSECTION\n2\nBLOCKS\n0\nBLOCK\n2\nA\n0\nLINE\n10\n0\n20\n0\n11\n1\n21\n1\n");
+    for k in 1..=refs {
+        s += &format!("0\nINSERT\n2\nA\n10\n{k}\n20\n0\n");
+    }
+    s + "0\nENDBLK\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nINSERT\n2\nA\n10\n0\n20\n0\n0\nENDSEC\n0\nEOF\n"
+}
+
+/// Blocks `F0`..`F14`, each holding four references to the next; `F15` holds a line (drawn
+/// 4^15 times: its references sit exactly at `MAX_BLOCK_DEPTH`).
+fn fan_out() -> String {
+    let mut s = String::from("0\nSECTION\n2\nBLOCKS\n");
+    for k in 0..15 {
+        s += &format!("0\nBLOCK\n2\nF{k}\n");
+        for x in 0..4 {
+            s += &format!("0\nINSERT\n2\nF{}\n10\n{x}\n20\n0\n", k + 1);
+        }
+        s += "0\nENDBLK\n";
+    }
+    s + "0\nBLOCK\n2\nF15\n0\nLINE\n10\n0\n20\n0\n11\n1\n21\n1\n0\nENDBLK\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nINSERT\n2\nF0\n10\n0\n20\n0\n0\nENDSEC\n0\nEOF\n"
+}
+
+/// One MINSERT of a one-line block with `cols` x `rows` copies 2 units apart.
+fn minsert(cols: u32, rows: u32) -> String {
+    format!(
+        "0\nSECTION\n2\nBLOCKS\n0\nBLOCK\n2\nA\n0\nLINE\n10\n0\n20\n0\n11\n1\n21\n1\n0\nENDBLK\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n0\nINSERT\n2\nA\n10\n0\n20\n0\n70\n{cols}\n71\n{rows}\n44\n2\n45\n2\n0\nENDSEC\n0\nEOF\n"
+    )
+}
+
+#[test]
+fn block_expansion_bombs_are_bounded() {
+    for text in [self_referencing(4), self_referencing(12), fan_out(), minsert(10_000, 10_000)] {
+        let d = read(&text);
+        let ext = d.extents(&Space::Model);
+        assert!(!ext.is_empty() && ext.min.is_finite() && ext.max.is_finite(), "{ext:?}");
+        let list = build(&d, &Space::Model, &Options::default());
+        assert!(!list.prims.is_empty() && list.prims.len() <= MAX_BLOCK_EXPANSION, "{}", list.prims.len());
+        // Saving computes the extents again ($EXTMIN/$EXTMAX).
+        assert!(cadcraft_io::write_dxf(&d).contains("$EXTMAX"));
+    }
+}
+
+#[test]
+fn ordinary_nested_and_arrayed_blocks_are_drawn_whole() {
+    // 3 x 2 copies of a one-line block: six lines, extents spanning all copies.
+    let d = read(&minsert(3, 2));
+    assert_eq!(build(&d, &Space::Model, &Options::default()).prims.len(), 6);
+    let ext = d.extents(&Space::Model);
+    assert_eq!((ext.min.x, ext.min.y, ext.max.x, ext.max.y), (0.0, 0.0, 5.0, 3.0));
+    // A self-referencing block draws its line at every level down to MAX_BLOCK_DEPTH.
+    let d = read(&self_referencing(1));
+    assert!(matches!(d.model.iter().next().map(|e| &e.kind), Some(EntityKind::Insert(_))));
+    assert_eq!(build(&d, &Space::Model, &Options::default()).prims.len(), cadcraft_doc::MAX_BLOCK_DEPTH);
+}

crates/render/src/lib.rs

@@ -61,6 +61,8 @@ pub struct DisplayList {
     /// The paper sheet when the list was built for a layout (paper space); the UI draws the
     /// white sheet, its shadow and the printable-area outline from it.
     pub sheet: Option<Sheet>,
+    /// Block contents expanded while building (see [`MAX_BLOCK_EXPANSION`]).
+    pub expanded: usize,
 }
 
 impl DisplayList {
@@ -130,6 +132,12 @@ struct Builder<'a> {
     plotting: bool,
 }
 
+/// Upper bound on the block contents drawn per display list (each block reference, MINSERT copy
+/// and entity drawn inside a block counts one). Nested, self-referencing or arrayed block
+/// references in a hostile file would otherwise expand exponentially (`MAX_BLOCK_DEPTH` only
+/// bounds the depth); past the limit the remaining block contents are not drawn.
+pub const MAX_BLOCK_EXPANSION: usize = 2_000_000;
+
 impl Builder<'_> {
     fn polyline(&mut self, ctx: &Ctx, color: Rgb, lw: f32, pts: &[Vec2]) {
         if pts.len() < 2 {
@@ -188,6 +196,11 @@ impl Builder<'_> {
         self.list.verts.push(ctx.xf.apply_vec(dir).normalized());
         self.list.prims.push(DPrim { handle: ctx.top, color, lw: 0.0, kind: Kind::Infinite { ray }, start, len: 2 });
     }
+    /// Take one unit of the block-expansion budget; false once it is spent.
+    fn expand(&mut self) -> bool {
+        self.list.expanded = self.list.expanded.saturating_add(1);
+        self.list.expanded <= MAX_BLOCK_EXPANSION
+    }
 }
 
 /// Build the display list for a space. For a layout (paper space) this includes the model-space
@@ -249,6 +262,9 @@ fn viewport(b: &mut Builder, d: &Drawing, e: &Entity, vp: &cadcraft_doc::Viewpor
     }
     // Border (on the viewport's layer; layer off hides only the border).
     entity(b, &top_ctx(d, Mat3::IDENTITY, e.handle, &[], &[]), e);
+    if b.list.expanded >= MAX_BLOCK_EXPANSION {
+        return;
+    }
     let center = vp.center.xy();
     let ok = |v: f64| v.is_finite() && v > 0.0;
     if !(ok(vp.width) && ok(vp.height) && ok(vp.view_height) && center.is_finite() && vp.view_center.is_finite()) {
@@ -277,6 +293,7 @@ fn viewport(b: &mut Builder, d: &Drawing, e: &Entity, vp: &cadcraft_doc::Viewpor
         }
         entity(&mut sub, &top_ctx(d, xf, VIEWPORT_CONTENT, &vp.frozen_layers, &vp.layer_colors), me);
     }
+    b.list.expanded = b.list.expanded.saturating_add(sub.list.expanded);
     append_clipped(&mut b.list, &sub.list, &rect);
 }
 
@@ -420,6 +437,9 @@ fn entity(b: &mut Builder, ctx: &Ctx, e: &Entity) {
             {
                 let sub = sub_ctx(ctx, e, Mat3::IDENTITY);
                 for be in blk.entities.iter() {
+                    if !b.expand() {
+                        break;
+                    }
                     entity(b, &sub, be);
                 }
             } else {
@@ -800,8 +820,11 @@ fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc::Insert, rg
     if let Some(blk) = ctx.d.block(&ins.block) {
         let cols = ins.cols.clamp(1, 10_000);
         let rows = ins.rows.clamp(1, 10_000);
-        for r in 0..rows {
+        'copies: for r in 0..rows {
             for c in 0..cols {
+                if !b.expand() {
+                    break 'copies;
+                }
                 let off = Vec2::new(ins.col_spacing * f64::from(c), ins.row_spacing * f64::from(r)).rotate(ins.rotation);
                 let m = Mat3::translate(off).then_before(ins.transform(blk.base.xy()));
                 let sub = sub_ctx(ctx, e, m);
@@ -810,6 +833,9 @@ fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc::Insert, rg
                     if matches!(be.kind, EntityKind::AttDef(_)) {
                         continue;
                     }
+                    if !b.expand() {
+                        break 'copies;
+                    }
                     entity(b, &sub, be);
                 }
             }