Skip to content

Commit b2b33f5

Browse files
komadorijames7132
authored andcommitted
Add ViewRangefinder3d to reduce boilerplate when enqueuing standard 3D 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.
1 parent 2e24a6a commit b2b33f5

File tree

8 files changed

+74
-35
lines changed

8 files changed

+74
-35
lines changed

crates/bevy_core_pipeline/src/core_3d/mod.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ pub mod graph {
1111
}
1212
}
1313

14+
use std::cmp::Reverse;
15+
1416
pub use camera_3d::*;
1517
pub use main_pass_3d_node::*;
1618

@@ -87,11 +89,12 @@ pub struct Opaque3d {
8789
}
8890

8991
impl PhaseItem for Opaque3d {
90-
type SortKey = FloatOrd;
92+
// NOTE: Values increase towards the camera. Front-to-back ordering for opaque means we need a descending sort.
93+
type SortKey = Reverse<FloatOrd>;
9194

9295
#[inline]
9396
fn sort_key(&self) -> Self::SortKey {
94-
FloatOrd(self.distance)
97+
Reverse(FloatOrd(self.distance))
9598
}
9699

97100
#[inline]
@@ -101,7 +104,8 @@ impl PhaseItem for Opaque3d {
101104

102105
#[inline]
103106
fn sort(items: &mut [Self]) {
104-
radsort::sort_by_key(items, |item| item.distance);
107+
// Key negated to match reversed SortKey ordering
108+
radsort::sort_by_key(items, |item| -item.distance);
105109
}
106110
}
107111

@@ -127,11 +131,12 @@ pub struct AlphaMask3d {
127131
}
128132

129133
impl PhaseItem for AlphaMask3d {
130-
type SortKey = FloatOrd;
134+
// NOTE: Values increase towards the camera. Front-to-back ordering for alpha mask means we need a descending sort.
135+
type SortKey = Reverse<FloatOrd>;
131136

132137
#[inline]
133138
fn sort_key(&self) -> Self::SortKey {
134-
FloatOrd(self.distance)
139+
Reverse(FloatOrd(self.distance))
135140
}
136141

137142
#[inline]
@@ -141,7 +146,8 @@ impl PhaseItem for AlphaMask3d {
141146

142147
#[inline]
143148
fn sort(items: &mut [Self]) {
144-
radsort::sort_by_key(items, |item| item.distance);
149+
// Key negated to match reversed SortKey ordering
150+
radsort::sort_by_key(items, |item| -item.distance);
145151
}
146152
}
147153

@@ -167,6 +173,7 @@ pub struct Transparent3d {
167173
}
168174

169175
impl PhaseItem for Transparent3d {
176+
// NOTE: Values increase towards the camera. Back-to-front ordering for transparent means we need an ascending sort.
170177
type SortKey = FloatOrd;
171178

172179
#[inline]

crates/bevy_pbr/src/material.rs

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,7 @@ pub fn queue_material_meshes<M: Material>(
348348
.get_id::<DrawMaterial<M>>()
349349
.unwrap();
350350

351-
let inverse_view_matrix = view.transform.compute_matrix().inverse();
352-
let inverse_view_row_2 = inverse_view_matrix.row(2);
351+
let rangefinder = view.rangefinder3d();
353352
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);
354353

355354
for visible_entity in &visible_entities.entities {
@@ -383,45 +382,31 @@ pub fn queue_material_meshes<M: Material>(
383382
}
384383
};
385384

386-
// NOTE: row 2 of the inverse view matrix dotted with column 3 of the model matrix
387-
// gives the z component of translation of the mesh in view space
388-
let mesh_z = inverse_view_row_2.dot(mesh_uniform.transform.col(3))
385+
let distance = rangefinder.distance(&mesh_uniform.transform)
389386
+ material.properties.depth_bias;
390387
match alpha_mode {
391388
AlphaMode::Opaque => {
392389
opaque_phase.add(Opaque3d {
393390
entity: *visible_entity,
394391
draw_function: draw_opaque_pbr,
395392
pipeline: pipeline_id,
396-
// NOTE: Front-to-back ordering for opaque with ascending sort means near should have the
397-
// lowest sort key and getting further away should increase. As we have
398-
// -z in front of the camera, values in view space decrease away from the
399-
// camera. Flipping the sign of mesh_z results in the correct front-to-back ordering
400-
distance: -mesh_z,
393+
distance,
401394
});
402395
}
403396
AlphaMode::Mask(_) => {
404397
alpha_mask_phase.add(AlphaMask3d {
405398
entity: *visible_entity,
406399
draw_function: draw_alpha_mask_pbr,
407400
pipeline: pipeline_id,
408-
// NOTE: Front-to-back ordering for alpha mask with ascending sort means near should have the
409-
// lowest sort key and getting further away should increase. As we have
410-
// -z in front of the camera, values in view space decrease away from the
411-
// camera. Flipping the sign of mesh_z results in the correct front-to-back ordering
412-
distance: -mesh_z,
401+
distance,
413402
});
414403
}
415404
AlphaMode::Blend => {
416405
transparent_phase.add(Transparent3d {
417406
entity: *visible_entity,
418407
draw_function: draw_transparent_pbr,
419408
pipeline: pipeline_id,
420-
// NOTE: Back-to-front ordering for transparent with ascending sort means far should have the
421-
// lowest sort key and getting closer should increase. As we have
422-
// -z in front of the camera, the largest distance is -far with values increasing toward the
423-
// camera. As such we can just use mesh_z as the distance
424-
distance: mesh_z,
409+
distance,
425410
});
426411
}
427412
}

crates/bevy_pbr/src/wireframe.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,7 @@ fn queue_wireframes(
119119
.unwrap();
120120
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);
121121
for (view, visible_entities, mut opaque_phase) in views.iter_mut() {
122-
let view_matrix = view.transform.compute_matrix();
123-
let view_row_2 = view_matrix.row(2);
122+
let rangefinder = view.rangefinder3d();
124123

125124
let add_render_phase =
126125
|(entity, mesh_handle, mesh_uniform): (Entity, &Handle<Mesh>, &MeshUniform)| {
@@ -144,7 +143,7 @@ fn queue_wireframes(
144143
entity,
145144
pipeline: pipeline_id,
146145
draw_function: draw_custom,
147-
distance: view_row_2.dot(mesh_uniform.transform.col(3)),
146+
distance: rangefinder.distance(&mesh_uniform.transform),
148147
});
149148
}
150149
};

crates/bevy_render/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ pub mod extract_component;
66
pub mod extract_resource;
77
pub mod mesh;
88
pub mod primitives;
9+
pub mod rangefinder;
910
pub mod render_asset;
1011
pub mod render_graph;
1112
pub mod render_phase;

crates/bevy_render/src/rangefinder.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
use bevy_math::{Mat4, Vec4};
2+
3+
/// A distance calculator for the draw order of [`PhaseItem`](crate::render_phase::PhaseItem)s.
4+
pub struct ViewRangefinder3d {
5+
inverse_view_row_2: Vec4,
6+
}
7+
8+
impl ViewRangefinder3d {
9+
/// Creates a 3D rangefinder for a view matrix
10+
pub fn from_view_matrix(view_matrix: &Mat4) -> ViewRangefinder3d {
11+
let inverse_view_matrix = view_matrix.inverse();
12+
ViewRangefinder3d {
13+
inverse_view_row_2: inverse_view_matrix.row(2),
14+
}
15+
}
16+
17+
/// Calculates the distance, or view-space Z value, for a transform
18+
#[inline]
19+
pub fn distance(&self, transform: &Mat4) -> f32 {
20+
// NOTE: row 2 of the inverse view matrix dotted with column 3 of the model matrix
21+
// gives the z component of translation of the mesh in view-space
22+
self.inverse_view_row_2.dot(transform.col(3))
23+
}
24+
}
25+
26+
#[cfg(test)]
27+
mod tests {
28+
use super::ViewRangefinder3d;
29+
use bevy_math::{Mat4, Vec3};
30+
31+
#[test]
32+
fn distance() {
33+
let view_matrix = Mat4::from_translation(Vec3::new(0.0, 0.0, -1.0));
34+
let rangefinder = ViewRangefinder3d::from_view_matrix(&view_matrix);
35+
assert_eq!(rangefinder.distance(&Mat4::IDENTITY), 1.0);
36+
assert_eq!(
37+
rangefinder.distance(&Mat4::from_translation(Vec3::new(0.0, 0.0, 1.0))),
38+
2.0
39+
);
40+
}
41+
}

crates/bevy_render/src/view/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use crate::{
1212
camera::ExtractedCamera,
1313
extract_resource::{ExtractResource, ExtractResourcePlugin},
1414
prelude::Image,
15+
rangefinder::ViewRangefinder3d,
1516
render_asset::RenderAssets,
1617
render_resource::{DynamicUniformBuffer, ShaderType, Texture, TextureView},
1718
renderer::{RenderDevice, RenderQueue},
@@ -84,6 +85,13 @@ pub struct ExtractedView {
8485
pub height: u32,
8586
}
8687

88+
impl ExtractedView {
89+
/// Creates a 3D rangefinder for a view
90+
pub fn rangefinder3d(&self) -> ViewRangefinder3d {
91+
ViewRangefinder3d::from_view_matrix(&self.transform.compute_matrix())
92+
}
93+
}
94+
8795
#[derive(Clone, ShaderType)]
8896
pub struct ViewUniform {
8997
view_proj: Mat4,

examples/shader/animate_shader.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,7 @@ fn queue_custom(
116116
| MeshPipelineKey::from_primitive_topology(PrimitiveTopology::TriangleList);
117117

118118
for (view, mut transparent_phase) in views.iter_mut() {
119-
let view_matrix = view.transform.compute_matrix();
120-
let view_row_2 = view_matrix.row(2);
119+
let rangefinder = view.rangefinder3d();
121120
for (entity, mesh_uniform, mesh_handle) in material_meshes.iter() {
122121
if let Some(mesh) = render_meshes.get(mesh_handle) {
123122
let pipeline = pipelines
@@ -127,7 +126,7 @@ fn queue_custom(
127126
entity,
128127
pipeline,
129128
draw_function: draw_custom,
130-
distance: view_row_2.dot(mesh_uniform.transform.col(3)),
129+
distance: rangefinder.distance(&mesh_uniform.transform),
131130
});
132131
}
133132
}

examples/shader/shader_instancing.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,7 @@ fn queue_custom(
117117
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);
118118

119119
for (view, mut transparent_phase) in views.iter_mut() {
120-
let view_matrix = view.transform.compute_matrix();
121-
let view_row_2 = view_matrix.row(2);
120+
let rangefinder = view.rangefinder3d();
122121
for (entity, mesh_uniform, mesh_handle) in material_meshes.iter() {
123122
if let Some(mesh) = meshes.get(mesh_handle) {
124123
let key =
@@ -130,7 +129,7 @@ fn queue_custom(
130129
entity,
131130
pipeline,
132131
draw_function: draw_custom,
133-
distance: view_row_2.dot(mesh_uniform.transform.col(3)),
132+
distance: rangefinder.distance(&mesh_uniform.transform),
134133
});
135134
}
136135
}

0 commit comments

Comments
 (0)