-
Notifications
You must be signed in to change notification settings - Fork 512
Unexpected, inconsistent NURBS failure? #408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
I can not reproduce this with current master. |
Neither can I, looks fixed! |
Does anyone want to do a bisect to find out when this got fixed and why? It is still bothering me.
|
Time to learn? |
@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. |
Thanks for the explanation and you are absolutely right that changing this line: Line 802 in 3694c9b
to if(pts.n <= 3) return; breaks the test again. |
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
The text was updated successfully, but these errors were encountered: