-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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 incomplete VoxelGrid creation from triangles intersecting multiple voxels #6325
Fix incomplete VoxelGrid creation from triangles intersecting multiple voxels #6325
Conversation
Thanks for submitting this pull request! The maintainers of this repository would appreciate if you could update the CHANGELOG.md based on your changes. |
Hi @sharon-br thanks for the bug fix. We'll get back with reviews shortly. Once the PR is merged, you can use the devel packages from here: http://www.open3d.org/docs/latest/getting_started.html (both C++ binaries and Python wheels are available) till the next release is ready. |
@theNded @reyanshsolis Any chance one of you guys can review this, please? |
Thanks for your contribution. Will do it by tonight. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Please change the PR title to a concise, imperative one, e.g. "Fix incomplete VoxelGrid creation from triangles intersecting multiple voxels."
…e voxels. Since the triangle can span across multiple voxels, we should not break when we find the first relevant voxel.
a145db1
to
70bcc97
Compare
Done - is there anything else we need in order to merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@reyanshsolis Your comment commit broke the style check and therefore the test failed, I pushed another commit that fixes that. |
Thanks @sharon-br for the bugfix. Thanks @theNded and @reyanshsolis for reviews! |
Set all the relevant voxels, since a triangle can span across multiple ones
Currently, we break after the first voxel in the depth axis for some reason, but a single triangle can be relevant to multiple voxels.
Is there any option to release a version including this PR, since I really want to integrate this into a solution I'm building.
Type
Motivation and Context
The current voxel grid generated is wrong and contains only the first voxel in the depth axis.
Checklist:
python util/check_style.py --apply
to apply Open3D code styleto my code.
updated accordingly.
results (e.g. screenshots or numbers) here.
Description
Mesh itself I've tested:
VoxelGrid before the fix:
VoxelGrid after the fix:
This change is