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

Skip particle passes in Ogre2DepthCamera if there are no particles in the scene #971

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Feb 28, 2024

🦟 Bug fix

Summary

Minor performance optimization to ogre2's depth camera implementation. Similar to #965, I used execution mask to skip rendering the particle passes if there are no particles in the scene. This yields a small speed up, see the time it takes to do 1 render operation (for a 320x240 depth camera) before and after the changes in this PR:

depth_camera_particle_profile

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Ian Chen <ichen@openrobotics.org>
Signed-off-by: Ian Chen <ichen@openrobotics.org>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Feb 28, 2024
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

linters

  /github/workspace/ogre2/src/Ogre2DepthCamera.cc:1067:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/ogre2/src/Ogre2DepthCamera.cc:1093:  Lines should be <= 80 characters long  [whitespace/line_length] [2]

Signed-off-by: Ian Chen <ichen@openrobotics.org>
@iche033
Copy link
Contributor Author

iche033 commented Feb 28, 2024

linters

  /github/workspace/ogre2/src/Ogre2DepthCamera.cc:1067:  Lines should be <= 80 characters long  [whitespace/line_length] [2]
  /github/workspace/ogre2/src/Ogre2DepthCamera.cc:1093:  Lines should be <= 80 characters long  [whitespace/line_length] [2]

fixed linter errors and tests in e4ac886

Copy link

codecov bot commented Feb 28, 2024

Codecov Report

Attention: Patch coverage is 95.12195% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 75.59%. Comparing base (dc9fe05) to head (e4ac886).

Files Patch % Lines
ogre2/src/Ogre2DepthCamera.cc 95.12% 2 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##           gz-rendering8     #971       +/-   ##
==================================================
+ Coverage               0   75.59%   +75.59%     
==================================================
  Files                  0      177      +177     
  Lines                  0    16934    +16934     
==================================================
+ Hits                   0    12802    +12802     
- Misses                 0     4132     +4132     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iche033 iche033 requested a review from ahcorde February 28, 2024 22:21
@iche033 iche033 merged commit b07e79d into gz-rendering8 Feb 28, 2024
9 of 10 checks passed
@iche033 iche033 deleted the particle_pass_depth branch February 28, 2024 23:33
iche033 added a commit that referenced this pull request Feb 29, 2024
… the scene (#971)

Signed-off-by: Ian Chen <ichen@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants