Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added visibility inheritance #2087

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
196 changes: 193 additions & 3 deletions crates/bevy_render/src/camera/visible_entities.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
use super::{Camera, DepthCalculation};
use crate::{draw::OutsideFrustum, prelude::Visible};
use bevy_core::FloatOrd;
use bevy_ecs::{entity::Entity, query::Without, reflect::ReflectComponent, system::Query};
use bevy_ecs::{
entity::Entity,
query::{Changed, Or, With, Without},
reflect::ReflectComponent,
system::{Commands, Query},
};
use bevy_reflect::Reflect;
use bevy_transform::prelude::GlobalTransform;
use bevy_transform::prelude::{Children, GlobalTransform, Parent};

// This struct reflects Visible and is used to store the effective value.
// The only one reason why it's not stored in the original struct is the `is_transparent` field.
// Having both that field and the effective value in Visible complicates creation of that struct.
#[derive(Default, Debug, Reflect)]
#[reflect(Component)]
pub struct VisibleEffective {
#[reflect(ignore)]
pub is_visible: bool,
#[reflect(ignore)]
pub is_transparent: bool,
}

#[derive(Debug)]
pub struct VisibleEntity {
Expand Down Expand Up @@ -197,14 +214,89 @@ mod rendering_mask_tests {
}
}

//Computes effective visibility for entities.
//
//In hirerarchies the actual visiblity of an entity isn't only the current value,
//but also depends on ancestors of the entity./ To avoid traversing each hierarchy
//and determine the effective visibility for each entity, this system listens to
//visiblity and hierarchy changes and only then computes a value to be cached and
//used by other systems.
pub fn visible_effective_system(
children_query: Query<&Children>,
changes_query: Query<
(Entity, Option<&Parent>),
(With<Visible>, Or<(Changed<Visible>, Changed<Parent>)>),
>,
mut visible_query: Query<(&Visible, Option<&mut VisibleEffective>)>,
mut commands: Commands,
) {
fn update_effective(
entity: Entity,
is_visible_parent: bool,
children_query: &Query<&Children>,
mut visible_query: &mut Query<(&Visible, Option<&mut VisibleEffective>)>,
mut commands: &mut Commands,
) {
if let Ok((visible, maybe_visible_effective)) = visible_query.get_mut(entity) {
let is_visible = visible.is_visible & is_visible_parent;
if let Some(mut visible_effective) = maybe_visible_effective {
visible_effective.is_transparent = visible.is_transparent;

if visible_effective.is_visible == is_visible {
return;
}

visible_effective.is_visible = is_visible;
} else {
commands.entity(entity).insert(VisibleEffective {
is_visible,
is_transparent: visible.is_transparent,
});
}

if let Ok(children) = children_query.get(entity) {
for child in children.iter() {
update_effective(
*child,
is_visible,
cart marked this conversation as resolved.
Show resolved Hide resolved
&children_query,
&mut visible_query,
&mut commands,
);
}
}
}
}

for (entity, maybe_parent) in changes_query.iter() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will end up being extremely expensive when constructing hierarchies (or when many nodes change on a given update). When all nodes have changed, it will walk each entity in the hierarchy's "subtree" (which is exponential). We need to do this in linear time for it to be workable (aka visit each node exactly once).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not obvious, but the code already handles that. The following lines check that the current entity as no parent or has an effective visibility already. Therefore, for a newly created subtree only the topmost node satisfies the condition.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though on it a bit and what can be done here is a better filter for changes_query, so an entity will be included if:

  • its visibility changed;
  • its parent changed and it already has effective visibility.

if let Ok(is_visible) = match maybe_parent {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes aren't guaranteed to iterate in hierarchy-order, which means that if we were to fix this impl to propagate "effective visibility" and only visit each node once, it might not be correct if we propagate in the wrong order.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly for that reason I do not update child entities which parent has no effective visibility. An advantage of the current implementation is that it completely avoids allocations to collect roots first. All spawned subtrees will be traversed only once.

None => Ok(true),
Some(parent) => visible_query
.get_component::<VisibleEffective>(parent.0)
.map(|v| v.is_visible),
} {
update_effective(
entity,
is_visible,
&children_query,
&mut visible_query,
&mut commands,
);
}
}
}

