From 73c838c30e43c35f792f37ff40f009d297304f8b Mon Sep 17 00:00:00 2001 From: Daniel McNab <36049421+DJMcNab@users.noreply.github.com> Date: Fri, 8 Jul 2022 23:56:33 +0000 Subject: [PATCH] Make `RenderStage::Extract` run on the render world (#4402) # Objective - Currently, the `Extract` `RenderStage` is executed on the main world, with the render world available as a resource. - However, when needing access to resources in the render world (e.g. to mutate them), the only way to do so was to get exclusive access to the whole `RenderWorld` resource. - This meant that effectively only one extract which wrote to resources could run at a time. - We didn't previously make `Extract`ing writing to the world a non-happy path, even though we want to discourage that. ## Solution - Move the extract stage to run on the render world. - Add the main world as a `MainWorld` resource. - Add an `Extract` `SystemParam` as a convenience to access a (read only) `SystemParam` in the main world during `Extract`. ## Future work It should be possible to avoid needing to use `get_or_spawn` for the render commands, since now the `Commands`' `Entities` matches up with the world being executed on. We need to determine how this interacts with https://github.com/bevyengine/bevy/pull/3519 It's theoretically possible to remove the need for the `value` method on `Extract`. However, that requires slightly changing the `SystemParam` interface, which would make it more complicated. That would probably mess up the `SystemState` api too. ## Todo I still need to add doc comments to `Extract`. --- ## Changelog ### Changed - The `Extract` `RenderStage` now runs on the render world (instead of the main world as before). You must use the `Extract` `SystemParam` to access the main world during the extract phase. Resources on the render world can now be accessed using `ResMut` during extract. ### Removed - `Commands::spawn_and_forget`. Use `Commands::get_or_spawn(e).insert_bundle(bundle)` instead ## Migration Guide The `Extract` `RenderStage` now runs on the render world (instead of the main world as before). You must use the `Extract` `SystemParam` to access the main world during the extract phase. `Extract` takes a single type parameter, which is any system parameter (such as `Res`, `Query` etc.). It will extract this from the main world, and returns the result of this extraction when `value` is called on it. For example, if previously your extract system looked like: ```rust fn extract_clouds(mut commands: Commands, clouds: Query>) { for cloud in clouds.iter() { commands.get_or_spawn(cloud).insert(Cloud); } } ``` the new version would be: ```rust fn extract_clouds(mut commands: Commands, mut clouds: Extract>>) { for cloud in clouds.value().iter() { commands.get_or_spawn(cloud).insert(Cloud); } } ``` The diff is: ```diff --- a/src/clouds.rs +++ b/src/clouds.rs @@ -1,5 +1,5 @@ -fn extract_clouds(mut commands: Commands, clouds: Query>) { - for cloud in clouds.iter() { +fn extract_clouds(mut commands: Commands, mut clouds: Extract>>) { + for cloud in clouds.value().iter() { commands.get_or_spawn(cloud).insert(Cloud); } } ``` You can now also access resources from the render world using the normal system parameters during `Extract`: ```rust fn extract_assets(mut render_assets: ResMut, source_assets: Extract>) { *render_assets = source_assets.clone(); } ``` Please note that all existing extract systems need to be updated to match this new style; even if they currently compile they will not run as expected. A warning will be emitted on a best-effort basis if this is not met. Co-authored-by: Carter Anderson --- crates/bevy_core_pipeline/src/core_2d/mod.rs | 4 +- crates/bevy_core_pipeline/src/core_3d/mod.rs | 4 +- crates/bevy_ecs/src/schedule/stage.rs | 28 +++- crates/bevy_ecs/src/system/commands/mod.rs | 6 - crates/bevy_pbr/src/material.rs | 12 +- crates/bevy_pbr/src/render/light.rs | 55 ++++---- crates/bevy_pbr/src/render/mesh.rs | 26 ++-- crates/bevy_render/src/camera/camera.rs | 17 ++- crates/bevy_render/src/extract_component.rs | 14 +- crates/bevy_render/src/extract_param.rs | 120 ++++++++++++++++++ crates/bevy_render/src/extract_resource.rs | 9 +- crates/bevy_render/src/lib.rs | 74 +++++++---- crates/bevy_render/src/render_asset.rs | 12 +- .../src/render_resource/pipeline_cache.rs | 9 +- crates/bevy_render/src/view/window.rs | 9 +- crates/bevy_sprite/src/mesh2d/mesh.rs | 4 +- crates/bevy_sprite/src/render/mod.rs | 28 ++-- crates/bevy_text/src/text2d.rs | 15 +-- crates/bevy_ui/src/render/mod.rs | 66 +++++----- examples/2d/mesh2d_manual.rs | 6 +- examples/shader/animate_shader.rs | 42 +++--- examples/stress_tests/many_lights.rs | 11 +- 22 files changed, 377 insertions(+), 194 deletions(-) create mode 100644 crates/bevy_render/src/extract_param.rs diff --git a/crates/bevy_core_pipeline/src/core_2d/mod.rs b/crates/bevy_core_pipeline/src/core_2d/mod.rs index 4e1e8098e93c9c..69db8d4ad45609 100644 --- a/crates/bevy_core_pipeline/src/core_2d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_2d/mod.rs @@ -25,7 +25,7 @@ use bevy_render::{ DrawFunctionId, DrawFunctions, EntityPhaseItem, PhaseItem, RenderPhase, }, render_resource::CachedRenderPipelineId, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_utils::FloatOrd; use std::ops::Range; @@ -123,7 +123,7 @@ impl BatchedPhaseItem for Transparent2d { pub fn extract_core_2d_camera_phases( mut commands: Commands, - cameras_2d: Query<(Entity, &Camera), With>, + cameras_2d: Extract>>, ) { for (entity, camera) in cameras_2d.iter() { if camera.is_active { diff --git a/crates/bevy_core_pipeline/src/core_3d/mod.rs b/crates/bevy_core_pipeline/src/core_3d/mod.rs index c49c8634b30f74..9470121bc49f3a 100644 --- a/crates/bevy_core_pipeline/src/core_3d/mod.rs +++ b/crates/bevy_core_pipeline/src/core_3d/mod.rs @@ -34,7 +34,7 @@ use bevy_render::{ renderer::RenderDevice, texture::TextureCache, view::ViewDepthTexture, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_utils::{FloatOrd, HashMap}; @@ -208,7 +208,7 @@ impl CachedRenderPipelinePhaseItem for Transparent3d { pub fn extract_core_3d_camera_phases( mut commands: Commands, - cameras_3d: Query<(Entity, &Camera), With>, + cameras_3d: Extract>>, ) { for (entity, camera) in cameras_3d.iter() { if camera.is_active { diff --git a/crates/bevy_ecs/src/schedule/stage.rs b/crates/bevy_ecs/src/schedule/stage.rs index ff26ced892a6f1..014423e2c4f135 100644 --- a/crates/bevy_ecs/src/schedule/stage.rs +++ b/crates/bevy_ecs/src/schedule/stage.rs @@ -12,7 +12,10 @@ use crate::{ }, world::{World, WorldId}, }; -use bevy_utils::{tracing::info, HashMap, HashSet}; +use bevy_utils::{ + tracing::{info, warn}, + HashMap, HashSet, +}; use downcast_rs::{impl_downcast, Downcast}; use fixedbitset::FixedBitSet; use std::fmt::Debug; @@ -88,6 +91,7 @@ pub struct SystemStage { last_tick_check: u32, /// If true, buffers will be automatically applied at the end of the stage. If false, buffers must be manually applied. apply_buffers: bool, + must_read_resource: Option, } impl SystemStage { @@ -110,6 +114,7 @@ impl SystemStage { uninitialized_at_end: vec![], last_tick_check: Default::default(), apply_buffers: true, + must_read_resource: None, } } @@ -139,6 +144,10 @@ impl SystemStage { self.executor = executor; } + pub fn set_must_read_resource(&mut self, resource_id: ComponentId) { + self.must_read_resource = Some(resource_id); + } + #[must_use] pub fn with_system(mut self, system: impl IntoSystemDescriptor) -> Self { self.add_system(system); @@ -563,6 +572,20 @@ impl SystemStage { } } + fn check_uses_resource(&self, resource_id: ComponentId, world: &World) { + debug_assert!(!self.systems_modified); + for system in &self.parallel { + let access = system.component_access().unwrap(); + if !access.has_read(resource_id) { + let component_name = world.components().get_info(resource_id).unwrap().name(); + warn!( + "System {} doesn't access resource {component_name}, despite being required to", + system.name() + ); + } + } + } + /// All system and component change ticks are scanned once the world counter has incremented /// at least [`CHECK_TICK_THRESHOLD`](crate::change_detection::CHECK_TICK_THRESHOLD) /// times since the previous `check_tick` scan. @@ -782,6 +805,9 @@ impl Stage for SystemStage { if world.contains_resource::() { self.report_ambiguities(world); } + if let Some(resource_id) = self.must_read_resource { + self.check_uses_resource(resource_id, world); + } } else if self.executor_modified { self.executor.rebuild_cached_data(&self.parallel); self.executor_modified = false; diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 29abf060243b5b..9888edac185c75 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -145,12 +145,6 @@ impl<'w, 's> Commands<'w, 's> { } } - /// Spawns a [`Bundle`] without pre-allocating an [`Entity`]. The [`Entity`] will be allocated - /// when this [`Command`] is applied. - pub fn spawn_and_forget(&mut self, bundle: impl Bundle) { - self.queue.push(Spawn { bundle }); - } - /// Creates a new entity with the components contained in `bundle`. /// /// This returns an [`EntityCommands`] builder, which enables inserting more components and diff --git a/crates/bevy_pbr/src/material.rs b/crates/bevy_pbr/src/material.rs index b2581a9b2a161f..b836aad972a815 100644 --- a/crates/bevy_pbr/src/material.rs +++ b/crates/bevy_pbr/src/material.rs @@ -34,7 +34,7 @@ use bevy_render::{ renderer::RenderDevice, texture::FallbackImage, view::{ExtractedView, Msaa, VisibleEntities}, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_utils::{tracing::error, HashMap, HashSet}; use std::hash::Hash; @@ -455,15 +455,15 @@ pub type RenderMaterials = HashMap, PreparedMaterial>; /// into the "render world". fn extract_materials( mut commands: Commands, - mut events: EventReader>, - assets: Res>, + mut events: Extract>>, + assets: Extract>>, ) { let mut changed_assets = HashSet::default(); let mut removed = Vec::new(); for event in events.iter() { match event { AssetEvent::Created { handle } | AssetEvent::Modified { handle } => { - changed_assets.insert(handle); + changed_assets.insert(handle.clone_weak()); } AssetEvent::Removed { handle } => { changed_assets.remove(handle); @@ -474,8 +474,8 @@ fn extract_materials( let mut extracted_assets = Vec::new(); for handle in changed_assets.drain() { - if let Some(asset) = assets.get(handle) { - extracted_assets.push((handle.clone_weak(), asset.clone())); + if let Some(asset) = assets.get(&handle) { + extracted_assets.push((handle, asset.clone())); } } diff --git a/crates/bevy_pbr/src/render/light.rs b/crates/bevy_pbr/src/render/light.rs index 5b7f16de9670b2..bf69c70dc25cac 100644 --- a/crates/bevy_pbr/src/render/light.rs +++ b/crates/bevy_pbr/src/render/light.rs @@ -28,6 +28,7 @@ use bevy_render::{ view::{ ExtractedView, ViewUniform, ViewUniformOffset, ViewUniforms, Visibility, VisibleEntities, }, + Extract, }; use bevy_transform::components::GlobalTransform; use bevy_utils::FloatOrd; @@ -386,7 +387,10 @@ pub struct ExtractedClustersPointLights { data: Vec, } -pub fn extract_clusters(mut commands: Commands, views: Query<(Entity, &Clusters), With>) { +pub fn extract_clusters( + mut commands: Commands, + views: Extract>>, +) { for (entity, clusters) in views.iter() { commands.get_or_spawn(entity).insert_bundle(( ExtractedClustersPointLights { @@ -404,20 +408,22 @@ pub fn extract_clusters(mut commands: Commands, views: Query<(Entity, &Clusters) #[allow(clippy::too_many_arguments)] pub fn extract_lights( mut commands: Commands, - point_light_shadow_map: Res, - directional_light_shadow_map: Res, - global_point_lights: Res, - mut point_lights: Query<(&PointLight, &mut CubemapVisibleEntities, &GlobalTransform)>, - mut spot_lights: Query<(&SpotLight, &mut VisibleEntities, &GlobalTransform)>, - mut directional_lights: Query< - ( - Entity, - &DirectionalLight, - &mut VisibleEntities, - &GlobalTransform, - &Visibility, - ), - Without, + point_light_shadow_map: Extract>, + directional_light_shadow_map: Extract>, + global_point_lights: Extract>, + point_lights: Extract>, + spot_lights: Extract>, + directional_lights: Extract< + Query< + ( + Entity, + &DirectionalLight, + &VisibleEntities, + &GlobalTransform, + &Visibility, + ), + Without, + >, >, mut previous_point_lights_len: Local, mut previous_spot_lights_len: Local, @@ -441,10 +447,10 @@ pub fn extract_lights( let mut point_lights_values = Vec::with_capacity(*previous_point_lights_len); for entity in global_point_lights.iter().copied() { - if let Ok((point_light, cubemap_visible_entities, transform)) = point_lights.get_mut(entity) - { - let render_cubemap_visible_entities = - std::mem::take(cubemap_visible_entities.into_inner()); + if let Ok((point_light, cubemap_visible_entities, transform)) = point_lights.get(entity) { + // TODO: This is very much not ideal. We should be able to re-use the vector memory. + // However, since exclusive access to the main world in extract is ill-advised, we just clone here. + let render_cubemap_visible_entities = cubemap_visible_entities.clone(); point_lights_values.push(( entity, ( @@ -475,8 +481,10 @@ pub fn extract_lights( let mut spot_lights_values = Vec::with_capacity(*previous_spot_lights_len); for entity in global_point_lights.iter().copied() { - if let Ok((spot_light, visible_entities, transform)) = spot_lights.get_mut(entity) { - let render_visible_entities = std::mem::take(visible_entities.into_inner()); + if let Ok((spot_light, visible_entities, transform)) = spot_lights.get(entity) { + // TODO: This is very much not ideal. We should be able to re-use the vector memory. + // However, since exclusive access to the main world in extract is ill-advised, we just clone here. + let render_visible_entities = visible_entities.clone(); let texel_size = 2.0 * spot_light.outer_angle.tan() / directional_light_shadow_map.size as f32; @@ -512,7 +520,7 @@ pub fn extract_lights( commands.insert_or_spawn_batch(spot_lights_values); for (entity, directional_light, visible_entities, transform, visibility) in - directional_lights.iter_mut() + directional_lights.iter() { if !visibility.is_visible { continue; @@ -530,7 +538,8 @@ pub fn extract_lights( ); let directional_light_texel_size = largest_dimension / directional_light_shadow_map.size as f32; - let render_visible_entities = std::mem::take(visible_entities.into_inner()); + // TODO: As above + let render_visible_entities = visible_entities.clone(); commands.get_or_spawn(entity).insert_bundle(( ExtractedDirectionalLight { color: directional_light.color, diff --git a/crates/bevy_pbr/src/render/mesh.rs b/crates/bevy_pbr/src/render/mesh.rs index 348c7c48eb88e5..c6ce76d4a2a96c 100644 --- a/crates/bevy_pbr/src/render/mesh.rs +++ b/crates/bevy_pbr/src/render/mesh.rs @@ -25,7 +25,7 @@ use bevy_render::{ BevyDefault, DefaultImageSampler, GpuImage, Image, ImageSampler, TextureFormatPixelInfo, }, view::{ComputedVisibility, ViewUniform, ViewUniformOffset, ViewUniforms}, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_transform::components::GlobalTransform; use std::num::NonZeroU64; @@ -118,14 +118,16 @@ pub fn extract_meshes( mut commands: Commands, mut prev_caster_commands_len: Local, mut prev_not_caster_commands_len: Local, - meshes_query: Query<( - Entity, - &ComputedVisibility, - &GlobalTransform, - &Handle, - Option>, - Option>, - )>, + meshes_query: Extract< + Query<( + Entity, + &ComputedVisibility, + &GlobalTransform, + &Handle, + Option>, + Option>, + )>, + >, ) { let mut caster_commands = Vec::with_capacity(*prev_caster_commands_len); let mut not_caster_commands = Vec::with_capacity(*prev_not_caster_commands_len); @@ -202,12 +204,12 @@ impl SkinnedMeshJoints { } pub fn extract_skinned_meshes( - query: Query<(Entity, &ComputedVisibility, &SkinnedMesh)>, - inverse_bindposes: Res>, - joint_query: Query<&GlobalTransform>, mut commands: Commands, mut previous_len: Local, mut previous_joint_len: Local, + query: Extract>, + inverse_bindposes: Extract>>, + joint_query: Extract>, ) { let mut values = Vec::with_capacity(*previous_len); let mut joints = Vec::with_capacity(*previous_joint_len); diff --git a/crates/bevy_render/src/camera/camera.rs b/crates/bevy_render/src/camera/camera.rs index e1e91c28486b82..d01374b527556b 100644 --- a/crates/bevy_render/src/camera/camera.rs +++ b/crates/bevy_render/src/camera/camera.rs @@ -4,6 +4,7 @@ use crate::{ render_asset::RenderAssets, render_resource::TextureView, view::{ExtractedView, ExtractedWindows, VisibleEntities}, + Extract, }; use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_derive::{Deref, DerefMut}; @@ -393,13 +394,15 @@ pub struct ExtractedCamera { pub fn extract_cameras( mut commands: Commands, - query: Query<( - Entity, - &Camera, - &CameraRenderGraph, - &GlobalTransform, - &VisibleEntities, - )>, + query: Extract< + Query<( + Entity, + &Camera, + &CameraRenderGraph, + &GlobalTransform, + &VisibleEntities, + )>, + >, ) { for (entity, camera, camera_render_graph, transform, visible_entities) in query.iter() { if !camera.is_active { diff --git a/crates/bevy_render/src/extract_component.rs b/crates/bevy_render/src/extract_component.rs index 673b23c30770ef..ffedd60c321676 100644 --- a/crates/bevy_render/src/extract_component.rs +++ b/crates/bevy_render/src/extract_component.rs @@ -2,15 +2,15 @@ use crate::{ render_resource::{encase::internal::WriteInto, DynamicUniformBuffer, ShaderType}, renderer::{RenderDevice, RenderQueue}, view::ComputedVisibility, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_app::{App, Plugin}; use bevy_asset::{Asset, Handle}; use bevy_ecs::{ component::Component, prelude::*, - query::{QueryItem, WorldQuery}, - system::{lifetimeless::Read, StaticSystemParam}, + query::{QueryItem, ReadOnlyWorldQuery, WorldQuery}, + system::lifetimeless::Read, }; use std::{marker::PhantomData, ops::Deref}; @@ -34,9 +34,9 @@ impl DynamicUniformIndex { /// in the [`RenderStage::Extract`](crate::RenderStage::Extract) step. pub trait ExtractComponent: Component { /// ECS [`WorldQuery`] to fetch the components to extract. - type Query: WorldQuery; + type Query: WorldQuery + ReadOnlyWorldQuery; /// Filters the entities with additional constraints. - type Filter: WorldQuery; + type Filter: WorldQuery + ReadOnlyWorldQuery; /// Defines how the component is transferred into the "render world". fn extract_component(item: QueryItem) -> Self; } @@ -182,7 +182,7 @@ impl ExtractComponent for Handle { fn extract_components( mut commands: Commands, mut previous_len: Local, - mut query: StaticSystemParam>, + mut query: Extract>, ) { let mut values = Vec::with_capacity(*previous_len); for (entity, query_item) in query.iter_mut() { @@ -196,7 +196,7 @@ fn extract_components( fn extract_visible_components( mut commands: Commands, mut previous_len: Local, - mut query: StaticSystemParam, C::Query), C::Filter>>, + mut query: Extract>, ) { let mut values = Vec::with_capacity(*previous_len); for (entity, computed_visibility, query_item) in query.iter_mut() { diff --git a/crates/bevy_render/src/extract_param.rs b/crates/bevy_render/src/extract_param.rs new file mode 100644 index 00000000000000..37e76a20e2bed5 --- /dev/null +++ b/crates/bevy_render/src/extract_param.rs @@ -0,0 +1,120 @@ +use crate::MainWorld; +use bevy_ecs::{ + prelude::*, + system::{ + ReadOnlySystemParamFetch, ResState, SystemMeta, SystemParam, SystemParamFetch, + SystemParamState, SystemState, + }, +}; +use std::ops::{Deref, DerefMut}; + +/// A helper for accessing [`MainWorld`] content using a system parameter. +/// +/// A [`SystemParam`] adapter which applies the contained `SystemParam` to the [`World`] +/// contained in [`MainWorld`]. This parameter only works for systems run +/// during [`RenderStage::Extract`]. +/// +/// This requires that the contained [`SystemParam`] does not mutate the world, as it +/// uses a read-only reference to [`MainWorld`] internally. +/// +/// ## Context +/// +/// [`RenderStage::Extract`] is used to extract (move) data from the simulation world ([`MainWorld`]) to the +/// render world. The render world drives rendering each frame (generally to a [Window]). +/// This design is used to allow performing calculations related to rendering a prior frame at the same +/// time as the next frame is simulated, which increases throughput (FPS). +/// +/// [`Extract`] is used to get data from the main world during [`RenderStage::Extract`]. +/// +/// ## Examples +/// +/// ```rust +/// use bevy_ecs::prelude::*; +/// use bevy_render::Extract; +/// # #[derive(Component)] +/// # struct Cloud; +/// fn extract_clouds(mut commands: Commands, clouds: Extract>>) { +/// for cloud in clouds.iter() { +/// commands.get_or_spawn(cloud).insert(Cloud); +/// } +/// } +/// ``` +/// +/// [`RenderStage::Extract`]: crate::RenderStage::Extract +/// [Window]: bevy_window::Window +pub struct Extract<'w, 's, P: SystemParam + 'static> +where + P::Fetch: ReadOnlySystemParamFetch, +{ + item: >::Item, +} + +impl<'w, 's, P: SystemParam> SystemParam for Extract<'w, 's, P> +where + P::Fetch: ReadOnlySystemParamFetch, +{ + type Fetch = ExtractState

; +} + +#[doc(hidden)] +pub struct ExtractState { + state: SystemState

