Switch bins from parallel key/value arrays to IndexMaps. (#17819)

Conceptually, bins are ordered hash maps. We currently implement these
as a list of keys with an associated hash map. But we already have a
data type that implements ordered hash maps directly: `IndexMap`. This
patch switches Bevy to use `IndexMap`s for bins. Because we're memory
bound, this doesn't affect performance much, but it is cleaner.
This commit is contained in:
Patrick Walton 2025-02-12 14:39:04 -08:00 committed by GitHub
parent 267a0d003c
commit 5ff7062c1c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 72 additions and 120 deletions

View File

@ -147,9 +147,9 @@ impl ViewNode for DeferredGBufferPrepassNode {
}
// Opaque draws
if !opaque_deferred_phase.multidrawable_mesh_keys.is_empty()
|| !opaque_deferred_phase.batchable_mesh_keys.is_empty()
|| !opaque_deferred_phase.unbatchable_mesh_keys.is_empty()
if !opaque_deferred_phase.multidrawable_meshes.is_empty()
|| !opaque_deferred_phase.batchable_meshes.is_empty()
|| !opaque_deferred_phase.unbatchable_meshes.is_empty()
{
#[cfg(feature = "trace")]
let _opaque_prepass_span = info_span!("opaque_deferred_prepass").entered();

View File

@ -1374,11 +1374,11 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
// Prepare multidrawables.
for batch_set_key in &phase.multidrawable_mesh_keys {
for (batch_set_key, bins) in &phase.multidrawable_meshes {
let mut batch_set = None;
let indirect_parameters_base =
indirect_parameters_buffers.batch_count(batch_set_key.indexed()) as u32;
for (bin_key, bin) in &phase.multidrawable_mesh_values[batch_set_key] {
for (bin_key, bin) in bins {
let first_output_index = data_buffer.len() as u32;
let mut batch: Option<BinnedRenderPhaseBatch> = None;
@ -1472,11 +1472,11 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
// Prepare batchables.
for key in &phase.batchable_mesh_keys {
for (key, bin) in &phase.batchable_meshes {
let first_output_index = data_buffer.len() as u32;
let mut batch: Option<BinnedRenderPhaseBatch> = None;
for (&main_entity, &input_index) in phase.batchable_mesh_values[key].entities() {
for (&main_entity, &input_index) in bin.entities() {
let output_index = data_buffer.add() as u32;
match batch {
@ -1589,9 +1589,7 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
}
// Prepare unbatchables.
for key in &phase.unbatchable_mesh_keys {
let unbatchables = phase.unbatchable_mesh_values.get_mut(key).unwrap();
for (key, unbatchables) in &mut phase.unbatchable_meshes {
// Allocate the indirect parameters if necessary.
let mut indirect_parameters_offset = if no_indirect_drawing {
None

View File

@ -190,23 +190,9 @@ where
BPI: BinnedPhaseItem,
{
for phase in phases.values_mut() {
phase.multidrawable_mesh_keys.clear();
phase
.multidrawable_mesh_keys
.extend(phase.multidrawable_mesh_values.keys().cloned());
phase.multidrawable_mesh_keys.sort_unstable();
phase.batchable_mesh_keys.clear();
phase
.batchable_mesh_keys
.extend(phase.batchable_mesh_values.keys().cloned());
phase.batchable_mesh_keys.sort_unstable();
phase.unbatchable_mesh_keys.clear();
phase
.unbatchable_mesh_keys
.extend(phase.unbatchable_mesh_values.keys().cloned());
phase.unbatchable_mesh_keys.sort_unstable();
phase.multidrawable_meshes.sort_unstable_keys();
phase.batchable_meshes.sort_unstable_keys();
phase.unbatchable_meshes.sort_unstable_keys();
}
}

View File

@ -108,9 +108,9 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
for phase in phases.values_mut() {
// Prepare batchables.
for key in &phase.batchable_mesh_keys {
for bin in phase.batchable_meshes.values_mut() {
let mut batch_set: SmallVec<[BinnedRenderPhaseBatch; 1]> = smallvec![];
for main_entity in phase.batchable_mesh_values[key].entities().keys() {
for main_entity in bin.entities().keys() {
let Some(buffer_data) =
GFBD::get_binned_batch_data(&system_param_item, *main_entity)
else {
@ -156,8 +156,7 @@ pub fn batch_and_prepare_binned_render_phase<BPI, GFBD>(
}
// Prepare unbatchables.
for key in &phase.unbatchable_mesh_keys {
let unbatchables = phase.unbatchable_mesh_values.get_mut(key).unwrap();
for unbatchables in phase.unbatchable_meshes.values_mut() {
for main_entity in unbatchables.entities.keys() {
let Some(buffer_data) =
GFBD::get_binned_batch_data(&system_param_item, *main_entity)

View File

@ -92,13 +92,7 @@ pub struct BinnedRenderPhase<BPI>
where
BPI: BinnedPhaseItem,
{
/// A list of `BatchSetKey`s for batchable, multidrawable items.
///
/// These are accumulated in `queue_material_meshes` and then sorted in
/// `batching::sort_binned_render_phase`.
pub multidrawable_mesh_keys: Vec<BPI::BatchSetKey>,
/// The multidrawable bins themselves.
/// The multidrawable bins.
///
/// Each batch set key maps to a *batch set*, which in this case is a set of
/// meshes that can be drawn together in one multidraw call. Each batch set
@ -111,36 +105,18 @@ where
/// the same pipeline. The first bin, corresponding to the cubes, will have
/// two entities in it. The second bin, corresponding to the sphere, will
/// have one entity in it.
pub multidrawable_mesh_values: HashMap<BPI::BatchSetKey, HashMap<BPI::BinKey, RenderBin>>,
/// A list of `BinKey`s for batchable items that aren't multidrawable.
///
/// These are accumulated in `queue_material_meshes` and then sorted in
/// `batch_and_prepare_binned_render_phase`.
///
/// Usually, batchable items aren't multidrawable due to platform or
/// hardware limitations. However, it's also possible to have batchable
/// items alongside multidrawable items with custom mesh pipelines. See
/// `specialized_mesh_pipeline` for an example.
pub batchable_mesh_keys: Vec<(BPI::BatchSetKey, BPI::BinKey)>,
pub multidrawable_meshes: IndexMap<BPI::BatchSetKey, IndexMap<BPI::BinKey, RenderBin>>,
/// The bins corresponding to batchable items that aren't multidrawable.
///
/// For multidrawable entities, use `multidrawable_mesh_values`; for
/// For multidrawable entities, use `multidrawable_meshes`; for
/// unbatchable entities, use `unbatchable_values`.
pub batchable_mesh_values: HashMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
/// A list of `BinKey`s for unbatchable items.
///
/// These are accumulated in `queue_material_meshes` and then sorted in
/// `batch_and_prepare_binned_render_phase`.
pub unbatchable_mesh_keys: Vec<(BPI::BatchSetKey, BPI::BinKey)>,
pub batchable_meshes: IndexMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
/// The unbatchable bins.
///
/// Each entity here is rendered in a separate drawcall.
pub unbatchable_mesh_values:
HashMap<(BPI::BatchSetKey, BPI::BinKey), UnbatchableBinnedEntities>,
pub unbatchable_meshes: IndexMap<(BPI::BatchSetKey, BPI::BinKey), UnbatchableBinnedEntities>,
/// Items in the bin that aren't meshes at all.
///
@ -149,7 +125,7 @@ where
/// entity are simply called in order at rendering time.
///
/// See the `custom_phase_item` example for an example of how to use this.
pub non_mesh_items: HashMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
pub non_mesh_items: IndexMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
/// Information on each batch set.
///
@ -456,16 +432,16 @@ where
) {
match phase_type {
BinnedRenderPhaseType::MultidrawableMesh => {
match self.multidrawable_mesh_values.entry(batch_set_key.clone()) {
Entry::Occupied(mut entry) => {
match self.multidrawable_meshes.entry(batch_set_key.clone()) {
indexmap::map::Entry::Occupied(mut entry) => {
entry
.get_mut()
.entry(bin_key.clone())
.or_default()
.insert(main_entity, input_uniform_index);
}
Entry::Vacant(entry) => {
let mut new_batch_set = HashMap::default();
indexmap::map::Entry::Vacant(entry) => {
let mut new_batch_set = IndexMap::default();
new_batch_set.insert(
bin_key.clone(),
RenderBin::from_entity(main_entity, input_uniform_index),
@ -477,13 +453,13 @@ where
BinnedRenderPhaseType::BatchableMesh => {
match self
.batchable_mesh_values
.batchable_meshes
.entry((batch_set_key.clone(), bin_key.clone()).clone())
{
Entry::Occupied(mut entry) => {
indexmap::map::Entry::Occupied(mut entry) => {
entry.get_mut().insert(main_entity, input_uniform_index);
}
Entry::Vacant(entry) => {
indexmap::map::Entry::Vacant(entry) => {
entry.insert(RenderBin::from_entity(main_entity, input_uniform_index));
}
}
@ -491,13 +467,13 @@ where
BinnedRenderPhaseType::UnbatchableMesh => {
match self
.unbatchable_mesh_values
.unbatchable_meshes
.entry((batch_set_key.clone(), bin_key.clone()))
{
Entry::Occupied(mut entry) => {
indexmap::map::Entry::Occupied(mut entry) => {
entry.get_mut().entities.insert(main_entity, entity);
}
Entry::Vacant(entry) => {
indexmap::map::Entry::Vacant(entry) => {
let mut entities = MainEntityHashMap::default();
entities.insert(main_entity, entity);
entry.insert(UnbatchableBinnedEntities {
@ -514,10 +490,10 @@ where
.non_mesh_items
.entry((batch_set_key.clone(), bin_key.clone()).clone())
{
Entry::Occupied(mut entry) => {
indexmap::map::Entry::Occupied(mut entry) => {
entry.get_mut().insert(main_entity, input_uniform_index);
}
Entry::Vacant(entry) => {
indexmap::map::Entry::Vacant(entry) => {
entry.insert(RenderBin::from_entity(main_entity, input_uniform_index));
}
}
@ -592,10 +568,10 @@ where
match self.batch_sets {
BinnedRenderPhaseBatchSets::DynamicUniforms(ref batch_sets) => {
debug_assert_eq!(self.batchable_mesh_keys.len(), batch_sets.len());
debug_assert_eq!(self.batchable_meshes.len(), batch_sets.len());
for ((batch_set_key, bin_key), batch_set) in
self.batchable_mesh_keys.iter().zip(batch_sets.iter())
self.batchable_meshes.keys().zip(batch_sets.iter())
{
for batch in batch_set {
let binned_phase_item = BPI::new(
@ -620,7 +596,7 @@ where
BinnedRenderPhaseBatchSets::Direct(ref batch_set) => {
for (batch, (batch_set_key, bin_key)) in
batch_set.iter().zip(self.batchable_mesh_keys.iter())
batch_set.iter().zip(self.batchable_meshes.keys())
{
let binned_phase_item = BPI::new(
batch_set_key.clone(),
@ -643,11 +619,11 @@ where
BinnedRenderPhaseBatchSets::MultidrawIndirect(ref batch_sets) => {
for (batch_set_key, batch_set) in self
.multidrawable_mesh_keys
.iter()
.multidrawable_meshes
.keys()
.chain(
self.batchable_mesh_keys
.iter()
self.batchable_meshes
.keys()
.map(|(batch_set_key, _)| batch_set_key),
)
.zip(batch_sets.iter())
@ -704,9 +680,9 @@ where
let draw_functions = world.resource::<DrawFunctions<BPI>>();
let mut draw_functions = draw_functions.write();
for (batch_set_key, bin_key) in &self.unbatchable_mesh_keys {
for (batch_set_key, bin_key) in self.unbatchable_meshes.keys() {
let unbatchable_entities =
&self.unbatchable_mesh_values[&(batch_set_key.clone(), bin_key.clone())];
&self.unbatchable_meshes[&(batch_set_key.clone(), bin_key.clone())];
for (entity_index, entity) in unbatchable_entities.entities.iter().enumerate() {
let unbatchable_dynamic_offset = match &unbatchable_entities.buffer_indices {
UnbatchableBinnedEntityIndexSet::NoEntities => {
@ -795,16 +771,13 @@ where
}
pub fn is_empty(&self) -> bool {
self.multidrawable_mesh_values.is_empty()
&& self.batchable_mesh_values.is_empty()
&& self.unbatchable_mesh_values.is_empty()
self.multidrawable_meshes.is_empty()
&& self.batchable_meshes.is_empty()
&& self.unbatchable_meshes.is_empty()
&& self.non_mesh_items.is_empty()
}
pub fn prepare_for_new_frame(&mut self) {
self.multidrawable_mesh_keys.clear();
self.batchable_mesh_keys.clear();
self.unbatchable_mesh_keys.clear();
self.batch_sets.clear();
self.valid_cached_entity_bin_keys.clear();
@ -858,9 +831,9 @@ where
remove_entity_from_bin(
entity,
&entity_bin_key,
&mut self.multidrawable_mesh_values,
&mut self.batchable_mesh_values,
&mut self.unbatchable_mesh_values,
&mut self.multidrawable_meshes,
&mut self.batchable_meshes,
&mut self.unbatchable_meshes,
&mut self.non_mesh_items,
);
}
@ -870,9 +843,9 @@ where
remove_entity_from_bin(
entity_that_changed_bins.main_entity,
&entity_that_changed_bins.old_bin_key,
&mut self.multidrawable_mesh_values,
&mut self.batchable_mesh_values,
&mut self.unbatchable_mesh_values,
&mut self.multidrawable_meshes,
&mut self.batchable_meshes,
&mut self.unbatchable_meshes,
&mut self.non_mesh_items,
);
}
@ -888,22 +861,19 @@ where
fn remove_entity_from_bin<BPI>(
entity: MainEntity,
entity_bin_key: &CachedBinKey<BPI>,
multidrawable_mesh_values: &mut HashMap<BPI::BatchSetKey, HashMap<BPI::BinKey, RenderBin>>,
batchable_mesh_values: &mut HashMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
unbatchable_mesh_values: &mut HashMap<
(BPI::BatchSetKey, BPI::BinKey),
UnbatchableBinnedEntities,
>,
non_mesh_items: &mut HashMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
multidrawable_meshes: &mut IndexMap<BPI::BatchSetKey, IndexMap<BPI::BinKey, RenderBin>>,
batchable_meshes: &mut IndexMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
unbatchable_meshes: &mut IndexMap<(BPI::BatchSetKey, BPI::BinKey), UnbatchableBinnedEntities>,
non_mesh_items: &mut IndexMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>,
) where
BPI: BinnedPhaseItem,
{
match entity_bin_key.phase_type {
BinnedRenderPhaseType::MultidrawableMesh => {
if let Entry::Occupied(mut batch_set_entry) =
multidrawable_mesh_values.entry(entity_bin_key.batch_set_key.clone())
if let indexmap::map::Entry::Occupied(mut batch_set_entry) =
multidrawable_meshes.entry(entity_bin_key.batch_set_key.clone())
{
if let Entry::Occupied(mut bin_entry) = batch_set_entry
if let indexmap::map::Entry::Occupied(mut bin_entry) = batch_set_entry
.get_mut()
.entry(entity_bin_key.bin_key.clone())
{
@ -911,19 +881,21 @@ fn remove_entity_from_bin<BPI>(
// If the bin is now empty, remove the bin.
if bin_entry.get_mut().is_empty() {
bin_entry.remove();
bin_entry.swap_remove();
}
}
// If the batch set is now empty, remove it.
// If the batch set is now empty, remove it. This will perturb
// the order, but that's OK because we're going to sort the bin
// afterwards.
if batch_set_entry.get_mut().is_empty() {
batch_set_entry.remove();
batch_set_entry.swap_remove();
}
}
}
BinnedRenderPhaseType::BatchableMesh => {
if let Entry::Occupied(mut bin_entry) = batchable_mesh_values.entry((
if let indexmap::map::Entry::Occupied(mut bin_entry) = batchable_meshes.entry((
entity_bin_key.batch_set_key.clone(),
entity_bin_key.bin_key.clone(),
)) {
@ -931,13 +903,13 @@ fn remove_entity_from_bin<BPI>(
// If the bin is now empty, remove the bin.
if bin_entry.get_mut().is_empty() {
bin_entry.remove();
bin_entry.swap_remove();
}
}
}
BinnedRenderPhaseType::UnbatchableMesh => {
if let Entry::Occupied(mut bin_entry) = unbatchable_mesh_values.entry((
if let indexmap::map::Entry::Occupied(mut bin_entry) = unbatchable_meshes.entry((
entity_bin_key.batch_set_key.clone(),
entity_bin_key.bin_key.clone(),
)) {
@ -945,13 +917,13 @@ fn remove_entity_from_bin<BPI>(
// If the bin is now empty, remove the bin.
if bin_entry.get_mut().entities.is_empty() {
bin_entry.remove();
bin_entry.swap_remove();
}
}
}
BinnedRenderPhaseType::NonMesh => {
if let Entry::Occupied(mut bin_entry) = non_mesh_items.entry((
if let indexmap::map::Entry::Occupied(mut bin_entry) = non_mesh_items.entry((
entity_bin_key.batch_set_key.clone(),
entity_bin_key.bin_key.clone(),
)) {
@ -959,7 +931,7 @@ fn remove_entity_from_bin<BPI>(
// If the bin is now empty, remove the bin.
if bin_entry.get_mut().is_empty() {
bin_entry.remove();
bin_entry.swap_remove();
}
}
}
@ -972,13 +944,10 @@ where
{
fn new(gpu_preprocessing: GpuPreprocessingMode) -> Self {
Self {
multidrawable_mesh_keys: vec![],
multidrawable_mesh_values: HashMap::default(),
batchable_mesh_keys: vec![],
batchable_mesh_values: HashMap::default(),
unbatchable_mesh_keys: vec![],
unbatchable_mesh_values: HashMap::default(),
non_mesh_items: HashMap::default(),
multidrawable_meshes: IndexMap::default(),
batchable_meshes: IndexMap::default(),
unbatchable_meshes: IndexMap::default(),
non_mesh_items: IndexMap::default(),
batch_sets: match gpu_preprocessing {
GpuPreprocessingMode::Culling => {
BinnedRenderPhaseBatchSets::MultidrawIndirect(vec![])