From 940fcc2a6b91c68e35735fec4ef60851a1fd6b23 Mon Sep 17 00:00:00 2001 From: floppyhammer Date: Tue, 20 Feb 2024 17:01:54 +0800 Subject: [PATCH 1/4] Fix incorrect distance given by ViewRangefinder3d --- .../src/render_phase/rangefinder.rs | 26 +++++++++---------- crates/bevy_render/src/view/mod.rs | 7 ++++- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/crates/bevy_render/src/render_phase/rangefinder.rs b/crates/bevy_render/src/render_phase/rangefinder.rs index 40e75183c8152..5eed66bddf6d1 100644 --- a/crates/bevy_render/src/render_phase/rangefinder.rs +++ b/crates/bevy_render/src/render_phase/rangefinder.rs @@ -2,33 +2,33 @@ use bevy_math::{Mat4, Vec3, Vec4}; /// A distance calculator for the draw order of [`PhaseItem`](crate::render_phase::PhaseItem)s. pub struct ViewRangefinder3d { - inverse_view_row_2: Vec4, + inverse_view_proj_row_2: Vec4, } impl ViewRangefinder3d { - /// Creates a 3D rangefinder for a view matrix. - pub fn from_view_matrix(view_matrix: &Mat4) -> ViewRangefinder3d { - let inverse_view_matrix = view_matrix.inverse(); + /// Creates a 3D rangefinder for a view-projection matrix. + pub fn from_view_proj_matrix(view_proj_matrix: &Mat4) -> ViewRangefinder3d { + let inverse_view_proj_matrix = view_proj_matrix.inverse(); ViewRangefinder3d { - inverse_view_row_2: inverse_view_matrix.row(2), + inverse_view_proj_row_2: inverse_view_proj_matrix.row(2), } } - /// Calculates the distance, or view-space `Z` value, for the given `translation`. + /// Calculates the distance for the given `translation`. #[inline] pub fn distance_translation(&self, translation: &Vec3) -> f32 { - // NOTE: row 2 of the inverse view matrix dotted with the translation from the model matrix - // gives the z component of translation of the mesh in view-space - self.inverse_view_row_2.dot(translation.extend(1.0)) + // NOTE: row 2 of the inverse view-projection matrix dotted with the translation from the model matrix + // gives the z component of translation of the mesh in clip-space + self.inverse_view_proj_row_2.dot(translation.extend(1.0)) } - /// Calculates the distance, or view-space `Z` value, for the given `transform`. + /// Calculates the distance for the given `transform`. #[inline] pub fn distance(&self, transform: &Mat4) -> f32 { - // NOTE: row 2 of the inverse view matrix dotted with column 3 of the model matrix - // gives the z component of translation of the mesh in view-space - self.inverse_view_row_2.dot(transform.col(3)) + // NOTE: row 2 of the inverse view-projection matrix dotted with column 3 of the model matrix + // gives the z component of translation of the mesh in clip-space + self.inverse_view_proj_row_2.dot(transform.col(3)) } } diff --git a/crates/bevy_render/src/view/mod.rs b/crates/bevy_render/src/view/mod.rs index b30ea9af30616..16ee929c41d16 100644 --- a/crates/bevy_render/src/view/mod.rs +++ b/crates/bevy_render/src/view/mod.rs @@ -123,7 +123,12 @@ pub struct ExtractedView { impl ExtractedView { /// Creates a 3D rangefinder for a view pub fn rangefinder3d(&self) -> ViewRangefinder3d { - ViewRangefinder3d::from_view_matrix(&self.transform.compute_matrix()) + if let Some(vp) = &self.view_projection { + ViewRangefinder3d::from_view_proj_matrix(vp) + } else { + let vp = self.projection * self.transform.compute_matrix().inverse(); + ViewRangefinder3d::from_view_proj_matrix(&vp) + } } } From 828781bfdee511e938a906bc0a5da70975e8c099 Mon Sep 17 00:00:00 2001 From: floppyhammer Date: Tue, 20 Feb 2024 17:15:29 +0800 Subject: [PATCH 2/4] Fix test --- crates/bevy_render/src/render_phase/rangefinder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/bevy_render/src/render_phase/rangefinder.rs b/crates/bevy_render/src/render_phase/rangefinder.rs index 5eed66bddf6d1..bf4b4802b991d 100644 --- a/crates/bevy_render/src/render_phase/rangefinder.rs +++ b/crates/bevy_render/src/render_phase/rangefinder.rs @@ -39,8 +39,8 @@ mod tests { #[test] fn distance() { - let view_matrix = Mat4::from_translation(Vec3::new(0.0, 0.0, -1.0)); - let rangefinder = ViewRangefinder3d::from_view_matrix(&view_matrix); + let view_proj_matrix = Mat4::from_translation(Vec3::new(0.0, 0.0, -1.0)); + let rangefinder = ViewRangefinder3d::from_view_proj_matrix(&view_proj_matrix); assert_eq!(rangefinder.distance(&Mat4::IDENTITY), 1.0); assert_eq!( rangefinder.distance(&Mat4::from_translation(Vec3::new(0.0, 0.0, 1.0))), From ed303b102bcf5d0e3308f1996e655ea690c64f49 Mon Sep 17 00:00:00 2001 From: floppyhammer Date: Mon, 11 Mar 2024 18:10:49 +0800 Subject: [PATCH 3/4] Fix incorrect view_proj_matrix --- .../src/render_phase/rangefinder.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/bevy_render/src/render_phase/rangefinder.rs b/crates/bevy_render/src/render_phase/rangefinder.rs index bf4b4802b991d..19b035787894b 100644 --- a/crates/bevy_render/src/render_phase/rangefinder.rs +++ b/crates/bevy_render/src/render_phase/rangefinder.rs @@ -1,34 +1,34 @@ use bevy_math::{Mat4, Vec3, Vec4}; -/// A distance calculator for the draw order of [`PhaseItem`](crate::render_phase::PhaseItem)s. +/// A depth calculator for the draw order of [`PhaseItem`](crate::render_phase::PhaseItem)s. pub struct ViewRangefinder3d { - inverse_view_proj_row_2: Vec4, + view_proj_row_2: Vec4, + view_proj_row_3: Vec4, } impl ViewRangefinder3d { /// Creates a 3D rangefinder for a view-projection matrix. pub fn from_view_proj_matrix(view_proj_matrix: &Mat4) -> ViewRangefinder3d { - let inverse_view_proj_matrix = view_proj_matrix.inverse(); - ViewRangefinder3d { - inverse_view_proj_row_2: inverse_view_proj_matrix.row(2), + view_proj_row_2: view_proj_matrix.row(2), + view_proj_row_3: view_proj_matrix.row(3), } } - /// Calculates the distance for the given `translation`. + /// Calculates the depth for the given `translation`. #[inline] pub fn distance_translation(&self, translation: &Vec3) -> f32 { - // NOTE: row 2 of the inverse view-projection matrix dotted with the translation from the model matrix + // NOTE: row 2 of the view-projection matrix dotted with the translation from the model matrix // gives the z component of translation of the mesh in clip-space - self.inverse_view_proj_row_2.dot(translation.extend(1.0)) + self.view_proj_row_2.dot(translation.extend(1.0)) / self.view_proj_row_3.dot(translation.extend(1.0)) } - /// Calculates the distance for the given `transform`. + /// Calculates the depth for the given `transform`. #[inline] pub fn distance(&self, transform: &Mat4) -> f32 { - // NOTE: row 2 of the inverse view-projection matrix dotted with column 3 of the model matrix + // NOTE: row 2 of the view-projection matrix dotted with column 3 of the model matrix // gives the z component of translation of the mesh in clip-space - self.inverse_view_proj_row_2.dot(transform.col(3)) + self.view_proj_row_2.dot(transform.col(3)) / self.view_proj_row_3.dot(transform.col(3)) } } From 860321d26877295f9b831905785414552863d4b2 Mon Sep 17 00:00:00 2001 From: floppyhammer Date: Thu, 14 Mar 2024 16:13:23 +0800 Subject: [PATCH 4/4] Tidy up code --- crates/bevy_render/src/render_phase/rangefinder.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/bevy_render/src/render_phase/rangefinder.rs b/crates/bevy_render/src/render_phase/rangefinder.rs index 19b035787894b..1afd30e85fcd7 100644 --- a/crates/bevy_render/src/render_phase/rangefinder.rs +++ b/crates/bevy_render/src/render_phase/rangefinder.rs @@ -19,16 +19,18 @@ impl ViewRangefinder3d { #[inline] pub fn distance_translation(&self, translation: &Vec3) -> f32 { // NOTE: row 2 of the view-projection matrix dotted with the translation from the model matrix - // gives the z component of translation of the mesh in clip-space - self.view_proj_row_2.dot(translation.extend(1.0)) / self.view_proj_row_3.dot(translation.extend(1.0)) + // gives the z component of translation of the mesh in clip-space. + // However, as we are using an infinite projection matrix for perspective projection, + // this value is meaningless. + let position = translation.extend(1.0); + self.view_proj_row_2.dot(position) / self.view_proj_row_3.dot(position) } /// Calculates the depth for the given `transform`. #[inline] pub fn distance(&self, transform: &Mat4) -> f32 { - // NOTE: row 2 of the view-projection matrix dotted with column 3 of the model matrix - // gives the z component of translation of the mesh in clip-space - self.view_proj_row_2.dot(transform.col(3)) / self.view_proj_row_3.dot(transform.col(3)) + let position = transform.col(3); + self.view_proj_row_2.dot(position) / self.view_proj_row_3.dot(position) } }