Manifold STLs for unions of stacked rings; loon prisms instead of a lying segment count - #837
Merged
Merged
Conversation
Field report: exported STLs of printed parts carried hundreds to thousands of edges not shared by exactly two triangles. Bambu Studio flagged them and its auto-repair resolved them by FILLING — a printed rotor came out with no shaft hole. Vertex welding did not move the count (flat from 1e-4 to 1e-1): the defect is topological. The reported repro never reached the mesh fallback. It came out of the analytic B-rep path, from two bugs in the planar circle splitter, both about a face's HOLES: 1. Existing hole loops were routed to the disk or the ring sub-face by testing the hole's CENTROID against the splitting circle. Every concentric loop — whatever its radius — has its centroid at the shared center, so a hole larger than the circle went to the disk: a face whose hole is bigger than its own outer loop, which the tessellator draws as the full disk (a membrane over the bore). Replaced with a real containment test on the loop's vertices (`loop_vs_circle`). 2. A circle lying entirely inside one of the face's holes counted as "inside the face" — only the outer loop was consulted — splitting off a disk over the hole plus a redundant nested hole on the ring. Both produce doubled surface, which is exactly what a slicer reports. Nested annular caps are what a union-of-differences makes, which is why this only showed up on stacked rings. Measured: the reported case goes 1902 -> 0 bad edges with volume 16965.4 against 16965.8 analytic truth; a real backplate 2063 -> 22 (and those 22 are unpaired hairline seams, no doubled surface). Also, so a doubled result can never again pass silently: - MeshReport/BooleanReport gain `overused_edges`. The existing `open_edges` is a NET DIRECTED count, so it cancels to zero on a doubled surface and could not see this defect at all. - Over-used edges now open the mesh fallback on their own, with no capability flag required, and the fallback is taken only when it is strictly better: manifold and volume-agreeing. - The mesh CSG cancels coincident triangle pairs (zero-thickness flaps). A second, distinct defect is captured with its diagnosis and an #[ignore]d test in coplanar_boss_straddling_a_split.rs: a boss whose caps are flush with the body's goes non-manifold when its footprint straddles a line an earlier boolean split clear across that face (gear teeth). Bisected to a ~3 degree band opposite an earlier tooth; a small overhang avoids it (72-tooth blank: 1286 -> 31 bad edges). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rism `[cylinder-n 7.5 24.0 6]` looks like a hex boss in source review and is not one: the third argument is a fidelity hint for the boolean and seam machinery, the surface stays an analytic cylinder, and the tessellator draws a circle. That lie shipped a "hex drive" that was a plain round bore and cost a print. Worse, for a bare primitive the count never reaches tessellation at all — a cylinder exports at 32 segments whether the source says 8, 16, 48 or 96 — while lib.loon told authors to reach for these forms when bore fidelity is load-bearing. The -n forms now refuse counts under 8 and name the primitive that does make facets. The kernel already had a true faceted Prism, so this adds `polygon-prism sides radius height` and `hex-prism across-flats height` (across-flats: what a wrench measures), and rewrites the lib.loon docstring to say what the hint actually does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new over-used-edge check was applied to BOTH fallback swaps, not just the one it was written for. Five torture cases (chain-00, chain-08, chain-10, rand-072, rand-220) pass precisely BECAUSE they take the watertightness swap, and a mesh fallback that closes every crack while carrying a few over-used edges was suddenly rejected — handing back the cracked B-rep and turning them into bad-geometry. Each swap is now judged against the defect it repairs and only that one: the non-manifold swap still demands a manifold fallback, the watertightness swap is back to its original acceptance (watertight, non-empty, volume-agreeing). The watertightness swap also reports the fallback's real `overused_edges` rather than an assumed zero. Torture track: no regressions, 6 improvements vs baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The non-manifold swap this branch added rested on a claim I have now disproved: that an edge shared by more than two triangles is never legitimate geometry. Instrumenting this crate's own catalogue shows known-good results scoring up to 13 over-used edges — b1's blade cut scores 1 — against 11 and 14 for the doubled-surface defects that motivated the measurement. The populations overlap exactly the way they do for open edges, so no count separates them, and the swap did what the comment above it warns about: it traded b1's analytic r45 wall for coarse soup and lost 505 mm³ (torr_boolean_catalogue::b1_difference_dual and b1_partition_identity, both failing on Rust nightly CI). `overused_edges` stays on MeshReport and BooleanReport, as advisory as open_edges and documented that way. It earns its place by being the only number that sees this class at all — open_edges is a NET DIRECTED count, so it cancels to zero on doubled surface. Repairing that class belongs at its root in the splitters, which is where this branch's actual fix is. Also reverts the widened operand-tessellation gate, which existed only to feed the swap. torr catalogue 24/24; torture track: no regressions vs baseline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two defects found while 3D-printing real parts from loon models. One is fixed; the second is diagnosed, bisected, and captured as an
#[ignore]d test rather than left as folklore.1. Non-manifold exports from unions of stacked rings
Exported STLs carried hundreds to thousands of edges not shared by exactly two triangles. Bambu Studio reports them ("3701 non-manifold edges") and its auto-repair resolves them by filling — a printed rotor came out with no shaft hole at all. Welding vertices did not move the count (flat from 1e-4 to 1e-1 tolerance), so it is topology, not float precision.
The reported repro never reached the mesh fallback. It came out of the analytic B-rep path, from two bugs in the planar circle splitter, both about a face's holes:
loop_vs_circle: Inside / Outside / Coincident / Straddles).Nested annular caps are exactly what a union-of-differences produces, which is why this only showed up on stacked rings.
Measured
pipe(ann(8,46,2.5), union(ann(24,27.5,1.5)))backplate.looncarrier.loonMeasurement, not repair.
MeshReport/BooleanReportgainoverused_edges. The existingopen_edgesis a net directed count, so it cancels to zero on a doubled surface — it could not see this defect at all.I first wired that up as a fallback trigger (swap in the mesh boolean whenever the B-rep result had over-used edges) and reverted it after CI caught the cost. The claim it rested on — that an over-used edge is never legitimate geometry — is false: instrumenting this crate's own catalogue shows known-good results scoring up to 13 over-used edges, b1's blade cut scoring 1, against 11 and 14 for the defects that motivated the measurement. The populations overlap exactly the way they do for open edges, and the swap did what the comment above it warns about: traded b1's analytic r45 wall for coarse soup, −505 mm³. So the count is reported and documented as advisory, and the repair stays where this PR actually puts it — at the root, in the splitters.
The mesh CSG also cancels coincident triangle pairs (zero-thickness flaps, which add no volume and so slip past the volume oracle).
2.
cylinder-n r h 6silently rendering a circle[cylinder-n 7.5 24.0 6]looks like a hex boss in source review and is not one: the third argument is a fidelity hint for the boolean/seam machinery, the surface stays an analytic cylinder, and the tessellator draws a circle. That shipped a "hex drive" that was a plain round bore and cost a print.Worse, for a bare primitive the count never reaches tessellation at all — a cylinder exports at 32 segments whether the source says 8, 16, 48 or 96 — while
lib.loontold authors to reach for these forms when bore fidelity is load-bearing.The
-nforms now fail closed under 8 and name the primitive that does make facets:The kernel already had a true faceted
Prism, so this addspolygon-prism sides radius heightandhex-prism across-flats height(across-flats — what a wrench or hex key measures;[hex-prism 13.0 24.0]verified at 13.0 mm flats, 12 verts / 20 tris), and rewrites the misleadinglib.loondocstring.Nothing in the repo or in the reporting model used a count below 8 (real usages are 16/20/24/48/64/96), so the floor breaks no existing source.
Known open defect, not fixed here
planet-72t(2133),ring-168t(1711),stator-proto(635) are unchanged — a second, distinct class: a boss whose top and bottom faces are flush with the body's goes non-manifold when its footprint straddles a line an earlier boolean split clear across that face. Gear teeth are the canonical case.Bisection: one tooth on a fresh blank is manifold at every angle; a second tooth is manifold everywhere except a ~3° band around 180° from the first — the diameter opposite it, because the first union's side-plane lines cut the cap all the way across. The doubled patch has zero net volume (a flap), which is why the volume oracle passes it.
Captured in
coplanar_boss_straddling_a_split.rswith the full diagnosis: one#[ignore]d failing test, and one passing test proving the arrangement is representable (0.5 mm of overhang takes a 72-tooth blank from 1286 to 31 bad edges). Workaround for models today: give bosses a small overhang past the body's faces. On the fullplanet-72tthat only gets 2133 → 922, so that part has further sources I did not chase.The root fix is in the pipeline, not the splitters this PR touches: either stop the line splitter cutting a face clear across where the other solid's face does not reach, or classify coplanar contact per-fragment across sub-faces.
Reviewer notes
DegradeReasongains aNonManifoldSwapvariant andBooleanReportanoverused_edgesfield — additive, but they are public API.nested_annulus_union_manifold.rs(3 cases: coplanar contact, interpenetrating, boss over the bore — each asserting manifoldness and volume, since a membrane over a bore is invisible to a volume check alone until it is filled).cargo testgreen acrossvcad-kernel-booleans,vcad-eval,vcad-kernel,vcad-kernel-tessellate,vcad-loon(38 test binaries, 0 failures); clippy andcargo fmt --checkclean.🤖 Generated with Claude Code