From c620eb7833bbc1465bda201e1199ab41a664031d Mon Sep 17 00:00:00 2001 From: Chris Juchem Date: Tue, 3 Sep 2024 15:45:15 -0400 Subject: [PATCH] Return `Result`s from `Camera`'s world/viewport conversion methods (#14989) # Objective - Fixes https://github.com/bevyengine/bevy/issues/14593. ## Solution - Add `ViewportConversionError` and return it from viewport conversion methods on Camera. ## Testing - I successfully compiled and ran all changed examples. ## Migration Guide The following methods on `Camera` now return a `Result` instead of an `Option` so that they can provide more information about failures: - `world_to_viewport` - `world_to_viewport_with_depth` - `viewport_to_world` - `viewport_to_world_2d` Call `.ok()` on the `Result` to turn it back into an `Option`, or handle the `Result` directly. --------- Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com> Co-authored-by: Alice Cecile Co-authored-by: Zachary Harrold --- .../src/ui_debug_overlay/inset.rs | 2 +- crates/bevy_picking/src/backend.rs | 2 +- crates/bevy_render/src/camera/camera.rs | 134 ++++++++++++------ crates/bevy_sprite/src/picking_backend.rs | 2 +- crates/bevy_ui/src/accessibility.rs | 2 +- examples/2d/2d_viewport_to_world.rs | 2 +- examples/3d/3d_viewport_to_world.rs | 2 +- examples/3d/irradiance_volumes.rs | 2 +- examples/ecs/observers.rs | 2 +- examples/games/desk_toy.rs | 8 +- examples/math/cubic_splines.rs | 9 +- 11 files changed, 110 insertions(+), 57 deletions(-) diff --git a/crates/bevy_dev_tools/src/ui_debug_overlay/inset.rs b/crates/bevy_dev_tools/src/ui_debug_overlay/inset.rs index 86be2146c7..92522f6fbe 100644 --- a/crates/bevy_dev_tools/src/ui_debug_overlay/inset.rs +++ b/crates/bevy_dev_tools/src/ui_debug_overlay/inset.rs @@ -137,7 +137,7 @@ impl<'w, 's> InsetGizmo<'w, 's> { let Ok(cam) = self.cam.get_single() else { return Vec2::ZERO; }; - if let Some(new_position) = cam.world_to_viewport(&zero, position.extend(0.)) { + if let Ok(new_position) = cam.world_to_viewport(&zero, position.extend(0.)) { position = new_position; }; position.xy() diff --git a/crates/bevy_picking/src/backend.rs b/crates/bevy_picking/src/backend.rs index fb46ac7864..c573d0e4b9 100644 --- a/crates/bevy_picking/src/backend.rs +++ b/crates/bevy_picking/src/backend.rs @@ -231,6 +231,6 @@ pub mod ray { let viewport_logical = camera.to_logical(viewport.physical_position)?; viewport_pos -= viewport_logical; } - camera.viewport_to_world(camera_tfm, viewport_pos) + camera.viewport_to_world(camera_tfm, viewport_pos).ok() } } diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 78dbec4a37..426450be8a 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -187,6 +187,34 @@ impl Default for PhysicalCameraParameters { } } +/// Error returned when a conversion between world-space and viewport-space coordinates fails. +/// +/// See [`world_to_viewport`][Camera::world_to_viewport] and [`viewport_to_world`][Camera::viewport_to_world]. +#[derive(Debug, Eq, PartialEq, Copy, Clone)] +pub enum ViewportConversionError { + /// The pre-computed size of the viewport was not available. + /// + /// This may be because the `Camera` was just created and [`camera_system`] has not been executed + /// yet, or because the [`RenderTarget`] is misconfigured in one of the following ways: + /// - it references the [`PrimaryWindow`](RenderTarget::Window) when there is none, + /// - it references a [`Window`](RenderTarget::Window) entity that doesn't exist or doesn't actually have a `Window` component, + /// - it references an [`Image`](RenderTarget::Image) that doesn't exist (invalid handle), + /// - it references a [`TextureView`](RenderTarget::TextureView) that doesn't exist (invalid handle). + NoViewportSize, + /// The computed coordinate was beyond the `Camera`'s near plane. + /// + /// Only applicable when converting from world-space to viewport-space. + PastNearPlane, + /// The computed coordinate was beyond the `Camera`'s far plane. + /// + /// Only applicable when converting from world-space to viewport-space. + PastFarPlane, + /// The Normalized Device Coordinates could not be computed because the `camera_transform`, the + /// `world_position`, or the projection matrix defined by [`CameraProjection`] contained `NAN` + /// (see [`world_to_ndc`][Camera::world_to_ndc] and [`ndc_to_world`][Camera::ndc_to_world]). + InvalidData, +} + /// The defining [`Component`] for camera entities, /// storing information about how and what to render through this camera. /// @@ -348,29 +376,35 @@ impl Camera { /// To get the coordinates in Normalized Device Coordinates, you should use /// [`world_to_ndc`](Self::world_to_ndc). /// - /// Returns `None` if any of these conditions occur: - /// - The computed coordinates are beyond the near or far plane - /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) - /// - The world coordinates cannot be mapped to the Normalized Device Coordinates. See [`world_to_ndc`](Camera::world_to_ndc) - /// May also panic if `glam_assert` is enabled. See [`world_to_ndc`](Camera::world_to_ndc). + /// # Panics + /// + /// Will panic if `glam_assert` is enabled and the `camera_transform` contains `NAN` + /// (see [`world_to_ndc`][Self::world_to_ndc]). #[doc(alias = "world_to_screen")] pub fn world_to_viewport( &self, camera_transform: &GlobalTransform, world_position: Vec3, - ) -> Option { - let target_size = self.logical_viewport_size()?; - let ndc_space_coords = self.world_to_ndc(camera_transform, world_position)?; + ) -> Result { + let target_size = self + .logical_viewport_size() + .ok_or(ViewportConversionError::NoViewportSize)?; + let ndc_space_coords = self + .world_to_ndc(camera_transform, world_position) + .ok_or(ViewportConversionError::InvalidData)?; // NDC z-values outside of 0 < z < 1 are outside the (implicit) camera frustum and are thus not in viewport-space - if ndc_space_coords.z < 0.0 || ndc_space_coords.z > 1.0 { - return None; + if ndc_space_coords.z < 0.0 { + return Err(ViewportConversionError::PastNearPlane); + } + if ndc_space_coords.z > 1.0 { + return Err(ViewportConversionError::PastFarPlane); } // Once in NDC space, we can discard the z element and rescale x/y to fit the screen let mut viewport_position = (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * target_size; // Flip the Y co-ordinate origin from the bottom to the top. viewport_position.y = target_size.y - viewport_position.y; - Some(viewport_position) + Ok(viewport_position) } /// Given a position in world space, use the camera to compute the viewport-space coordinates and depth. @@ -378,22 +412,28 @@ impl Camera { /// To get the coordinates in Normalized Device Coordinates, you should use /// [`world_to_ndc`](Self::world_to_ndc). /// - /// Returns `None` if any of these conditions occur: - /// - The computed coordinates are beyond the near or far plane - /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) - /// - The world coordinates cannot be mapped to the Normalized Device Coordinates. See [`world_to_ndc`](Camera::world_to_ndc) - /// May also panic if `glam_assert` is enabled. See [`world_to_ndc`](Camera::world_to_ndc). + /// # Panics + /// + /// Will panic if `glam_assert` is enabled and the `camera_transform` contains `NAN` + /// (see [`world_to_ndc`][Self::world_to_ndc]). #[doc(alias = "world_to_screen_with_depth")] pub fn world_to_viewport_with_depth( &self, camera_transform: &GlobalTransform, world_position: Vec3, - ) -> Option { - let target_size = self.logical_viewport_size()?; - let ndc_space_coords = self.world_to_ndc(camera_transform, world_position)?; + ) -> Result { + let target_size = self + .logical_viewport_size() + .ok_or(ViewportConversionError::NoViewportSize)?; + let ndc_space_coords = self + .world_to_ndc(camera_transform, world_position) + .ok_or(ViewportConversionError::InvalidData)?; // NDC z-values outside of 0 < z < 1 are outside the (implicit) camera frustum and are thus not in viewport-space - if ndc_space_coords.z < 0.0 || ndc_space_coords.z > 1.0 { - return None; + if ndc_space_coords.z < 0.0 { + return Err(ViewportConversionError::PastNearPlane); + } + if ndc_space_coords.z > 1.0 { + return Err(ViewportConversionError::PastFarPlane); } // Stretching ndc depth to value via near plane and negating result to be in positive room again. @@ -403,7 +443,7 @@ impl Camera { let mut viewport_position = (ndc_space_coords.truncate() + Vec2::ONE) / 2.0 * target_size; // Flip the Y co-ordinate origin from the bottom to the top. viewport_position.y = target_size.y - viewport_position.y; - Some(viewport_position.extend(depth)) + Ok(viewport_position.extend(depth)) } /// Returns a ray originating from the camera, that passes through everything beyond `viewport_position`. @@ -415,16 +455,18 @@ impl Camera { /// To get the world space coordinates with Normalized Device Coordinates, you should use /// [`ndc_to_world`](Self::ndc_to_world). /// - /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) - /// - The near or far plane cannot be computed. This can happen if the `camera_transform`, the `world_position`, or the projection matrix defined by [`CameraProjection`] contain `NAN`. - /// Panics if the projection matrix is null and `glam_assert` is enabled. + /// # Panics + /// + /// Will panic if the camera's projection matrix is invalid (has a determinant of 0) and + /// `glam_assert` is enabled (see [`ndc_to_world`](Self::ndc_to_world). pub fn viewport_to_world( &self, camera_transform: &GlobalTransform, mut viewport_position: Vec2, - ) -> Option { - let target_size = self.logical_viewport_size()?; + ) -> Result { + let target_size = self + .logical_viewport_size() + .ok_or(ViewportConversionError::NoViewportSize)?; // Flip the Y co-ordinate origin from the top to the bottom. viewport_position.y = target_size.y - viewport_position.y; let ndc = viewport_position * 2. / target_size - Vec2::ONE; @@ -436,12 +478,12 @@ impl Camera { let world_far_plane = ndc_to_world.project_point3(ndc.extend(f32::EPSILON)); // The fallible direction constructor ensures that world_near_plane and world_far_plane aren't NaN. - Dir3::new(world_far_plane - world_near_plane).map_or(None, |direction| { - Some(Ray3d { + Dir3::new(world_far_plane - world_near_plane) + .map_err(|_| ViewportConversionError::InvalidData) + .map(|direction| Ray3d { origin: world_near_plane, direction, }) - }) } /// Returns a 2D world position computed from a position on this [`Camera`]'s viewport. @@ -451,23 +493,27 @@ impl Camera { /// To get the world space coordinates with Normalized Device Coordinates, you should use /// [`ndc_to_world`](Self::ndc_to_world). /// - /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) - /// - The viewport position cannot be mapped to the world. See [`ndc_to_world`](Camera::ndc_to_world) - /// May panic. See [`ndc_to_world`](Camera::ndc_to_world). + /// # Panics + /// + /// Will panic if the camera's projection matrix is invalid (has a determinant of 0) and + /// `glam_assert` is enabled (see [`ndc_to_world`](Self::ndc_to_world). pub fn viewport_to_world_2d( &self, camera_transform: &GlobalTransform, mut viewport_position: Vec2, - ) -> Option { - let target_size = self.logical_viewport_size()?; + ) -> Result { + let target_size = self + .logical_viewport_size() + .ok_or(ViewportConversionError::NoViewportSize)?; // Flip the Y co-ordinate origin from the top to the bottom. viewport_position.y = target_size.y - viewport_position.y; let ndc = viewport_position * 2. / target_size - Vec2::ONE; - let world_near_plane = self.ndc_to_world(camera_transform, ndc.extend(1.))?; + let world_near_plane = self + .ndc_to_world(camera_transform, ndc.extend(1.)) + .ok_or(ViewportConversionError::InvalidData)?; - Some(world_near_plane.truncate()) + Ok(world_near_plane.truncate()) } /// Given a position in world space, use the camera's viewport to compute the Normalized Device Coordinates. @@ -478,7 +524,10 @@ impl Camera { /// [`world_to_viewport`](Self::world_to_viewport). /// /// Returns `None` if the `camera_transform`, the `world_position`, or the projection matrix defined by [`CameraProjection`] contain `NAN`. - /// Panics if the `camera_transform` contains `NAN` and the `glam_assert` feature is enabled. + /// + /// # Panics + /// + /// Will panic if the `camera_transform` contains `NAN` and the `glam_assert` feature is enabled. pub fn world_to_ndc( &self, camera_transform: &GlobalTransform, @@ -501,7 +550,10 @@ impl Camera { /// [`world_to_viewport`](Self::world_to_viewport). /// /// Returns `None` if the `camera_transform`, the `world_position`, or the projection matrix defined by [`CameraProjection`] contain `NAN`. - /// Panics if the projection matrix is null and `glam_assert` is enabled. + /// + /// # Panics + /// + /// Will panic if the projection matrix is invalid (has a determinant of 0) and `glam_assert` is enabled. pub fn ndc_to_world(&self, camera_transform: &GlobalTransform, ndc: Vec3) -> Option { // Build a transformation matrix to convert from NDC to world space using camera data let ndc_to_world = diff --git a/crates/bevy_sprite/src/picking_backend.rs b/crates/bevy_sprite/src/picking_backend.rs index cc1728b151..f47ca22a5a 100644 --- a/crates/bevy_sprite/src/picking_backend.rs +++ b/crates/bevy_sprite/src/picking_backend.rs @@ -70,7 +70,7 @@ pub fn sprite_picking( continue; }; - let Some(cursor_ray_world) = camera.viewport_to_world(cam_transform, location.position) + let Ok(cursor_ray_world) = camera.viewport_to_world(cam_transform, location.position) else { continue; }; diff --git a/crates/bevy_ui/src/accessibility.rs b/crates/bevy_ui/src/accessibility.rs index 8ca46e124e..53cfb272b2 100644 --- a/crates/bevy_ui/src/accessibility.rs +++ b/crates/bevy_ui/src/accessibility.rs @@ -41,7 +41,7 @@ fn calc_bounds( if let Ok((camera, camera_transform)) = camera.get_single() { for (mut accessible, node, transform) in &mut nodes { if node.is_changed() || transform.is_changed() { - if let Some(translation) = + if let Ok(translation) = camera.world_to_viewport(camera_transform, transform.translation()) { let bounds = Rect::new( diff --git a/examples/2d/2d_viewport_to_world.rs b/examples/2d/2d_viewport_to_world.rs index c065d1c119..3913a5c5e9 100644 --- a/examples/2d/2d_viewport_to_world.rs +++ b/examples/2d/2d_viewport_to_world.rs @@ -26,7 +26,7 @@ fn draw_cursor( }; // Calculate a world position based on the cursor's position. - let Some(point) = camera.viewport_to_world_2d(camera_transform, cursor_position) else { + let Ok(point) = camera.viewport_to_world_2d(camera_transform, cursor_position) else { return; }; diff --git a/examples/3d/3d_viewport_to_world.rs b/examples/3d/3d_viewport_to_world.rs index 98c143c553..569a10a003 100644 --- a/examples/3d/3d_viewport_to_world.rs +++ b/examples/3d/3d_viewport_to_world.rs @@ -24,7 +24,7 @@ fn draw_cursor( }; // Calculate a ray pointing from the camera into the world based on the cursor's position. - let Some(ray) = camera.viewport_to_world(camera_transform, cursor_position) else { + let Ok(ray) = camera.viewport_to_world(camera_transform, cursor_position) else { return; }; diff --git a/examples/3d/irradiance_volumes.rs b/examples/3d/irradiance_volumes.rs index 0c4c44f00b..d52b8452a5 100644 --- a/examples/3d/irradiance_volumes.rs +++ b/examples/3d/irradiance_volumes.rs @@ -483,7 +483,7 @@ fn handle_mouse_clicks( }; // Figure out where the user clicked on the plane. - let Some(ray) = camera.viewport_to_world(camera_transform, mouse_position) else { + let Ok(ray) = camera.viewport_to_world(camera_transform, mouse_position) else { return; }; let Some(ray_distance) = ray.intersect_plane(Vec3::ZERO, InfinitePlane3d::new(Vec3::Y)) else { diff --git a/examples/ecs/observers.rs b/examples/ecs/observers.rs index 8820ce4919..82100f6b7e 100644 --- a/examples/ecs/observers.rs +++ b/examples/ecs/observers.rs @@ -184,7 +184,7 @@ fn handle_click( if let Some(pos) = windows .single() .cursor_position() - .and_then(|cursor| camera.viewport_to_world(camera_transform, cursor)) + .and_then(|cursor| camera.viewport_to_world(camera_transform, cursor).ok()) .map(|ray| ray.origin.truncate()) { if mouse_button_input.just_pressed(MouseButton::Left) { diff --git a/examples/games/desk_toy.rs b/examples/games/desk_toy.rs index 7da9859919..3147694176 100644 --- a/examples/games/desk_toy.rs +++ b/examples/games/desk_toy.rs @@ -222,9 +222,11 @@ fn get_cursor_world_pos( let primary_window = q_primary_window.single(); let (main_camera, main_camera_transform) = q_camera.single(); // Get the cursor position in the world - cursor_world_pos.0 = primary_window - .cursor_position() - .and_then(|cursor_pos| main_camera.viewport_to_world_2d(main_camera_transform, cursor_pos)); + cursor_world_pos.0 = primary_window.cursor_position().and_then(|cursor_pos| { + main_camera + .viewport_to_world_2d(main_camera_transform, cursor_pos) + .ok() + }); } /// Update whether the window is clickable or not diff --git a/examples/math/cubic_splines.rs b/examples/math/cubic_splines.rs index 37fab11ef6..4e4a86e60c 100644 --- a/examples/math/cubic_splines.rs +++ b/examples/math/cubic_splines.rs @@ -357,11 +357,10 @@ fn handle_mouse_press( }; // Convert the starting point and end point (current mouse pos) into world coords: - let Some(point) = camera.viewport_to_world_2d(camera_transform, start) else { + let Ok(point) = camera.viewport_to_world_2d(camera_transform, start) else { continue; }; - let Some(end_point) = camera.viewport_to_world_2d(camera_transform, mouse_pos) - else { + let Ok(end_point) = camera.viewport_to_world_2d(camera_transform, mouse_pos) else { continue; }; let tangent = end_point - point; @@ -396,10 +395,10 @@ fn draw_edit_move( // Resources store data in viewport coordinates, so we need to convert to world coordinates // to display them: - let Some(start) = camera.viewport_to_world_2d(camera_transform, start) else { + let Ok(start) = camera.viewport_to_world_2d(camera_transform, start) else { return; }; - let Some(end) = camera.viewport_to_world_2d(camera_transform, mouse_pos) else { + let Ok(end) = camera.viewport_to_world_2d(camera_transform, mouse_pos) else { return; };