Skip to content

Commit

Permalink
Implement WorldQuery for MainWorld and RenderWorld components (#…
Browse files Browse the repository at this point in the history
…15745)

# Objective

#15320 is a particularly painful breaking change, and the new
`RenderEntity` in particular is very noisy, with a lot of `let entity =
entity.id()` spam.

## Solution

Implement `WorldQuery`, `QueryData` and `ReadOnlyQueryData` for
`RenderEntity` and `WorldEntity`.

These work the same as the `Entity` impls from a user-facing
perspective: they simply return an owned (copied) `Entity` identifier.
This dramatically reduces noise and eases migration.

Under the hood, these impls defer to the implementations for `&T` for
everything other than the "call .id() for the user" bit, as they involve
read-only access to component data. Doing it this way (as opposed to
implementing a custom fetch, as tried in the first commit) dramatically
reduces the maintenance risk of complex unsafe code outside of
`bevy_ecs`.

To make this easier (and encourage users to do this themselves!), I've
made `ReadFetch` and `WriteFetch` slightly more public: they're no
longer `doc(hidden)`. This is a good change, since trying to vendor the
logic is much worse than just deferring to the existing tested impls.

## Testing

I've run a handful of rendering examples (breakout, alien_cake_addict,
auto_exposure, fog_volumes, box_shadow) and nothing broke.

## Follow-up

We should lint for the uses of `&RenderEntity` and `&MainEntity` in
queries: this is just less nice for no reason.

---------

Co-authored-by: Trashtalk217 <trashtalk217@gmail.com>
  • Loading branch information
