From 897b13bf9d003c18d2ac4cb127bb779d3dff7185 Mon Sep 17 00:00:00 2001 From: Joona Aalto Date: Tue, 21 Nov 2023 03:49:35 +0200 Subject: [PATCH] Use minor and major radii for `Torus` primitive shape (#10643) # Objective First, some terminology: - **Minor radius**: The radius of the tube of a torus, i.e. the "half-thickness" - **Major radius**: The distance from the center of the tube to the center of the torus - **Inner radius**: The radius of the hole (if it exists), `major_radius - minor_radius` - **Outer radius**: The radius of the overall shape, `major_radius + minor_radius` - **Ring torus**: The familiar donut shape with a hole in the center, `major_radius > minor_radius` - **Horn torus**: A torus that doesn't have a hole but also isn't self-intersecting, `major_radius == minor_radius` - **Spindle torus**: A self-intersecting torus, `major_radius < minor_radius` Different tori from [Wikipedia](https://en.wikipedia.org/wiki/Torus), where *R* is the major radius and *r* is the minor radius: ![kuva](https://github.com/bevyengine/bevy/assets/57632562/53ead786-2402-43a7-ae8a-5720e6e54dcc) Currently, Bevy's torus is represented by a `radius` and `ring_radius`. I believe these correspond to the outer radius and minor radius, but they are rather confusing and inconsistent names, and they make the assumption that the torus always has a ring. I also couldn't find any other big engines using this representation; [Godot](https://docs.godotengine.org/en/stable/classes/class_torusmesh.html) and [Unity ProBuilder](https://docs.unity3d.com/Packages/com.unity.probuilder@4.0/manual/Torus.html) use the inner and outer radii, while [Unreal](https://docs.unrealengine.com/5.3/en-US/BlueprintAPI/GeometryScript/Primitives/AppendTorus/) uses the minor and major radii. [Blender](https://docs.blender.org/manual/en/latest/modeling/meshes/primitives.html#torus) supports both, but defaults to minor/major. Bevy's `Torus` primitive should have an efficient, consistent, clear and flexible representation, and the current `radius` and `ring_radius` properties are not ideal for that. ## Solution Change `Torus` to be represented by a `minor_radius` and `major_radius`. - Mathematically correct and consistent - Flexible, not restricted to ring tori - Computations and conversions are efficient - `inner_radius = major_radius - minor_radius` - `outer_radius = major_radius + minor_radius` - Mathematical formulae for things like area and volume rely on the minor and major radii, no conversion needed Perhaps the primary issue with this representation is that "minor radius" and "major radius" are rather mathematical, and an inner/outer radius can be more intuitive in some cases. However, this can be mitigated with constructors and helpers. --- crates/bevy_math/src/primitives/dim3.rs | 96 +++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 7 deletions(-) diff --git a/crates/bevy_math/src/primitives/dim3.rs b/crates/bevy_math/src/primitives/dim3.rs index 60105622f57ce..891f51558fd3b 100644 --- a/crates/bevy_math/src/primitives/dim3.rs +++ b/crates/bevy_math/src/primitives/dim3.rs @@ -161,7 +161,7 @@ pub struct Cuboid { impl Primitive3d for Cuboid {} impl Cuboid { - /// Create a cuboid from a full x, y and z length + /// Create a cuboid from a full x, y, and z length pub fn new(x_length: f32, y_length: f32, z_length: f32) -> Self { Self::from_size(Vec3::new(x_length, y_length, z_length)) } @@ -240,12 +240,94 @@ pub struct ConicalFrustum { } impl Primitive3d for ConicalFrustum {} -/// A torus (AKA donut) primitive. -#[derive(Clone, Copy, Debug)] +/// The type of torus determined by the minor and major radii +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum TorusKind { + /// A torus that has a ring. + /// The major radius is greater than the minor radius + Ring, + /// A torus that has no hole but also doesn't intersect itself. + /// The major radius is equal to the minor radius + Horn, + /// A self-intersecting torus. + /// The major radius is less than the minor radius + Spindle, + /// A torus with non-geometric properties like + /// a minor or major radius that is non-positive, + /// infinite, or `NaN` + Invalid, +} + +/// A torus primitive, often representing a ring or donut shape +#[derive(Clone, Copy, Debug, PartialEq)] pub struct Torus { - /// The radius of the overall shape - pub radius: f32, - /// The radius of the internal ring - pub ring_radius: f32, + /// The radius of the tube of the torus + #[doc( + alias = "ring_radius", + alias = "tube_radius", + alias = "cross_section_radius" + )] + pub minor_radius: f32, + /// The distance from the center of the torus to the center of the tube + #[doc(alias = "radius_of_revolution")] + pub major_radius: f32, } impl Primitive3d for Torus {} + +impl Torus { + /// Create a new `Torus` from an inner and outer radius. + /// + /// The inner radius is the radius of the hole, and the outer radius + /// is the radius of the entire object + pub fn new(inner_radius: f32, outer_radius: f32) -> Self { + let minor_radius = (outer_radius - inner_radius) / 2.0; + let major_radius = outer_radius - minor_radius; + + Self { + minor_radius, + major_radius, + } + } + + /// Get the inner radius of the torus. + /// For a ring torus, this corresponds to the radius of the hole, + /// or `major_radius - minor_radius` + #[inline] + pub fn inner_radius(&self) -> f32 { + self.major_radius - self.minor_radius + } + + /// Get the outer radius of the torus. + /// This corresponds to the overall radius of the entire object, + /// or `major_radius + minor_radius` + #[inline] + pub fn outer_radius(&self) -> f32 { + self.major_radius + self.minor_radius + } + + /// Get the [`TorusKind`] determined by the minor and major radii. + /// + /// The torus can either be a *ring torus* that has a hole, + /// a *horn torus* that doesn't have a hole but also isn't self-intersecting, + /// or a *spindle torus* that is self-intersecting. + /// + /// If the minor or major radius is non-positive, infinite, or `NaN`, + /// [`TorusKind::Invalid`] is returned + #[inline] + pub fn kind(&self) -> TorusKind { + // Invalid if minor or major radius is non-positive, infinite, or NaN + if self.minor_radius <= 0.0 + || !self.minor_radius.is_finite() + || self.major_radius <= 0.0 + || !self.major_radius.is_finite() + { + return TorusKind::Invalid; + } + + match self.major_radius.partial_cmp(&self.minor_radius).unwrap() { + std::cmp::Ordering::Greater => TorusKind::Ring, + std::cmp::Ordering::Equal => TorusKind::Horn, + std::cmp::Ordering::Less => TorusKind::Spindle, + } + } +}