Skip to content

Commit

Permalink
Synchronize removed components with the render world (#15582)
Browse files Browse the repository at this point in the history
# Objective

Fixes #15560
Fixes (most of) #15570

Currently a lot of examples (and presumably some user code) depend on
toggling certain render features by adding/removing a single component
to an entity, e.g. `SpotLight` to toggle a light. Because of the
retained render world this no longer works: Extract will add any new
components, but when it is removed the entity persists unchanged in the
render world.

## Solution

Add `SyncComponentPlugin<C: Component>` that registers
`SyncToRenderWorld` as a required component for `C`, and adds a
component hook that will clear all components from the render world
entity when `C` is removed. We add this plugin to
`ExtractComponentPlugin` which fixes most instances of the problem. For
custom extraction logic we can manually add `SyncComponentPlugin` for
that component.

We also rename `WorldSyncPlugin` to `SyncWorldPlugin` so we start a
naming convention like all the `Extract` plugins.

In this PR I also fixed a bunch of breakage related to the retained
render world, stemming from old code that assumed that `Entity` would be
the same in both worlds.

I found that using the `RenderEntity` wrapper instead of `Entity` in
data structures when referring to render world entities makes intent
much clearer, so I propose we make this an official pattern.

## Testing

Run examples like

```
cargo run --features pbr_multi_layer_material_textures --example clearcoat
cargo run --example volumetric_fog
```

and see that they work, and that toggles work correctly. But really we
should test every single example, as we might not even have caught all
the breakage yet.

---

## Migration Guide

The retained render world notes should be updated to explain this edge
case and `SyncComponentPlugin`

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
  • Loading branch information
3 people authored Oct 8, 2024
1 parent 45eff09 commit 2d1b493
Show file tree
Hide file tree
Showing 36 changed files with 151 additions and 84 deletions.
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/auto_exposure/buffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bevy_ecs::prelude::*;
use bevy_render::{
render_resource::{StorageBuffer, UniformBuffer},
renderer::{RenderDevice, RenderQueue},
world_sync::RenderEntity,
sync_world::RenderEntity,
Extract,
};
use bevy_utils::{Entry, HashMap};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/camera_2d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
};
use bevy_ecs::prelude::*;
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_render::world_sync::SyncToRenderWorld;
use bevy_render::sync_world::SyncToRenderWorld;
use bevy_render::{
camera::{
Camera, CameraMainTextureUsages, CameraProjection, CameraRenderGraph,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ use bevy_render::{
TextureFormat, TextureUsages,
},
renderer::RenderDevice,
sync_world::RenderEntity,
texture::TextureCache,
view::{Msaa, ViewDepthTexture},
world_sync::RenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_3d/camera_3d.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use bevy_render::{
extract_component::ExtractComponent,
primitives::Frustum,
render_resource::{LoadOp, TextureUsages},
sync_world::SyncToRenderWorld,
view::{ColorGrading, Msaa, VisibleEntities},
world_sync::SyncToRenderWorld,
};
use bevy_transform::prelude::{GlobalTransform, Transform};
use serde::{Deserialize, Serialize};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ use bevy_render::{
Texture, TextureDescriptor, TextureDimension, TextureFormat, TextureUsages, TextureView,
},
renderer::RenderDevice,
sync_world::RenderEntity,
texture::{BevyDefault, ColorAttachment, Image, TextureCache},
view::{ExtractedView, ViewDepthTexture, ViewTarget},
world_sync::RenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_utils::{tracing::warn, HashMap};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/dof/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ use bevy_render::{
TextureDescriptor, TextureDimension, TextureFormat, TextureSampleType, TextureUsages,
},
renderer::{RenderContext, RenderDevice},
sync_world::RenderEntity,
texture::{BevyDefault, CachedTexture, TextureCache},
view::{
prepare_view_targets, ExtractedView, Msaa, ViewDepthTexture, ViewTarget, ViewUniform,
ViewUniformOffset, ViewUniforms,
},
world_sync::RenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_utils::{info_once, prelude::default, warn_once};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_core_pipeline/src/taa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ use bevy_render::{
TextureDimension, TextureFormat, TextureSampleType, TextureUsages,
},
renderer::{RenderContext, RenderDevice},
sync_world::RenderEntity,
texture::{BevyDefault, CachedTexture, TextureCache},
view::{ExtractedView, Msaa, ViewTarget},
world_sync::RenderEntity,
ExtractSchedule, MainWorld, Render, RenderApp, RenderSet,
};
use bevy_utils::tracing::warn;
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_gizmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ use {
ShaderStages, ShaderType, VertexFormat,
},
renderer::RenderDevice,
world_sync::TemporaryRenderEntity,
sync_world::TemporaryRenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
},
bytemuck::cast_slice,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_render::{
mesh::Mesh3d,
primitives::{CascadesFrusta, CubemapFrusta, Frustum},
sync_world::SyncToRenderWorld,
view::{InheritedVisibility, ViewVisibility, Visibility},
world_sync::SyncToRenderWorld,
};
use bevy_transform::components::{GlobalTransform, Transform};

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/cluster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use bevy_render::{
UniformBuffer,
},
renderer::{RenderDevice, RenderQueue},
world_sync::RenderEntity,
sync_world::RenderEntity,
Extract,
};
use bevy_utils::{hashbrown::HashSet, tracing::warn};
Expand Down
7 changes: 6 additions & 1 deletion crates/bevy_pbr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ use bevy_render::{
render_asset::prepare_assets,
render_graph::RenderGraph,
render_resource::Shader,
sync_component::SyncComponentPlugin,
texture::{GpuImage, Image},
view::{check_visibility, VisibilitySystems},
ExtractSchedule, Render, RenderApp, RenderSet,
Expand Down Expand Up @@ -314,7 +315,6 @@ impl Plugin for PbrPlugin {
.register_type::<PointLight>()
.register_type::<PointLightShadowMap>()
.register_type::<SpotLight>()
.register_type::<DistanceFog>()
.register_type::<ShadowFilteringMethod>()
.init_resource::<AmbientLight>()
.init_resource::<GlobalVisibleClusterableObjects>()
Expand Down Expand Up @@ -346,6 +346,11 @@ impl Plugin for PbrPlugin {
VolumetricFogPlugin,
ScreenSpaceReflectionsPlugin,
))
.add_plugins((
SyncComponentPlugin::<DirectionalLight>::default(),
SyncComponentPlugin::<PointLight>::default(),
SyncComponentPlugin::<SpotLight>::default(),
))
.configure_sets(
PostUpdate,
(
Expand Down
5 changes: 2 additions & 3 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, world_sync::SyncToRenderWorld};
use bevy_render::view::Visibility;

use super::*;

Expand Down Expand Up @@ -57,8 +57,7 @@ use super::*;
CascadeShadowConfig,
CascadesVisibleEntities,
Transform,
Visibility,
SyncToRenderWorld
Visibility
)]
pub struct DirectionalLight {
/// The color of the light.
Expand Down
10 changes: 2 additions & 8 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, world_sync::SyncToRenderWorld};
use bevy_render::view::Visibility;

