Optimize Visibility Systems (#22226)

# Objective

- Make visibility systems less slow.
- Saves as much as 1.2ms of CPU time on a (GPU bound) 6.7ms frame,
rendering caldera hotel.
- Fixes #22256

## Solution

- Replace the `EntityHashSet` with a component added directly to
entities. This still allows for correct change detection triggers on
visibility, but avoids hashing. This also enables parallel updates.

## Summary

- This PR lead to a rabbit hole that uncovered #22256 
- This PR resolves the regression introduced since 0.17 released
- A more fair comparison is not to main (which has a regression), but to
0.17
- Compared to 0.17.3, this is a 27% speedup to the `PostUpdate` schedule
for caldera hotel on an M4 Max

<img width="558" height="251" alt="image"
src="https://github.com/user-attachments/assets/2d711e3c-b65e-4c3a-9d2a-a587f8f36aae"
/>

## Testing

- Many cubes, many foxes, caldera
- Yellow is new, red is old.
- Note the bimodality in the old traces, this was consistently
repeatable, and seems to have something to do with `EntityHashSet` and
`EntityHashMap`. Worth investigating that further, as I've seen that
bimodal behavior before, and blamed it on pcores vs ecores, but I
verified that is not happening.
- Summary: caldera hotel no longer exhibits bimodal performance with a
pathological mode that runs very slowly. Roughly comparing the ~90th
percentile performance, the optimized code is about 1.2ms faster. This
is particularly significant when you consider this is running on a
device that is already hitting 150fps (GPU bound), so it only has a
frame budget of 6.7ms.
- Notably, visibility checking was the last egregiously performing set
of systems in the common hot path of most bevy apps, and no longer stick
out as obviously slow in a frame profile:

A high level comparison of the change in CPU time by looking at just
PostUpdate between old(red) and new(yellow)

<img width="1738" height="512" alt="image"
src="https://github.com/user-attachments/assets/5252ff0f-d9f3-49d1-a3ce-309bd84d5485"
/>

### caldera hotel - all entities in view

For this test, I didn't touch the camera, so all entities were in view
at all times.

`check_visibility`

<img width="1262" height="397" alt="image"
src="https://github.com/user-attachments/assets/c80c470a-1548-42d2-838c-4e74525a6cb8"
/>

`reset_view_visibility`

<img width="1262" height="407" alt="image"
src="https://github.com/user-attachments/assets/7b550823-f832-41f1-9e42-20cab342b0f2"
/>

`mark_newly_hidden_entities_invisible`

This is 23us slower than the "fast mode" of the existing code on main,
but 180-250us *faster* than the weird "slow mode" on main. The new code
is significantly more stable, and does not exhibit the super slow mode.

<img width="1256" height="418" alt="image"
src="https://github.com/user-attachments/assets/b7cbf57d-a221-468a-9af8-3381981874b2"
/>

### caldera hotel - no entities in view

For this test, I immediately rotated the camera so the hotel was not in
view.

`check_visibility`

This is largely a wash. The old code is 2us faster, but this is likely
in the realm of noise.

<img width="1255" height="372" alt="image"
src="https://github.com/user-attachments/assets/2c795f72-ebea-4b35-ba19-a8eccd4c56fc"
/>

`reset_view_visibility`

This is a big win. The main peak is about 30us faster now, but the major
win is the worst case, which is nearly 500us faster.

<img width="1252" height="398" alt="image"
src="https://github.com/user-attachments/assets/f8a9d99f-a6e1-48db-8639-d52276dba9ab"
/>


`mark_newly_hidden_entities_invisible`

Same story as the other caldera comparison with everything in view, this
is a bit slower than the fast mode, but way faster than the slow mode on
main.

<img width="1248" height="400" alt="image"
src="https://github.com/user-attachments/assets/4564d94e-911a-4b3f-a584-c6e102ef2dd0"
/>

Author: aevyrie

crates/bevy_camera/src/visibility/mod.rs

@@ -3,7 +3,7 @@ mod render_layers;
 
 use core::any::TypeId;
 
-use bevy_ecs::entity::{EntityHashMap, EntityHashSet};
+use bevy_ecs::entity::EntityHashMap;
 use bevy_ecs::lifecycle::HookContext;
 use bevy_ecs::world::DeferredWorld;
 use derive_more::derive::{Deref, DerefMut};
@@ -163,46 +163,85 @@ impl InheritedVisibility {
 #[component(clone_behavior=Ignore)]
 pub struct VisibilityClass(pub SmallVec<[TypeId; 1]>);
 
-/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering.
+/// Algorithmically computed indication of whether an entity is visible and should be extracted for
+/// rendering.
 ///
-/// Each frame, this will be reset to `false` during [`VisibilityPropagate`] systems in [`PostUpdate`].
-/// Later in the frame, systems in [`CheckVisibility`] will mark any visible entities using [`ViewVisibility::set`].
-/// Because of this, values of this type will be marked as changed every frame, even when they do not change.
+/// Each frame, this will be reset to `false` during [`VisibilityPropagate`] systems in
+/// [`PostUpdate`]. Later in the frame, systems in [`CheckVisibility`] will mark any visible
+/// entities using [`ViewVisibility::set`]. Because of this, values of this type will be marked as
+/// changed every frame, even when they do not change.
 ///
-/// If you wish to add custom visibility system that sets this value, make sure you add it to the [`CheckVisibility`] set.
+/// If you wish to add a custom visibility system that sets this value, be sure to add it to the
+/// [`CheckVisibility`] set.
 ///
 /// [`VisibilityPropagate`]: VisibilitySystems::VisibilityPropagate
 /// [`CheckVisibility`]: VisibilitySystems::CheckVisibility
-#[derive(Component, Deref, Debug, Default, Clone, Copy, Reflect, PartialEq, Eq)]
+#[derive(Component, Debug, Default, Clone, Copy, Reflect, PartialEq, Eq)]
 #[reflect(Component, Default, Debug, PartialEq, Clone)]
-pub struct ViewVisibility(bool);
+pub struct ViewVisibility(
+    /// Bit packed booleans to track current and previous view visibility state.
+    ///
+    /// Previous visibility is used as a scratch space to ensure that [`ViewVisibility`] is only
+    /// mutated (triggering change detection) when necessary.
+    ///
+    /// This is needed because an entity might be seen by many views (cameras, lights that cast
+    /// shadows, etc.), so it is easy to know if an entity is visible to something, but hard to know
+    /// if it is *globally* non-visible to any view. To solve this, we track the visibility from the
+    /// previous frame. Then, during the [`VisibilitySystems::CheckVisibility`] system set, systems
+    /// call [`SetViewVisibility::set_visible`] to mark entities as visible.
+    ///
+    /// Finally, we can look for entities that were previously visible but are no longer visible
+    /// and set their current state to hidden, ensuring that we have only triggered change detection
+    /// when necessary.
+    u8,
+);
 
 impl ViewVisibility {
     /// An entity that cannot be seen from any views.
-    pub const HIDDEN: Self = Self(false);
+    pub const HIDDEN: Self = Self(0);
 
     /// Returns `true` if the entity is visible in any view.
     /// Otherwise, returns `false`.
     #[inline]
     pub fn get(self) -> bool {
-        self.0
+        self.0 & 1 != 0
     }
 
-    /// Sets the visibility to `true`. This should not be considered reversible for a given frame,
-    /// as this component tracks whether or not the entity visible in _any_ view.
-    ///
-    /// This will be automatically reset to `false` every frame in [`VisibilityPropagate`] and then set
-    /// to the proper value in [`CheckVisibility`].
+    /// Returns `true` if this entity was visible in the previous frame but is now hidden.
+    #[inline]
+    fn was_visible_now_hidden(self) -> bool {
+        // The first bit is false (current), and the second bit is true (previous).
+        self.0 == 0b10
+    }
+
+    #[inline]
+    fn update(&mut self) {
+        // Copy the first bit (current) to the second bit position (previous)
+        // Clear the second bit, then set it based on the first bit
+        self.0 = (self.0 & !2) | ((self.0 & 1) << 1);
+    }
+}
+
+pub trait SetViewVisibility {
+    /// Sets the visibility to `true` if not already visible, triggering change detection only when
+    /// needed. This should not be considered reversible for a given frame, as this component tracks
+    /// if the entity is visible in _any_ view.
     ///
     /// You should only manually set this if you are defining a custom visibility system,
     /// in which case the system should be placed in the [`CheckVisibility`] set.
     /// For normal user-defined entity visibility, see [`Visibility`].
     ///
-    /// [`VisibilityPropagate`]: VisibilitySystems::VisibilityPropagate
     /// [`CheckVisibility`]: VisibilitySystems::CheckVisibility
+    fn set_visible(&mut self);
+}
+
+impl<'a> SetViewVisibility for Mut<'a, ViewVisibility> {
     #[inline]
-    pub fn set(&mut self) {
-        self.0 = true;
+    fn set_visible(&mut self) {
+        if !self.as_ref().get() {
+            // Set the first bit (current vis) to true
+            self.0 |= 1;
+        }
     }
 }
 
@@ -369,7 +408,6 @@ impl Plugin for VisibilityPlugin {
                     .ambiguous_with(CalculateBounds)
                     .ambiguous_with(mark_3d_meshes_as_changed_if_their_assets_changed),
             )
-            .init_resource::<PreviousVisibleEntities>()
             .add_systems(
                 PostUpdate,
                 (
@@ -522,28 +560,11 @@ fn propagate_recursive(
     Ok(())
 }
 
-/// Stores all entities that were visible in the previous frame.
-///
-/// As systems that check visibility judge entities visible, they remove them
-/// from this set. Afterward, the `mark_newly_hidden_entities_invisible` system
-/// runs and marks every mesh still remaining in this set as hidden.
-#[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<(Entity, &ViewVisibility)>,
-    mut previous_visible_entities: ResMut<PreviousVisibleEntities>,
-) {
-    previous_visible_entities.clear();
-
-    query.iter_mut().for_each(|(entity, view_visibility)| {
-        // Record the entities that were previously visible.
-        if view_visibility.get() {
-            previous_visible_entities.insert(entity);
-        }
+/// Track entities that were visible last frame, used to granularly update [`ViewVisibility`] this
+/// frame without spurious `Change` detecation.
+fn reset_view_visibility(mut query: Query<&mut ViewVisibility>) {
+    query.par_iter_mut().for_each(|mut view_visibility| {
+        view_visibility.bypass_change_detection().update();
     });
 }
 
@@ -577,7 +598,6 @@ pub fn check_visibility(
         Has<VisibilityRange>,
     )>,
     visible_entity_ranges: Option<Res<VisibleEntityRanges>>,
-    mut previous_visible_entities: ResMut<PreviousVisibleEntities>,
 ) {
     let visible_entity_ranges = visible_entity_ranges.as_deref();
 
@@ -645,17 +665,12 @@ pub fn check_visibility(
                     }
                 }
 
-                // Make sure we don't trigger changed notifications
-                // unnecessarily by checking whether the flag is set before
-                // setting it.
-                if !**view_visibility {
-                    view_visibility.set();
-                }
+                view_visibility.set_visible();
 
                 // The visibility class may be None here because AABB gizmos can be enabled via
                 // config without a renderable component being added to the entity. This workaround
-                // allows view visibility to be set for entities without a renderable component but
-                // that still need to render gizmos.
+                // allows view visibility to be set for entities without a renderable component, but
+                // still need to render gizmos.
                 if let Some(visibility_class) = visibility_class {
                     // Add the entity to the queue for all visibility classes the entity is in.
                     for visibility_class_id in visibility_class.iter() {
@@ -670,40 +685,23 @@ pub fn check_visibility(
         // Drain all the thread queues into the `visible_entities` list.
         for class_queues in thread_queues.iter_mut() {
             for (class, entities) in class_queues {
-                let visible_entities_for_class = visible_entities.get_mut(*class);
-                for entity in entities.drain(..) {
-                    // As we mark entities as visible, we remove them from the
-                    // `previous_visible_entities` list. At the end, all of the
-                    // entities remaining in `previous_visible_entities` will be
-                    // entities that were visible last frame but are no longer
-                    // visible this frame.
-                    previous_visible_entities.remove(&entity);
-
-                    visible_entities_for_class.push(entity);
-                }
+                visible_entities.get_mut(*class).append(entities);
             }
         }
     }
 }
 
-/// Marks any entities that weren't judged visible this frame as invisible.
-///
-/// As visibility-determining systems run, they remove entities that they judge
-/// visible from [`PreviousVisibleEntities`]. At the end of visibility
-/// determination, all entities that remain in [`PreviousVisibleEntities`] must
-/// be invisible. This system goes through those entities and marks them newly
-/// invisible (which sets the change flag for them).
-fn mark_newly_hidden_entities_invisible(
-    mut view_visibilities: Query<&mut ViewVisibility>,
-    mut previous_visible_entities: ResMut<PreviousVisibleEntities>,
-) {
-    // Whatever previous visible entities are left are entities that were
-    // visible last frame but just became invisible.
-    for entity in previous_visible_entities.drain() {
-        if let Ok(mut view_visibility) = view_visibilities.get_mut(entity) {
-            *view_visibility = ViewVisibility::HIDDEN;
-        }
-    }
+/// The last step in the visibility pipeline. Looks at entities that were visible last frame but not
+/// marked as visible this frame and marks them as hidden by setting the [`ViewVisibility`]. This
+/// process is needed to ensure we only trigger change detection on [`ViewVisibility`] when needed.
+fn mark_newly_hidden_entities_invisible(mut view_visibilities: Query<&mut ViewVisibility>) {
+    view_visibilities
+        .par_iter_mut()
+        .for_each(|mut view_visibility| {
+            if view_visibility.as_ref().was_visible_now_hidden() {
+                *view_visibility = ViewVisibility::HIDDEN;
+            }
+        });
 }
 
 /// A generic component add hook that automatically adds the appropriate

crates/bevy_light/src/lib.rs

@@ -5,8 +5,8 @@ use bevy_camera::{
     primitives::{Aabb, CascadesFrusta, CubemapFrusta, Frustum, Sphere},
     visibility::{
         CascadesVisibleEntities, CubemapVisibleEntities, InheritedVisibility, NoFrustumCulling,
-        PreviousVisibleEntities, RenderLayers, ViewVisibility, VisibilityRange, VisibilitySystems,
-        VisibleEntityRanges, VisibleMeshEntities,
+        RenderLayers, ViewVisibility, VisibilityRange, VisibilitySystems, VisibleEntityRanges,
+        VisibleMeshEntities,
     },
     CameraUpdateSystems,
 };
@@ -26,6 +26,8 @@ use cluster::{
 };
 mod ambient_light;
 pub use ambient_light::{AmbientLight, GlobalAmbientLight};
+use bevy_camera::visibility::SetViewVisibility;
+
 mod probe;
 pub use probe::{
     AtmosphereEnvironmentMapLight, EnvironmentMapLight, GeneratedEnvironmentMapLight,
@@ -188,9 +190,10 @@ impl Plugin for LightPlugin {
                         .after(VisibilitySystems::CalculateBounds)
                         .after(TransformSystems::Propagate)
                         .after(SimulationLightSystems::UpdateLightFrusta)
-                        // 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
+                        // Lights can "see" entities and mark them as visible. This is done to
+                        // correctly render shadows for entities that are not in view of a camera,
+                        // but must be renderable to cast shadows. Because of this, we need to check
+                        // entity visibility and mark as visible before they can be hidden.
                         .after(VisibilitySystems::CheckVisibility)
                         .before(VisibilitySystems::MarkNewlyHiddenEntitiesInvisible),
                     build_directional_light_cascades
@@ -450,23 +453,13 @@ pub fn check_dir_light_mesh_visibility(
     // TODO: use resource to avoid unnecessary memory alloc
     let mut defer_queue = core::mem::take(defer_visible_entities_queue.deref_mut());
     commands.queue(move |world: &mut World| {
-        world.resource_scope::<PreviousVisibleEntities, _>(
-            |world, mut previous_visible_entities| {
-                let mut query = world.query::<(Entity, &mut ViewVisibility)>();
-                for entities in defer_queue.iter_mut() {
-                    let mut iter = query.iter_many_mut(world, entities.iter());
-                    while let Some((entity, mut view_visibility)) = iter.fetch_next() {
-                        if !**view_visibility {
-                            view_visibility.set();
-                        }
-
-                        // Remove any entities that were discovered to be
-                        // visible from the `PreviousVisibleEntities` resource.
-                        previous_visible_entities.remove(&entity);
-                    }
-                }
-            },
-        );
+        let mut query = world.query::<&mut ViewVisibility>();
+        for entities in defer_queue.iter_mut() {
+            let mut iter = query.iter_many_mut(world, entities.iter());
+            while let Some(mut view_visibility) = iter.fetch_next() {
+                view_visibility.set_visible();
+            }
+        }
     });
 }
 
@@ -504,7 +497,6 @@ pub fn check_point_light_mesh_visibility(
         ),
     >,
     visible_entity_ranges: Option<Res<VisibleEntityRanges>>,
-    mut previous_visible_entities: ResMut<PreviousVisibleEntities>,
     mut cubemap_visible_entities_queue: Local<Parallel<[Vec<Entity>; 6]>>,
     mut spot_visible_entities_queue: Local<Parallel<Vec<Entity>>>,
     mut checked_lights: Local<EntityHashSet>,
@@ -587,16 +579,12 @@ pub fn check_point_light_mesh_visibility(
                                 if has_no_frustum_culling
                                     || frustum.intersects_obb(aabb, &model_to_world, true, true)
                                 {
-                                    if !**view_visibility {
-                                        view_visibility.set();
-                                    }
+                                    view_visibility.set_visible();
                                     visible_entities.push(entity);
                                 }
                             }
                         } else {
-                            if !**view_visibility {
-                                view_visibility.set();
-                            }
+                            view_visibility.set_visible();
                             for visible_entities in cubemap_visible_entities_local_queue.iter_mut()
                             {
                                 visible_entities.push(entity);
@@ -609,12 +597,6 @@ pub fn check_point_light_mesh_visibility(
                     for (dst, source) in
                         cubemap_visible_entities.iter_mut().zip(entities.iter_mut())
                     {
-                        // Remove any entities that were discovered to be
-                        // visible from the `PreviousVisibleEntities` resource.
-                        for entity in source.iter() {
-                            previous_visible_entities.remove(entity);
-                        }
-
                         dst.entities.append(source);
                     }
                 }
@@ -683,28 +665,18 @@ pub fn check_point_light_mesh_visibility(
                             if has_no_frustum_culling
                                 || frustum.intersects_obb(aabb, &model_to_world, true, true)
                             {
-                                if !**view_visibility {
-                                    view_visibility.set();
-                                }
+                                view_visibility.set_visible();
                                 spot_visible_entities_local_queue.push(entity);
                             }
                         } else {
-                            if !**view_visibility {
-                                view_visibility.set();
-                            }
+                            view_visibility.set_visible();
                             spot_visible_entities_local_queue.push(entity);
                         }
                     },
                 );
 
                 for entities in spot_visible_entities_queue.iter_mut() {
                     visible_entities.append(entities);
-
-                    // Remove any entities that were discovered to be visible
-                    // from the `PreviousVisibleEntities` resource.
-                    for entity in entities {
-                        previous_visible_entities.remove(entity);
-                    }
                 }
 
                 shrink_entities(visible_entities.deref_mut());