Skip to content
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

Avoid zero tangnet vectors on degenerate NURBS edges. Fixes #652 #746

Merged
merged 1 commit into from Oct 18, 2020

Conversation

phkahler
Copy link
Member

@ruevs This is probably the clearest and cleanest we can get. What do you think?

@ruevs
Copy link
Member

ruevs commented Oct 17, 2020

Calling recursively looks clean, I may change mine here to do the same. But you use the flag to avoid endless recursion (in some weird potential "NURBS universe"). I don't like the flag as an exposed interface (#736 (comment))

How does v+(1-v)*0.000001 work if v=1? I think you meant v+(0.5-v)*LENGTH_EPS.

You need to pass a tolerance to Equals, IMHO the default is too big.

Check tv first. So far I could not figure out how to construct anything pinched along a u edge. Makes it faster by avoiding a check that may never be true with the current construction tools.

"sungularities" = singularities

As for my overall opinion on the "numerical" (vs. logical) approach - did you take a look at this?
#736 (comment)

@phkahler
Copy link
Member Author

phkahler commented Oct 17, 2020

How does v+(1-v)*0.000001 work if v=1? I think you meant v+(0.5-v)*LENGTH_EPS.

Oops. I started out thinking a weighted average so 0.99v + 0.01(1-v) and then was thinking something more like what you suggest. Brain just didn't write either one correctly. I prefer (0.5-v). Isn't LENGTH_EPS really small?

Using the flag to avoid infinite recursion just seems like a good idea, especially if you want to use such a tiny movement. Like I said before, it also allows flexibility at the call site but we don't really need that once it's fixed.

EDIT: hahaha joke is on me. LENGTH_EPS = 0.000001

@phkahler
Copy link
Member Author

You mean switch the order? OK. Does that mean you're ok with "check result and retry" vs the "check for degenerate curve and pre-emptively move u,v" ?

@ruevs
Copy link
Member

ruevs commented Oct 18, 2020

This one is shorter, that is it's advantage. The other one is faster (but I need to measure, this is just based on amount of calculations) and "more correct" (whatever that meas) but more verbose...

The problem is that I have not had time to work on it more...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants