Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Make RenderStage::Extract run on the render world #4402

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -123,7 +123,7 @@ impl BatchedPhaseItem for Transparent2d {

pub fn extract_core_2d_camera_phases(
mut commands: Commands,
cameras_2d: Query<(Entity, &Camera), With<Camera2d>>,
cameras_2d: Extract<Query<(Entity, &Camera), With<Camera2d>>>,
) {
for (entity, camera) in cameras_2d.iter() {
if camera.is_active {
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use bevy_render::{
renderer::RenderDevice,
texture::TextureCache,
view::ViewDepthTexture,
RenderApp, RenderStage,
Extract, RenderApp, RenderStage,
};
use bevy_utils::{FloatOrd, HashMap};

Expand Down Expand Up @@ -208,7 +208,7 @@ impl CachedRenderPipelinePhaseItem for Transparent3d {

pub fn extract_core_3d_camera_phases(
mut commands: Commands,
cameras_3d: Query<(Entity, &Camera), With<Camera3d>>,
cameras_3d: Extract<Query<(Entity, &Camera), With<Camera3d>>>,
) {
for (entity, camera) in cameras_3d.iter() {
if camera.is_active {
Expand Down
28 changes: 27 additions & 1 deletion crates/bevy_ecs/src/schedule/stage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<ComponentId>,
}

impl SystemStage {
Expand All @@ -110,6 +114,7 @@ impl SystemStage {
uninitialized_at_end: vec![],
last_tick_check: Default::default(),
apply_buffers: true,
must_read_resource: None,
}
}

Expand Down Expand Up @@ -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<Params>(mut self, system: impl IntoSystemDescriptor<Params>) -> Self {
self.add_system(system);
Expand Down Expand Up @@ -563,6 +572,20 @@ impl SystemStage {
}
}

fn check_uses_resource(&self, resource_id: ComponentId, world: &World) {
cart marked this conversation as resolved.
Show resolved Hide resolved
debug_assert!(!self.systems_modified);
for system in &self.parallel {
let access = system.component_access().unwrap();
if !access.has_read(resource_id) {
cart marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -782,6 +805,9 @@ impl Stage for SystemStage {
if world.contains_resource::<ReportExecutionOrderAmbiguities>() {
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;
Expand Down
6 changes: 0 additions & 6 deletions crates/bevy_ecs/src/system/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions crates/bevy_pbr/src/material.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -455,15 +455,15 @@ pub type RenderMaterials<T> = HashMap<Handle<T>, PreparedMaterial<T>>;
/// into the "render world".
fn extract_materials<M: Material>(
mut commands: Commands,
mut events: EventReader<AssetEvent<M>>,
assets: Res<Assets<M>>,
mut events: Extract<EventReader<AssetEvent<M>>>,
assets: Extract<Res<Assets<M>>>,
) {
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);
Expand All @@ -474,8 +474,8 @@ fn extract_materials<M: Material>(

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()));
}
}

Expand Down
55 changes: 32 additions & 23 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use bevy_render::{
view::{
ExtractedView, ViewUniform, ViewUniformOffset, ViewUniforms, Visibility, VisibleEntities,
},
Extract,
};
use bevy_transform::components::GlobalTransform;
use bevy_utils::FloatOrd;
Expand Down Expand Up @@ -386,7 +387,10 @@ pub struct ExtractedClustersPointLights {
data: Vec<VisiblePointLights>,
}

pub fn extract_clusters(mut commands: Commands, views: Query<(Entity, &Clusters), With<Camera>>) {
pub fn extract_clusters(
mut commands: Commands,
views: Extract<Query<(Entity, &Clusters), With<Camera>>>,
) {
for (entity, clusters) in views.iter() {
commands.get_or_spawn(entity).insert_bundle((
ExtractedClustersPointLights {
Expand All @@ -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<PointLightShadowMap>,
directional_light_shadow_map: Res<DirectionalLightShadowMap>,
global_point_lights: Res<GlobalVisiblePointLights>,
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<SpotLight>,
point_light_shadow_map: Extract<Res<PointLightShadowMap>>,
directional_light_shadow_map: Extract<Res<DirectionalLightShadowMap>>,
global_point_lights: Extract<Res<GlobalVisiblePointLights>>,
point_lights: Extract<Query<(&PointLight, &CubemapVisibleEntities, &GlobalTransform)>>,
spot_lights: Extract<Query<(&SpotLight, &VisibleEntities, &GlobalTransform)>>,
directional_lights: Extract<
Query<
(
Entity,
&DirectionalLight,
&VisibleEntities,
&GlobalTransform,
&Visibility,
),
Without<SpotLight>,
>,
>,
mut previous_point_lights_len: Local<usize>,
mut previous_spot_lights_len: Local<usize>,
Expand All @@ -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();
cart marked this conversation as resolved.
Show resolved Hide resolved
point_lights_values.push((
entity,
(
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
26 changes: 14 additions & 12 deletions crates/bevy_pbr/src/render/mesh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -118,14 +118,16 @@ pub fn extract_meshes(
mut commands: Commands,
mut prev_caster_commands_len: Local<usize>,
mut prev_not_caster_commands_len: Local<usize>,
meshes_query: Query<(
Entity,
&ComputedVisibility,
&GlobalTransform,
&Handle<Mesh>,
Option<With<NotShadowReceiver>>,
Option<With<NotShadowCaster>>,
)>,
meshes_query: Extract<
Query<(
Entity,
&ComputedVisibility,
&GlobalTransform,
&Handle<Mesh>,
Option<With<NotShadowReceiver>>,
Option<With<NotShadowCaster>>,
)>,
>,
) {
let mut caster_commands = Vec::with_capacity(*prev_caster_commands_len);
let mut not_caster_commands = Vec::with_capacity(*prev_not_caster_commands_len);
Expand Down Expand Up @@ -202,12 +204,12 @@ impl SkinnedMeshJoints {
}

pub fn extract_skinned_meshes(
query: Query<(Entity, &ComputedVisibility, &SkinnedMesh)>,
inverse_bindposes: Res<Assets<SkinnedMeshInverseBindposes>>,
joint_query: Query<&GlobalTransform>,
mut commands: Commands,
mut previous_len: Local<usize>,
mut previous_joint_len: Local<usize>,
query: Extract<Query<(Entity, &ComputedVisibility, &SkinnedMesh)>>,
inverse_bindposes: Extract<Res<Assets<SkinnedMeshInverseBindposes>>>,
joint_query: Extract<Query<&GlobalTransform>>,
) {
let mut values = Vec::with_capacity(*previous_len);
let mut joints = Vec::with_capacity(*previous_joint_len);
Expand Down
17 changes: 10 additions & 7 deletions crates/bevy_render/src/camera/camera.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 7 additions & 7 deletions crates/bevy_render/src/extract_component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -34,9 +34,9 @@ impl<C: Component> DynamicUniformIndex<C> {
/// 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::Query>) -> Self;
}
Expand Down Expand Up @@ -182,7 +182,7 @@ impl<T: Asset> ExtractComponent for Handle<T> {
fn extract_components<C: ExtractComponent>(
mut commands: Commands,
mut previous_len: Local<usize>,
mut query: StaticSystemParam<Query<(Entity, C::Query), C::Filter>>,
mut query: Extract<Query<(Entity, C::Query), C::Filter>>,
) {
let mut values = Vec::with_capacity(*previous_len);
for (entity, query_item) in query.iter_mut() {
Expand All @@ -196,7 +196,7 @@ fn extract_components<C: ExtractComponent>(
fn extract_visible_components<C: ExtractComponent>(
mut commands: Commands,
mut previous_len: Local<usize>,
mut query: StaticSystemParam<Query<(Entity, Read<ComputedVisibility>, C::Query), C::Filter>>,
mut query: Extract<Query<(Entity, &ComputedVisibility, C::Query), C::Filter>>,
) {
let mut values = Vec::with_capacity(*previous_len);
for (entity, computed_visibility, query_item) in query.iter_mut() {
Expand Down
Loading