Merge remote-tracking branch 'upstream/main' into remove_with_required

benches/benches/bevy_ecs/iteration/iter_simple_foreach_hybrid.rs

@@ -20,7 +20,7 @@ impl<'w> Benchmark<'w> {
 
  let mut v = vec![];
  for _ in 0..10000 {
- world.spawn((TableData(0.0), SparseData(0.0))).id();
+ world.spawn((TableData(0.0), SparseData(0.0)));
  v.push(world.spawn(TableData(0.)).id());
  }
 

benches/benches/bevy_ecs/iteration/mod.rs

@@ -20,6 +20,7 @@ mod iter_simple_system;
 mod iter_simple_wide;
 mod iter_simple_wide_sparse_set;
 mod par_iter_simple;
+mod par_iter_simple_foreach_hybrid;
 
 use heavy_compute::*;
 
@@ -135,4 +136,8 @@ fn par_iter_simple(c: &mut Criterion) {
  b.iter(move || bench.run());
  });
  }
+ group.bench_function(format!("hybrid"), |b| {
+ let mut bench = par_iter_simple_foreach_hybrid::Benchmark::new();
+ b.iter(move || bench.run());
+ });
 }

benches/benches/bevy_ecs/iteration/par_iter_simple_foreach_hybrid.rs

@@ -0,0 +1,45 @@
+use bevy_ecs::prelude::*;
+use bevy_tasks::{ComputeTaskPool, TaskPool};
+use rand::{prelude::SliceRandom, SeedableRng};
+use rand_chacha::ChaCha8Rng;
+
+#[derive(Component, Copy, Clone)]
+struct TableData(f32);
+
+#[derive(Component, Copy, Clone)]
+#[component(storage = "SparseSet")]
+struct SparseData(f32);
+
+fn deterministic_rand() -> ChaCha8Rng {
+ ChaCha8Rng::seed_from_u64(42)
+}
+pub struct Benchmark<'w>(World, QueryState<(&'w mut TableData, &'w SparseData)>);
+
+impl<'w> Benchmark<'w> {
+ pub fn new() -> Self {
+ let mut world = World::new();
+ ComputeTaskPool::get_or_init(TaskPool::default);
+
+ let mut v = vec![];
+ for _ in 0..100000 {
+ world.spawn((TableData(0.0), SparseData(0.0)));
+ v.push(world.spawn(TableData(0.)).id());
+ }
+
+ // by shuffling ,randomize the archetype iteration order to significantly deviate from the table order. This maximizes the loss of cache locality during archetype-based iteration.
+ v.shuffle(&mut deterministic_rand());
+ for e in v.into_iter() {
+ world.entity_mut(e).despawn();
+ }
+
+ let query = world.query::<(&mut TableData, &SparseData)>();
+ Self(world, query)
+ }
+
+ #[inline(never)]
+ pub fn run(&mut self) {
+ self.1
+ .par_iter_mut(&mut self.0)
+ .for_each(|(mut v1, v2)| v1.0 += v2.0)
+ }
+}

crates/bevy_animation/src/lib.rs

@@ -1234,7 +1234,7 @@ impl Plugin for AnimationPlugin {
  (
  advance_transitions,
  advance_animations,
- animate_targets,
+ animate_targets.after(bevy_render::mesh::morph::inherit_weights),
  expire_completed_transitions,
  )
  .chain()

crates/bevy_app/src/terminal_ctrl_c_handler.rs

@@ -48,7 +48,7 @@ impl TerminalCtrlCHandlerPlugin {
  }
 
  /// Sends a [`AppExit`] event when the user presses `Ctrl+C` on the terminal.
- fn exit_on_flag(mut events: EventWriter<AppExit>) {
+ pub fn exit_on_flag(mut events: EventWriter<AppExit>) {
  if SHOULD_EXIT.load(Ordering::Relaxed) {
  events.send(AppExit::from_code(130));
  }

crates/bevy_asset/src/lib.rs

@@ -222,7 +222,11 @@ impl Plugin for AssetPlugin {
  .init_asset::<()>()
  .add_event::<UntypedAssetLoadFailedEvent>()
  .configure_sets(PreUpdate, TrackAssets.after(handle_internal_asset_events))
- .add_systems(PreUpdate, handle_internal_asset_events)
+ // `handle_internal_asset_events` requires the use of `&mut World`,
+ // and as a result has ambiguous system ordering with all other systems in `PreUpdate`.
+ // This is virtually never a real problem: asset loading is async and so anything that interacts directly with it
+ // needs to be robust to stochastic delays anyways.
+ .add_systems(PreUpdate, handle_internal_asset_events.ambiguous_with_all())
  .register_type::<AssetPath>();
  }
 }

crates/bevy_dev_tools/src/ci_testing/mod.rs

@@ -6,7 +6,7 @@ mod systems;
 pub use self::config::*;
 
 use bevy_app::prelude::*;
-use bevy_ecs::schedule::IntoSystemConfigs;
+use bevy_ecs::prelude::*;
 use bevy_render::view::screenshot::trigger_screenshots;
 use bevy_time::TimeUpdateStrategy;
 use std::time::Duration;
@@ -54,7 +54,19 @@ impl Plugin for CiTestingPlugin {
  Update,
  systems::send_events
  .before(trigger_screenshots)
- .before(bevy_window::close_when_requested),
+ .before(bevy_window::close_when_requested)
+ .in_set(SendEvents),
  );
+
+ // The offending system does not exist in the wasm32 target.
+ // As a result, we must conditionally order the two systems using a system set.
+ #[cfg(not(target_arch = "wasm32"))]
+ app.configure_sets(
+ Update,
+ SendEvents.before(bevy_app::TerminalCtrlCHandlerPlugin::exit_on_flag),
+ );
  }
 }
+
+#[derive(SystemSet, Debug, Clone, PartialEq, Eq, Hash)]
+struct SendEvents;

crates/bevy_dev_tools/src/ui_debug_overlay/inset.rs

@@ -137,7 +137,7 @@ impl<'w, 's> InsetGizmo<'w, 's> {
  let Ok(cam) = self.cam.get_single() else {
  return Vec2::ZERO;
  };
- if let Some(new_position) = cam.world_to_viewport(&zero, position.extend(0.)) {
+ if let Ok(new_position) = cam.world_to_viewport(&zero, position.extend(0.)) {
  position = new_position;
  };
  position.xy()

crates/bevy_ecs/src/query/iter.rs

@@ -122,6 +122,67 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
  }
  }
 
+ /// Executes the equivalent of [`Iterator::fold`] over a contiguous segment
+ /// from an storage.
+ ///
+ /// # Safety
+ /// - `range` must be in `[0, storage::entity_count)` or None.
+ #[inline]
+ pub(super) unsafe fn fold_over_storage_range<B, Func>(
+ &mut self,
+ mut accum: B,
+ func: &mut Func,
+ storage: StorageId,
+ range: Option<Range<usize>>,
+ ) -> B
+ where
+ Func: FnMut(B, D::Item<'w>) -> B,
+ {
+ if self.cursor.is_dense {
+ // SAFETY: `self.cursor.is_dense` is true, so storage ids are guaranteed to be table ids.
+ let table_id = unsafe { storage.table_id };
+ // SAFETY: Matched table IDs are guaranteed to still exist.
+ let table = unsafe { self.tables.get(table_id).debug_checked_unwrap() };
+
+ let range = range.unwrap_or(0..table.entity_count());
+ accum =
+ // SAFETY: 
+ // - The fetched table matches both D and F
+ // - caller ensures `range` is within `[0, table.entity_count)`
+ // - The if block ensures that the query iteration is dense
+ unsafe { self.fold_over_table_range(accum, func, table, range) };
+ } else {
+ // SAFETY: `self.cursor.is_dense` is false, so storage ids are guaranteed to be archetype ids.
+ let archetype_id = unsafe { storage.archetype_id };
+ // SAFETY: Matched archetype IDs are guaranteed to still exist.
+ let archetype = unsafe { self.archetypes.get(archetype_id).debug_checked_unwrap() };
+ // SAFETY: Matched table IDs are guaranteed to still exist.
+ let table = unsafe { self.tables.get(archetype.table_id()).debug_checked_unwrap() };
+
+ let range = range.unwrap_or(0..archetype.len());
+
+ // When an archetype and its table have equal entity counts, dense iteration can be safely used.
+ // this leverages cache locality to optimize performance.
+ if table.entity_count() == archetype.len() {
+ accum =
+ // SAFETY:
+ // - The fetched archetype matches both D and F
+ // - The provided archetype and its' table have the same length.
+ // - caller ensures `range` is within `[0, archetype.len)`
+ // - The if block ensures that the query iteration is not dense.
+ unsafe { self.fold_over_dense_archetype_range(accum, func, archetype,range) };
+ } else {
+ accum =
+ // SAFETY:
+ // - The fetched archetype matches both D and F
+ // - caller ensures `range` is within `[0, archetype.len)`
+ // - The if block ensures that the query iteration is not dense.
+ unsafe { self.fold_over_archetype_range(accum, func, archetype,range) };
+ }
+ }
+ accum
+ }
+
  /// Executes the equivalent of [`Iterator::fold`] over a contiguous segment
  /// from an table.
  ///
@@ -143,7 +204,7 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
  if table.is_empty() {
  return accum;
  }
- assert!(
+ debug_assert!(
  rows.end <= u32::MAX as usize,
  "TableRow is only valid up to u32::MAX"
  );
@@ -267,12 +328,11 @@ impl<'w, 's, D: QueryData, F: QueryFilter> QueryIter<'w, 's, D, F> {
  if archetype.is_empty() {
  return accum;
  }
- assert!(
+ debug_assert!(
  rows.end <= u32::MAX as usize,
  "TableRow is only valid up to u32::MAX"
  );
  let table = self.tables.get(archetype.table_id()).debug_checked_unwrap();
-
  debug_assert!(
  archetype.len() == table.entity_count(),
  "archetype and it's table must have the same length. "
@@ -1032,48 +1092,10 @@ impl<'w, 's, D: QueryData, F: QueryFilter> Iterator for QueryIter<'w, 's, D, F>
  accum = func(accum, item);
  }
 
- if self.cursor.is_dense {
- for id in self.cursor.storage_id_iter.clone() {
- // SAFETY: `self.cursor.is_dense` is true, so storage ids are guaranteed to be table ids.
- let table_id = unsafe { id.table_id };
- // SAFETY: Matched table IDs are guaranteed to still exist.
- let table = unsafe { self.tables.get(table_id).debug_checked_unwrap() };
-
- accum =
- // SAFETY: 
- // - The fetched table matches both D and F
- // - The provided range is equivalent to [0, table.entity_count)
- // - The if block ensures that the query iteration is dense
- unsafe { self.fold_over_table_range(accum, &mut func, table, 0..table.entity_count()) };
- }
- } else {
- for id in self.cursor.storage_id_iter.clone() {
- // SAFETY: `self.cursor.is_dense` is false, so storage ids are guaranteed to be archetype ids.
- let archetype_id = unsafe { id.archetype_id };
- // SAFETY: Matched archetype IDs are guaranteed to still exist.
- let archetype = unsafe { self.archetypes.get(archetype_id).debug_checked_unwrap() };
- // SAFETY: Matched table IDs are guaranteed to still exist.
- let table = unsafe { self.tables.get(archetype.table_id()).debug_checked_unwrap() };
-
- // When an archetype and its table have equal entity counts, dense iteration can be safely used.
- // this leverages cache locality to optimize performance.
- if table.entity_count() == archetype.len() {
- accum =
- // SAFETY:
- // - The fetched archetype matches both D and F
- // - The provided archetype and its' table have the same length.
- // - The provided range is equivalent to [0, archetype.len)
- // - The if block ensures that the query iteration is not dense.
- unsafe { self.fold_over_dense_archetype_range(accum, &mut func, archetype, 0..archetype.len()) };
- } else {
- accum =
- // SAFETY:
- // - The fetched archetype matches both D and F
- // - The provided range is equivalent to [0, archetype.len)
- // - The if block ensures that the query iteration is not dense.
- unsafe { self.fold_over_archetype_range(accum, &mut func, archetype, 0..archetype.len()) };
- }
- }
+ for id in self.cursor.storage_id_iter.clone().copied() {
+ // SAFETY:
+ // - The range(None) is equivalent to [0, storage.entity_count)
+ accum = unsafe { self.fold_over_storage_range(accum, &mut func, id, None) };
  }
  accum
  }

crates/bevy_ecs/src/query/state.rs

@@ -1505,25 +1505,7 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  let mut iter = self.iter_unchecked_manual(world, last_run, this_run);
  let mut accum = init_accum();
  for storage_id in queue {
- if self.is_dense {
- let id = storage_id.table_id;
- let table = &world.storages().tables.get(id).debug_checked_unwrap();
- accum = iter.fold_over_table_range(
- accum,
- &mut func,
- table,
- 0..table.entity_count(),
- );
- } else {
- let id = storage_id.archetype_id;
- let archetype = world.archetypes().get(id).debug_checked_unwrap();
- accum = iter.fold_over_archetype_range(
- accum,
- &mut func,
- archetype,
- 0..archetype.len(),
- );
- }
+ accum = iter.fold_over_storage_range(accum, &mut func, storage_id, None);
  }
  });
  };
@@ -1539,17 +1521,8 @@ impl<D: QueryData, F: QueryFilter> QueryState<D, F> {
  #[cfg(feature = "trace")]
  let _span = self.par_iter_span.enter();
  let accum = init_accum();
- if self.is_dense {
- let id = storage_id.table_id;
- let table = world.storages().tables.get(id).debug_checked_unwrap();
- self.iter_unchecked_manual(world, last_run, this_run)
- .fold_over_table_range(accum, &mut func, table, batch);
- } else {
- let id = storage_id.archetype_id;
- let archetype = world.archetypes().get(id).debug_checked_unwrap();
- self.iter_unchecked_manual(world, last_run, this_run)
- .fold_over_archetype_range(accum, &mut func, archetype, batch);
- }
+ self.iter_unchecked_manual(world, last_run, this_run)
+ .fold_over_storage_range(accum, &mut func, storage_id, Some(batch));
  });
  }
  };

