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

simplify: Make hasTriangleFlip predicate use an angle cutoff #710

Merged
merged 1 commit into from
Jun 25, 2024
Merged

Conversation

zeux
Copy link
Owner

@zeux zeux commented Jun 24, 2024

Instead of checking if the dot product is 0 or negative (90 degree limit on triangle rotations), use a cutoff of ~75 degrees. The motivation here is that meshes with complex curvature can end up flipping triangles through a series of collapses, and since we always evaluate flips compared to the current mesh we miss that. Technically this can happen even with a 75 degree limit, but in practice this solves cases of flips that have been reported before, and doesn't noticeably constrain simplification or slow it down.

For numerical stability, we avoid dividing by the triangle areas (as they can be close to zero) and instead use the areas to compute a bound for dot product. Even if the area product underflows, we just get the same check that we used to have -- areas should be in [0..1] so there should be no risk of overflow.

Fixes #346.

Instead of checking if the dot product is 0 or negative (90 degree limit
on triangle rotations), use a cutoff of ~75 degrees. The motivation here
is that meshes with complex curvature can end up flipping triangles
through a series of collapses, and since we always evaluate flips
compared to the current mesh we miss that. Technically this can happen
even with a 75 degree limit, but in practice this solves cases of flips
that have been reported before, and doesn't noticeably constrain
simplification or slow it down.

For numerical stability, we avoid dividing by the triangle areas (as
they can be close to zero) and instead use the areas to compute a
bound for dot product. Even if the area product underflows, we just get
the same check that we used to have -- areas should be in [0..1] so
there should be no risk of overflow.
@zeux
Copy link
Owner Author

zeux commented Jun 25, 2024

Here's a comparison in terms of the evaluated collapses: for each pass, this plots the log10 of collapses we've evaluated (X) vs what percentage of the evaluations were "wasteful", as in resulted in an invalid collapse (Y). Blue points are before the change, red are after, the data set is a wide set of different meshes and scenes of varying complexity.

There is some impact here, as in we do evaluate more collapses which does result in a higher waste % sometimes, but overall the impact of this seems fairly moderate. There are improvements to the pass scheduling I am planning to investigate in the future which would probably offset some of this loss as well.

image

@zeux
Copy link
Owner Author

zeux commented Jun 25, 2024

P.S. I should note that while this workaround is pragmatic, I don't love it 😓 Hopefully it will be replaced by something else in the future. But with a steady volume of reports about edge case flips and this solving them, it's a reasonable stop-gap.

@zeux zeux merged commit 452f7d1 into master Jun 25, 2024
12 checks passed
@zeux zeux deleted the simp-flip branch June 25, 2024 16:55
@RikoOphorst
Copy link

RikoOphorst commented Aug 9, 2024

@zeux Only just seen this MR, so a bit late on the reply here - but wanted to express that your work on this is appreciated. It's great to see your attention to detail and your impact analysis. Looking forward to trying this out!

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

Successfully merging this pull request may close these issues.

Flipped triangles when simplifying mesh
2 participants