use super::*;

Expand All @@ -21,13 +21,7 @@ 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,
SyncToRenderWorld
)]
#[require(CubemapFrusta, CubemapVisibleEntities, Transform, Visibility)]
pub struct PointLight {
/// The color of this light source.
pub color: Color,
Expand Down
4 changes: 2 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, world_sync::SyncToRenderWorld};
use bevy_render::view::Visibility;

use super::*;

Expand All @@ -9,7 +9,7 @@ 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, SyncToRenderWorld)]
#[require(Frustum, VisibleMeshEntities, Transform, Visibility)]
pub struct SpotLight {
/// The color of the light.
///
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/light_probe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ use bevy_render::{
render_resource::{DynamicUniformBuffer, Sampler, Shader, ShaderType, TextureView},
renderer::{RenderDevice, RenderQueue},
settings::WgpuFeatures,
sync_world::RenderEntity,
texture::{FallbackImage, GpuImage, Image},
view::ExtractedView,
world_sync::RenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_transform::{components::Transform, prelude::GlobalTransform};
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 @@ -3,7 +3,7 @@ mod prepass_bindings;
use bevy_render::{
mesh::{Mesh3d, MeshVertexBufferLayoutRef, RenderMesh},
render_resource::binding_types::uniform_buffer,
world_sync::RenderEntity,
sync_world::RenderEntity,
};
pub use prepass_bindings::*;

Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use bevy_ecs::{
system::lifetimeless::Read,
};
use bevy_math::{ops, Mat4, UVec4, Vec2, Vec3, Vec3Swizzles, Vec4, Vec4Swizzles};
use bevy_render::world_sync::RenderEntity;
use bevy_render::sync_world::RenderEntity;
use bevy_render::{
diagnostic::RecordDiagnostics,
mesh::RenderMesh,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/ssao/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ use bevy_render::{
*,
},
renderer::{RenderAdapter, RenderContext, RenderDevice, RenderQueue},
sync_world::RenderEntity,
texture::{CachedTexture, TextureCache},
view::{Msaa, ViewUniform, ViewUniformOffset, ViewUniforms},
world_sync::RenderEntity,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_utils::{
Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_pbr/src/volumetric_fog/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ use bevy_render::{
mesh::{Mesh, Meshable},
render_graph::{RenderGraphApp, ViewNodeRunner},
render_resource::{Shader, SpecializedRenderPipelines},
sync_component::SyncComponentPlugin,
texture::Image,
view::{InheritedVisibility, ViewVisibility, Visibility},
ExtractSchedule, Render, RenderApp, RenderSet,
Expand Down Expand Up @@ -231,6 +232,8 @@ impl Plugin for VolumetricFogPlugin {
app.register_type::<VolumetricFog>()
.register_type::<VolumetricLight>();

app.add_plugins(SyncComponentPlugin::<FogVolume>::default());

let Some(render_app) = app.get_sub_app_mut(RenderApp) else {
return;
};
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_pbr/src/volumetric_fog/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ use bevy_render::{
TextureSampleType, TextureUsages, VertexState,
},
renderer::{RenderContext, RenderDevice, RenderQueue},
sync_world::RenderEntity,
texture::{BevyDefault as _, GpuImage, Image},
view::{ExtractedView, Msaa, ViewDepthTexture, ViewTarget, ViewUniformOffset},
world_sync::RenderEntity,
Extract,
};
use bevy_transform::components::GlobalTransform;
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ use crate::{
render_asset::RenderAssets,
render_graph::{InternedRenderSubGraph, RenderSubGraph},
render_resource::TextureView,
sync_world::{RenderEntity, SyncToRenderWorld},
texture::GpuImage,
view::{
ColorGrading, ExtractedView, ExtractedWindows, GpuCulling, Msaa, RenderLayers, Visibility,
VisibleEntities,
},
world_sync::{RenderEntity, SyncToRenderWorld},
Extract,
};
use bevy_asset::{AssetEvent, AssetId, Assets, Handle};
Expand Down
23 changes: 6 additions & 17 deletions crates/bevy_render/src/extract_component.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use crate::{
render_resource::{encase::internal::WriteInto, DynamicUniformBuffer, ShaderType},
renderer::{RenderDevice, RenderQueue},
sync_component::SyncComponentPlugin,
sync_world::RenderEntity,
view::ViewVisibility,
world_sync::{RenderEntity, SyncToRenderWorld},
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_app::{App, Plugin};
Expand All @@ -12,7 +13,6 @@ use bevy_ecs::{
prelude::*,
query::{QueryFilter, QueryItem, ReadOnlyQueryData},
system::lifetimeless::Read,
world::OnAdd,
};
use core::{marker::PhantomData, ops::Deref};

Expand Down Expand Up @@ -160,15 +160,6 @@ fn prepare_uniform_components<C>(
/// This plugin extracts the components into the render world for synced entities.
///
/// To do so, it sets up the [`ExtractSchedule`] step for the specified [`ExtractComponent`].
///
/// # Warning
///
/// Be careful when removing the [`ExtractComponent`] from an entity. When an [`ExtractComponent`]
/// is added to an entity, that entity is automatically synced with the render world (see also
/// [`WorldSyncPlugin`](crate::world_sync::WorldSyncPlugin)). When removing the entity in the main
/// world, the synced entity also gets removed. However, if only the [`ExtractComponent`] is removed
/// this *doesn't* happen, and the synced entity stays around with the old extracted data.
/// We recommend despawning the entire entity, instead of only removing [`ExtractComponent`].
pub struct ExtractComponentPlugin<C, F = ()> {
only_extract_visible: bool,
marker: PhantomData<fn() -> (C, F)>,
Expand All @@ -194,10 +185,8 @@ impl<C, F> ExtractComponentPlugin<C, F> {

impl<C: ExtractComponent> Plugin for ExtractComponentPlugin<C> {
fn build(&self, app: &mut App) {
// TODO: use required components
app.observe(|trigger: Trigger<OnAdd, C>, mut commands: Commands| {
commands.entity(trigger.entity()).insert(SyncToRenderWorld);
});
app.add_plugins(SyncComponentPlugin::<C>::default());

if let Some(render_app) = app.get_sub_app_mut(RenderApp) {
if self.only_extract_visible {
render_app.add_systems(ExtractSchedule, extract_visible_components::<C>);
Expand All @@ -219,7 +208,7 @@ impl<T: Asset> ExtractComponent for Handle<T> {
}
}

/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are synced via [`SyncToRenderWorld`].
/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are synced via [`crate::sync_world::SyncToRenderWorld`].
fn extract_components<C: ExtractComponent>(
mut commands: Commands,
mut previous_len: Local<usize>,
Expand All @@ -235,7 +224,7 @@ fn extract_components<C: ExtractComponent>(
commands.insert_or_spawn_batch(values);
}

/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are visible and synced via [`SyncToRenderWorld`].
/// This system extracts all components of the corresponding [`ExtractComponent`], for entities that are visible and synced via [`crate::sync_world::SyncToRenderWorld`].
fn extract_visible_components<C: ExtractComponent>(
mut commands: Commands,
mut previous_len: Local<usize>,
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_render/src/extract_param.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use core::ops::{Deref, DerefMut};
/// ```
/// use bevy_ecs::prelude::*;
/// use bevy_render::Extract;
/// use bevy_render::world_sync::RenderEntity;
/// use bevy_render::sync_world::RenderEntity;
/// # #[derive(Component)]
/// // Do make sure to sync the cloud entities before extracting them.
/// # struct Cloud;
Expand Down
7 changes: 4 additions & 3 deletions crates/bevy_render/src/gpu_readback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::{
render_resource::{Buffer, BufferUsages, Extent3d, ImageDataLayout, Texture, TextureFormat},
renderer::{render_system, RenderDevice},
storage::{GpuShaderStorageBuffer, ShaderStorageBuffer},
sync_world::MainEntity,
texture::{GpuImage, TextureFormatPixelInfo},
ExtractSchedule, MainWorld, Render, RenderApp, RenderSet,
};
Expand Down Expand Up @@ -232,7 +233,7 @@ fn prepare_buffers(
mut buffer_pool: ResMut<GpuReadbackBufferPool>,
gpu_images: Res<RenderAssets<GpuImage>>,
ssbos: Res<RenderAssets<GpuShaderStorageBuffer>>,
handles: Query<(Entity, &Readback)>,
handles: Query<(&MainEntity, &Readback)>,
) {
for (entity, readback) in handles.iter() {
match readback {
Expand All @@ -254,7 +255,7 @@ fn prepare_buffers(
);
let (tx, rx) = async_channel::bounded(1);
readbacks.requested.push(GpuReadback {
entity,
entity: entity.id(),
src: ReadbackSource::Texture {
texture: gpu_image.texture.clone(),
layout,
Expand All @@ -272,7 +273,7 @@ fn prepare_buffers(
let buffer = buffer_pool.get(&render_device, size);
let (tx, rx) = async_channel::bounded(1);
readbacks.requested.push(GpuReadback {
entity,
entity: entity.id(),
src: ReadbackSource::Buffer {
src_start: 0,
dst_start: 0,
Expand Down
Loading

0 comments on commit 2d1b493

Please sign in to comment.