You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 :)
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:
implSphere{#[inline]pubfnintersects_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.pubstructObb3d{pubaffine:Affine3A,pubhalf_size:Vec3,}
Then, we can implement the functionality that currently exists on Aabb and Sphere:
implObb3d{/// Creates an [`Obb3d`] from an [`Aabb3d`] and [`Affine3A`].pubfnfrom_aabb_and_affine(aabb:Aabb3d,mutaffine: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]pubfnrelative_radius(&self,plane_normal:Vec3A) -> f32{// NOTE: dot products on Vec3A use SIMD and even with the overhead of conversion are net faster than Vec3let 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)}}implIntersectsVolume<Obb3d>forBoundingSphere{// This is `Sphere::intersect_obb`, but slightly modified and better commented :)fnintersects(&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.
This is an overview of the current
bevy_renderprimitive vs.bevy_mathprimitive 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_rendercurrently has rendering primitives like the following:AabbSphereHalfSpaceFrustumAll 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_mathalready hasAabb2d,Aabb3d, andBoundingSphere, and #13882 even moves the remaining two types intobevy_math. Nice, this means that we can just remove the rendering primitives frombevy_renderand have them all live happily inbevy_math, right?Well, not quite. These rendering primitives are quite interconnected and have some details that are specific to rendering. For example:
Sphereis used on its own, but it also needs anintersects_obbmethod that takes anAabb(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 bothSphereandAabb(or OBB) for the actual frustum culling.These methods currently rely on
Aabbhaving its center + half-extents representation, which is different fromAabb3d, so we cannot currently have for exampleFrustum::intersects_obbinbevy_mathwithout also havingAabbthere, unless we're fine with some extra cost or rework the logic. And we obviously don't want to have bothAabbandAabb3dinbevy_math, at least under such similar names.In addition,
AabbandFrustumneed to be components inbevy_renderanyway.Aabbis a component used for mesh/sprite/text bounds and frustum culling.Frustumis 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_ecsfeature and alegacy_bevy_rendermodule tobevy_mathto 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 thinkbevy_mathshould 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:
Aabbinbevy_render.Aabb2d/Aabb3dthough.Aabb3d(and benchmark!)Spherein favor of justBoundingSphere.BoundingSphereisintersects_obb. This is kind of a weird one in terms of its input, and currently depends on the center + half-extents representation ofAabb.Spherefrombevy_render, I think our options are:sphere_intersects_obbfunction inbevy_render, usingBoundingSphereand (for now)Aabb.BoundingSphere.bevy_math. See the "OBBs and removingSphere" section.HalfSpacetobevy_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.Frustumtobevy_math, either option is fine to me.ViewFrustum. It is not a general frustum geometry-wise, and we already have e.g.ConicalFrustum. If we hadFrustum, you'd thinkConicalFrustumis a specialized version of it, but it isn't; they're both distinct frustum types.bevy_render.intersects_obbissue as withSphere; we don't want the renderingAabbinbevy_math.I do not want:
bevy_ecsinbevy_matheven 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.bevy_math. From a design standpoint, why wouldbevy_mathhave a temporary stash forbevy_rendertypes? We could just as well have alegacy_primitivesmodule inbevy_renderwhich is what the types are actually for. Although they're not even legacy types necessarily, sincebevy_renderneeds to keep some AABB component type (and frustum component) anyway, andSpherejust needs to be refactored away.OBBs and removing
SphereFor the
intersects_obbissue mentioned earlier, one option is to add anObb3dbounding volume type tobevy_math. It can be useful even outside of rendering.Currently, the method looks like this:
We could store the required data in an actual type:
Then, we can implement the functionality that currently exists on
AabbandSphere:(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 onAabbfor bothSphereandFrustum, and overall just givebevy_mathsome 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 ofAabb3d, and the scaling inAffine3Amight 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.