Skip to content

Resolve overlap between bevy_render primitives and bevy_math primitives #13945

@Jondolf

Description

@Jondolf

This is an overview of the current bevy_render primitive vs. bevy_math primitive situation from my viewpoint, and how I personally think we should tackle this. This is somewhat in response to #13882 (see also #13878 and vaguely related #13455) but is something that has been in discussion for a long time.

Feel free to disagree and suggest alternative approaches :)

The problems

bevy_render currently has rendering primitives like the following:

  • Aabb
  • Sphere
  • HalfSpace
  • Frustum

All of these can be generally useful types that would be great to have in bevy_math, for numerous reasons (interoperability, consistency, more complete APIs, etc.). In fact, bevy_math already has Aabb2d, Aabb3d, and BoundingSphere, and #13882 even moves the remaining two types into bevy_math. Nice, this means that we can just remove the rendering primitives from bevy_render and have them all live happily in bevy_math, right?

Well, not quite. These rendering primitives are quite interconnected and have some details that are specific to rendering. For example:

  • Sphere is used on its own, but it also needs an intersects_obb method that takes an Aabb (which is actually treated more like an OBB) for cheaply pruning out meshes outside of light spheres.
  • Frustum (which is a view frustum, not just any frustum) needs to have methods for intersections against both Sphere and Aabb (or OBB) for the actual frustum culling.

These methods currently rely on Aabb having its center + half-extents representation, which is different from Aabb3d, so we cannot currently have for example Frustum::intersects_obb in bevy_math without also having Aabb there, unless we're fine with some extra cost or rework the logic. And we obviously don't want to have both Aabb and Aabb3d in bevy_math, at least under such similar names.

In addition, Aabb and Frustum need to be components in bevy_render anyway.

  • Aabb is a component used for mesh/sprite/text bounds and frustum culling.
  • Frustum is a component used by both lights and cameras for some culling.

They even have documentation specific to their behavior and usage in rendering, which would not make any sense in bevy_math.

#13882 takes the approach of just adding an optional bevy_ecs feature and a legacy_bevy_render module to bevy_math to simply chuck these types in there, with the intent of cleaning up later. However, I think this is a bad idea especially without a clear plan of how and when we will actually clean it up. This is not a particularly trivial thing to resolve or "clean up", and even easier things have slipped into releases even though the intent was to resolve them quickly after the initial merge (ex: missing plane subdivisions). I don't think bevy_math should ever have these kinds of things even momentarily without pressing reasons.

(Aside: Personally, I'm not sure if a view frustum even makes sense living in bevy_math, especially since it'd need some custom intersection methods anyway. It's not what I typically consider to be a geometric primitive even if it is a rendering primitive. But I guess it could still be considered to be a "generally useful gamedev math type", so I wouldn't fight against it.)

What solution would you like?

Okay, so all of this is tricky and I feel like there's no clear solution, but I'll start by listing what I personally think we should do:

  • Keep Aabb in bevy_render.
    • We need the component for mesh/sprite/text bounds and frustum culling anyway.
    • In my opinion it should definitely be renamed to be distinct from Aabb2d/Aabb3d though.
    • Later, we should test the implications of changing the internal representation to use Aabb3d (and benchmark!)
  • Maybe remove Sphere in favor of just BoundingSphere.
    • The only method missing from BoundingSphere is intersects_obb. This is kind of a weird one in terms of its input, and currently depends on the center + half-extents representation of Aabb.
    • If we remove Sphere from bevy_render, I think our options are:
      • Just have a standalone sphere_intersects_obb function in bevy_render, using BoundingSphere and (for now) Aabb.
      • Same as above, but with an extension trait for BoundingSphere.
      • Add an OBB type in bevy_math. See the "OBBs and removing Sphere" section.
  • Move HalfSpace to bevy_math, like Move Frustum, HalfSpace from bevy_render to bevy_math #13882 does. I think it's generally useful enough (also usable for collision stuff, for example), and not tied to rendering like the other types kind of are.
  • Move or don't move Frustum to bevy_math, either option is fine to me.
    • If we do move it, I would rename it to ViewFrustum. It is not a general frustum geometry-wise, and we already have e.g. ConicalFrustum. If we had Frustum, you'd think ConicalFrustum is a specialized version of it, but it isn't; they're both distinct frustum types.
    • Additionally, if we do move it, we need a newtype component in bevy_render.
    • The same intersects_obb issue as with Sphere; we don't want the rendering Aabb in bevy_math.

