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

Point clouds for RGBD cameras #17

Merged
merged 11 commits into from
Jun 20, 2019
Merged

Point clouds for RGBD cameras #17

merged 11 commits into from
Jun 20, 2019

Conversation

@chapulina
Copy link
Contributor Author

chapulina commented Jun 12, 2019

Colors fixed in faa07bd, but I'm not sure why I had to do BGR instead of RGB when populating the point cloud. I checked that the ignition::rendering::Image and the sensor_msgs::Image are storing RGB, but that seems to be swapped from the PointCloud2 message. I'm still investigating, but this is usable and I think all other PRs can be merged.

Note also that the color seems to be slightly offset with respect to the points:

bgr

@chapulina chapulina changed the title [WIP] Point cloud Point clouds for RGBD cameras Jun 17, 2019
@chapulina
Copy link
Contributor Author

This is ready for review.

I think the offset between the depth and RGB images is coming from ign-rendering. Note how the falling cone's position is slightly different in the depth and color images:

mismatched_imgs

@scpeters
Copy link
Member

I'm going to rebase this on meta_pkg

@chapulina chapulina changed the base branch from meta_pkg to master June 17, 2019 23:11
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Works great! I left some comments that should be easily addressed.

ros1_ign_point_cloud/package.xml Show resolved Hide resolved
ros1_ign_point_cloud/src/point_cloud.cc Outdated Show resolved Hide resolved
ros1_ign_point_cloud/src/point_cloud.cc Outdated Show resolved Hide resolved
ros1_ign_point_cloud/src/point_cloud.cc Outdated Show resolved Hide resolved
ros1_ign_point_cloud/src/point_cloud.cc Outdated Show resolved Hide resolved
@chapulina
Copy link
Contributor Author

Thanks for the review, @azeey , I think I've addressed all your comments.

@azeey
Copy link
Contributor

azeey commented Jun 19, 2019

Looks like CI is failing because we added ignition packages to package.xml. Would doing rosdep -r help? Otherwise, we might have to revert those changes 🙁

@IeiuniumLux
Copy link

@azeey how about #18?

@azeey
Copy link
Contributor

azeey commented Jun 19, 2019

I just tested with ogre vs ogre2 with a 1920x1080 rgbd camera. The weird offset thing you mentioned is much more visible in higher resolution in ogre2, but it doesn't happen in ogre.

Ogre2
ogre2

Ogre
ogre

* .travis/build: rosdep skip ignition keys

* Update build
@scpeters
Copy link
Member

CI is 💚

Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

I don't think the offset issue is caused by this PR so I'm good with merging this.

@chapulina
Copy link
Contributor Author

Nice catch with ogre vs ogre2, @azeey . We can debug more on the ign-rendering side.

@chapulina chapulina merged commit 18f04c2 into master Jun 20, 2019
@chapulina chapulina deleted the pointcloud branch June 20, 2019 13:02
@iche033
Copy link
Contributor

iche033 commented Jun 20, 2019

for ogre2, do you know if it's the depth data that are at an offset or the color data?

@chapulina
Copy link
Contributor Author

@iche033 , taking ogre1 as ground truth, depth seems to be the wrong one. It seems to be a bit more zoomed in:

rgb_diff
depth_diff

@iche033
Copy link
Contributor

iche033 commented Jun 20, 2019

thanks that's helpful. I'm looking into it

@iche033
Copy link
Contributor

iche033 commented Jun 20, 2019

issue should be fixed by ign-rendering pull request 173:

https://bitbucket.org/ignitionrobotics/ign-rendering/pull-requests/173

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.

None yet

5 participants