Make Bounded3d dyn-compatible

[omeranson]

Objective

Make Bounded2d and Bounded3d dyn-compatible objects. This is so that components can contain a dyn Bounded3d type member, and won't need a template.

There may be other traits that need to go through the same process.

Solution

Refractor Bounded2d and Bounded3d so that its methods accept Isometry2d and Isometry3d, rather than an Into<IsometryXd>. This removes the single item limiting these traits from being dyn-compatible.

Added a migration guide.

Testing

• Compiled bevy
• cargo clippy --workspace --all-targets --all-features -- -Dwarnings
• Compiled and ran a small example and examples/custom_primitives.

Alternative

As an alternative, a new dyn-compatible trait can be created, with a blanket implementation for Bounded3d. This requires no code change from other implementations of Bounded3d to change their signature.

pub trait Bounded3dDyn {
    /// Get an axis-aligned bounding box for the shape translated and rotated by the given isometry.
    fn aabb_3d_dyn(&self, isometry: bevy_::Isometry3d) -> Aabb3d;
    /// Get a bounding sphere for the shape translated and rotated by the given isometry.
    fn bounding_sphere_dyn(&self, isometry: bevy_::Isometry3d) -> BoundingSphere;
}

impl<T: Bounded3d> Bounded3dDyn for T {
    /// Get an axis-aligned bounding box for the shape translated and rotated by the given isometry.
    fn aabb_3d_dyn(&self, isometry: Isometry3d) -> Aabb3d {
        self.aabb_3d(isometry)
    }
    /// Get a bounding sphere for the shape translated and rotated by the given isometry.
    fn bounding_sphere_dyn(&self, isometry: Isometry3d) -> BoundingSphere {
        self.bounding_sphere(isometry)
    }
}

Related: #19491

[Jondolf]

Oh fun, this is actually something I ran into in Peck (my WIP collision detection library)! I had to work around it with your alternative solution:

/// A trait with methods that return 3D bounding volumes for a shape.
///
/// This exists as a dyn-compatible alternative to [`Bounded3d`].
/// It is blanket-implemented for all types that implement [`Bounded3d`].
pub trait ComputeBoundingVolume3d {
    /// Computes the axis-aligned bounding box of the shape in `self`
    /// transformed by the given `isometry`.
    fn aabb_3d(&self, isometry: Isometry3d) -> Aabb3d;
    /// Computes the bounding circle of the shape in `self`
    /// transformed by the given `isometry`.
    fn bounding_sphere(&self, isometry: Isometry3d) -> BoundingSphere;
}

impl<T: Bounded3d> ComputeBoundingVolume3d for T {
    #[inline]
    fn aabb_3d(&self, isometry: Isometry3d) -> Aabb3d {
        self.aabb_3d(isometry)
    }

    #[inline]
    fn bounding_sphere(&self, isometry: Isometry3d) -> BoundingSphere {
        self.bounding_sphere(isometry)
    }
}

For my use case, I need bounding volume computation to be dyn-compatible so that I can have a more general GeometricShape3d trait that currently looks like this:

/// Generic trait for geometric shapes supported by [`Shape3d`].
pub trait GeometricShape3d:
    RayCast3d
    + PointQuery3d
    + ClosestNormal<Dim3>
    + ComputeBoundingVolume3d
    + ComputeMassProperties3d
    + Debug
    + DowncastSync
{
    /// Returns a dynamic reference to `self` as a [`SupportMap`] if it has one.
    fn as_support_map(&self) -> Option<&dyn SupportMap<Vec3A>> {
        None
    }

    /// Returns a dynamic reference to `self` as a [`SupportFeatureMap`] if it has one.
    fn as_support_feature_map(&self) -> Option<&dyn SupportFeatureMap<Dim3>> {
        None
    }
}

The reason it needs to be dyn-compatible is (1) it's needed for some downcasting logic, and (2) I need to store an Arc<dyn GeometricShape3d> to support custom shapes:

/// An enum for supported 3D shapes.
#[derive(Clone, Debug)]
#[allow(missing_docs)]
pub enum Shape3d {
    Sphere(Sphere),
    Cuboid(Cuboid),
    Cylinder(Cylinder),
    Cone(Cone),
    Capsule(Capsule3d),
    Triangle(Triangle3d),
    ConvexHull(ConvexHull3d),
    Custom(Arc<dyn GeometricShape3d>),
}

So I would personally be in favor of this change. More generally, I think most of our geometry traits should be dyn-compatible to support more dynamic use cases like this.

If we do this, please also give Bounded2d the same treatment (edit: ah I see you mentioned this 😄). 2D and 3D APIs should align here. This also needs a migration guide since it's a breaking change.

[omeranson]

Hi. Thanks for looking over this.

If we do this, please also give Bounded2d the same treatment (edit: ah I see you mentioned this 😄). 2D and 3D APIs should align here. This also needs a migration guide since it's a breaking change.

I added two commits to this PR.