feat(loft): preserve curved corners for two-profile lofts#797
Conversation
Loft previously sampled every profile to a polygon and built straight Line ring edges with planar side faces, so a rounded-rect loft became an octagonal frustum. A coincident-cap fuse of such a faceted frustum against an arc-cornered solid could then never annihilate the cap cleanly (the long-open fuse_coincident_rrect_cap_with_frustum / #790-residual case). This adds a curve-preserving path for two-profile lofts: when both profiles share the same boundary edge structure (same edge count, matching per-edge curve type) and wind CCW, build the loft preserving each edge's curve — ring edges keep their EdgeCurve, arc corners become ruled side faces, and the caps are the real arc-edged profile wires. Mirrors the reference BRepFill_Generator: - Coaxial corner arcs (centers differ only along the axis) become an exact Cylinder (equal radii) or Cone (unequal radii) — analytic surfaces the boolean handles robustly. - Genuinely non-coaxial corner arcs become a degree-(1,2) ruled NURBS. Line edges keep planar quad side faces. Orientation is set per face against the outward direction. Scope: gated to two profiles (a single ruled band). Multi-section curve lofts (e.g. a 5-section gridfinity lip) chain curved bands whose downstream cut/fuse/shell sequences hit near-degenerate corner-junction robustness gaps; they fall back to the existing polygon loft, which already handles them. So gridfinity and every other multi-profile loft are unchanged. Closes fuse_coincident_rrect_cap_with_frustum (now a passing test, was #[ignore]d). Adds loft_rounded_rect_preserves_arc_corners (10 faces, 4 NURBS arc corners, watertight). Zero regressions: operations 664, algo 113, io 156, wasm 211 (gridfinity 24).
WASM Binary Size
|
There was a problem hiding this comment.
Pull request overview
Adds a curve-preserving loft path for the two-profile case so profiles with circular-arc corners (e.g., rounded rectangles) loft into true curved frustums instead of being faceted into polygonal frustums, improving downstream boolean robustness for coincident cap fuses.
Changes:
- Introduces a gated two-profile “matching boundary structure” loft path that reuses per-edge
EdgeCurve(Line/Circle) and builds ruled side faces (analytic Cylinder/Cone when coaxial, otherwise ruled NURBS). - Builds caps directly from the curve-preserving ring edges (so arc-cornered profile wires remain arc-cornered).
- Un-ignores and updates the coincident rounded-rect cap fuse boolean test now that the loft preserves arc corners.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/operations/src/loft.rs | Adds curve-preserving two-profile loft implementation, including ruled arc side faces and a new rounded-rect loft test. |
| crates/operations/src/boolean/tests.rs | Re-enables and updates documentation for fuse_coincident_rrect_cap_with_frustum now that loft no longer facets rounded-rect corners. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let Some(surf) = | ||
| ruled_arc_surface(c0, p0s, p0e, c1, p1s, profs[s + 1][i].end) | ||
| else { | ||
| return Ok(None); | ||
| }; |
|
Addressed the Copilot review in 16031c5: |
Greptile SummaryThis PR adds a curve-preserving fast path (
Confidence Score: 4/5Safe to merge for the targeted two-profile rounded-rect case; the multi-profile polygon path is completely untouched. The wire topology, cone/cylinder formula, and pre-mutation surface computation are all correct. Two small findings — a duplicated centroid helper and a coaxial axis check that admits anti-parallel normals — are unlikely to affect any real geometry but worth tidying. crates/operations/src/loft.rs — specifically the Important Files Changed
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
crates/operations/src/loft.rs:543-553
**Duplicated centroid helper**
The `centroid` closure (with its `#[allow(clippy::cast_precision_loss)]`) replicates `crate::winding::polygon_centroid` which already takes `&[Point3]` and returns `Point3`. Using the existing utility would remove the suppression annotation and the duplication.
### Issue 2 of 2
crates/operations/src/loft.rs:420-423
**Coaxial check accepts anti-parallel arc normals**
`.abs()` on the dot product lets arcs whose normals are anti-parallel (one `+Z`, one `-Z`) pass the coaxial gate and fall into the Cylinder/Cone branch. The resulting surface would have its axis direction flipped relative to the wire winding, potentially producing an inside-out face. In practice, a CCW rounded-rect profile will always have all corner-arc normals pointing the same direction, so this is unlikely to fire — but an explicit same-sign check (`c1.normal().normalize().ok()?.dot(n) > 1.0 - tol.angular`) would prevent any possibility of a mis-oriented analytic patch.
Reviews (1): Last reviewed commit: "refactor(loft): pre-compute side surface..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #[allow(clippy::cast_precision_loss)] | ||
| let centroid = |pts: &[Point3]| -> Point3 { | ||
| let (mut x, mut y, mut z) = (0.0, 0.0, 0.0); | ||
| for p in pts { | ||
| x += p.x(); | ||
| y += p.y(); | ||
| z += p.z(); | ||
| } | ||
| let inv = 1.0 / pts.len() as f64; | ||
| Point3::new(x * inv, y * inv, z * inv) | ||
| }; |
There was a problem hiding this comment.
The centroid closure (with its #[allow(clippy::cast_precision_loss)]) replicates crate::winding::polygon_centroid which already takes &[Point3] and returns Point3. Using the existing utility would remove the suppression annotation and the duplication.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/operations/src/loft.rs
Line: 543-553
Comment:
**Duplicated centroid helper**
The `centroid` closure (with its `#[allow(clippy::cast_precision_loss)]`) replicates `crate::winding::polygon_centroid` which already takes `&[Point3]` and returns `Point3`. Using the existing utility would remove the suppression annotation and the duplication.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| let n = c0.normal().normalize().ok()?; | ||
| // Both arcs must share the same axis direction. | ||
| if (c1.normal().normalize().ok()?.dot(n)).abs() < 1.0 - tol.angular { | ||
| return None; |
There was a problem hiding this comment.
.abs() on the dot product lets arcs whose normals are anti-parallel (one +Z, one -Z) pass the coaxial gate and fall into the Cylinder/Cone branch. The resulting surface would have its axis direction flipped relative to the wire winding, potentially producing an inside-out face. In practice, a CCW rounded-rect profile will always have all corner-arc normals pointing the same direction, so this is unlikely to fire — but an explicit same-sign check (c1.normal().normalize().ok()?.dot(n) > 1.0 - tol.angular) would prevent any possibility of a mis-oriented analytic patch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/operations/src/loft.rs
Line: 420-423
Comment:
**Coaxial check accepts anti-parallel arc normals**
`.abs()` on the dot product lets arcs whose normals are anti-parallel (one `+Z`, one `-Z`) pass the coaxial gate and fall into the Cylinder/Cone branch. The resulting surface would have its axis direction flipped relative to the wire winding, potentially producing an inside-out face. In practice, a CCW rounded-rect profile will always have all corner-arc normals pointing the same direction, so this is unlikely to fire — but an explicit same-sign check (`c1.normal().normalize().ok()?.dot(n) > 1.0 - tol.angular`) would prevent any possibility of a mis-oriented analytic patch.
How can I resolve this? If you propose a fix, please make it concise.🤖 I have created a release *beep* *boop* --- ## [2.103.0](v2.102.13...v2.103.0) (2026-06-14) ### Features * **loft:** preserve curved corners for two-profile lofts ([#797](#797)) ([29ea1b3](29ea1b3)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: brepkit[bot] <265643962+brepkit[bot]@users.noreply.github.com>
Summary
loftsampled every profile to a polygon and built straightLinering edges with planar side faces, so a rounded-rect loft became an octagonal frustum. A coincident-cap fuse of such a faceted frustum against an arc-cornered solid could then never annihilate the cap cleanly — the long-openfuse_coincident_rrect_cap_with_frustum(#790-residual) case.This adds a curve-preserving path for two-profile lofts. When both profiles share the same boundary edge structure (same edge count, matching per-edge curve type) and wind CCW, the loft preserves each edge's curve: ring edges keep their
EdgeCurve, arc corners become ruled side faces, and the caps are the real arc-edged profile wires. Mirrors the referenceBRepFill_Generator:Cylinder(equal radii) /Cone(unequal radii) — analytic surfaces the boolean handles robustly.Lineedges keep planar quad side faces. Orientation is set per face against the outward direction.Scope (deliberately conservative)
Gated to two profiles (a single ruled band). Multi-section curve lofts (e.g. a 5-section gridfinity lip) chain curved bands whose downstream cut/fuse/shell sequences hit near-degenerate corner-junction robustness gaps — those fall back to the existing polygon loft, which already handles them. So gridfinity and every other multi-profile loft are unchanged.
The deeper multi-section curved-corner boolean robustness (cut → fixed via cone/cylinder recognition on a WIP branch; fuse → isolated to near-degenerate tiny-corner junctions) is tracked for a future effort.
Testing
fuse_coincident_rrect_cap_with_frustum(now passing, was#[ignore]d).loft_rounded_rect_preserves_arc_corners(10 faces, 4 NURBS arc corners, watertight, euler=2).Review note
Contained to the loft (operations L3), gated to the two-profile single-band case. It does feed curved-corner solids into the boolean, but only the well-conditioned single-band geometry the FF-trimming + edge-merge from #795 already handle. Not set to auto-merge — flagging for review given the history.