Allow Visibility to be inherited

crates/bevy_pbr/src/light.rs

@@ -659,7 +659,7 @@ pub(crate) fn assign_lights_to_clusters(
     lights.extend(
         lights_query
             .iter()
-            .filter(|(.., visibility)| visibility.is_visible)
+            .filter(|(.., visibility)| visibility.is_visible())
             .map(
                 |(entity, transform, light, _visibility)| PointLightAssignmentData {
                     entity,
@@ -1174,7 +1174,7 @@ pub fn update_directional_light_frusta(
         // The frustum is used for culling meshes to the light for shadow mapping
         // so if shadow mapping is disabled for this light, then the frustum is
         // not needed.
-        if !directional_light.shadows_enabled || !visibility.is_visible {
+        if !directional_light.shadows_enabled || !visibility.is_visible() {
             continue;
         }
 
@@ -1269,7 +1269,7 @@ pub fn check_light_mesh_visibility(
         visible_entities.entities.clear();
 
         // NOTE: If shadow mapping is disabled for the light then it must have no visible entities
-        if !directional_light.shadows_enabled || !visibility.is_visible {
+        if !directional_light.shadows_enabled || !visibility.is_visible() {
             continue;
         }
 
@@ -1284,7 +1284,7 @@ pub fn check_light_mesh_visibility(
             maybe_transform,
         ) in visible_entity_query.iter_mut()
         {
-            if !visibility.is_visible {
+            if !visibility.is_visible() {
                 continue;
             }
 
@@ -1343,7 +1343,7 @@ pub fn check_light_mesh_visibility(
                     maybe_transform,
                 ) in visible_entity_query.iter_mut()
                 {
-                    if !visibility.is_visible {
+                    if !visibility.is_visible() {
                         continue;
                     }
 

crates/bevy_pbr/src/render/light.rs

@@ -464,7 +464,7 @@ pub fn extract_lights(
     for (entity, directional_light, visible_entities, transform, visibility) in
         directional_lights.iter_mut()
     {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
 

crates/bevy_render/Cargo.toml

@@ -34,6 +34,7 @@ bevy_core = { path = "../bevy_core", version = "0.8.0-dev" }
 bevy_derive = { path = "../bevy_derive", version = "0.8.0-dev" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.8.0-dev" }
 bevy_encase_derive = { path = "../bevy_encase_derive", version = "0.8.0-dev" }
+bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.8.0-dev" }
 bevy_math = { path = "../bevy_math", version = "0.8.0-dev" }
 bevy_mikktspace = { path = "../bevy_mikktspace", version = "0.8.0-dev" }
 bevy_reflect = { path = "../bevy_reflect", version = "0.8.0-dev", features = ["bevy"] }

crates/bevy_render/src/view/visibility/mod.rs

@@ -6,6 +6,7 @@ pub use render_layers::*;
 use bevy_app::{CoreStage, Plugin};
 use bevy_asset::{Assets, Handle};
 use bevy_ecs::prelude::*;
+use bevy_hierarchy::{Children, HierarchySystem, Parent};
 use bevy_reflect::std_traits::ReflectDefault;
 use bevy_reflect::Reflect;
 use bevy_transform::components::GlobalTransform;
@@ -20,15 +21,49 @@ use crate::{
 };
 
 /// User indication of whether an entity is visible
-#[derive(Component, Clone, Reflect, Debug)]
+#[derive(Component, Clone, Reflect, Debug, Eq, PartialEq)]
 #[reflect(Component, Default)]
 pub struct Visibility {
-    pub is_visible: bool,
+    self_visible: bool,
+    inherited: bool,
+}
+
+impl Visibility {
+    /// Checks if the entity is visible or not.
+    ///
+    /// This value can be affected by the entity's local visibility
+    /// or it inherited from it's ancestors in the hierarchy. An entity
+    /// is only visible in the hierarchy if and only if all of it's
+    /// ancestors are visible. Setting an entity to be invisible will
+    /// hide all of it's children and descendants.
+    ///
+    /// If the systems labeled [`VisibilitySystems::VisibilityPropagate`]
+    /// have not yet run, this value may be out of date with the state of
+    /// the hierarchy.
+    pub fn is_visible(&self) -> bool {
+        self.self_visible && self.inherited
+    }
+
+    /// Checks the local visibility state of the entity.
+    ///
+    /// Unlike [`is_visible`](Self::is_visible), this value is always up to date.
+    pub fn is_self_visible(&self) -> bool {
+        self.self_visible
+    }
+
+    /// Sets whether the entity is visible or not. If set to false,
+    /// all descendants will be marked as invisible.
+    pub fn set_visible(&mut self, visible: bool) {
+        self.self_visible = visible;
+    }
 }
 
 impl Default for Visibility {
     fn default() -> Self {
-        Self { is_visible: true }
+        Self {
+            self_visible: true,
+            inherited: true,
+        }
     }
 }
 
@@ -88,6 +123,7 @@ pub enum VisibilitySystems {
     UpdateOrthographicFrusta,
     UpdatePerspectiveFrusta,
     UpdateProjectionFrusta,
+    VisibilityPropagate,
     /// Label for the [`check_visibility()`] system updating each frame the [`ComputedVisibility`]
     /// of each entity and the [`VisibleEntities`] of each view.
     CheckVisibility,
@@ -121,6 +157,12 @@ impl Plugin for VisibilityPlugin {
                 .label(UpdateProjectionFrusta)
                 .after(TransformSystem::TransformPropagate),
         )
+        .add_system_to_stage(
+            CoreStage::PostUpdate,
+            visibility_propagate_system
+                .label(VisibilityPropagate)
+                .after(HierarchySystem::ParentUpdate),
+        )
         .add_system_to_stage(
             CoreStage::PostUpdate,
             check_visibility
@@ -129,6 +171,7 @@ impl Plugin for VisibilityPlugin {
                 .after(UpdateOrthographicFrusta)
                 .after(UpdatePerspectiveFrusta)
                 .after(UpdateProjectionFrusta)
+                .after(VisibilityPropagate)
                 .after(TransformSystem::TransformPropagate),
         );
     }
@@ -163,6 +206,59 @@ pub fn update_frusta<T: Component + CameraProjection + Send + Sync + 'static>(
     }
 }
 
+fn visibility_propagate_system(
+    mut root_query: Query<(Option<&Children>, &mut Visibility, Entity), Without<Parent>>,
+    mut visibility_query: Query<(&mut Visibility, &Parent)>,
+    children_query: Query<&Children, (With<Parent>, With<Visibility>)>,
+) {
+    for (children, mut visibility, entity) in root_query.iter_mut() {
+        // Avoid triggering change detection if nothing has changed.
+        if !visibility.inherited {
+            visibility.inherited = true;
+        }
+        if let Some(children) = children {
+            let is_visible = visibility.is_visible();
+            for child in children.iter() {
+                let _ = propagate_recursive(
+                    is_visible,
+                    &mut visibility_query,
+                    &children_query,
+                    *child,
+                    entity,
+                );
+            }
+        }
+    }
+}
+
+fn propagate_recursive(
+    parent_visible: bool,
+    visibility_query: &mut Query<(&mut Visibility, &Parent)>,
+    children_query: &Query<&Children, (With<Parent>, With<Visibility>)>,
+    entity: Entity,
+    expected_parent: Entity,
+    // We use a result here to use the `?` operator. Ideally we'd use a try block instead
+) -> Result<(), ()> {
+    let is_visible = {
+        let (mut visibility, child_parent) = visibility_query.get_mut(entity).map_err(drop)?;
+        // Note that for parallelising, this check cannot occur here, since there is an `&mut GlobalTransform` (in global_transform)
+        assert_eq!(
+            child_parent.0, expected_parent,
+            "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
+        );
+        // Avoid triggering change detection if nothing has changed.
+        if visibility.inherited != parent_visible {
+            visibility.inherited = parent_visible;
+        }
+        visibility.is_visible()
+    };
+
+    for child in children_query.get(entity).map_err(drop)?.iter() {
+        let _ = propagate_recursive(is_visible, visibility_query, children_query, *child, entity);
+    }
+    Ok(())
+}
+
 /// System updating the visibility of entities each frame.
 ///
 /// The system is labelled with [`VisibilitySystems::CheckVisibility`]. Each frame, it updates the
@@ -203,7 +299,7 @@ pub fn check_visibility(
                 maybe_no_frustum_culling,
                 maybe_transform,
             )| {
-                if !visibility.is_visible {
+                if !visibility.is_visible() {
                     return;
                 }
 
@@ -244,3 +340,125 @@ pub fn check_visibility(
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use bevy_app::prelude::*;
+    use bevy_ecs::prelude::*;
+
+    use super::*;
+
+    use bevy_hierarchy::{parent_update_system, BuildWorldChildren, Children, Parent};
+
+    #[test]
+    fn did_propagate() {
+        let mut world = World::default();
+
+        let mut update_stage = SystemStage::parallel();
+        update_stage.add_system(parent_update_system);
+        update_stage.add_system(visibility_propagate_system.after(parent_update_system));
+
+        let mut schedule = Schedule::default();
+        schedule.add_stage("update", update_stage);
+
+        // Root entity
+        world.spawn().insert(Visibility::default());
+
+        let mut children = Vec::new();
+        world
+            .spawn()
+            .insert(Visibility {
+                self_visible: false,
+                inherited: false,
+            })
+            .with_children(|parent| {
+                children.push(parent.spawn().insert(Visibility::default()).id());
+                children.push(parent.spawn().insert(Visibility::default()).id());
+            });
+        schedule.run(&mut world);
+
+        assert_eq!(
+            *world.get::<Visibility>(children[0]).unwrap(),
+            Visibility {
+                self_visible: true,
+                inherited: false,
+            }
+        );
+
+        assert_eq!(
+            *world.get::<Visibility>(children[1]).unwrap(),
+            Visibility {
+                self_visible: true,
+                inherited: false,
+            }
+        );
+    }
+
+    #[test]
+    fn correct_visibility_when_no_children() {
+        let mut app = App::new();
+
+        app.add_system(parent_update_system);
+        app.add_system(visibility_propagate_system.after(parent_update_system));
+
+        let parent = app
+            .world
+            .spawn()
+            .insert(Visibility {
+                self_visible: false,
+                inherited: false,
+            })
+            .insert(GlobalTransform::default())
+            .id();
+
+        let child = app
+            .world
+            .spawn()
+            .insert_bundle((Visibility::default(), Parent(parent)))
+            .id();
+
+        let grandchild = app
+            .world
+            .spawn()
+            .insert_bundle((Visibility::default(), Parent(child)))
+            .id();
+
+        app.update();
+
+        // check the `Children` structure is spawned
+        assert_eq!(&**app.world.get::<Children>(parent).unwrap(), &[child]);
+        assert_eq!(&**app.world.get::<Children>(child).unwrap(), &[grandchild]);
+        // Note that at this point, the `GlobalTransform`s will not have updated yet, due to `Commands` delay
+        app.update();
+
+        let mut state = app.world.query::<&Visibility>();
+        for visibility in state.iter(&app.world) {
+            assert!(!visibility.is_visible());
+        }
+    }
+
+    #[test]
+    #[should_panic]
+    fn panic_when_hierarchy_cycle() {
+        let mut world = World::default();
+        // This test is run on a single thread in order to avoid breaking the global task pool by panicking
+        // This fixes the flaky tests reported in https://github.com/bevyengine/bevy/issues/4996
+        let mut update_stage = SystemStage::single_threaded();
+
+        update_stage.add_system(parent_update_system);
+        update_stage.add_system(visibility_propagate_system.after(parent_update_system));
+
+        let child = world.spawn().insert(Visibility::default()).id();
+
+        let grandchild = world
+            .spawn()
+            .insert_bundle((Visibility::default(), Parent(child)))
+            .id();
+        world
+            .spawn()
+            .insert_bundle((Visibility::default(), Children::with(&[child])));
+        world.entity_mut(child).insert(Parent(grandchild));
+
+        update_stage.run(&mut world);
+    }
+}

crates/bevy_sprite/src/render/mod.rs

@@ -234,7 +234,7 @@ pub fn extract_sprites(
     let mut extracted_sprites = render_world.resource_mut::<ExtractedSprites>();
     extracted_sprites.sprites.clear();
     for (visibility, sprite, transform, handle) in sprite_query.iter() {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
         // PERF: we don't check in this function that the `Image` asset is ready, since it should be in most cases and hashing the handle is expensive
@@ -252,7 +252,7 @@ pub fn extract_sprites(
         });
     }
     for (visibility, atlas_sprite, transform, texture_atlas_handle) in atlas_query.iter() {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
         if let Some(texture_atlas) = texture_atlases.get(texture_atlas_handle) {

crates/bevy_text/src/text2d.rs

@@ -72,7 +72,7 @@ pub fn extract_text2d_sprite(
     let scale_factor = windows.scale_factor(WindowId::primary()) as f32;
 
     for (entity, visibility, text, transform, calculated_size) in text2d_query.iter() {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
         let (width, height) = (calculated_size.size.x, calculated_size.size.y);

crates/bevy_ui/src/render/mod.rs

@@ -188,7 +188,7 @@ pub fn extract_uinodes(
     let mut extracted_uinodes = render_world.resource_mut::<ExtractedUiNodes>();
     extracted_uinodes.uinodes.clear();
     for (uinode, transform, color, image, visibility, clip) in uinode_query.iter() {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
         let image = image.0.clone_weak();
@@ -289,7 +289,7 @@ pub fn extract_text_uinodes(
     let scale_factor = windows.scale_factor(WindowId::primary()) as f32;
 
     for (entity, uinode, transform, text, visibility, clip) in uinode_query.iter() {
-        if !visibility.is_visible {
+        if !visibility.is_visible() {
             continue;
         }
         // Skip if size is set to zero (e.g. when a parent is set to `Display::None`)