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

Apply particle scattering effect to depth cameras #251

Merged
merged 33 commits into from
Feb 17, 2021

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 16, 2021

depends on #240 and #241

Summary

Particles emitted by the particle emitter used to be rendered as large 2D squares in the depth camera view due to the way they are represented in ogre (2d sprites / billboards). This PR applies "scattering effect" to the depth image output to make it appear as if the particle model is composed of a cluster of smaller particles. Gaussian noise is also applied to the particle depth values. Some scattering effect params are currently hardcoded since we cannot provide APIs to set them as it will break ABI.

Test it

You can test this using ign-gazebo by launching this sensor_particles.sdf world:

ign gazebo -v 4 sensor_particles.sdf

You should see:

depth_camera_particles

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

iche033 and others added 24 commits February 4, 2021 23:11
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ashton Larkin <ashton@openrobotics.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@chapulina chapulina moved this from Inbox to In review in Core development Feb 16, 2021
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #251 (3f71f83) into ign-rendering4 (194bdf8) will increase coverage by 0.36%.
The diff coverage is 100.00%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-rendering4     #251      +/-   ##
==================================================
+ Coverage           52.92%   53.29%   +0.36%     
==================================================
  Files                 143      143              
  Lines               13566    13672     +106     
==================================================
+ Hits                 7180     7286     +106     
  Misses               6386     6386              
Impacted Files Coverage Δ
ogre2/src/Ogre2Scene.cc 90.43% <ø> (ø)
ogre2/src/Ogre2DepthCamera.cc 94.50% <100.00%> (+0.87%) ⬆️
ogre2/src/Ogre2GpuRays.cc 94.75% <100.00%> (+0.51%) ⬆️
ogre2/src/Ogre2Visual.cc 85.14% <100.00%> (+0.30%) ⬆️

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 194bdf8...3f71f83. Read the comment docs.

Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

I left a few minor comments. I tested the example world, and things seem to work for me. I had a hard time understanding a lot of what was going on in this PR though, so it might be worth having someone else review this as well to make sure I didn't miss anything.

ogre2/src/Ogre2DepthCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2DepthCamera.cc Show resolved Hide resolved
ogre2/src/Ogre2DepthCamera.cc Show resolved Hide resolved
ogre2/src/Ogre2DepthCamera.cc Outdated Show resolved Hide resolved
ogre2/src/Ogre2DepthCamera.cc Outdated Show resolved Hide resolved
test/integration/depth_camera.cc Outdated Show resolved Hide resolved
test/integration/depth_camera.cc Outdated Show resolved Hide resolved
test/integration/depth_camera.cc Outdated Show resolved Hide resolved
test/integration/depth_camera.cc Show resolved Hide resolved
test/integration/depth_camera.cc Outdated Show resolved Hide resolved
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Signed-off-by: Ian Chen <ichen@osrfoundation.org>
Copy link
Contributor

@adlarkin adlarkin left a comment

Choose a reason for hiding this comment

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

As I mentioned before, my understanding of what's going on here isn't perfect, so if someone else wants to take a look as well, go for it, but this looks good to me.

Base automatically changed from thermal_particles to ign-rendering4 February 17, 2021 21:43
@adlarkin adlarkin enabled auto-merge (squash) February 17, 2021 21:57
@adlarkin adlarkin merged commit b2e747b into ign-rendering4 Feb 17, 2021
Core development automation moved this from In review to Done Feb 17, 2021
@adlarkin adlarkin deleted the depth_particles branch February 17, 2021 23:36
@adlarkin adlarkin mentioned this pull request Feb 18, 2021
7 tasks
@peci1 peci1 mentioned this pull request Feb 26, 2021
@j-rivero j-rivero removed this from Done in Core development May 6, 2022
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.

None yet

2 participants