From f0d80a1d7e07e7ebe46c76502b2336de01fd2f3b Mon Sep 17 00:00:00 2001 From: Lasse Lian Date: Tue, 13 Oct 2026 11:36:22 +0200 Subject: [PATCH] fix: filleting beside a modeled thread no longer freezes the app (#618) Fixes #602. Collinear-edge merging, piece naming and the dialog preview each work in one pass instead of quadratic loops; the issue's example goes from 86 s to 5.1 s. --- crates/doc/src/naming.rs | 14 +- crates/engine/src/thread_tests.rs | 35 +++++ crates/kernel/src/heal.rs | 215 +++++++++++++++++++++++------- crates/ui-egui/src/preview.rs | 183 +++++++++++++++++++++---- 4 files changed, 365 insertions(+), 82 deletions(-) diff --git a/crates/doc/src/naming.rs b/crates/doc/src/naming.rs index 80e001b2..baa6a516 100644 --- a/crates/doc/src/naming.rs +++ b/crates/doc/src/naming.rs @@ -474,14 +474,12 @@ pub fn strip_edge_piece(n: &str) -> String { /// Edges sharing a name: number them along the longest axis of their spread. fn number_pieces(names: &mut [String], pts: &[Vec], mark: char) { - let mut seen: Vec = Vec::new(); - for i in 0..names.len() { - let n = names[i].clone(); - if seen.contains(&n) { - continue; - } - seen.push(n.clone()); - let same: Vec = (0..names.len()).filter(|j| names[*j] == n).collect(); + // The pieces of each name, gathered in one pass (a faceted body has thousands of names). + let mut groups: std::collections::BTreeMap> = std::collections::BTreeMap::new(); + for (i, n) in names.iter().enumerate() { + groups.entry(n.clone()).or_default().push(i); + } + for (n, same) in groups { if same.len() < 2 { continue; } diff --git a/crates/engine/src/thread_tests.rs b/crates/engine/src/thread_tests.rs index a9227e36..efac56e4 100644 --- a/crates/engine/src/thread_tests.rs +++ b/crates/engine/src/thread_tests.rs @@ -67,3 +67,38 @@ fn modelled_internal_threads_m3_to_m20() { assert!((got - want).abs() < want * 0.03, "M{d}: removed {got}, ISO profile {want}"); } } + +/// Issue 602: rounding the edges of a hex boss on a threaded shaft froze the app. The thread's +/// thousands of facets were healed pair by pair, once per feature after it, and the fillet's +/// preview then compared each of them with the whole old body. +#[test] +fn fillet_beside_a_modelled_thread_finishes() { + let mut s = Session::default(); + for (id, p) in [ + ("sketch.create", json!({"plane": "XY"})), + ("sketch.circle.center", json!({"center": [0, 0], "diameter": 5})), + ("sketch.finish", json!({})), + ("solid.extrude", json!({"distance": 10, "body_name": "Screw"})), + ("sketch.create", json!({"plane": {"face": [0, 0, 10]}})), + ("sketch.polygon.circumscribed", json!({"center": [0, 0], "radius": 2, "sides": 6})), + ("sketch.finish", json!({})), + ("solid.extrude", json!({"distance": 3, "profiles": [{"point": [0, 0]}], "operation": "join", "targets": ["Screw"]})), + ("solid.thread", json!({"face": [2.5, 0, 5], "designation": "M5", "length": 4, "modeled": true})), + ] { + run(&mut s, id, p); + } + let faces_before = s.model.state().body("Screw").map(|b| b.body.face_count()).unwrap_or(0); + assert!(faces_before > 1000, "the thread is faceted: {faces_before} faces"); + let volume_before = volume(&mut s, "Screw"); + let top: Vec = (0..6) + .map(|i| { + let a = (30.0 + 60.0 * i as f64).to_radians(); + json!([2.0 * a.cos(), 2.0 * a.sin(), 13]) + }) + .collect(); + let started = std::time::Instant::now(); + run(&mut s, "solid.fillet", json!({"edges": top, "radius": 0.3})); + assert!(started.elapsed().as_secs() < 30, "the fillet took {:?}", started.elapsed()); + let volume_after = volume(&mut s, "Screw"); + assert!(volume_after < volume_before && volume_after > volume_before * 0.99, "{volume_before} -> {volume_after}"); +} diff --git a/crates/kernel/src/heal.rs b/crates/kernel/src/heal.rs index 52da22e2..ae464947 100644 --- a/crates/kernel/src/heal.rs +++ b/crates/kernel/src/heal.rs @@ -199,60 +199,129 @@ fn merge_coplanar(shell: &mt::Shell, tol: f64, keep: &[Vec]) -> Option<(mt Some((out.into(), from)) } -/// Join pairs of collinear straight edges at vertices only they use. +/// Join runs of collinear straight edges at vertices only they use: each run becomes one edge, +/// every face it borders is rebuilt once (linear in the shell, however long the runs; a faceted +/// body such as a modeled thread has thousands of such edges). fn merge_collinear(shell: &mt::Shell, trace: &mut Trace) -> Option { let mut faces: Vec = shell.face_iter().cloned().collect(); - let mut changed = false; - for _round in 0..10_000 { - let mut at: HashMap> = HashMap::new(); - let mut seen = std::collections::HashSet::new(); - for f in &faces { - for e in f.edge_iter() { - if seen.insert(e.id()) { - at.entry(e.front().id()).or_default().push(e.absolute_clone()); - at.entry(e.back().id()).or_default().push(e.absolute_clone()); - } - } - } - let pair = at.iter().find_map(|(v, es)| { - let [a, b] = &es[..] else { return None }; - if !matches!(a.curve(), mt::Curve::Line(_)) || !matches!(b.curve(), mt::Curve::Line(_)) { - return None; - } - let other = |e: &mt::Edge| if e.front().id() == *v { e.back().clone() } else { e.front().clone() }; - let mid = if a.front().id() == *v { a.front().clone() } else { a.back().clone() }; - let (pa, pm, pb) = (from_p3(other(a).point()), from_p3(mid.point()), from_p3(other(b).point())); - let (u, w) = ((pm - pa).normalized()?, (pb - pm).normalized()?); - (u.dot(w) > 1.0 - 1e-10).then(|| (a.clone(), b.clone(), other(a), other(b))) - }); - let Some((a, b, va, vb)) = pair else { break }; - let joined = builder::line(&va, &vb); - trace.made(&joined, [a.id(), b.id()]); - for f in &mut faces { - if !f.edge_iter().any(|e| e.id() == a.id()) { + let mut edges: Vec = Vec::new(); + let mut index: HashMap = HashMap::new(); + let mut at: HashMap> = HashMap::new(); + for f in &faces { + for e in f.edge_iter() { + if index.contains_key(&e.id()) { continue; } - let mut wires = Vec::new(); - for w in f.absolute_boundaries() { - let mut es: Vec = Vec::new(); - for e in w.edge_iter() { - if e.id() == b.id() { - continue; - } - if e.id() == a.id() { - // a+b (in either order) become the joined edge, in this wire's direction. - es.push(if e.front() == &va { joined.clone() } else { joined.inverse() }); - continue; - } - es.push(e.clone()); - } - wires.push(mt::Wire::from(es)); - } - *f = absolute_face(f, wires)?; + index.insert(e.id(), edges.len()); + at.entry(e.front().id()).or_default().push(edges.len()); + at.entry(e.back().id()).or_default().push(edges.len()); + edges.push(e.absolute_clone()); } - changed = true; } - changed.then(|| faces.into()) + // Edges that continue each other in line through a vertex nothing else uses. + let mut parent: Vec = (0..edges.len()).collect(); + let mut any = false; + for (v, es) in &at { + let [ia, ib] = es[..] else { continue }; + let (Some(a), Some(b)) = (edges.get(ia), edges.get(ib)) else { continue }; + if !matches!(a.curve(), mt::Curve::Line(_)) || !matches!(b.curve(), mt::Curve::Line(_)) { + continue; + } + let other = |e: &mt::Edge| if e.front().id() == *v { e.back().clone() } else { e.front().clone() }; + let mid = if a.front().id() == *v { a.front().clone() } else { a.back().clone() }; + let (pa, pm, pb) = (from_p3(other(a).point()), from_p3(mid.point()), from_p3(other(b).point())); + let (Some(u), Some(w)) = ((pm - pa).normalized(), (pb - pm).normalized()) else { continue }; + if u.dot(w) > 1.0 - 1e-10 { + let (ra, rb) = (find(&mut parent, ia), find(&mut parent, ib)); + if ra != rb + && let Some(x) = parent.get_mut(ra) + { + *x = rb; + any = true; + } + } + } + if !any { + return None; + } + let mut runs: HashMap> = HashMap::new(); + for i in 0..edges.len() { + let r = find(&mut parent, i); + runs.entry(r).or_default().push(i); + } + // Each run's joined edge, and the position of its vertices along it. + let mut joined: Vec<(mt::Edge, HashMap)> = Vec::new(); + let mut run_of: HashMap = HashMap::new(); + for members in runs.values().filter(|m| m.len() > 1) { + let mut ends: HashMap = HashMap::new(); + for i in members { + let Some(e) = edges.get(*i) else { continue }; + for v in [e.front(), e.back()] { + ends.entry(v.id()).or_insert((0, v.clone())).0 += 1; + } + } + let mut ends = ends.into_values().filter(|(n, _)| *n == 1).map(|(_, v)| v); + let (Some(first), Some(last), None) = (ends.next(), ends.next(), ends.next()) else { continue }; + // Walk the run from one end, collecting its vertices in order. + let mut order = vec![first.id()]; + let (mut cur, mut prev) = (first.id(), usize::MAX); + for _ in 0..members.len() { + let next = at.get(&cur).and_then(|es| es.iter().copied().find(|i| *i != prev && members.contains(i))); + let Some(e) = next.and_then(|i| edges.get(i).map(|e| (i, e))) else { break }; + cur = if e.1.front().id() == cur { e.1.back().id() } else { e.1.front().id() }; + prev = e.0; + order.push(cur); + } + if order.last() != Some(&last.id()) || order.len() != members.len() + 1 { + continue; + } + // Every piece must lie along the whole run, not just along its neighbour. + let Some(dir) = (from_p3(last.point()) - from_p3(first.point())).normalized() else { continue }; + let straight = members + .iter() + .filter_map(|i| edges.get(*i)) + .all(|e| (from_p3(e.back().point()) - from_p3(e.front().point())).normalized().is_some_and(|d| d.dot(dir).abs() > 1.0 - 1e-10)); + if !straight { + continue; + } + let line = builder::line(&first, &last); + trace.made_of(&line, members.iter().filter_map(|i| edges.get(*i)).map(|e| e.id())); + for i in members { + if let Some(e) = edges.get(*i) { + run_of.insert(e.id(), joined.len()); + } + } + joined.push((line, order.into_iter().enumerate().map(|(k, v)| (v, k)).collect())); + } + if joined.is_empty() { + return None; + } + for f in &mut faces { + if !f.edge_iter().any(|e| run_of.contains_key(&e.id())) { + continue; + } + let mut wires = Vec::new(); + for w in f.absolute_boundaries() { + let mut done = std::collections::HashSet::new(); + let mut es: Vec = Vec::new(); + for e in w.edge_iter() { + let Some(r) = run_of.get(&e.id()).copied() else { + es.push(e.clone()); + continue; + }; + // The run's pieces become its joined edge, in this wire's direction. + if done.insert(r) + && let Some((line, pos)) = joined.get(r) + { + let forward = pos.get(&e.front().id()) < pos.get(&e.back().id()); + es.push(if forward { line.clone() } else { line.inverse() }); + } + } + wires.push(mt::Wire::from(es)); + } + *f = absolute_face(f, wires)?; + } + Some(faces.into()) } /// The exact curve an approximate edge (a boolean's intersection curve) really is: a line or a @@ -504,10 +573,15 @@ impl Trace { /// `edge` was made from the live edges `from`. fn made(&mut self, edge: &mt::Edge, from: [mt::EdgeID; N]) { + self.made_of(edge, from); + } + + /// `edge` was made from the live edges `from`, however many. + fn made_of(&mut self, edge: &mt::Edge, from: impl IntoIterator) { if !self.on { return; } - let mut under: Vec = from.iter().flat_map(|id| self.of(id)).collect(); + let mut under: Vec = from.into_iter().flat_map(|id| self.of(&id)).collect(); under.sort_unstable(); under.dedup(); self.made.insert(edge.id(), under); @@ -571,6 +645,49 @@ fn heal_shells(solid: &Solid, size: f64, keep: &[Vec], traced: bool) -> Op #[cfg(test)] mod tests { use super::*; + use crate::Body; + + /// Distinct faces, edges and vertices of a shell (its iterators repeat shared ones). + fn counts(shell: &mt::Shell) -> (usize, usize, usize) { + let edges: std::collections::HashSet<_> = shell.edge_iter().map(|e| e.id()).collect(); + let vertices: std::collections::HashSet<_> = shell.vertex_iter().map(|v| v.id()).collect(); + (shell.face_iter().count(), edges.len(), vertices.len()) + } + + /// A prism whose front face was cut into `n` strips along a single line: the strips merge + /// into one face, which leaves `n` collinear pieces on each of its long edges. + fn strip_prism(n: usize) -> Solid { + let pts: Vec = (0..=n) + .map(|k| builder::vertex(mt::Point3::new(k as f64, 0.0, 0.0))) + .chain([builder::vertex(mt::Point3::new(n as f64, 3.0, 0.0)), builder::vertex(mt::Point3::new(0.0, 3.0, 0.0))]) + .collect(); + let edges: Vec = pts.iter().zip(pts.iter().cycle().skip(1)).map(|(a, b)| builder::line(a, b)).collect(); + let face = builder::try_attach_plane(&[mt::Wire::from(edges)]).expect("a plane"); + builder::tsweep(&face, mt::Vector3::new(0.0, 0.0, 2.0)) + } + + /// Runs of collinear edges join in one pass, however long: past the 10 000 pieces the old + /// pairwise loop stopped at, and in time (a modeled thread has thousands of such pieces, + /// and filleting beside one froze the app). + #[test] + fn long_collinear_runs_join_whole() { + let n = 12_000; + let started = std::time::Instant::now(); + let healed = heal(strip_prism(n), n as f64); + let shell = healed.boundaries().first().expect("a shell").clone(); + assert_eq!(counts(&shell), (6, 12, 8)); + let body = Body::new(healed).expect("a valid body"); + let volume = crate::measure::measure(&body).expect("measures").volume; + assert!((volume - n as f64 * 3.0 * 2.0).abs() < 1e-6 * volume, "{volume}"); + assert!(started.elapsed().as_secs() < 30, "took {:?}", started.elapsed()); + } + + #[test] + fn short_collinear_runs_join_and_keep_the_corners() { + let healed = heal(strip_prism(3), 3.0); + let shell = healed.boundaries().first().expect("a shell").clone(); + assert_eq!(counts(&shell), (6, 12, 8)); + } #[test] fn approximate_arcs_become_exact() { diff --git a/crates/ui-egui/src/preview.rs b/crates/ui-egui/src/preview.rs index f4b0b579..e14c355a 100644 --- a/crates/ui-egui/src/preview.rs +++ b/crates/ui-egui/src/preview.rs @@ -329,7 +329,10 @@ pub fn build(before: &ModelState, after: &ModelState, c: Colors) -> Built { continue; } let nf = am.tri_face.iter().max().map_or(0, |m| *m as usize + 1); - let added: Vec = (0..nf).map(|f| old.as_ref().is_none_or(|bm| face_is_new(&am, f, bm))).collect(); + let added: Vec = match &old { + Some(bm) => new_faces(&am, nf, bm), + None => vec![true; nf], + }; // Material removed from a body (a hole, a pocket): its new faces also show through the // body in the cut colour, so the cut is seen from any side. let cut = old.as_ref().is_some_and(|bm| am.measure().volume < bm.measure().volume - 1e-6); @@ -379,34 +382,111 @@ pub fn build(before: &ModelState, after: &ModelState, c: Colors) -> Built { Built { scene: sc, replaced, error: None, bounds: Aabb3::EMPTY, applied: None } } -/// Is face `f` of `m` new surface, i.e. not on the old mesh? Decided by a vote of points on it -/// (triangle centroids spread over the face). -fn face_is_new(m: &Mesh, f: usize, old: &Mesh) -> bool { - let cents: Vec = m - .triangles - .iter() - .zip(&m.tri_face) - .filter(|(_, tf)| **tf as usize == f) - .filter_map(|(t, _)| m.tri(t)) - .map(|[a, b, c]| (a + b + c) / 3.0) - .collect(); - if cents.is_empty() { - return true; +/// Which of the `nf` faces of `m` are new surface, i.e. not on the old mesh? Each is decided by +/// a vote of points on it (triangle centroids spread over the face). One pass over `m`'s +/// triangles gathers the centroids, and the old mesh is indexed once, so the cost stays +/// near-linear however many faces there are (a modeled thread has thousands). +fn new_faces(m: &Mesh, nf: usize, old: &Mesh) -> Vec { + let mut cents: Vec> = vec![Vec::new(); nf]; + for (t, f) in m.triangles.iter().zip(&m.tri_face) { + if let (Some(face), Some([a, b, c])) = (cents.get_mut(*f as usize), m.tri(t)) { + face.push((a + b + c) / 3.0); + } } - let step = cents.len().div_ceil(9); - let samples: Vec<&Vec3> = cents.iter().step_by(step.max(1)).collect(); - let off = samples.iter().filter(|p| !on_surface(old, ***p)).count(); - off * 3 > samples.len() + let index = SurfaceIndex::new(old); + cents + .iter() + .map(|cents| { + let Some(index) = &index else { return true }; + if cents.is_empty() { + return true; + } + let step = cents.len().div_ceil(9); + let samples: Vec<&Vec3> = cents.iter().step_by(step.max(1)).collect(); + let off = samples.iter().filter(|p| !index.contains(***p)).count(); + off * 3 > samples.len() + }) + .collect() } -/// Does `p` lie on the surface of mesh `m` (within its tessellation tolerance)? -fn on_surface(m: &Mesh, p: Vec3) -> bool { - let bb = m.bounds(); - let tol = (bb.diagonal() * 2e-3).max(1e-4); - if bb.is_empty() { - return false; +/// The triangles of a mesh in a uniform grid, to ask whether a point lies on the surface (within +/// its tessellation tolerance) by looking only at the triangles near it. +struct SurfaceIndex { + tris: Vec<[Vec3; 3]>, + tol: f64, + lo: [f64; 3], + cell: [f64; 3], + res: usize, + cells: Vec>, +} + +impl SurfaceIndex { + /// Most triangle-in-cell entries the grid may hold; a coarser grid is used beyond it. + const BUDGET: usize = 4_000_000; + + /// None for a mesh with no surface. + fn new(m: &Mesh) -> Option { + let bb = m.bounds(); + if bb.is_empty() { + return None; + } + let tol = (bb.diagonal() * 2e-3).max(1e-4); + let tris: Vec<[Vec3; 3]> = m.triangles.iter().filter_map(|t| m.tri(t)).collect(); + let lo = [bb.min.x - tol, bb.min.y - tol, bb.min.z - tol]; + let ext = [bb.max.x - bb.min.x + 2.0 * tol, bb.max.y - bb.min.y + 2.0 * tol, bb.max.z - bb.min.z + 2.0 * tol]; + let mut res = ((tris.len() as f64).cbrt().round() as usize).clamp(1, 48); + loop { + let cell = [ext[0] / res as f64, ext[1] / res as f64, ext[2] / res as f64]; + let mut index = SurfaceIndex { tris: Vec::new(), tol, lo, cell, res, cells: vec![Vec::new(); res * res * res] }; + let mut entries = 0usize; + 'fill: for (i, t) in tris.iter().enumerate() { + let (a, b) = index.range(t); + for x in a[0]..=b[0] { + for y in a[1]..=b[1] { + for z in a[2]..=b[2] { + entries += 1; + if entries > Self::BUDGET && res > 1 { + break 'fill; + } + if let Some(c) = index.cells.get_mut((x * res + y) * res + z) { + c.push(i as u32); + } + } + } + } + } + if entries <= Self::BUDGET || res == 1 { + index.tris = tris; + return Some(index); + } + res /= 2; + } + } + + fn cell_of(&self, v: f64, axis: usize) -> usize { + let (lo, cell) = (self.lo.get(axis).copied().unwrap_or(0.0), self.cell.get(axis).copied().unwrap_or(1.0)); + let k = ((v - lo) / cell).floor(); + if k.is_finite() { (k.max(0.0) as usize).min(self.res - 1) } else { 0 } + } + + /// The cells (inclusive corners) a triangle, grown by the tolerance, touches. + fn range(&self, t: &[Vec3; 3]) -> ([usize; 3], [usize; 3]) { + let (mut a, mut b) = ([0; 3], [0; 3]); + for axis in 0..3 { + let c = t.map(|p| [p.x, p.y, p.z][axis]); + let (lo, hi) = (c[0].min(c[1]).min(c[2]) - self.tol, c[0].max(c[1]).max(c[2]) + self.tol); + a[axis] = self.cell_of(lo, axis); + b[axis] = self.cell_of(hi, axis); + } + (a, b) + } + + /// Does `p` lie on the surface? + fn contains(&self, p: Vec3) -> bool { + let (x, y, z) = (self.cell_of(p.x, 0), self.cell_of(p.y, 1), self.cell_of(p.z, 2)); + let Some(cell) = self.cells.get((x * self.res + y) * self.res + z) else { return false }; + cell.iter().filter_map(|i| self.tris.get(*i as usize)).any(|[a, b, c]| point_triangle_dist(p, *a, *b, *c) < self.tol) } - m.triangles.iter().filter_map(|t| m.tri(t)).any(|[a, b, c]| point_triangle_dist(p, a, b, c) < tol) } pub fn point_triangle_dist(p: Vec3, a: Vec3, b: Vec3, c: Vec3) -> f64 { @@ -508,6 +588,59 @@ mod tests { s } + /// The surface test the preview used to run per face against every triangle. + fn brute_force_on_surface(m: &Mesh, p: Vec3) -> bool { + let bb = m.bounds(); + let tol = (bb.diagonal() * 2e-3).max(1e-4); + m.triangles.iter().filter_map(|t| m.tri(t)).any(|[a, b, c]| point_triangle_dist(p, a, b, c) < tol) + } + + /// A flat sheet of `n × n` squares, two triangles and one face each, lifted by `z`. + fn sheet(n: usize, z: f64) -> Mesh { + let mut m = Mesh::default(); + for i in 0..=n { + for j in 0..=n { + m.positions.push(Vec3::new(i as f64, j as f64, z)); + m.normals.push(Vec3::Z); + } + } + for i in 0..n { + for j in 0..n { + let a = (i * (n + 1) + j) as u32; + let (b, c, d) = (a + 1, a + n as u32 + 1, a + n as u32 + 2); + m.triangles.extend([[a, c, d], [a, d, b]]); + m.tri_face.extend([(i * n + j) as u32; 2]); + } + } + m + } + + /// The grid answers as a scan of every triangle does, inside, beside and far from the mesh. + #[test] + fn surface_index_matches_a_scan() { + let m = sheet(12, 0.0); + let index = SurfaceIndex::new(&m).unwrap(); + for k in 0..2000 { + let f = k as f64; + let p = Vec3::new((f * 0.37).rem_euclid(15.0) - 1.5, (f * 0.61).rem_euclid(15.0) - 1.5, ((f * 0.13).rem_euclid(1.0) - 0.5) * 0.05); + assert_eq!(index.contains(p), brute_force_on_surface(&m, p), "{p:?}"); + } + assert!(SurfaceIndex::new(&Mesh::default()).is_none()); + } + + /// Thousands of faces (a modeled thread's) are classified at once, not face by face against + /// every triangle: filleting beside a thread froze the app for minutes. + #[test] + fn many_faces_are_classified_quickly() { + let started = std::time::Instant::now(); + let n = 120; + let (old, same, lifted) = (sheet(n, 0.0), sheet(n, 0.0), sheet(n, 5.0)); + let nf = n * n; + assert!(new_faces(&same, nf, &old).iter().all(|new| !new), "faces on the old surface are not new"); + assert!(new_faces(&lifted, nf, &old).iter().all(|new| *new), "faces off the old surface are new"); + assert!(started.elapsed().as_secs() < 10, "took {:?}", started.elapsed()); + } + /// A fillet preview stands in for the body: the round face is new, the rest is kept, and the /// old body is the ghost. The session is unchanged. #[test]