alice-i-cecile and Trashtalk217 authored Oct 13, 2024
1 parent d96a9d1 commit a7e9330
Show file tree
Hide file tree
Showing 21 changed files with 291 additions and 87 deletions.
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/auto_exposure/buffers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ pub(super) struct ExtractedStateBuffers {

pub(super) fn extract_buffers(
mut commands: Commands,
changed: Extract<Query<(&RenderEntity, &AutoExposure), Changed<AutoExposure>>>,
changed: Extract<Query<(RenderEntity, &AutoExposure), Changed<AutoExposure>>>,
mut removed: Extract<RemovedComponents<AutoExposure>>,
) {
commands.insert_resource(ExtractedStateBuffers {
changed: changed
.iter()
.map(|(entity, settings)| (entity.id(), settings.clone()))
.map(|(entity, settings)| (entity, settings.clone()))
.collect(),
removed: removed.read().collect(),
});
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_core_pipeline/src/core_2d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ pub fn extract_core_2d_camera_phases(
mut transparent_2d_phases: ResMut<ViewSortedRenderPhases<Transparent2d>>,
mut opaque_2d_phases: ResMut<ViewBinnedRenderPhases<Opaque2d>>,
mut alpha_mask_2d_phases: ResMut<ViewBinnedRenderPhases<AlphaMask2d>>,
cameras_2d: Extract<Query<(&RenderEntity, &Camera), With<Camera2d>>>,
cameras_2d: Extract<Query<(RenderEntity, &Camera), With<Camera2d>>>,
mut live_entities: Local<EntityHashSet>,
) {
live_entities.clear();
Expand All @@ -384,7 +384,6 @@ pub fn extract_core_2d_camera_phases(
if !camera.is_active {
continue;
}
let entity = entity.id();
transparent_2d_phases.insert_or_clear(entity);
opaque_2d_phases.insert_or_clear(entity);
alpha_mask_2d_phases.insert_or_clear(entity);
Expand Down
18 changes: 5 additions & 13 deletions crates/bevy_core_pipeline/src/core_3d/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -528,17 +528,16 @@ pub fn extract_core_3d_camera_phases(
mut alpha_mask_3d_phases: ResMut<ViewBinnedRenderPhases<AlphaMask3d>>,
mut transmissive_3d_phases: ResMut<ViewSortedRenderPhases<Transmissive3d>>,
mut transparent_3d_phases: ResMut<ViewSortedRenderPhases<Transparent3d>>,
cameras_3d: Extract<Query<(&RenderEntity, &Camera), With<Camera3d>>>,
cameras_3d: Extract<Query<(RenderEntity, &Camera), With<Camera3d>>>,
mut live_entities: Local<EntityHashSet>,
) {
live_entities.clear();

for (render_entity, camera) in &cameras_3d {
for (entity, camera) in &cameras_3d {
if !camera.is_active {
continue;
}

let entity = render_entity.id();
opaque_3d_phases.insert_or_clear(entity);
alpha_mask_3d_phases.insert_or_clear(entity);
transmissive_3d_phases.insert_or_clear(entity);
Expand All @@ -563,7 +562,7 @@ pub fn extract_camera_prepass_phase(
cameras_3d: Extract<
Query<
(
&RenderEntity,
RenderEntity,
&Camera,
Has<DepthPrepass>,
Has<NormalPrepass>,
Expand All @@ -577,20 +576,13 @@ pub fn extract_camera_prepass_phase(
) {
live_entities.clear();

for (
render_entity,
camera,
depth_prepass,
normal_prepass,
motion_vector_prepass,
deferred_prepass,
) in cameras_3d.iter()
for (entity, camera, depth_prepass, normal_prepass, motion_vector_prepass, deferred_prepass) in
cameras_3d.iter()
{
if !camera.is_active {
continue;
}

let entity = render_entity.id();
if depth_prepass || normal_prepass || motion_vector_prepass {
opaque_3d_prepass_phases.insert_or_clear(entity);
alpha_mask_3d_prepass_phases.insert_or_clear(entity);
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_core_pipeline/src/dof/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ impl SpecializedRenderPipeline for DepthOfFieldPipeline {
/// Extracts all [`DepthOfField`] components into the render world.
fn extract_depth_of_field_settings(
mut commands: Commands,
mut query: Extract<Query<(&RenderEntity, &DepthOfField, &Projection)>>,
mut query: Extract<Query<(RenderEntity, &DepthOfField, &Projection)>>,
) {
if !DEPTH_TEXTURE_SAMPLING_SUPPORTED {
info_once!(
Expand All @@ -820,7 +820,6 @@ fn extract_depth_of_field_settings(
}

for (entity, depth_of_field, projection) in query.iter_mut() {
let entity = entity.id();
// Depth of field is nonsensical without a perspective projection.
let Projection::Perspective(ref perspective_projection) = *projection else {
continue;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_core_pipeline/src/taa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ impl SpecializedRenderPipeline for TaaPipeline {

fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut<MainWorld>) {
let mut cameras_3d = main_world.query_filtered::<(
&RenderEntity,
RenderEntity,
&Camera,
&Projection,
&mut TemporalAntiAliasing,
Expand All @@ -375,7 +375,7 @@ fn extract_taa_settings(mut commands: Commands, mut main_world: ResMut<MainWorld
let has_perspective_projection = matches!(camera_projection, Projection::Perspective(_));
if camera.is_active && has_perspective_projection {
commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Camera entity wasn't synced.")
.insert(taa_settings.clone());
taa_settings.reset = false;
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/query/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,7 @@ unsafe impl QueryData for &Archetype {
/// SAFETY: access is read only
unsafe impl ReadOnlyQueryData for &Archetype {}

#[doc(hidden)]
/// The [`WorldQuery::Fetch`] type for `& T`.
pub struct ReadFetch<'w, T: Component> {
components: StorageSwitch<
T,
Expand Down Expand Up @@ -1415,7 +1415,7 @@ unsafe impl<'__w, T: Component> QueryData for Ref<'__w, T> {
/// SAFETY: access is read only
unsafe impl<'__w, T: Component> ReadOnlyQueryData for Ref<'__w, T> {}

#[doc(hidden)]
/// The [`WorldQuery::Fetch`] type for `&mut T`.
pub struct WriteFetch<'w, T: Component> {
components: StorageSwitch<
T,
Expand Down
8 changes: 4 additions & 4 deletions crates/bevy_pbr/src/cluster/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,8 +526,8 @@ pub(crate) fn clusterable_object_order(
/// Extracts clusters from the main world from the render world.
pub fn extract_clusters(
mut commands: Commands,
views: Extract<Query<(&RenderEntity, &Clusters, &Camera)>>,
mapper: Extract<Query<&RenderEntity>>,
views: Extract<Query<(RenderEntity, &Clusters, &Camera)>>,
mapper: Extract<Query<RenderEntity>>,
) {
for (entity, clusters, camera) in &views {
if !camera.is_active {
Expand All @@ -548,14 +548,14 @@ pub fn extract_clusters(
for clusterable_entity in &cluster_objects.entities {
if let Ok(entity) = mapper.get(*clusterable_entity) {
data.push(ExtractedClusterableObjectElement::ClusterableObjectEntity(
entity.id(),
entity,
));
}
}
}

commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Clusters entity wasn't synced.")
.insert((
ExtractedClusterableObjects { data },
Expand Down
11 changes: 5 additions & 6 deletions crates/bevy_pbr/src/light_probe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ impl Plugin for LightProbePlugin {
/// Compared to the `ExtractComponentPlugin`, this implementation will create a default instance
/// if one does not already exist.
fn gather_environment_map_uniform(
view_query: Extract<Query<(&RenderEntity, Option<&EnvironmentMapLight>), With<Camera3d>>>,
view_query: Extract<Query<(RenderEntity, Option<&EnvironmentMapLight>), With<Camera3d>>>,
mut commands: Commands,
) {
for (view_entity, environment_map_light) in view_query.iter() {
Expand All @@ -386,7 +386,7 @@ fn gather_environment_map_uniform(
EnvironmentMapUniform::default()
};
commands
.get_entity(view_entity.id())
.get_entity(view_entity)
.expect("Environment map light entity wasn't synced.")
.insert(environment_map_uniform);
}
Expand All @@ -398,7 +398,7 @@ fn gather_light_probes<C>(
image_assets: Res<RenderAssets<GpuImage>>,
light_probe_query: Extract<Query<(&GlobalTransform, &C), With<LightProbe>>>,
view_query: Extract<
Query<(&RenderEntity, &GlobalTransform, &Frustum, Option<&C>), With<Camera3d>>,
Query<(RenderEntity, &GlobalTransform, &Frustum, Option<&C>), With<Camera3d>>,
>,
mut reflection_probes: Local<Vec<LightProbeInfo<C>>>,
mut view_reflection_probes: Local<Vec<LightProbeInfo<C>>>,
Expand Down Expand Up @@ -437,16 +437,15 @@ fn gather_light_probes<C>(
// Gather up the light probes in the list.
render_view_light_probes.maybe_gather_light_probes(&view_reflection_probes);

let entity = view_entity.id();
// Record the per-view light probes.
if render_view_light_probes.is_empty() {
commands
.get_entity(entity)
.get_entity(view_entity)
.expect("View entity wasn't synced.")
.remove::<RenderViewLightProbes<C>>();
} else {
commands
.get_entity(entity)
.get_entity(view_entity)
.expect("View entity wasn't synced.")
.insert(render_view_light_probes);
}
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_pbr/src/prepass/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,10 @@ where
// Extract the render phases for the prepass
pub fn extract_camera_previous_view_data(
mut commands: Commands,
cameras_3d: Extract<Query<(&RenderEntity, &Camera, Option<&PreviousViewData>), With<Camera3d>>>,
cameras_3d: Extract<Query<(RenderEntity, &Camera, Option<&PreviousViewData>), With<Camera3d>>>,
) {
for (entity, camera, maybe_previous_view_data) in cameras_3d.iter() {
if camera.is_active {
let entity = entity.id();
let mut entity = commands
.get_entity(entity)
.expect("Camera entity wasn't synced.");
Expand Down
23 changes: 11 additions & 12 deletions crates/bevy_pbr/src/render/light.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ pub fn extract_lights(
global_point_lights: Extract<Res<GlobalVisibleClusterableObjects>>,
point_lights: Extract<
Query<(
&RenderEntity,
RenderEntity,
&PointLight,
&CubemapVisibleEntities,
&GlobalTransform,
Expand All @@ -204,7 +204,7 @@ pub fn extract_lights(
>,
spot_lights: Extract<
Query<(
&RenderEntity,
RenderEntity,
&SpotLight,
&VisibleMeshEntities,
&GlobalTransform,
Expand All @@ -216,7 +216,7 @@ pub fn extract_lights(
directional_lights: Extract<
Query<
(
&RenderEntity,
RenderEntity,
&DirectionalLight,
&CascadesVisibleEntities,
&Cascades,
Expand All @@ -230,7 +230,7 @@ pub fn extract_lights(
Without<SpotLight>,
>,
>,
mapper: Extract<Query<&RenderEntity>>,
mapper: Extract<Query<RenderEntity>>,
mut previous_point_lights_len: Local<usize>,
mut previous_spot_lights_len: Local<usize>,
) {
Expand Down Expand Up @@ -298,7 +298,7 @@ pub fn extract_lights(
volumetric: volumetric_light.is_some(),
};
point_lights_values.push((
render_entity.id(),
render_entity,
(
extracted_point_light,
render_cubemap_visible_entities,
Expand Down Expand Up @@ -331,7 +331,7 @@ pub fn extract_lights(
2.0 * ops::tan(spot_light.outer_angle) / directional_light_shadow_map.size as f32;

spot_lights_values.push((
render_entity.id(),
render_entity,
(
ExtractedPointLight {
color: spot_light.color.into(),
Expand Down Expand Up @@ -388,22 +388,22 @@ pub fn extract_lights(
let mut cascade_visible_entities = EntityHashMap::default();
for (e, v) in cascades.cascades.iter() {
if let Ok(entity) = mapper.get(*e) {
extracted_cascades.insert(entity.id(), v.clone());
extracted_cascades.insert(entity, v.clone());
} else {
break;
}
}
for (e, v) in frusta.frusta.iter() {
if let Ok(entity) = mapper.get(*e) {
extracted_frusta.insert(entity.id(), v.clone());
extracted_frusta.insert(entity, v.clone());
} else {
break;
}
}
for (e, v) in visible_entities.entities.iter() {
if let Ok(entity) = mapper.get(*e) {
cascade_visible_entities.insert(
entity.id(),
entity,
v.iter()
.map(|v| create_render_visible_mesh_entities(&mut commands, &mapper, v))
.collect(),
Expand All @@ -414,7 +414,7 @@ pub fn extract_lights(
}

commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Light entity wasn't synced.")
.insert((
ExtractedDirectionalLight {
Expand Down Expand Up @@ -442,7 +442,7 @@ pub fn extract_lights(

fn create_render_visible_mesh_entities(
commands: &mut Commands,
mapper: &Extract<Query<&RenderEntity>>,
mapper: &Extract<Query<RenderEntity>>,
visible_entities: &VisibleMeshEntities,
) -> RenderVisibleMeshEntities {
RenderVisibleMeshEntities {
Expand All @@ -451,7 +451,6 @@ fn create_render_visible_mesh_entities(
.map(|e| {
let render_entity = mapper
.get(*e)
.map(RenderEntity::id)
.unwrap_or_else(|_| commands.spawn(TemporaryRenderEntity).id());
(render_entity, MainEntity::from(*e))
})
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_pbr/src/ssao/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ fn extract_ssao_settings(
mut commands: Commands,
cameras: Extract<
Query<
(&RenderEntity, &Camera, &ScreenSpaceAmbientOcclusion, &Msaa),
(RenderEntity, &Camera, &ScreenSpaceAmbientOcclusion, &Msaa),
(With<Camera3d>, With<DepthPrepass>, With<NormalPrepass>),
>,
>,
Expand All @@ -537,7 +537,7 @@ fn extract_ssao_settings(
}
if camera.is_active {
commands
.get_entity(entity.id())
.get_entity(entity)
.expect("SSAO entity wasn't synced.")
.insert(ssao_settings.clone());
}
Expand Down
12 changes: 6 additions & 6 deletions crates/bevy_pbr/src/volumetric_fog/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,32 +271,32 @@ impl FromWorld for VolumetricFogPipeline {
/// from the main world to the render world.
pub fn extract_volumetric_fog(
mut commands: Commands,
view_targets: Extract<Query<(&RenderEntity, &VolumetricFog)>>,
fog_volumes: Extract<Query<(&RenderEntity, &FogVolume, &GlobalTransform)>>,
volumetric_lights: Extract<Query<(&RenderEntity, &VolumetricLight)>>,
view_targets: Extract<Query<(RenderEntity, &VolumetricFog)>>,
fog_volumes: Extract<Query<(RenderEntity, &FogVolume, &GlobalTransform)>>,
volumetric_lights: Extract<Query<(RenderEntity, &VolumetricLight)>>,
) {
if volumetric_lights.is_empty() {
return;
}

for (entity, volumetric_fog) in view_targets.iter() {
commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Volumetric fog entity wasn't synced.")
.insert(*volumetric_fog);
}

for (entity, fog_volume, fog_transform) in fog_volumes.iter() {
commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Fog volume entity wasn't synced.")
.insert((*fog_volume).clone())
.insert(*fog_transform);
}

for (entity, volumetric_light) in volumetric_lights.iter() {
commands
.get_entity(entity.id())
.get_entity(entity)
.expect("Volumetric light entity wasn't synced.")
.insert(*volumetric_light);
}
Expand Down
Loading

0 comments on commit a7e9330

Please sign in to comment.