Skip to content

Split Boolean curves where they cross a curve of the other shell. Fixes the cube_cut_2 case from #1743 - #6

Open
BoykoNeov wants to merge 2 commits into
masterfrom
fix-1743-cross-vertex-split
Open

Split Boolean curves where they cross a curve of the other shell. Fixes the cube_cut_2 case from #1743#6
BoykoNeov wants to merge 2 commits into
masterfrom
fix-1743-cross-vertex-split

Conversation

@BoykoNeov

Copy link
Copy Markdown
Owner

Written by Claude Opus 5 — both this text and the code it describes; posted by @BoykoNeov.

Fixes the cube_cut_2 case from solvespace#1743. Two commits on top of master 790bf74c.

Root cause

@phkahler's hypothesis in solvespace#1743 is right, and this is the trace that proves it.

Where the two shells touch rather than pass through each other, a curve of one shell can cross a curve of the other at a point that is a vertex of neither input shell, and that lies on no surface which either curve crosses transversally. SCurve::MakeCopySplitAgainst() splits curves against surfaces, so at such a point it splits only whichever of the two curves does meet a transversal surface there, and leaves the other one whole.

The unsplit curve then contributes a single trim edge whose two halves classify differently. That cannot be represented: MakeCopyTrimAgainst() assembles the trim polygon from chains that FindChainAvoiding() builds, classifies each chain from one representative edge, and keeps or discards it whole. So the edge is lost along with the rest of its chain, AssemblePolygon() fails, and the face goes missing.

@ruevs' simplified model is the clearest example. A 2×2×2 triangular prism, minus a tool swept along a bezier profile from (1,0) to (0,1), z ∈ [−0.5, 0.5]. The tool's flat base face lies in the plane y=0, coincident with the prism's y=0 face, and the profile is tangent to y=0 at its (1,0) end. So the tool's corner edge x=1, y=0 runs within the prism's y=0 face, and crosses the prism's own bottom-front edge y=0, z=0 at (1,0,0).

The tool's edge does get split there — it crosses the prism's z=0 face transversally. The prism's edge does not: the only tool surfaces it meets at x=1 are the coplanar base face and the tangent swept face, neither of which it crosses. An instrumented run then shows the whole 5-edge boundary of the prism's y=0 face arriving as one chain:

ORIGCHAIN A3 n=5 (0,0,0.5)->(2,0,2) ins=CSAME outs=CSAME KEEP=0
...
final: 2 edges
failed: I=3, avoid=3

CSAME is correct for the half of that boundary inside the coincident patch (x ≤ 1) and wrong for the half outside it — which is exactly the situation the missing split creates.

The asymmetry within the same model is worth noting, because it shows the rule at work: at the profile's other end the tool's third face crosses the prism's x=0 face transversally, so that prism edge did get split, at y=1. Only the touching end fails.

The fix

SShell::SplitCurvesAtCrossings(), called from MakeFromBoolean() immediately after the two CopyCurvesSplitAgainst() calls: collect the interior vertex == true points of every source-A curve and every source-B curve, and insert each into the other operand's curves wherever it lies within one of their piecewise linear segments (axis-aligned bounding box prefilter, strict 0 < t < 1, within LENGTH_EPS of the chord) and — for exact curves — within LENGTH_EPS of the exact curve, since the chord of a curved segment is not the curve.

Interior points only, deliberately. A curve's endpoints are its own shell's vertices, which FindVertsOnCurve() already propagates from the trims, and an exact intersection curve's endpoints may lie outside the real geometry entirely — which is what solvespace#1452 was about. test/group/boolean_coplanar_union stays green.

It runs before MakeIntersectionCurvesAgainst(), and RemoveShortSegments() runs after it, so a point landing very close to an existing one cannot leave a degenerate segment behind (the Equals check already rejects anything within LENGTH_EPS of an existing point, so the shortest new segment is longer than that).

Regression test

test/group/boolean_cross_vertex/@phkahler's cube_cut_2 from the issue, unmodified. Watertight, not self-intersecting, and a volume check.

The expected volume is the value the same model gives when built as a mesh Boolean instead of a NURBS one (Group.forceToMesh): 203485.224213202 against the NURBS result's 203485.224213201, which is agreement to 2.9e-15 relative. That is the real validation — the constant is not just "whatever this build printed".

No CHECK_SAVE/CHECK_RENDER, following the other Boolean cases here: the surface cache holds last-ulp floats that PrepareSavefile does not normalize.

The committed test against unmodified boolean.cpp fails (inters) = true with two failed to assemble polygon to trim nurbs surface in uv space messages.

Verification

model master this branch mesh-Boolean truth
cube_cut_2 28 tris / 171862.60 / 17 naked 38 / 202243.1316 / 0 naked 56 / 202243.1315 / 0
ruevs' simplified 22 / 10 naked 24 / 6 naked 48 / 0 naked

What this does not fix

ruevs' simplified model still has 6 naked edges on this branch alone. It needs a second, independent fix — the tangent-crossing classification in ClassifyEdge() — and with that stacked on top it comes out clean at 28 triangles, 0 naked. The two defects are genuinely separate: cube_cut_2 is fixed by this change alone, and the classification change alone leaves cube_cut_2 at 17 naked edges.

curve_curve.slvs is a third, different defect, and this does not touch it. It is not the original-curve split at all. The failing curves are the two intersection curves on the plane y = 21.873392, which should each be split where they cross the box's x=0 face at z = ∓13.164015. One of them is, by AllPointsIntersecting() inside surfinter.cpp's own MakeCopySplitAgainst() call — leaving a spurious segment 1.9e-5 long. Its mirror image is not: its nearest piecewise linear point sits 2.5e-5 away from the point it needs. FindVertsOnCurve() is not involved anywhere on this model; instrumented, it yields zero candidates. So the gap is about 1e-5 wide and LENGTH_EPS is 1e-6 — propagating vertices cannot bridge it, and closing it means changing a tolerance, which I did not want to do speculatively.

So the two models @phkahler attached are not two instances of one cause. cube_cut_2's two missing faces are (that is the "two of them for the same reason"); curve_curve is a separate accuracy problem in the intersection-curve split.

One more thing worth knowing about curve_curve: it is OpenMP-nondeterministic on master and on every branch here, cycling through four outcomes (88 tris/4 naked, 75/13, 71/15, 63/17) and deterministic only with OMP_NUM_THREADS=1. Single-run comparisons on that model will mislead whoever picks it up.


If this looks right, please fetch and fast-forward rather than using the merge button, so the history stays linear for upstream.

BoykoNeov and others added 2 commits July 29, 2026 15:43
Where the two shells touch without passing through each other, a curve
of one shell can cross a curve of the other at a point that is a vertex
of neither input shell, and that lies on no surface which either curve
crosses transversally. SCurve::MakeCopySplitAgainst() splits curves
against surfaces, so at such a point it splits only whichever of the two
curves does meet a transversal surface there, and leaves the other one
whole.

The unsplit curve then contributes a single trim edge whose two halves
classify differently. That cannot be represented: the trim polygon is
assembled from chains that are kept or discarded whole, so the edge is
lost along with the rest of its chain, the polygon fails to assemble,
and the face goes missing.

A tool swept along a profile that is tangent to a face of the workpiece
does exactly this, when the tool's cap face is coincident with that face
of the workpiece: the tool's tangent edge runs within the workpiece's
face and crosses the workpiece's own edge there. The tool's edge does
cross the workpiece's face transversally, so it gets split; the
workpiece's edge meets only the coincident cap face and the tangent
swept face, and so does not.

So propagate the splits after making them. Any vertex that splitting
created within a curve from one operand, and that lies within a curve
from the other operand, becomes a vertex of that curve too. Only the
interior points are propagated: the endpoints of a curve are its own
shell's vertices, which FindVertsOnCurve() already handles, and an exact
curve's endpoints may lie outside the real geometry entirely, which is
what issue solvespace#1452 was about.

Fixes the cube_cut_2 case from solvespace#1743.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The fixture is phkahler's cube_cut_2 from the bug report, unmodified: a
cube minus a tool swept along a spline profile that is tangent to one of
the cube's faces, with the tool's cap face coincident with that same
face. Two faces of the result went missing, for the one reason.

The expected volume is the value that the same model gives when it is
built as a mesh Boolean instead of a NURBS one, which agrees with the
NURBS result to twelve significant figures once the faces are there.
There is no CHECK_SAVE or CHECK_RENDER: the Boolean surface cache holds
last-ulp floats that PrepareSavefile does not normalize.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ruevs

ruevs commented Jul 29, 2026

Copy link
Copy Markdown

ruevs' simplified model still has 6 naked edges on this branch alone. It needs a second, independent fix — the tangent-crossing classification in ClassifyEdge() — and with that stacked on top it comes out clean at 28 triangles, 0 naked. The two defects are genuinely separate: cube_cut_2 is fixed by this change alone, and the classification change alone leaves cube_cut_2 at 17 naked edges.

I can confirm. This alone fixes the "left side":

image

Combined with #5 fix-1291-tangent-crossing both sides are fixed:

image

The model: solvespace#1743 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants