Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix view z calculation for 'distance' used as sort key #4330

Closed
wants to merge 3 commits into from
Closed

Fix view z calculation for 'distance' used as sort key #4330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7966d7
examples: Fix view z calculation for 'distance' used as sort key
superdump Mar 24, 2022
5027282
Centralize view z calculation in phase type constructors
superdump Mar 24, 2022
684edd4
Clean up unused imports
superdump May 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/bevy_core_pipeline/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ trace = []
bevy_app = { path = "../bevy_app", version = "0.8.0-dev" }
bevy_asset = { path = "../bevy_asset", version = "0.8.0-dev" }
bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev" }
bevy_math = { path = "../bevy_math", version = "0.8.0-dev" }
bevy_render = { path = "../bevy_render", version = "0.8.0-dev" }
bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" }

127 changes: 110 additions & 17 deletions crates/bevy_core_pipeline/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod prelude {
pub use crate::ClearColor;
}

use bevy_math::Mat4;
use bevy_utils::HashMap;

pub use clear_pass::*;
Expand Down Expand Up @@ -196,12 +197,32 @@ impl Plugin for CorePipelinePlugin {
}

pub struct Transparent2d {
pub sort_key: FloatOrd,
pub entity: Entity,
pub pipeline: CachedRenderPipelineId,
pub draw_function: DrawFunctionId,
sort_key: FloatOrd,
entity: Entity,
pipeline: CachedRenderPipelineId,
draw_function: DrawFunctionId,
/// Range in the vertex buffer of this item
pub batch_range: Option<Range<u32>>,
batch_range: Option<Range<u32>>,
}

impl Transparent2d {
// NOTE: This method exists for API consistency with the other phases defined below
#[inline]
pub fn new(
entity: Entity,
pipeline: CachedRenderPipelineId,
draw_function: DrawFunctionId,
sort_key: f32,
batch_range: Option<Range<u32>>,
) -> Self {
Self {
sort_key: FloatOrd(sort_key),
entity,
pipeline,
draw_function,
batch_range,
}
}
}

impl PhaseItem for Transparent2d {
Expand Down Expand Up @@ -243,10 +264,34 @@ impl BatchedPhaseItem for Transparent2d {
}

pub struct Opaque3d {
pub distance: f32,
pub pipeline: CachedRenderPipelineId,
pub entity: Entity,
pub draw_function: DrawFunctionId,
distance: f32,
pipeline: CachedRenderPipelineId,
entity: Entity,
draw_function: DrawFunctionId,
}

impl Opaque3d {
#[inline]
pub fn from_mesh_transform(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would there ever be a use case for creating an Opaque3d without first calculating the matrices? It looks like from_mesh_transform is the only way to construct the phase item now, which may be restricting?

Could be solved by either making the field public again or adding another unopinionated constructor if this is even useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I did it this way to prevent people from doing the wrong thing. You could pass in zero or identity matrices or something I guess.

entity: Entity,
pipeline: CachedRenderPipelineId,
draw_function: DrawFunctionId,
inverse_view: &Mat4,
mesh_transform: &Mat4,
) -> Self {
Self {
// NOTE: row 2 of the inverse view transform dotted with column 3 of the mesh transform
// gives the z component of translation of the mesh in view space
// 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
// results in the correct front-to-back ordering
distance: -inverse_view.row(2).dot(mesh_transform.col(3)),
pipeline,
entity,
draw_function,
}
}
}

impl PhaseItem for Opaque3d {
Expand Down Expand Up @@ -278,10 +323,34 @@ impl CachedRenderPipelinePhaseItem for Opaque3d {
}

pub struct AlphaMask3d {
pub distance: f32,
pub pipeline: CachedRenderPipelineId,
pub entity: Entity,
pub draw_function: DrawFunctionId,
distance: f32,
pipeline: CachedRenderPipelineId,
entity: Entity,
draw_function: DrawFunctionId,
}

impl AlphaMask3d {
#[inline]
pub fn from_mesh_transform(
entity: Entity,
pipeline: CachedRenderPipelineId,
draw_function: DrawFunctionId,
inverse_view: &Mat4,
mesh_transform: &Mat4,
) -> Self {
Self {
// NOTE: row 2 of the inverse view transform dotted with column 3 of the mesh transform
// gives the z component of translation of the mesh in view space
// 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 results in the correct front-to-back ordering
distance: -inverse_view.row(2).dot(mesh_transform.col(3)),
pipeline,
entity,
draw_function,
}
}
}

impl PhaseItem for AlphaMask3d {
Expand Down Expand Up @@ -313,10 +382,34 @@ impl CachedRenderPipelinePhaseItem for AlphaMask3d {
}

pub struct Transparent3d {
pub distance: f32,
pub pipeline: CachedRenderPipelineId,
pub entity: Entity,
pub draw_function: DrawFunctionId,
distance: f32,
pipeline: CachedRenderPipelineId,
entity: Entity,
draw_function: DrawFunctionId,
}

impl Transparent3d {
#[inline]
pub fn from_mesh_transform(
entity: Entity,
pipeline: CachedRenderPipelineId,
draw_function: DrawFunctionId,
inverse_view: &Mat4,
mesh_transform: &Mat4,
) -> Self {
Self {
// NOTE: row 2 of the inverse view transform dotted with column 3 of the mesh transform
// gives the z component of translation of the mesh in view space
// 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 view z as it is
distance: inverse_view.row(2).dot(mesh_transform.col(3)),
pipeline,
entity,
draw_function,
}
}
}

impl PhaseItem for Transparent3d {
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/light.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,9 @@ pub(crate) fn assign_lights_to_clusters(
lights
.iter()
.map(|light| {
// NOTE: row 2 of the inverse view matrix dotted with the world space
// translation gives the z component of translation of the mesh in view
// space
-inverse_view_row_2.dot(light.translation.extend(1.0)) + light.range
})
.reduce(f32::max)
Expand Down
55 changes: 21 additions & 34 deletions crates/bevy_pbr/src/material.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,6 @@ pub fn queue_material_meshes<M: SpecializedMaterial>(
.unwrap();

let inverse_view_matrix = view.transform.compute_matrix().inverse();
let inverse_view_row_2 = inverse_view_matrix.row(2);
let msaa_key = MeshPipelineKey::from_msaa_samples(msaa.samples);

for visible_entity in &visible_entities.entities {
Expand Down Expand Up @@ -376,45 +375,33 @@ pub fn queue_material_meshes<M: SpecializedMaterial>(
}
};

// 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));
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,
});
opaque_phase.add(Opaque3d::from_mesh_transform(
*visible_entity,
pipeline_id,
draw_opaque_pbr,
&inverse_view_matrix,
&mesh_uniform.transform,
));
}
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,
});
alpha_mask_phase.add(AlphaMask3d::from_mesh_transform(
*visible_entity,
pipeline_id,
draw_alpha_mask_pbr,
&inverse_view_matrix,
&mesh_uniform.transform,
));
}
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,
});
transparent_phase.add(Transparent3d::from_mesh_transform(
*visible_entity,
pipeline_id,
draw_transparent_pbr,
&inverse_view_matrix,
&mesh_uniform.transform,
));
}
}
}
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_pbr/src/wireframe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,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 inverse_view_matrix = view.transform.compute_matrix().inverse();

