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

Make depth_image_proc register fill_upsampling_holes work with full range of relative angles #721

Open
lucasw opened this issue Jan 16, 2022 · 3 comments · May be fixed by #722
Open

Make depth_image_proc register fill_upsampling_holes work with full range of relative angles #721

lucasw opened this issue Jan 16, 2022 · 3 comments · May be fixed by #722

Comments

@lucasw
Copy link
Contributor

lucasw commented Jan 16, 2022

The current implementation doesn't draw anything from certain relative angles between the source depth image frame and target frame (called rgb in the code and documentation but it is just any valid camera info with a valid frame), draws increasingly thin slivers as it approaches those angles, and then when a range check fails it doesn't draw anything at all.

I have the beginning of a quick fix here lucasw@478fd5b but it'll have to do 3 or 4x the computation. It's not the full triangle rasterization mentioned in a todo but would get decent results in more cases. It gets a little ridiculous to embed what looks more and more like a full software 3D renderer here into this node but making it a little better seems reasonable.

register_upsampling_slivers

This image shows off the filled areas getting reduced to slivers using the min/max of two values approach in the commit above, without it the slivers can get reduced to nothing at all because the range check can fail entirely.

This is the original code with for loops that sometimes don't execute at all:
https://github.com/ros-perception/image_pipeline/blob/noetic/depth_image_proc/src/nodelets/register.cpp#L291-L293

There look to be some other obvious refactors/cleanups to do in that same code (it's doing the same thing in three nearly identical blocks). Also I can look at creating a unit test for this (or is there one somewhere and I'm not seeing it?).

Probably few people who are using this have crazy high angles between the depth camera and the other camera frame, I'm only running into it now trying this out with synthetic data (I may get some of the synthetic data generation into the unit test, but it's nice to have manual test support also).

@lucasw lucasw changed the title Make depth_image_proc fill_upsampling_holes work with full range of relative angles Make depth_image_proc register fill_upsampling_holes work with full range of relative angles Jan 16, 2022
@flynneva
Copy link
Contributor

@lucasw do you think you could also port this to the ros2 branch of depth_image_proc? I did a pretty major rewrite of that package awhile ago so it might be a bit easier to add it there.

@lucasw
Copy link
Contributor Author

lucasw commented Jan 24, 2022

@flynneva I'll take a look, but would probably be slow to get that done.

@flynneva
Copy link
Contributor

@lucasw I could take a stab at it later this week and tag you for a review 👍🏼 after looking through your updates I think it'll fit in nicely with the rewrite i did (lots of consolidation and templating)

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

Successfully merging a pull request may close this issue.

3 participants