gz-sim icon indicating copy to clipboard operation
gz-sim copied to clipboard

Physics System populates link bounding boxes

Open gabrielfpacheco opened this issue 8 months ago • 4 comments

🎉 New feature

Follow-up of #2787

Summary

Computes the link bounding boxes for all link entities that have components::AxisAlignedBox present. The computation follows the same logic as the models bbox, but falls back to the SDF calculation introduced in #2787 in case the feature is not present for the selected physics plugin.

Test it

Run the following integration test:

./build/gz-sim9/bin/INTEGRATION_physics_system --gtest_filter=*LinkBoundingBox*

It will load bounding_boxes.sdf world (see below) and perform link AABB checks while the models free-fall until reaching the ground.

image

Note: after the previous PR is merged, this PR will rebase to gz-sim9 and be marked as ready to review

Checklist

  • [x] Signed all commits for DCO
  • [x] Added tests
  • [ ] Added example and/or tutorial
  • [ ] Updated documentation (as needed)
  • [ ] Updated migration guide (as needed)
  • [ ] Consider updating Python bindings (if the library has them)
  • [x] codecheck passed (See contributing)
  • [x] 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.

gabrielfpacheco avatar Mar 12 '25 17:03 gabrielfpacheco

@iche033, I believe I addressed all your comments. If you agree with the proposed changes, could you please approve #2787 before? I would like to rebase this PR after the previous one is merged so that it focuses only on the physics system changes.

gabrielfpacheco avatar Mar 25 '25 18:03 gabrielfpacheco

@iche033, I believe I addressed all your comments. If you agree with the proposed changes, could you please approve #2787 before? I would like to rebase this PR after the previous one is merged so that it focuses only on the physics system changes.

Currently there's a build error as it needs the new API from sdformat. So I'm waiting for https://github.com/gazebosim/sdformat/pull/1547 to be merged first. We also need a new sdformat release afterwards in order to run CI in #2787 and make sure everything is green. We can only approve and merge a PR when the required CI checks are green.

iche033 avatar Mar 26 '25 17:03 iche033

So I'm waiting for gazebosim/sdformat#1547 to be merged first. We also need a new sdformat release afterwards in order to run CI in https://github.com/gazebosim/gz-sim/pull/2787 and make sure everything is green.

Yes, sure! I initially thought there might be a way to trigger the CI with dependent packages at specific branches, but then realized that the merge/release process is required, as you said.

Besides the dependency that prevents the proper CI checks, though, there are still some unresolved threads here from your first pass review, are those solved?

Regarding the sdformat PR, the CI is already green, and I believe all comments have been addressed. I've pinged the reviewers in https://github.com/gazebosim/sdformat/pull/1547#issuecomment-2752122608 recently, so I think we should be close to a merge.

gabrielfpacheco avatar Mar 28 '25 21:03 gabrielfpacheco

Besides the dependency that prevents the proper CI checks, though, there are still some unresolved threads here from your first pass review, are those solved?

yep looks good to me. Feel free to mark them as resolved once you address the comments.

iche033 avatar Mar 31 '25 21:03 iche033

@iche033, friendly ping :)

gabrielfpacheco avatar Jul 07 '25 21:07 gabrielfpacheco

@mergify backport gz-sim8

iche033 avatar Jul 07 '25 23:07 iche033

backport gz-sim8

✅ Backports have been created

mergify[bot] avatar Jul 07 '25 23:07 mergify[bot]