Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix infinite asset preparation due to undrained AssetEvent events #11383

Merged
merged 1 commit into from
Jan 27, 2024
Merged

Fix infinite asset preparation due to undrained AssetEvent events #11383

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 57 additions & 35 deletions crates/bevy_render/src/render_asset.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use bevy_ecs::{
prelude::{Commands, EventReader, IntoSystemConfigs, ResMut, Resource},
schedule::SystemConfigs,
system::{StaticSystemParam, SystemParam, SystemParamItem, SystemState},
world::{FromWorld, Mut},
};
use bevy_reflect::Reflect;
use bevy_utils::{thiserror::Error, HashMap, HashSet};
Expand Down Expand Up @@ -88,6 +89,7 @@ impl<A: RenderAsset, AFTER: RenderAssetDependency + 'static> Plugin
for RenderAssetPlugin<A, AFTER>
{
fn build(&self, app: &mut App) {
app.init_resource::<CachedExtractRenderAssetSystemState<A>>();
if let Ok(render_app) = app.get_sub_app_mut(RenderApp) {
render_app
.init_resource::<ExtractedAssets<A>>()
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit: import World directly instead of writing out the full path

state: SystemState::new(world),
}
}
}

/// This system extracts all created or modified assets of the corresponding [`RenderAsset`] type
/// into the "render world".
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>>)> =
SystemState::new(&mut main_world);
let (mut events, mut assets) = system_state.get_mut(&mut main_world);

let mut changed_assets = HashSet::default();
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
main_world.resource_scope(
|world, mut cached_state: Mut<CachedExtractRenderAssetSystemState<A>>| {
let (mut events, mut assets) = cached_state.state.get_mut(world);

let mut changed_assets = HashSet::default();
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 id in changed_assets.drain() {
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));
let mut extracted_assets = Vec::new();
for id in changed_assets.drain() {
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()));
}
}
} else {
extracted_assets.push((id, asset.clone()));
}
}
}

commands.insert_resource(ExtractedAssets {
extracted: extracted_assets,
removed,
});
commands.insert_resource(ExtractedAssets {
extracted: extracted_assets,
removed,
});
cached_state.state.apply(world);
},
);
}

// TODO: consider storing inside system?
Expand Down