Skip to content

Commit

Permalink
Use minor and major radii for Torus primitive shape (#10643)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
Jondolf authored Nov 21, 2023
1 parent a22020b commit 897b13b
Showing 1 changed file with 89 additions and 7 deletions.
96 changes: 89 additions & 7 deletions crates/bevy_math/src/primitives/dim3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down Expand Up @@ -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,
}
}
}

0 comments on commit 897b13b

Please sign in to comment.