Skip to content

Commit bfd89f0

Browse files
james7132Niliradsuperdumpcolepoirier
authored andcommitted
Allow unbatched render phases to use unstable sorts (bevyengine#5049)
# Objective Partially addresses bevyengine#4291. Speed up the sort phase for unbatched render phases. ## Solution Split out one of the optimizations in bevyengine#4899 and allow implementors of `PhaseItem` to change what kind of sort is used when sorting the items in the phase. This currently includes Stable, Unstable, and Unsorted. Each of these corresponds to `Vec::sort_by_key`, `Vec::sort_unstable_by_key`, and no sorting at all. The default is `Unstable`. The last one can be used as a default if users introduce a preliminary depth prepass. ## Performance This will not impact the performance of any batched phases, as it is still using a stable sort. 2D's only phase is unchanged. All 3D phases are unbatched currently, and will benefit from this change. On `many_cubes`, where the primary phase is opaque, this change sees a speed up from 907.02us -> 477.62us, a 47.35% reduction. ![image](https://user-images.githubusercontent.com/3137680/174471253-22424874-30d5-4db5-b5b4-65fb2c612a9c.png) ## Future Work There were prior discussions to add support for faster radix sorts in bevyengine#4291, which in theory should be a `O(n)` instead of a `O(nlog(n))` time. [`voracious`](https://crates.io/crates/voracious_radix_sort) has been proposed, but it seems to be optimize for use cases with more than 30,000 items, which may be atypical for most systems. Another optimization included in bevyengine#4899 is to reduce the size of a few of the IDs commonly used in `PhaseItem` implementations to shrink the types to make swapping/sorting faster. Both `CachedPipelineId` and `DrawFunctionId` could be reduced to `u32` instead of `usize`. Ideally, this should automatically change to use stable sorts when `BatchedPhaseItem` is implemented on the same phase item type, but this requires specialization, which may not land in stable Rust for a short while. --- ## Changelog Added: `PhaseItem::sort` ## Migration Guide RenderPhases now default to a unstable sort (via `slice::sort_unstable_by_key`). This can typically improve sort phase performance, but may produce incorrect batching results when implementing `BatchedPhaseItem`. To revert to the older stable sort, manually implement `PhaseItem::sort` to implement a stable sort (i.e. via `slice::sort_by_key`). Co-authored-by: Federico Rinaldi <[email protected]> Co-authored-by: Robert Swain <[email protected]> Co-authored-by: colepoirier <[email protected]>
1 parent 20b10c0 commit bfd89f0

File tree

7 files changed

+49
-3
lines changed

7 files changed

+49
-3
lines changed

crates/bevy_core_pipeline/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,4 @@ bevy_transform = { path = "../bevy_transform", version = "0.8.0-dev" }
2828
bevy_utils = { path = "../bevy_utils", version = "0.8.0-dev" }
2929

3030
serde = { version = "1", features = ["derive"] }
31-
31+
radsort = "0.1"

crates/bevy_core_pipeline/src/core_2d/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ impl PhaseItem for Transparent2d {
9090
fn draw_function(&self) -> DrawFunctionId {
9191
self.draw_function
9292
}
93+
94+
#[inline]
95+
fn sort(items: &mut [Self]) {
96+
items.sort_by_key(|item| item.sort_key());
97+
}
9398
}
9499

95100
impl EntityPhaseItem for Transparent2d {

crates/bevy_core_pipeline/src/core_3d/mod.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,11 @@ impl PhaseItem for Opaque3d {
9898
fn draw_function(&self) -> DrawFunctionId {
9999
self.draw_function
100100
}
101+
102+
#[inline]
103+
fn sort(items: &mut [Self]) {
104+
radsort::sort_by_key(items, |item| item.distance);
105+
}
101106
}
102107

103108
impl EntityPhaseItem for Opaque3d {
@@ -133,6 +138,11 @@ impl PhaseItem for AlphaMask3d {
133138
fn draw_function(&self) -> DrawFunctionId {
134139
self.draw_function
135140
}
141+
142+
#[inline]
143+
fn sort(items: &mut [Self]) {
144+
radsort::sort_by_key(items, |item| item.distance);
145+
}
136146
}
137147

138148
impl EntityPhaseItem for AlphaMask3d {
@@ -168,6 +178,11 @@ impl PhaseItem for Transparent3d {
168178
fn draw_function(&self) -> DrawFunctionId {
169179
self.draw_function
170180
}
181+
182+
#[inline]
183+
fn sort(items: &mut [Self]) {
184+
radsort::sort_by_key(items, |item| item.distance);
185+
}
171186
}
172187

173188
impl EntityPhaseItem for Transparent3d {

crates/bevy_pbr/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,4 @@ bevy_window = { path = "../bevy_window", version = "0.8.0-dev" }
2828
bitflags = "1.2"
2929
# direct dependency required for derive macro
3030
bytemuck = { version = "1", features = ["derive"] }
31+
radsort = "0.1"

crates/bevy_pbr/src/render/light.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1421,6 +1421,11 @@ impl PhaseItem for Shadow {
14211421
fn draw_function(&self) -> DrawFunctionId {
14221422
self.draw_function
14231423
}
1424+
1425+
#[inline]
1426+
fn sort(items: &mut [Self]) {
1427+
radsort::sort_by_key(items, |item| item.distance);
1428+
}
14241429
}
14251430

14261431
impl EntityPhaseItem for Shadow {

crates/bevy_render/src/render_phase/draw.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,30 @@ pub trait Draw<P: PhaseItem>: Send + Sync + 'static {
3535
/// Afterwards it will be sorted and rendered automatically in the
3636
/// [`RenderStage::PhaseSort`](crate::RenderStage::PhaseSort) stage and
3737
/// [`RenderStage::Render`](crate::RenderStage::Render) stage, respectively.
38-
pub trait PhaseItem: Send + Sync + 'static {
38+
pub trait PhaseItem: Sized + Send + Sync + 'static {
3939
/// The type used for ordering the items. The smallest values are drawn first.
4040
type SortKey: Ord;
4141
/// Determines the order in which the items are drawn during the corresponding [`RenderPhase`](super::RenderPhase).
4242
fn sort_key(&self) -> Self::SortKey;
4343
/// Specifies the [`Draw`] function used to render the item.
4444
fn draw_function(&self) -> DrawFunctionId;
45+
46+
/// Sorts a slice of phase items into render order. Generally if the same type
47+
/// implements [`BatchedPhaseItem`], this should use a stable sort like [`slice::sort_by_key`].
48+
/// In almost all other cases, this should not be altered from the default,
49+
/// which uses a unstable sort, as this provides the best balance of CPU and GPU
50+
/// performance.
51+
///
52+
/// Implementers can optionally not sort the list at all. This is generally advisable if and
53+
/// only if the renderer supports a depth prepass, which is by default not supported by
54+
/// the rest of Bevy's first party rendering crates. Even then, this may have a negative
55+
/// impact on GPU-side performance due to overdraw.
56+
///
57+
/// It's advised to always profile for performance changes when changing this implementation.
58+
#[inline]
59+
fn sort(items: &mut [Self]) {
60+
items.sort_unstable_by_key(|item| item.sort_key());
61+
}
4562
}
4663

4764
// TODO: make this generic?
@@ -170,6 +187,9 @@ pub trait CachedRenderPipelinePhaseItem: PhaseItem {
170187
///
171188
/// Batching is an optimization that regroups multiple items in the same vertex buffer
172189
/// to render them in a single draw call.
190+
///
191+
/// If this is implemented on a type, the implementation of [`PhaseItem::sort`] should
192+
/// be changed to implement a stable sort, or incorrect/suboptimal batching may result.
173193
pub trait BatchedPhaseItem: EntityPhaseItem {
174194
/// Range in the vertex buffer of this item
175195
fn batch_range(&self) -> &Option<Range<u32>>;

crates/bevy_render/src/render_phase/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ impl<I: PhaseItem> RenderPhase<I> {
2929

3030
/// Sorts all of its [`PhaseItems`](PhaseItem).
3131
pub fn sort(&mut self) {
32-
self.items.sort_by_key(|d| d.sort_key());
32+
I::sort(&mut self.items);
3333
}
3434
}
3535

0 commit comments

Comments
 (0)