let add_render_phase =
|(entity, mesh_handle, mesh_uniform): (Entity, &Handle<Mesh>, &MeshUniform)| {
Expand All @@ -144,12 +143,13 @@ fn queue_wireframes(
return;
}
};
opaque_phase.add(Opaque3d {
opaque_phase.add(Opaque3d::from_mesh_transform(
entity,
pipeline: pipeline_id,
draw_function: draw_custom,
distance: view_row_2.dot(mesh_uniform.transform.col(3)),
});
pipeline_id,
draw_custom,
&inverse_view_matrix,
&mesh_uniform.transform,
));
}
};

Expand Down
20 changes: 7 additions & 13 deletions crates/bevy_sprite/src/mesh2d/material.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use bevy_render::{
RenderApp, RenderStage,
};
use bevy_transform::components::{GlobalTransform, Transform};
use bevy_utils::FloatOrd;
use std::hash::Hash;
use std::marker::PhantomData;

Expand Down Expand Up @@ -347,18 +346,13 @@ pub fn queue_material2d_meshes<M: SpecializedMaterial2d>(
};

let mesh_z = mesh2d_uniform.transform.w_axis.z;
transparent_phase.add(Transparent2d {
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
sort_key: FloatOrd(mesh_z),
// This material is not batched
batch_range: None,
});
transparent_phase.add(Transparent2d::new(
*visible_entity,
pipeline_id,
draw_transparent_pbr,
mesh_z,
None,
));
}
}
}
Expand Down
25 changes: 12 additions & 13 deletions crates/bevy_sprite/src/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use bevy_render::{
RenderWorld,
};
use bevy_transform::components::GlobalTransform;
use bevy_utils::FloatOrd;
use bevy_utils::HashMap;
use bytemuck::{Pod, Zeroable};
use copyless::VecHelper;
Expand Down Expand Up @@ -497,7 +496,7 @@ pub fn queue_sprites(
});

// These items will be sorted by depth with other phase items
let sort_key = FloatOrd(extracted_sprite.transform.translation.z);
let sort_key = extracted_sprite.transform.translation.z;

// Store the vertex data and add the item to the render phase
if current_batch.colored {
Expand All @@ -512,13 +511,13 @@ pub fn queue_sprites(
colored_index += QUAD_INDICES.len() as u32;
let item_end = colored_index;

transparent_phase.add(Transparent2d {
draw_function: draw_sprite_function,
pipeline: colored_pipeline,
entity: current_batch_entity,
transparent_phase.add(Transparent2d::new(
current_batch_entity,
colored_pipeline,
draw_sprite_function,
sort_key,
batch_range: Some(item_start..item_end),
});
Some(item_start..item_end),
));
} else {
for i in QUAD_INDICES.iter() {
sprite_meta.vertices.push(SpriteVertex {
Expand All @@ -530,13 +529,13 @@ pub fn queue_sprites(
index += QUAD_INDICES.len() as u32;
let item_end = index;

transparent_phase.add(Transparent2d {
draw_function: draw_sprite_function,
transparent_phase.add(Transparent2d::new(
current_batch_entity,
pipeline,
entity: current_batch_entity,
draw_sprite_function,
sort_key,
batch_range: Some(item_start..item_end),
});
Some(item_start..item_end),
));
}
}
}
Expand Down
Loading