PR #1751 fallback fixes fron Claude Opus 5 - #1760

From here #1751 (comment) squashed into one commit and rebased on master after merging #1751.

Original text:

First, your question — you're right, and I'm withdrawing two claims

what do you mean by that? For a fillet surface the intersection is exactly on the boundary by construction.

You're right and I can't defend what I wrote. "A boundary hit is precisely the degenerate case for both ray-cast parity and trim assembly" was too strong. Your fillet is the counterexample: the intersection is on the boundary by construction, and finding it is the whole point of the change. Being on a boundary is not the defect.

What I actually had evidence for was much narrower — on these models, the hits that broke things happened to land at endpoints. I generalised that into a statement about boundaries in general, which the evidence didn't support. I've since found what the defect really is, and it isn't where the hit lands.

Second correction from that same comment, which I found while checking the first: I said the uv dedup lets three hits on one edge survive into the trim. That's wrong. Those three were three separate ray casts, and the dedup only runs within a single call, so it never had the opportunity to merge them and isn't at fault. I shouldn't have made the claim.

Where each of your five stands

Both commits sit directly on 70b444c3, this PR's head as it stands — nothing rebased, so they fast-forward:

git fetch https://github.com/BoykoNeov/solvespace pr1751-fallback-fixes
git cherry-pick 62d92299 f70c58ad     # or just merge FETCH_HEAD

1. The parallel guard (your #2)

The test had two holes.

It misses antiparallel. When the two directions point opposite ways d1.Dot(d2) is -1, so fabs(1.0 - d1.Dot(d2)) evaluates fabs(2.0) and nothing is rejected. The edge direction comes off the surface's control grid, which always runs in increasing u and v, while p0 -> p1 is whatever the caller had; which of the two relative orientations you get is arbitrary.

And it gates on degree where the property is geometric. deg + curve->deg == 2 admits only a pair of degree-one curves, but a Bézier whose control points are collinear is a straight line whatever its degree and whatever its weights, and the edge EdgeCurveIntersection() hands you carries whatever degree the surface has in that direction, not the degree its shape deserves.

One test closes both: the magnitude of the cross product of the two unit directions is the sine of the angle between them, which is zero for parallel and antiparallel alike. It's also the quantity that actually matters, since Vector::ClosestPointBetweenLines() divides by its square — that's why the answer is worthless in this case, the division is 0/0. And it's linear in the angle near zero where 1 - cos(angle) is quadratic, so I set the tolerance to the same angle the cosine form accepted at 10*RATPOLY_EPS, about 4.5e-4 rad, rather than reusing that constant, which against a sine would have meant a cone 4500 times narrower.

This changes nothing I can measure, and I'd rather say so than claim a fix. Meshes from 1291_1743_cube_cut_tangent_outside_still_fails_simplified.slvs, cube_cut_2.slvs and curve_curve.slvs are byte-identical with and without it, and the suite is unchanged, OpenMP on and off. On the first model the guard rejects ten line pairs, every one exactly parallel and same-facing — so the old test caught them too. And the cubic edges that used to slip past the degree gate turn out not to be straight at all: they bow 0.354 mm off a 1.41 mm chord. Both holes are real but latent on the models I have. Robustness, not behaviour.

2. The regression, which is the part that matters

boolean_tangent_crossing — the test in master from #1291 — fails on this branch. Here is why, and it is not about boundaries.

PointOnThisAndCurve() is returning the guess it was given. It stops as soon as its two points agree to within RATPOLY_EPS. Where the edge runs tangent to the line being cast, that is already true of the seed it starts from, so it converges on the first iteration and hands the seed straight back. Six of the ten fallback hits on that model are of this kind, and they aren't close calls:

Eight orders of magnitude between the two populations, so the threshold isn't a delicate one.

This is the same degeneracy your guard was written for. You had it right in the thread — "it was somehow passing the test because the lines were coincident." Your guard misses these because it asks whether a curve is straight globally, and here the curves are genuinely curved cubics that happen to run tangent to the line at the point where they meet it. Local, not global.

Why one manufactured point wrecks the model. What comes back isn't an intersection, it's the centre of whatever sub-patch AllPointsIntersectingUntrimmed() had subdivided down to, projected onto the surface. It lands 1.3e-6 to 2.5e-6 mm from the vertex that's really there — and LENGTH_EPS is 1e-6, so that's the worst distance it could have picked: too close to be a second feature, too far to be recognised as the same one. Then, in order:

1. The endpoint test in AllPointsIntersecting() would have dropped these — this is your Add cxx feature cxx_noexcept #3, and you're right that it's the place to look — but its tolerance is LENGTH_EPS/bam and it's outrun by a factor of 1.75 to 2.5.
2. MakeCopySplitAgainst() splits a trim curve twice, within 2e-8 of uv.
3. That leaves a zero-length trim edge, and the Boolean fails.

So on #3: the endpoint filter isn't missing, it's being beaten. Tightening it means picking a tolerance larger than the error of a point that shouldn't exist, which I don't think is winnable. Better not to manufacture the point.

The fix is to require, after PointOnThisAndCurve() succeeds, that the curves actually cross at the point it found — same sine test, same tolerance as the global check. Commit 2.

• boolean_tangent_crossing passes again; whole suite 264 cases, 937 checks green, OpenMP on and off, matching master.
• cube_cut_2.slvs — still fixed, mesh byte-identical to this branch. The new test never fires on it.
• 1291_1743_…simplified.slvs — still fixed, mesh byte-identical. The new test fires four times and changes nothing, so those four hits were doing no work.
• boolean_tangent_spline — fires three times, stays green.

3. A separate bug I hit on the way, worth its own issue

The zero-length trim edge in step 3 above is fatal to AssemblePolygon(), and I think that's a pre-existing hole rather than anything to do with this PR. AssembleContour() searches for a continuation edge before it tests whether the contour has closed, so an edge whose start and end are the same point can never assemble — the search for a successor fails first. And nothing culls one: CullExtraneousEdges() only removes duplicate and antiparallel pairs. So any code path that produces a degenerate trim edge takes the Boolean down, with failed: as the only symptom.

If I remove those edges by hand the Boolean assembles, but the mesh still leaks — the same vertex ends up at two positions 4e-6 mm apart and won't weld. So culling is a band-aid; the real fix is upstream, which is commit 2. But the assembler's inability to survive a degenerate edge seems worth hardening regardless.

4. The limit of my fix, and a question back to you

I've overstated things once on this thread already, so plainly: this shows the new rule breaks none of the models I have. It doesn't show no model needs a tangential hit.

The one shape it rejects is a boundary curve running tangent to the line being cast, at a point where the touch genuinely has to be found. And that is uncomfortably close to your own motivating case — so, concretely: on your fillet, does the boundary curve run tangent to the cast line at the intersection, or does it cross it? If it runs tangent, commit 2 is wrong for you and I'd want the model to work against. If it crosses — the surface is tangent but the boundary curve isn't — then the two cases are cleanly separable and I think this holds.

That's the one thing I can't answer from here, and it decides whether commit 2 is right.

5. Your #5, and an offer

The exact-curve refinement step is untouched. I wanted the regression understood before adding another change on top, and the same question above governs it — if a tangential touch is sometimes legitimate, the criterion for both places needs to be different from the one I've used.

Standing offer from before still holds: send me any patch and I'll run it against the full suite, both OpenMP settings, plus your three models, and report whatever it says.

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