Skip to content

Commit

Permalink
Add ViewRangefinder3d to reduce boilerplate when enqueuing standard 3…
Browse files Browse the repository at this point in the history
…D PhaseItems. (bevyengine#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.
  • Loading branch information
komadori authored and ItsDoot committed Feb 1, 2023
1 parent dd0b83a commit e72d8b7
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 35 deletions.
19 changes: 13 additions & 6 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ pub mod graph {
}
}

use std::cmp::Reverse;

pub use camera_3d::*;
pub use main_pass_3d_node::*;

Expand Down Expand Up @@ -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<FloatOrd>;

#[inline]
fn sort_key(&self) -> Self::SortKey {
FloatOrd(self.distance)
Reverse(FloatOrd(self.distance))
}

#[inline]
Expand All @@ -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);
}
}

Expand All @@ -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<FloatOrd>;

#[inline]
fn sort_key(&self) -> Self::SortKey {
FloatOrd(self.distance)
Reverse(FloatOrd(self.distance))
}

#[inline]
Expand All @@ -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);
}
}

Expand All @@ -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]
Expand Down
25 changes: 5 additions & 20 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,7 @@ pub fn queue_material_meshes<M: Material>(
.get_id::<DrawMaterial<M>>()
.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 {
Expand Down Expand Up @@ -383,45 +382,31 @@ pub fn queue_material_meshes<M: Material>(
}
};

// 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 => {
opaque_phase.add(Opaque3d {
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(_) => {
alpha_mask_phase.add(AlphaMask3d {
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 => {
transparent_phase.add(Transparent3d {
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,
});
}
}
Expand Down
5 changes: 2 additions & 3 deletions crates/bevy_pbr/src/wireframe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Mesh>, &MeshUniform)| {
Expand All @@ -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),
});
}
};
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 41 additions & 0 deletions crates/bevy_render/src/rangefinder.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
}
8 changes: 8 additions & 0 deletions crates/bevy_render/src/view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 2 additions & 3 deletions examples/shader/animate_shader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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),
});
}
}
Expand Down
5 changes: 2 additions & 3 deletions examples/shader/shader_instancing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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),
});
}
}
Expand Down

0 comments on commit e72d8b7

Please sign in to comment.