Skip to content

Commit

Permalink
Improve SpatialQuery APIs and docs, and add more configuration (#510)
Browse files Browse the repository at this point in the history
# Objective

Fixes #458, among many issues that have come up in discussions.

The API for shape casts through `SpatialQuery` is quite hard to read. If we supported all of Parry's configuration options for shape casts, `shape_hits` would end up looking like this:

```rust
// Note: target_distance and compute_impact_on_penetration don't exist prior to this PR
let hits = spatial.shape_hits(
    &shape,
    origin,
    shape_rotation,
    direction,
    max_time_of_impact,
    max_hits,
    target_distance,
    compute_impact_on_penetration,
    ignore_origin_penetration,
    &query_filter,
);
```

Without variables for everything, it would be almost entirely unreadable:

```rust
let hits = spatial.shape_hits(
    &Collider::sphere(0.5),
    Vec3::ZERO,
    Quat::default(),
    Dir3::ZERO,
    100.0,
    10,
    0.0,
    true,
    false,
    &default(),
);
```

Aside from bad ergonomics and poor readability, this is also bad for documentation, as the docs for each configuration option must be duplicated for each method that uses them. There are also no sane defaults, as *every* setting must be configured explicitly, even if the user is unlikely to need to care about some of them. This is especially bad for new users for whom the long list of arguments may be confusing and daunting.

Another slightly unrelated issue is that many properties and docs use the term "time of impact", which has reportedly been a bit confusing for many users. In Avian's case, for ray casts and shape casts, it is actually just the distance travelled along the ray, because the cast direction is normalized. Most occurrences of "time of impact" should likely be renamed to just "distance", except for time-of-impact queries where both shapes are moving or there is angular velocity involved.

## Solution

This PR fixes several API and documentation issues, along with doing general cleanup. It's a bit large with some slightly unrelated changes, so apologies in advance for potential reviewers!

Shape casting methods now take a `ShapeCastConfig`. It holds general configuration options for the cast, and could be extended in the future to even have things like debug rendering options.

The earlier example would now look like this:

```rust
let hits = spatial.shape_hits(
    &shape,
    origin,
    shape_rotation,
    direction,
    max_hits,
    &ShapeCastConfig {
        max_distance,
        target_distance,
        compute_impact_on_penetration,
        ignore_origin_penetration,
    },
    &filter,
);
```

In practice, you can often use default values for most properties, use constructor methods, and might move the configuration outside the method call (and even reuse it across shape casts), so it could actually look like this:

```rust
let config = ShapeCastConfig::from_max_distance(100.0);
let hits = spatial.shape_hits(
    &Collider::sphere(0.5),
    Vec3::ZERO,
    Quat::default(),
    Dir3::ZERO,
    10,
    &config,
    &filter,
);
```

As you may have noticed, I also changed `max_time_of_impact` to `max_distance`. Most occurrences of "time of impact" have indeed been renamed to "distance" as per the reasons mentioned earlier.

Additionally, I fixed several documentation issues and added missing methods and configuration options. See the changelog below for a slightly more thorough overview.

---

## Changelog

- Added `ShapeCastConfig`. Most shape casting methods now take the config instead of many separate arguments
- Renamed `RayHitData::time_of_impact`, `ShapeHitData::time_of_impact`, and many other occurrences of "time of impact" to `distance`
- Added missing predicate versions of spatial query methods which existed on `SpatialQueryPipeline`, but not `SpatialQuery`
- Added `target_distance` and `compute_impact_on_penetration` configuration options for shape casts, to match Parry's capabilities
- Fixed several documentation issues, such as:
    - Ray and shape hit data is in *world space*, not local space
    - Many intra-doc for "See also" sections link to self, not other methods
    - Mentions of "ray" in shape casting docs
    - Confusing wording for `predicate` argument docs
- Improved documentation examples
- Improved clarity and readability in a lot of docs
- Changed `excluded_entities` to use an `EntityHashSet`

## Migration Guide

### Shape Casting Configuration

`SpatialQuery` methods like `cast_shape` and `shape_hits` now take a `ShapeCastConfig`, which contains a lot of the existing configuration options, along with a few new options.

```rust
// Before
let hits = spatial.shape_hits(
    &Collider::sphere(0.5),
    Vec3::ZERO,
    Quat::default(),
    Dir3::ZERO,
    100.0,
    10,
    false,
    &SpatialQueryFilter::from_mask(LayerMask::ALL),
);

// After
let hits = spatial.shape_hits(
    &Collider::sphere(0.5),
    Vec3::ZERO,
    Quat::default(),
    Dir3::ZERO,
    10,
    &ShapeCastConfig::from_max_distance(100.0),
    &SpatialQueryFilter::from_mask(LayerMask::ALL),
);
```

### Time of Impact → Distance

Spatial query APIs that mention the "time of impact" have been changed to refer to "distance". This affects names of properties and methods, such as:

- `RayCaster::max_time_of_impact` → `RayCaster::max_distance`
- `RayCaster::with_max_time_of_impact` → `RayCaster::with_max_distance`
- `ShapeCaster::max_time_of_impact` → `ShapeCaster::max_distance`
- `ShapeCaster::with_max_time_of_impact` → `ShapeCaster::with_max_distance`
- `RayHitData::time_of_impact` → `RayHitData::distance`
- `ShapeHitData::time_of_impact` → `ShapeHitData::distance`
- `max_time_of_impact` on `SpatialQuery` methods → `RayCastConfig::max_distance` or `ShapeCastConfig::max_distance`

This was changed because "distance" is clearer than "time of impact" for many users, and it is still an accurate term, as the cast directions in Avian are always normalized, so the "velocity" is of unit length.
  • Loading branch information
Jondolf authored Dec 8, 2024
1 parent 5b8e64c commit ba88219
Show file tree
Hide file tree
Showing 16 changed files with 852 additions and 500 deletions.
2 changes: 1 addition & 1 deletion crates/avian2d/examples/dynamic_character_2d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ impl CharacterControllerBundle {
rigid_body: RigidBody::Dynamic,
collider,
ground_caster: ShapeCaster::new(caster_shape, Vector::ZERO, 0.0, Dir2::NEG_Y)
.with_max_time_of_impact(10.0),
.with_max_distance(10.0),
locked_axes: LockedAxes::ROTATION_LOCKED,
movement: MovementBundle::default(),
}
Expand Down
2 changes: 1 addition & 1 deletion crates/avian2d/examples/kinematic_character_2d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl CharacterControllerBundle {
rigid_body: RigidBody::Kinematic,
collider,
ground_caster: ShapeCaster::new(caster_shape, Vector::ZERO, 0.0, Dir2::NEG_Y)
.with_max_time_of_impact(10.0),
.with_max_distance(10.0),
gravity: ControllerGravity(gravity),
movement: MovementBundle::default(),
}
Expand Down
6 changes: 1 addition & 5 deletions crates/avian2d/examples/ray_caster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ fn render_rays(mut rays: Query<(&mut RayCaster, &mut RayHits)>, mut gizmos: Gizm
let direction = ray.global_direction().f32();

for hit in hits.iter() {
gizmos.line_2d(
origin,
origin + direction * hit.time_of_impact as f32,
GREEN,
);
gizmos.line_2d(origin, origin + direction * hit.distance as f32, GREEN);
}
if hits.is_empty() {
gizmos.line_2d(origin, origin + direction * 1_000_000.0, ORANGE_RED);
Expand Down
33 changes: 14 additions & 19 deletions crates/avian3d/examples/cast_ray_predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,38 +145,33 @@ fn raycast(
query: SpatialQuery,
mut materials: ResMut<Assets<StandardMaterial>>,
cubes: Query<(&MeshMaterial3d<StandardMaterial>, &OutOfGlass)>,
mut indicator_transform: Query<&mut Transform, With<RayIndicator>>,
mut indicator_transform: Single<&mut Transform, With<RayIndicator>>,
) {
let origin = Vector::new(-200.0, 2.0, 0.0);
let direction = Dir3::X;
let filter = SpatialQueryFilter::default();

let mut ray_indicator_transform = indicator_transform.single_mut();

if let Some(ray_hit_data) = query.cast_ray_predicate(
origin,
direction,
Scalar::MAX,
true,
&SpatialQueryFilter::default(),
&|entity| {
if let Some(ray_hit_data) =
query.cast_ray_predicate(origin, direction, Scalar::MAX, true, &filter, &|entity| {
// Only look at cubes not made out of glass.
if let Ok((_, out_of_glass)) = cubes.get(entity) {
return !out_of_glass.0; // only look at cubes not out of glass
return !out_of_glass.0;
}
true // if the collider has no OutOfGlass component, then check it nevertheless
},
) {
// set color of hit object to red
true
})
{
// Set the color of the hit object to red.
if let Ok((material_handle, _)) = cubes.get(ray_hit_data.entity) {
if let Some(material) = materials.get_mut(material_handle) {
material.base_color = RED.into();
}
}

// set length of ray indicator to look more like a laser
let contact_point = (origin + direction.adjust_precision() * ray_hit_data.time_of_impact).x;
// Set the length of the ray indicator to look more like a laser,
let contact_point = (origin + direction.adjust_precision() * ray_hit_data.distance).x;
let target_scale = 1000.0 + contact_point * 2.0;
ray_indicator_transform.scale.x = target_scale as f32;
indicator_transform.scale.x = target_scale as f32;
} else {
ray_indicator_transform.scale.x = 2000.0;
indicator_transform.scale.x = 2000.0;
}
}
2 changes: 1 addition & 1 deletion crates/avian3d/examples/dynamic_character_3d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl CharacterControllerBundle {
Quaternion::default(),
Dir3::NEG_Y,
)
.with_max_time_of_impact(0.2),
.with_max_distance(0.2),
locked_axes: LockedAxes::ROTATION_LOCKED,
movement: MovementBundle::default(),
}
Expand Down
2 changes: 1 addition & 1 deletion crates/avian3d/examples/kinematic_character_3d/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl CharacterControllerBundle {
Quaternion::default(),
Dir3::NEG_Y,
)
.with_max_time_of_impact(0.2),
.with_max_distance(0.2),
gravity: ControllerGravity(gravity),
movement: MovementBundle::default(),
}
Expand Down
16 changes: 8 additions & 8 deletions src/collision/collider/parry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,16 +624,16 @@ impl Collider {
.contains_point(&make_isometry(translation, rotation), &point.into())
}

/// Computes the time of impact and normal between the given ray and `self`
/// Computes the distance and normal between the given ray and `self`
/// transformed by `translation` and `rotation`.
///
/// The returned tuple is in the format `(time_of_impact, normal)`.
/// The returned tuple is in the format `(distance, normal)`.
///
/// # Arguments
///
/// - `ray_origin`: Where the ray is cast from.
/// - `ray_direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `max_distance`: The maximum distance the ray can travel.
/// - `solid`: If true and the ray origin is inside of a collider, the hit point will be the ray origin itself.
/// Otherwise, the collider will be treated as hollow, and the hit point will be at the collider's boundary.
pub fn cast_ray(
Expand All @@ -642,13 +642,13 @@ impl Collider {
rotation: impl Into<Rotation>,
ray_origin: Vector,
ray_direction: Vector,
max_time_of_impact: Scalar,
max_distance: Scalar,
solid: bool,
) -> Option<(Scalar, Vector)> {
let hit = self.shape_scaled().cast_ray_and_get_normal(
&make_isometry(translation, rotation),
&parry::query::Ray::new(ray_origin.into(), ray_direction.into()),
max_time_of_impact,
max_distance,
solid,
);
hit.map(|hit| (hit.time_of_impact, hit.normal.into()))
Expand All @@ -660,19 +660,19 @@ impl Collider {
///
/// - `ray_origin`: Where the ray is cast from.
/// - `ray_direction`: What direction the ray is cast in.
/// - `max_time_of_impact`: The maximum distance that the ray can travel.
/// - `max_distance`: The maximum distance the ray can travel.
pub fn intersects_ray(
&self,
translation: impl Into<Position>,
rotation: impl Into<Rotation>,
ray_origin: Vector,
ray_direction: Vector,
max_time_of_impact: Scalar,
max_distance: Scalar,
) -> bool {
self.shape_scaled().intersects_ray(
&make_isometry(translation, rotation),
&parry::query::Ray::new(ray_origin.into(), ray_direction.into()),
max_time_of_impact,
max_distance,
)
}

Expand Down
28 changes: 14 additions & 14 deletions src/debug_render/gizmos.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub trait PhysicsGizmoExt {
&mut self,
origin: Vector,
direction: Dir,
max_time_of_impact: Scalar,
max_distance: Scalar,
hits: &[RayHitData],
ray_color: Color,
point_color: Color,
Expand All @@ -75,7 +75,7 @@ pub trait PhysicsGizmoExt {
origin: Vector,
shape_rotation: impl Into<Rotation>,
direction: Dir,
max_time_of_impact: Scalar,
max_distance: Scalar,
hits: &[ShapeHitData],
ray_color: Color,
shape_color: Color,
Expand Down Expand Up @@ -458,29 +458,29 @@ impl PhysicsGizmoExt for Gizmos<'_, '_, PhysicsGizmos> {
&mut self,
origin: Vector,
direction: Dir,
max_time_of_impact: Scalar,
max_distance: Scalar,
hits: &[RayHitData],
ray_color: Color,
point_color: Color,
normal_color: Color,
length_unit: Scalar,
) {
let max_toi = hits
let max_distance = hits
.iter()
.max_by(|a, b| a.time_of_impact.total_cmp(&b.time_of_impact))
.map_or(max_time_of_impact, |hit| hit.time_of_impact);
.max_by(|a, b| a.distance.total_cmp(&b.distance))
.map_or(max_distance, |hit| hit.distance);

// Draw ray as arrow
self.draw_arrow(
origin,
origin + direction.adjust_precision() * max_toi,
origin + direction.adjust_precision() * max_distance,
0.1 * length_unit,
ray_color,
);

// Draw all hit points and normals
for hit in hits {
let point = origin + direction.adjust_precision() * hit.time_of_impact;
let point = origin + direction.adjust_precision() * hit.distance;

// Draw hit point
#[cfg(feature = "2d")]
Expand Down Expand Up @@ -510,7 +510,7 @@ impl PhysicsGizmoExt for Gizmos<'_, '_, PhysicsGizmos> {
origin: Vector,
shape_rotation: impl Into<Rotation>,
direction: Dir,
max_time_of_impact: Scalar,
max_distance: Scalar,
hits: &[ShapeHitData],
ray_color: Color,
shape_color: Color,
Expand All @@ -522,10 +522,10 @@ impl PhysicsGizmoExt for Gizmos<'_, '_, PhysicsGizmos> {
#[cfg(feature = "3d")]
let shape_rotation = Rotation(shape_rotation.normalize());

let max_toi = hits
let max_distance = hits
.iter()
.max_by(|a, b| a.time_of_impact.total_cmp(&b.time_of_impact))
.map_or(max_time_of_impact, |hit| hit.time_of_impact);
.max_by(|a, b| a.distance.total_cmp(&b.distance))
.map_or(max_distance, |hit| hit.distance);

// Draw collider at origin
self.draw_collider(shape, origin, shape_rotation, shape_color);
Expand All @@ -534,7 +534,7 @@ impl PhysicsGizmoExt for Gizmos<'_, '_, PhysicsGizmos> {
// TODO: We could render the swept collider outline instead
self.draw_arrow(
origin,
origin + max_toi * direction.adjust_precision(),
origin + max_distance * direction.adjust_precision(),
0.1 * length_unit,
ray_color,
);
Expand All @@ -558,7 +558,7 @@ impl PhysicsGizmoExt for Gizmos<'_, '_, PhysicsGizmos> {
// Draw collider at hit point
self.draw_collider(
shape,
origin + hit.time_of_impact * direction.adjust_precision(),
origin + hit.distance * direction.adjust_precision(),
shape_rotation,
shape_color.with_alpha(0.3),
);
Expand Down
4 changes: 2 additions & 2 deletions src/debug_render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ fn debug_render_raycasts(
ray.global_origin(),
ray.global_direction(),
// f32::MAX renders nothing, but this number seems to be fine :P
ray.max_time_of_impact.min(1_000_000_000_000_000_000.0),
ray.max_distance.min(1_000_000_000_000_000_000.0),
hits.as_slice(),
ray_color,
point_color,
Expand Down Expand Up @@ -459,7 +459,7 @@ fn debug_render_shapecasts(
shape_caster.global_shape_rotation(),
shape_caster.global_direction(),
// f32::MAX renders nothing, but this number seems to be fine :P
shape_caster.max_time_of_impact.min(1_000_000_000_000_000.0),
shape_caster.max_distance.min(1_000_000_000_000_000.0),
hits.as_slice(),
ray_color,
shape_color,
Expand Down
6 changes: 3 additions & 3 deletions src/picking/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ pub fn update_hits(
)
.map(|ray_hit_data| {
#[allow(clippy::unnecessary_cast)]
let toi = ray_hit_data.time_of_impact as f32;
let distance = ray_hit_data.distance as f32;
let hit_data = HitData::new(
ray_id.camera,
toi,
Some(ray.origin + *ray.direction * toi),
distance,
Some(ray.get_point(distance)),
Some(ray_hit_data.normal.f32()),
);
(ray_hit_data.entity, hit_data)
Expand Down
14 changes: 6 additions & 8 deletions src/spatial_query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
//! a variety of things like getting information about the environment for character controllers and AI,
//! and even rendering using ray tracing.
//!
//! For each hit during raycasting, the hit entity, a *time of impact* and a normal will be stored in [`RayHitData`].
//! The time of impact refers to how long the ray travelled, which is essentially the distance from the ray origin to
//! the point of intersection.
//! For each hit during raycasting, the hit entity, a distance, and a normal will be stored in [`RayHitData`].
//! The distance is the distance from the ray origin to the point of intersection, indicating how far the ray travelled.
//!
//! There are two ways to perform raycasts.
//!
Expand Down Expand Up @@ -59,7 +58,7 @@
//! println!(
//! "Hit entity {} at {} with normal {}",
//! hit.entity,
//! ray.origin + *ray.direction * hit.time_of_impact,
//! ray.origin + *ray.direction * hit.distance,
//! hit.normal,
//! );
//! }
Expand All @@ -76,9 +75,8 @@
//! we have an entire shape travelling along a half-line. One use case is determining how far an object can move
//! before it hits the environment.
//!
//! For each hit during shapecasting, the hit entity, the *time of impact*, two local points of intersection and two local
//! normals will be stored in [`ShapeHitData`]. The time of impact refers to how long the shape travelled before the initial
//! hit, which is essentially the distance from the shape origin to the global point of intersection.
//! For each hit during shapecasting, the hit entity, a distance, two world-space points of intersection and two world-space
//! normals will be stored in [`ShapeHitData`]. The distance refers to how far the shape travelled before the initial hit.
//!
//! There are two ways to perform shapecasts.
//!
Expand Down Expand Up @@ -107,7 +105,7 @@
//! Collider::sphere(0.5), // Shape
//! Vec3::ZERO, // Origin
//! Quat::default(), // Shape rotation
//! Dir3::X // Direction
//! Dir3::X // Direction
//! ));
//! // ...spawn colliders and other things
//! }
Expand Down
Loading

0 comments on commit ba88219

Please sign in to comment.