pub fn visible_entities_system(
mut camera_query: Query<(
&Camera,
&GlobalTransform,
&mut VisibleEntities,
Option<&RenderLayers>,
)>,
visible_query: Query<(Entity, &Visible, Option<&RenderLayers>), Without<OutsideFrustum>>,
visible_query: Query<
(Entity, &VisibleEffective, Option<&RenderLayers>),
Without<OutsideFrustum>,
>,
visible_transform_query: Query<&GlobalTransform, Without<OutsideFrustum>>,
) {
for (camera, camera_global_transform, mut visible_entities, maybe_camera_mask) in
Expand Down Expand Up @@ -258,3 +350,101 @@ pub fn visible_entities_system(
// to prevent holding unneeded memory
}
}

#[cfg(test)]
mod test {
use super::*;
use bevy_ecs::{
schedule::{Schedule, Stage, SystemStage},
system::IntoSystem,
world::World,
};
use bevy_transform::hierarchy::{parent_update_system, BuildWorldChildren};

#[test]
fn propagates_visibility() {
YohDeadfall marked this conversation as resolved.
Show resolved Hide resolved
let mut world = World::default();
let mut update_stage = SystemStage::parallel();
update_stage.add_system(parent_update_system.system());
update_stage.add_system(visible_effective_system.system());

let mut schedule = Schedule::default();
schedule.add_stage("update", update_stage);

let mut child1 = Option::<Entity>::None;
let mut child2 = Option::<Entity>::None;
let parent = world
.spawn()
.insert(Visible {
is_visible: false,
is_transparent: false,
})
.with_children(|parent| {
child1 = Some(
parent
.spawn()
.insert(Visible::default())
.with_children(|parent| {
child2 = Some(parent.spawn().insert(Visible::default()).id())
})
.id(),
)
})
.id();

let child1 = child1.unwrap();
let child2 = child2.unwrap();

schedule.run(&mut world);
assert_eq!(false, is_visible(&world, parent));
assert_eq!(false, is_visible(&world, child1));
assert_eq!(false, is_visible(&world, child2));

world
.get_mut::<Visible>(parent)
.map(|mut v| v.is_visible = true)
.unwrap();

schedule.run(&mut world);
assert_eq!(true, is_visible(&world, parent));
assert_eq!(true, is_visible(&world, child1));
assert_eq!(true, is_visible(&world, child2));

world
.get_mut::<Visible>(child1)
.map(|mut v| v.is_visible = false)
.unwrap();

schedule.run(&mut world);
assert_eq!(true, is_visible(&world, parent));
assert_eq!(false, is_visible(&world, child1));
assert_eq!(false, is_visible(&world, child2));

world
.get_mut::<Visible>(parent)
.map(|mut v| v.is_visible = false)
.unwrap();

schedule.run(&mut world);
assert_eq!(false, is_visible(&world, parent));
assert_eq!(false, is_visible(&world, child1));
assert_eq!(false, is_visible(&world, child2));

world
.get_mut::<Visible>(parent)
.map(|mut v| v.is_visible = true)
.unwrap();

schedule.run(&mut world);
assert_eq!(true, is_visible(&world, parent));
assert_eq!(false, is_visible(&world, child1));
assert_eq!(false, is_visible(&world, child2));

fn is_visible(world: &World, entity: Entity) -> bool {
world
.get::<VisibleEffective>(entity)
.map(|v| v.is_visible)
.unwrap()
}
}
}
9 changes: 8 additions & 1 deletion crates/bevy_render/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use bevy_asset::{AddAsset, AssetStage};
use bevy_ecs::schedule::{StageLabel, SystemLabel};
use camera::{
ActiveCameras, Camera, DepthCalculation, OrthographicProjection, PerspectiveProjection,
RenderLayers, ScalingMode, VisibleEntities, WindowOrigin,
RenderLayers, ScalingMode, VisibleEffective, VisibleEntities, WindowOrigin,
};
use pipeline::{
IndexFormat, PipelineCompiler, PipelineDescriptor, PipelineSpecialization, PrimitiveTopology,
Expand Down Expand Up @@ -140,6 +140,7 @@ impl Plugin for RenderPlugin {
.register_type::<DepthCalculation>()
.register_type::<Draw>()
.register_type::<Visible>()
.register_type::<VisibleEffective>()
.register_type::<OutsideFrustum>()
.register_type::<RenderPipelines>()
.register_type::<OrthographicProjection>()
Expand Down Expand Up @@ -183,6 +184,12 @@ impl Plugin for RenderPlugin {
.system()
.before(RenderSystem::VisibleEntities),
)
.add_system_to_stage(
CoreStage::PostUpdate,
camera::visible_effective_system
.system()
.before(RenderSystem::VisibleEntities),
)
.add_system_to_stage(
CoreStage::PostUpdate,
camera::visible_entities_system
Expand Down