This is a combination of 2 commits.
- This is the 1st commit message:
Fix the parallel test in PointOnNonparallelCurve().
Two straight segments running along the same line are the case that
PointOnThisAndCurve() cannot answer. If they are coincident it reports
whichever point it happened to start from, and if they are merely parallel
it iterates twenty times to no purpose. The test that is supposed to reject
them has 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
happened to have, so which of the two relative orientations we 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 Bezier whose control points
are collinear is a straight line whatever its degree and whatever its
weights, and the edge that EdgeCurveIntersection() hands us carries whatever
degree the surface has in that direction, not the degree its shape deserves.
So add SBezier::IsLine(), which reports collinear control points and hands
back the direction, and gate on that instead.
One test closes both holes: 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 for antiparallel alike. It is also the quantity that matters
here, since Vector::ClosestPointBetweenLines() divides by its square -- that
is why the answer is worthless in this case, the division is 0/0. And it is
linear in the angle near zero where 1 - cos(angle) is quadratic, so the
tolerance is set to the same angle the cosine form accepted at
10*RATPOLY_EPS, about 4.5e-4 radians, rather than reusing that constant,
which against a sine would have meant a cone 4500 times narrower.
This changes no result I can measure, and I would rather say that plainly
than claim a fix. Meshes exported from
1291_1743_cube_cut_tangent_outside_still_fails_simplified.slvs,
cube_cut_2.slvs and curve_curve.slvs are byte-identical to the ones this
branch produces without it, and the test suite gives exactly the same result,
with OpenMP both on and off. On the first of those models the guard rejects
ten line pairs, every one of them exactly parallel and same-facing, so the
old test caught them too. The cubic edges that used to slip past the degree
gate are not straight at all -- they bow 0.354 mm off a 1.41 mm chord, some
350000 times LENGTH_EPS -- so admitting them to the test changes nothing
there either. Both holes are real but latent on the models I have, which
makes this a robustness fix rather than a behaviour change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- This is the commit message #2:
Reject a fallback hit where the curves are tangent rather than crossing.
The parallel test at the top of PointOnNonparallelCurve() is a global one:
it asks whether two curves run along each other for their whole length. The
same degeneracy arises at a single point, between two curves that are each
genuinely curved, and there it is just as fatal. That is what breaks
boolean_tangent_crossing on this branch.
PointOnThisAndCurve() 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 returns that seed unchanged. Six of the ten fallback hits on
boolean_tangent_crossing are of this kind, and they are not close calls: the
sine of the angle between the two tangents is 1e-8, the gap the iteration is
trying to close is 2e-14 to 1e-11, and the point moves by that same 1e-14 to
1e-11 before being declared converged. The other four hits meet at a sine of
1, square on, and move 1e-4 to get there. The two populations are eight
orders of magnitude apart, so the threshold is not a delicate one.
What comes back is therefore not an intersection but 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 is
really there, and LENGTH_EPS is 1e-6, so that is the worst distance it
could have picked -- too close to be a second feature, too far to be
recognised as the same one. In order: the endpoint test in
AllPointsIntersecting() would have dropped these, but it is outrun by 1.75
to 2.5 times; MakeCopySplitAgainst() splits a trim curve twice within 2e-8
of uv; and the zero-length trim edge that leaves can neither be assembled
into a contour nor culled, because AssembleContour() looks for a
continuation before it tests closure and CullExtraneousEdges() only removes
duplicate and antiparallel pairs. The Boolean reports failure.
So after PointOnThisAndCurve() succeeds, require that the curves really do
cross at the point it found, by the same sine test and the same tolerance
the global check uses. The parameters are recomputed from that point rather
than threaded out of the iteration, deliberately: this is meant to be a test
on the answer, not on the path taken to reach it.
boolean_tangent_crossing passes again, and the whole suite is 264 cases and
937 checks green with OpenMP both on and off, matching master. The new test
fires four times on
1291_1743_cube_cut_tangent_outside_still_fails_simplified.slvs and leaves
its mesh byte-identical, so those four hits were doing no work; it does not
fire at all on cube_cut_2.slvs, whose mesh is likewise byte-identical; and
it fires three times on boolean_tangent_spline, which stays green.
One limit worth stating: this says the rule breaks none of the models I
have, not that no model needs a tangential hit on a boundary curve. A fillet
edge lying along the line being cast, where the touch genuinely has to be
found, would be rejected by it, and I have no test for that shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No description provided.