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

Add AxisAlignedBox getters for all relevant geometries #1547

Open
wants to merge 3 commits into
base: sdf15
Choose a base branch
from

Conversation

gabrielfpacheco
Copy link

@gabrielfpacheco gabrielfpacheco commented Feb 19, 2025

🎉 New feature

Part of gazebosim/gz-sim#2787

Summary

This PR adds AxisAlignedBox support for SDF geometries. Now each volumetric geometry type such as Box, Capsule, Cone, Cylinder, Ellipsoid, Sphere, and Mesh has a new AxisAlignedBox() function that computes its bounding box from the geometry dimensions. The Geometry class uses these functions to determine the correct AABB, allowing a custom calculator for meshes , which is a special case since no mesh loading is performed in the sdf::Mesh class.

Test it

  • Checkout the appropriate sdformat branch
  • Build this package
$ colcon build --packages-select sdformat15 --merge-install
  • Run the following sdformat15 tests:
$ ./build/sdformat15/bin/UNIT_Box_TEST && \
./build/sdformat15/bin/UNIT_Capsule_TEST && \
./build/sdformat15/bin/UNIT_Cone_TEST && \
./build/sdformat15/bin/UNIT_Cylinder_TEST && \
./build/sdformat15/bin/UNIT_Ellipsoid_TEST && \
./build/sdformat15/bin/UNIT_Sphere_TEST && \
./build/sdformat15/bin/UNIT_Mesh_TEST && \
./build/sdformat15/bin/UNIT_Geometry_TEST
  • Run all package tests to veify everything is still passing:
$ colcon test --packages-select sdformat15 --merge-install

Checklist

  • Signed all commits for DCO
  • 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)
  • 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.

* Mesh is a special case since it forwards the calculations for
the caller. If callback is not set, the return is nullptr
* This has been done since this library does not depend on gz-common
* Geometry types that do not have a volume, return nullptr.

Signed-off-by: Gabriel Pacheco <gabriel.fpacheco@gmail.com>
Copy link
Collaborator

@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.

These new methods should also be included in the python API (tests too), if you don't the time to do it, please open an issue about adding them

@gabrielfpacheco
Copy link
Author

These new methods should also be included in the python API (tests too), if you don't the time to do it, please open an issue about adding them

I'm on it already

Signed-off-by: Gabriel Pacheco <gabriel.fpacheco@gmail.com>
@scpeters
Copy link
Member

scpeters commented Mar 4, 2025

it's reasonable to add gz::math::AxisAlignedBox accessors to the sdf Shape APIs. I'm going to open a feature request to move the conversion logic to gz-math, since I think it would be more broadly useful there. Something like gz/math/AxisAlignedBoxHelpers.hh with static functions like:

static gz::math::AxisAlignedBox ConvertToAxisAlignedBox(const gz::math::Box &);
static gz::math::AxisAlignedBox ConvertToAxisAlignedBox(const gz::math::Sphere &);
static gz::math::AxisAlignedBox ConvertToAxisAlignedBox(const gz::math::Capsule &);
etc.

The implementation here could then be refactored to use those helper functions, but we don't need to block this PR for that

@scpeters
Copy link
Member

scpeters commented Mar 4, 2025

it's reasonable to add gz::math::AxisAlignedBox accessors to the sdf Shape APIs. I'm going to open a feature request to move the conversion logic to gz-math, since I think it would be more broadly useful there.

The implementation here could then be refactored to use those helper functions, but we don't need to block this PR for that

gazebosim/gz-math#666

@@ -76,6 +77,10 @@ namespace sdf
public: std::optional<gz::math::Inertiald>
CalculateInertial(double _density);

/// \brief Get the Axis-aligned box for this Box.
Copy link
Member

Choose a reason for hiding this comment

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

For this and other similar comments, please state where the center of the Axis-aligned box is located. In most cases, I believe it will be at the center of the SDFormat shape, but I think that is not always the case for meshes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏛️ ionic Gazebo Ionic 🪵 jetty Gazebo Jetty
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants