From 61350cd36f505c662624307f07bcdb2345973b9d Mon Sep 17 00:00:00 2001 From: akimakinai <105044389+akimakinai@users.noreply.github.com> Date: Sun, 20 Oct 2024 00:13:39 +0900 Subject: [PATCH] Remove components if not extracted (#15948) # Objective - Fixes https://github.com/bevyengine/bevy/issues/15871 (Camera is done in #15946) ## Solution - Do the same as #15904 for other extraction systems - Added missing `SyncComponentPlugin` for DOF, TAA, and SSAO (According to the [documentation](https://dev-docs.bevyengine.org/bevy/render/sync_component/struct.SyncComponentPlugin.html), this plugin "needs to be added for manual extraction implementations." We may need to check this is done.) ## Testing Modified example locally to add toggles if not exist. - [x] DOF - toggling DOF component and perspective in `depth_of_field` example - [x] TAA - toggling `Camera.is_active` and TAA component - [x] clusters - not entirely sure, toggling `Camera.is_active` in `many_lights` example (no crash/glitch even without this PR) - [x] previous_view - toggling `Camera.is_active` in `skybox` (no crash/glitch even without this PR) - [x] lights - toggling `Visibility` of `DirectionalLight` in `lighting` example - [x] SSAO - toggling `Camera.is_active` and SSAO component in `ssao` example - [x] default UI camera view - toggling `Camera.is_active` (nop without #15946 because UI defaults to some camera even if `DefaultCameraView` is not there) - [x] volumetric fog - toggling existence of volumetric light. Looks like optimization, no change in behavior/visuals --- crates/bevy_core_pipeline/src/dof/mod.rs | 48 ++++++++++++-------- crates/bevy_core_pipeline/src/taa/mod.rs | 19 ++++++-- crates/bevy_pbr/src/cluster/mod.rs | 23 +++++----- crates/bevy_pbr/src/lib.rs | 1 + crates/bevy_pbr/src/prepass/mod.rs | 9 ++-- crates/bevy_pbr/src/render/light.rs | 14 ++++++ crates/bevy_pbr/src/ssao/mod.rs | 13 ++++-- crates/bevy_pbr/src/volumetric_fog/render.rs | 9 ++++ crates/bevy_ui/src/render/mod.rs | 4 ++ 9 files changed, 99 insertions(+), 41 deletions(-) diff --git a/crates/bevy_core_pipeline/src/dof/mod.rs b/crates/bevy_core_pipeline/src/dof/mod.rs index 628fc63075..ccf54dd4d5 100644 --- a/crates/bevy_core_pipeline/src/dof/mod.rs +++ b/crates/bevy_core_pipeline/src/dof/mod.rs @@ -46,6 +46,7 @@ use bevy_render::{ TextureDescriptor, TextureDimension, TextureFormat, TextureSampleType, TextureUsages, }, renderer::{RenderContext, RenderDevice}, + sync_component::SyncComponentPlugin, sync_world::RenderEntity, texture::{BevyDefault, CachedTexture, TextureCache}, view::{ @@ -211,6 +212,8 @@ impl Plugin for DepthOfFieldPlugin { app.register_type::(); app.add_plugins(UniformComponentPlugin::::default()); + app.add_plugins(SyncComponentPlugin::::default()); + let Some(render_app) = app.get_sub_app_mut(RenderApp) else { return; }; @@ -820,8 +823,21 @@ fn extract_depth_of_field_settings( } for (entity, depth_of_field, projection) in query.iter_mut() { + let mut entity_commands = commands + .get_entity(entity) + .expect("Depth of field entity wasn't synced."); + // Depth of field is nonsensical without a perspective projection. let Projection::Perspective(ref perspective_projection) = *projection else { + // TODO: needs better strategy for cleaning up + entity_commands.remove::<( + DepthOfField, + DepthOfFieldUniform, + // components added in prepare systems (because `DepthOfFieldNode` does not query extracted components) + DepthOfFieldPipelines, + AuxiliaryDepthOfFieldTexture, + ViewDepthOfFieldBindGroupLayouts, + )>(); continue; }; @@ -829,24 +845,20 @@ fn extract_depth_of_field_settings( calculate_focal_length(depth_of_field.sensor_height, perspective_projection.fov); // Convert `DepthOfField` to `DepthOfFieldUniform`. - commands - .get_entity(entity) - .expect("Depth of field entity wasn't synced.") - .insert(( - *depth_of_field, - DepthOfFieldUniform { - focal_distance: depth_of_field.focal_distance, - focal_length, - coc_scale_factor: focal_length * focal_length - / (depth_of_field.sensor_height * depth_of_field.aperture_f_stops), - max_circle_of_confusion_diameter: depth_of_field - .max_circle_of_confusion_diameter, - max_depth: depth_of_field.max_depth, - pad_a: 0, - pad_b: 0, - pad_c: 0, - }, - )); + entity_commands.insert(( + *depth_of_field, + DepthOfFieldUniform { + focal_distance: depth_of_field.focal_distance, + focal_length, + coc_scale_factor: focal_length * focal_length + / (depth_of_field.sensor_height * depth_of_field.aperture_f_stops), + max_circle_of_confusion_diameter: depth_of_field.max_circle_of_confusion_diameter, + max_depth: depth_of_field.max_depth, + pad_a: 0, + pad_b: 0, + pad_c: 0, + }, + )); } } diff --git a/crates/bevy_core_pipeline/src/taa/mod.rs b/crates/bevy_core_pipeline/src/taa/mod.rs index d2f84f5de7..f8333fc538 100644 --- a/crates/bevy_core_pipeline/src/taa/mod.rs +++ b/crates/bevy_core_pipeline/src/taa/mod.rs @@ -32,6 +32,7 @@ use bevy_render::{ TextureDimension, TextureFormat, TextureSampleType, TextureUsages, }, renderer::{RenderContext, RenderDevice}, + sync_component::SyncComponentPlugin, sync_world::RenderEntity, texture::{BevyDefault, CachedTexture, TextureCache}, view::{ExtractedView, Msaa, ViewTarget}, @@ -52,6 +53,8 @@ impl Plugin for TemporalAntiAliasPlugin { app.register_type::(); + app.add_plugins(SyncComponentPlugin::::default()); + let Some(render_app) = app.get_sub_app_mut(RenderApp) else { return; }; @@ -373,12 +376,20 @@ fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut(); } } } diff --git a/crates/bevy_pbr/src/cluster/mod.rs b/crates/bevy_pbr/src/cluster/mod.rs index 2972f4e116..73944f17a0 100644 --- a/crates/bevy_pbr/src/cluster/mod.rs +++ b/crates/bevy_pbr/src/cluster/mod.rs @@ -530,7 +530,11 @@ pub fn extract_clusters( mapper: Extract>, ) { for (entity, clusters, camera) in &views { + let mut entity_commands = commands + .get_entity(entity) + .expect("Clusters entity wasn't synced."); if !camera.is_active { + entity_commands.remove::<(ExtractedClusterableObjects, ExtractedClusterConfig)>(); continue; } @@ -554,17 +558,14 @@ pub fn extract_clusters( } } - commands - .get_entity(entity) - .expect("Clusters entity wasn't synced.") - .insert(( - ExtractedClusterableObjects { data }, - ExtractedClusterConfig { - near: clusters.near, - far: clusters.far, - dimensions: clusters.dimensions, - }, - )); + entity_commands.insert(( + ExtractedClusterableObjects { data }, + ExtractedClusterConfig { + near: clusters.near, + far: clusters.far, + dimensions: clusters.dimensions, + }, + )); } } diff --git a/crates/bevy_pbr/src/lib.rs b/crates/bevy_pbr/src/lib.rs index da63545eed..559fa99dcc 100644 --- a/crates/bevy_pbr/src/lib.rs +++ b/crates/bevy_pbr/src/lib.rs @@ -455,6 +455,7 @@ impl Plugin for PbrPlugin { render_app .world_mut() .add_observer(remove_light_view_entities); + render_app.world_mut().add_observer(extracted_light_removed); let shadow_pass_node = ShadowPassNode::new(render_app.world_mut()); let mut graph = render_app.world_mut().resource_mut::(); diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 39de9d029a..63ff1d87b1 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -588,14 +588,15 @@ pub fn extract_camera_previous_view_data( cameras_3d: Extract), With>>, ) { for (entity, camera, maybe_previous_view_data) in cameras_3d.iter() { + let mut entity = commands + .get_entity(entity) + .expect("Camera entity wasn't synced."); if camera.is_active { - let mut entity = commands - .get_entity(entity) - .expect("Camera entity wasn't synced."); - if let Some(previous_view_data) = maybe_previous_view_data { entity.insert(previous_view_data.clone()); } + } else { + entity.remove::(); } } } diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 9cc3fec765..a502f1d735 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -379,6 +379,10 @@ pub fn extract_lights( ) in &directional_lights { if !view_visibility.get() { + commands + .get_entity(entity) + .expect("Light entity wasn't synced.") + .remove::<(ExtractedDirectionalLight, RenderCascadesVisibleEntities)>(); continue; } @@ -473,6 +477,16 @@ pub(crate) fn add_light_view_entities( } } +/// Removes [`LightViewEntities`] when light is removed. See [`add_light_view_entities`]. +pub(crate) fn extracted_light_removed( + trigger: Trigger, + mut commands: Commands, +) { + if let Some(mut v) = commands.get_entity(trigger.entity()) { + v.remove::(); + } +} + pub(crate) fn remove_light_view_entities( trigger: Trigger, query: Query<&LightViewEntities>, diff --git a/crates/bevy_pbr/src/ssao/mod.rs b/crates/bevy_pbr/src/ssao/mod.rs index 9d1e1a1db6..42ac9978f6 100644 --- a/crates/bevy_pbr/src/ssao/mod.rs +++ b/crates/bevy_pbr/src/ssao/mod.rs @@ -30,6 +30,7 @@ use bevy_render::{ *, }, renderer::{RenderAdapter, RenderContext, RenderDevice, RenderQueue}, + sync_component::SyncComponentPlugin, sync_world::RenderEntity, texture::{CachedTexture, TextureCache}, view::{Msaa, ViewUniform, ViewUniformOffset, ViewUniforms}, @@ -72,6 +73,8 @@ impl Plugin for ScreenSpaceAmbientOcclusionPlugin { ); app.register_type::(); + + app.add_plugins(SyncComponentPlugin::::default()); } fn finish(&self, app: &mut App) { @@ -531,11 +534,13 @@ fn extract_ssao_settings( ); return; } + let mut entity_commands = commands + .get_entity(entity) + .expect("SSAO entity wasn't synced."); if camera.is_active { - commands - .get_entity(entity) - .expect("SSAO entity wasn't synced.") - .insert(ssao_settings.clone()); + entity_commands.insert(ssao_settings.clone()); + } else { + entity_commands.remove::(); } } } diff --git a/crates/bevy_pbr/src/volumetric_fog/render.rs b/crates/bevy_pbr/src/volumetric_fog/render.rs index f3f8f26cf2..d1dd500f44 100644 --- a/crates/bevy_pbr/src/volumetric_fog/render.rs +++ b/crates/bevy_pbr/src/volumetric_fog/render.rs @@ -276,6 +276,15 @@ pub fn extract_volumetric_fog( volumetric_lights: Extract>, ) { if volumetric_lights.is_empty() { + // TODO: needs better way to handle clean up in render world + for (entity, ..) in view_targets.iter() { + commands + .entity(entity) + .remove::<(VolumetricFog, ViewVolumetricFogPipelines, ViewVolumetricFog)>(); + } + for (entity, ..) in fog_volumes.iter() { + commands.entity(entity).remove::(); + } return; } diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index b56e3ce13e..4e1ac3069a 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -537,6 +537,10 @@ pub fn extract_default_ui_camera_view( for (entity, camera, ui_anti_alias, shadow_samples) in &query { // ignore inactive cameras if !camera.is_active { + commands + .get_entity(entity) + .expect("Camera entity wasn't synced.") + .remove::<(DefaultCameraView, UiAntiAlias, UiBoxShadowSamples)>(); continue; }