diff --git a/crates/bevy_pbr/src/light.rs b/crates/bevy_pbr/src/light.rs index 575c926b4ce40..1f6344445064d 100644 --- a/crates/bevy_pbr/src/light.rs +++ b/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; } diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 2509cff2554e2..fea68ebd3b820 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/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; } diff --git a/crates/bevy_render/Cargo.toml b/crates/bevy_render/Cargo.toml index 879dbbbf9af9d..5d5c9fabe83bc 100644 --- a/crates/bevy_render/Cargo.toml +++ b/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"] } diff --git a/crates/bevy_render/src/view/visibility/mod.rs b/crates/bevy_render/src/view/visibility/mod.rs index ef859404a2b72..7443947625415 100644 --- a/crates/bevy_render/src/view/visibility/mod.rs +++ b/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( } } +fn visibility_propagate_system( + mut root_query: Query<(Option<&Children>, &mut Visibility, Entity), Without>, + mut visibility_query: Query<(&mut Visibility, &Parent)>, + children_query: Query<&Children, (With, With)>, +) { + 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, With)>, + 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::(children[0]).unwrap(), + Visibility { + self_visible: true, + inherited: false, + } + ); + + assert_eq!( + *world.get::(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::(parent).unwrap(), &[child]); + assert_eq!(&**app.world.get::(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); + } +} diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index 2c51bb8f5ad47..e39f7e697fe33 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -234,7 +234,7 @@ pub fn extract_sprites( let mut extracted_sprites = render_world.resource_mut::(); 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) { diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index d0bebbad3609a..fdaade5ee410d 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/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); diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index 598dd10eaa866..a2d9122726cc2 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -188,7 +188,7 @@ pub fn extract_uinodes( let mut extracted_uinodes = render_world.resource_mut::(); 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`)