Skip to content

Commit

Permalink
Change Ellipse representation and improve helpers (#11435)
Browse files Browse the repository at this point in the history
# Objective

Currently, the `Ellipse` primitive is represented by a `half_width` and
`half_height`. To improve consistency (similarly to #11434), it might
make more sense to use a `Vec2` `half_size` instead.

Alternatively, to make the elliptical nature clearer, the properties
could also be called `radius_x` and `radius_y`.

Secondly, `Ellipse::new` currently takes a *full* width and height
instead of two radii. I would expect it to take the half-width and
half-height because ellipses and circles are almost always defined using
radii. I wouldn't expect `Circle::new` to take a diameter (if we had
that method).

## Solution

Change `Ellipse` to store a `half_size` and `new` to take the half-width
and half-height.

I also added a `from_size` method similar to `Rectangle::from_size`, and
added the `semi_minor` and `semi_major` helpers to get the
semi-minor/major radius.
  • Loading branch information
Jondolf authored Jan 20, 2024
1 parent 6337fb3 commit b592a72
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 14 deletions.
9 changes: 3 additions & 6 deletions crates/bevy_math/src/bounding/bounded2d/primitive_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl Bounded2d for Ellipse {
// ### ###
// ###########

let (hw, hh) = (self.half_width, self.half_height);
let (hw, hh) = (self.half_size.x, self.half_size.y);

// Sine and cosine of rotation angle alpha.
let (alpha_sin, alpha_cos) = rotation.sin_cos();
Expand All @@ -56,7 +56,7 @@ impl Bounded2d for Ellipse {
}

fn bounding_circle(&self, translation: Vec2, _rotation: f32) -> BoundingCircle {
BoundingCircle::new(translation, self.half_width.max(self.half_height))
BoundingCircle::new(translation, self.semi_major())
}
}

Expand Down Expand Up @@ -276,10 +276,7 @@ mod tests {

#[test]
fn ellipse() {
let ellipse = Ellipse {
half_width: 1.0,
half_height: 0.5,
};
let ellipse = Ellipse::new(1.0, 0.5);
let translation = Vec2::new(2.0, 1.0);

let aabb = ellipse.aabb_2d(translation, 0.0);
Expand Down
40 changes: 32 additions & 8 deletions crates/bevy_math/src/primitives/dim2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,45 @@ impl Primitive2d for Circle {}
/// An ellipse primitive
#[derive(Clone, Copy, Debug)]
pub struct Ellipse {
/// The half "width" of the ellipse
pub half_width: f32,
/// The half "height" of the ellipse
pub half_height: f32,
/// Half of the width and height of the ellipse.
///
/// This corresponds to the two perpendicular radii defining the ellipse.
pub half_size: Vec2,
}
impl Primitive2d for Ellipse {}

impl Ellipse {
/// Create a new `Ellipse` from a "width" and a "height"
pub fn new(width: f32, height: f32) -> Self {
/// Create a new `Ellipse` from half of its width and height.
///
/// This corresponds to the two perpendicular radii defining the ellipse.
#[inline]
pub const fn new(half_width: f32, half_height: f32) -> Self {
Self {
half_size: Vec2::new(half_width, half_height),
}
}

/// Create a new `Ellipse` from a given full size.
///
/// `size.x` is the diameter along the X axis, and `size.y` is the diameter along the Y axis.
#[inline]
pub fn from_size(size: Vec2) -> Self {
Self {
half_width: width / 2.0,
half_height: height / 2.0,
half_size: size / 2.0,
}
}

/// Returns the length of the semi-major axis. This corresponds to the longest radius of the ellipse.
#[inline]
pub fn semi_major(self) -> f32 {
self.half_size.max_element()
}

/// Returns the length of the semi-minor axis. This corresponds to the shortest radius of the ellipse.
#[inline]
pub fn semi_minor(self) -> f32 {
self.half_size.min_element()
}
}

/// An unbounded plane in 2D space. It forms a separating surface through the origin,
Expand Down

0 comments on commit b592a72

Please sign in to comment.