From 589fc7dfe0899bd6a0cd92f7864ed40ba7e98ef2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sat, 29 Mar 2025 23:58:06 -0700 Subject: [PATCH 1/6] Don't use commands for mutation! --- crates/bevy_pbr/src/prepass/mod.rs | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 7b70df38aab70..7dd73c5829cff 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -268,18 +268,30 @@ type PreviousMeshFilter = Or<(With, With)>; pub fn update_mesh_previous_global_transforms( mut commands: Commands, views: Query<&Camera, Or<(With, With)>>, - meshes: Query<(Entity, &GlobalTransform, Option<&PreviousGlobalTransform>), PreviousMeshFilter>, + mut meshes: Query< + ( + Entity, + &GlobalTransform, + Option<&mut PreviousGlobalTransform>, + ), + PreviousMeshFilter, + >, ) { let should_run = views.iter().any(|camera| camera.is_active); if should_run { - for (entity, transform, old_previous_transform) in &meshes { + for (entity, transform, old_previous_transform) in &mut meshes { let new_previous_transform = PreviousGlobalTransform(transform.affine()); - // Make sure not to trigger change detection on - // `PreviousGlobalTransform` if the previous transform hasn't - // changed. - if old_previous_transform != Some(&new_previous_transform) { - commands.entity(entity).try_insert(new_previous_transform); + match old_previous_transform { + None => { + commands.entity(entity).try_insert(new_previous_transform); + } + // Make sure not to trigger change detection on + // `PreviousGlobalTransform` if the previous transform hasn't + // changed. + Some(mut old_previous_transform) => { + old_previous_transform.set_if_neq(new_previous_transform); + } } } } From f9e1e679cc7b79694c5b1d7b3dc2812a74aa1386 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 30 Mar 2025 00:02:19 -0700 Subject: [PATCH 2/6] Move should_run into an actual run condition --- crates/bevy_pbr/src/prepass/mod.rs | 36 ++++++++++++++++-------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 7dd73c5829cff..87e46767df7ba 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -178,7 +178,7 @@ where .add_systems( PreUpdate, ( - update_mesh_previous_global_transforms, + update_mesh_previous_global_transforms.run_if(any_camera_active), update_previous_view_data, ), ) @@ -265,9 +265,15 @@ type PreviousMeshFilter = With; #[cfg(feature = "meshlet")] type PreviousMeshFilter = Or<(With, With)>; +/// A run condition for [`update_mesh_previous_global_transforms`] that checks if any camera is active. +pub fn any_camera_active(views: Query<&Camera, Or<(With, With)>>) -> bool { + views.iter().any(|camera| camera.is_active) +} + +/// Updates the [`PreviousGlobalTransform`] of all meshes that have a [`PreviousMeshFilter`] component, +/// inserting it if needed. pub fn update_mesh_previous_global_transforms( mut commands: Commands, - views: Query<&Camera, Or<(With, With)>>, mut meshes: Query< ( Entity, @@ -277,21 +283,17 @@ pub fn update_mesh_previous_global_transforms( PreviousMeshFilter, >, ) { - let should_run = views.iter().any(|camera| camera.is_active); - - if should_run { - for (entity, transform, old_previous_transform) in &mut meshes { - let new_previous_transform = PreviousGlobalTransform(transform.affine()); - match old_previous_transform { - None => { - commands.entity(entity).try_insert(new_previous_transform); - } - // Make sure not to trigger change detection on - // `PreviousGlobalTransform` if the previous transform hasn't - // changed. - Some(mut old_previous_transform) => { - old_previous_transform.set_if_neq(new_previous_transform); - } + for (entity, transform, old_previous_transform) in &mut meshes { + let new_previous_transform = PreviousGlobalTransform(transform.affine()); + match old_previous_transform { + None => { + commands.entity(entity).try_insert(new_previous_transform); + } + // Make sure not to trigger change detection on + // `PreviousGlobalTransform` if the previous transform hasn't + // changed. + Some(mut old_previous_transform) => { + old_previous_transform.set_if_neq(new_previous_transform); } } } From b0bf2d75f4836cc3cafd0e3bc23dce42b7f73cb2 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 30 Mar 2025 00:13:41 -0700 Subject: [PATCH 3/6] Statically avoid branching by stealing @aevyrie's homework --- crates/bevy_pbr/src/prepass/mod.rs | 31 +++++++++++------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index 87e46767df7ba..e8024072421cd 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -274,28 +274,19 @@ pub fn any_camera_active(views: Query<&Camera, Or<(With, With, - ), - PreviousMeshFilter, + mut meshes: Query<(&GlobalTransform, &mut PreviousGlobalTransform), PreviousMeshFilter>, + new_meshes: Query< + (Entity, &GlobalTransform), + (PreviousMeshFilter, Without), >, ) { - for (entity, transform, old_previous_transform) in &mut meshes { - let new_previous_transform = PreviousGlobalTransform(transform.affine()); - match old_previous_transform { - None => { - commands.entity(entity).try_insert(new_previous_transform); - } - // Make sure not to trigger change detection on - // `PreviousGlobalTransform` if the previous transform hasn't - // changed. - Some(mut old_previous_transform) => { - old_previous_transform.set_if_neq(new_previous_transform); - } - } + for (entity, transform) in &new_meshes { + commands + .entity(entity) + .try_insert(PreviousGlobalTransform(transform.affine())); + } + for (transform, mut previous) in &mut meshes { + previous.set_if_neq(PreviousGlobalTransform(transform.affine())); } } From 34b1e5f6b88f455cfb0a567363d64efddc2de0b5 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 30 Mar 2025 00:14:30 -0700 Subject: [PATCH 4/6] Make docs more correct: filter is not a component --- crates/bevy_pbr/src/prepass/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index e8024072421cd..fef8b41829e6a 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -270,7 +270,7 @@ pub fn any_camera_active(views: Query<&Camera, Or<(With, With Date: Sun, 30 Mar 2025 00:23:09 -0700 Subject: [PATCH 5/6] Required components and slap some par_iter on that thang --- crates/bevy_pbr/src/prepass/mod.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index fef8b41829e6a..d4379aa374736 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -187,7 +187,13 @@ where BinnedRenderPhasePlugin::::new( self.debug_flags, ), - )); + )) + // This can't be an attribute on the mesh types due to the shape of our dependency graph + .register_required_components::(); + + // + #[cfg(feature = "meshlet")] + app.register_required_components::(); } let Some(render_app) = app.get_sub_app_mut(RenderApp) else { @@ -257,6 +263,11 @@ pub fn update_previous_view_data( } } +/// Tracks the previous [`GlobalTransform`] of a mesh, +/// assuming that the mesh's transform is affine. +/// +/// This is added as a required component for [`Mesh3d`] and the meshlet equivalent in +/// the [`PrepassPlugin`]. #[derive(Component, PartialEq, Default)] pub struct PreviousGlobalTransform(pub Affine3A); @@ -270,24 +281,13 @@ pub fn any_camera_active(views: Query<&Camera, Or<(With, With, - new_meshes: Query< - (Entity, &GlobalTransform), - (PreviousMeshFilter, Without), - >, ) { - for (entity, transform) in &new_meshes { - commands - .entity(entity) - .try_insert(PreviousGlobalTransform(transform.affine())); - } - for (transform, mut previous) in &mut meshes { + meshes.par_iter_mut().for_each(|(transform, mut previous)| { previous.set_if_neq(PreviousGlobalTransform(transform.affine())); - } + }); } #[derive(Resource)] From 36c6ea9fe28d9cf95843d082f998f142ce90892f Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Sun, 30 Mar 2025 14:38:02 -0400 Subject: [PATCH 6/6] Stray comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: François Mockers --- crates/bevy_pbr/src/prepass/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/bevy_pbr/src/prepass/mod.rs b/crates/bevy_pbr/src/prepass/mod.rs index d4379aa374736..c51d1639f472b 100644 --- a/crates/bevy_pbr/src/prepass/mod.rs +++ b/crates/bevy_pbr/src/prepass/mod.rs @@ -191,7 +191,6 @@ where // This can't be an attribute on the mesh types due to the shape of our dependency graph .register_required_components::(); - // #[cfg(feature = "meshlet")] app.register_required_components::(); }