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

Optimize rendering sensor pose updates #2425

Merged
merged 2 commits into from
Jun 5, 2024
Merged

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented May 31, 2024

🦟 Bug fix

Related PR: gazebosim/gz-sensors#439

Summary

Currently we are updating the pose of all rendering sensors every iteration. This is not necessary as we currently do not expect sensor pose to change at runtime. This PR removes the pose updates every iteration and it now just sets the pose of these rendering sensors once they created. For camera based sensors, their pose transform is the combined result of //sensor/pose and //sensor/camera/pose (previously the //sensor/camera/pose sdf element was ignored and never used).

Here is an example world for test: camera_test.sdf. The world consists of 2 cameras sensors (rgb and depth) with pose offsets in //sensor/pose and //sensor/camera/pose. Together with gazebosim/gz-sensors#439, the camera sensor pose should be set correctly and you should see images like below:

camera_pose_offset

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>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label May 31, 2024
@azeey
Copy link
Contributor

azeey commented Jun 4, 2024

previously the //sensor/camera/pose sdf element was ignored and never used)

I imagine this would break some existing SDF files. I wonder if we should provide a flag to enable/disable this bugfix. We should add an entry in the migration guide regardless.

@iche033
Copy link
Contributor Author

iche033 commented Jun 4, 2024

if we should provide a flag to enable/disable this bugfix

I am thinking that I can remove the camera pose change in this PR which is targeting a release branch so we don't break existing sdf files in Harmonic. I'll then create a separate PR targeting main with the camera pose change and a migration guide entry. How does that sound?

@azeey
Copy link
Contributor

azeey commented Jun 4, 2024

That sounds great!

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

iche033 commented Jun 5, 2024

removed camera pose changes in a129d93. I'll merge this forward to main first and then apply the camera pose changes in a separate PR.

@iche033 iche033 merged commit 9e60fdc into gz-sim8 Jun 5, 2024
7 checks passed
@iche033 iche033 deleted the set_rendering_sensor_pose branch June 5, 2024 07:45
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.

None yet

3 participants