Skip to content

Commit 36c6f29

Browse files
Lighting Should Only hold Vec<Entity> instead of TypeId<Vec<Entity>> (#14073)
# Objective - After #13894, I noticed the performance of `many_lights `dropped from 120+ to 60+. I reviewed the PR but couldn't identify any mistakes. After profiling, I discovered that `Hashmap::Clone `was very slow when its not empty, causing `extract_light` to increase from 3ms to 8ms. - Lighting only checks visibility for 3D Meshes. We don't need to maintain a TypeIdMap for this, as it not only impacts performance negatively but also reduces ergonomics. ## Solution - use VisibleMeshEntities for lighint visibility checking. ## Performance cargo run --release --example many_lights --features bevy/trace_tracy name="bevy_pbr::light::check_point_light_mesh_visibility"} ![image](https://github.com/bevyengine/bevy/assets/45868716/8bad061a-f936-45a0-9bb9-4fbdaceec08b) system{name="bevy_pbr::render::light::extract_lights"} ![image](https://github.com/bevyengine/bevy/assets/45868716/ca75b46c-b4ad-45d3-8c8d-66442447b753) ## Migration Guide > now `SpotLightBundle` , `CascadesVisibleEntities `and `CubemapVisibleEntities `use VisibleMeshEntities instead of `VisibleEntities` --------- Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
1 parent e13c72d commit 36c6f29

File tree

3 files changed

+36
-30
lines changed

3 files changed

+36
-30
lines changed

crates/bevy_pbr/src/bundle.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,14 @@ use crate::{
33
StandardMaterial,
44
};
55
use bevy_asset::Handle;
6-
use bevy_ecs::entity::EntityHashMap;
6+
use bevy_derive::{Deref, DerefMut};
7+
use bevy_ecs::entity::{Entity, EntityHashMap};
78
use bevy_ecs::{bundle::Bundle, component::Component, reflect::ReflectComponent};
89
use bevy_reflect::Reflect;
910
use bevy_render::{
1011
mesh::Mesh,
1112
primitives::{CascadesFrusta, CubemapFrusta, Frustum},
12-
view::{InheritedVisibility, ViewVisibility, Visibility, VisibleEntities},
13+
view::{InheritedVisibility, ViewVisibility, Visibility},
1314
};
1415
use bevy_transform::components::{GlobalTransform, Transform};
1516

@@ -45,27 +46,37 @@ impl<M: Material> Default for MaterialMeshBundle<M> {
4546
}
4647
}
4748

49+
/// Collection of mesh entities visible for 3D lighting.
50+
/// This component contains all mesh entities visible from the current light view.
51+
/// The collection is updated automatically by [`crate::SimulationLightSystems`].
52+
#[derive(Component, Clone, Debug, Default, Reflect, Deref, DerefMut)]
53+
#[reflect(Component)]
54+
pub struct VisibleMeshEntities {
55+
#[reflect(ignore)]
56+
pub entities: Vec<Entity>,
57+
}
58+
4859
#[derive(Component, Clone, Debug, Default, Reflect)]
4960
#[reflect(Component)]
5061
pub struct CubemapVisibleEntities {
5162
#[reflect(ignore)]
52-
data: [VisibleEntities; 6],
63+
data: [VisibleMeshEntities; 6],
5364
}
5465

5566
impl CubemapVisibleEntities {
56-
pub fn get(&self, i: usize) -> &VisibleEntities {
67+
pub fn get(&self, i: usize) -> &VisibleMeshEntities {
5768
&self.data[i]
5869
}
5970

60-
pub fn get_mut(&mut self, i: usize) -> &mut VisibleEntities {
71+
pub fn get_mut(&mut self, i: usize) -> &mut VisibleMeshEntities {
6172
&mut self.data[i]
6273
}
6374

64-
pub fn iter(&self) -> impl DoubleEndedIterator<Item = &VisibleEntities> {
75+
pub fn iter(&self) -> impl DoubleEndedIterator<Item = &VisibleMeshEntities> {
6576
self.data.iter()
6677
}
6778

68-
pub fn iter_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut VisibleEntities> {
79+
pub fn iter_mut(&mut self) -> impl DoubleEndedIterator<Item = &mut VisibleMeshEntities> {
6980
self.data.iter_mut()
7081
}
7182
}
@@ -75,7 +86,7 @@ impl CubemapVisibleEntities {
7586
pub struct CascadesVisibleEntities {
7687
/// Map of view entity to the visible entities for each cascade frustum.
7788
#[reflect(ignore)]
78-
pub entities: EntityHashMap<Vec<VisibleEntities>>,
89+
pub entities: EntityHashMap<Vec<VisibleMeshEntities>>,
7990
}
8091

8192
/// A component bundle for [`PointLight`] entities.
@@ -98,7 +109,7 @@ pub struct PointLightBundle {
98109
#[derive(Debug, Bundle, Default, Clone)]
99110
pub struct SpotLightBundle {
100111
pub spot_light: SpotLight,
101-
pub visible_entities: VisibleEntities,
112+
pub visible_entities: VisibleMeshEntities,
102113
pub frustum: Frustum,
103114
pub transform: Transform,
104115
pub global_transform: GlobalTransform,

crates/bevy_pbr/src/light/mod.rs

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,7 @@ use bevy_render::{
1111
mesh::Mesh,
1212
primitives::{Aabb, CascadesFrusta, CubemapFrusta, Frustum, Sphere},
1313
view::{
14-
InheritedVisibility, RenderLayers, ViewVisibility, VisibilityRange, VisibleEntities,
15-
VisibleEntityRanges, WithMesh,
14+
InheritedVisibility, RenderLayers, ViewVisibility, VisibilityRange, VisibleEntityRanges,
1615
},
1716
};
1817
use bevy_transform::components::{GlobalTransform, Transform};
@@ -100,7 +99,7 @@ impl Default for PointLightShadowMap {
10099
}
101100

102101
/// A convenient alias for `Or<(With<PointLight>, With<SpotLight>,
103-
/// With<DirectionalLight>)>`, for use with [`VisibleEntities`].
102+
/// With<DirectionalLight>)>`, for use with [`bevy_render::view::VisibleEntities`].
104103
pub type WithLight = Or<(With<PointLight>, With<SpotLight>, With<DirectionalLight>)>;
105104

106105
/// Controls the resolution of [`DirectionalLight`] shadow maps.
@@ -698,9 +697,7 @@ pub fn check_dir_light_mesh_visibility(
698697
match frusta.frusta.get(view) {
699698
Some(view_frusta) => {
700699
cascade_view_entities.resize(view_frusta.len(), Default::default());
701-
cascade_view_entities
702-
.iter_mut()
703-
.for_each(VisibleEntities::clear::<WithMesh>);
700+
cascade_view_entities.iter_mut().for_each(|x| x.clear());
704701
}
705702
None => views_to_remove.push(*view),
706703
};
@@ -709,7 +706,7 @@ pub fn check_dir_light_mesh_visibility(
709706
visible_entities
710707
.entities
711708
.entry(*view)
712-
.or_insert_with(|| vec![VisibleEntities::default(); frusta.len()]);
709+
.or_insert_with(|| vec![VisibleMeshEntities::default(); frusta.len()]);
713710
}
714711

715712
for v in views_to_remove {
@@ -790,7 +787,6 @@ pub fn check_dir_light_mesh_visibility(
790787
.get_mut(view)
791788
.unwrap()
792789
.iter_mut()
793-
.map(VisibleEntities::get_mut::<WithMesh>)
794790
.zip(entities.iter_mut())
795791
.for_each(|(dst, source)| {
796792
dst.append(source);
@@ -801,7 +797,7 @@ pub fn check_dir_light_mesh_visibility(
801797
for (_, cascade_view_entities) in &mut visible_entities.entities {
802798
cascade_view_entities
803799
.iter_mut()
804-
.map(VisibleEntities::get_mut::<WithMesh>)
800+
.map(DerefMut::deref_mut)
805801
.for_each(shrink_entities);
806802
}
807803
}
@@ -833,7 +829,7 @@ pub fn check_point_light_mesh_visibility(
833829
&SpotLight,
834830
&GlobalTransform,
835831
&Frustum,
836-
&mut VisibleEntities,
832+
&mut VisibleMeshEntities,
837833
Option<&RenderLayers>,
838834
)>,
839835
mut visible_entity_query: Query<
@@ -869,7 +865,7 @@ pub fn check_point_light_mesh_visibility(
869865
)) = point_lights.get_mut(light_entity)
870866
{
871867
for visible_entities in cubemap_visible_entities.iter_mut() {
872-
visible_entities.clear::<WithMesh>();
868+
visible_entities.entities.clear();
873869
}
874870

875871
// NOTE: If shadow mapping is disabled for the light then it must have no visible entities
@@ -940,21 +936,20 @@ pub fn check_point_light_mesh_visibility(
940936
for entities in cubemap_visible_entities_queue.iter_mut() {
941937
cubemap_visible_entities
942938
.iter_mut()
943-
.map(VisibleEntities::get_mut::<WithMesh>)
944939
.zip(entities.iter_mut())
945-
.for_each(|(dst, source)| dst.append(source));
940+
.for_each(|(dst, source)| dst.entities.append(source));
946941
}
947942

948943
for visible_entities in cubemap_visible_entities.iter_mut() {
949-
shrink_entities(visible_entities.get_mut::<WithMesh>());
944+
shrink_entities(visible_entities);
950945
}
951946
}
952947

953948
// Spot lights
954949
if let Ok((point_light, transform, frustum, mut visible_entities, maybe_view_mask)) =
955950
spot_lights.get_mut(light_entity)
956951
{
957-
visible_entities.clear::<WithMesh>();
952+
visible_entities.clear();
958953

959954
// NOTE: If shadow mapping is disabled for the light then it must have no visible entities
960955
if !point_light.shadows_enabled {
@@ -1015,10 +1010,10 @@ pub fn check_point_light_mesh_visibility(
10151010
);
10161011

10171012
for entities in spot_visible_entities_queue.iter_mut() {
1018-
visible_entities.get_mut::<WithMesh>().append(entities);
1013+
visible_entities.append(entities);
10191014
}
10201015

1021-
shrink_entities(visible_entities.get_mut::<WithMesh>());
1016+
shrink_entities(visible_entities.deref_mut());
10221017
}
10231018
}
10241019
}

crates/bevy_pbr/src/render/light.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ use bevy_render::{
1515
render_resource::*,
1616
renderer::{RenderContext, RenderDevice, RenderQueue},
1717
texture::*,
18-
view::{ExtractedView, RenderLayers, ViewVisibility, VisibleEntities, WithMesh},
18+
view::{ExtractedView, RenderLayers, ViewVisibility},
1919
Extract,
2020
};
2121
use bevy_transform::{components::GlobalTransform, prelude::Transform};
@@ -186,7 +186,7 @@ pub fn extract_lights(
186186
spot_lights: Extract<
187187
Query<(
188188
&SpotLight,
189-
&VisibleEntities,
189+
&VisibleMeshEntities,
190190
&GlobalTransform,
191191
&ViewVisibility,
192192
&Frustum,
@@ -1174,7 +1174,7 @@ pub fn queue_shadows<M: Material>(
11741174
mut view_light_entities: Query<&LightEntity>,
11751175
point_light_entities: Query<&CubemapVisibleEntities, With<ExtractedPointLight>>,
11761176
directional_light_entities: Query<&CascadesVisibleEntities, With<ExtractedDirectionalLight>>,
1177-
spot_light_entities: Query<&VisibleEntities, With<ExtractedPointLight>>,
1177+
spot_light_entities: Query<&VisibleMeshEntities, With<ExtractedPointLight>>,
11781178
) where
11791179
M::Data: PartialEq + Eq + Hash + Clone,
11801180
{
@@ -1218,7 +1218,7 @@ pub fn queue_shadows<M: Material>(
12181218
// NOTE: Lights with shadow mapping disabled will have no visible entities
12191219
// so no meshes will be queued
12201220

1221-
for entity in visible_entities.iter::<WithMesh>().copied() {
1221+
for entity in visible_entities.iter().copied() {
12221222
let Some(mesh_instance) = render_mesh_instances.render_mesh_queue_data(entity)
12231223
else {
12241224
continue;

0 commit comments

Comments
 (0)