diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index 3be5d6f2f9a8d..0f68411eba48a 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -384,7 +384,8 @@ impl Plugin for PbrPlugin { .in_set(SimulationLightSystems::AssignLightsToClusters) .after(TransformSystem::TransformPropagate) .after(VisibilitySystems::CheckVisibility) - .after(CameraUpdateSystem), + .after(CameraUpdateSystem) + .ambiguous_with(VisibilitySystems::VisibilityChangeDetect), clear_directional_light_cascades .in_set(SimulationLightSystems::UpdateDirectionalLightCascades) .after(TransformSystem::TransformPropagate) @@ -398,7 +399,8 @@ impl Plugin for PbrPlugin { // We assume that no entity will be both a directional light and a spot light, // so these systems will run independently of one another. // FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481. - .ambiguous_with(update_spot_light_frusta), + .ambiguous_with(update_spot_light_frusta) + .ambiguous_with(VisibilitySystems::VisibilityChangeDetect), update_point_light_frusta .in_set(SimulationLightSystems::UpdateLightFrusta) .after(TransformSystem::TransformPropagate) @@ -419,7 +421,8 @@ impl Plugin for PbrPlugin { // NOTE: This MUST be scheduled AFTER the core renderer visibility check // because that resets entity `ViewVisibility` for the first view // which would override any results from this otherwise - .after(VisibilitySystems::CheckVisibility), + .after(VisibilitySystems::CheckVisibility) + .ambiguous_with(VisibilitySystems::VisibilityChangeDetect), ), ); diff --git a/crates/bevy_pbr/src/render/gpu_preprocess.rs b/crates/bevy_pbr/src/render/gpu_preprocess.rs index 2350b8c7043e5..c4381c010637a 100644 --- a/crates/bevy_pbr/src/render/gpu_preprocess.rs +++ b/crates/bevy_pbr/src/render/gpu_preprocess.rs @@ -404,8 +404,8 @@ pub fn prepare_preprocess_bind_groups( } = batched_instance_buffers.into_inner(); let (Some(current_input_buffer), Some(previous_input_buffer), Some(data_buffer)) = ( - current_input_buffer_vec.buffer(), - previous_input_buffer_vec.buffer(), + current_input_buffer_vec.buffer.buffer(), + previous_input_buffer_vec.buffer.buffer(), data_buffer_vec.buffer(), ) else { return; @@ -483,5 +483,4 @@ pub fn write_mesh_culling_data_buffer( mut mesh_culling_data_buffer: ResMut, ) { mesh_culling_data_buffer.write_buffer(&render_device, &render_queue); - mesh_culling_data_buffer.clear(); } diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 2080a143c3b84..996dfe7e2b5e2 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -1,4 +1,4 @@ -use core::mem::{self, size_of}; +use core::mem::size_of; use crate::material_bind_groups::{MaterialBindGroupIndex, MaterialBindGroupSlot}; use allocator::MeshAllocator; @@ -21,6 +21,7 @@ use bevy_render::{ batching::{ gpu_preprocessing::{ self, GpuPreprocessingSupport, IndirectParameters, IndirectParametersBuffer, + InstanceInputUniformBuffer, }, no_gpu_preprocessing, GetBatchData, GetFullBatchData, NoAutomaticBatching, }, @@ -43,8 +44,9 @@ use bevy_render::{ }; use bevy_transform::components::GlobalTransform; use bevy_utils::{ + hashbrown::hash_map::Entry, tracing::{error, warn}, - Entry, HashMap, Parallel, + HashMap, Parallel, }; use material_bind_groups::MaterialBindingId; @@ -317,7 +319,7 @@ pub struct MeshUniform { /// the full [`MeshUniform`]. /// /// This is essentially a subset of the fields in [`MeshUniform`] above. -#[derive(ShaderType, Pod, Zeroable, Clone, Copy)] +#[derive(ShaderType, Pod, Zeroable, Clone, Copy, Default, Debug)] #[repr(C)] pub struct MeshInputUniform { /// Affine 4x3 matrix transposed to 3x4. @@ -358,7 +360,7 @@ pub struct MeshInputUniform { /// Information about each mesh instance needed to cull it on GPU. /// /// This consists of its axis-aligned bounding box (AABB). -#[derive(ShaderType, Pod, Zeroable, Clone, Copy)] +#[derive(ShaderType, Pod, Zeroable, Clone, Copy, Default)] #[repr(C)] pub struct MeshCullingData { /// The 3D center of the AABB in model space, padded with an extra unused @@ -563,11 +565,25 @@ pub enum RenderMeshInstanceGpuQueue { /// The version of [`RenderMeshInstanceGpuQueue`] that omits the /// [`MeshCullingData`], so that we don't waste space when GPU /// culling is disabled. - CpuCulling(Vec<(MainEntity, RenderMeshInstanceGpuBuilder)>), + CpuCulling { + /// Stores GPU data for each entity that became visible or changed in + /// such a way that necessitates updating the [`MeshInputUniform`] (e.g. + /// changed transform). + changed: Vec<(MainEntity, RenderMeshInstanceGpuBuilder)>, + /// Stores the IDs of entities that became invisible this frame. + removed: Vec, + }, /// The version of [`RenderMeshInstanceGpuQueue`] that contains the /// [`MeshCullingData`], used when any view has GPU culling /// enabled. - GpuCulling(Vec<(MainEntity, RenderMeshInstanceGpuBuilder, MeshCullingData)>), + GpuCulling { + /// Stores GPU data for each entity that became visible or changed in + /// such a way that necessitates updating the [`MeshInputUniform`] (e.g. + /// changed transform). + changed: Vec<(MainEntity, RenderMeshInstanceGpuBuilder, MeshCullingData)>, + /// Stores the IDs of entities that became invisible this frame. + removed: Vec, + }, } /// The per-thread queues containing mesh instances, populated during the @@ -636,6 +652,22 @@ pub struct RenderMeshInstancesCpu(MainEntityHashMap); #[derive(Default, Deref, DerefMut)] pub struct RenderMeshInstancesGpu(MainEntityHashMap); +/// The result of an entity removal. +/// +/// We want the mesh input uniform array to have no gaps in it in order to +/// simplify the GPU logic. Therefore, whenever entity data is removed from the +/// [`MeshInputUniform`] buffer, the last entity is swapped in to fill it. If +/// culling is enabled, the mesh culling data corresponding to an entity must +/// have the same indices as the mesh input uniforms for that entity. Thus we +/// need to pass this information to [`MeshCullingDataBuffer::remove`] so that +/// it can update its buffer to match the [`MeshInputUniform`] buffer. +struct RemovedMeshInputUniformIndices { + /// The index of the mesh input that was removed. + removed_index: usize, + /// The index of the mesh input that was moved to fill its place. + moved_index: usize, +} + /// Maps each mesh instance to the material ID, and allocated binding ID, /// associated with that mesh instance. #[derive(Resource, Default)] @@ -746,10 +778,26 @@ impl RenderMeshInstanceGpuQueue { /// enabled. fn init(&mut self, any_gpu_culling: bool) { match (any_gpu_culling, &mut *self) { - (true, RenderMeshInstanceGpuQueue::GpuCulling(queue)) => queue.clear(), - (true, _) => *self = RenderMeshInstanceGpuQueue::GpuCulling(vec![]), - (false, RenderMeshInstanceGpuQueue::CpuCulling(queue)) => queue.clear(), - (false, _) => *self = RenderMeshInstanceGpuQueue::CpuCulling(vec![]), + (true, RenderMeshInstanceGpuQueue::GpuCulling { changed, removed }) => { + changed.clear(); + removed.clear(); + } + (true, _) => { + *self = RenderMeshInstanceGpuQueue::GpuCulling { + changed: vec![], + removed: vec![], + } + } + (false, RenderMeshInstanceGpuQueue::CpuCulling { changed, removed }) => { + changed.clear(); + removed.clear(); + } + (false, _) => { + *self = RenderMeshInstanceGpuQueue::CpuCulling { + changed: vec![], + removed: vec![], + } + } } } @@ -761,24 +809,59 @@ impl RenderMeshInstanceGpuQueue { culling_data_builder: Option, ) { match (&mut *self, culling_data_builder) { - (&mut RenderMeshInstanceGpuQueue::CpuCulling(ref mut queue), None) => { + ( + &mut RenderMeshInstanceGpuQueue::CpuCulling { + changed: ref mut queue, + .. + }, + None, + ) => { queue.push((entity, instance_builder)); } ( - &mut RenderMeshInstanceGpuQueue::GpuCulling(ref mut queue), + &mut RenderMeshInstanceGpuQueue::GpuCulling { + changed: ref mut queue, + .. + }, Some(culling_data_builder), ) => { queue.push((entity, instance_builder, culling_data_builder)); } (_, None) => { - *self = RenderMeshInstanceGpuQueue::CpuCulling(vec![(entity, instance_builder)]); + *self = RenderMeshInstanceGpuQueue::CpuCulling { + changed: vec![(entity, instance_builder)], + removed: vec![], + }; } (_, Some(culling_data_builder)) => { - *self = RenderMeshInstanceGpuQueue::GpuCulling(vec![( - entity, - instance_builder, - culling_data_builder, - )]); + *self = RenderMeshInstanceGpuQueue::GpuCulling { + changed: vec![(entity, instance_builder, culling_data_builder)], + removed: vec![], + }; + } + } + } + + /// Adds the given entity to the `removed` list, queuing it for removal. + /// + /// The `gpu_culling` parameter specifies whether GPU culling is enabled. + fn remove(&mut self, entity: MainEntity, gpu_culling: bool) { + match (&mut *self, gpu_culling) { + (RenderMeshInstanceGpuQueue::None, false) => { + *self = RenderMeshInstanceGpuQueue::CpuCulling { + changed: vec![], + removed: vec![entity], + } + } + (RenderMeshInstanceGpuQueue::None, true) => { + *self = RenderMeshInstanceGpuQueue::GpuCulling { + changed: vec![], + removed: vec![entity], + } + } + (RenderMeshInstanceGpuQueue::CpuCulling { removed, .. }, _) + | (RenderMeshInstanceGpuQueue::GpuCulling { removed, .. }, _) => { + removed.push(entity); } } } @@ -786,12 +869,13 @@ impl RenderMeshInstanceGpuQueue { impl RenderMeshInstanceGpuBuilder { /// Flushes this mesh instance to the [`RenderMeshInstanceGpu`] and - /// [`MeshInputUniform`] tables. - fn add_to( + /// [`MeshInputUniform`] tables, replacing the existing entry if applicable. + fn update( self, entity: MainEntity, render_mesh_instances: &mut MainEntityHashMap, - current_input_buffer: &mut RawBufferVec, + current_input_buffer: &mut InstanceInputUniformBuffer, + previous_input_buffer: &mut InstanceInputUniformBuffer, mesh_allocator: &MeshAllocator, ) -> usize { let first_vertex_index = match mesh_allocator.mesh_vertex_slice(&self.shared.mesh_asset_id) @@ -800,37 +884,103 @@ impl RenderMeshInstanceGpuBuilder { None => 0, }; - // Push the mesh input uniform. - let current_uniform_index = current_input_buffer.push(MeshInputUniform { + // Create the mesh input uniform. + let mut mesh_input_uniform = MeshInputUniform { world_from_local: self.world_from_local.to_transpose(), lightmap_uv_rect: self.lightmap_uv_rect, flags: self.mesh_flags.bits(), - previous_input_index: match self.previous_input_index { - Some(previous_input_index) => previous_input_index.into(), - None => u32::MAX, - }, + previous_input_index: u32::MAX, first_vertex_index, material_bind_group_slot: *self.shared.material_bindings_index.slot, pad_a: 0, pad_b: 0, - }); + }; - // Record the [`RenderMeshInstance`]. - render_mesh_instances.insert( - entity, - RenderMeshInstanceGpu { - translation: self.world_from_local.translation, - shared: self.shared, - current_uniform_index: (current_uniform_index as u32) - .try_into() - .unwrap_or_default(), - }, - ); + // Did the last frame contain this entity as well? + let current_uniform_index; + match render_mesh_instances.entry(entity) { + Entry::Occupied(mut occupied_entry) => { + // Yes, it did. Replace its entry with the new one. + + // Reserve a slot. + current_uniform_index = + u32::from(occupied_entry.get_mut().current_uniform_index) as usize; + + // Save the old mesh input uniform. The mesh preprocessing + // shader will need it to compute motion vectors. + let previous_mesh_input_uniform = + current_input_buffer.buffer.values()[current_uniform_index]; + let previous_input_index = previous_input_buffer + .buffer + .push(previous_mesh_input_uniform); + previous_input_buffer.main_entities.push(entity); + mesh_input_uniform.previous_input_index = previous_input_index as u32; + + // Write in the new mesh input uniform. + current_input_buffer.buffer.values_mut()[current_uniform_index] = + mesh_input_uniform; + + occupied_entry.replace_entry(RenderMeshInstanceGpu { + translation: self.world_from_local.translation, + shared: self.shared, + current_uniform_index: NonMaxU32::new(current_uniform_index as u32) + .unwrap_or_default(), + }); + } + + Entry::Vacant(vacant_entry) => { + // No, this is a new entity. Push its data on to the buffer. + current_uniform_index = current_input_buffer.buffer.push(mesh_input_uniform); + current_input_buffer.main_entities.push(entity); + vacant_entry.insert(RenderMeshInstanceGpu { + translation: self.world_from_local.translation, + shared: self.shared, + current_uniform_index: NonMaxU32::new(current_uniform_index as u32) + .unwrap_or_default(), + }); + } + } current_uniform_index } } +/// Removes a [`MeshInputUniform`] corresponding to an entity that became +/// invisible from the buffer. +fn remove_mesh_input_uniform( + entity: MainEntity, + render_mesh_instances: &mut MainEntityHashMap, + current_input_buffer: &mut InstanceInputUniformBuffer, +) -> Option { + // Remove the uniform data. + let removed_render_mesh_instance = render_mesh_instances.remove(&entity)?; + let removed_uniform_index = removed_render_mesh_instance.current_uniform_index.get() as usize; + + // Now *move* the final mesh uniform to fill its place in the buffer, so + // that the buffer remains contiguous. + let moved_uniform_index = current_input_buffer.buffer.len() - 1; + if moved_uniform_index != removed_uniform_index { + let moved_uniform = current_input_buffer.buffer.pop().unwrap(); + let moved_entity = current_input_buffer.main_entities.pop().unwrap(); + + current_input_buffer.buffer.values_mut()[removed_uniform_index] = moved_uniform; + + if let Some(moved_render_mesh_instance) = render_mesh_instances.get_mut(&moved_entity) { + moved_render_mesh_instance.current_uniform_index = + NonMaxU32::new(removed_uniform_index as u32).unwrap_or_default(); + } + } else { + current_input_buffer.buffer.pop(); + current_input_buffer.main_entities.pop(); + } + + // Tell our caller what we did. + Some(RemovedMeshInputUniformIndices { + removed_index: removed_uniform_index, + moved_index: moved_uniform_index, + }) +} + impl MeshCullingData { /// Returns a new [`MeshCullingData`] initialized with the given AABB. /// @@ -850,9 +1000,16 @@ impl MeshCullingData { } /// Flushes this mesh instance culling data to the - /// [`MeshCullingDataBuffer`]. - fn add_to(&self, mesh_culling_data_buffer: &mut MeshCullingDataBuffer) -> usize { - mesh_culling_data_buffer.push(*self) + /// [`MeshCullingDataBuffer`], replacing the existing entry if applicable. + fn update( + &self, + mesh_culling_data_buffer: &mut MeshCullingDataBuffer, + instance_data_index: usize, + ) { + while mesh_culling_data_buffer.len() < instance_data_index + 1 { + mesh_culling_data_buffer.push(MeshCullingData::default()); + } + mesh_culling_data_buffer.values_mut()[instance_data_index] = *self; } } @@ -863,6 +1020,26 @@ impl Default for MeshCullingDataBuffer { } } +impl MeshCullingDataBuffer { + /// Updates the culling data buffer after a mesh has been removed from the scene. + /// + /// [`remove_mesh_input_uniform`] removes a mesh input uniform from the + /// buffer, swapping in the last uniform to fill the space if necessary. The + /// index of each mesh in the mesh culling data and that index of the mesh + /// in the mesh input uniform buffer must match. Thus we have to perform the + /// corresponding operation here too. [`remove_mesh_input_uniform`] returns + /// a [`RemovedMeshInputUniformIndices`] value to tell us what to do, and + /// this function is where we process that value. + fn remove(&mut self, removed_indices: RemovedMeshInputUniformIndices) { + if removed_indices.moved_index != removed_indices.removed_index { + let moved_culling_data = self.pop().unwrap(); + self.values_mut()[removed_indices.removed_index] = moved_culling_data; + } else { + self.pop(); + } + } +} + /// Data that [`crate::material::queue_material_meshes`] and similar systems /// need in order to place entities that contain meshes in the right batch. #[derive(Deref)] @@ -994,29 +1171,51 @@ pub fn extract_meshes_for_cpu_building( /// Extracts meshes from the main world into the render world and queues /// [`MeshInputUniform`]s to be uploaded to the GPU. /// +/// This is optimized to only look at entities that have changed since the last +/// frame. +/// /// This is the variant of the system that runs when we're using GPU /// [`MeshUniform`] building. +#[allow(clippy::too_many_arguments)] pub fn extract_meshes_for_gpu_building( mut render_mesh_instances: ResMut, render_visibility_ranges: Res, mesh_material_ids: Res, mut render_mesh_instance_queues: ResMut, - meshes_query: Extract< - Query<( - Entity, - &ViewVisibility, - &GlobalTransform, - Option<&PreviousGlobalTransform>, - Option<&Lightmap>, - Option<&Aabb>, - &Mesh3d, - Has, - Has, - Has, - Has, - Has, - )>, + changed_meshes_query: Extract< + Query< + ( + Entity, + &ViewVisibility, + &GlobalTransform, + Option<&PreviousGlobalTransform>, + Option<&Lightmap>, + Option<&Aabb>, + &Mesh3d, + Has, + Has, + Has, + Has, + Has, + ), + Or<( + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + Changed, + )>, + >, >, + mut removed_visibilities_query: Extract>, + mut removed_global_transforms_query: Extract>, + mut removed_meshes_query: Extract>, cameras_query: Extract, With)>>, ) { let any_gpu_culling = !cameras_query.is_empty(); @@ -1025,6 +1224,7 @@ pub fn extract_meshes_for_gpu_building( } // Collect render mesh instances. Build up the uniform buffer. + let RenderMeshInstances::GpuBuilding(ref mut render_mesh_instances) = *render_mesh_instances else { panic!( @@ -1033,7 +1233,9 @@ pub fn extract_meshes_for_gpu_building( ); }; - meshes_query.par_iter().for_each_init( + // Find all meshes that have changed, and record information needed to + // construct the `MeshInputUniform` for them. + changed_meshes_query.par_iter().for_each_init( || render_mesh_instance_queues.borrow_local_mut(), |queue, ( @@ -1051,6 +1253,7 @@ pub fn extract_meshes_for_gpu_building( visibility_range, )| { if !view_visibility.get() { + queue.remove(entity.into(), any_gpu_culling); return; } @@ -1117,6 +1320,21 @@ pub fn extract_meshes_for_gpu_building( ); }, ); + + // Also record info about each mesh that became invisible. + let mut queue = render_mesh_instance_queues.borrow_local_mut(); + for entity in removed_visibilities_query + .read() + .chain(removed_global_transforms_query.read()) + .chain(removed_meshes_query.read()) + { + // Only queue a mesh for removal if we didn't pick it up above. + // It's possible that a necessary component was removed and re-added in + // the same frame. + if !changed_meshes_query.contains(entity) { + queue.remove(entity.into(), any_gpu_culling); + } + } } /// A system that sets the [`RenderMeshInstanceFlags`] for each mesh based on @@ -1166,49 +1384,82 @@ pub fn collect_meshes_for_gpu_building( }; // Collect render mesh instances. Build up the uniform buffer. - let gpu_preprocessing::BatchedInstanceBuffers { ref mut current_input_buffer, ref mut previous_input_buffer, .. } = batched_instance_buffers.into_inner(); - // Swap buffers. - mem::swap(current_input_buffer, previous_input_buffer); + previous_input_buffer.clear(); // Build the [`RenderMeshInstance`]s and [`MeshInputUniform`]s. - render_mesh_instances.clear(); for queue in render_mesh_instance_queues.iter_mut() { match *queue { RenderMeshInstanceGpuQueue::None => { // This can only happen if the queue is empty. } - RenderMeshInstanceGpuQueue::CpuCulling(ref mut queue) => { - for (entity, mesh_instance_builder) in queue.drain(..) { - mesh_instance_builder.add_to( + + RenderMeshInstanceGpuQueue::CpuCulling { + ref mut changed, + ref mut removed, + } => { + for (entity, mesh_instance_builder) in changed.drain(..) { + mesh_instance_builder.update( entity, &mut *render_mesh_instances, current_input_buffer, + previous_input_buffer, &mesh_allocator, ); } + + for entity in removed.drain(..) { + remove_mesh_input_uniform( + entity, + &mut *render_mesh_instances, + current_input_buffer, + ); + } } - RenderMeshInstanceGpuQueue::GpuCulling(ref mut queue) => { - for (entity, mesh_instance_builder, mesh_culling_builder) in queue.drain(..) { - let instance_data_index = mesh_instance_builder.add_to( + + RenderMeshInstanceGpuQueue::GpuCulling { + ref mut changed, + ref mut removed, + } => { + for (entity, mesh_instance_builder, mesh_culling_builder) in changed.drain(..) { + let instance_data_index = mesh_instance_builder.update( entity, &mut *render_mesh_instances, current_input_buffer, + previous_input_buffer, &mesh_allocator, ); - let culling_data_index = - mesh_culling_builder.add_to(&mut mesh_culling_data_buffer); - debug_assert_eq!(instance_data_index, culling_data_index); + mesh_culling_builder.update(&mut mesh_culling_data_buffer, instance_data_index); + } + + for entity in removed.drain(..) { + if let Some(removed_indices) = remove_mesh_input_uniform( + entity, + &mut *render_mesh_instances, + current_input_buffer, + ) { + mesh_culling_data_buffer.remove(removed_indices); + } } } } } + + // Buffers can't be empty. Make sure there's something in the previous input buffer. + if previous_input_buffer.buffer.is_empty() { + previous_input_buffer + .buffer + .push(MeshInputUniform::default()); + previous_input_buffer + .main_entities + .push(Entity::PLACEHOLDER.into()); + } } /// All data needed to construct a pipeline for rendering 3D meshes. diff --git a/crates/bevy_render/src/batching/gpu_preprocessing.rs b/crates/bevy_render/src/batching/gpu_preprocessing.rs index d6b490f8c40b4..1e8decf73d48e 100644 --- a/crates/bevy_render/src/batching/gpu_preprocessing.rs +++ b/crates/bevy_render/src/batching/gpu_preprocessing.rs @@ -23,6 +23,7 @@ use crate::{ }, render_resource::{BufferVec, GpuArrayBufferable, RawBufferVec, UninitBufferVec}, renderer::{RenderAdapter, RenderDevice, RenderQueue}, + sync_world::MainEntity, view::{ExtractedView, GpuCulling, ViewTarget}, Render, RenderApp, RenderSet, }; @@ -103,7 +104,7 @@ where /// The uniform data inputs for the current frame. /// /// These are uploaded during the extraction phase. - pub current_input_buffer: RawBufferVec, + pub current_input_buffer: InstanceInputUniformBuffer, /// The uniform data inputs for the previous frame. /// @@ -112,7 +113,58 @@ where /// can spawn or despawn between frames. Instead, each current buffer /// data input uniform is expected to contain the index of the /// corresponding buffer data input uniform in this list. - pub previous_input_buffer: RawBufferVec, + pub previous_input_buffer: InstanceInputUniformBuffer, +} + +/// Holds the GPU buffer of instance input data, which is the data about each +/// mesh instance that the CPU provides. +/// +/// `BDI` is the *buffer data input* type, which the GPU mesh preprocessing +/// shader is expected to expand to the full *buffer data* type. +pub struct InstanceInputUniformBuffer +where + BDI: Pod, +{ + /// The buffer containing the data that will be uploaded to the GPU. + pub buffer: RawBufferVec, + + /// The main-world entity that each item in + /// [`InstanceInputUniformBuffer::buffer`] corresponds to. + /// + /// This array must have the same length as + /// [`InstanceInputUniformBuffer::buffer`]. This bookkeeping is needed when + /// moving the data for an entity in the buffer (which happens when an + /// entity is removed), so that we can update other data structures with the + /// new buffer index of that entity. + pub main_entities: Vec, +} + +impl InstanceInputUniformBuffer +where + BDI: Pod, +{ + /// Creates a new, empty buffer. + pub fn new() -> InstanceInputUniformBuffer { + InstanceInputUniformBuffer { + buffer: RawBufferVec::new(BufferUsages::STORAGE), + main_entities: vec![], + } + } + + /// Clears the buffer and entity list out. + pub fn clear(&mut self) { + self.buffer.clear(); + self.main_entities.clear(); + } +} + +impl Default for InstanceInputUniformBuffer +where + BDI: Pod, +{ + fn default() -> Self { + Self::new() + } } /// The buffer of GPU preprocessing work items for a single view. @@ -275,8 +327,8 @@ where BatchedInstanceBuffers { data_buffer: UninitBufferVec::new(BufferUsages::STORAGE), work_item_buffers: EntityHashMap::default(), - current_input_buffer: RawBufferVec::new(BufferUsages::STORAGE), - previous_input_buffer: RawBufferVec::new(BufferUsages::STORAGE), + current_input_buffer: InstanceInputUniformBuffer::new(), + previous_input_buffer: InstanceInputUniformBuffer::new(), } } @@ -292,8 +344,6 @@ where /// Clears out the buffers in preparation for a new frame. pub fn clear(&mut self) { self.data_buffer.clear(); - self.current_input_buffer.clear(); - self.previous_input_buffer.clear(); for work_item_buffer in self.work_item_buffers.values_mut() { work_item_buffer.buffer.clear(); } @@ -671,13 +721,16 @@ pub fn write_batched_instance_buffers( ref mut data_buffer, work_item_buffers: ref mut index_buffers, ref mut current_input_buffer, - previous_input_buffer: _, + ref mut previous_input_buffer, } = gpu_array_buffer.into_inner(); data_buffer.write_buffer(&render_device); - current_input_buffer.write_buffer(&render_device, &render_queue); - // There's no need to write `previous_input_buffer`, as we wrote - // that on the previous frame, and it hasn't changed. + current_input_buffer + .buffer + .write_buffer(&render_device, &render_queue); + previous_input_buffer + .buffer + .write_buffer(&render_device, &render_queue); for index_buffer in index_buffers.values_mut() { index_buffer diff --git a/crates/bevy_render/src/render_resource/buffer_vec.rs b/crates/bevy_render/src/render_resource/buffer_vec.rs index ab384fadcf100..3a04e5c45be1d 100644 --- a/crates/bevy_render/src/render_resource/buffer_vec.rs +++ b/crates/bevy_render/src/render_resource/buffer_vec.rs @@ -174,6 +174,11 @@ impl RawBufferVec { self.values.clear(); } + /// Removes and returns the last element in the buffer. + pub fn pop(&mut self) -> Option { + self.values.pop() + } + pub fn values(&self) -> &Vec { &self.values } diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index e2e439ec79699..88a32cd0fb53a 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/crates/bevy_render/src/view/visibility/mod.rs @@ -5,13 +5,14 @@ mod render_layers; use core::any::TypeId; +use bevy_ecs::entity::EntityHashSet; pub use range::*; pub use render_layers::*; use bevy_app::{Plugin, PostUpdate}; use bevy_asset::Assets; -use bevy_derive::Deref; -use bevy_ecs::{prelude::*, query::QueryFilter}; +use bevy_derive::{Deref, DerefMut}; +use bevy_ecs::{change_detection::DetectChangesMut as _, prelude::*, query::QueryFilter}; use bevy_hierarchy::{Children, Parent}; use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_transform::{components::GlobalTransform, TransformSystem}; @@ -331,6 +332,10 @@ pub enum VisibilitySystems { /// the order of systems within this set is irrelevant, as [`check_visibility`] /// assumes that its operations are irreversible during the frame. CheckVisibility, + /// Label for the system that runs after visibility checking and marks + /// entities that have gone from visible to invisible, or vice versa, as + /// changed. + VisibilityChangeDetect, } pub struct VisibilityPlugin; @@ -346,12 +351,15 @@ impl Plugin for VisibilityPlugin { .after(TransformSystem::TransformPropagate), ) .configure_sets(PostUpdate, CheckVisibility.ambiguous_with(CheckVisibility)) + .configure_sets(PostUpdate, VisibilityChangeDetect.after(CheckVisibility)) + .init_resource::() .add_systems( PostUpdate, ( calculate_bounds.in_set(CalculateBounds), (visibility_propagate_system, reset_view_visibility).in_set(VisibilityPropagate), check_visibility::>.in_set(CheckVisibility), + mark_view_visibility_as_changed_if_necessary.in_set(VisibilityChangeDetect), ), ); } @@ -456,14 +464,28 @@ fn propagate_recursive( Ok(()) } +/// Stores all entities that were visible in the previous frame. +/// +/// At the end of visibility checking, we compare the visible entities against +/// these and set the [`ViewVisibility`] change flags accordingly. +#[derive(Resource, Default, Deref, DerefMut)] +pub struct PreviousVisibleEntities(EntityHashSet); + /// Resets the view visibility of every entity. /// Entities that are visible will be marked as such later this frame /// by a [`VisibilitySystems::CheckVisibility`] system. -fn reset_view_visibility(mut query: Query<&mut ViewVisibility>) { - query.iter_mut().for_each(|mut view_visibility| { - // NOTE: We do not use `set_if_neq` here, as we don't care about - // change detection for view visibility, and adding a branch to every - // loop iteration would pessimize performance. +fn reset_view_visibility( + mut query: Query<(Entity, &mut ViewVisibility)>, + mut previous_visible_entities: ResMut, +) { + previous_visible_entities.clear(); + + query.iter_mut().for_each(|(entity, mut view_visibility)| { + // Record the entities that were previously visible. + if view_visibility.get() { + previous_visible_entities.insert(entity); + } + *view_visibility.bypass_change_detection() = ViewVisibility::HIDDEN; }); } @@ -569,7 +591,10 @@ pub fn check_visibility( } } - view_visibility.set(); + // Make sure we don't trigger changed notifications + // unnecessarily. + view_visibility.bypass_change_detection().set(); + queue.push(entity); }, ); @@ -579,6 +604,25 @@ pub fn check_visibility( } } +/// A system that marks [`ViewVisibility`] components as changed if their +/// visibility changed this frame. +/// +/// Ordinary change detection doesn't work for this use case because we use +/// multiple [`check_visibility`] systems, and visibility is set if *any one* of +/// those systems judges the entity to be visible. Thus, in order to perform +/// change detection, we need this system, which is a follow-up pass that has a +/// global view of whether the entity became visible or not. +fn mark_view_visibility_as_changed_if_necessary( + mut query: Query<(Entity, &mut ViewVisibility)>, + previous_visible_entities: Res, +) { + for (entity, mut view_visibility) in query.iter_mut() { + if previous_visible_entities.contains(&entity) != view_visibility.get() { + view_visibility.set_changed(); + } + } +} + #[cfg(test)] mod test { use super::*;