From 7b544122ec6f50ecdd8f7057168266b4fcbee957 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Mon, 3 Feb 2025 15:58:02 -0800 Subject: [PATCH 1/5] Retain bins from frame to frame. This PR makes Bevy keep entities in bins from frame to frame if they haven't changed. This reduces the time spent in `queue_material_meshes` and related functions to near zero for static geometry. This patch uses the same change tick technique that #17567 to detect when meshes have changed in such a way as to require re-binning. In order to quickly find the relevant bin for an entity when that entity has changed, we introduce a new type of cache, the *bin key cache*. This cache stores a mapping from main world entity ID to cached bin key, as well as the tick of the most recent change to the entity. As we iterate through the visible entities in `queue_material_meshes`, we check the cache to see whether the entity needs to be re-binned. If it doesn't, then we mark it as clean in the `valid_cached_entity_bin_keys` bitset. At the end, all bin keys not marked as clean are removed from the bins. This patch has a dramatic effect on the rendering performance of most benchmarks, as it effectively eliminates `queue_material_meshes` from the profile. Note, however, that it generally simultaneously regresses `batch_and_prepare_binned_render_phase` by a bit (not by enough to outweigh the win, however). I believe that's because, before this patch, `queue_material_meshes` put the bins in the CPU cache for `batch_and_prepare_binned_render_phase` to use, while with this patch, `batch_and_prepare_binned_render_phase` must load the batches into the CPU cache itself. --- crates/bevy_core_pipeline/src/core_2d/mod.rs | 5 +- crates/bevy_core_pipeline/src/core_3d/mod.rs | 14 +- crates/bevy_pbr/src/material.rs | 18 +- crates/bevy_pbr/src/prepass/mod.rs | 24 +- crates/bevy_pbr/src/render/light.rs | 20 +- crates/bevy_render/Cargo.toml | 5 + .../src/batching/gpu_preprocessing.rs | 20 +- crates/bevy_render/src/batching/mod.rs | 14 + .../src/batching/no_gpu_preprocessing.rs | 11 +- crates/bevy_render/src/render_phase/mod.rs | 399 +++++++++++++++--- crates/bevy_render/src/sync_world.rs | 2 +- crates/bevy_sprite/src/mesh2d/material.rs | 18 +- examples/shader/custom_phase_item.rs | 7 + examples/shader/specialized_mesh_pipeline.rs | 8 +- 14 files changed, 480 insertions(+), 85 deletions(-) diff --git a/crates/bevy_core_pipeline/src/core_2d/mod.rs b/crates/bevy_core_pipeline/src/core_2d/mod.rs index f820a245cecff..f32775727772f 100644 --- a/crates/bevy_core_pipeline/src/core_2d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_2d/mod.rs @@ -436,8 +436,9 @@ pub fn extract_core_2d_camera_phases( let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0); transparent_2d_phases.insert_or_clear(retained_view_entity); - opaque_2d_phases.insert_or_clear(retained_view_entity, GpuPreprocessingMode::None); - alpha_mask_2d_phases.insert_or_clear(retained_view_entity, GpuPreprocessingMode::None); + opaque_2d_phases.prepare_for_new_frame(retained_view_entity, GpuPreprocessingMode::None); + alpha_mask_2d_phases + .prepare_for_new_frame(retained_view_entity, GpuPreprocessingMode::None); live_entities.insert(retained_view_entity); } diff --git a/crates/bevy_core_pipeline/src/core_3d/mod.rs b/crates/bevy_core_pipeline/src/core_3d/mod.rs index 0d6df5c8873ae..7572d80b3944e 100644 --- a/crates/bevy_core_pipeline/src/core_3d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_3d/mod.rs @@ -629,8 +629,8 @@ pub fn extract_core_3d_camera_phases( // This is the main 3D camera, so use the first subview index (0). let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0); - opaque_3d_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); - alpha_mask_3d_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + opaque_3d_phases.prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); + alpha_mask_3d_phases.prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); transmissive_3d_phases.insert_or_clear(retained_view_entity); transparent_3d_phases.insert_or_clear(retained_view_entity); @@ -698,18 +698,20 @@ pub fn extract_camera_prepass_phase( let retained_view_entity = RetainedViewEntity::new(main_entity.into(), None, 0); if depth_prepass || normal_prepass || motion_vector_prepass { - opaque_3d_prepass_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + opaque_3d_prepass_phases + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); alpha_mask_3d_prepass_phases - .insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); } else { opaque_3d_prepass_phases.remove(&retained_view_entity); alpha_mask_3d_prepass_phases.remove(&retained_view_entity); } if deferred_prepass { - opaque_3d_deferred_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + opaque_3d_deferred_phases + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); alpha_mask_3d_deferred_phases - .insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); } else { opaque_3d_deferred_phases.remove(&retained_view_entity); alpha_mask_3d_deferred_phases.remove(&retained_view_entity); diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index 5b863eaa96d93..c1618917abbb6 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -940,12 +940,20 @@ pub fn queue_material_meshes( let rangefinder = view.rangefinder3d(); for (render_entity, visible_entity) in visible_entities.iter::() { - let Some(pipeline_id) = specialized_material_pipeline_cache + let Some((current_change_tick, pipeline_id)) = specialized_material_pipeline_cache .get(&(*view_entity, *visible_entity)) - .map(|(_, pipeline_id)| *pipeline_id) + .map(|(current_change_tick, pipeline_id)| (*current_change_tick, *pipeline_id)) else { continue; }; + + // Skip the entity if it's cached in a bin and up to date. + if opaque_phase.validate_cached_entity(*visible_entity, current_change_tick) + || alpha_mask_phase.validate_cached_entity(*visible_entity, current_change_tick) + { + continue; + } + let Some(material_asset_id) = render_material_instances.get(visible_entity) else { continue; }; @@ -997,6 +1005,7 @@ pub fn queue_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + current_change_tick, ); } // Alpha mask @@ -1019,6 +1028,7 @@ pub fn queue_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + current_change_tick, ); } RenderPhaseType::Transparent => { @@ -1036,6 +1046,10 @@ pub fn queue_material_meshes( } } } + + // Remove invalid entities from the bins. + opaque_phase.sweep_old_entities(); + alpha_mask_phase.sweep_old_entities(); } } diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 97ea684cd3d68..b4e092bf95532 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -1089,11 +1089,21 @@ pub fn queue_prepass_material_meshes( } for (render_entity, visible_entity) in visible_entities.iter::() { - let Some((_, pipeline_id)) = + let Some((current_change_tick, pipeline_id)) = specialized_material_pipeline_cache.get(&(*view_entity, *visible_entity)) else { continue; }; + + // Skip the entity if it's cached in a bin and up to date. + if opaque_phase.as_mut().is_some_and(|opaque_phase| { + opaque_phase.validate_cached_entity(*visible_entity, *current_change_tick) + }) || alpha_mask_phase.as_mut().is_some_and(|alpha_mask_phase| { + alpha_mask_phase.validate_cached_entity(*visible_entity, *current_change_tick) + }) { + continue; + } + let Some(material_asset_id) = render_material_instances.get(visible_entity) else { continue; }; @@ -1134,6 +1144,7 @@ pub fn queue_prepass_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + *current_change_tick, ); } else if let Some(opaque_phase) = opaque_phase.as_mut() { let (vertex_slab, index_slab) = @@ -1157,6 +1168,7 @@ pub fn queue_prepass_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + *current_change_tick, ); } } @@ -1182,6 +1194,7 @@ pub fn queue_prepass_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + *current_change_tick, ); } else if let Some(alpha_mask_phase) = alpha_mask_phase.as_mut() { let (vertex_slab, index_slab) = @@ -1204,12 +1217,21 @@ pub fn queue_prepass_material_meshes( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + *current_change_tick, ); } } _ => {} } } + + // Remove invalid entities from the bins. + if let Some(opaque_phase) = opaque_phase { + opaque_phase.sweep_old_entities(); + } + if let Some(alpha_mask_phase) = alpha_mask_phase { + alpha_mask_phase.sweep_old_entities(); + } } } diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 5cdea0627b7b7..a5e42deff7a91 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -1299,7 +1299,7 @@ pub fn prepare_lights( if first { // Subsequent views with the same light entity will reuse the same shadow map shadow_render_phases - .insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); live_shadow_mapping_lights.insert(retained_view_entity); } } @@ -1396,7 +1396,8 @@ pub fn prepare_lights( if first { // Subsequent views with the same light entity will reuse the same shadow map - shadow_render_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + shadow_render_phases + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); live_shadow_mapping_lights.insert(retained_view_entity); } } @@ -1539,7 +1540,8 @@ pub fn prepare_lights( // Subsequent views with the same light entity will **NOT** reuse the same shadow map // (Because the cascades are unique to each view) // TODO: Implement GPU culling for shadow passes. - shadow_render_phases.insert_or_clear(retained_view_entity, gpu_preprocessing_mode); + shadow_render_phases + .prepare_for_new_frame(retained_view_entity, gpu_preprocessing_mode); live_shadow_mapping_lights.insert(retained_view_entity); } } @@ -1884,11 +1886,17 @@ pub fn queue_shadows( }; for (entity, main_entity) in visible_entities.iter().copied() { - let Some((_, pipeline_id)) = + let Some((current_change_tick, pipeline_id)) = specialized_material_pipeline_cache.get(&(view_light_entity, main_entity)) else { continue; }; + + // Skip the entity if it's cached in a bin and up to date. + if shadow_phase.validate_cached_entity(main_entity, *current_change_tick) { + continue; + } + let Some(mesh_instance) = render_mesh_instances.render_mesh_queue_data(main_entity) else { continue; @@ -1920,8 +1928,12 @@ pub fn queue_shadows( mesh_instance.should_batch(), &gpu_preprocessing_support, ), + *current_change_tick, ); } + + // Remove invalid entities from the bins. + shadow_phase.sweep_old_entities(); } } } diff --git a/crates/bevy_render/Cargo.toml b/crates/bevy_render/Cargo.toml index 4962125622270..5d8018784735a 100644 --- a/crates/bevy_render/Cargo.toml +++ b/crates/bevy_render/Cargo.toml @@ -96,6 +96,8 @@ smallvec = { version = "1.11", features = ["const_new"] } offset-allocator = "0.2" variadics_please = "1.1" tracing = { version = "0.1", default-features = false, features = ["std"] } +indexmap = { version = "2" } +fixedbitset = { version = "0.5" } [target.'cfg(not(target_arch = "wasm32"))'.dependencies] # Omit the `glsl` feature in non-WebAssembly by default. @@ -103,6 +105,9 @@ naga_oil = { version = "0.16", default-features = false, features = [ "test_shader", ] } +[dev-dependencies] +proptest = "1" + [target.'cfg(target_arch = "wasm32")'.dependencies] naga_oil = "0.16" js-sys = "0.3" diff --git a/crates/bevy_render/src/batching/gpu_preprocessing.rs b/crates/bevy_render/src/batching/gpu_preprocessing.rs index 69e759da2498d..b620b6055410c 100644 --- a/crates/bevy_render/src/batching/gpu_preprocessing.rs +++ b/crates/bevy_render/src/batching/gpu_preprocessing.rs @@ -4,6 +4,7 @@ use core::any::TypeId; use bevy_app::{App, Plugin}; use bevy_ecs::{ + prelude::Entity, query::{Has, With}, resource::Resource, schedule::IntoSystemConfigs as _, @@ -1326,8 +1327,9 @@ pub fn batch_and_prepare_binned_render_phase( let first_output_index = data_buffer.len() as u32; let mut batch: Option = None; - for &(entity, main_entity) in &bin.entities { - let Some(input_index) = GFBD::get_binned_index(&system_param_item, main_entity) + for main_entity in bin.entities() { + let Some(input_index) = + GFBD::get_binned_index(&system_param_item, *main_entity) else { continue; }; @@ -1378,7 +1380,7 @@ pub fn batch_and_prepare_binned_render_phase( }, ); batch = Some(BinnedRenderPhaseBatch { - representative_entity: (entity, main_entity), + representative_entity: (Entity::PLACEHOLDER, *main_entity), instance_range: output_index..output_index + 1, extra_index: PhaseItemExtraIndex::maybe_indirect_parameters_index( NonMaxU32::new(indirect_parameters_index), @@ -1424,8 +1426,8 @@ pub fn batch_and_prepare_binned_render_phase( let first_output_index = data_buffer.len() as u32; let mut batch: Option = None; - for &(entity, main_entity) in &phase.batchable_mesh_values[key].entities { - let Some(input_index) = GFBD::get_binned_index(&system_param_item, main_entity) + for main_entity in phase.batchable_mesh_values[key].entities() { + let Some(input_index) = GFBD::get_binned_index(&system_param_item, *main_entity) else { continue; }; @@ -1487,7 +1489,7 @@ pub fn batch_and_prepare_binned_render_phase( }, ); batch = Some(BinnedRenderPhaseBatch { - representative_entity: (entity, main_entity), + representative_entity: (Entity::PLACEHOLDER, *main_entity), instance_range: output_index..output_index + 1, extra_index: PhaseItemExtraIndex::IndirectParametersIndex { range: indirect_parameters_index..(indirect_parameters_index + 1), @@ -1507,7 +1509,7 @@ pub fn batch_and_prepare_binned_render_phase( }, ); batch = Some(BinnedRenderPhaseBatch { - representative_entity: (entity, main_entity), + representative_entity: (Entity::PLACEHOLDER, *main_entity), instance_range: output_index..output_index + 1, extra_index: PhaseItemExtraIndex::None, }); @@ -1559,8 +1561,8 @@ pub fn batch_and_prepare_binned_render_phase( ) }; - for &(_, main_entity) in &unbatchables.entities { - let Some(input_index) = GFBD::get_binned_index(&system_param_item, main_entity) + for main_entity in unbatchables.entities.keys() { + let Some(input_index) = GFBD::get_binned_index(&system_param_item, *main_entity) else { continue; }; diff --git a/crates/bevy_render/src/batching/mod.rs b/crates/bevy_render/src/batching/mod.rs index 214fdda13644e..9569f2ce8c05f 100644 --- a/crates/bevy_render/src/batching/mod.rs +++ b/crates/bevy_render/src/batching/mod.rs @@ -182,8 +182,22 @@ 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(); } } diff --git a/crates/bevy_render/src/batching/no_gpu_preprocessing.rs b/crates/bevy_render/src/batching/no_gpu_preprocessing.rs index 6e0b893028f8d..7206bb4539742 100644 --- a/crates/bevy_render/src/batching/no_gpu_preprocessing.rs +++ b/crates/bevy_render/src/batching/no_gpu_preprocessing.rs @@ -1,6 +1,7 @@ //! Batching functionality when GPU preprocessing isn't in use. use bevy_derive::{Deref, DerefMut}; +use bevy_ecs::entity::Entity; use bevy_ecs::resource::Resource; use bevy_ecs::system::{Res, ResMut, StaticSystemParam}; use smallvec::{smallvec, SmallVec}; @@ -109,9 +110,9 @@ pub fn batch_and_prepare_binned_render_phase( for key in &phase.batchable_mesh_keys { let mut batch_set: SmallVec<[BinnedRenderPhaseBatch; 1]> = smallvec![]; - for &(entity, main_entity) in &phase.batchable_mesh_values[key].entities { + for main_entity in phase.batchable_mesh_values[key].entities() { let Some(buffer_data) = - GFBD::get_binned_batch_data(&system_param_item, main_entity) + GFBD::get_binned_batch_data(&system_param_item, *main_entity) else { continue; }; @@ -128,7 +129,7 @@ pub fn batch_and_prepare_binned_render_phase( == PhaseItemExtraIndex::maybe_dynamic_offset(instance.dynamic_offset) }) { batch_set.push(BinnedRenderPhaseBatch { - representative_entity: (entity, main_entity), + representative_entity: (Entity::PLACEHOLDER, *main_entity), instance_range: instance.index..instance.index, extra_index: PhaseItemExtraIndex::maybe_dynamic_offset( instance.dynamic_offset, @@ -157,9 +158,9 @@ pub fn batch_and_prepare_binned_render_phase( // Prepare unbatchables. for key in &phase.unbatchable_mesh_keys { let unbatchables = phase.unbatchable_mesh_values.get_mut(key).unwrap(); - for &(_, main_entity) in &unbatchables.entities { + for main_entity in unbatchables.entities.keys() { let Some(buffer_data) = - GFBD::get_binned_batch_data(&system_param_item, main_entity) + GFBD::get_binned_batch_data(&system_param_item, *main_entity) else { continue; }; diff --git a/crates/bevy_render/src/render_phase/mod.rs b/crates/bevy_render/src/render_phase/mod.rs index f8a2f7f3ad24d..092295f562084 100644 --- a/crates/bevy_render/src/render_phase/mod.rs +++ b/crates/bevy_render/src/render_phase/mod.rs @@ -30,18 +30,22 @@ mod rangefinder; use bevy_app::{App, Plugin}; use bevy_derive::{Deref, DerefMut}; +use bevy_ecs::component::Tick; +use bevy_ecs::entity::EntityHash; use bevy_platform_support::collections::{hash_map::Entry, HashMap}; use bevy_utils::default; pub use draw::*; pub use draw_state::*; use encase::{internal::WriteInto, ShaderSize}; +use fixedbitset::{Block, FixedBitSet}; +use indexmap::{IndexMap, IndexSet}; use nonmax::NonMaxU32; pub use rangefinder::*; use wgpu::Features; use crate::batching::gpu_preprocessing::{GpuPreprocessingMode, GpuPreprocessingSupport}; use crate::renderer::RenderDevice; -use crate::sync_world::MainEntity; +use crate::sync_world::{MainEntity, MainEntityHashMap}; use crate::view::RetainedViewEntity; use crate::{ batching::{ @@ -145,7 +149,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: Vec<(BPI::BatchSetKey, BPI::BinKey, (Entity, MainEntity))>, + pub non_mesh_items: HashMap<(BPI::BatchSetKey, BPI::BinKey), RenderBin>, /// Information on each batch set. /// @@ -158,6 +162,20 @@ where /// Multidrawable entities come first, then batchable entities, then /// unbatchable entities. pub(crate) batch_sets: BinnedRenderPhaseBatchSets, + + /// The batch and bin key for each entity. + /// + /// We retain these so that, when the entity changes, + /// [`Self::sweep_old_entities`] can quickly find the bin it was located in + /// and remove it. + cached_entity_bin_keys: IndexMap, EntityHash>, + + /// The set of indices in [`Self::cached_entity_bin_keys`] that are + /// confirmed to be up to date. + /// + /// Note that each bit in this bit set refers to an *index* in the + /// [`IndexMap`] (i.e. a bucket in the hash table). They aren't entity IDs. + valid_cached_entity_bin_keys: FixedBitSet, } /// All entities that share a mesh and a material and can be batched as part of @@ -165,7 +183,25 @@ where #[derive(Default)] pub struct RenderBin { /// A list of the entities in each bin. - pub entities: Vec<(Entity, MainEntity)>, + entities: IndexSet, +} + +/// Information that we keep about an entity currently within a bin. +pub struct CachedBinKey +where + BPI: BinnedPhaseItem, +{ + /// The key of the batch set containing the entity. + pub batch_set_key: BPI::BatchSetKey, + /// The key of the bin containing the entity. + pub bin_key: BPI::BinKey, + /// The type of render phase that we use to render the entity: multidraw, + /// plain batch, etc. + pub phase_type: BinnedRenderPhaseType, + /// The last modified tick of the entity. + /// + /// We use this to detect when the entity needs to be invalidated. + pub change_tick: Tick, } /// How we store and render the batch sets. @@ -233,7 +269,7 @@ pub struct BinnedRenderPhaseBatch { /// Information about the unbatchable entities in a bin. pub struct UnbatchableBinnedEntities { /// The entities. - pub entities: Vec<(Entity, MainEntity)>, + pub entities: MainEntityHashMap, /// The GPU array buffer indices of each unbatchable binned entity. pub(crate) buffer_indices: UnbatchableBinnedEntityIndexSet, @@ -337,13 +373,13 @@ impl ViewBinnedRenderPhases where BPI: BinnedPhaseItem, { - pub fn insert_or_clear( + pub fn prepare_for_new_frame( &mut self, retained_view_entity: RetainedViewEntity, gpu_preprocessing: GpuPreprocessingMode, ) { match self.entry(retained_view_entity) { - Entry::Occupied(mut entry) => entry.get_mut().clear(), + Entry::Occupied(mut entry) => entry.get_mut().prepare_for_new_frame(), Entry::Vacant(entry) => { entry.insert(BinnedRenderPhase::::new(gpu_preprocessing)); } @@ -366,6 +402,7 @@ where bin_key: BPI::BinKey, (entity, main_entity): (Entity, MainEntity), phase_type: BinnedRenderPhaseType, + change_tick: Tick, ) { match phase_type { BinnedRenderPhaseType::MultidrawableMesh => { @@ -373,20 +410,13 @@ where Entry::Occupied(mut entry) => { entry .get_mut() - .entry(bin_key) + .entry(bin_key.clone()) .or_default() - .entities - .push((entity, main_entity)); + .insert(main_entity); } Entry::Vacant(entry) => { - self.multidrawable_mesh_keys.push(batch_set_key); let mut new_batch_set = HashMap::default(); - new_batch_set.insert( - bin_key, - RenderBin { - entities: vec![(entity, main_entity)], - }, - ); + new_batch_set.insert(bin_key.clone(), RenderBin::from_entity(main_entity)); entry.insert(new_batch_set); } } @@ -398,13 +428,10 @@ where .entry((batch_set_key.clone(), bin_key.clone()).clone()) { Entry::Occupied(mut entry) => { - entry.get_mut().entities.push((entity, main_entity)); + entry.get_mut().insert(main_entity); } Entry::Vacant(entry) => { - self.batchable_mesh_keys.push((batch_set_key, bin_key)); - entry.insert(RenderBin { - entities: vec![(entity, main_entity)], - }); + entry.insert(RenderBin::from_entity(main_entity)); } } } @@ -415,12 +442,13 @@ where .entry((batch_set_key.clone(), bin_key.clone())) { Entry::Occupied(mut entry) => { - entry.get_mut().entities.push((entity, main_entity)); + entry.get_mut().entities.insert(main_entity, entity); } Entry::Vacant(entry) => { - self.unbatchable_mesh_keys.push((batch_set_key, bin_key)); + let mut entities = MainEntityHashMap::default(); + entities.insert(main_entity, entity); entry.insert(UnbatchableBinnedEntities { - entities: vec![(entity, main_entity)], + entities, buffer_indices: default(), }); } @@ -429,10 +457,34 @@ where BinnedRenderPhaseType::NonMesh => { // We don't process these items further. - self.non_mesh_items - .push((batch_set_key, bin_key, (entity, main_entity))); + match self + .non_mesh_items + .entry((batch_set_key.clone(), bin_key.clone()).clone()) + { + Entry::Occupied(mut entry) => { + entry.get_mut().insert(main_entity); + } + Entry::Vacant(entry) => { + entry.insert(RenderBin::from_entity(main_entity)); + } + } } } + + let index = self + .cached_entity_bin_keys + .insert_full( + main_entity, + CachedBinKey { + batch_set_key, + bin_key, + phase_type, + change_tick, + }, + ) + .0; + + self.valid_cached_entity_bin_keys.grow_and_insert(index); } /// Encodes the GPU commands needed to render all entities in this phase. @@ -589,7 +641,7 @@ where for (batch_set_key, bin_key) in &self.unbatchable_mesh_keys { let unbatchable_entities = &self.unbatchable_mesh_values[&(batch_set_key.clone(), bin_key.clone())]; - for (entity_index, &entity) in unbatchable_entities.entities.iter().enumerate() { + for (entity_index, entity) in unbatchable_entities.entities.iter().enumerate() { let unbatchable_dynamic_offset = match &unbatchable_entities.buffer_indices { UnbatchableBinnedEntityIndexSet::NoEntities => { // Shouldn't happen… @@ -622,7 +674,7 @@ where let binned_phase_item = BPI::new( batch_set_key.clone(), bin_key.clone(), - entity, + (*entity.1, *entity.0), unbatchable_dynamic_offset.instance_index ..(unbatchable_dynamic_offset.instance_index + 1), unbatchable_dynamic_offset.extra_index, @@ -652,44 +704,161 @@ where let draw_functions = world.resource::>(); let mut draw_functions = draw_functions.write(); - for &(ref batch_set_key, ref bin_key, entity) in &self.non_mesh_items { - // Come up with a fake batch range and extra index. The draw - // function is expected to manage any sort of batching logic itself. - let binned_phase_item = BPI::new( - batch_set_key.clone(), - bin_key.clone(), - entity, - 0..1, - PhaseItemExtraIndex::None, - ); + for ((batch_set_key, bin_key), bin) in &self.non_mesh_items { + for &entity in &bin.entities { + // Come up with a fake batch range and extra index. The draw + // function is expected to manage any sort of batching logic itself. + let binned_phase_item = BPI::new( + batch_set_key.clone(), + bin_key.clone(), + (Entity::PLACEHOLDER, entity), + 0..1, + PhaseItemExtraIndex::None, + ); - let Some(draw_function) = draw_functions.get_mut(binned_phase_item.draw_function()) - else { - continue; - }; + let Some(draw_function) = draw_functions.get_mut(binned_phase_item.draw_function()) + else { + continue; + }; - draw_function.draw(world, render_pass, view, &binned_phase_item)?; + draw_function.draw(world, render_pass, view, &binned_phase_item)?; + } } Ok(()) } pub fn is_empty(&self) -> bool { - self.multidrawable_mesh_keys.is_empty() - && self.batchable_mesh_keys.is_empty() - && self.unbatchable_mesh_keys.is_empty() + self.multidrawable_mesh_values.is_empty() + && self.batchable_mesh_values.is_empty() + && self.unbatchable_mesh_values.is_empty() && self.non_mesh_items.is_empty() } - pub fn clear(&mut self) { + pub fn prepare_for_new_frame(&mut self) { self.multidrawable_mesh_keys.clear(); - self.multidrawable_mesh_values.clear(); self.batchable_mesh_keys.clear(); - self.batchable_mesh_values.clear(); self.unbatchable_mesh_keys.clear(); - self.unbatchable_mesh_values.clear(); - self.non_mesh_items.clear(); self.batch_sets.clear(); + + self.valid_cached_entity_bin_keys.clear(); + self.valid_cached_entity_bin_keys + .grow(self.cached_entity_bin_keys.len()); + self.valid_cached_entity_bin_keys + .set_range(self.cached_entity_bin_keys.len().., true); + } + + /// Checks to see whether the entity is in a bin and returns true if it's + /// both in a bin and up to date. + /// + /// If this function returns true, we also add the entry to the + /// `valid_cached_entity_bin_keys` list. + pub fn validate_cached_entity( + &mut self, + visible_entity: MainEntity, + current_change_tick: Tick, + ) -> bool { + if let indexmap::map::Entry::Occupied(entry) = + self.cached_entity_bin_keys.entry(visible_entity) + { + if entry.get().change_tick == current_change_tick { + self.valid_cached_entity_bin_keys.insert(entry.index()); + return true; + } + } + + false + } + + /// Removes all entities not marked as clean from the bins. + /// + /// During `queue_material_meshes`, we process all visible entities and mark + /// each as clean as we come to it. Then we call this method, which removes + /// entities that aren't marked as clean from the bins. + pub fn sweep_old_entities(&mut self) { + // Search for entities not marked as valid. We have to do this in + // reverse order because `swap_remove_index` will potentially invalidate + // all indices after the one we remove. + for index in ReverseFixedBitSetZeroesIterator::new(&self.valid_cached_entity_bin_keys) { + // If we found an invalid entity, remove it. Note that this + // potentially invalidates later indices, but that's OK because + // we're going in reverse order. + let Some((entity, entity_bin_key)) = + self.cached_entity_bin_keys.swap_remove_index(index) + else { + continue; + }; + + // Remove the entity from the bin. If this makes the bin empty, + // remove the bin as well. + match entity_bin_key.phase_type { + BinnedRenderPhaseType::MultidrawableMesh => { + if let Entry::Occupied(mut batch_set_entry) = self + .multidrawable_mesh_values + .entry(entity_bin_key.batch_set_key.clone()) + { + if let Entry::Occupied(mut bin_entry) = batch_set_entry + .get_mut() + .entry(entity_bin_key.bin_key.clone()) + { + bin_entry.get_mut().remove(entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); + } + } + + // If the batch set is now empty, remove it. + if batch_set_entry.get_mut().is_empty() { + batch_set_entry.remove(); + } + } + } + + BinnedRenderPhaseType::BatchableMesh => { + if let Entry::Occupied(mut bin_entry) = self.batchable_mesh_values.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().remove(entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); + } + } + } + + BinnedRenderPhaseType::UnbatchableMesh => { + if let Entry::Occupied(mut bin_entry) = self.unbatchable_mesh_values.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().entities.remove(&entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().entities.is_empty() { + bin_entry.remove(); + } + } + } + + BinnedRenderPhaseType::NonMesh => { + if let Entry::Occupied(mut bin_entry) = self.non_mesh_items.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().remove(entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); + } + } + } + } + } } } @@ -705,7 +874,7 @@ where batchable_mesh_values: HashMap::default(), unbatchable_mesh_keys: vec![], unbatchable_mesh_values: HashMap::default(), - non_mesh_items: vec![], + non_mesh_items: HashMap::default(), batch_sets: match gpu_preprocessing { GpuPreprocessingMode::Culling => { BinnedRenderPhaseBatchSets::MultidrawIndirect(vec![]) @@ -715,6 +884,8 @@ where } GpuPreprocessingMode::None => BinnedRenderPhaseBatchSets::DynamicUniforms(vec![]), }, + cached_entity_bin_keys: IndexMap::default(), + valid_cached_entity_bin_keys: FixedBitSet::new(), } } } @@ -1357,3 +1528,127 @@ impl BinnedRenderPhaseType { } } } + +impl RenderBin { + /// Creates a [`RenderBin`] containing a single entity. + fn from_entity(entity: MainEntity) -> RenderBin { + let mut entities = IndexSet::default(); + entities.insert(entity); + RenderBin { entities } + } + + /// Inserts an entity into the bin. + fn insert(&mut self, entity: MainEntity) { + self.entities.insert(entity); + } + + /// Removes an entity from the bin. + fn remove(&mut self, entity_to_remove: MainEntity) { + self.entities.swap_remove(&entity_to_remove); + } + + /// Returns true if the bin contains no entities. + fn is_empty(&self) -> bool { + self.entities.is_empty() + } + + /// Returns the [`IndexSet`] containing all the entities in the bin. + #[inline] + pub fn entities(&self) -> &IndexSet { + &self.entities + } +} + +/// An iterator that efficiently finds the indices of all zero bits in a +/// [`FixedBitSet`] and returns them in reverse order. +/// +/// [`FixedBitSet`] doesn't natively offer this functionality, so we have to +/// implement it ourselves. +#[derive(Debug)] +struct ReverseFixedBitSetZeroesIterator<'a> { + /// The bit set. + bitset: &'a FixedBitSet, + /// The next bit index we're going to scan when [`Iterator::next`] is + /// called. + bit_index: isize, +} + +impl<'a> ReverseFixedBitSetZeroesIterator<'a> { + fn new(bitset: &'a FixedBitSet) -> ReverseFixedBitSetZeroesIterator<'a> { + ReverseFixedBitSetZeroesIterator { + bitset, + bit_index: (bitset.len() as isize) - 1, + } + } +} + +impl<'a> Iterator for ReverseFixedBitSetZeroesIterator<'a> { + type Item = usize; + + fn next(&mut self) -> Option { + while self.bit_index >= 0 { + // Unpack the bit index into block and bit. + let block_index = self.bit_index / (Block::BITS as isize); + let bit_pos = self.bit_index % (Block::BITS as isize); + + // Grab the block. Mask off all bits before the one we're scanning + // from by setting them all to 1. + let mut block = self.bitset.as_slice()[block_index as usize]; + if bit_pos + 1 < (Block::BITS as isize) { + block |= (!0) << (bit_pos + 1); + } + + // Search for the next unset bit. Note that the `leading_ones` + // function counts from the MSB to the LSB, so we need to flip it to + // get the bit number. + let pos = (Block::BITS as isize) - (block.leading_ones() as isize) - 1; + + // If we found an unset bit, return it. + if pos != -1 { + let result = block_index * (Block::BITS as isize) + pos; + self.bit_index = result - 1; + return Some(result as usize); + } + + // Otherwise, go to the previous block. + self.bit_index = block_index * (Block::BITS as isize) - 1; + } + + None + } +} + +#[cfg(test)] +mod test { + use super::ReverseFixedBitSetZeroesIterator; + use fixedbitset::FixedBitSet; + use proptest::{collection::vec, prop_assert_eq, proptest}; + + proptest! { + #[test] + fn reverse_fixed_bit_set_zeroes_iterator( + bits in vec(0usize..1024usize, 0usize..1024usize), + size in 0usize..1024usize, + ) { + // Build a random bit set. + let mut bitset = FixedBitSet::new(); + bitset.grow(size); + for bit in bits { + if bit < size { + bitset.set(bit, true); + } + } + + // Iterate over the bit set backwards in a naive way, and check that + // that iteration sequence corresponds to the optimized one. + let mut iter = ReverseFixedBitSetZeroesIterator::new(&bitset); + for bit_index in (0..size).rev() { + if !bitset.contains(bit_index) { + prop_assert_eq!(iter.next(), Some(bit_index)); + } + } + + prop_assert_eq!(iter.next(), None); + } + } +} diff --git a/crates/bevy_render/src/sync_world.rs b/crates/bevy_render/src/sync_world.rs index d22a27c72987b..3c9dc57ff8f40 100644 --- a/crates/bevy_render/src/sync_world.rs +++ b/crates/bevy_render/src/sync_world.rs @@ -153,7 +153,7 @@ unsafe impl TrustedEntityBorrow for RenderEntity {} /// Component added on the render world entities to keep track of the corresponding main world entity. /// /// Can also be used as a newtype wrapper for main world entities. -#[derive(Component, Deref, Copy, Clone, Debug, Eq, Hash, PartialEq)] +#[derive(Component, Deref, Copy, Clone, Debug, Eq, Hash, PartialEq, PartialOrd, Ord)] pub struct MainEntity(Entity); impl MainEntity { #[inline] diff --git a/crates/bevy_sprite/src/mesh2d/material.rs b/crates/bevy_sprite/src/mesh2d/material.rs index 745dee2f2ecd1..4b5935ca5b01d 100644 --- a/crates/bevy_sprite/src/mesh2d/material.rs +++ b/crates/bevy_sprite/src/mesh2d/material.rs @@ -754,12 +754,20 @@ pub fn queue_material2d_meshes( }; for (render_entity, visible_entity) in visible_entities.iter::() { - let Some(pipeline_id) = specialized_material_pipeline_cache + let Some((current_change_tick, pipeline_id)) = specialized_material_pipeline_cache .get(&(*view_entity, *visible_entity)) - .map(|(_, pipeline_id)| *pipeline_id) + .map(|(current_change_tick, pipeline_id)| (*current_change_tick, *pipeline_id)) else { continue; }; + + // Skip the entity if it's cached in a bin and up to date. + if opaque_phase.validate_cached_entity(*visible_entity, current_change_tick) + || alpha_mask_phase.validate_cached_entity(*visible_entity, current_change_tick) + { + continue; + } + let Some(material_asset_id) = render_material_instances.get(visible_entity) else { continue; }; @@ -802,6 +810,7 @@ pub fn queue_material2d_meshes( bin_key, (*render_entity, *visible_entity), binned_render_phase_type, + current_change_tick, ); } AlphaMode2d::Mask(_) => { @@ -818,6 +827,7 @@ pub fn queue_material2d_meshes( bin_key, (*render_entity, *visible_entity), binned_render_phase_type, + current_change_tick, ); } AlphaMode2d::Blend => { @@ -838,6 +848,10 @@ pub fn queue_material2d_meshes( } } } + + // Remove invalid entities from the bins. + opaque_phase.sweep_old_entities(); + alpha_mask_phase.sweep_old_entities(); } } diff --git a/examples/shader/custom_phase_item.rs b/examples/shader/custom_phase_item.rs index be4fb105f834e..2ba3fab707a79 100644 --- a/examples/shader/custom_phase_item.rs +++ b/examples/shader/custom_phase_item.rs @@ -10,6 +10,7 @@ use bevy::{ core_pipeline::core_3d::{Opaque3d, Opaque3dBatchSetKey, Opaque3dBinKey, CORE_3D_DEPTH_FORMAT}, ecs::{ + component::Tick, query::ROQueryItem, system::{lifetimeless::SRes, SystemParamItem}, }, @@ -224,6 +225,7 @@ fn queue_custom_phase_item( opaque_draw_functions: Res>, mut specialized_render_pipelines: ResMut>, views: Query<(&ExtractedView, &RenderVisibleEntities, &Msaa)>, + mut next_tick: Local, ) { let draw_custom_phase_item = opaque_draw_functions .read() @@ -250,6 +252,10 @@ fn queue_custom_phase_item( *msaa, ); + // Bump the change tick in order to force Bevy to rebuild the bin. + let this_tick = next_tick.get() + 1; + next_tick.set(this_tick); + // Add the custom render item. We use the // [`BinnedRenderPhaseType::NonMesh`] type to skip the special // handling that Bevy has for meshes (preprocessing, indirect @@ -272,6 +278,7 @@ fn queue_custom_phase_item( }, entity, BinnedRenderPhaseType::NonMesh, + *next_tick, ); } } diff --git a/examples/shader/specialized_mesh_pipeline.rs b/examples/shader/specialized_mesh_pipeline.rs index 060799160179b..12adfe9822e02 100644 --- a/examples/shader/specialized_mesh_pipeline.rs +++ b/examples/shader/specialized_mesh_pipeline.rs @@ -8,7 +8,7 @@ use bevy::{ core_pipeline::core_3d::{Opaque3d, Opaque3dBatchSetKey, Opaque3dBinKey, CORE_3D_DEPTH_FORMAT}, - ecs::system::StaticSystemParam, + ecs::{component::Tick, system::StaticSystemParam}, math::{vec3, vec4}, pbr::{ DrawMesh, MeshPipeline, MeshPipelineKey, MeshPipelineViewLayoutKey, RenderMeshInstances, @@ -298,6 +298,7 @@ fn queue_custom_mesh_pipeline( >, >, mut indirect_parameters_buffers: ResMut, + mut change_tick: Local, ) { let system_param_item = param.into_inner(); @@ -400,6 +401,10 @@ fn queue_custom_mesh_pipeline( // can fail you need to handle the error here .expect("Failed to specialize mesh pipeline"); + // Bump the change tick so that Bevy is forced to rebuild the bin. + let next_change_tick = change_tick.get() + 1; + change_tick.set(next_change_tick); + // Add the mesh with our specialized pipeline opaque_phase.add( Opaque3dBatchSetKey { @@ -420,6 +425,7 @@ fn queue_custom_mesh_pipeline( // This example supports batching, but if your pipeline doesn't // support it you can use `BinnedRenderPhaseType::UnbatchableMesh` BinnedRenderPhaseType::BatchableMesh, + *change_tick, ); // Create a *work item*. A work item tells the Bevy renderer to From dbb7426183808e8e890e1322b1daacd0a89b43fd Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Wed, 5 Feb 2025 19:16:00 -0800 Subject: [PATCH 2/5] "before" -> "above" --- crates/bevy_render/src/render_phase/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_render/src/render_phase/mod.rs b/crates/bevy_render/src/render_phase/mod.rs index 092295f562084..61f027e8a0c68 100644 --- a/crates/bevy_render/src/render_phase/mod.rs +++ b/crates/bevy_render/src/render_phase/mod.rs @@ -1591,7 +1591,7 @@ impl<'a> Iterator for ReverseFixedBitSetZeroesIterator<'a> { let block_index = self.bit_index / (Block::BITS as isize); let bit_pos = self.bit_index % (Block::BITS as isize); - // Grab the block. Mask off all bits before the one we're scanning + // Grab the block. Mask off all bits above the one we're scanning // from by setting them all to 1. let mut block = self.bitset.as_slice()[block_index as usize]; if bit_pos + 1 < (Block::BITS as isize) { From 90fdd40a038225557b98f46f8f41c318fbaa0a00 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 6 Feb 2025 12:23:04 -0800 Subject: [PATCH 3/5] Fix `deferred_rendering` --- crates/bevy_pbr/src/prepass/mod.rs | 26 ++- crates/bevy_render/src/render_phase/mod.rs | 230 +++++++++++++++------ 2 files changed, 182 insertions(+), 74 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index b4e092bf95532..337234480d99d 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -1096,10 +1096,14 @@ pub fn queue_prepass_material_meshes( }; // Skip the entity if it's cached in a bin and up to date. - if opaque_phase.as_mut().is_some_and(|opaque_phase| { - opaque_phase.validate_cached_entity(*visible_entity, *current_change_tick) - }) || alpha_mask_phase.as_mut().is_some_and(|alpha_mask_phase| { - alpha_mask_phase.validate_cached_entity(*visible_entity, *current_change_tick) + if opaque_phase.as_mut().is_some_and(|phase| { + phase.validate_cached_entity(*visible_entity, *current_change_tick) + }) || alpha_mask_phase.as_mut().is_some_and(|phase| { + phase.validate_cached_entity(*visible_entity, *current_change_tick) + }) || opaque_deferred_phase.as_mut().is_some_and(|phase| { + phase.validate_cached_entity(*visible_entity, *current_change_tick) + }) || alpha_mask_deferred_phase.as_mut().is_some_and(|phase| { + phase.validate_cached_entity(*visible_entity, *current_change_tick) }) { continue; } @@ -1226,11 +1230,17 @@ pub fn queue_prepass_material_meshes( } // Remove invalid entities from the bins. - if let Some(opaque_phase) = opaque_phase { - opaque_phase.sweep_old_entities(); + if let Some(phase) = opaque_phase { + phase.sweep_old_entities(); } - if let Some(alpha_mask_phase) = alpha_mask_phase { - alpha_mask_phase.sweep_old_entities(); + if let Some(phase) = alpha_mask_phase { + phase.sweep_old_entities(); + } + if let Some(phase) = opaque_deferred_phase { + phase.sweep_old_entities(); + } + if let Some(phase) = alpha_mask_deferred_phase { + phase.sweep_old_entities(); } } } diff --git a/crates/bevy_render/src/render_phase/mod.rs b/crates/bevy_render/src/render_phase/mod.rs index 61f027e8a0c68..f7c18eac0e145 100644 --- a/crates/bevy_render/src/render_phase/mod.rs +++ b/crates/bevy_render/src/render_phase/mod.rs @@ -176,6 +176,15 @@ where /// Note that each bit in this bit set refers to an *index* in the /// [`IndexMap`] (i.e. a bucket in the hash table). They aren't entity IDs. valid_cached_entity_bin_keys: FixedBitSet, + + /// The set of entities that changed bins this frame. + /// + /// An entity will only be present in this list if it was in one bin on the + /// previous frame and is in a new bin on this frame. Each list entry + /// specifies the bin the entity used to be in. We use this in order to + /// remove the entity from the old bin during + /// [`BinnedRenderPhase::sweep_old_entities`]. + entities_that_changed_bins: Vec>, } /// All entities that share a mesh and a material and can be batched as part of @@ -186,6 +195,18 @@ pub struct RenderBin { entities: IndexSet, } +/// Information that we track about an entity that was in one bin on the +/// previous frame and is in a different bin this frame. +struct EntityThatChangedBins +where + BPI: BinnedPhaseItem, +{ + /// The entity. + main_entity: MainEntity, + /// The key that identifies the bin that this entity used to be in. + old_bin_key: CachedBinKey, +} + /// Information that we keep about an entity currently within a bin. pub struct CachedBinKey where @@ -204,6 +225,32 @@ where pub change_tick: Tick, } +impl Clone for CachedBinKey +where + BPI: BinnedPhaseItem, +{ + fn clone(&self) -> Self { + CachedBinKey { + batch_set_key: self.batch_set_key.clone(), + bin_key: self.bin_key.clone(), + phase_type: self.phase_type, + change_tick: self.change_tick, + } + } +} + +impl PartialEq for CachedBinKey +where + BPI: BinnedPhaseItem, +{ + fn eq(&self, other: &Self) -> bool { + self.batch_set_key == other.batch_set_key + && self.bin_key == other.bin_key + && self.phase_type == other.phase_type + && self.change_tick == other.change_tick + } +} + /// How we store and render the batch sets. /// /// Each one of these corresponds to a [`GpuPreprocessingMode`]. @@ -471,19 +518,29 @@ where } } - let index = self + let new_bin_key = CachedBinKey { + batch_set_key, + bin_key, + phase_type, + change_tick, + }; + + let (index, old_bin_key) = self .cached_entity_bin_keys - .insert_full( - main_entity, - CachedBinKey { - batch_set_key, - bin_key, - phase_type, - change_tick, - }, - ) - .0; + .insert_full(main_entity, new_bin_key.clone()); + + // If the entity changed bins, record its old bin so that we can remove + // the entity from it. + if let Some(old_bin_key) = old_bin_key { + if old_bin_key != new_bin_key { + self.entities_that_changed_bins.push(EntityThatChangedBins { + main_entity, + old_bin_key, + }); + } + } + // Mark the entity as valid. self.valid_cached_entity_bin_keys.grow_and_insert(index); } @@ -746,6 +803,8 @@ where .grow(self.cached_entity_bin_keys.len()); self.valid_cached_entity_bin_keys .set_range(self.cached_entity_bin_keys.len().., true); + + self.entities_that_changed_bins.clear(); } /// Checks to see whether the entity is in a bin and returns true if it's @@ -789,73 +848,111 @@ where continue; }; - // Remove the entity from the bin. If this makes the bin empty, - // remove the bin as well. - match entity_bin_key.phase_type { - BinnedRenderPhaseType::MultidrawableMesh => { - if let Entry::Occupied(mut batch_set_entry) = self - .multidrawable_mesh_values - .entry(entity_bin_key.batch_set_key.clone()) - { - if let Entry::Occupied(mut bin_entry) = batch_set_entry - .get_mut() - .entry(entity_bin_key.bin_key.clone()) - { - bin_entry.get_mut().remove(entity); + 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.non_mesh_items, + ); + } - // If the bin is now empty, remove the bin. - if bin_entry.get_mut().is_empty() { - bin_entry.remove(); - } - } + // If an entity changed bins, we need to remove it from its old bin. + for entity_that_changed_bins in self.entities_that_changed_bins.drain(..) { + 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.non_mesh_items, + ); + } + } +} - // If the batch set is now empty, remove it. - if batch_set_entry.get_mut().is_empty() { - batch_set_entry.remove(); - } +/// Removes an entity from a bin. +/// +/// If this makes the bin empty, this function removes the bin as well. +/// +/// This is a standalone function instead of a method on [`BinnedRenderPhase`] +/// for borrow check reasons. +fn remove_entity_from_bin( + entity: MainEntity, + entity_bin_key: &CachedBinKey, + multidrawable_mesh_values: &mut HashMap>, + 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>, +) 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 Entry::Occupied(mut bin_entry) = batch_set_entry + .get_mut() + .entry(entity_bin_key.bin_key.clone()) + { + bin_entry.get_mut().remove(entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); } } - BinnedRenderPhaseType::BatchableMesh => { - if let Entry::Occupied(mut bin_entry) = self.batchable_mesh_values.entry(( - entity_bin_key.batch_set_key.clone(), - entity_bin_key.bin_key.clone(), - )) { - bin_entry.get_mut().remove(entity); + // If the batch set is now empty, remove it. + if batch_set_entry.get_mut().is_empty() { + batch_set_entry.remove(); + } + } + } - // If the bin is now empty, remove the bin. - if bin_entry.get_mut().is_empty() { - bin_entry.remove(); - } - } + BinnedRenderPhaseType::BatchableMesh => { + if let Entry::Occupied(mut bin_entry) = batchable_mesh_values.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().remove(entity); + + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); } + } + } - BinnedRenderPhaseType::UnbatchableMesh => { - if let Entry::Occupied(mut bin_entry) = self.unbatchable_mesh_values.entry(( - entity_bin_key.batch_set_key.clone(), - entity_bin_key.bin_key.clone(), - )) { - bin_entry.get_mut().entities.remove(&entity); + BinnedRenderPhaseType::UnbatchableMesh => { + if let Entry::Occupied(mut bin_entry) = unbatchable_mesh_values.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().entities.remove(&entity); - // If the bin is now empty, remove the bin. - if bin_entry.get_mut().entities.is_empty() { - bin_entry.remove(); - } - } + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().entities.is_empty() { + bin_entry.remove(); } + } + } - BinnedRenderPhaseType::NonMesh => { - if let Entry::Occupied(mut bin_entry) = self.non_mesh_items.entry(( - entity_bin_key.batch_set_key.clone(), - entity_bin_key.bin_key.clone(), - )) { - bin_entry.get_mut().remove(entity); + BinnedRenderPhaseType::NonMesh => { + if let Entry::Occupied(mut bin_entry) = non_mesh_items.entry(( + entity_bin_key.batch_set_key.clone(), + entity_bin_key.bin_key.clone(), + )) { + bin_entry.get_mut().remove(entity); - // If the bin is now empty, remove the bin. - if bin_entry.get_mut().is_empty() { - bin_entry.remove(); - } - } + // If the bin is now empty, remove the bin. + if bin_entry.get_mut().is_empty() { + bin_entry.remove(); } } } @@ -886,6 +983,7 @@ where }, cached_entity_bin_keys: IndexMap::default(), valid_cached_entity_bin_keys: FixedBitSet::new(), + entities_that_changed_bins: vec![], } } } From a287a6d07883a685bbbf3e025a794fad72de46bd Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Thu, 6 Feb 2025 13:30:13 -0800 Subject: [PATCH 4/5] Unbreak shadows --- crates/bevy_render/src/render_phase/mod.rs | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/crates/bevy_render/src/render_phase/mod.rs b/crates/bevy_render/src/render_phase/mod.rs index f7c18eac0e145..e9baccf141dbe 100644 --- a/crates/bevy_render/src/render_phase/mod.rs +++ b/crates/bevy_render/src/render_phase/mod.rs @@ -239,18 +239,6 @@ where } } -impl PartialEq for CachedBinKey -where - BPI: BinnedPhaseItem, -{ - fn eq(&self, other: &Self) -> bool { - self.batch_set_key == other.batch_set_key - && self.bin_key == other.bin_key - && self.phase_type == other.phase_type - && self.change_tick == other.change_tick - } -} - /// How we store and render the batch sets. /// /// Each one of these corresponds to a [`GpuPreprocessingMode`]. @@ -532,7 +520,10 @@ where // If the entity changed bins, record its old bin so that we can remove // the entity from it. if let Some(old_bin_key) = old_bin_key { - if old_bin_key != new_bin_key { + if old_bin_key.batch_set_key != new_bin_key.batch_set_key + || old_bin_key.bin_key != new_bin_key.bin_key + || old_bin_key.phase_type != new_bin_key.phase_type + { self.entities_that_changed_bins.push(EntityThatChangedBins { main_entity, old_bin_key, From 1d27b20eef5625ec7944d3d3b08026d7dd7c37eb Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Fri, 7 Feb 2025 13:40:44 -0800 Subject: [PATCH 5/5] Fix 2D meshes --- crates/bevy_sprite/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/bevy_sprite/src/lib.rs b/crates/bevy_sprite/src/lib.rs index e68dc6eae4991..568701725d605 100644 --- a/crates/bevy_sprite/src/lib.rs +++ b/crates/bevy_sprite/src/lib.rs @@ -38,10 +38,11 @@ pub use texture_slice::*; use bevy_app::prelude::*; use bevy_asset::{load_internal_asset, weak_handle, AssetEvents, Assets, Handle}; -use bevy_core_pipeline::core_2d::Transparent2d; +use bevy_core_pipeline::core_2d::{AlphaMask2d, Opaque2d, Transparent2d}; use bevy_ecs::prelude::*; use bevy_image::{prelude::*, TextureAtlasPlugin}; use bevy_render::{ + batching::sort_binned_render_phase, mesh::{Mesh, Mesh2d, MeshAabb}, primitives::Aabb, render_phase::AddRenderCommand, @@ -151,6 +152,8 @@ impl Plugin for SpritePlugin { .ambiguous_with(queue_material2d_meshes::), prepare_sprite_image_bind_groups.in_set(RenderSet::PrepareBindGroups), prepare_sprite_view_bind_groups.in_set(RenderSet::PrepareBindGroups), + sort_binned_render_phase::.in_set(RenderSet::PhaseSort), + sort_binned_render_phase::.in_set(RenderSet::PhaseSort), ), ); };