Skip to content

Add support for Gaussian noise render pass in Ogre2DepthCamera #122

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

Merged
merged 3 commits into from
Aug 7, 2020

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Aug 6, 2020

Depth cameras in ign-rendering generates both depth and color data using custom shader scripts so we can not apply regular image based Gaussian noise pass to its output. Instead, I updated Ogre2DepthCamera class to support its own render pass chain, and wrote a separate fragment shader that works with the depth camera data format.

The Gaussian noise is applied individually to each component in the point cloud data, i.e. x, y, z, and r, g, b.

Here's a depth_noise.sdf for testing in ign-gazebo (requires gazebosim/gz-sensors#35). To launch:

ign gazebo -v 4 depth_noise.sdf

Select the /camera/image topic in the ImageDisplay plugin to see the following image output:

depth_color_noise

As for the depth image, the effect is not very noticeable in the /camera/depth_image topic in ImageDisplay because I think the conversion of depth to color image normalizes the data and that reduces the effect of noise. Here's a better visualization in rviz that shows noise in the point cloud data generated by the depth camera:

rgbd_noise

@github-actions github-actions bot added the 📜 blueprint Ignition Blueprint label Aug 6, 2020
@codecov
Copy link

codecov bot commented Aug 6, 2020

Codecov Report

Merging #122 into ign-rendering2 will increase coverage by 0.62%.
The diff coverage is 93.22%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering2     #122      +/-   ##
==================================================
+ Coverage           50.75%   51.38%   +0.62%     
==================================================
  Files                 117      117              
  Lines                9922    10071     +149     
==================================================
+ Hits                 5036     5175     +139     
- Misses               4886     4896      +10     
Impacted Files Coverage Δ
ogre2/src/Ogre2RenderTarget.cc 73.67% <91.66%> (+0.49%) ⬆️
ogre2/src/Ogre2DepthCamera.cc 93.81% <93.05%> (-0.88%) ⬇️
ogre2/src/Ogre2GaussianNoisePass.cc 90.00% <95.83%> (+6.66%) ⬆️
...e/ignition/rendering/base/BaseGaussianNoisePass.hh 96.66% <0.00%> (-3.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3f8eb38...b17ccc0. Read the comment docs.

@nkoenig nkoenig merged commit e725abc into ign-rendering2 Aug 7, 2020
@nkoenig nkoenig deleted the depth_noise branch August 7, 2020 13:05
@chapulina
Copy link
Contributor

This PR broke ABI on Blueprint, then was ported to Citadel and broke ABI there too. Now it's being ported to Dome in #167 and we only caught the ABI breakage at that point.

The ABI breakage happened from:

  • 2.4.0 to 2.5.0
  • 3.1.0 to 3.2.0

The ABI checker failed on the last commit of this PR, so in theory we should have caught it and at least discussed it:

https://build.osrfoundation.org/job/ignition_rendering-abichecker-any_to_any-ubuntu_auto-amd64/254/API_5fABI_20report/

We should think about how we can avoid this in the future. One thing I did was making the ABI checker required for all release branches. It can still be overridden by admins, but that should at least be an extra checkpoint.

@chapulina chapulina mentioned this pull request Oct 27, 2020
@chapulina chapulina mentioned this pull request Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants