1b-agent/solidcraft-upstream
1
0 Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: filleting beside a modeled thread no longer freezes the app (#618)

Browse Source 
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.
This commit is contained in:
Lasse Lian authored and GitHub committed 2026-10-13 05:36:22 -04:00
1 parent 0f0bb09858
commit f0d80a1d7e
4 files changed
+365 -82
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/doc/src/naming.rs
+6 -8
View File
@@ -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<Vec3>], mark: char) {
let mut seen: Vec<String> = 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<usize> = (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<String, Vec<usize>> = 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;
}
crates/engine/src/thread_tests.rs
+35
View File
@@ -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<Value> = (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}");
}
crates/kernel/src/heal.rs
+166 -49
View File
@@ -199,60 +199,129 @@ fn merge_coplanar(shell: &mt::Shell, tol: f64, keep: &[Vec<Vec3>]) -> 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<mt::Shell> {
let mut faces: Vec<mt::Face> = shell.face_iter().cloned().collect();
let mut changed = false;
for _round in 0..10_000 {
let mut at: HashMap<mt::VertexID, Vec<mt::Edge>> = 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<mt::Edge> = Vec::new();
let mut index: HashMap<mt::EdgeID, usize> = HashMap::new();
let mut at: HashMap<mt::VertexID, Vec<usize>> = 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<mt::Edge> = 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<usize> = (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<usize, Vec<usize>> = 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<mt::VertexID, usize>)> = Vec::new();
let mut run_of: HashMap<mt::EdgeID, usize> = HashMap::new();
for members in runs.values().filter(|m| m.len() > 1) {
let mut ends: HashMap<mt::VertexID, (usize, mt::Vertex)> = 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<mt::Edge> = 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<const N: usize>(&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<Item = mt::EdgeID>) {
if !self.on {
return;
}
let mut under: Vec<usize> = from.iter().flat_map(|id| self.of(id)).collect();
let mut under: Vec<usize> = 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<Vec3>], 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<mt::Vertex> = (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<mt::Edge> = 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() {
crates/ui-egui/src/preview.rs
+158 -25
View File
@@ -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<bool> = (0..nf).map(|f| old.as_ref().is_none_or(|bm| face_is_new(&am, f, bm))).collect();
let added: Vec<bool> = 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<Vec3> = 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<bool> {
let mut cents: Vec<Vec<Vec3>> = 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<Vec<u32>>,
}
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<SurfaceIndex> {
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]