, + main_world_state: ResState, +} + +// SAFETY: only accesses MainWorld resource with read only system params using ResState, +// which is initialized in init() +unsafe impl SystemParamState for ExtractState

{ + fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self { + let mut main_world = world.resource_mut::(); + Self { + state: SystemState::new(&mut main_world), + main_world_state: ResState::init(world, system_meta), + } + } +} + +impl<'w, 's, P: SystemParam + 'static> SystemParamFetch<'w, 's> for ExtractState

+where + P::Fetch: ReadOnlySystemParamFetch, +{ + type Item = Extract<'w, 's, P>; + + unsafe fn get_param( + state: &'s mut Self, + system_meta: &SystemMeta, + world: &'w World, + change_tick: u32, + ) -> Self::Item { + let main_world = ResState::::get_param( + &mut state.main_world_state, + system_meta, + world, + change_tick, + ); + let item = state.state.get(main_world.into_inner()); + Extract { item } + } +} + +impl<'w, 's, P: SystemParam> Deref for Extract<'w, 's, P> +where + P::Fetch: ReadOnlySystemParamFetch, +{ + type Target = >::Item; + + #[inline] + fn deref(&self) -> &Self::Target { + &self.item + } +} + +impl<'w, 's, P: SystemParam> DerefMut for Extract<'w, 's, P> +where + P::Fetch: ReadOnlySystemParamFetch, +{ + #[inline] + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.item + } +} diff --git a/crates/bevy_render/src/extract_resource.rs b/crates/bevy_render/src/extract_resource.rs index 5db3be904b263a..57be9ab7e1c824 100644 --- a/crates/bevy_render/src/extract_resource.rs +++ b/crates/bevy_render/src/extract_resource.rs @@ -4,7 +4,7 @@ use bevy_app::{App, Plugin}; use bevy_ecs::system::{Commands, Res, Resource}; pub use bevy_render_macros::ExtractResource; -use crate::{RenderApp, RenderStage}; +use crate::{Extract, RenderApp, RenderStage}; /// Describes how a resource gets extracted for rendering. /// @@ -39,8 +39,11 @@ impl Plugin for ExtractResourcePlugin { /// This system extracts the resource of the corresponding [`Resource`] type /// by cloning it. -pub fn extract_resource(mut commands: Commands, resource: Res) { +pub fn extract_resource( + mut commands: Commands, + resource: Extract>, +) { if resource.is_changed() { - commands.insert_resource(R::extract_resource(resource.into_inner())); + commands.insert_resource(R::extract_resource(&*resource)); } } diff --git a/crates/bevy_render/src/lib.rs b/crates/bevy_render/src/lib.rs index b69eb85c35c172..f25ef26e36de7a 100644 --- a/crates/bevy_render/src/lib.rs +++ b/crates/bevy_render/src/lib.rs @@ -3,6 +3,7 @@ extern crate core; pub mod camera; pub mod color; pub mod extract_component; +mod extract_param; pub mod extract_resource; pub mod mesh; pub mod primitives; @@ -16,6 +17,8 @@ pub mod settings; pub mod texture; pub mod view; +pub use extract_param::Extract; + pub mod prelude { #[doc(hidden)] pub use crate::{ @@ -45,7 +48,10 @@ use bevy_app::{App, AppLabel, Plugin}; use bevy_asset::{AddAsset, AssetServer}; use bevy_ecs::prelude::*; use bevy_utils::tracing::debug; -use std::ops::{Deref, DerefMut}; +use std::{ + any::TypeId, + ops::{Deref, DerefMut}, +}; /// Contains the default Bevy rendering backend based on wgpu. #[derive(Default)] @@ -79,11 +85,14 @@ pub enum RenderStage { Cleanup, } -/// The Render App World. This is only available as a resource during the Extract step. +/// The simulation [`World`] of the application, stored as a resource. +/// This resource is only available during [`RenderStage::Extract`] and not +/// during command application of that stage. +/// See [`Extract`] for more details. #[derive(Default)] -pub struct RenderWorld(World); +pub struct MainWorld(World); -impl Deref for RenderWorld { +impl Deref for MainWorld { type Target = World; fn deref(&self) -> &Self::Target { @@ -91,7 +100,7 @@ impl Deref for RenderWorld { } } -impl DerefMut for RenderWorld { +impl DerefMut for MainWorld { fn deref_mut(&mut self) -> &mut Self::Target { &mut self.0 } @@ -107,11 +116,6 @@ pub mod main_graph { #[derive(Debug, Clone, Copy, Hash, PartialEq, Eq, AppLabel)] pub struct RenderApp; -/// A "scratch" world used to avoid allocating new worlds every frame when -/// swapping out the [`RenderWorld`]. -#[derive(Default)] -struct ScratchRenderWorld(World); - impl Plugin for RenderPlugin { /// Initializes the renderer, sets up the [`RenderStage`](RenderStage) and creates the rendering sub-app. fn build(&self, app: &mut App) { @@ -150,7 +154,7 @@ impl Plugin for RenderPlugin { app.insert_resource(device.clone()) .insert_resource(queue.clone()) .insert_resource(adapter_info.clone()) - .init_resource::() + .init_resource::() .register_type::() .register_type::(); @@ -160,8 +164,20 @@ impl Plugin for RenderPlugin { let mut render_app = App::empty(); let mut extract_stage = SystemStage::parallel().with_system(PipelineCache::extract_shaders); + // Get the ComponentId for MainWorld. This does technically 'waste' a `WorldId`, but that's probably fine + render_app.init_resource::(); + render_app.world.remove_resource::(); + let main_world_in_render = render_app + .world + .components() + .get_resource_id(TypeId::of::()); + // `Extract` systems must read from the main world. We want to emit an error when that doesn't occur + // Safe to unwrap: Ensured it existed just above + extract_stage.set_must_read_resource(main_world_in_render.unwrap()); // don't apply buffers when the stage finishes running - // extract stage runs on the app world, but the buffers are applied to the render world + // extract stage runs on the render world, but buffers are applied + // after access to the main world is removed + // See also https://github.com/bevyengine/bevy/issues/5082 extract_stage.set_apply_buffers(false); render_app .add_stage(RenderStage::Extract, extract_stage) @@ -300,6 +316,11 @@ impl Plugin for RenderPlugin { } } +/// A "scratch" world used to avoid allocating new worlds every frame when +/// swapping out the [`MainWorld`] for [`RenderStage::Extract`]. +#[derive(Default)] +struct ScratchMainWorld(World); + /// Executes the [`Extract`](RenderStage::Extract) stage of the renderer. /// This updates the render world with the extracted ECS data of the current frame. fn extract(app_world: &mut World, render_app: &mut App) { @@ -308,17 +329,20 @@ fn extract(app_world: &mut World, render_app: &mut App) { .get_stage_mut::(&RenderStage::Extract) .unwrap(); - // temporarily add the render world to the app world as a resource - let scratch_world = app_world.remove_resource::().unwrap(); - let render_world = std::mem::replace(&mut render_app.world, scratch_world.0); - app_world.insert_resource(RenderWorld(render_world)); - - extract.run(app_world); - - // add the render world back to the render app - let render_world = app_world.remove_resource::().unwrap(); - let scratch_world = std::mem::replace(&mut render_app.world, render_world.0); - app_world.insert_resource(ScratchRenderWorld(scratch_world)); - - extract.apply_buffers(&mut render_app.world); + // temporarily add the app world to the render world as a resource + let scratch_world = app_world.remove_resource::().unwrap(); + let inserted_world = std::mem::replace(app_world, scratch_world.0); + let running_world = &mut render_app.world; + running_world.insert_resource(MainWorld(inserted_world)); + + extract.run(running_world); + // move the app world back, as if nothing happened. + let inserted_world = running_world.remove_resource::().unwrap(); + let scratch_world = std::mem::replace(app_world, inserted_world.0); + app_world.insert_resource(ScratchMainWorld(scratch_world)); + + // Note: We apply buffers (read, Commands) after the `MainWorld` has been removed from the render app's world + // so that in future, pipelining will be able to do this too without any code relying on it. + // see + extract.apply_buffers(running_world); } diff --git a/crates/bevy_render/src/render_asset.rs b/crates/bevy_render/src/render_asset.rs index b666b7c819a5a7..1348dd35413da3 100644 --- a/crates/bevy_render/src/render_asset.rs +++ b/crates/bevy_render/src/render_asset.rs @@ -1,4 +1,4 @@ -use crate::{RenderApp, RenderStage}; +use crate::{Extract, RenderApp, RenderStage}; use bevy_app::{App, Plugin}; use bevy_asset::{Asset, AssetEvent, Assets, Handle}; use bevy_ecs::{ @@ -123,15 +123,15 @@ pub type RenderAssets = HashMap, ::PreparedAsset> /// into the "render world". fn extract_render_asset( mut commands: Commands, - mut events: EventReader>, - assets: Res>, + mut events: Extract>>, + assets: Extract>>, ) { let mut changed_assets = HashSet::default(); let mut removed = Vec::new(); for event in events.iter() { match event { AssetEvent::Created { handle } | AssetEvent::Modified { handle } => { - changed_assets.insert(handle); + changed_assets.insert(handle.clone_weak()); } AssetEvent::Removed { handle } => { changed_assets.remove(handle); @@ -142,8 +142,8 @@ fn extract_render_asset( let mut extracted_assets = Vec::new(); for handle in changed_assets.drain() { - if let Some(asset) = assets.get(handle) { - extracted_assets.push((handle.clone_weak(), asset.extract_asset())); + if let Some(asset) = assets.get(&handle) { + extracted_assets.push((handle, asset.extract_asset())); } } diff --git a/crates/bevy_render/src/render_resource/pipeline_cache.rs b/crates/bevy_render/src/render_resource/pipeline_cache.rs index 71021fbde9ccaa..e93d2f3008af53 100644 --- a/crates/bevy_render/src/render_resource/pipeline_cache.rs +++ b/crates/bevy_render/src/render_resource/pipeline_cache.rs @@ -7,7 +7,7 @@ use crate::{ ShaderProcessor, ShaderReflectError, }, renderer::RenderDevice, - RenderWorld, + Extract, }; use bevy_asset::{AssetEvent, Assets, Handle}; use bevy_ecs::event::EventReader; @@ -546,11 +546,10 @@ impl PipelineCache { } pub(crate) fn extract_shaders( - mut world: ResMut, - shaders: Res>, - mut events: EventReader>, + mut cache: ResMut, + shaders: Extract>>, + mut events: Extract>>, ) { - let mut cache = world.resource_mut::(); for event in events.iter() { match event { AssetEvent::Created { handle } | AssetEvent::Modified { handle } => { diff --git a/crates/bevy_render/src/view/window.rs b/crates/bevy_render/src/view/window.rs index 85557b5ed80afd..0104b85e86bd55 100644 --- a/crates/bevy_render/src/view/window.rs +++ b/crates/bevy_render/src/view/window.rs @@ -2,7 +2,7 @@ use crate::{ render_resource::TextureView, renderer::{RenderDevice, RenderInstance}, texture::BevyDefault, - RenderApp, RenderStage, RenderWorld, + Extract, RenderApp, RenderStage, }; use bevy_app::{App, Plugin}; use bevy_ecs::prelude::*; @@ -68,11 +68,10 @@ impl DerefMut for ExtractedWindows { } fn extract_windows( - mut render_world: ResMut, - mut closed: EventReader, - windows: Res, + mut extracted_windows: ResMut, + mut closed: Extract>, + windows: Extract>, ) { - let mut extracted_windows = render_world.get_resource_mut::().unwrap(); for window in windows.iter() { let (new_width, new_height) = ( window.physical_width().max(1), diff --git a/crates/bevy_sprite/src/mesh2d/mesh.rs b/crates/bevy_sprite/src/mesh2d/mesh.rs index 7a6ebfeadff931..662b490b2df66b 100644 --- a/crates/bevy_sprite/src/mesh2d/mesh.rs +++ b/crates/bevy_sprite/src/mesh2d/mesh.rs @@ -17,7 +17,7 @@ use bevy_render::{ BevyDefault, DefaultImageSampler, GpuImage, Image, ImageSampler, TextureFormatPixelInfo, }, view::{ComputedVisibility, ExtractedView, ViewUniform, ViewUniformOffset, ViewUniforms}, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }; use bevy_transform::components::GlobalTransform; @@ -116,7 +116,7 @@ bitflags::bitflags! { pub fn extract_mesh2d( mut commands: Commands, mut previous_len: Local, - query: Query<(Entity, &ComputedVisibility, &GlobalTransform, &Mesh2dHandle)>, + query: Extract>, ) { let mut values = Vec::with_capacity(*previous_len); for (entity, computed_visibility, transform, handle) in query.iter() { diff --git a/crates/bevy_sprite/src/render/mod.rs b/crates/bevy_sprite/src/render/mod.rs index 1c3d39dbeaf59a..6dd8b5a0525e6b 100644 --- a/crates/bevy_sprite/src/render/mod.rs +++ b/crates/bevy_sprite/src/render/mod.rs @@ -23,7 +23,7 @@ use bevy_render::{ renderer::{RenderDevice, RenderQueue}, texture::{BevyDefault, Image}, view::{Msaa, ViewUniform, ViewUniformOffset, ViewUniforms, Visibility}, - RenderWorld, + Extract, }; use bevy_transform::components::GlobalTransform; use bevy_utils::FloatOrd; @@ -197,10 +197,9 @@ pub struct SpriteAssetEvents { } pub fn extract_sprite_events( - mut render_world: ResMut, - mut image_events: EventReader>, + mut events: ResMut, + mut image_events: Extract>>, ) { - let mut events = render_world.resource_mut::(); let SpriteAssetEvents { ref mut images } = *events; images.clear(); @@ -221,17 +220,18 @@ pub fn extract_sprite_events( } pub fn extract_sprites( - mut render_world: ResMut, - texture_atlases: Res>, - sprite_query: Query<(&Visibility, &Sprite, &GlobalTransform, &Handle)>, - atlas_query: Query<( - &Visibility, - &TextureAtlasSprite, - &GlobalTransform, - &Handle, - )>, + mut extracted_sprites: ResMut, + texture_atlases: Extract>>, + sprite_query: Extract)>>, + atlas_query: Extract< + Query<( + &Visibility, + &TextureAtlasSprite, + &GlobalTransform, + &Handle, + )>, + >, ) { - let mut extracted_sprites = render_world.resource_mut::(); extracted_sprites.sprites.clear(); for (visibility, sprite, transform, handle) in sprite_query.iter() { if !visibility.is_visible { diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index d0bebbad3609a5..d019bd3137110b 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/crates/bevy_text/src/text2d.rs @@ -10,7 +10,7 @@ use bevy_ecs::{ }; use bevy_math::{Vec2, Vec3}; use bevy_reflect::Reflect; -use bevy_render::{texture::Image, view::Visibility, RenderWorld}; +use bevy_render::{texture::Image, view::Visibility, Extract}; use bevy_sprite::{Anchor, ExtractedSprite, ExtractedSprites, TextureAtlas}; use bevy_transform::prelude::{GlobalTransform, Transform}; use bevy_utils::HashSet; @@ -61,16 +61,13 @@ pub struct Text2dBundle { } pub fn extract_text2d_sprite( - mut render_world: ResMut, - texture_atlases: Res>, - text_pipeline: Res, - windows: Res, - text2d_query: Query<(Entity, &Visibility, &Text, &GlobalTransform, &Text2dSize)>, + mut extracted_sprites: ResMut, + texture_atlases: Extract>>, + text_pipeline: Extract>, + windows: Extract>, + text2d_query: Extract>, ) { - let mut extracted_sprites = render_world.resource_mut::(); - let scale_factor = windows.scale_factor(WindowId::primary()) as f32; - for (entity, visibility, text, transform, calculated_size) in text2d_query.iter() { if !visibility.is_visible { continue; diff --git a/crates/bevy_ui/src/render/mod.rs b/crates/bevy_ui/src/render/mod.rs index cc356a00df7f84..0e7a0d73919870 100644 --- a/crates/bevy_ui/src/render/mod.rs +++ b/crates/bevy_ui/src/render/mod.rs @@ -21,7 +21,7 @@ use bevy_render::{ renderer::{RenderDevice, RenderQueue}, texture::Image, view::{ExtractedView, ViewUniforms, Visibility}, - RenderApp, RenderStage, RenderWorld, + Extract, RenderApp, RenderStage, }; use bevy_sprite::{Rect, SpriteAssetEvents, TextureAtlas}; use bevy_text::{DefaultTextPipeline, Text}; @@ -174,18 +174,19 @@ pub struct ExtractedUiNodes { } pub fn extract_uinodes( - mut render_world: ResMut, - images: Res>, - uinode_query: Query<( - &Node, - &GlobalTransform, - &UiColor, - &UiImage, - &Visibility, - Option<&CalculatedClip>, - )>, + mut extracted_uinodes: ResMut, + images: Extract>>, + uinode_query: Extract< + Query<( + &Node, + &GlobalTransform, + &UiColor, + &UiImage, + &Visibility, + Option<&CalculatedClip>, + )>, + >, ) { - let mut extracted_uinodes = render_world.resource_mut::(); extracted_uinodes.uinodes.clear(); for (uinode, transform, color, image, visibility, clip) in uinode_query.iter() { if !visibility.is_visible { @@ -226,8 +227,7 @@ pub struct DefaultCameraView(pub Entity); pub fn extract_default_ui_camera_view( mut commands: Commands, - render_world: Res, - query: Query<(Entity, &Camera, Option<&UiCameraConfig>), With>, + query: Extract), With>>, ) { for (entity, camera, camera_ui) in query.iter() { // ignore cameras with disabled ui @@ -245,10 +245,8 @@ pub fn extract_default_ui_camera_view( ..Default::default() }; projection.update(logical_size.x, logical_size.y); - // This roundabout approach is required because spawn().id() won't work in this context - let default_camera_view = render_world.entities().reserve_entity(); - commands - .get_or_spawn(default_camera_view) + let default_camera_view = commands + .spawn() .insert(ExtractedView { projection: projection.get_projection_matrix(), transform: GlobalTransform::from_xyz( @@ -258,7 +256,8 @@ pub fn extract_default_ui_camera_view( ), width: physical_size.x, height: physical_size.y, - }); + }) + .id(); commands.get_or_spawn(entity).insert_bundle(( DefaultCameraView(default_camera_view), RenderPhase::::default(), @@ -268,23 +267,22 @@ pub fn extract_default_ui_camera_view( } pub fn extract_text_uinodes( - mut render_world: ResMut, - texture_atlases: Res>, - text_pipeline: Res, - windows: Res, - uinode_query: Query<( - Entity, - &Node, - &GlobalTransform, - &Text, - &Visibility, - Option<&CalculatedClip>, - )>, + mut extracted_uinodes: ResMut, + texture_atlases: Extract>>, + text_pipeline: Extract>, + windows: Extract>, + uinode_query: Extract< + Query<( + Entity, + &Node, + &GlobalTransform, + &Text, + &Visibility, + Option<&CalculatedClip>, + )>, + >, ) { - let mut extracted_uinodes = render_world.resource_mut::(); - let scale_factor = windows.scale_factor(WindowId::primary()) as f32; - for (entity, uinode, transform, text, visibility, clip) in uinode_query.iter() { if !visibility.is_visible { continue; diff --git a/examples/2d/mesh2d_manual.rs b/examples/2d/mesh2d_manual.rs index 2e6c9f6c479a76..f9049c16d925b8 100644 --- a/examples/2d/mesh2d_manual.rs +++ b/examples/2d/mesh2d_manual.rs @@ -19,7 +19,7 @@ use bevy::{ }, texture::BevyDefault, view::VisibleEntities, - RenderApp, RenderStage, + Extract, RenderApp, RenderStage, }, sprite::{ DrawMesh2d, Mesh2dHandle, Mesh2dPipeline, Mesh2dPipelineKey, Mesh2dUniform, @@ -286,7 +286,9 @@ impl Plugin for ColoredMesh2dPlugin { pub fn extract_colored_mesh2d( mut commands: Commands, mut previous_len: Local, - query: Query<(Entity, &ComputedVisibility), With>, + // When extracting, you must use `Extract` to mark the `SystemParam`s + // which should be taken from the main world. + query: Extract>>, ) { let mut values = Vec::with_capacity(*previous_len); for (entity, computed_visibility) in query.iter() { diff --git a/examples/shader/animate_shader.rs b/examples/shader/animate_shader.rs index d06e0f43f648c8..f8ca9466f5a38a 100644 --- a/examples/shader/animate_shader.rs +++ b/examples/shader/animate_shader.rs @@ -4,13 +4,18 @@ use bevy::{ core_pipeline::core_3d::Transparent3d, - ecs::system::{lifetimeless::SRes, SystemParamItem}, + ecs::system::{ + lifetimeless::{Read, SRes}, + SystemParamItem, + }, pbr::{ DrawMesh, MeshPipeline, MeshPipelineKey, MeshUniform, SetMeshBindGroup, SetMeshViewBindGroup, }, prelude::*, render::{ + extract_component::{ExtractComponent, ExtractComponentPlugin}, + extract_resource::{ExtractResource, ExtractResourcePlugin}, mesh::MeshVertexBufferLayout, render_asset::RenderAssets, render_phase::{ @@ -64,6 +69,8 @@ impl Plugin for CustomMaterialPlugin { usage: BufferUsages::UNIFORM | BufferUsages::COPY_DST, mapped_at_creation: false, }); + app.add_plugin(ExtractComponentPlugin::::default()) + .add_plugin(ExtractResourcePlugin::::default()); app.sub_app_mut(RenderApp) .add_render_command::() @@ -73,26 +80,20 @@ impl Plugin for CustomMaterialPlugin { }) .init_resource::() .init_resource::>() - .add_system_to_stage(RenderStage::Extract, extract_time) - .add_system_to_stage(RenderStage::Extract, extract_custom_material) .add_system_to_stage(RenderStage::Prepare, prepare_time) .add_system_to_stage(RenderStage::Queue, queue_custom) .add_system_to_stage(RenderStage::Queue, queue_time_bind_group); } } -// extract the `CustomMaterial` component into the render world -fn extract_custom_material( - mut commands: Commands, - mut previous_len: Local, - mut query: Query>, -) { - let mut values = Vec::with_capacity(*previous_len); - for entity in query.iter_mut() { - values.push((entity, (CustomMaterial,))); +impl ExtractComponent for CustomMaterial { + type Query = Read; + + type Filter = (); + + fn extract_component(_: bevy::ecs::query::QueryItem) -> Self { + CustomMaterial } - *previous_len = values.len(); - commands.insert_or_spawn_batch(values); } // add each entity with a mesh and a `CustomMaterial` to every view's `Transparent3d` render phase using the `CustomPipeline` @@ -138,11 +139,14 @@ struct ExtractedTime { seconds_since_startup: f32, } -// extract the passed time into a resource in the render world -fn extract_time(mut commands: Commands, time: Res