Skip to content

Commit

Permalink
Remove the type parameter from check_visibility, and only invoke it…
Browse files Browse the repository at this point in the history
… once. (#16812)

Currently, `check_visibility` is parameterized over a query filter that
specifies the type of potentially-visible object. This has the
unfortunate side effect that we need a separate system,
`mark_view_visibility_as_changed_if_necessary`, to trigger view
visibility change detection. That system is quite slow because it must
iterate sequentially over all entities in the scene.

This PR moves the query filter from `check_visibility` to a new
component, `VisibilityClass`. `VisibilityClass` stores a list of type
IDs, each corresponding to one of the query filters we used to use.
Because `check_visibility` is no longer specialized to the query filter
at the type level, Bevy now only needs to invoke it once, leading to
better performance as `check_visibility` can do change detection on the
fly rather than delegating it to a separate system.

This commit also has ergonomic improvements, as there's no need for
applications that want to add their own custom renderable components to
add specializations of the `check_visibility` system to the schedule.
Instead, they only need to ensure that the `ViewVisibility` component is
properly kept up to date. The recommended way to do this, and the way
that's demonstrated in the `custom_phase_item` and
`specialized_mesh_pipeline` examples, is to make `ViewVisibility` a
required component and to add the type ID to it in a component add hook.
This patch does this for `Mesh3d`, `Mesh2d`, `Sprite`, `Light`, and
`Node`, which means that most app code doesn't need to change at all.

Note that, although this patch has a large impact on the performance of
visibility determination, it doesn't actually improve the end-to-end
frame time of `many_cubes`. That's because the render world was already
effectively hiding the latency from
`mark_view_visibility_as_changed_if_necessary`. This patch is, however,
necessary for *further* improvements to `many_cubes` performance.

`many_cubes` trace before:
![Screenshot 2024-12-13
015318](https://github.com/user-attachments/assets/d0b1881b-fb75-4a39-b05d-1a16eabfa2c5)

`many_cubes` trace after:
![Screenshot 2024-12-13
145735](https://github.com/user-attachments/assets/0a364289-e942-41bb-9cc2-b05d07e3722d)

## Migration Guide

* `check_visibility` no longer takes a `QueryFilter`, and there's no
need to add it manually to your app schedule anymore for custom
rendering items. Instead, entities with custom renderable components
should add the appropriate type IDs to `VisibilityClass`. See
`custom_phase_item` for an example.
  • Loading branch information
pcwalton authored Dec 17, 2024
1 parent ac1faf0 commit 40df1ea
Show file tree
Hide file tree
Showing 20 changed files with 225 additions and 203 deletions.
12 changes: 4 additions & 8 deletions crates/bevy_pbr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ use bevy_render::{
render_resource::Shader,
sync_component::SyncComponentPlugin,
texture::GpuImage,
view::{check_visibility, VisibilitySystems},
view::VisibilitySystems,
ExtractSchedule, Render, RenderApp, RenderSet,
};

Expand Down Expand Up @@ -383,8 +383,7 @@ impl Plugin for PbrPlugin {
.in_set(SimulationLightSystems::AssignLightsToClusters)
.after(TransformSystem::TransformPropagate)
.after(VisibilitySystems::CheckVisibility)
.after(CameraUpdateSystem)
.ambiguous_with(VisibilitySystems::VisibilityChangeDetect),
.after(CameraUpdateSystem),
clear_directional_light_cascades
.in_set(SimulationLightSystems::UpdateDirectionalLightCascades)
.after(TransformSystem::TransformPropagate)
Expand All @@ -398,8 +397,7 @@ impl Plugin for PbrPlugin {
// We assume that no entity will be both a directional light and a spot light,
// so these systems will run independently of one another.
// FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481.
.ambiguous_with(update_spot_light_frusta)
.ambiguous_with(VisibilitySystems::VisibilityChangeDetect),
.ambiguous_with(update_spot_light_frusta),
update_point_light_frusta
.in_set(SimulationLightSystems::UpdateLightFrusta)
.after(TransformSystem::TransformPropagate)
Expand All @@ -408,7 +406,6 @@ impl Plugin for PbrPlugin {
.in_set(SimulationLightSystems::UpdateLightFrusta)
.after(TransformSystem::TransformPropagate)
.after(SimulationLightSystems::AssignLightsToClusters),
check_visibility::<WithLight>.in_set(VisibilitySystems::CheckVisibility),
(
check_dir_light_mesh_visibility,
check_point_light_mesh_visibility,
Expand All @@ -420,8 +417,7 @@ impl Plugin for PbrPlugin {
// 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
.after(VisibilitySystems::CheckVisibility)
.ambiguous_with(VisibilitySystems::VisibilityChangeDetect),
.after(VisibilitySystems::CheckVisibility),
),
);

Expand Down
6 changes: 4 additions & 2 deletions crates/bevy_pbr/src/light/directional_light.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use bevy_render::view::Visibility;
use bevy_render::view::{self, Visibility};

use super::*;

Expand Down Expand Up @@ -57,8 +57,10 @@ use super::*;
CascadeShadowConfig,
CascadesVisibleEntities,
Transform,
Visibility
Visibility,
VisibilityClass
)]
#[component(on_add = view::add_visibility_class::<LightVisibilityClass>)]
pub struct DirectionalLight {
/// The color of the light.
///
Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_pbr/src/light/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use bevy_render::{
mesh::Mesh3d,
primitives::{Aabb, CascadesFrusta, CubemapFrusta, Frustum, Sphere},
view::{
InheritedVisibility, NoFrustumCulling, RenderLayers, ViewVisibility, VisibilityRange,
VisibleEntityRanges,
InheritedVisibility, NoFrustumCulling, RenderLayers, ViewVisibility, VisibilityClass,
VisibilityRange, VisibleEntityRanges,
},
};
use bevy_transform::components::{GlobalTransform, Transform};
Expand Down Expand Up @@ -503,6 +503,9 @@ pub enum ShadowFilteringMethod {
Temporal,
}

/// The [`VisibilityClass`] used for all lights (point, directional, and spot).
pub struct LightVisibilityClass;

/// System sets used to run light-related systems.
#[derive(Debug, Hash, PartialEq, Eq, Clone, SystemSet)]
pub enum SimulationLightSystems {
Expand Down
11 changes: 9 additions & 2 deletions crates/bevy_pbr/src/light/point_light.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use bevy_render::view::Visibility;
use bevy_render::view::{self, Visibility};

use super::*;

Expand All @@ -21,7 +21,14 @@ use super::*;
/// Source: [Wikipedia](https://en.wikipedia.org/wiki/Lumen_(unit)#Lighting)
#[derive(Component, Debug, Clone, Copy, Reflect)]
#[reflect(Component, Default, Debug)]
#[require(CubemapFrusta, CubemapVisibleEntities, Transform, Visibility)]
#[require(
CubemapFrusta,
CubemapVisibleEntities,
Transform,
Visibility,
VisibilityClass
)]
#[component(on_add = view::add_visibility_class::<LightVisibilityClass>)]
pub struct PointLight {
/// The color of this light source.
pub color: Color,
Expand Down
5 changes: 3 additions & 2 deletions crates/bevy_pbr/src/light/spot_light.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use bevy_render::view::Visibility;
use bevy_render::view::{self, Visibility};

use super::*;

Expand All @@ -9,7 +9,8 @@ use super::*;
/// the transform, and can be specified with [`Transform::looking_at`](Transform::looking_at).
#[derive(Component, Debug, Clone, Copy, Reflect)]
#[reflect(Component, Default, Debug)]
#[require(Frustum, VisibleMeshEntities, Transform, Visibility)]
#[require(Frustum, VisibleMeshEntities, Transform, Visibility, VisibilityClass)]
#[component(on_add = view::add_visibility_class::<LightVisibilityClass>)]
pub struct SpotLight {
/// The color of the light.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@ pub fn queue_material_meshes<M: Material>(
}

let rangefinder = view.rangefinder3d();
for (render_entity, visible_entity) in visible_entities.iter::<With<Mesh3d>>() {
for (render_entity, visible_entity) in visible_entities.iter::<Mesh3d>() {
let Some(material_asset_id) = render_material_instances.get(visible_entity) else {
continue;
};
Expand Down
16 changes: 6 additions & 10 deletions crates/bevy_pbr/src/meshlet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use self::{
visibility_buffer_raster_node::MeshletVisibilityBufferRasterPassNode,
};
use crate::{graph::NodePbr, Material, MeshMaterial3d, PreviousGlobalTransform};
use bevy_app::{App, Plugin, PostUpdate};
use bevy_app::{App, Plugin};
use bevy_asset::{load_internal_asset, AssetApp, AssetId, Handle};
use bevy_core_pipeline::{
core_3d::graph::{Core3d, Node3d},
Expand All @@ -70,7 +70,6 @@ use bevy_ecs::{
bundle::Bundle,
component::{require, Component},
entity::Entity,
prelude::With,
query::Has,
reflect::ReflectComponent,
schedule::IntoSystemConfigs,
Expand All @@ -83,8 +82,8 @@ use bevy_render::{
renderer::RenderDevice,
settings::WgpuFeatures,
view::{
check_visibility, prepare_view_targets, InheritedVisibility, Msaa, ViewVisibility,
Visibility, VisibilitySystems,
self, prepare_view_targets, InheritedVisibility, Msaa, ViewVisibility, Visibility,
VisibilityClass,
},
ExtractSchedule, Render, RenderApp, RenderSet,
};
Expand Down Expand Up @@ -219,11 +218,7 @@ impl Plugin for MeshletPlugin {
);

app.init_asset::<MeshletMesh>()
.register_asset_loader(MeshletMeshLoader)
.add_systems(
PostUpdate,
check_visibility::<With<MeshletMesh3d>>.in_set(VisibilitySystems::CheckVisibility),
);
.register_asset_loader(MeshletMeshLoader);
}

fn finish(&self, app: &mut App) {
Expand Down Expand Up @@ -303,7 +298,8 @@ impl Plugin for MeshletPlugin {
/// The meshlet mesh equivalent of [`bevy_render::mesh::Mesh3d`].
#[derive(Component, Clone, Debug, Default, Deref, DerefMut, Reflect, PartialEq, Eq, From)]
#[reflect(Component, Default)]
#[require(Transform, PreviousGlobalTransform, Visibility)]
#[require(Transform, PreviousGlobalTransform, Visibility, VisibilityClass)]
#[component(on_add = view::add_visibility_class::<MeshletMesh3d>)]
pub struct MeshletMesh3d(pub Handle<MeshletMesh>);

impl From<MeshletMesh3d> for AssetId<MeshletMesh> {
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,7 @@ pub fn queue_prepass_material_meshes<M: Material>(
view_key |= MeshPipelineKey::MOTION_VECTOR_PREPASS;
}

for (render_entity, visible_entity) in visible_entities.iter::<With<Mesh3d>>() {
for (render_entity, visible_entity) in visible_entities.iter::<Mesh3d>() {
let Some(material_asset_id) = render_material_instances.get(visible_entity) else {
continue;
};
Expand Down
11 changes: 8 additions & 3 deletions crates/bevy_render/src/mesh/components.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use crate::{mesh::Mesh, view::Visibility};
use crate::{
mesh::Mesh,
view::{self, Visibility, VisibilityClass},
};
use bevy_asset::{AssetId, Handle};
use bevy_derive::{Deref, DerefMut};
use bevy_ecs::{component::Component, prelude::require, reflect::ReflectComponent};
Expand Down Expand Up @@ -35,7 +38,8 @@ use derive_more::derive::From;
/// ```
#[derive(Component, Clone, Debug, Default, Deref, DerefMut, Reflect, PartialEq, Eq, From)]
#[reflect(Component, Default)]
#[require(Transform, Visibility)]
#[require(Transform, Visibility, VisibilityClass)]
#[component(on_add = view::add_visibility_class::<Mesh2d>)]
pub struct Mesh2d(pub Handle<Mesh>);

impl From<Mesh2d> for AssetId<Mesh> {
Expand Down Expand Up @@ -82,7 +86,8 @@ impl From<&Mesh2d> for AssetId<Mesh> {
/// ```
#[derive(Component, Clone, Debug, Default, Deref, DerefMut, Reflect, PartialEq, Eq, From)]
#[reflect(Component, Default)]
#[require(Transform, Visibility)]
#[require(Transform, Visibility, VisibilityClass)]
#[component(on_add = view::add_visibility_class::<Mesh3d>)]
pub struct Mesh3d(pub Handle<Mesh>);

impl From<Mesh3d> for AssetId<Mesh> {
Expand Down
Loading

0 comments on commit 40df1ea

Please sign in to comment.