Skip to content

Commit

Permalink
Add the AssetChanged query filter
Browse files Browse the repository at this point in the history
Solves bevyengine#5069.

`AssetChanged<T>` is just a thin wrapper around the `&Handle<T>` query,
with additional logic to check in `Assets<T>` if the corresponding
`HandleId` was changed between the last and current runs of the system.

This adds some overhead in the `asset_event_system` for keeping track of
when which handle got updated. Otherwise, the overhead is constrained to
the `AssetChanged` query, which is ripe for optimizations.

NOTE: We chose to update the `change_ticks` hash map during asset event
propagation rather than when accessing the asset, because we need access
somehow to the current `change_tick`. Updating the tick hash map during
modification would require passing the `change_tick` to all methods of
`Assets` that accepts a `&mut self`. Which would be a huge ergonomic
loss.

As a result, the asset change expressed by `AssetChanged` is only
effective after `asset_event_system` ran. This is in line with how
`AssetEvents` currently works, but might cause a 1 frame lag for asset
change detection.

A potential future update could implement some sort of specialization so
that when `Assets` is accessed through a `ResMut`, the called methods
are called with the `ResMut`'s change_tick. But with current rust, this
is either impossible or causing huge ergonomic loss on the `Assets` API.
  • Loading branch information
nicopap committed Jun 28, 2022
1 parent 86dd6f0 commit a88c07b
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 3 deletions.
25 changes: 22 additions & 3 deletions crates/bevy_asset/src/assets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::{
use bevy_app::App;
use bevy_ecs::{
event::{EventWriter, Events},
system::ResMut,
system::{ResMut, SystemChangeTick},
world::FromWorld,
};
use bevy_utils::HashMap;
Expand Down Expand Up @@ -65,6 +65,7 @@ impl<T: Asset> Debug for AssetEvent<T> {
#[derive(Debug)]
pub struct Assets<T: Asset> {
assets: HashMap<HandleId, T>,
pub(crate) change_ticks: HashMap<HandleId, u32>,
events: Events<AssetEvent<T>>,
pub(crate) ref_change_sender: Sender<RefChange>,
}
Expand All @@ -73,6 +74,7 @@ impl<T: Asset> Assets<T> {
pub(crate) fn new(ref_change_sender: Sender<RefChange>) -> Self {
Assets {
assets: HashMap::default(),
change_ticks: HashMap::default(),
events: Events::default(),
ref_change_sender,
}
Expand Down Expand Up @@ -239,13 +241,30 @@ impl<T: Asset> Assets<T> {
}

pub fn asset_event_system(
mut events: EventWriter<AssetEvent<T>>,
ticks: SystemChangeTick,
mut event_writer: EventWriter<AssetEvent<T>>,
mut assets: ResMut<Assets<T>>,
) {
// Check if the events are empty before calling `drain`.
// As `drain` triggers change detection.
if !assets.events.is_empty() {
events.send_batch(assets.events.drain());
let current_tick = ticks.change_tick();
let Assets {
change_ticks,
events,
..
} = &mut *assets;
let update_change_map = |event| {
use AssetEvent::{Created, Modified, Removed};
match &event {
Removed { handle } => change_ticks.remove(&handle.id),
Created { handle } | Modified { handle } => {
change_ticks.insert(handle.id, current_tick)
}
};
event
};
event_writer.send_batch(events.drain().map(update_change_map));
}
}

Expand Down
3 changes: 3 additions & 0 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ mod info;
mod io;
mod loader;
mod path;
pub mod query;

pub mod prelude {
#[doc(hidden)]
pub use crate::query::AssetChanged;
#[doc(hidden)]
pub use crate::{AddAsset, AssetEvent, AssetServer, Assets, Handle, HandleUntyped};
}
Expand Down
166 changes: 166 additions & 0 deletions crates/bevy_asset/src/query.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
//! Filter query to limit iteration over [`Handle`]s which content was updated recently.
use std::marker::PhantomData;

use bevy_ecs::{
archetype::{Archetype, ArchetypeComponentId},
change_detection::MAX_CHANGE_AGE,
component::ComponentId,
prelude::World,
query::{
Access, ComponentIdState, Fetch, FetchState, FilteredAccess, QueryItem, ReadFetch,
ReadOnlyWorldQuery, WorldQuery, WorldQueryGats,
},
storage::{Table, Tables},
};
use bevy_utils::HashMap;

use crate::{Asset, Assets, Handle, HandleId};

struct AssetChangeCheck<'w> {
handle_change_map: Option<&'w HashMap<HandleId, u32>>,
prev_system_tick: u32,
current_tick: u32,
}
impl<'w> AssetChangeCheck<'w> {
fn new<T: Asset>(assets: Option<&'w Assets<T>>, last_change: u32, current: u32) -> Self {
let handle_change_map = assets.map(|a| &a.change_ticks);
Self {
handle_change_map,
prev_system_tick: last_change,
current_tick: current,
}
}
fn ticks_since(&self, event_tick: u32) -> u32 {
self.current_tick
.wrapping_sub(event_tick)
.min(MAX_CHANGE_AGE)
}
// TODO: some sort of caching? Each check has two levels of indirection,
// which is not optimal.
fn has_changed<T: Asset>(&self, handle: &Handle<T>) -> bool {
let is_changed = || {
let map = self.handle_change_map?;
let handle_change_tick = *map.get(&handle.id)?;
// This replicates the logic used in `ComponentTicks::is_changed`
let ticks_since_change = self.ticks_since(handle_change_tick);
let ticks_since_system = self.ticks_since(self.prev_system_tick);
Some(ticks_since_system >= ticks_since_change)
};
is_changed().unwrap_or(true)
}
}

/// Query filter to get all entities which `Handle<T>` component underlying asset changed.
pub struct AssetChanged<T>(PhantomData<T>);

/// Fetch for [`AssetChanged`]
#[doc(hidden)]
pub struct AssetChangedFetch<'w, T: Asset> {
inner: ReadFetch<'w, Handle<T>>,
check: AssetChangeCheck<'w>,
}

/// State for [`AssetChanged`]
pub struct AssetChangedState<T: Asset> {
inner: ComponentIdState<Handle<T>>,
}

// SAFETY: `ROQueryFetch<Self>` is the same as `QueryFetch<Self>`
unsafe impl<T: Asset> WorldQuery for AssetChanged<T> {
type ReadOnly = Self;
type State = AssetChangedState<T>;

fn shrink<'wlong: 'wshort, 'wshort>(item: QueryItem<'wlong, Self>) -> QueryItem<'wshort, Self> {
item
}
}

impl<T: Asset> FetchState for AssetChangedState<T> {
fn init(world: &mut World) -> Self {
Self {
inner: ComponentIdState::init(world),
}
}

fn matches_component_set(&self, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool {
self.inner.matches_component_set(set_contains_id)
}
}

impl<'w, T: Asset> WorldQueryGats<'w> for AssetChanged<T> {
type Fetch = AssetChangedFetch<'w, T>;
type _State = AssetChangedState<T>;
}

// SAFETY: this reads the T component. archetype component access and component access are updated to reflect that
unsafe impl<'w, T: Asset> Fetch<'w> for AssetChangedFetch<'w, T> {
type State = AssetChangedState<T>;
type Item = bool;

unsafe fn init(
world: &'w World,
state: &AssetChangedState<T>,
last_change_tick: u32,
change_tick: u32,
) -> Self {
Self {
inner: ReadFetch::init(world, &state.inner, last_change_tick, change_tick),
check: AssetChangeCheck::new::<T>(world.get_resource(), last_change_tick, change_tick),
}
}

const IS_DENSE: bool = <ReadFetch<'w, Handle<T>>>::IS_DENSE;

// This Fetch filters more than just the archetype.
const IS_ARCHETYPAL: bool = false;

unsafe fn set_table(&mut self, state: &Self::State, table: &'w Table) {
self.inner.set_table(&state.inner, table);
}

unsafe fn set_archetype(
&mut self,
state: &Self::State,
archetype: &'w Archetype,
tables: &'w Tables,
) {
self.inner.set_archetype(&state.inner, archetype, tables);
}

unsafe fn table_fetch(&mut self, table_row: usize) -> Self::Item {
let handle = self.inner.table_fetch(table_row);
self.check.has_changed(handle)
}

unsafe fn archetype_fetch(&mut self, archetype_index: usize) -> Self::Item {
let handle = self.inner.archetype_fetch(archetype_index);
self.check.has_changed(handle)
}

#[inline]
unsafe fn table_filter_fetch(&mut self, table_row: usize) -> bool {
self.table_fetch(table_row)
}

#[inline]
unsafe fn archetype_filter_fetch(&mut self, archetype_index: usize) -> bool {
self.archetype_fetch(archetype_index)
}

#[inline]
fn update_component_access(state: &Self::State, access: &mut FilteredAccess<ComponentId>) {
ReadFetch::<Handle<T>>::update_component_access(&state.inner, access);
}

#[inline]
fn update_archetype_component_access(
state: &Self::State,
archetype: &Archetype,
access: &mut Access<ArchetypeComponentId>,
) {
ReadFetch::<Handle<T>>::update_archetype_component_access(&state.inner, archetype, access);
}
}

/// SAFETY: read-only access
unsafe impl<T: Asset> ReadOnlyWorldQuery for AssetChanged<T> {}
7 changes: 7 additions & 0 deletions crates/bevy_ecs/src/change_detection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,13 @@ change_detection_impl!(NonSendMut<'a, T>, T,);
impl_into_inner!(NonSendMut<'a, T>, T,);
impl_debug!(NonSendMut<'a, T>,);

// Note on how change detection works.
//
// The `World` maintains a `Table` for each `StorageType::Table` component,
// When accessing a `Mut` from a query, a somewhat mutable reference to the
// tick stored in the table is passed to the `Mut`'s `ticks.component_ticks`
// field, going from a `&UnsafeCell<ComponentTicks>` to a `&mut ComponentTicks`
// using unsafe (in a context where the reference is guaranteed to be unique).
/// Unique mutable borrow of an entity's component
pub struct Mut<'a, T> {
pub(crate) value: &'a mut T,
Expand Down
7 changes: 7 additions & 0 deletions examples/asset/hot_asset_reloading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,17 @@ fn main() {
..default()
})
.add_plugins(DefaultPlugins)
.add_system(asset_updated)
.add_startup_system(setup)
.run();
}

fn asset_updated(query: Query<&Name, AssetChanged<Mesh>>) {
for name in query.iter() {
println!("{name} mesh got updated");
}
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
// Load our mesh:
let scene_handle = asset_server.load("models/monkey/Monkey.gltf#Scene0");
Expand Down

0 comments on commit a88c07b

Please sign in to comment.