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.

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

[Merged by Bors] - Allow unbatched render phases to use unstable sorts #5049

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from 16 commits
Commits
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
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,4 @@ bevy_transform = { path = "../bevy_transform", version = "0.8.0-dev" }
bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" }

serde = { version = "1", features = ["derive"] }

radsort = "0.1"
james7132 marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ impl PhaseItem for Transparent2d {
fn draw_function(&self) -> DrawFunctionId {
self.draw_function
}

#[inline]
fn sort(items: &mut [Self]) {
items.sort_by_key(|item| item.sort_key())
}
}

impl EntityPhaseItem for Transparent2d {
Expand Down
15 changes: 15 additions & 0 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ impl PhaseItem for Opaque3d {
fn draw_function(&self) -> DrawFunctionId {
self.draw_function
}

#[inline]
fn sort(items: &mut [Self]) {
radsort::sort_by_key(items, |item| item.distance);
}
}

impl EntityPhaseItem for Opaque3d {
Expand Down Expand Up @@ -133,6 +138,11 @@ impl PhaseItem for AlphaMask3d {
fn draw_function(&self) -> DrawFunctionId {
self.draw_function
}

#[inline]
fn sort(items: &mut [Self]) {
radsort::sort_by_key(items, |item| item.distance);
}
}

impl EntityPhaseItem for AlphaMask3d {
Expand Down Expand Up @@ -168,6 +178,11 @@ impl PhaseItem for Transparent3d {
fn draw_function(&self) -> DrawFunctionId {
self.draw_function
}

#[inline]
fn sort(items: &mut [Self]) {
radsort::sort_by_key(items, |item| item.distance);
}
}

impl EntityPhaseItem for Transparent3d {
Expand Down
22 changes: 21 additions & 1 deletion crates/bevy_render/src/render_phase/draw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,30 @@ pub trait Draw<P: PhaseItem>: Send + Sync + 'static {
/// Afterwards it will be sorted and rendered automatically in the
/// [`RenderStage::PhaseSort`](crate::RenderStage::PhaseSort) stage and
/// [`RenderStage::Render`](crate::RenderStage::Render) stage, respectively.
pub trait PhaseItem: Send + Sync + 'static {
pub trait PhaseItem: Sized + Send + Sync + 'static {
/// The type used for ordering the items. The smallest values are drawn first.
type SortKey: Ord;
/// Determines the order in which the items are drawn during the corresponding [`RenderPhase`](super::RenderPhase).
fn sort_key(&self) -> Self::SortKey;
/// Specifies the [`Draw`] function used to render the item.
fn draw_function(&self) -> DrawFunctionId;

/// Sorts a slice of phase items into render order. Generally if the same type
/// implements [`BatchedPhaseItem`], this should use a stable sort like [`slice::sort_by_key`].
/// In almost all other cases, this should not be altered from the default,
/// which uses a unstable sort, as this provides the best balance of CPU and GPU
/// performance.
///
/// Implementors can optionally not sort the list at all. This is generally adviseble if and
james7132 marked this conversation as resolved.
Show resolved Hide resolved
/// only if the renderer supports a depth prepass, which is by default not supported by
/// the rest of Bevy's first party rendering crates. Even then, this may have a negative
/// impact on GPU-side performance due to overdraw.
///
/// It's advised to always profile for performance changes when changing this implementation.
#[inline]
fn sort(items: &mut [Self]) {
items.sort_unstable_by_key(|item| item.sort_key())
}
}

// TODO: make this generic?
Expand Down Expand Up @@ -170,6 +187,9 @@ pub trait CachedRenderPipelinePhaseItem: PhaseItem {
///
/// Batching is an optimization that regroups multiple items in the same vertex buffer
/// to render them in a single draw call.
///
/// If this is implemented on a type, the implementation of [`PhaseItem::sort`] should
/// be changed to implement a stable sort, or incorrect/suboptimal batching may result.
pub trait BatchedPhaseItem: EntityPhaseItem {
/// Range in the vertex buffer of this item
fn batch_range(&self) -> &Option<Range<u32>>;
Expand Down
6 changes: 5 additions & 1 deletion crates/bevy_render/src/render_phase/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<I: PhaseItem> RenderPhase<I> {

/// Sorts all of its [`PhaseItems`](PhaseItem).
pub fn sort(&mut self) {
self.items.sort_by_key(|d| d.sort_key());
I::sort(&mut self.items)
}
}

Expand Down Expand Up @@ -98,6 +98,10 @@ mod tests {
fn draw_function(&self) -> DrawFunctionId {
unimplemented!();
}

fn sort(items: &mut [Self]) {
items.sort_by_key(|item| item.sort_key());
}
}
impl EntityPhaseItem for TestPhaseItem {
fn entity(&self) -> bevy_ecs::entity::Entity {
Expand Down