From 340d03a92560eebb5d45dd368d24b3df658794e1 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 18 Aug 2024 18:57:35 +0100 Subject: [PATCH] drop pending loads --- crates/bevy_asset/src/server/info.rs | 7 +++ crates/bevy_asset/src/server/mod.rs | 71 +++++++++++++++------------- crates/bevy_gltf/Cargo.toml | 1 + crates/bevy_gltf/src/loader.rs | 42 ++++++++-------- 4 files changed, 68 insertions(+), 53 deletions(-) diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index b28de5efe88bd..012e47b76adef 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -5,6 +5,7 @@ use crate::{ UntypedAssetId, UntypedHandle, }; use bevy_ecs::world::World; +use bevy_tasks::Task; use bevy_utils::tracing::warn; use bevy_utils::{Entry, HashMap, HashSet, TypeIdMap}; use crossbeam_channel::Sender; @@ -76,6 +77,7 @@ pub(crate) struct AssetInfos { pub(crate) dependency_loaded_event_sender: TypeIdMap, pub(crate) dependency_failed_event_sender: TypeIdMap, AssetLoadError)>, + pub(crate) pending_tasks: HashMap>, } impl std::fmt::Debug for AssetInfos { @@ -364,6 +366,7 @@ impl AssetInfos { &mut self.path_to_id, &mut self.loader_dependants, &mut self.living_labeled_assets, + &mut self.pending_tasks, self.watching_for_changes, id, ) @@ -647,6 +650,7 @@ impl AssetInfos { path_to_id: &mut HashMap, TypeIdMap>, loader_dependants: &mut HashMap, HashSet>>, living_labeled_assets: &mut HashMap, HashSet>>, + pending_tasks: &mut HashMap>, watching_for_changes: bool, id: UntypedAssetId, ) -> bool { @@ -661,6 +665,8 @@ impl AssetInfos { return false; } + pending_tasks.remove(&id); + let type_id = entry.key().type_id(); let info = entry.remove(); @@ -703,6 +709,7 @@ impl AssetInfos { &mut self.path_to_id, &mut self.loader_dependants, &mut self.living_labeled_assets, + &mut self.pending_tasks, self.watching_for_changes, id.untyped(provider.type_id), ); diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 91253e32197b6..c83882ada108a 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -368,7 +368,8 @@ impl AssetServer { guard: G, ) -> Handle { let path = path.into().into_owned(); - let (handle, should_load) = self.data.infos.write().get_or_create_path_handle::( + let mut infos = self.data.infos.write(); + let (handle, should_load) = infos.get_or_create_path_handle::( path.clone(), HandleLoadingMode::Request, meta_transform, @@ -377,14 +378,15 @@ impl AssetServer { if should_load { let owned_handle = Some(handle.clone().untyped()); let server = self.clone(); - IoTaskPool::get() - .spawn(async move { + infos.pending_tasks.insert( + handle.id().untyped(), + IoTaskPool::get().spawn(async move { if let Err(err) = server.load_internal(owned_handle, path, false, None).await { error!("{}", err); } drop(guard); - }) - .detach(); + }), + ); } handle @@ -414,25 +416,24 @@ impl AssetServer { CowArc::Owned(format!("{source}--{UNTYPED_SOURCE_SUFFIX}").into()) } }); - let (handle, should_load) = self - .data - .infos - .write() - .get_or_create_path_handle::( - path.clone().with_source(untyped_source), - HandleLoadingMode::Request, - meta_transform, - ); + let mut infos = self.data.infos.write(); + let (handle, should_load) = infos.get_or_create_path_handle::( + path.clone().with_source(untyped_source), + HandleLoadingMode::Request, + meta_transform, + ); if !should_load { return handle; } let id = handle.id().untyped(); + let owned_handle = Some(handle.clone().untyped()); let server = self.clone(); - IoTaskPool::get() - .spawn(async move { + infos.pending_tasks.insert( + id, + IoTaskPool::get().spawn(async move { let path_clone = path.clone(); - match server.load_untyped_async(path).await { + match server.load_internal(owned_handle, path, false, None).await { Ok(handle) => server.send_asset_event(InternalAssetEvent::Loaded { id, loaded_asset: LoadedAsset::new_with_dependencies( @@ -450,8 +451,8 @@ impl AssetServer { }); } } - }) - .detach(); + }), + ); handle } @@ -488,7 +489,7 @@ impl AssetServer { /// avoid looking up `should_load` twice, but it means you _must_ be sure a load is necessary when calling this function with [`Some`]. async fn load_internal<'a>( &self, - input_handle: Option, + mut input_handle: Option, path: AssetPath<'a>, force: bool, meta_transform: Option, @@ -513,6 +514,13 @@ impl AssetServer { e })?; + if let Some(meta_transform) = input_handle.as_ref().and_then(|h| h.meta_transform()) { + (*meta_transform)(&mut *meta); + } + // downgrade the input handle so we don't keep the asset alive just because we're loading it + // note we can't just pass a weak handle in, as only strong handles contain the asset meta transform + input_handle = input_handle.map(|h| h.clone_weak()); + // This contains Some(UntypedHandle), if it was retrievable // If it is None, that is because it was _not_ retrievable, due to // 1. The handle was not already passed in for this path, meaning we can't just use that @@ -581,10 +589,6 @@ impl AssetServer { (handle.clone().unwrap(), path.clone()) }; - if let Some(meta_transform) = base_handle.meta_transform() { - (*meta_transform)(&mut *meta); - } - match self .load_with_meta_loader_and_reader(&base_path, meta, &*loader, &mut *reader, true, false) .await @@ -722,17 +726,16 @@ impl AssetServer { &self, future: impl Future> + Send + 'static, ) -> Handle { - let handle = self - .data - .infos - .write() + let mut infos = self.data.infos.write(); + let handle = infos .create_loading_handle_untyped(std::any::TypeId::of::(), std::any::type_name::()); let id = handle.id(); let event_sender = self.data.asset_event_sender.clone(); - IoTaskPool::get() - .spawn(async move { + infos.pending_tasks.insert( + id, + IoTaskPool::get().spawn(async move { match future.await { Ok(asset) => { let loaded_asset = LoadedAsset::new_with_dependencies(asset, None).into(); @@ -754,8 +757,8 @@ impl AssetServer { .unwrap(); } } - }) - .detach(); + }), + ); handle.typed_debug_checked() } @@ -1312,6 +1315,10 @@ pub fn handle_internal_asset_events(world: &mut World) { info!("Reloading {path} because it has changed"); server.reload(path); } + + infos + .pending_tasks + .retain(|_, load_task| !load_task.is_finished()); }); } diff --git a/crates/bevy_gltf/Cargo.toml b/crates/bevy_gltf/Cargo.toml index d6493e75adfb4..55b2d1793a480 100644 --- a/crates/bevy_gltf/Cargo.toml +++ b/crates/bevy_gltf/Cargo.toml @@ -58,6 +58,7 @@ base64 = "0.22.0" percent-encoding = "2.1" serde = { version = "1.0", features = ["derive"] } serde_json = "1" +futures = "0.3" smallvec = "1.11" [lints] diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 062af1a024668..7c8cb56931081 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -38,10 +38,10 @@ use bevy_render::{ }; use bevy_scene::Scene; #[cfg(not(target_arch = "wasm32"))] -use bevy_tasks::IoTaskPool; use bevy_transform::components::Transform; use bevy_utils::tracing::{error, info_span, warn}; use bevy_utils::{HashMap, HashSet}; +use futures::future::join_all; use gltf::image::Source; use gltf::{ accessor::Iter, @@ -389,25 +389,22 @@ async fn load_gltf<'a, 'b, 'c>( } } else { #[cfg(not(target_arch = "wasm32"))] - IoTaskPool::get() - .scope(|scope| { - gltf.textures().for_each(|gltf_texture| { - let parent_path = load_context.path().parent().unwrap(); - let linear_textures = &linear_textures; - let buffer_data = &buffer_data; - scope.spawn(async move { - load_image( - gltf_texture, - buffer_data, - linear_textures, - parent_path, - loader.supported_compressed_formats, - settings.load_materials, - ) - .await - }); - }); - }) + let futures = gltf.textures().map(|gltf_texture| { + let parent_path = load_context.path().parent().unwrap(); + let linear_textures = &linear_textures; + let buffer_data = &buffer_data; + load_image( + gltf_texture, + buffer_data, + linear_textures, + parent_path, + loader.supported_compressed_formats, + settings.load_materials, + ) + }); + + join_all(futures) + .await .into_iter() .for_each(|result| match result { Ok(image) => { @@ -1469,7 +1466,10 @@ fn load_node( } fn primitive_name(mesh: &gltf::Mesh, primitive: &Primitive) -> String { - let mesh_name = mesh.name().map(ToOwned::to_owned).unwrap_or_else(|| format!("Mesh{}", mesh.index())); + let mesh_name = mesh + .name() + .map(ToOwned::to_owned) + .unwrap_or_else(|| format!("Mesh{}", mesh.index())); if mesh.primitives().len() > 1 { format!("{}/Primitive{}", mesh_name, primitive.index()) } else {