Skip to content

Commit c77695e

Browse files
kfc35tychedelia
authored andcommitted
fix(light despawn / pcss example): add extracted lights and shadow related components to SyncComponent for lights (bevyengine#23790)
# Objective - Fixes bevyengine#23769 ! ## Solution I `git bisect`’d the issue to bbcc1e6 which has to do with `SyncComponent`s. The problem was that not all the relevant components were being despawned when toggling an entity between different Light components within the example. Expanding `SyncComponents` for each light ensures that the lights clean up properly between toggling. I also had to review bevyengine#23713 and I connected the dots that de-spawning the light should also despawn the light view entity. Duh! ## Testing - `cargo run --example pcss --features=“experimental_pbr_pcss”`: toggling works!
1 parent b343da8 commit c77695e

2 files changed

Lines changed: 44 additions & 104 deletions

File tree

crates/bevy_pbr/src/lib.rs

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ pub mod contact_shadows;
3030
#[cfg(feature = "bevy_gltf")]
3131
mod gltf;
3232
use bevy_light::cluster::GlobalClusterSettings;
33-
use bevy_render::sync_component::SyncComponent;
33+
use bevy_render::{
34+
sync_component::SyncComponent,
35+
view::{RenderExtractedShadowMapVisibleEntities, RenderShadowMapVisibleEntities},
36+
};
3437
pub use contact_shadows::{
3538
ContactShadows, ContactShadowsBuffer, ContactShadowsPlugin, ContactShadowsUniform,
3639
ViewContactShadowsUniformOffset,
@@ -418,7 +421,9 @@ impl Plugin for PbrPlugin {
418421
render_app
419422
.world_mut()
420423
.add_observer(remove_light_view_entities);
421-
render_app.world_mut().add_observer(extracted_light_removed);
424+
render_app
425+
.world_mut()
426+
.add_observer(remove_point_and_spot_light_view_entities);
422427

423428
render_app.add_systems(
424429
Core3d,
@@ -477,16 +482,34 @@ pub fn stbn_placeholder() -> Image {
477482
}
478483

479484
impl SyncComponent<PbrPlugin> for DirectionalLight {
480-
type Target = Self;
485+
type Target = (
486+
Self,
487+
ExtractedDirectionalLight,
488+
RenderExtractedShadowMapVisibleEntities,
489+
RenderShadowMapVisibleEntities,
490+
DirectionalLightViewEntities,
491+
);
481492
}
482493
impl SyncComponent<PbrPlugin> for PointLight {
483-
type Target = Self;
494+
type Target = (
495+
Self,
496+
ExtractedPointLight,
497+
RenderExtractedShadowMapVisibleEntities,
498+
RenderShadowMapVisibleEntities,
499+
PointAndSpotLightViewEntities,
500+
);
484501
}
485502
impl SyncComponent<PbrPlugin> for SpotLight {
486-
type Target = Self;
503+
type Target = (
504+
Self,
505+
ExtractedPointLight,
506+
RenderExtractedShadowMapVisibleEntities,
507+
RenderShadowMapVisibleEntities,
508+
PointAndSpotLightViewEntities,
509+
);
487510
}
488511
impl SyncComponent<PbrPlugin> for RectLight {
489-
type Target = Self;
512+
type Target = (Self, ExtractedRectLight);
490513
}
491514
impl SyncComponent<PbrPlugin> for AmbientLight {
492515
type Target = Self;

crates/bevy_pbr/src/render/light.rs

Lines changed: 15 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ use bevy_render::mesh::allocator::MeshSlabs;
4545
use bevy_render::occlusion_culling::{
4646
OcclusionCulling, OcclusionCullingSubview, OcclusionCullingSubviewEntities,
4747
};
48-
use bevy_render::sync_world::{MainEntity, RenderEntity};
49-
use bevy_render::sync_world::{MainEntityHashMap, MainEntityHashSet};
48+
use bevy_render::sync_world::{MainEntity, MainEntityHashMap, RenderEntity};
5049
use bevy_render::view::{
5150
RenderExtractedShadowMapVisibleEntities, RenderShadowMapVisibleEntities, RenderVisibleEntities,
5251
VisibilityExtractionSystemParam,
@@ -421,17 +420,6 @@ pub fn extract_lights(
421420
&mut RenderExtractedShadowMapVisibleEntities,
422421
&mut RenderShadowMapVisibleEntities,
423422
)>,
424-
(
425-
mut removed_point_lights,
426-
mut removed_spot_lights,
427-
mut removed_directional_lights,
428-
mut removed_rect_lights,
429-
): (
430-
Extract<RemovedComponents<PointLight>>,
431-
Extract<RemovedComponents<SpotLight>>,
432-
Extract<RemovedComponents<DirectionalLight>>,
433-
Extract<RemovedComponents<RectLight>>,
434-
),
435423
) {
436424
let mapper = &visibility_extraction_system_param.mapper;
437425

@@ -453,14 +441,6 @@ pub fn extract_lights(
453441
// https://catlikecoding.com/unity/tutorials/custom-srp/point-and-spot-shadows/
454442
let point_light_texel_size = 2.0 / point_light_shadow_map.size as f32;
455443

456-
// Keep track of all entities of a type that we updated this frame, so that
457-
// we don't incorrectly remove the components later even if they show up in
458-
// `RemovedComponents`.
459-
let mut seen_point_light_main_entities = MainEntityHashSet::default();
460-
let mut seen_spot_light_main_entities = MainEntityHashSet::default();
461-
let mut seen_directional_light_main_entities = MainEntityHashSet::default();
462-
let mut seen_rect_light_main_entities = MainEntityHashSet::default();
463-
464444
for (
465445
main_entity,
466446
render_entity,
@@ -472,8 +452,6 @@ pub fn extract_lights(
472452
volumetric_light,
473453
) in point_lights.iter()
474454
{
475-
seen_point_light_main_entities.insert(main_entity.into());
476-
477455
if !view_visibility.get() {
478456
if let Ok(mut entity_commands) = commands.get_entity(render_entity) {
479457
entity_commands.remove::<ExtractedPointLight>();
@@ -586,8 +564,6 @@ pub fn extract_lights(
586564
volumetric_light,
587565
) in spot_lights.iter()
588566
{
589-
seen_spot_light_main_entities.insert(main_entity.into());
590-
591567
if !view_visibility.get() {
592568
if let Ok(mut entity_commands) = commands.get_entity(render_entity) {
593569
entity_commands.remove::<ExtractedPointLight>();
@@ -706,8 +682,6 @@ pub fn extract_lights(
706682
sun_disk,
707683
) in &directional_lights
708684
{
709-
seen_directional_light_main_entities.insert(main_entity.into());
710-
711685
if !view_visibility.get() {
712686
commands
713687
.get_entity(entity)
@@ -850,8 +824,6 @@ pub fn extract_lights(
850824
}
851825

852826
for (main_entity, render_entity, rect_light, transform, view_visibility) in &rect_lights {
853-
seen_rect_light_main_entities.insert(main_entity.into());
854-
855827
if !view_visibility.get() {
856828
if let Ok(mut entity_commands) = commands.get_entity(render_entity) {
857829
entity_commands.remove::<ExtractedRectLight>();
@@ -879,64 +851,6 @@ pub fn extract_lights(
879851
));
880852
}
881853

882-
// Remove extracted light components from entities that have had their
883-
// light components removed.
884-
remove_components::<PointLight, ExtractedPointLight>(
885-
&mut commands,
886-
mapper,
887-
&mut removed_point_lights,
888-
&seen_point_light_main_entities,
889-
);
890-
remove_components::<SpotLight, ExtractedPointLight>(
891-
&mut commands,
892-
mapper,
893-
&mut removed_spot_lights,
894-
&seen_spot_light_main_entities,
895-
);
896-
remove_components::<DirectionalLight, ExtractedDirectionalLight>(
897-
&mut commands,
898-
mapper,
899-
&mut removed_directional_lights,
900-
&seen_directional_light_main_entities,
901-
);
902-
remove_components::<RectLight, ExtractedRectLight>(
903-
&mut commands,
904-
mapper,
905-
&mut removed_rect_lights,
906-
&seen_rect_light_main_entities,
907-
);
908-
909-
// A helper function that removes a render-world component `RWC` when a
910-
// main-world component `MC` is removed.
911-
//
912-
// `seen_entities` is the list of all entities with that component that were
913-
// updated this frame. It's needed because presence in the
914-
// `RemovedComponents` table for the main-world component isn't enough to
915-
// determine whether the render-world component can be removed, as the
916-
// main-world component might have been removed and then re-added in the
917-
// same frame.
918-
fn remove_components<MC, RWC>(
919-
commands: &mut Commands,
920-
mapper: &Query<&RenderEntity>,
921-
removed_components: &mut RemovedComponents<MC>,
922-
seen_entities: &MainEntityHashSet,
923-
) where
924-
MC: Component,
925-
RWC: Component,
926-
{
927-
// As usual, only remove components if we didn't process them in the
928-
// outer extraction function, because of the possibility that the
929-
// component might have been removed and re-added in the same frame.
930-
for main_entity in removed_components.read() {
931-
if !seen_entities.contains(&MainEntity::from(main_entity))
932-
&& let Ok(render_entity) = mapper.get(main_entity)
933-
&& let Ok(mut entity_commands) = commands.get_entity(**render_entity)
934-
{
935-
entity_commands.remove::<RWC>();
936-
}
937-
}
938-
}
939-
940854
/// Clears out any shadow maps that may be present for a light with shadow
941855
/// mapping turned off.
942856
fn clear_shadow_maps(commands: &mut Commands, render_entity: Entity) {
@@ -965,17 +879,6 @@ pub(crate) fn add_light_view_entities(
965879
}
966880
}
967881

968-
/// Removes [`DirectionalLightViewEntities`] when light is removed. See
969-
/// [`add_light_view_entities`].
970-
pub(crate) fn extracted_light_removed(
971-
remove: On<Remove, ExtractedDirectionalLight>,
972-
mut commands: Commands,
973-
) {
974-
if let Ok(mut v) = commands.get_entity(remove.entity) {
975-
v.try_remove::<DirectionalLightViewEntities>();
976-
}
977-
}
978-
979882
pub(crate) fn remove_light_view_entities(
980883
remove: On<Remove, DirectionalLightViewEntities>,
981884
query: Query<&DirectionalLightViewEntities>,
@@ -992,6 +895,20 @@ pub(crate) fn remove_light_view_entities(
992895
}
993896
}
994897

898+
pub(crate) fn remove_point_and_spot_light_view_entities(
899+
remove: On<Remove, PointAndSpotLightViewEntities>,
900+
query: Query<&PointAndSpotLightViewEntities>,
901+
mut commands: Commands,
902+
) {
903+
if let Ok(entities) = query.get(remove.entity) {
904+
for e in entities.0.iter().copied() {
905+
if let Ok(mut v) = commands.get_entity(e) {
906+
v.despawn();
907+
}
908+
}
909+
}
910+
}
911+
995912
/// A component that stores the shadow maps associated with a point or spot
996913
/// light.
997914
///

0 commit comments

Comments
 (0)