From 4caaee50f21c7cda4d40aa2884dd2db95edafd33 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 13 Aug 2023 02:03:24 +0100 Subject: [PATCH 1/4] preregister loader --- crates/bevy_asset/Cargo.toml | 1 + crates/bevy_asset/src/asset_server.rs | 115 ++++++++++++++++++++++---- crates/bevy_asset/src/assets.rs | 11 +++ crates/bevy_gltf/src/lib.rs | 3 +- 4 files changed, 112 insertions(+), 18 deletions(-) diff --git a/crates/bevy_asset/Cargo.toml b/crates/bevy_asset/Cargo.toml index 59c2b48aa5866..6f7d0886f3489 100644 --- a/crates/bevy_asset/Cargo.toml +++ b/crates/bevy_asset/Cargo.toml @@ -32,6 +32,7 @@ downcast-rs = "1.2.0" fastrand = "1.7.0" notify = { version = "6.0.0", optional = true } parking_lot = "0.12.1" +async-channel = "1.4.2" [target.'cfg(target_os = "android")'.dependencies] bevy_winit = { path = "../bevy_winit", version = "0.12.0-dev" } diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index d97f206bbdc36..1d68b2b9eea2d 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -62,6 +62,15 @@ pub(crate) struct AssetRefCounter { pub(crate) mark_unused_assets: Arc>>, } +#[derive(Clone)] +enum MaybeAssetLoader { + Ready(Arc), + Pending { + sender: async_channel::Sender<()>, + receiver: async_channel::Receiver<()>, + }, +} + /// Internal data for the asset server. /// /// [`AssetServer`] is the public API for interacting with the asset server. @@ -70,7 +79,7 @@ pub struct AssetServerInternal { pub(crate) asset_ref_counter: AssetRefCounter, pub(crate) asset_sources: Arc>>, pub(crate) asset_lifecycles: Arc>>>, - loaders: RwLock>>, + loaders: RwLock>, extension_to_loader_index: RwLock>, handle_to_path: Arc>>>, } @@ -157,6 +166,23 @@ impl AssetServer { Assets::new(self.server.asset_ref_counter.channel.sender.clone()) } + /// Pre-register a loader that will later be added. + /// + /// Assets loaded with matching extensions will be blocked until the + /// real loader is added. + pub fn preregister_loader(&self, extensions: &[&str]) { + let mut loaders = self.server.loaders.write(); + let loader_index = loaders.len(); + for extension in extensions { + self.server + .extension_to_loader_index + .write() + .insert(extension.to_string(), loader_index); + } + let (sender, receiver) = async_channel::bounded(1); + loaders.push(MaybeAssetLoader::Pending { sender, receiver }); + } + /// Adds the provided asset loader to the server. /// /// If `loader` has one or more supported extensions in conflict with loaders that came before @@ -167,13 +193,44 @@ impl AssetServer { { let mut loaders = self.server.loaders.write(); let loader_index = loaders.len(); + let mut maybe_existing_loader_index = None; + let mut loader_map = self.server.extension_to_loader_index.write(); + let mut senders = Vec::default(); for extension in loader.extensions() { - self.server - .extension_to_loader_index - .write() - .insert(extension.to_string(), loader_index); + if let Some(&extension_index) = loader_map.get(*extension) { + if extension_index != loader_index { + // replacing an existing loader + match maybe_existing_loader_index { + None => { + match &loaders[extension_index] { + MaybeAssetLoader::Ready(_) => (), + MaybeAssetLoader::Pending { sender, .. } => { + // the loader was pre-registered, store the channel to notify pending assets + senders.push(sender.clone()); + } + } + } + Some(index) => { + // ensure the loader extensions are consistent + assert_eq!(index, extension_index); + } + } + + maybe_existing_loader_index = Some(extension_index); + } + } else { + loader_map.insert(extension.to_string(), loader_index); + } + } + if let Some(existing_index) = maybe_existing_loader_index { + loaders[existing_index] = MaybeAssetLoader::Ready(Arc::new(loader)); + for sender in senders { + // notify after replacing the loader + sender.send_blocking(()).unwrap(); + } + } else { + loaders.push(MaybeAssetLoader::Ready(Arc::new(loader))); } - loaders.push(Arc::new(loader)); } /// Gets a strong handle for an asset with the provided id. @@ -188,7 +245,7 @@ impl AssetServer { HandleUntyped::strong(id.into(), sender) } - fn get_asset_loader(&self, extension: &str) -> Result, AssetServerError> { + fn get_asset_loader(&self, extension: &str) -> Result { let index = { // scope map to drop lock as soon as possible let map = self.server.extension_to_loader_index.read(); @@ -204,7 +261,7 @@ impl AssetServer { fn get_path_asset_loader>( &self, path: P, - ) -> Result, AssetServerError> { + ) -> Result { let s = path .as_ref() .file_name() @@ -354,7 +411,7 @@ impl AssetServer { }; // get the according asset loader - let asset_loader = match self.get_path_asset_loader(asset_path.path()) { + let mut maybe_asset_loader = match self.get_path_asset_loader(asset_path.path()) { Ok(loader) => loader, Err(err) => { set_asset_failed(); @@ -362,6 +419,21 @@ impl AssetServer { } }; + while let MaybeAssetLoader::Pending { receiver, .. } = maybe_asset_loader { + println!("blocking asset"); + let _ = receiver.recv().await; + println!("unblocked"); + maybe_asset_loader = match self.get_path_asset_loader(asset_path.path()) { + Ok(loader) => loader, + Err(err) => { + set_asset_failed(); + return Err(err); + } + }; + } + + let MaybeAssetLoader::Ready(asset_loader) = maybe_asset_loader else { panic!() }; + // load the asset bytes let bytes = match self.asset_io().load_path(asset_path.path()).await { Ok(bytes) => bytes, @@ -711,8 +783,11 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let t = asset_server.get_path_asset_loader("test.png"); - assert_eq!(t.unwrap().extensions()[0], "png"); + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.png") else { + panic!(); + }; + + assert_eq!(t.extensions()[0], "png"); } #[test] @@ -720,8 +795,10 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let t = asset_server.get_path_asset_loader("test.PNG"); - assert_eq!(t.unwrap().extensions()[0], "png"); + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.PNG") else { + panic!(); + }; + assert_eq!(t.extensions()[0], "png"); } #[test] @@ -771,8 +848,10 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let t = asset_server.get_path_asset_loader("test-v1.2.3.png"); - assert_eq!(t.unwrap().extensions()[0], "png"); + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test-v1.2.3.png") else { + panic!(); + }; + assert_eq!(t.extensions()[0], "png"); } #[test] @@ -780,8 +859,10 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakeMultipleDotLoader); - let t = asset_server.get_path_asset_loader("test.test.png"); - assert_eq!(t.unwrap().extensions()[0], "test.png"); + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.test.png") else { + panic!(); + }; + assert_eq!(t.extensions()[0], "test.png"); } fn create_dir_and_file(file: impl AsRef) -> tempfile::TempDir { diff --git a/crates/bevy_asset/src/assets.rs b/crates/bevy_asset/src/assets.rs index 6669d9bc59632..8a91fd701ad8b 100644 --- a/crates/bevy_asset/src/assets.rs +++ b/crates/bevy_asset/src/assets.rs @@ -317,6 +317,10 @@ pub trait AddAsset { fn add_asset_loader(&mut self, loader: T) -> &mut Self where T: AssetLoader; + + /// Preregisters a loader for the given extensions, that will block asset loads until a real loader + /// is registered. + fn preregister_asset_loader(&mut self, extensions: &[&str]) -> &mut Self; } impl AddAsset for App { @@ -404,6 +408,13 @@ impl AddAsset for App { self.world.resource_mut::().add_loader(loader); self } + + fn preregister_asset_loader(&mut self, extensions: &[&str]) -> &mut Self { + self.world + .resource_mut::() + .preregister_loader(extensions); + self + } } /// Loads an internal asset from a project source file. diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs index 90f8443aabc36..bd77ed2deba02 100644 --- a/crates/bevy_gltf/src/lib.rs +++ b/crates/bevy_gltf/src/lib.rs @@ -44,7 +44,8 @@ impl Plugin for GltfPlugin { .add_asset::() .add_asset::() .add_asset::() - .add_asset::(); + .add_asset::() + .preregister_asset_loader(&["gltf", "glb"]); } fn finish(&self, app: &mut App) { From 75e75c8e1b0277b3722f621275b9c19133237a39 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 13 Aug 2023 02:22:29 +0100 Subject: [PATCH 2/4] just block on the channel once --- crates/bevy_asset/src/asset_server.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 1d68b2b9eea2d..452807769f294 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -11,7 +11,7 @@ use bevy_tasks::IoTaskPool; use bevy_utils::{Entry, HashMap, Uuid}; use crossbeam_channel::TryRecvError; use parking_lot::{Mutex, RwLock}; -use std::{path::Path, sync::Arc}; +use std::{ffi::OsStr, path::Path, sync::Arc}; use thiserror::Error; /// Errors that occur while loading assets with an [`AssetServer`]. @@ -226,7 +226,7 @@ impl AssetServer { loaders[existing_index] = MaybeAssetLoader::Ready(Arc::new(loader)); for sender in senders { // notify after replacing the loader - sender.send_blocking(()).unwrap(); + let _ = sender.send_blocking(()); } } else { loaders.push(MaybeAssetLoader::Ready(Arc::new(loader))); @@ -419,10 +419,8 @@ impl AssetServer { } }; - while let MaybeAssetLoader::Pending { receiver, .. } = maybe_asset_loader { - println!("blocking asset"); + if let MaybeAssetLoader::Pending { receiver, .. } = maybe_asset_loader { let _ = receiver.recv().await; - println!("unblocked"); maybe_asset_loader = match self.get_path_asset_loader(asset_path.path()) { Ok(loader) => loader, Err(err) => { @@ -432,7 +430,12 @@ impl AssetServer { }; } - let MaybeAssetLoader::Ready(asset_loader) = maybe_asset_loader else { panic!() }; + let MaybeAssetLoader::Ready(asset_loader) = maybe_asset_loader else { + set_asset_failed(); + return Err(AssetServerError::MissingAssetLoader { + extensions: vec![asset_path.path().extension().and_then(OsStr::to_str).map(ToOwned::to_owned)].into_iter().flatten().collect(), + }); + }; // load the asset bytes let bytes = match self.asset_io().load_path(asset_path.path()).await { From 27ed531d3882f2f6a7346dfcaff340d7fcf84fe2 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 13 Aug 2023 23:26:11 +0100 Subject: [PATCH 3/4] preregister image loader --- .../bevy_render/src/texture/image_texture_loader.rs | 4 ++-- crates/bevy_render/src/texture/mod.rs | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/crates/bevy_render/src/texture/image_texture_loader.rs b/crates/bevy_render/src/texture/image_texture_loader.rs index 247d65e5a279f..da5845159b09a 100644 --- a/crates/bevy_render/src/texture/image_texture_loader.rs +++ b/crates/bevy_render/src/texture/image_texture_loader.rs @@ -17,7 +17,7 @@ pub struct ImageTextureLoader { supported_compressed_formats: CompressedImageFormats, } -const FILE_EXTENSIONS: &[&str] = &[ +pub(crate) const IMG_FILE_EXTENSIONS: &[&str] = &[ #[cfg(feature = "basis-universal")] "basis", #[cfg(feature = "bmp")] @@ -73,7 +73,7 @@ impl AssetLoader for ImageTextureLoader { } fn extensions(&self) -> &[&str] { - FILE_EXTENSIONS + IMG_FILE_EXTENSIONS } } diff --git a/crates/bevy_render/src/texture/mod.rs b/crates/bevy_render/src/texture/mod.rs index 25a65e5198a6f..350e31338889f 100644 --- a/crates/bevy_render/src/texture/mod.rs +++ b/crates/bevy_render/src/texture/mod.rs @@ -96,6 +96,17 @@ impl Plugin for ImagePlugin { update_texture_cache_system.in_set(RenderSet::Cleanup), ); } + + #[cfg(any( + feature = "png", + feature = "dds", + feature = "tga", + feature = "jpeg", + feature = "bmp", + feature = "basis-universal", + feature = "ktx2", + ))] + app.preregister_asset_loader(IMG_FILE_EXTENSIONS); } fn finish(&self, app: &mut App) { From dcf1ce0f17d7410d19d240586186ff72b38ea482 Mon Sep 17 00:00:00 2001 From: robtfm <50659922+robtfm@users.noreply.github.com> Date: Sun, 13 Aug 2023 23:56:59 +0100 Subject: [PATCH 4/4] clean up --- crates/bevy_asset/src/asset_server.rs | 104 +++++++++++++------------- 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/crates/bevy_asset/src/asset_server.rs b/crates/bevy_asset/src/asset_server.rs index 452807769f294..2217c80c993fd 100644 --- a/crates/bevy_asset/src/asset_server.rs +++ b/crates/bevy_asset/src/asset_server.rs @@ -11,7 +11,7 @@ use bevy_tasks::IoTaskPool; use bevy_utils::{Entry, HashMap, Uuid}; use crossbeam_channel::TryRecvError; use parking_lot::{Mutex, RwLock}; -use std::{ffi::OsStr, path::Path, sync::Arc}; +use std::{path::Path, sync::Arc}; use thiserror::Error; /// Errors that occur while loading assets with an [`AssetServer`]. @@ -174,10 +174,15 @@ impl AssetServer { let mut loaders = self.server.loaders.write(); let loader_index = loaders.len(); for extension in extensions { - self.server + if self + .server .extension_to_loader_index .write() - .insert(extension.to_string(), loader_index); + .insert(extension.to_string(), loader_index) + .is_some() + { + warn!("duplicate preregistration for `{extension}`, any assets loaded with the previous loader will never complete."); + } } let (sender, receiver) = async_channel::bounded(1); loaders.push(MaybeAssetLoader::Pending { sender, receiver }); @@ -192,39 +197,44 @@ impl AssetServer { T: AssetLoader, { let mut loaders = self.server.loaders.write(); - let loader_index = loaders.len(); + let next_loader_index = loaders.len(); let mut maybe_existing_loader_index = None; let mut loader_map = self.server.extension_to_loader_index.write(); - let mut senders = Vec::default(); + let mut maybe_sender = None; + for extension in loader.extensions() { if let Some(&extension_index) = loader_map.get(*extension) { - if extension_index != loader_index { - // replacing an existing loader - match maybe_existing_loader_index { - None => { - match &loaders[extension_index] { - MaybeAssetLoader::Ready(_) => (), - MaybeAssetLoader::Pending { sender, .. } => { - // the loader was pre-registered, store the channel to notify pending assets - senders.push(sender.clone()); - } + // replacing an existing entry + match maybe_existing_loader_index { + None => { + match &loaders[extension_index] { + MaybeAssetLoader::Ready(_) => { + // replacing an existing loader, nothing special to do + } + MaybeAssetLoader::Pending { sender, .. } => { + // the loader was pre-registered, store the channel to notify pending assets + maybe_sender = Some(sender.clone()); } } - Some(index) => { - // ensure the loader extensions are consistent - assert_eq!(index, extension_index); + } + Some(index) => { + // ensure the loader extensions are consistent + if index != extension_index { + warn!("inconsistent extensions between loader preregister_loader and add_loader, \ + loading `{extension}` assets will never complete."); } } - - maybe_existing_loader_index = Some(extension_index); } + + maybe_existing_loader_index = Some(extension_index); } else { - loader_map.insert(extension.to_string(), loader_index); + loader_map.insert(extension.to_string(), next_loader_index); } } + if let Some(existing_index) = maybe_existing_loader_index { loaders[existing_index] = MaybeAssetLoader::Ready(Arc::new(loader)); - for sender in senders { + if let Some(sender) = maybe_sender { // notify after replacing the loader let _ = sender.send_blocking(()); } @@ -261,6 +271,7 @@ impl AssetServer { fn get_path_asset_loader>( &self, path: P, + include_pending: bool, ) -> Result { let s = path .as_ref() @@ -280,7 +291,9 @@ impl AssetServer { ext = &ext[idx + 1..]; exts.push(ext); if let Ok(loader) = self.get_asset_loader(ext) { - return Ok(loader); + if include_pending || matches!(loader, MaybeAssetLoader::Ready(_)) { + return Ok(loader); + } } } Err(AssetServerError::MissingAssetLoader { @@ -411,30 +424,21 @@ impl AssetServer { }; // get the according asset loader - let mut maybe_asset_loader = match self.get_path_asset_loader(asset_path.path()) { - Ok(loader) => loader, - Err(err) => { - set_asset_failed(); - return Err(err); - } - }; + let mut maybe_asset_loader = self.get_path_asset_loader(asset_path.path(), true); - if let MaybeAssetLoader::Pending { receiver, .. } = maybe_asset_loader { + // if it's still pending, block until notified and refetch the new asset loader + if let Ok(MaybeAssetLoader::Pending { receiver, .. }) = maybe_asset_loader { let _ = receiver.recv().await; - maybe_asset_loader = match self.get_path_asset_loader(asset_path.path()) { - Ok(loader) => loader, - Err(err) => { - set_asset_failed(); - return Err(err); - } - }; + maybe_asset_loader = self.get_path_asset_loader(asset_path.path(), false); } - let MaybeAssetLoader::Ready(asset_loader) = maybe_asset_loader else { - set_asset_failed(); - return Err(AssetServerError::MissingAssetLoader { - extensions: vec![asset_path.path().extension().and_then(OsStr::to_str).map(ToOwned::to_owned)].into_iter().flatten().collect(), - }); + let asset_loader = match maybe_asset_loader { + Ok(MaybeAssetLoader::Ready(loader)) => loader, + Err(err) => { + set_asset_failed(); + return Err(err); + } + Ok(MaybeAssetLoader::Pending { .. }) => unreachable!(), }; // load the asset bytes @@ -567,7 +571,7 @@ impl AssetServer { if self.asset_io().is_dir(&child_path) { handles.extend(self.load_folder(&child_path)?); } else { - if self.get_path_asset_loader(&child_path).is_err() { + if self.get_path_asset_loader(&child_path, true).is_err() { continue; } let handle = @@ -786,7 +790,7 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.png") else { + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.png", true) else { panic!(); }; @@ -798,7 +802,7 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.PNG") else { + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.PNG", true) else { panic!(); }; assert_eq!(t.extensions()[0], "png"); @@ -807,7 +811,7 @@ mod test { #[test] fn no_loader() { let asset_server = setup("."); - let t = asset_server.get_path_asset_loader("test.pong"); + let t = asset_server.get_path_asset_loader("test.pong", true); assert!(t.is_err()); } @@ -816,7 +820,7 @@ mod test { let asset_server = setup("."); assert!( - match asset_server.get_path_asset_loader("test.v1.2.3.pong") { + match asset_server.get_path_asset_loader("test.v1.2.3.pong", true) { Err(AssetServerError::MissingAssetLoader { extensions }) => extensions == vec!["v1.2.3.pong", "2.3.pong", "3.pong", "pong"], _ => false, @@ -851,7 +855,7 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakePngLoader); - let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test-v1.2.3.png") else { + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test-v1.2.3.png", true) else { panic!(); }; assert_eq!(t.extensions()[0], "png"); @@ -862,7 +866,7 @@ mod test { let asset_server = setup("."); asset_server.add_loader(FakeMultipleDotLoader); - let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.test.png") else { + let Ok(MaybeAssetLoader::Ready(t)) = asset_server.get_path_asset_loader("test.test.png", true) else { panic!(); }; assert_eq!(t.extensions()[0], "test.png");