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

Fix infinite loop bug in mergeFacesAndHoles #83

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

birgerbr
Copy link

When an element in containing_faces has a size greater than 1 the
while (unassigned) loop will never stop.

I don't know this algorithm and I'm not sure if this is the correct way of handling
this case, but it solves the infinite loop case.

When an element in `containing_faces` has size greater than 1 the
`while (unassigned)` loop will never stop.
Copy link
Owner

@folded folded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a long while since I looked at this code, but IIRC containing_faces maps from each face, to the faces that contain it, of which might be more than one. The aim is to connect each face to the one that it is directly nested within. As we peel away the outermost faces, there should always be at least one face that only has the outermost face as a parent. If that's not the case, then there's a cycle in the embedding graph, which would cause this, but the error is as a result of an earlier error.

Do you have an example that triggers this?

@birgerbr
Copy link
Author

The case was when using ifcplusplus to extract the geometry from an IFC file from a customer. Unfortunately, I could not find any notes saying which IFC file it was.

@arnholm
Copy link

arnholm commented Jun 29, 2020

I am interested in this issue. I have observed infinite loops in carve sometimes, not sure if it is related to this. I tried the proposed fix and tested it with https://github.com/arnholm/angelcad-samples, but this proposed carve fix caused 2 samples to fail, so I think it is not 100% correct. PS. thank you for the carve library.

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.

3 participants