Sweep bins after queuing so as to only sweep them once. (#17787)

Currently, we *sweep*, or remove entities from bins when those entities
became invisible or changed phases, during `queue_material_meshes` and
similar phases. This, however, is wrong, because `queue_material_meshes`
executes once per material type, not once per phase. This could result
in sweeping bins multiple times per phase, which can corrupt the bins.
This commit fixes the issue by moving sweeping to a separate system that
runs after queuing.

This manifested itself as entities appearing and disappearing seemingly
at random.

Closes #17759.

---------

Co-authored-by: Robert Swain <[email protected]>

Author: pcwalton

crates/bevy_pbr/src/material.rs

@@ -1047,10 +1047,6 @@ pub fn queue_material_meshes<M: Material>(
                 }
             }
         }
-
-        // Remove invalid entities from the bins.
-        opaque_phase.sweep_old_entities();
-        alpha_mask_phase.sweep_old_entities();
     }
 }
 

crates/bevy_pbr/src/prepass/mod.rs

@@ -1229,20 +1229,6 @@ pub fn queue_prepass_material_meshes<M: Material>(
                 _ => {}
             }
         }
-
-        // Remove invalid entities from the bins.
-        if let Some(phase) = opaque_phase {
-            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();
-        }
     }
 }
 

crates/bevy_pbr/src/render/light.rs

@@ -1953,9 +1953,6 @@ pub fn queue_shadows<M: Material>(
                     *current_change_tick,
                 );
             }
-
-            // Remove invalid entities from the bins.
-            shadow_phase.sweep_old_entities();
         }
     }
 }

crates/bevy_render/src/lib.rs

@@ -146,6 +146,9 @@ pub enum RenderSet {
     Queue,
     /// A sub-set within [`Queue`](RenderSet::Queue) where mesh entity queue systems are executed. Ensures `prepare_assets::<RenderMesh>` is completed.
     QueueMeshes,
+    /// A sub-set within [`Queue`](RenderSet::Queue) where meshes that have
+    /// become invisible or changed phases are removed from the bins.
+    QueueSweep,
     // TODO: This could probably be moved in favor of a system ordering
     // abstraction in `Render` or `Queue`
     /// Sort the [`SortedRenderPhase`](render_phase::SortedRenderPhase)s and
@@ -201,7 +204,8 @@ impl Render {
 
         schedule.configure_sets((ExtractCommands, PrepareAssets, PrepareMeshes, Prepare).chain());
         schedule.configure_sets(
-            QueueMeshes
+            (QueueMeshes, QueueSweep)
+                .chain()
                 .in_set(Queue)
                 .after(prepare_assets::<RenderMesh>),
         );

crates/bevy_render/src/render_phase/mod.rs

@@ -823,16 +823,14 @@ where
     /// 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.
+    /// each as clean as we come to it. Then, in [`sweep_old_entities`], 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 {
@@ -1068,6 +1066,7 @@ where
                             ),
                     )
                         .in_set(RenderSet::PrepareResources),
+                    sweep_old_entities::<BPI>.in_set(RenderSet::QueueSweep),
                 ),
             );
     }
@@ -1605,6 +1604,18 @@ where
     }
 }
 
+/// Removes entities that became invisible or changed phases from the bins.
+///
+/// This must run after queuing.
+pub fn sweep_old_entities<BPI>(mut render_phases: ResMut<ViewBinnedRenderPhases<BPI>>)
+where
+    BPI: BinnedPhaseItem,
+{
+    for phase in render_phases.0.values_mut() {
+        phase.sweep_old_entities();
+    }
+}
+
 impl BinnedRenderPhaseType {
     pub fn mesh(
         batchable: bool,

crates/bevy_sprite/src/mesh2d/material.rs

@@ -848,10 +848,6 @@ pub fn queue_material2d_meshes<M: Material2d>(
                 }
             }
         }
-
-        // Remove invalid entities from the bins.
-        opaque_phase.sweep_old_entities();
-        alpha_mask_phase.sweep_old_entities();
     }
 }