Loft: preserve holes through matching sketch sections (#2435) (#2447)

Rebase the complete PR change onto current main, preserving changes previously stored in merge commits.

ROADMAP.md

@@ -466,7 +466,9 @@ completed drawings publish atomically. The browser still exports synchronously.
   filleted boxes, bosses, domes, countersinks; corners where four or more planes meet, such as a
   floor falling to a drain, as long as the corner's edges are all convex or all concave); draft on convex planar bodies and on walls (planes, cylinders → cones) between caps; lofts through three or more sections are smooth (B-spline sides through every section), two
   sections ruled. Circular-meridian barrels retain curved inner walls through either axial
-  opening (#2421).
+  opening (#2421). Solid lofts preserve holes through sections with matching hole counts
+  (#2435), including inner walls and caps with openings; a hole cannot appear or disappear
+  between sections, and sections with holes cannot collapse to a point.
 
 - Sketch corner tools: fillet and all three chamfer modes pick a connected corner even when
   its endpoint overlaps the fixed origin or a loose point. Ordinary drawing still snaps to

crates/doc/src/eval.rs

@@ -1472,15 +1472,12 @@ pub(crate) fn val(vals: &BTreeMap<String, Value>, e: &str, k: Kind) -> Result<f6
     Document::eval_in(vals, e, k)
 }
 
-/// A loft builds each section from its outline alone, so a section with a hole (a ring) is
-/// refused rather than lofted as if it were filled.
+/// Collapsing the outline and its holes onto one apex cannot make a manifold solid.
 fn refuse_loft_holes(r: &Region2) -> Result<()> {
     if r.holes.is_empty() {
         return Ok(());
     }
-    Err(DocError::Invalid(
-        "not supported yet: a loft section with a hole; loft the outline and the hole as two bodies and cut one from the other".into(),
-    ))
+    Err(DocError::Invalid("not supported yet: lofting a section with holes to a point".into()))
 }
 
 pub fn select_profiles(ss: &SolvedSketch, sel: &ProfileSel) -> Result<Vec<Region2>> {
@@ -3808,14 +3805,15 @@ fn feature_tools_with_constructor(
                     .ok_or_else(|| DocError::Unknown(format!("sketch {} (it must come earlier in the timeline)", sec.sketch)))?;
                 let regions = solidcraft_sketch::merge_regions(&select_profiles(ss, &sec.profiles)?);
                 let [r] = &regions[..] else { return Err(DocError::Invalid("each loft section must be one profile".into())) };
-                refuse_loft_holes(r)?;
-                secs.push((ss.plane, r.outer.clone()));
+                secs.push((ss.plane, r.clone()));
                 sketches.push(ss);
             }
             // Its caps are the first and last sections, its sides named by the curves they run
             // between.
-            let body = kernel::loft(&secs)?;
-            let roles = crate::lineage::SectionRoles { sections: sketches.into_iter().zip(secs.iter().map(|(_, lp)| lp)).collect() };
+            let body = kernel::loft_regions(&secs)?;
+            let roles = crate::lineage::SectionRoles {
+                sections: sketches.into_iter().zip(secs.iter().map(|(_, r)| std::iter::once(&r.outer).chain(&r.holes).collect())).collect(),
+            };
             Ok(vec![roles.tool(body)])
         }
         FeatureKind::Sweep { sketch, profiles, path_sketch, path, orientation, twist, scale, rails, rail_fit, .. } => {

crates/doc/src/eval_surface.rs

@@ -420,7 +420,7 @@ pub(super) fn eval(doc: &Document, vals: &BTreeMap<String, Value>, f: &Feature,
             let body = kernel::loft_sheet(&secs)?;
             // Each wall named by the section curves it runs between, as a loft's sides are.
             let loops: Vec<solidcraft_geom::Loop2> = secs.iter().map(|(_, c)| solidcraft_geom::Loop2 { segs: c.clone() }).collect();
-            let traced = crate::lineage::SectionRoles { sections: sketches.iter().zip(&loops).collect() }.tool(body);
+            let traced = crate::lineage::SectionRoles { sections: sketches.iter().zip(&loops).map(|(s, lp)| (s, vec![lp])).collect() }.tool(body);
             push_sheet(st, f, traced);
             Ok(())
         }

crates/doc/src/lineage.rs

@@ -302,8 +302,8 @@ impl ChainRoles<'_> {
 /// the section with the most segments (the one each side spans exactly one segment of) and
 /// answering to the others; its caps `start` and `end`.
 pub(crate) struct SectionRoles<'a> {
-    /// Each section's sketch and the loop lofted through it, in the order given.
-    pub sections: Vec<(&'a SolvedSketch, &'a solidcraft_geom::Loop2)>,
+    /// Each section's sketch and loops (outer, then holes), in the order given.
+    pub sections: Vec<(&'a SolvedSketch, Vec<&'a solidcraft_geom::Loop2>)>,
 }
 
 impl SectionRoles<'_> {
@@ -316,14 +316,19 @@ impl SectionRoles<'_> {
     fn of(&self, history: &History) -> Vec<Vec<String>> {
         // Each profile segment (numbered through the sections in order): its section and role.
         let mut segments: Vec<(usize, Option<String>)> = Vec::new();
-        for (k, (sketch, lp)) in self.sections.iter().enumerate() {
+        for (k, (sketch, loops)) in self.sections.iter().enumerate() {
             let curves = crate::config::ExactSketchCurves::new(sketch);
-            for seg in &lp.segs {
+            for seg in loops.iter().flat_map(|lp| &lp.segs) {
                 let role = curves.find_along(seg).map(|id| if k == 0 { format!("side:{id}") } else { format!("side:s{k}.{id}") });
                 segments.push((k, role));
             }
         }
-        let lead = self.sections.iter().enumerate().max_by_key(|(k, (_, lp))| (lp.segs.len(), std::cmp::Reverse(*k))).map_or(0, |(k, _)| k);
+        let lead = self
+            .sections
+            .iter()
+            .enumerate()
+            .max_by_key(|(k, (_, loops))| (loops.iter().map(|lp| lp.segs.len()).sum::<usize>(), std::cmp::Reverse(*k)))
+            .map_or(0, |(k, _)| k);
         history
             .faces
             .iter()

crates/engine/src/cmd/features.rs

@@ -27,7 +27,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SOLID", "CREATE")
         .icon("sweep")
         .params("sketch: profile sketch; profiles?; path_sketch: sketch; path: [curve ids in order]; orientation?: perpendicular (default: square to the path, turning with it) | parallel (as drawn: the profile moves without turning; the path must keep crossing its plane) | [x,y,z] (square to the path, its turn about the path fixed by this direction); twist?: angle expr (the profile turns this much about the path over its length; within 4 turns); scale?: expr (the profile's scale at the end, linear along the path; 0.01…100); rails?: [{sketch?: sketch (default path_sketch), curves: [curve ids in order]}] (guide rails, one or two, each meeting every plane square to the path once along it, from the end nearest the path's start: one rail turns the profile so its point where the rail starts stays on the rail and scales it with the rail's distance from the path; two rails carry it between them, its points where they start staying on both; rails take the default orientation, no twist and no scale, and a path without sharp corners); rail_fit?: scale (default: the profile grows and shrinks evenly) | stretch (only across to the rail, between the rails); operation?, targets?, name?, body_name?"),
-    CommandSpec::new("solid.loft", "Loft", loft).at("SOLID", "CREATE").icon("loft").params("sections: [{sketch, profiles?} | {sketch, point: id} (an apex)] (2 or more, in order); operation?, targets?, name?, body_name?"),
+    CommandSpec::new("solid.loft", "Loft", loft).at("SOLID", "CREATE").icon("loft").params("sections: [{sketch, profiles?} | {sketch, point: id} (an apex)] (2…50, in order; matching hole counts, no holes at an apex); operation?, targets?, name?, body_name?"),
     CommandSpec::new("solid.box", "Box", prim_box).at("SOLID", "CREATE").icon("box").params("length, width, height: expr; corner?: [x,y,z] | center?: [x,y,z] (numbers in mm or length expressions; default: the origin); operation?, targets?: [body], name?, body_name?"),
     CommandSpec::new("solid.cylinder", "Cylinder", prim_cylinder).at("SOLID", "CREATE").icon("cylinder").params("radius | diameter, height: expr; base?: [x,y,z] (centre of the bottom: numbers in mm or length expressions; default: the origin); axis?: X|Y|Z|[x,y,z] (default Z); operation?, targets?: [body], name?, body_name?"),
     CommandSpec::new("solid.sphere", "Sphere", prim_sphere).at("SOLID", "CREATE").icon("sphere").params("radius | diameter: expr; center?: [x,y,z] (numbers in mm or length expressions; default: the origin); operation?, targets?: [body], name?, body_name?"),

crates/engine/src/tests/loft_ring.rs

@@ -1,5 +1,5 @@
-//! A loft section that is a ring (a profile with a hole) is refused (#1184): the loft builds each
-//! section from its outline, so it used to fill the hole and report success.
+//! A loft section that is a ring keeps its passage (#2435); incompatible sections
+//! are refused atomically rather than silently filling the hole (#1184).
 
 use super::*;
 
@@ -11,24 +11,42 @@ fn ring(s: &mut Session, name: &str, z: f64) {
     run(s, "sketch.finish", json!({}));
 }
 
-fn refused(s: &mut Session, sections: Value) {
+fn refused(s: &mut Session, sections: Value, message: &str) {
     let saved = s.doc.to_json();
     let err = s.execute("solid.loft", &json!({"sections": sections, "operation": "new"})).err().map(|e| e.to_string());
-    assert!(err.as_deref().is_some_and(|e| e.contains("not supported yet: a loft section with a hole")), "{err:?}");
+    assert!(err.as_deref().is_some_and(|e| e.contains(message)), "{err:?}");
     assert_eq!(s.doc.to_json(), saved);
 }
 
 #[test]
-fn a_ring_section_is_refused_not_filled() {
+fn a_ring_section_keeps_its_hole_and_rejects_a_disc() {
     let mut s = Session::default();
     ring(&mut s, "A", 0.0);
     ring(&mut s, "B", 20.0);
     let ring_at = |n: &str| json!({"sketch": n, "profiles": [{"point": [16, 0]}]});
-    refused(&mut s, json!([ring_at("A"), ring_at("B")]));
+    let sections = json!([ring_at("A"), ring_at("B")]);
+    run(&mut s, "solid.loft", json!({"sections": sections, "operation": "new"}));
+    let want = PI * (400.0 - 144.0) * 20.0;
+    assert!(rel(volume(&mut s), want) < 1e-3);
+    let reopened = Session::new(solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap());
+    assert!(reopened.model.results.iter().all(|r| r.error.is_none()));
+    let st = reopened.model.state();
+    let body = &st.bodies[0];
+    assert!(body.body.validity().is_empty());
+    assert_eq!(body.body.shell_count(), 1);
+    for face in 0..body.body.face_count() {
+        assert!(solidcraft_doc::naming::face_name_exact(body, face), "face {face} lost its sketch-curve provenance");
+    }
+    assert!(!body.body.tessellate(0.001).unwrap().contains(solidcraft_geom::Vec3::new(0.0, 0.0, 10.0)));
+    run(&mut s, "edit.undo", json!({}));
+    assert!(s.model.state().bodies.is_empty());
+    run(&mut s, "edit.redo", json!({}));
+    assert!(rel(volume(&mut s), want) < 1e-3);
+    run(&mut s, "edit.undo", json!({}));
     // A ring and a plain disc, in either order.
     let disc_b = json!({"sketch": "B", "profiles": [{"point": [0, 0]}]});
-    refused(&mut s, json!([ring_at("A"), disc_b]));
-    refused(&mut s, json!([disc_b, ring_at("A")]));
+    refused(&mut s, json!([ring_at("A"), disc_b]), "same number of holes");
+    refused(&mut s, json!([disc_b, ring_at("A")]), "same number of holes");
     // The discs inside the holes still loft into a cylinder of radius 12.
     run(&mut s, "solid.loft", json!({"sections": [{"sketch": "A", "profiles": [{"point": [0, 0]}]}, disc_b], "operation": "new"}));
     let want = std::f64::consts::PI * 12.0 * 12.0 * 20.0;
@@ -47,5 +65,39 @@ fn a_ring_lofted_to_a_point_is_refused() {
     run(&mut s, "sketch.create", json!({"plane": "XY", "offset": 20, "name": "Top"}));
     run(&mut s, "sketch.point", json!({"point": [0, 0], "id": "apex"}));
     run(&mut s, "sketch.finish", json!({}));
-    refused(&mut s, json!([{"sketch": "A", "profiles": [{"point": [16, 0]}]}, {"sketch": "Top", "point": "apex"}]));
+    refused(
+        &mut s,
+        json!([{"sketch": "A", "profiles": [{"point": [16, 0]}]}, {"sketch": "Top", "point": "apex"}]),
+        "lofting a section with holes to a point",
+    );
+}
+
+#[test]
+fn a_ring_loft_joins_cuts_and_intersects_without_filling_the_hole() {
+    for operation in ["join", "cut", "intersect"] {
+        let mut s = Session::default();
+        run(&mut s, "solid.box", json!({"length": 50, "width": 50, "height": 10, "corner": [-25, -25, -5]}));
+        ring(&mut s, "A", 0.0);
+        ring(&mut s, "B", 20.0);
+        let sections = json!([{"sketch": "A", "profiles": [{"point": [16, 0]}]}, {"sketch": "B", "profiles": [{"point": [16, 0]}]}]);
+        run(&mut s, "solid.loft", json!({"sections": sections, "operation": operation}));
+        let area = PI * (400.0 - 144.0);
+        let want = match operation {
+            "join" => 25000.0 + area * 15.0,
+            "cut" => 25000.0 - area * 5.0,
+            _ => area * 5.0,
+        };
+        assert!(rel(volume(&mut s), want) < 1e-3, "{operation}");
+        for b in &s.model.state().bodies {
+            assert!(b.body.validity().is_empty(), "{operation}: {:?}", b.body.validity());
+        }
+    }
+}
+
+#[test]
+fn tapered_ring_loft_reproduction() {
+    let mut s = Session::default();
+    s.run_script(&serde_json::from_str(include_str!("../../../../examples/regressions/loft_rings.json")).unwrap()).unwrap();
+    let want = PI * 30.0 / 3.0 * ((400.0 + 280.0 + 196.0) - (144.0 + 96.0 + 64.0));
+    assert!(rel(volume(&mut s), want) < 1e-3);
 }

crates/kernel/src/build.rs

@@ -1816,10 +1816,135 @@ pub fn loft(sections: &[(Plane, Loop2)]) -> Result<Body> {
     })
 }
 
+/// A loft through regions with matching hole counts. Hole walls are built alongside the
+/// outline and share the end caps, so the result is one shell with open passages.
+/// Holes are paired by their positions relative to the outline, independently of sketch
+/// curve order. A hole cannot appear or disappear partway through a loft.
+pub fn loft_regions(sections: &[(Plane, Region2)]) -> Result<Body> {
+    if !(2..=50).contains(&sections.len()) {
+        return Err(KernelError::Invalid("a loft needs 2…50 sections".into()));
+    }
+    let holes = sections.first().map_or(0, |(_, r)| r.holes.len());
+    if sections.iter().any(|(_, r)| r.holes.len() != holes) {
+        return Err(KernelError::Invalid("every loft section must have the same number of holes".into()));
+    }
+    // Bound the quadratic matching work as well as the geometry built per section.
+    if holes > 128 || sections.iter().any(|(_, r)| numbered::profile_loops(std::slice::from_ref(r)).map(|l| l.segs.len()).sum::<usize>() > MAX_SEGS) {
+        return Err(KernelError::Invalid("too many loops or segments in a loft section".into()));
+    }
+    for (_, r) in sections {
+        region_ok(r)?;
+    }
+    if holes == 0 {
+        return loft(&sections.iter().map(|(p, r)| (*p, r.outer.clone())).collect::<Vec<_>>());
+    }
+    let regions: Vec<Region2> = sections.iter().map(|(_, r)| r.clone()).collect();
+    let mut numbered = numbered::number_regions(&regions);
+    let outlines: Vec<_> = sections.iter().map(|(p, _)| *p).zip(numbered.iter().map(|r| r.outer.clone())).collect();
+    let reversed = loft_reversed(&sections.iter().map(|(p, r)| (*p, r.outer.clone())).collect::<Vec<_>>())?;
+    // Match in canonical section order, so reversing the input cannot change the pairing.
+    if reversed {
+        numbered.reverse();
+    }
+    let planes: Vec<_> = if reversed { sections.iter().rev().map(|(p, _)| *p).collect() } else { sections.iter().map(|(p, _)| *p).collect() };
+    let mut previous: Vec<Vec3> = Vec::new();
+    for (p, r) in planes.iter().zip(&mut numbered) {
+        let centre = r.outer.lp.centroid();
+        let scale = r.outer.lp.signed_area().abs().sqrt().max(1e-9);
+        let position = |l: &NumberedLoop| {
+            let d = l.lp.centroid() - centre;
+            (p.x * d.x + p.y * d.y) * (1.0 / scale)
+        };
+        if !previous.is_empty() {
+            let positions: Vec<_> = r.holes.iter().map(position).collect();
+            let mut used = vec![false; positions.len()];
+            let mut order = Vec::new();
+            for point in &previous {
+                let (k, distance) = positions
+                    .iter()
+                    .enumerate()
+                    .map(|(k, p)| (k, p.dist(*point)))
+                    .min_by(|(_, a), (_, b)| a.total_cmp(b))
+                    .ok_or_else(|| KernelError::Invalid("missing loft hole".into()))?;
+                let occupied = used.get_mut(k).ok_or_else(|| KernelError::Invalid("missing loft hole".into()))?;
+                if *occupied
+                    || !distance.is_finite()
+                    || positions.iter().enumerate().any(|(j, p)| j != k && (p.dist(*point) - distance).abs() <= 1e-9)
+                {
+                    return Err(KernelError::Invalid("loft hole correspondence is ambiguous; each hole needs a unique nearest match".into()));
+                }
+                *occupied = true;
+                order.push(k);
+            }
+            let mut remaining: Vec<_> = std::mem::take(&mut r.holes).into_iter().map(Some).collect();
+            for k in order {
+                r.holes.push(remaining.get_mut(k).and_then(Option::take).ok_or_else(|| KernelError::Invalid("missing loft hole".into()))?);
+            }
+        }
+        previous = r.holes.iter().map(position).collect();
+    }
+    // Matching resamples each loop family to its largest section. Different holes can
+    // be the large one in different sketches: bound the combined result, not just each
+    // input section, before allocating the wires.
+    let mut counts = vec![0usize; holes + 1];
+    for r in &numbered {
+        for (count, lp) in counts.iter_mut().zip(r.loops()) {
+            *count = (*count).max(lp.lp.segs.len());
+        }
+    }
+    if counts.iter().sum::<usize>() > MAX_SEGS {
+        return Err(KernelError::Invalid("matching loft sections would create too many segments".into()));
+    }
+    guard("loft with holes", || {
+        let mut loops = vec![loft_numbered_wires(&outlines, reversed)?];
+        for i in 0..holes {
+            let secs = planes
+                .iter()
+                .zip(&numbered)
+                .map(|(p, r)| r.holes.get(i).cloned().map(|h| (*p, h)).ok_or_else(|| KernelError::Invalid("missing loft hole".into())))
+                .collect::<Result<Vec<_>>>()?;
+            // The holes have already been ordered with the outline.
+            loops.push(loft_numbered_wires(&secs, false)?);
+        }
+        let close = |smooth: bool| {
+            let mut sides = Vec::new();
+            let mut first = Vec::new();
+            let mut last = Vec::new();
+            for (i, (wires, sources)) in loops.iter().enumerate() {
+                let faces = if smooth { smooth_loft_faces(wires, sources, true)? } else { ruled_loft_faces(wires, sources)? };
+                let (Some(a), Some(b)) = (wires.first(), wires.last()) else { return Err(KernelError::Invalid("no loft sections".into())) };
+                if i == 0 {
+                    sides.extend(faces);
+                    first.push(a.clone());
+                    last.push(b.clone());
+                } else {
+                    sides.extend(faces.into_iter().map(|(f, source)| (f.inverse(), source)));
+                    first.push(a.inverse());
+                    last.push(b.inverse());
+                }
+            }
+            cap_and_close(sides, &first, &last, reversed)
+        };
+        if sections.len() >= 3
+            && let Ok(body) = guard("smooth loft with holes", || close(true))
+        {
+            return Ok(body);
+        }
+        close(false)
+    })
+}
+
 /// The wires of a loft's closed sections, matched: oriented alike, resampled to the same
 /// segment count and turned to start alike; with the profile segments under each wire edge, and
 /// whether the sections were turned round into the canonical order.
 pub(crate) fn loft_wires(sections: &[(Plane, Loop2)]) -> Result<(Vec<mt::Wire>, Vec<Vec<Vec<usize>>>, bool)> {
+    let reversed = loft_reversed(sections)?;
+    let ordered = sections.iter().map(|(p, _)| *p).zip(numbered::number_loops(sections.iter().map(|(_, l)| l))).collect::<Vec<_>>();
+    let (wires, sources) = guard("loft sections", || loft_numbered_wires(&ordered, reversed))?;
+    Ok((wires, sources, reversed))
+}
+
+fn loft_reversed(sections: &[(Plane, Loop2)]) -> Result<bool> {
     if sections.len() < 2 || sections.len() > 50 {
         return Err(KernelError::Invalid("a loft needs 2…50 sections".into()));
     }
@@ -1836,10 +1961,16 @@ pub(crate) fn loft_wires(sections: &[(Plane, Loop2)]) -> Result<(Vec<mt::Wire>,
     let (Some(first), Some(last)) = (sections.first(), sections.last()) else { return Err(KernelError::Invalid("no sections".into())) };
     let step = centre(last) - centre(first);
     let big = [step.x, step.y, step.z].into_iter().max_by(|a, b| a.abs().total_cmp(&b.abs())).unwrap_or(0.0);
-    let reversed = big < 0.0;
+    Ok(big < 0.0)
+}
+
+fn loft_numbered_wires(sections: &[(Plane, NumberedLoop)], reversed: bool) -> Result<(Vec<mt::Wire>, Vec<Vec<Vec<usize>>>)> {
+    let n = sections.iter().map(|(_, l)| l.lp.segs.len()).max().unwrap_or(0);
+    if n == 0 || n > MAX_SEGS {
+        return Err(KernelError::Invalid("empty section".into()));
+    }
     // The sections' segments are numbered in the order given.
-    let mut ordered: Vec<(Plane, NumberedLoop)> =
-        sections.iter().map(|(p, _)| *p).zip(numbered::number_loops(sections.iter().map(|(_, l)| l))).collect();
+    let mut ordered = sections.to_vec();
     if reversed {
         ordered.reverse();
     }
@@ -1847,6 +1978,7 @@ pub(crate) fn loft_wires(sections: &[(Plane, Loop2)]) -> Result<(Vec<mt::Wire>,
     // turns the solid outward), then match segment counts and starting points to the section
     // with the most segments.
     let n0 = ordered.first().map(|(p, _)| p.normal()).unwrap_or(Vec3::Z);
+    let parallel = ordered.iter().all(|(p, _)| p.normal().dot(n0).abs() > 1.0 - 1e-9);
     let oriented: Vec<(Plane, NumberedLoop)> = ordered
         .iter()
         .map(|(p, l)| {
@@ -1865,6 +1997,12 @@ pub(crate) fn loft_wires(sections: &[(Plane, Loop2)]) -> Result<(Vec<mt::Wire>,
         // Break angles: the lead section's vertex directions seen in this plane.
         let angles: Vec<f64> = lead_dirs.iter().map(|d| Vec2::new(d.dot(p.x), d.dot(p.y)).angle()).collect();
         let mut r = l.resample(n, &angles);
+        // Circle resampling creates increasing local angles. An antiparallel sketch
+        // plane must run clockwise locally to keep the same winding in world space.
+        // Curving a run through a right angle does not reverse a section's winding.
+        if parallel && p.normal().dot(n0) < 0.0 {
+            r = r.oriented(true);
+        }
         // Start where every vertex lines up best with the lead's vertex of the same number (the
         // largest sum of dot products of their directions from the centroids). The one vertex
         // closest in direction to the lead's first vertex twisted non-convex sections (#881): a
@@ -1880,7 +2018,7 @@ pub(crate) fn loft_wires(sections: &[(Plane, Loop2)]) -> Result<(Vec<mt::Wire>,
         wires.push(uniform_wire(p, &r.lp)?);
         edge_sources.push(r.from);
     }
-    Ok((wires, edge_sources, reversed))
+    Ok((wires, edge_sources))
 }
 
 /// Ruled faces between each two neighbouring section wires; each comes from the section edges

crates/kernel/src/lib.rs

@@ -81,6 +81,8 @@ mod issue_871_tests;
 mod join_between_caps_tests;
 mod lip;
 pub mod loft_blend;
+#[cfg(test)]
+mod loft_region_tests;
 mod loopblend;
 mod matchsurf;
 mod measure;
@@ -157,7 +159,7 @@ pub use boundary::{Boundary, boundary_patch};
 pub use boundary_network::{BoundaryCurve, CurveSource, MAX_CHAIN_CURVES, MAX_EDGE_PICKS, boundary_join, boundary_surface};
 pub use bridge::{Continuity, Frame, blend_between, bridge, chain, chain_closes, frames, frames_across, sample_grid};
 pub use build::{
-    PathSeg, box_solid, canonical_revolve_axis, cylinder, extrude, extrude_tapered, extrude_tapered_between, extrude_whole_sides, loft,
+    PathSeg, box_solid, canonical_revolve_axis, cylinder, extrude, extrude_tapered, extrude_tapered_between, extrude_whole_sides, loft, loft_regions,
     path_polyline, revolve, revolve_whole, sphere, sweep, sweep_with, torus,
 };
 pub use clearance::{Clearance, Occupancy};

crates/kernel/src/loft_region_tests.rs

@@ -0,0 +1,121 @@
+//! Analytic regressions for lofted passages (#2435).
+
+use std::f64::consts::PI;
+
+use solidcraft_geom::{Loop2, Plane, Region2, Vec2, Vec3};
+
+use crate::{Body, loft_regions, measure, step_export, step_import};
+
+fn square(half: f64) -> Loop2 {
+    Loop2::polygon(&[Vec2::new(-half, -half), Vec2::new(half, -half), Vec2::new(half, half), Vec2::new(-half, half)])
+}
+
+fn check(body: &Body, volume: f64, tol: f64) {
+    assert!(body.validity().is_empty(), "{:?}", body.validity());
+    assert_eq!(body.shell_count(), 1);
+    let got = measure(body).unwrap().volume;
+    assert!((got - volume).abs() < tol * volume, "{got} vs {volume}");
+}
+
+#[test]
+fn loft_regions_preserve_a_tapered_round_passage_and_step() {
+    let ring = |r, h| Region2 { outer: Loop2::circle(Vec2::ZERO, r), holes: vec![Loop2::circle(Vec2::ZERO, h)] };
+    let secs = [(Plane::XY, ring(20.0, 12.0)), (Plane::XY.offset(30.0), ring(14.0, 8.0))];
+    let want = PI * 30.0 / 3.0 * ((400.0 + 280.0 + 196.0) - (144.0 + 96.0 + 64.0));
+    for secs in [secs.to_vec(), secs.into_iter().rev().collect()] {
+        let body = loft_regions(&secs).unwrap();
+        check(&body, want, 1e-3);
+        assert!(!body.tessellate(0.001).unwrap().contains(Vec3::new(0.0, 0.0, 10.0)), "passage was filled");
+        let imported = step_import(&step_export(&[&body], "SolidCraft").unwrap()).unwrap();
+        assert_eq!(imported.bodies.len(), 1);
+        check(&imported.bodies[0].body, want, 1e-3);
+    }
+}
+
+#[test]
+fn loft_regions_smooth_inner_and_outer_walls_together() {
+    // Half-width 10 + 10 (2t-1)²; the hole is half as wide. Integral of the
+    // material cross-section over 40 mm is 40 * 3 * (100 + 200/3 + 100/5).
+    let ring = |r| Region2 { outer: square(r), holes: vec![square(r * 0.5)] };
+    let secs = [(Plane::XY, ring(20.0)), (Plane::XY.offset(20.0), ring(10.0)), (Plane::XY.offset(40.0), ring(20.0))];
+    for secs in [secs.to_vec(), secs.into_iter().rev().collect()] {
+        let b = loft_regions(&secs).unwrap();
+        check(&b, 22400.0, 1e-4);
+        assert!(!b.tessellate(0.001).unwrap().contains(Vec3::new(0.0, 0.0, 10.0)), "passage was filled");
+        assert_eq!(measure(&b).unwrap().merged.faces, 10, "four outer walls, four inner walls, two caps");
+    }
+}
+
+#[test]
+fn loft_regions_match_holes_in_opposite_curve_orders_and_facing_planes() {
+    let holes = vec![Loop2::circle(Vec2::new(-8.0, 0.0), 3.0), Loop2::circle(Vec2::new(8.0, 0.0), 5.0)];
+    let a = Region2 { outer: square(20.0), holes: holes.clone() };
+    // Reversing the plane's x direction mirrors local x. Opposite normals must
+    // leave the same physical holes and inward-facing walls.
+    let down = Plane::new(Vec3::new(0.0, 0.0, 20.0), -Vec3::X, Vec3::Y).unwrap();
+    let b = Region2 { outer: square(20.0), holes: vec![Loop2::circle(Vec2::new(-8.0, 0.0), 5.0), Loop2::circle(Vec2::new(8.0, 0.0), 3.0)] };
+    let secs = [(Plane::XY, a), (down, b)];
+    for secs in [secs.to_vec(), secs.into_iter().rev().collect()] {
+        let body = loft_regions(&secs).unwrap();
+        check(&body, (1600.0 - PI * 34.0) * 20.0, 1e-3);
+        let mesh = body.tessellate(0.001).unwrap();
+        assert!(!mesh.contains(Vec3::new(-8.0, 0.0, 10.0)));
+        assert!(!mesh.contains(Vec3::new(8.0, 0.0, 10.0)));
+        assert!(mesh.contains(Vec3::new(0.0, 12.0, 10.0)));
+        let history = body.history().unwrap();
+        // Both holes keep the original section/segment numbering, even after pairing.
+        for segment in 0..(8 + holes.iter().map(|h| h.segs.len()).sum::<usize>() * 2) {
+            assert!(history.faces.iter().any(|f| f.origins.contains(&crate::history::FaceOrigin::Profile { segment })), "{segment}");
+        }
+    }
+}
+
+#[test]
+fn loft_regions_refuse_missing_holes_and_hostile_sections() {
+    let a = Region2 { outer: square(20.0), holes: vec![square(10.0)] };
+    let b = Region2 { outer: square(20.0), holes: vec![] };
+    for secs in [vec![], vec![(Plane::XY, a.clone())], vec![(Plane::XY, a.clone()); 51]] {
+        assert!(loft_regions(&secs).is_err());
+    }
+    assert!(loft_regions(&[(Plane::XY, a.clone()), (Plane::XY.offset(20.0), b)]).unwrap_err().to_string().contains("same number of holes"));
+    let many = Region2 { outer: square(20.0), holes: vec![square(1.0); 129] };
+    assert!(loft_regions(&[(Plane::XY, many.clone()), (Plane::XY.offset(20.0), many)]).is_err());
+    let bad = Region2 { outer: square(20.0), holes: vec![Loop2::circle(Vec2::ZERO, f64::NAN)] };
+    assert!(loft_regions(&[(Plane::XY, a), (Plane::XY.offset(20.0), bad)]).is_err());
+}
+
+#[test]
+fn loft_regions_bound_segments_after_matching() {
+    // Each sketch fits the input budget, but independently resampling both holes
+    // would exceed it in every section. Refuse before allocating the topology.
+    let circle = |x, n| Loop2 {
+        segs: (0..n)
+            .map(|i| solidcraft_geom::Seg2::Arc {
+                center: Vec2::new(x, 0.0),
+                radius: 1.0,
+                start: i as f64 * std::f64::consts::TAU / n as f64,
+                sweep: std::f64::consts::TAU / n as f64,
+            })
+            .collect(),
+    };
+    let a = Region2 { outer: square(20.0), holes: vec![circle(-8.0, 10_001), circle(8.0, 2)] };
+    let b = Region2 { outer: square(20.0), holes: vec![circle(-8.0, 2), circle(8.0, 10_001)] };
+    let err = loft_regions(&[(Plane::XY, a), (Plane::XY.offset(20.0), b)]).unwrap_err().to_string();
+    assert!(err.contains("matching loft sections would create too many segments"), "{err}");
+}
+
+#[test]
+fn loft_regions_refuse_ambiguous_hole_pairings_in_any_draw_order() {
+    let ring = |centres: [Vec2; 2]| Region2 { outer: square(20.0), holes: centres.into_iter().map(|c| Loop2::circle(c, 1.0)).collect() };
+    let a = ring([Vec2::new(-5.0, 0.0), Vec2::new(5.0, 0.0)]);
+    // Ties, and two source holes whose unique nearest candidate is the same hole.
+    for centres in [[Vec2::new(0.0, -5.0), Vec2::new(0.0, 5.0)], [Vec2::new(4.0, 0.0), Vec2::new(10.0, 0.0)]] {
+        for centres in [centres, [centres[1], centres[0]]] {
+            let secs = [(Plane::XY, a.clone()), (Plane::XY.offset(20.0), ring(centres))];
+            for secs in [secs.to_vec(), secs.into_iter().rev().collect()] {
+                let err = loft_regions(&secs).unwrap_err().to_string();
+                assert!(err.contains("hole correspondence is ambiguous"), "{err}");
+            }
+        }
+    }
+}

examples/regressions/loft_rings.json

@@ -0,0 +1,100 @@
+{
+  "description": "#2435: a 30 mm loft from radii 20/12 to 14/8 keeps its tapered passage. Expected volume: 5720*pi mm^3. Previously refused every section containing a hole.",
+  "commands": [
+    {
+      "command": "sketch.create",
+      "params": {
+        "plane": "XY",
+        "offset": 0,
+        "name": "Bottom"
+      }
+    },
+    {
+      "command": "sketch.circle.center",
+      "params": {
+        "center": [
+          0,
+          0
+        ],
+        "radius": 20
+      }
+    },
+    {
+      "command": "sketch.circle.center",
+      "params": {
+        "center": [
+          0,
+          0
+        ],
+        "radius": 12
+      }
+    },
+    {
+      "command": "sketch.finish",
+      "params": {}
+    },
+    {
+      "command": "sketch.create",
+      "params": {
+        "plane": "XY",
+        "offset": 30,
+        "name": "Top"
+      }
+    },
+    {
+      "command": "sketch.circle.center",
+      "params": {
+        "center": [
+          0,
+          0
+        ],
+        "radius": 14
+      }
+    },
+    {
+      "command": "sketch.circle.center",
+      "params": {
+        "center": [
+          0,
+          0
+        ],
+        "radius": 8
+      }
+    },
+    {
+      "command": "sketch.finish",
+      "params": {}
+    },
+    {
+      "command": "solid.loft",
+      "params": {
+        "sections": [
+          {
+            "sketch": "Bottom",
+            "profiles": [
+              {
+                "point": [
+                  16,
+                  0
+                ]
+              }
+            ]
+          },
+          {
+            "sketch": "Top",
+            "profiles": [
+              {
+                "point": [
+                  11,
+                  0
+                ]
+              }
+            ]
+          }
+        ],
+        "operation": "new",
+        "name": "Hollow loft"
+      }
+    }
+  ]
+}