Solvespace: Unexpected, inconsistent NURBS failure?

[rpavlik]

System information

SolveSpace version: 3.0~201e15e6

Operating system: Debian testing/buster

Expected behavior

The result of "show naked edges" suggests the model is fine without forcing to mesh. I would expect the export to also complete successfully then.

Actual behavior

On export as mesh without force to mesh set for this group, get a self-intersecting warning (on an extrude cut through a lathe cylinder-ish thing.)

Turning on the "force to mesh" setting allowed the export to complete without the warning.

Additional information

If this is just a "model too complicated, just force to mesh" issue, feel free to close, sorry for the noise. Just seemed weird that it would look OK via one tool but then export would complain, so I figured I'd submit it as long as I already remembered how to use the screencast tool. Pretty sure this isn't exactly #171, since the sample file there shows the warning interactively (turns red) and using Show Naked Edges, not just on mesh export. (and I've gotten pretty good at dodging that issue over time.)

The file itself:
screwdriver-sheath-with-taper.zip

Screencast, showing behavior and workaround.
slvs nurbs.zip

[ruevs]

I can not reproduce this with current master.

[rpavlik]

Neither can I, looks fixed!

[ruevs]

Does anyone want to do a bisect to find out when this got fixed and why? It is still bothering me.
Only 174 commits but I still have not learned how to efficiently do a bisect in git :sweat_smile: :innocent:

git rev-list --count 201e15e6..7baf585
174

[whitequark]

Time to learn?

[ruevs]

Hey I learned to do git bisect :-)
This was fixed in e83e48392e0b65eb6e1b7b6bf7d5d1d4882481d1 on 2020-06-12.

Now if only @phkahler could explain why (TF) that commit fixes this problem :-) It's not entirely idle curiosity either - this is interesting.

[phkahler]

@rpavlik To address the original question of why it only fails on export. Probably related to chord tolerance. In normal operation it's using the larger chord tolerance and everything is OK, then when exporting it uses the export value and the failure happens. Forcing to triangle mesh "fixes" it because it probably uses the on-screen chord tolerance - which I think would be a bug? Or because it prevents a NURBS boolean failure that was only happening at the lower chord tolerance.

@ruevs Why does that commit make the problem go away? I'm speculating here. That function SCurve::RemoveShortSegments() exists to fix the problem. Apparently there are issues if 2 points in the PWL representation of a curve are too close together, so that function eliminates points along the curve that are not needed to maintain chord tolerance. When 2 points are very close - as the result of a boolean adding one - it's possible to remove one without violating chord tolerance. But this point removal also caused small circles to collapse to squares even when the PWL was created with 2 segments per curve to make them at least octagons. In order to prevent the octagons turning into squares, the condition was added to exit early if there are 3 or less points.

My speculation is that the sketch in question has a boolean that creates a "short segment" that needs to be removed, but it probably where a straight line (with only one segment) gets split in two right near the end. The short segment is not removed because that early exit was added.

The change in the commit was needed in order to allow even more "unnecessary" segments. i wanted circles to be at least 16 segments or 4 per curve, and they need to not be removed by this function. The idea is if the curve t-parameter is "large" between a point and both its neighbours then that point was probably put there deliberately and should not be removed. With that criteria in place the early exit when there are only 3 points could be eliminated which is probably what fixed the issue here by allowing a problematic short segment to get removed.

If that is correct then putting the condition back to <=3 points (vs 2 as it is now) should re-introduce the problem with the sketch.

On a related note, allowing more segments than needed by the chord tolerance was also useful for getting better triangulation of helical surfaces. It allows straight trim curves to be split even though they do not deviate from the twisted surface they sweep out.

[ruevs]

Thanks for the explanation and you are absolutely right that changing this line:
https://github.com/solvespace/solvespace/blob/3694c9b3eee5a2acc25457e07fde4bc05ca29a53/src/srf/curve.cpp#L802
to
if(pts.n <= 3) return;
breaks the test again.