crates/bevy_gizmos/src/aabb.rs

@@ -40,6 +40,7 @@ impl Plugin for AabbGizmoPlugin {
  config.config::<AabbGizmoConfigGroup>().1.draw_all
  }),
  )
+ .after(bevy_render::view::VisibilitySystems::CalculateBounds)
  .after(TransformSystem::TransformPropagate),
  );
  }

crates/bevy_gizmos/src/lib.rs

@@ -242,6 +242,9 @@ impl AppGizmoBuilder for App {
  handles.list.insert(TypeId::of::<Config>(), None);
  handles.strip.insert(TypeId::of::<Config>(), None);
 
+ // These handles are safe to mutate in any order
+ self.allow_ambiguous_resource::<LineGizmoHandles>();
+
  self.init_resource::<GizmoStorage<Config, ()>>()
  .init_resource::<GizmoStorage<Config, Fixed>>()
  .init_resource::<GizmoStorage<Config, Swap<Fixed>>>()

crates/bevy_internal/src/default_plugins.rs

@@ -90,6 +90,11 @@ impl Plugin for IgnoreAmbiguitiesPlugin {
  bevy_animation::advance_animations,
  bevy_ui::ui_layout_system,
  );
+ app.ignore_ambiguity(
+ bevy_app::PostUpdate,
+ bevy_animation::animate_targets,
+ bevy_ui::ui_layout_system,
+ );
  }
  }
 }

crates/bevy_pbr/src/lib.rs

@@ -340,10 +340,22 @@ impl Plugin for PbrPlugin {
  )
  .chain(),
  )
+ .configure_sets(
+ PostUpdate,
+ SimulationLightSystems::UpdateDirectionalLightCascades
+ .ambiguous_with(SimulationLightSystems::UpdateDirectionalLightCascades),
+ )
+ .configure_sets(
+ PostUpdate,
+ SimulationLightSystems::CheckLightVisibility
+ .ambiguous_with(SimulationLightSystems::CheckLightVisibility),
+ )
  .add_systems(
  PostUpdate,
  (
- add_clusters.in_set(SimulationLightSystems::AddClusters),
+ add_clusters
+ .in_set(SimulationLightSystems::AddClusters)
+ .after(CameraUpdateSystem),
  assign_objects_to_clusters
  .in_set(SimulationLightSystems::AssignLightsToClusters)
  .after(TransformSystem::TransformPropagate)