From 2e35ff00a9f4408de9ac7f52bb967bb3ef8011bf Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Wed, 14 Jun 2023 19:28:01 +0200 Subject: [PATCH 1/8] Document when Camera::viewport_to_world and related methods return None --- crates/bevy_render/src/camera/camera.rs | 41 +++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 7651401970234..a282242e37384 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -169,6 +169,10 @@ impl Camera { /// of the current [`RenderTarget`]. /// For logic that requires the full logical size of the /// [`RenderTarget`], prefer [`Camera::logical_target_size`]. + /// + /// Returns None if both: + /// - The viewport is not set + /// - The render target is not set #[inline] pub fn logical_viewport_size(&self) -> Option { self.viewport @@ -218,6 +222,15 @@ 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 logical viewport size cannot be computed. This can happen if both: + /// - The viewport is not set + /// - The render target is not set + /// - The projection matrix is invalid + /// - The camera transform is invalid or cannot be inverted + /// - The world position is invalid + /// - The computed coordinates are beyond the near or far plane #[doc(alias = "world_to_screen")] pub fn world_to_viewport( &self, @@ -246,6 +259,15 @@ 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. This can happen if both: + /// - The viewport is not set + /// - The render target is not set + /// - The near or far plane cannot be computed. This can happen if: + /// - The projection matrix is invalid or cannot be inverted + /// - The camera transform is invalid + /// - The viewport position is invalid pub fn viewport_to_world( &self, camera_transform: &GlobalTransform, @@ -274,6 +296,15 @@ 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. This can happen if both: + /// - The viewport is not set + /// - The render target is not set + /// - The viewport position cannot be mapped to the world. This can happen if: + /// - The projection matrix is invalid or cannot be inverted + /// - The camera transform is invalid + /// - The viewport position is invalid pub fn viewport_to_world_2d( &self, camera_transform: &GlobalTransform, @@ -295,6 +326,11 @@ impl Camera { /// and between 0.0 and 1.0 on the Z axis. /// To get the coordinates in the render target's viewport dimensions, you should use /// [`world_to_viewport`](Self::world_to_viewport). + /// + /// Returns None if any of these conditions occur: + /// - The projection matrix is invalid + /// - The camera transform is invalid or cannot be inverted + /// - The world position is invalid pub fn world_to_ndc( &self, camera_transform: &GlobalTransform, @@ -315,6 +351,11 @@ impl Camera { /// and between 0.0 and 1.0 on the Z axis. /// To get the world space coordinates with the viewport position, you should use /// [`world_to_viewport`](Self::world_to_viewport). + /// + /// Returns None if any of these conditions occur: + /// - The projection matrix is invalid or cannot be inverted + /// - The camera transform is invalid + /// - The ndc is invalid 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 = From 0aa192b34794310f8e46f2e03b797bf7c4fd21e7 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Thu, 29 Jun 2023 03:03:51 +0200 Subject: [PATCH 2/8] fixup: backtick None in comments --- crates/bevy_render/src/camera/camera.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index a282242e37384..e07f990100d28 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -170,7 +170,7 @@ impl Camera { /// For logic that requires the full logical size of the /// [`RenderTarget`], prefer [`Camera::logical_target_size`]. /// - /// Returns None if both: + /// Returns `None` if both: /// - The viewport is not set /// - The render target is not set #[inline] @@ -223,7 +223,7 @@ 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: + /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set /// - The render target is not set @@ -260,7 +260,7 @@ 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: + /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set /// - The render target is not set @@ -297,7 +297,7 @@ 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: + /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set /// - The render target is not set @@ -327,7 +327,7 @@ impl Camera { /// To get the coordinates in the render target's viewport dimensions, you should use /// [`world_to_viewport`](Self::world_to_viewport). /// - /// Returns None if any of these conditions occur: + /// Returns `None` if any of these conditions occur: /// - The projection matrix is invalid /// - The camera transform is invalid or cannot be inverted /// - The world position is invalid @@ -352,7 +352,7 @@ impl Camera { /// To get the world space coordinates with the viewport position, you should use /// [`world_to_viewport`](Self::world_to_viewport). /// - /// Returns None if any of these conditions occur: + /// Returns `None` if any of these conditions occur: /// - The projection matrix is invalid or cannot be inverted /// - The camera transform is invalid /// - The ndc is invalid From 8fd32f324ecd6dd455480750b0eb9a6d93d43ba4 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Thu, 29 Jun 2023 03:39:37 +0200 Subject: [PATCH 3/8] fixup?: clarify render target comment --- crates/bevy_render/src/camera/camera.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index e07f990100d28..dce7a54e03cf4 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -172,7 +172,9 @@ impl Camera { /// /// Returns `None` if both: /// - The viewport is not set - /// - The render target is not set + /// - Bevy has not set the render target. This can happen if: + /// - The function is called just after the `Camera` is created, before `camera_system` is executed + /// - The target isn't correctly set (for example it points to an image that doesn't exist) #[inline] pub fn logical_viewport_size(&self) -> Option { self.viewport @@ -226,7 +228,9 @@ impl Camera { /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set - /// - The render target is not set + /// - Bevy has not set the render target. This can happen if: + /// - The function is called just after the `Camera` is created, before `camera_system` is executed + /// - The target isn't correctly set (for example it points to an image that doesn't exist) /// - The projection matrix is invalid /// - The camera transform is invalid or cannot be inverted /// - The world position is invalid @@ -263,7 +267,9 @@ impl Camera { /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set - /// - The render target is not set + /// - Bevy has not set the render target. This can happen if: + /// - The function is called just after the `Camera` is created, before `camera_system` is executed + /// - The target isn't correctly set (for example it points to an image that doesn't exist) /// - The near or far plane cannot be computed. This can happen if: /// - The projection matrix is invalid or cannot be inverted /// - The camera transform is invalid @@ -300,7 +306,9 @@ impl Camera { /// Returns `None` if any of these conditions occur: /// - The logical viewport size cannot be computed. This can happen if both: /// - The viewport is not set - /// - The render target is not set + /// - Bevy has not set the render target. This can happen if: + /// - The function is called just after the `Camera` is created, before `camera_system` is executed + /// - The target isn't correctly set (for example it points to an image that doesn't exist) /// - The viewport position cannot be mapped to the world. This can happen if: /// - The projection matrix is invalid or cannot be inverted /// - The camera transform is invalid From f7c871effc75ec5127fe69447df5eb3cf3f62e9c Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Sat, 1 Jul 2023 17:22:54 +0200 Subject: [PATCH 4/8] fixup: logical_viewport_size doc improvements --- crates/bevy_render/src/camera/camera.rs | 30 +++++++++---------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index dce7a54e03cf4..bd9f77799b3fc 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -170,11 +170,13 @@ impl Camera { /// For logic that requires the full logical size of the /// [`RenderTarget`], prefer [`Camera::logical_target_size`]. /// - /// Returns `None` if both: - /// - The viewport is not set - /// - Bevy has not set the render target. This can happen if: - /// - The function is called just after the `Camera` is created, before `camera_system` is executed - /// - The target isn't correctly set (for example it points to an image that doesn't exist) + /// Returns `None` if either: + /// - the function is called just after the `Camera` is created, before `camera_system` is executed, + /// - the [`RenderTarget`] isn't correctly set: + /// - 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). #[inline] pub fn logical_viewport_size(&self) -> Option { self.viewport @@ -226,11 +228,7 @@ impl Camera { /// [`world_to_ndc`](Self::world_to_ndc). /// /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. This can happen if both: - /// - The viewport is not set - /// - Bevy has not set the render target. This can happen if: - /// - The function is called just after the `Camera` is created, before `camera_system` is executed - /// - The target isn't correctly set (for example it points to an image that doesn't exist) + /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) /// - The projection matrix is invalid /// - The camera transform is invalid or cannot be inverted /// - The world position is invalid @@ -265,11 +263,7 @@ impl Camera { /// [`ndc_to_world`](Self::ndc_to_world). /// /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. This can happen if both: - /// - The viewport is not set - /// - Bevy has not set the render target. This can happen if: - /// - The function is called just after the `Camera` is created, before `camera_system` is executed - /// - The target isn't correctly set (for example it points to an image that doesn't exist) + /// - 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 projection matrix is invalid or cannot be inverted /// - The camera transform is invalid @@ -304,11 +298,7 @@ impl Camera { /// [`ndc_to_world`](Self::ndc_to_world). /// /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. This can happen if both: - /// - The viewport is not set - /// - Bevy has not set the render target. This can happen if: - /// - The function is called just after the `Camera` is created, before `camera_system` is executed - /// - The target isn't correctly set (for example it points to an image that doesn't exist) + /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) /// - The viewport position cannot be mapped to the world. This can happen if: /// - The projection matrix is invalid or cannot be inverted /// - The camera transform is invalid From b2a63bfa35426783cdc2b75cc464c6f3747c99c5 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Sat, 1 Jul 2023 18:07:42 +0200 Subject: [PATCH 5/8] fixup: world_to_ndc doc improvements --- crates/bevy_render/src/camera/camera.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index bd9f77799b3fc..d8cb5ae5de7bd 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -228,11 +228,10 @@ impl Camera { /// [`world_to_ndc`](Self::world_to_ndc). /// /// Returns `None` if any of these conditions occur: - /// - The logical viewport size cannot be computed. See [`logical_viewport_size`](Camera::logical_viewport_size) - /// - The projection matrix is invalid - /// - The camera transform is invalid or cannot be inverted - /// - The world position is invalid /// - 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). #[doc(alias = "world_to_screen")] pub fn world_to_viewport( &self, @@ -325,10 +324,8 @@ impl Camera { /// To get the coordinates in the render target's viewport dimensions, you should use /// [`world_to_viewport`](Self::world_to_viewport). /// - /// Returns `None` if any of these conditions occur: - /// - The projection matrix is invalid - /// - The camera transform is invalid or cannot be inverted - /// - The world position is invalid + /// 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. pub fn world_to_ndc( &self, camera_transform: &GlobalTransform, From 3b0002cc684b688b1bf06de8ac505afa0f9fa746 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Sat, 1 Jul 2023 19:57:24 +0200 Subject: [PATCH 6/8] ndc_to_world doc improvements. --- crates/bevy_render/src/camera/camera.rs | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index d8cb5ae5de7bd..f65efd95094e6 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -263,10 +263,8 @@ impl Camera { /// /// 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 projection matrix is invalid or cannot be inverted - /// - The camera transform is invalid - /// - The viewport position is invalid + /// - 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. pub fn viewport_to_world( &self, camera_transform: &GlobalTransform, @@ -298,10 +296,8 @@ impl Camera { /// /// 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. This can happen if: - /// - The projection matrix is invalid or cannot be inverted - /// - The camera transform is invalid - /// - The viewport position is invalid + /// - 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). pub fn viewport_to_world_2d( &self, camera_transform: &GlobalTransform, @@ -347,10 +343,8 @@ impl Camera { /// To get the world space coordinates with the viewport position, you should use /// [`world_to_viewport`](Self::world_to_viewport). /// - /// Returns `None` if any of these conditions occur: - /// - The projection matrix is invalid or cannot be inverted - /// - The camera transform is invalid - /// - The ndc is invalid + /// 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. 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 = From 9e065e7d73a7af555c4d0af1f3fbc3791073435c Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Sat, 1 Jul 2023 20:01:48 +0200 Subject: [PATCH 7/8] fixup: implicit frustum note --- crates/bevy_render/src/camera/camera.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index f65efd95094e6..2b15d67746644 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -240,7 +240,7 @@ impl Camera { ) -> Option { let target_size = self.logical_viewport_size()?; let ndc_space_coords = self.world_to_ndc(camera_transform, world_position)?; - // NDC z-values outside of 0 < z < 1 are outside the camera frustum and are thus not in viewport-space + // 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; } From 23996f09720d07611067821bf99e7ccbf794ca46 Mon Sep 17 00:00:00 2001 From: "Tormod G. Hellen" Date: Sat, 1 Jul 2023 20:06:20 +0200 Subject: [PATCH 8/8] Fixup: unbalanced backtics --- crates/bevy_render/src/camera/camera.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index 2b15d67746644..c85e8d68cfc9d 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -321,7 +321,7 @@ 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 if the `camera_transform` contains `NAN` and the `glam_assert` feature is enabled. pub fn world_to_ndc( &self, camera_transform: &GlobalTransform,