Skip to content

Commit

Permalink
Use Dir2/Dir3 instead of Vec2/Vec3 for Ray2d::new/`Ray3d::n…
Browse files Browse the repository at this point in the history
…ew` (#15735)

# Objective

The `new` constructors for our ray types currently take a `Vec2`/`Vec3`
instead of a `Dir2`/`Dir3`. This is confusing and footgunny for several
reasons.

- Which one of these is the direction? You can't see it from the type.

```rust
let ray = Ray2d::new(Vec2::X, Vec2::X);
```

- Many engines allow unnormalized rays, and this can affect ray cast
results by scaling the time of impact. However, in Bevy, rays are
*always* normalized despite what the input argument in this case
implies, and ray cast results are *not* scaled.

```rust
// The true ray direction is still normalized, unlike what you'd expect.
let ray = Ray2d::new(Vec2::X, Vec2::new(5.0, 0.0, 0.0)));
```

These cases are what the direction types are intended for, and we should
use them as such.

## Solution

Use `Dir2`/`Dir3` in the constructors.

```rust
let ray = Ray2d::new(Vec2::X, Dir2::X);
```

We *could* also use `impl TryInto<DirN>`, which would allow both vectors
and direction types, and then panic if the input is not normalized. This
could be fine for ergonomics in some cases, but especially for rays, I
think it's better to take an explicit direction type here.

---

## Migration Guide

`Ray2d::new` and `Ray3d::new` now take a `Dir2` and `Dir3` instead of
`Vec2` and `Vec3` respectively for the ray direction.
  • Loading branch information
Jondolf authored Oct 8, 2024
1 parent 82aa2e3 commit f6cd6a4
Showing 1 changed file with 6 additions and 20 deletions.
26 changes: 6 additions & 20 deletions crates/bevy_math/src/ray.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,9 @@ pub struct Ray2d {

impl Ray2d {
/// Create a new `Ray2d` from a given origin and direction
///
/// # Panics
///
/// Panics if the given `direction` is zero (or very close to zero), or non-finite.
#[inline]
pub fn new(origin: Vec2, direction: Vec2) -> Self {
Self {
origin,
direction: Dir2::new(direction).expect("ray direction must be nonzero and finite"),
}
pub const fn new(origin: Vec2, direction: Dir2) -> Self {
Self { origin, direction }
}

/// Get a point at a given distance along the ray
Expand Down Expand Up @@ -74,16 +67,9 @@ pub struct Ray3d {

impl Ray3d {
/// Create a new `Ray3d` from a given origin and direction
///
/// # Panics
///
/// Panics if the given `direction` is zero (or very close to zero), or non-finite.
#[inline]
pub fn new(origin: Vec3, direction: Vec3) -> Self {
Self {
origin,
direction: Dir3::new(direction).expect("ray direction must be nonzero and finite"),
}
pub const fn new(origin: Vec3, direction: Dir3) -> Self {
Self { origin, direction }
}

/// Get a point at a given distance along the ray
Expand Down Expand Up @@ -112,7 +98,7 @@ mod tests {

#[test]
fn intersect_plane_2d() {
let ray = Ray2d::new(Vec2::ZERO, Vec2::Y);
let ray = Ray2d::new(Vec2::ZERO, Dir2::Y);

// Orthogonal, and test that an inverse plane_normal has the same result
assert_eq!(
Expand Down Expand Up @@ -152,7 +138,7 @@ mod tests {

#[test]
fn intersect_plane_3d() {
let ray = Ray3d::new(Vec3::ZERO, Vec3::Z);
let ray = Ray3d::new(Vec3::ZERO, Dir3::Z);

// Orthogonal, and test that an inverse plane_normal has the same result
assert_eq!(
Expand Down

0 comments on commit f6cd6a4

Please sign in to comment.