Fix infinite asset preparation due to undrained AssetEvent events (#11383)
# Objective
After #10520, I was experiencing seriously degraded performance that
ended up being due to never-drained `AssetEvent` events causing havoc
inside `extract_render_asset::<A>`. The same events being read over and
over again meant the same assets were being prepared every frame for
eternity. For what it's worth, I was noticing this on a static scene
about every 3rd or so time running my project.
* References #10520
* Fixes #11240
Why these events aren't sometimes drained between frames is beyond me
and perhaps worthy of another investigation, but the approach in this PR
effectively restores the original cached `EventReader` behavior (which
fixes it).
## Solution
I followed the [`CachedSystemState`
example](3a666cab23/crates/bevy_ecs/src/system/function_system.rs (L155))
to make sure that the `EventReader` state is cached between frames like
it used to be when it was an argument of `extract_render_asset::<A>`.
This commit is contained in:
parent
d66c868e6f
commit
c227fc9fad
@ -5,6 +5,7 @@ use bevy_ecs::{
|
|||||||
prelude::{Commands, EventReader, IntoSystemConfigs, ResMut, Resource},
|
prelude::{Commands, EventReader, IntoSystemConfigs, ResMut, Resource},
|
||||||
schedule::SystemConfigs,
|
schedule::SystemConfigs,
|
||||||
system::{StaticSystemParam, SystemParam, SystemParamItem, SystemState},
|
system::{StaticSystemParam, SystemParam, SystemParamItem, SystemState},
|
||||||
|
world::{FromWorld, Mut},
|
||||||
};
|
};
|
||||||
use bevy_reflect::Reflect;
|
use bevy_reflect::Reflect;
|
||||||
use bevy_utils::{thiserror::Error, HashMap, HashSet};
|
use bevy_utils::{thiserror::Error, HashMap, HashSet};
|
||||||
@ -88,6 +89,7 @@ impl<A: RenderAsset, AFTER: RenderAssetDependency + 'static> Plugin
|
|||||||
for RenderAssetPlugin<A, AFTER>
|
for RenderAssetPlugin<A, AFTER>
|
||||||
{
|
{
|
||||||
fn build(&self, app: &mut App) {
|
fn build(&self, app: &mut App) {
|
||||||
|
app.init_resource::<CachedExtractRenderAssetSystemState<A>>();
|
||||||
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
|
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
|
||||||
render_app
|
render_app
|
||||||
.init_resource::<ExtractedAssets<A>>()
|
.init_resource::<ExtractedAssets<A>>()
|
||||||
@ -176,49 +178,69 @@ impl<A: RenderAsset> RenderAssets<A> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Resource)]
|
||||||
|
struct CachedExtractRenderAssetSystemState<A: RenderAsset> {
|
||||||
|
state: SystemState<(
|
||||||
|
EventReader<'static, 'static, AssetEvent<A>>,
|
||||||
|
ResMut<'static, Assets<A>>,
|
||||||
|
)>,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl<A: RenderAsset> FromWorld for CachedExtractRenderAssetSystemState<A> {
|
||||||
|
fn from_world(world: &mut bevy_ecs::world::World) -> Self {
|
||||||
|
Self {
|
||||||
|
state: SystemState::new(world),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// This system extracts all created or modified assets of the corresponding [`RenderAsset`] type
|
/// This system extracts all created or modified assets of the corresponding [`RenderAsset`] type
|
||||||
/// into the "render world".
|
/// into the "render world".
|
||||||
fn extract_render_asset<A: RenderAsset>(mut commands: Commands, mut main_world: ResMut<MainWorld>) {
|
fn extract_render_asset<A: RenderAsset>(mut commands: Commands, mut main_world: ResMut<MainWorld>) {
|
||||||
let mut system_state: SystemState<(EventReader<AssetEvent<A>>, ResMut<Assets<A>>)> =
|
main_world.resource_scope(
|
||||||
SystemState::new(&mut main_world);
|
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
|
||||||
let (mut events, mut assets) = system_state.get_mut(&mut main_world);
|
let (mut events, mut assets) = cached_state.state.get_mut(world);
|
||||||
|
|
||||||
let mut changed_assets = HashSet::default();
|
let mut changed_assets = HashSet::default();
|
||||||
let mut removed = Vec::new();
|
let mut removed = Vec::new();
|
||||||
for event in events.read() {
|
|
||||||
#[allow(clippy::match_same_arms)]
|
|
||||||
match event {
|
|
||||||
AssetEvent::Added { id } | AssetEvent::Modified { id } => {
|
|
||||||
changed_assets.insert(*id);
|
|
||||||
}
|
|
||||||
AssetEvent::Removed { .. } => {}
|
|
||||||
AssetEvent::Unused { id } => {
|
|
||||||
changed_assets.remove(id);
|
|
||||||
removed.push(*id);
|
|
||||||
}
|
|
||||||
AssetEvent::LoadedWithDependencies { .. } => {
|
|
||||||
// TODO: handle this
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
let mut extracted_assets = Vec::new();
|
for event in events.read() {
|
||||||
for id in changed_assets.drain() {
|
#[allow(clippy::match_same_arms)]
|
||||||
if let Some(asset) = assets.get(id) {
|
match event {
|
||||||
if asset.persistence_policy() == RenderAssetPersistencePolicy::Unload {
|
AssetEvent::Added { id } | AssetEvent::Modified { id } => {
|
||||||
if let Some(asset) = assets.remove(id) {
|
changed_assets.insert(*id);
|
||||||
extracted_assets.push((id, asset));
|
}
|
||||||
|
AssetEvent::Removed { .. } => {}
|
||||||
|
AssetEvent::Unused { id } => {
|
||||||
|
changed_assets.remove(id);
|
||||||
|
removed.push(*id);
|
||||||
|
}
|
||||||
|
AssetEvent::LoadedWithDependencies { .. } => {
|
||||||
|
// TODO: handle this
|
||||||
|
}
|
||||||
}
|
}
|
||||||
} else {
|
|
||||||
extracted_assets.push((id, asset.clone()));
|
|
||||||
}
|
}
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
commands.insert_resource(ExtractedAssets {
|
let mut extracted_assets = Vec::new();
|
||||||
extracted: extracted_assets,
|
for id in changed_assets.drain() {
|
||||||
removed,
|
if let Some(asset) = assets.get(id) {
|
||||||
});
|
if asset.persistence_policy() == RenderAssetPersistencePolicy::Unload {
|
||||||
|
if let Some(asset) = assets.remove(id) {
|
||||||
|
extracted_assets.push((id, asset));
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
extracted_assets.push((id, asset.clone()));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
commands.insert_resource(ExtractedAssets {
|
||||||
|
extracted: extracted_assets,
|
||||||
|
removed,
|
||||||
|
});
|
||||||
|
cached_state.state.apply(world);
|
||||||
|
},
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO: consider storing inside system?
|
// TODO: consider storing inside system?
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user