From 5b5013d54035da09eebdd365b513486248f6726e Mon Sep 17 00:00:00 2001 From: Robin KAY Date: Tue, 5 Jul 2022 06:13:39 +0000 Subject: [PATCH] Add ViewRangefinder3d to reduce boilerplate when enqueuing standard 3D PhaseItems. (#5014) # Objective Reduce the boilerplate code needed to make draw order sorting work correctly when queuing items through new common functionality. Also fix several instances in the bevy code-base (mostly examples) where this boilerplate appears to be incorrect. ## Solution - Moved the logic for handling back-to-front vs front-to-back draw ordering into the PhaseItems by inverting the sort key ordering of Opaque3d and AlphaMask3d. The means that all the standard 3d rendering phases measure distance in the same way. Clients of these structs no longer need to know to negate the distance. - Added a new utility struct, ViewRangefinder3d, which encapsulates the maths needed to calculate a "distance" from an ExtractedView and a mesh's transform matrix. - Converted all the occurrences of the distance calculations in Bevy and its examples to use ViewRangefinder3d. Several of these occurrences appear to be buggy because they don't invert the view matrix or don't negate the distance where appropriate. This leads me to the view that Bevy should expose a facility to correctly perform this calculation. ## Migration Guide Code which creates Opaque3d, AlphaMask3d, or Transparent3d phase items _should_ use ViewRangefinder3d to calculate the distance value. Code which manually calculated the distance for Opaque3d or AlphaMask3d phase items and correctly negated the z value will no longer depth sort correctly. However, incorrect depth sorting for these types will not impact the rendered output as sorting is only a performance optimisation when drawing with depth-testing enabled. Code which manually calculated the distance for Transparent3d phase items will continue to work as before. --- crates/bevy_core_pipeline/src/core_3d/mod.rs | 19 ++++++--- crates/bevy_pbr/src/material.rs | 25 +++--------- crates/bevy_pbr/src/wireframe.rs | 5 +-- crates/bevy_render/src/lib.rs | 1 + crates/bevy_render/src/rangefinder.rs | 41 ++++++++++++++++++++ crates/bevy_render/src/view/mod.rs | 8 ++++ examples/shader/animate_shader.rs | 5 +-- examples/shader/shader_instancing.rs | 5 +-- 8 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 crates/bevy_render/src/rangefinder.rs diff --git a/crates/bevy_core_pipeline/src/core_3d/mod.rs b/crates/bevy_core_pipeline/src/core_3d/mod.rs index e3b230a1e7..c49c8634b3 100644 --- a/crates/bevy_core_pipeline/src/core_3d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_3d/mod.rs @@ -11,6 +11,8 @@ pub mod graph { } } +use std::cmp::Reverse; + pub use camera_3d::*; pub use main_pass_3d_node::*; @@ -87,11 +89,12 @@ pub struct Opaque3d { } impl PhaseItem for Opaque3d { - type SortKey = FloatOrd; + // NOTE: Values increase towards the camera. Front-to-back ordering for opaque means we need a descending sort. + type SortKey = Reverse; #[inline] fn sort_key(&self) -> Self::SortKey { - FloatOrd(self.distance) + Reverse(FloatOrd(self.distance)) } #[inline] @@ -101,7 +104,8 @@ impl PhaseItem for Opaque3d { #[inline] fn sort(items: &mut [Self]) { - radsort::sort_by_key(items, |item| item.distance); + // Key negated to match reversed SortKey ordering + radsort::sort_by_key(items, |item| -item.distance); } } @@ -127,11 +131,12 @@ pub struct AlphaMask3d { } impl PhaseItem for AlphaMask3d { - type SortKey = FloatOrd; + // NOTE: Values increase towards the camera. Front-to-back ordering for alpha mask means we need a descending sort. + type SortKey = Reverse; #[inline] fn sort_key(&self) -> Self::SortKey { - FloatOrd(self.distance) + Reverse(FloatOrd(self.distance)) } #[inline] @@ -141,7 +146,8 @@ impl PhaseItem for AlphaMask3d { #[inline] fn sort(items: &mut [Self]) { - radsort::sort_by_key(items, |item| item.distance); + // Key negated to match reversed SortKey ordering + radsort::sort_by_key(items, |item| -item.distance); } } @@ -167,6 +173,7 @@ pub struct Transparent3d { } impl PhaseItem for Transparent3d { + // NOTE: Values increase towards the camera. Back-to-front ordering for transparent means we need an ascending sort. type SortKey = FloatOrd; #[inline] diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index a21aed617b..b2581a9b2a 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -348,8 +348,7 @@ pub fn queue_material_meshes( .get_id::>() .unwrap(); - let inverse_view_matrix = view.transform.compute_matrix().inverse(); - let inverse_view_row_2 = inverse_view_matrix.row(2); + let rangefinder = view.rangefinder3d(); let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples); for visible_entity in &visible_entities.entities { @@ -383,9 +382,7 @@ pub fn queue_material_meshes( } }; - // 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 - let mesh_z = inverse_view_row_2.dot(mesh_uniform.transform.col(3)) + let distance = rangefinder.distance(&mesh_uniform.transform) + material.properties.depth_bias; match alpha_mode { AlphaMode::Opaque => { @@ -393,11 +390,7 @@ pub fn queue_material_meshes( entity: *visible_entity, draw_function: draw_opaque_pbr, pipeline: pipeline_id, - // NOTE: Front-to-back ordering for opaque with ascending sort means near should have the - // lowest sort key and getting further away should increase. As we have - // -z in front of the camera, values in view space decrease away from the - // camera. Flipping the sign of mesh_z results in the correct front-to-back ordering - distance: -mesh_z, + distance, }); } AlphaMode::Mask(_) => { @@ -405,11 +398,7 @@ pub fn queue_material_meshes( entity: *visible_entity, draw_function: draw_alpha_mask_pbr, pipeline: pipeline_id, - // NOTE: Front-to-back ordering for alpha mask with ascending sort means near should have the - // lowest sort key and getting further away should increase. As we have - // -z in front of the camera, values in view space decrease away from the - // camera. Flipping the sign of mesh_z results in the correct front-to-back ordering - distance: -mesh_z, + distance, }); } AlphaMode::Blend => { @@ -417,11 +406,7 @@ pub fn queue_material_meshes( entity: *visible_entity, draw_function: draw_transparent_pbr, pipeline: pipeline_id, - // NOTE: Back-to-front ordering for transparent with ascending sort means far should have the - // lowest sort key and getting closer should increase. As we have - // -z in front of the camera, the largest distance is -far with values increasing toward the - // camera. As such we can just use mesh_z as the distance - distance: mesh_z, + distance, }); } } diff --git a/crates/bevy_pbr/src/wireframe.rs b/crates/bevy_pbr/src/wireframe.rs index c16e396f8e..6698959b93 100644 --- a/crates/bevy_pbr/src/wireframe.rs +++ b/crates/bevy_pbr/src/wireframe.rs @@ -119,8 +119,7 @@ fn queue_wireframes( .unwrap(); let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples); for (view, visible_entities, mut opaque_phase) in views.iter_mut() { - let view_matrix = view.transform.compute_matrix(); - let view_row_2 = view_matrix.row(2); + let rangefinder = view.rangefinder3d(); let add_render_phase = |(entity, mesh_handle, mesh_uniform): (Entity, &Handle, &MeshUniform)| { @@ -144,7 +143,7 @@ fn queue_wireframes( entity, pipeline: pipeline_id, draw_function: draw_custom, - distance: view_row_2.dot(mesh_uniform.transform.col(3)), + distance: rangefinder.distance(&mesh_uniform.transform), }); } }; diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index 0caa1a75bc..b69eb85c35 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -6,6 +6,7 @@ pub mod extract_component; pub mod extract_resource; pub mod mesh; pub mod primitives; +pub mod rangefinder; pub mod render_asset; pub mod render_graph; pub mod render_phase; diff --git a/crates/bevy_render/src/rangefinder.rs b/crates/bevy_render/src/rangefinder.rs new file mode 100644 index 0000000000..c11b8e679d --- /dev/null +++ b/crates/bevy_render/src/rangefinder.rs @@ -0,0 +1,41 @@ +use bevy_math::{Mat4, Vec4}; + +/// A distance calculator for the draw order of [`PhaseItem`](crate::render_phase::PhaseItem)s. +pub struct ViewRangefinder3d { + inverse_view_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(); + ViewRangefinder3d { + inverse_view_row_2: inverse_view_matrix.row(2), + } + } + + /// Calculates the distance, or view-space Z value, for a 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)) + } +} + +#[cfg(test)] +mod tests { + use super::ViewRangefinder3d; + use bevy_math::{Mat4, Vec3}; + + #[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); + assert_eq!(rangefinder.distance(&Mat4::IDENTITY), 1.0); + assert_eq!( + rangefinder.distance(&Mat4::from_translation(Vec3::new(0.0, 0.0, 1.0))), + 2.0 + ); + } +} diff --git a/crates/bevy_render/src/view/mod.rs b/crates/bevy_render/src/view/mod.rs index e1e3f311fe..155307475a 100644 --- a/crates/bevy_render/src/view/mod.rs +++ b/crates/bevy_render/src/view/mod.rs @@ -12,6 +12,7 @@ use crate::{ camera::ExtractedCamera, extract_resource::{ExtractResource, ExtractResourcePlugin}, prelude::Image, + rangefinder::ViewRangefinder3d, render_asset::RenderAssets, render_resource::{DynamicUniformBuffer, ShaderType, Texture, TextureView}, renderer::{RenderDevice, RenderQueue}, @@ -84,6 +85,13 @@ pub struct ExtractedView { pub height: u32, } +impl ExtractedView { + /// Creates a 3D rangefinder for a view + pub fn rangefinder3d(&self) -> ViewRangefinder3d { + ViewRangefinder3d::from_view_matrix(&self.transform.compute_matrix()) + } +} + #[derive(Clone, ShaderType)] pub struct ViewUniform { view_proj: Mat4, diff --git a/examples/shader/animate_shader.rs b/examples/shader/animate_shader.rs index ef0647cc31..d06e0f43f6 100644 --- a/examples/shader/animate_shader.rs +++ b/examples/shader/animate_shader.rs @@ -116,8 +116,7 @@ fn queue_custom( | MeshPipelineKey::from_primitive_topology(PrimitiveTopology::TriangleList); for (view, mut transparent_phase) in views.iter_mut() { - let view_matrix = view.transform.compute_matrix(); - let view_row_2 = view_matrix.row(2); + let rangefinder = view.rangefinder3d(); for (entity, mesh_uniform, mesh_handle) in material_meshes.iter() { if let Some(mesh) = render_meshes.get(mesh_handle) { let pipeline = pipelines @@ -127,7 +126,7 @@ fn queue_custom( entity, pipeline, draw_function: draw_custom, - distance: view_row_2.dot(mesh_uniform.transform.col(3)), + distance: rangefinder.distance(&mesh_uniform.transform), }); } } diff --git a/examples/shader/shader_instancing.rs b/examples/shader/shader_instancing.rs index 6bf6858678..e497920629 100644 --- a/examples/shader/shader_instancing.rs +++ b/examples/shader/shader_instancing.rs @@ -117,8 +117,7 @@ fn queue_custom( let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples); for (view, mut transparent_phase) in views.iter_mut() { - let view_matrix = view.transform.compute_matrix(); - let view_row_2 = view_matrix.row(2); + let rangefinder = view.rangefinder3d(); for (entity, mesh_uniform, mesh_handle) in material_meshes.iter() { if let Some(mesh) = meshes.get(mesh_handle) { let key = @@ -130,7 +129,7 @@ fn queue_custom( entity, pipeline, draw_function: draw_custom, - distance: view_row_2.dot(mesh_uniform.transform.col(3)), + distance: rangefinder.distance(&mesh_uniform.transform), }); } }