Skip to content

Commit

Permalink
Check for NaN in Camera::world_to_screen() (#3268)
Browse files Browse the repository at this point in the history
# Objective

- Checks for NaN in computed NDC space coordinates, fixing unexpected NaN in a fallible (`Option<T>`) function.

## Solution

- Adds a NaN check, in addition to the existing NDC bounds checks.
- This is a helper function, and should have no performance impact to the engine itself.
- This will help prevent hard-to-trace NaN propagation in user code, by returning `None` instead of `Some(NaN)`.


Depends on #3269 for CI error fix.
  • Loading branch information
aevyrie committed Dec 8, 2021
1 parent bf96f26 commit 38c7d5e
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
8 changes: 6 additions & 2 deletions crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,17 @@ impl Camera {
let world_to_ndc: Mat4 =
self.projection_matrix * camera_transform.compute_matrix().inverse();
let ndc_space_coords: Vec3 = world_to_ndc.project_point3(world_position);
// NDC z-values outside of 0 < z < 1 are behind the camera and are thus not in screen space
// NDC z-values outside of 0 < z < 1 are outside the camera frustum and are thus not in screen space
if ndc_space_coords.z < 0.0 || ndc_space_coords.z > 1.0 {
return None;
}
// Once in NDC space, we can discard the z element and rescale x/y to fit the screen
let screen_space_coords = (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * window_size;
Some(screen_space_coords)
if !screen_space_coords.is_nan() {
Some(screen_space_coords)
} else {
None
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions pipelined/bevy_render2/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,17 @@ impl Camera {
let world_to_ndc: Mat4 =
self.projection_matrix * camera_transform.compute_matrix().inverse();
let ndc_space_coords: Vec3 = world_to_ndc.project_point3(world_position);
// NDC z-values outside of 0 < z < 1 are behind the camera and are thus not in screen space
// NDC z-values outside of 0 < z < 1 are outside the camera frustum and are thus not in screen space
if ndc_space_coords.z < 0.0 || ndc_space_coords.z > 1.0 {
return None;
}
// Once in NDC space, we can discard the z element and rescale x/y to fit the screen
let screen_space_coords = (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * window_size;
Some(screen_space_coords)
if !screen_space_coords.is_nan() {
Some(screen_space_coords)
} else {
None
}
}
}

Expand Down

0 comments on commit 38c7d5e

Please sign in to comment.