Skip to content

Commit

Permalink
Fix the fast_forward() error case.
Browse files Browse the repository at this point in the history
Fuzzing revealed that it can (unsurprisingly in hindsight) spend way too
much CPU time on our fallback of “don't fast-forward”, so we'll just
have to be technically incorrect and return nothing. This should not
affect any practical situations.
  • Loading branch information
kpreid committed Sep 13, 2023
1 parent 7ff3357 commit ea71e10
Showing 1 changed file with 26 additions and 12 deletions.
38 changes: 26 additions & 12 deletions all-is-cubes/src/raycast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,7 @@ impl Raycaster {
Some(cube) => cube.lower_bounds(),
None => {
// Return a raycaster which emits no cubes.
return Self {
ray: Ray::new(origin, direction),
emit_current: false, // emit no cubes
cube: GridPoint::origin(),
step: Vector3::zero(),
t_max: Vector3::zero(),
t_delta: Vector3::new(f64::INFINITY, f64::INFINITY, f64::INFINITY),
last_face: Face7::Within,
last_t_distance: 0.0,
bounds: None,
};
return Self::new_empty();
}
};

Expand All @@ -254,6 +244,22 @@ impl Raycaster {
}
}

/// Construct a [`Raycaster`] which will produce no items.
/// This is used when numeric overflow prevents success.
fn new_empty() -> Self {
Self {
ray: Ray::new(Point3::origin(), Vector3::zero()),
emit_current: false,
cube: GridPoint::origin(),
step: Vector3::zero(),
t_max: Vector3::zero(),
t_delta: Vector3::new(f64::INFINITY, f64::INFINITY, f64::INFINITY),
last_face: Face7::Within,
last_t_distance: 0.0,
bounds: None,
}
}

/// Restrict the cubes iterated over to those which lie within the given [`GridAab`].
///
/// This makes the iterator finite: [`next()`](Self::next) will return [`None`]
Expand Down Expand Up @@ -433,7 +439,10 @@ impl Raycaster {
let ff_ray = self.ray.advance(t_start);

let Some(cube) = Cube::containing(ff_ray.origin) else {
// Can't fast-forward if we would numeric overflow
// Can't fast-forward if we would numeric overflow.
// But in that case, also, we almost certainly can't feasibly
// raycast either.
*self = Self::new_empty();
return;
};

Expand Down Expand Up @@ -1091,6 +1100,11 @@ mod tests {
.within(bounds);
}

#[test]
fn no_steps() {
assert_no_steps(Raycaster::new_empty());
}

/// An example of an axis-aligned ray that wasn't working.
#[test]
fn regression_test_1() {
Expand Down

0 comments on commit ea71e10

Please sign in to comment.