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

depth_image_proc register cover all angles with fill_upsampling_holes #722

Open
wants to merge 7 commits into
base: noetic
Choose a base branch
from

Conversation

lucasw
Copy link
Contributor

@lucasw lucasw commented Jan 17, 2022

Resolves #721

This is without the PR (the rviz CameraInfo plugin looks to not draw lines properly from certain angles)

depth_image_proc_holes.mp4

This is with the PR applied:

depth_image_proc_holes_fix.mp4

Here's another but without the negative z clipping
https://user-images.githubusercontent.com/1334122/149797323-fc6bf43f-3d12-47ba-9d99-fb334f4c220e.mp4

There are still gaps when there is a big angle between the two tf frames or big depth difference between adjacent pixels/points, making assumptions about adjacent points (that they are part of a continuous surface) and doing the whole software triangle rasterization approach could fix that but seems excessive.

I found adding a lookup transform timeout a62174a necessary in my test setup (probably in most real world cases /tf is way ahead of >megapixel depth images, but here I'm synthesizing 320x240 depth images so few image transport delays) but left it out of this PR, it could go into a different one?

I can adapt the test setup to here if there is interest, but it will bring in a few <test_depends>.

Could squash this down to one or two commits but maybe the commit progression makes it easier to review.

There is a really basic unit test, I can add more to it after getting some feedback.

Comment on lines +355 to +350
// fill in the square defined by uv range
const T new_depth = DepthTraits<T>::fromMeters(new_depth_sum / static_cast<double>(count));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could keep track of the depth values at each corner of the square and interpolate between them

@JWhitleyWork
Copy link
Collaborator

@lucasw Once you've implemented your own suggestions (?) I'll give this a look. Please ping me when it's ready.

@lucasw lucasw force-pushed the register-depth-fill-angles branch from f951f64 to d114516 Compare May 7, 2022 19:16
@JWhitleyWork
Copy link
Collaborator

@lucasw It looks like you've done a lot of work on this. Is it ready for an external review?

@lucasw
Copy link
Contributor Author

lucasw commented Dec 7, 2022

I have some other commits in another branch that are related but those could go in a different PR, but I'll scan through them for anything that ought to be here. Yes if you can make time try it out and review.

… and increase clarity more but not sure if it will result in performance changes
…ases, but it still reduces to a sliver from some angles
…for depth_image_proc register when filling holes, eliminating the sliver effect from some angles (low angles will stil have them, would need to use depth values from adjacent pixels to avoid that)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make depth_image_proc register fill_upsampling_holes work with full range of relative angles
3 participants