I do not want:

  • A dependency on bevy_ecs in bevy_math even optionally, unless it is for a very pressing reason. The geometric primitives should not be components, for that we should just create wrapper types in the respective crates.
  • A legacy rendering module or similar in bevy_math. From a design standpoint, why would bevy_math have a temporary stash for bevy_render types? We could just as well have a legacy_primitives module in bevy_render which is what the types are actually for. Although they're not even legacy types necessarily, since bevy_render needs to keep some AABB component type (and frustum component) anyway, and Sphere just needs to be refactored away.

OBBs and removing Sphere

For the intersects_obb issue mentioned earlier, one option is to add an Obb3d bounding volume type to bevy_math. It can be useful even outside of rendering.

Currently, the method looks like this:

impl Sphere {
    #[inline]
    pub fn intersects_obb(&self, aabb: &Aabb, world_from_local: &Affine3A) -> bool {
        let aabb_center_world = world_from_local.transform_point3a(aabb.center);
        let v = aabb_center_world - self.center;
        let d = v.length();
        let relative_radius = aabb.relative_radius(&(v / d), &world_from_local.matrix3);
        d < self.radius + relative_radius
    }
}

We could store the required data in an actual type:

// Note: I'm not sold on this representation. It includes scale unlike other bounding volumes,
//       and I believe it is not the best for e.g. OBB-OBB intersection tests.
pub struct Obb3d {
    pub affine: Affine3A,
    pub half_size: Vec3,
}

Then, we can implement the functionality that currently exists on Aabb and Sphere:

impl Obb3d {
    /// Creates an [`Obb3d`] from an [`Aabb3d`] and [`Affine3A`].
    pub fn from_aabb_and_affine(aabb: Aabb3d, mut affine: Affine3A) -> Self {
        affine.translation = affine.transform_point3a(aabb.center());
        
        Self {
            affine,
            half_size: aabb.half_size(),
        }
    }
    
    // This is currently on `Aabb`, although I don't love the method name
    /// Calculate the relative radius of the AABB with respect to a plane
    #[inline]
    pub fn relative_radius(&self, plane_normal: Vec3A) -> f32 {
        // NOTE: dot products on Vec3A use SIMD and even with the overhead of conversion are net faster than Vec3
        let half_extents = self.half_size;
        Vec3A::new(
            plane_normal.dot(self.affine.matrix3.x_axis),
            plane_normal.dot(self.affine.matrix3.y_axis),
            plane_normal.dot(self.affine.matrix3.z_axis),
        )
        .abs()
        .dot(half_extents)
    }
}

impl IntersectsVolume<Obb3d> for BoundingSphere {
    // This is `Sphere::intersect_obb`, but slightly modified and better commented :)
    fn intersects(&self, obb: &Obb3d) -> bool {
        let center_offset = obb.affine.translation - self.center;
        let center_distance = center_offset.length();

        // Compute the distance from the OBB center to its boundary in the direction of the sphere center.
        let relative_radius = obb.relative_radius(center_offset / center_distance);

        // If the minimum distance from the sphere center to the OBB
        // is less than the sphere radius, the shapes intersect.
        center_distance < self.radius() + relative_radius
    }
}

(Note: I have not tested the above, but functionally it should hopefully be the same as before)

This would let us cleanly get rid of Sphere, remove the dependence on Aabb for both Sphere and Frustum, and overall just give bevy_math some more useful functionality (I also have a form of OBB-OBB intersections implemented already). The OBB construction has a slight extra cost because of the min + max representation of Aabb3d, and the scaling in Affine3A might be a bit iffy for some things, but it should probably be alright. Again, benchmarking would probably be important for rendering people.

Alternatives

Just leave things as is. I think we should do something though, like (some of) the things I listed earlier.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-MathFundamental domain-agnostic mathematical operationsA-RenderingDrawing game state to the screenC-Code-QualityA section of code that is hard to understand or changeS-Ready-For-ImplementationThis issue is ready for an implementation PR. Go for it!

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions