gazebosim / gz-sim

Open source robotics simulator. The latest version of Gazebo.
https://gazebosim.org
Apache License 2.0
618 stars 251 forks source link

Optimize rendering sensor pose updates #2425

Closed iche033 closed 3 weeks ago

iche033 commented 1 month ago

🦟 Bug fix

Related PR: https://github.com/gazebosim/gz-sensors/pull/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 https://github.com/gazebosim/gz-sensors/pull/439, the camera sensor pose should be set correctly and you should see images like below:

camera_pose_offset

Checklist

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.

azeey commented 4 weeks ago

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 commented 4 weeks ago

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 commented 4 weeks ago

That sounds great!

iche033 commented 3 weeks ago

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.