Skip to content

Commit

Permalink
Visibilty Inheritance, universal ComputedVisibility and RenderLayers …
Browse files Browse the repository at this point in the history
…support (bevyengine#5310)

# Objective

Fixes bevyengine#4907. Fixes bevyengine#838. Fixes bevyengine#5089.
Supersedes bevyengine#5146. Supersedes bevyengine#2087. Supersedes bevyengine#865. Supersedes bevyengine#5114

Visibility is currently entirely local. Set a parent entity to be invisible, and the children are still visible. This makes it hard for users to hide entire hierarchies of entities.

Additionally, the semantics of `Visibility` vs `ComputedVisibility` are inconsistent across entity types. 3D meshes use `ComputedVisibility` as the "definitive" visibility component, with `Visibility` being just one data source. Sprites just use `Visibility`, which means they can't feed off of `ComputedVisibility` data, such as culling information, RenderLayers, and (added in this pr) visibility inheritance information.

## Solution

Splits `ComputedVisibilty::is_visible` into `ComputedVisibilty::is_visible_in_view` and `ComputedVisibilty::is_visible_in_hierarchy`. For each visible entity, `is_visible_in_hierarchy` is computed by propagating visibility down the hierarchy. The `ComputedVisibility::is_visible()` function combines these two booleans for the canonical "is this entity visible" function.

Additionally, all entities that have `Visibility` now also have `ComputedVisibility`.  Sprites, Lights, and UI entities now use `ComputedVisibility` when appropriate.

This means that in addition to visibility inheritance, everything using Visibility now also supports RenderLayers. Notably, Sprites (and other 2d objects) now support `RenderLayers` and work properly across multiple views.

Also note that this does increase the amount of work done per sprite. Bevymark with 100,000 sprites on `main` runs in `0.017612` seconds and this runs in `0.01902`. That is certainly a gap, but I believe the api consistency and extra functionality this buys us is worth it. See [this thread](bevyengine#5146 (comment)) for more info. Note that bevyengine#5146 in combination with bevyengine#5114 _are_ a viable alternative to this PR and _would_ perform better, but that comes at the cost of api inconsistencies and doing visibility calculations in the "wrong" place. The current visibility system does have potential for performance improvements. I would prefer to evolve that one system as a whole rather than doing custom hacks / different behaviors for each feature slice.

Here is a "split screen" example where the left camera uses RenderLayers to filter out the blue sprite.

![image](https://user-images.githubusercontent.com/2694663/178814868-2e9a2173-bf8c-4c79-8815-633899d492c3.png)


Note that this builds directly on bevyengine#5146 and that @james7132 deserves the credit for the baseline visibility inheritance work. This pr moves the inherited visibility field into `ComputedVisibility`, then does the additional work of porting everything to `ComputedVisibility`. See my [comments here](bevyengine#5146 (comment)) for rationale. 

## Follow up work

* Now that lights use ComputedVisibility, VisibleEntities now includes "visible lights" in the entity list. Functionally not a problem as we use queries to filter the list down in the desired context. But we should consider splitting this out into a separate`VisibleLights` collection for both clarity and performance reasons. And _maybe_ even consider scoping `VisibleEntities` down to `VisibleMeshes`?.
* Investigate alternative sprite rendering impls (in combination with visibility system tweaks) that avoid re-generating a per-view fixedbitset of visible entities every frame, then checking each ExtractedEntity. This is where most of the performance overhead lives. Ex: we could generate ExtractedEntities per-view using the VisibleEntities list, avoiding the need for the bitset.
* Should ComputedVisibility use bitflags under the hood? This would cut down on the size of the component, potentially speed up the `is_visible()` function, and allow us to cheaply expand ComputedVisibility with more data (ex: split out local visibility and parent visibility, add more culling classes, etc).
---

## Changelog

* ComputedVisibility now takes hierarchy visibility into account.
* 2D, UI and Light entities now use the ComputedVisibility component.

## Migration Guide

If you were previously reading `Visibility::is_visible` as the "actual visibility" for sprites or lights, use `ComputedVisibilty::is_visible()` instead:

```rust
// before (0.7)
fn system(query: Query<&Visibility>) {
  for visibility in query.iter() {
    if visibility.is_visible {
       log!("found visible entity");
    }
  }
}

// after (0.8)
fn system(query: Query<&ComputedVisibility>) {
  for visibility in query.iter() {
    if visibility.is_visible() {
       log!("found visible entity");
    }
  }
}
``` 


Co-authored-by: Carter Anderson <mcanders1@gmail.com>
  • Loading branch information
james7132 and cart committed Oct 28, 2022
1 parent cc95ebc commit 8f91c12
Show file tree
Hide file tree
Showing 17 changed files with 472 additions and 114 deletions.
6 changes: 6 additions & 0 deletions crates/bevy_pbr/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub struct PointLightBundle {
pub global_transform: GlobalTransform,
/// Enables or disables the light
pub visibility: Visibility,
/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering
pub computed_visibility: ComputedVisibility,
}

/// A component bundle for spot light entities
Expand All @@ -85,6 +87,8 @@ pub struct SpotLightBundle {
pub global_transform: GlobalTransform,
/// Enables or disables the light
pub visibility: Visibility,
/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering
pub computed_visibility: ComputedVisibility,
}

/// A component bundle for [`DirectionalLight`] entities.
Expand All @@ -97,4 +101,6 @@ pub struct DirectionalLightBundle {
pub global_transform: GlobalTransform,
/// Enables or disables the light
pub visibility: Visibility,
/// Algorithmically-computed indication of whether an entity is visible and should be extracted for rendering
pub computed_visibility: ComputedVisibility,
}
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,16 @@ impl Plugin for PbrPlugin {
assign_lights_to_clusters
.label(SimulationLightSystems::AssignLightsToClusters)
.after(TransformSystem::TransformPropagate)
.after(VisibilitySystems::CheckVisibility)
.after(CameraUpdateSystem)
.after(ModifiesWindows),
)
.add_system_to_stage(
CoreStage::PostUpdate,
update_directional_light_frusta
.label(SimulationLightSystems::UpdateLightFrusta)
// This must run after CheckVisibility because it relies on ComputedVisibility::is_visible()
.after(VisibilitySystems::CheckVisibility)
.after(TransformSystem::TransformPropagate),
)
.add_system_to_stage(
Expand Down
60 changes: 28 additions & 32 deletions crates/bevy_pbr/src/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use bevy_render::{
primitives::{Aabb, CubemapFrusta, Frustum, Plane, Sphere},
render_resource::BufferBindingType,
renderer::RenderDevice,
view::{ComputedVisibility, RenderLayers, Visibility, VisibleEntities},
view::{ComputedVisibility, RenderLayers, VisibleEntities},
};
use bevy_transform::components::GlobalTransform;
use bevy_utils::tracing::warn;
Expand Down Expand Up @@ -793,8 +793,8 @@ pub(crate) fn assign_lights_to_clusters(
&mut Clusters,
Option<&mut VisiblePointLights>,
)>,
point_lights_query: Query<(Entity, &GlobalTransform, &PointLight, &Visibility)>,
spot_lights_query: Query<(Entity, &GlobalTransform, &SpotLight, &Visibility)>,
point_lights_query: Query<(Entity, &GlobalTransform, &PointLight, &ComputedVisibility)>,
spot_lights_query: Query<(Entity, &GlobalTransform, &SpotLight, &ComputedVisibility)>,
mut lights: Local<Vec<PointLightAssignmentData>>,
mut cluster_aabb_spheres: Local<Vec<Option<Sphere>>>,
mut max_point_lights_warning_emitted: Local<bool>,
Expand All @@ -811,7 +811,7 @@ pub(crate) fn assign_lights_to_clusters(
lights.extend(
point_lights_query
.iter()
.filter(|(.., visibility)| visibility.is_visible)
.filter(|(.., visibility)| visibility.is_visible())
.map(
|(entity, transform, point_light, _visibility)| PointLightAssignmentData {
entity,
Expand All @@ -826,7 +826,7 @@ pub(crate) fn assign_lights_to_clusters(
lights.extend(
spot_lights_query
.iter()
.filter(|(.., visibility)| visibility.is_visible)
.filter(|(.., visibility)| visibility.is_visible())
.map(
|(entity, transform, spot_light, _visibility)| PointLightAssignmentData {
entity,
Expand Down Expand Up @@ -1415,7 +1415,7 @@ pub fn update_directional_light_frusta(
&GlobalTransform,
&DirectionalLight,
&mut Frustum,
&Visibility,
&ComputedVisibility,
),
Or<(Changed<GlobalTransform>, Changed<DirectionalLight>)>,
>,
Expand All @@ -1424,7 +1424,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;
}

Expand Down Expand Up @@ -1541,45 +1541,43 @@ pub fn check_light_mesh_visibility(
&Frustum,
&mut VisibleEntities,
Option<&RenderLayers>,
&Visibility,
&ComputedVisibility,
),
Without<SpotLight>,
>,
mut visible_entity_query: Query<
(
Entity,
&Visibility,
&mut ComputedVisibility,
Option<&RenderLayers>,
Option<&Aabb>,
Option<&GlobalTransform>,
),
Without<NotShadowCaster>,
(Without<NotShadowCaster>, Without<DirectionalLight>),
>,
) {
// Directonal lights
for (directional_light, frustum, mut visible_entities, maybe_view_mask, visibility) in
&mut directional_lights
// Directional lights
for (
directional_light,
frustum,
mut visible_entities,
maybe_view_mask,
light_computed_visibility,
) in &mut directional_lights
{
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 || !light_computed_visibility.is_visible() {
continue;
}

let view_mask = maybe_view_mask.copied().unwrap_or_default();

for (
entity,
visibility,
mut computed_visibility,
maybe_entity_mask,
maybe_aabb,
maybe_transform,
) in &mut visible_entity_query
for (entity, mut computed_visibility, maybe_entity_mask, maybe_aabb, maybe_transform) in
&mut visible_entity_query
{
if !visibility.is_visible {
if !computed_visibility.is_visible_in_hierarchy() {
continue;
}

Expand All @@ -1595,7 +1593,7 @@ pub fn check_light_mesh_visibility(
}
}

computed_visibility.is_visible = true;
computed_visibility.set_visible_in_view();
visible_entities.entities.push(entity);
}

Expand Down Expand Up @@ -1631,14 +1629,13 @@ pub fn check_light_mesh_visibility(

for (
entity,
visibility,
mut computed_visibility,
maybe_entity_mask,
maybe_aabb,
maybe_transform,
) in &mut visible_entity_query
{
if !visibility.is_visible {
if !computed_visibility.is_visible_in_hierarchy() {
continue;
}

Expand All @@ -1660,12 +1657,12 @@ pub fn check_light_mesh_visibility(
.zip(cubemap_visible_entities.iter_mut())
{
if frustum.intersects_obb(aabb, &model_to_world, true) {
computed_visibility.is_visible = true;
computed_visibility.set_visible_in_view();
visible_entities.entities.push(entity);
}
}
} else {
computed_visibility.is_visible = true;
computed_visibility.set_visible_in_view();
for visible_entities in cubemap_visible_entities.iter_mut() {
visible_entities.entities.push(entity);
}
Expand Down Expand Up @@ -1695,14 +1692,13 @@ pub fn check_light_mesh_visibility(

for (
entity,
visibility,
mut computed_visibility,
maybe_entity_mask,
maybe_aabb,
maybe_transform,
) in visible_entity_query.iter_mut()
{
if !visibility.is_visible {
if !computed_visibility.is_visible_in_hierarchy() {
continue;
}

Expand All @@ -1720,11 +1716,11 @@ pub fn check_light_mesh_visibility(
}

if frustum.intersects_obb(aabb, &model_to_world, true) {
computed_visibility.is_visible = true;
computed_visibility.set_visible_in_view();
visible_entities.entities.push(entity);
}
} else {
computed_visibility.is_visible = true;
computed_visibility.set_visible_in_view();
visible_entities.entities.push(entity);
}
}
Expand Down
37 changes: 30 additions & 7 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ use bevy_render::{
renderer::{RenderContext, RenderDevice, RenderQueue},
texture::*,
view::{
ExtractedView, ViewUniform, ViewUniformOffset, ViewUniforms, Visibility, VisibleEntities,
ComputedVisibility, ExtractedView, ViewUniform, ViewUniformOffset, ViewUniforms,
VisibleEntities,
},
Extract,
};
Expand Down Expand Up @@ -411,16 +412,30 @@ pub fn extract_lights(
point_light_shadow_map: Extract<Res<PointLightShadowMap>>,
directional_light_shadow_map: Extract<Res<DirectionalLightShadowMap>>,
global_point_lights: Extract<Res<GlobalVisiblePointLights>>,
point_lights: Extract<Query<(&PointLight, &CubemapVisibleEntities, &GlobalTransform)>>,
spot_lights: Extract<Query<(&SpotLight, &VisibleEntities, &GlobalTransform)>>,
point_lights: Extract<
Query<(
&PointLight,
&CubemapVisibleEntities,
&GlobalTransform,
&ComputedVisibility,
)>,
>,
spot_lights: Extract<
Query<(
&SpotLight,
&VisibleEntities,
&GlobalTransform,
&ComputedVisibility,
)>,
>,
directional_lights: Extract<
Query<
(
Entity,
&DirectionalLight,
&VisibleEntities,
&GlobalTransform,
&Visibility,
&ComputedVisibility,
),
Without<SpotLight>,
>,
Expand All @@ -447,7 +462,12 @@ pub fn extract_lights(

let mut point_lights_values = Vec::with_capacity(*previous_point_lights_len);
for entity in global_point_lights.iter().copied() {
if let Ok((point_light, cubemap_visible_entities, transform)) = point_lights.get(entity) {
if let Ok((point_light, cubemap_visible_entities, transform, visibility)) =
point_lights.get(entity)
{
if !visibility.is_visible() {
continue;
}
// TODO: This is very much not ideal. We should be able to re-use the vector memory.
// However, since exclusive access to the main world in extract is ill-advised, we just clone here.
let render_cubemap_visible_entities = cubemap_visible_entities.clone();
Expand Down Expand Up @@ -481,7 +501,10 @@ pub fn extract_lights(

let mut spot_lights_values = Vec::with_capacity(*previous_spot_lights_len);
for entity in global_point_lights.iter().copied() {
if let Ok((spot_light, visible_entities, transform)) = spot_lights.get(entity) {
if let Ok((spot_light, visible_entities, transform, visibility)) = spot_lights.get(entity) {
if !visibility.is_visible() {
continue;
}
// TODO: This is very much not ideal. We should be able to re-use the vector memory.
// However, since exclusive access to the main world in extract is ill-advised, we just clone here.
let render_visible_entities = visible_entities.clone();
Expand Down Expand Up @@ -522,7 +545,7 @@ pub fn extract_lights(
for (entity, directional_light, visible_entities, transform, visibility) in
directional_lights.iter()
{
if !visibility.is_visible {
if !visibility.is_visible() {
continue;
}

Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ pub fn extract_meshes(
) {
let mut caster_commands = Vec::with_capacity(*prev_caster_commands_len);
let mut not_caster_commands = Vec::with_capacity(*prev_not_caster_commands_len);
let visible_meshes = meshes_query.iter().filter(|(_, vis, ..)| vis.is_visible);
let visible_meshes = meshes_query.iter().filter(|(_, vis, ..)| vis.is_visible());

for (entity, _, transform, handle, not_receiver, not_caster) in visible_meshes {
let transform = transform.compute_matrix();
Expand Down Expand Up @@ -224,7 +224,7 @@ pub fn extract_skinned_meshes(
let mut last_start = 0;

for (entity, computed_visibility, skin) in query.iter() {
if !computed_visibility.is_visible {
if !computed_visibility.is_visible() {
continue;
}
// PERF: This can be expensive, can we move this to prepare?
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_render/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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_log = { path = "../bevy_log", 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" }
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/extract_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ fn extract_visible_components<C: ExtractComponent>(
) {
let mut values = Vec::with_capacity(*previous_len);
for (entity, computed_visibility, query_item) in query.iter_mut() {
if computed_visibility.is_visible {
if computed_visibility.is_visible() {
values.push((entity, (C::extract_component(query_item),)));
}
}
Expand Down
Loading

0 comments on commit 8f91c12

Please sign in to comment.