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

Fixed grid traversal with near and far planes #204

Merged
merged 4 commits into from
Apr 14, 2023

Conversation

arterms
Copy link
Contributor

@arterms arterms commented Apr 13, 2023

This PR fixes wrong points sampling behind the far_plane, as described in #201

@liruilong940607
Copy link
Collaborator

Hi thanks for doing this fix.

It does not seem correct to me as clamping t_traverse to far plane would not terminate marching. As a result there would be many zero length segments returned in intervals.vals

I think it should be just break the loop once t_traverse exceed the far plane

Have you printed out the returned intervals?

@arterms
Copy link
Contributor Author

arterms commented Apr 14, 2023

Yes, I have checked this PR by printing out the returned intervals. It doesn't produce degenerated samples with intervals of zero length. Moreover I have added a test named test_traverse_grids_with_near_far_planes to verify that no points are sampled behind far_planes. As for me, with this fix the function works as expected. If I clearly understand the code, the loop of marching will be terminated as the sampled point exceeds the t_traversal value (clamped to far plane value).

@liruilong940607
Copy link
Collaborator

Actually I revisited the code and I agree it would not create degenerated samples because of this line. Yeah now I agree this is the right fix. I’ll merge it then. Thanks for the contribution!!

@liruilong940607
Copy link
Collaborator

There is a formatting test failure. You can fix it by running:

pip install .[dev]
black docs/ nerfacc/ scripts/ examples/ tests/ --exclude examples/pycolmap --line-length 80

@arterms
Copy link
Contributor Author

arterms commented Apr 14, 2023

Just fixed the formatting

@liruilong940607 liruilong940607 merged commit 4b7819b into nerfstudio-project:master Apr 14, 2023
liruilong940607 pushed a commit that referenced this pull request May 10, 2023
* Fixed traversal with far_plane

* test added for traversal with near and far planes

* More correct test

* black formatting for test
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.

2 participants