Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ray2d and Ray3d primitives #10843

Closed
wants to merge 1 commit into from
Closed

Conversation

Jondolf
Copy link
Contributor

@Jondolf Jondolf commented Dec 2, 2023

Objective

Add 2D and 3D geometric primitives for rays as defined in the Primitive Shapes RFC, continuing the work on #10572.

Solution

Add Ray2d and Ray3d, defined like this:

/// An infinite half-line starting at `origin` and going in `direction` in 2D space.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Ray2d {
    /// The origin of the ray.
    pub origin: Vec2,
    /// The direction of the ray.
    pub direction: Direction2d,
}
/// An infinite half-line starting at `origin` and going in `direction` in 3D space.
#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Ray3d {
    /// The origin of the ray.
    pub origin: Vec3,
    /// The direction of the ray.
    pub direction: Direction3d,
}

They are mostly a copy of the existing Ray struct, but I decided not to add intersect_plane because ray intersections are out of scope for this PR and the method would be better handled by a general raycasting trait that is implemented for each shape separately.

Questions

Should the old Ray be removed already? It could be replaced by Ray3d, but it would be a breaking change as intersect_plane wouldn't exist anymore (for now). I could also just add intersect_plane if people find it to be important to have.

@Jondolf Jondolf added C-Feature A new feature, making something new possible A-Math Fundamental domain-agnostic mathematical operations labels Dec 2, 2023
@Jondolf Jondolf mentioned this pull request Dec 2, 2023
46 tasks
@eidloi
Copy link

eidloi commented Dec 2, 2023

Intersecting a plane is quite important, I frequently find uses for it. Would rather not re-implement it as an utility outside Bevy (even if it is not much math).

@Jondolf
Copy link
Contributor Author

Jondolf commented Dec 2, 2023

I'll add intersect_plane back for now at least.

After discussion on Discord, I believe Ray2d and Ray3d also shouldn't necessarily be primitives because primitives aren't supposed to have an origin. I'll move them into the existing ray module and change this PR to just split Ray into Ray2d and Ray3d

@Jondolf
Copy link
Contributor Author

Jondolf commented Dec 3, 2023

Closing in favor of #10856.

@Jondolf Jondolf closed this Dec 3, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2023
…10856)

# Objective

A better alternative version of #10843.

Currently, Bevy has a single `Ray` struct for 3D. To allow better
interoperability with Bevy's primitive shapes (#10572) and some third
party crates (that handle e.g. spatial queries), it would be very useful
to have separate versions for 2D and 3D respectively.

## Solution

Separate `Ray` into `Ray2d` and `Ray3d`. These new structs also take
advantage of the new primitives by using `Direction2d`/`Direction3d` for
the direction:

```rust
pub struct Ray2d {
    pub origin: Vec2,
    pub direction: Direction2d,
}

pub struct Ray3d {
    pub origin: Vec3,
    pub direction: Direction3d,
}
```

and by using `Plane2d`/`Plane3d` in `intersect_plane`:

```rust
impl Ray2d {
    // ...
    pub fn intersect_plane(&self, plane_origin: Vec2, plane: Plane2d) -> Option<f32> {
        // ...
    }
}
```

---

## Changelog

### Added

- `Ray2d` and `Ray3d`
- `Ray2d::new` and `Ray3d::new` constructors
- `Plane2d::new` and `Plane3d::new` constructors

### Removed

- Removed `Ray` in favor of `Ray3d`

### Changed

- `direction` is now a `Direction2d`/`Direction3d` instead of a vector,
which provides guaranteed normalization
- `intersect_plane` now takes a `Plane2d`/`Plane3d` instead of just a
vector for the plane normal
- `Direction2d` and `Direction3d` now derive `Serialize` and
`Deserialize` to preserve ray (de)serialization

## Migration Guide

`Ray` has been renamed to `Ray3d`.

### Ray creation

Before:

```rust
Ray {
    origin: Vec3::ZERO,
    direction: Vec3::new(0.5, 0.6, 0.2).normalize(),
}
```

After:

```rust
// Option 1:
Ray3d {
    origin: Vec3::ZERO,
    direction: Direction3d::new(Vec3::new(0.5, 0.6, 0.2)).unwrap(),
}

// Option 2:
Ray3d::new(Vec3::ZERO, Vec3::new(0.5, 0.6, 0.2))
```

### Plane intersections

Before:

```rust
let result = ray.intersect_plane(Vec2::X, Vec2::Y);
```

After:

```rust
let result = ray.intersect_plane(Vec2::X, Plane2d::new(Vec2::Y));
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations C-Feature A new feature, making something new possible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants