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

Added preserve_border argument to lossless method. #37

Merged
merged 8 commits into from
Oct 23, 2024

Conversation

Kramer84
Copy link
Owner

Added the small change to the cpp function to accept the preserve_border argument, hope it is as straightforward than that, but seems to work.

Also was thinking about saving the bunny as a ".npy" file to remove the need to use trimesh for the tests. Also maybe generate a non watertight mesh (tilted cylinder with open lid or something), to do some more extensive testing for the preserve_border option, but that may be some more work to generate all that using numpy.

…d by a better more generalistic method later. Added noexcept optimization on _nogil functions (maybe risky?).
pyfqmr/Simplify.pyx Outdated Show resolved Hide resolved
pyfqmr/Simplify.pyx Outdated Show resolved Hide resolved
@bwoodsend
Copy link
Collaborator

Nice!

One day we probably should introduce some optional sanitization that asserts all faces are between 0 and len(vertices) but that's another day's problem.

@Kramer84
Copy link
Owner Author

I'll merge it then! But for the sanitization I'm not so sure, cause in practice - for any mesh- it would be possible to append as much vertices as we want. As long as they are not referenced by the faces they are "inactive" in some sense, but would not break it. So we could have more vertices than the max value in faces. I don't know how the simplification script would react to this kind of input though. I guess we need to create more thorough tests to be sure about how to handle those cases. For now users will have to deal with their potential segfaults ! 😄

@Kramer84 Kramer84 merged commit e3732d3 into master Oct 23, 2024
36 checks passed
@bwoodsend bwoodsend deleted the preserve-border-lossless branch October 23, 2024 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants