From dad7d1c138f585f9b4b21db7ea4f3d545a66fc54 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 18:11:23 -0500 Subject: [PATCH] fix(render): charge every block entry, index attribute definitions, and walk dashes and outlines in linear time (#2724) * fix(render): charge every block entry, index attribute definitions, and walk dashes and outlines in linear time Array copies skipped ATTDEF entries before charging the expansion budget, each attribute searched its block for a definition, each dash scanned every centreline sample, and outline chaining searched a vertex's segment list from its start on every visit. Each is now charged, indexed or cursored. * address review: drop wall-clock bound from outline test --- crates/render/src/blocks.rs | 26 +++++++++++++++++--------- crates/render/src/outline.rs | 32 ++++++++++++++++++++++++++++---- crates/render/src/wide.rs | 4 +++- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/crates/render/src/blocks.rs b/crates/render/src/blocks.rs index ad4689682..ca8304eb0 100644 --- a/crates/render/src/blocks.rs +++ b/crates/render/src/blocks.rs @@ -56,13 +56,15 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc: let m = Mat3::translate(off).then_before(ins.transform(blk.base.xy())); let sub = Ctx { block_pstyle: b.named, block_transparency: b.transparency, ..sub_ctx(ctx, e, m) }; for be in blk.entities.iter() { + // Every visit is charged, drawn or not, so a block of definitions can't make + // the copies loop free. + if !b.expand() { + break 'copies; + } // Constant/visible attribute definitions inside blocks are not drawn; attribs are. if matches!(be.kind, EntityKind::AttDef(_)) { continue; } - if !b.expand() { - break 'copies; - } entity(b, &sub, be); } } @@ -73,6 +75,17 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc: let attmode = ctx.d.header.i64("ATTMODE", 1); // The reference's transparency; each attribute draws with its own. let reference = b.transparency; + // The block's first definition per tag (without case), built once for all the attributes. + let mut defs: std::collections::HashMap = std::collections::HashMap::new(); + if !ins.attribs.is_empty() + && let Some(blk) = ctx.d.block(&ins.block) + { + for be in blk.entities.iter() { + if let EntityKind::AttDef(d) = &be.kind { + defs.entry(d.tag.to_ascii_lowercase()).or_insert(be.as_ref()); + } + } + } for a in &ins.attribs { if attmode == 0 || (a.invisible && attmode != 2) { continue; @@ -83,7 +96,7 @@ pub(crate) fn insert(b: &mut Builder, ctx: &Ctx, e: &Entity, ins: &cadcraft_doc: // Properties of the attribute's definition (as AutoCAD copies them to the attribute // on insertion), except those set on the attribute itself: ByBlock = the reference's, // layer 0 = the reference's layer. - let def = ctx.d.block(&ins.block).and_then(|blk| blk.entities.iter().find(|be| attdef_tag(be, &a.tag))); + let def = defs.get(&a.tag.to_ascii_lowercase()).copied(); let (ink, lw, shown, t) = match (def, a.props.is_empty()) { (None, true) => (rgb, resolve(ctx, e, b.plotting).1, true, reference), (def, _) => { @@ -117,8 +130,3 @@ fn by_block() -> cadcraft_doc::Common { ..cadcraft_doc::Common::default() } } - -/// The attribute definition with tag `tag` (any case). -fn attdef_tag(e: &Entity, tag: &str) -> bool { - matches!(&e.kind, EntityKind::AttDef(d) if d.tag.eq_ignore_ascii_case(tag)) -} diff --git a/crates/render/src/outline.rs b/crates/render/src/outline.rs index 730d7fb39..96244fc01 100644 --- a/crates/render/src/outline.rs +++ b/crates/render/src/outline.rs @@ -106,10 +106,12 @@ fn chain(mut segs: Vec<(Vec2, Vec2)>) -> Vec> { } } segs.sort_unstable_by(|(a, b), (c, d)| cmp(*a, *c).then_with(|| cmp(*b, *d))); - let mut at: HashMap> = HashMap::new(); + // Segments at each vertex with a cursor past the used ones, so many segments meeting at one + // vertex are each examined once rather than from the start on every visit. + let mut at: HashMap, usize)> = HashMap::new(); for (i, (a, b)) in segs.iter().enumerate() { - at.entry(key(*a)).or_default().push(i); - at.entry(key(*b)).or_default().push(i); + at.entry(key(*a)).or_default().0.push(i); + at.entry(key(*b)).or_default().0.push(i); } let mut used = vec![false; segs.len()]; let mut out = Vec::new(); @@ -125,7 +127,15 @@ fn chain(mut segs: Vec<(Vec2, Vec2)>) -> Vec> { let mut end = b; // Each step uses one segment, so the walk ends within `segs.len()` steps. loop { - let next = at.get(&key(end)).and_then(|ids| ids.iter().copied().find(|&i| !used.get(i).copied().unwrap_or(true))); + let next = at.get_mut(&key(end)).and_then(|(ids, cursor)| { + while let Some(&i) = ids.get(*cursor) { + if !used.get(i).copied().unwrap_or(true) { + return Some(i); + } + *cursor += 1; + } + None + }); let Some(i) = next else { break }; let Some(&(p, q)) = segs.get(i) else { break }; if let Some(u) = used.get_mut(i) { @@ -147,6 +157,20 @@ mod tests { lines.iter().flat_map(|l| l.windows(2).map(|w| w[0].dist(w[1]))).sum() } + /// Thousands of lobes meeting at one vertex chain in one pass over that vertex's segments. + #[test] + fn many_lobes_at_one_vertex_chain_quickly() { + let mut tris = Vec::new(); + for i in 0..20_000 { + let x = f64::from(i + 1); + tris.extend_from_slice(&[Vec2::ZERO, Vec2::new(x, 1.0), Vec2::new(x + 0.5, 2.0)]); + } + // No wall-clock bound (flaky on loaded runners): a quadratic scan would make this test crawl + // rather than fail, so check the output is complete instead. + let lines = fill_outlines(&tris); + assert_eq!(lines.iter().map(|l| l.len() - 1).sum::(), 60_000, "every triangle edge is emitted once"); + } + #[test] fn a_square_of_two_triangles_is_one_loop() { let (a, b, c, d) = (Vec2::new(0.0, 0.0), Vec2::new(2.0, 0.0), Vec2::new(2.0, 1.0), Vec2::new(0.0, 1.0)); diff --git a/crates/render/src/wide.rs b/crates/render/src/wide.rs index d0d874d3f..5df6f42f3 100644 --- a/crates/render/src/wide.rs +++ b/crates/render/src/wide.rs @@ -95,7 +95,9 @@ pub fn dashed(p: &LwPolyline, tol: f64, lt: &Linetype, scale: f64, min_pattern: continue; } let (Some(first), Some(last)) = (at(run, a), at(run, b)) else { continue }; - let inner = run.iter().filter(|x| x.s > a && x.s < b).copied(); + // The samples strictly between a and b: `run` is ordered by `s`, so a slice, not a scan. + let (lo, hi) = (run.partition_point(|x| x.s <= a), run.partition_point(|x| x.s < b)); + let inner = run.get(lo..hi.max(lo)).unwrap_or_default().iter().copied(); let pts: Vec = std::iter::once(first).chain(inner).chain(std::iter::once(last)).collect(); for w in pts.windows(2) { if let [x, y] = w {