From 5a57dfc3471e53bf05d2f03891c86bc445ef150b Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sat, 25 Nov 2023 19:53:28 -0800 Subject: [PATCH 1/4] Fix GLTF scene dependencies and make full scene renders predictable --- crates/bevy_asset/src/loader.rs | 5 ++++ crates/bevy_asset/src/server/info.rs | 4 +++ crates/bevy_asset/src/server/mod.rs | 37 +++++++++++++++++++--------- crates/bevy_gltf/src/loader.rs | 13 +++++++--- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index 008312f200b70..c1b1337d7d59d 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -353,6 +353,11 @@ impl<'a> LoadContext<'a> { /// This will add the given `asset` as a "labeled [`Asset`]" with the `label` label. /// + /// Warning: this will not assign dependencies to the given `asset`. If adding an asset + /// with dependencies generated from calls such as [`LoadContext::load`], use + /// [`LoadContext::labeled_asset_scope`] or [`LoadContext::begin_labeled_asset`] to generate a + /// new [`LoadContext`] to track the dependencies for the labeled asset. + /// /// See [`AssetPath`] for more on labeled assets. pub fn add_labeled_asset(&mut self, label: String, asset: A) -> Handle { self.labeled_asset_scope(label, |_| asset) diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs index 2786a7d4b86b3..1297daec49ba8 100644 --- a/crates/bevy_asset/src/server/info.rs +++ b/crates/bevy_asset/src/server/info.rs @@ -260,6 +260,10 @@ impl AssetInfos { self.infos.get(&id) } + pub(crate) fn contains_key(&self, id: UntypedAssetId) -> bool { + self.infos.contains_key(&id) + } + pub(crate) fn get_mut(&mut self, id: UntypedAssetId) -> Option<&mut AssetInfo> { self.infos.get_mut(&id) } diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index 9ec46439e81e6..f38be5e876d20 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -456,7 +456,7 @@ impl AssetServer { .load_with_meta_loader_and_reader(&base_path, meta, &*loader, &mut *reader, true, false) .await { - Ok(mut loaded_asset) => { + Ok(loaded_asset) => { let final_handle = if let Some(label) = path.label_cow() { match loaded_asset.labeled_assets.get(&label) { Some(labeled_asset) => labeled_asset.handle.clone(), @@ -472,17 +472,7 @@ impl AssetServer { handle.unwrap() }; - for (_, labeled_asset) in loaded_asset.labeled_assets.drain() { - self.send_asset_event(InternalAssetEvent::Loaded { - id: labeled_asset.handle.id(), - loaded_asset: labeled_asset.asset, - }); - } - - self.send_asset_event(InternalAssetEvent::Loaded { - id: base_handle.id(), - loaded_asset, - }); + self.send_loaded_asset(base_handle.id(), loaded_asset); Ok(final_handle) } Err(err) => { @@ -494,6 +484,16 @@ impl AssetServer { } } + /// Sends a load event for the given `loaded_asset` and does the same recursively for all + /// labeled assets. + fn send_loaded_asset(&self, id: UntypedAssetId, mut loaded_asset: ErasedLoadedAsset) { + for (_, labeled_asset) in loaded_asset.labeled_assets.drain() { + self.send_loaded_asset(labeled_asset.handle.id(), labeled_asset.asset); + } + + self.send_asset_event(InternalAssetEvent::Loaded { id, loaded_asset }); + } + /// Kicks off a reload of the asset stored at the given path. This will only reload the asset if it currently loaded. pub fn reload<'a>(&self, path: impl Into>) { let server = self.clone(); @@ -711,6 +711,13 @@ impl AssetServer { .unwrap_or(RecursiveDependencyLoadState::NotLoaded) } + /// Returns true if the asset and all of its dependencies (recursive) have been loaded. + pub fn is_loaded_with_dependencies(&self, id: impl Into) -> bool { + let id = id.into(); + self.load_state(id) == LoadState::Loaded + && self.recursive_dependency_load_state(id) == RecursiveDependencyLoadState::Loaded + } + /// Returns an active handle for the given path, if the asset at the given path has already started loading, /// or is still "alive". pub fn get_handle<'a, A: Asset>(&self, path: impl Into>) -> Option> { @@ -726,6 +733,12 @@ impl AssetServer { self.data.infos.read().get_id_handle(id) } + /// Returns `true` if the given `id` corresponds to an asset that is managed by this [`AssetServer`]. + /// Otherwise, returns false. + pub fn is_managed(&self, id: impl Into) -> bool { + self.data.infos.read().contains_key(id.into()) + } + /// Returns an active untyped handle for the given path, if the asset at the given path has already started loading, /// or is still "alive". pub fn get_handle_untyped<'a>(&self, path: impl Into>) -> Option { diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index bc1f3c3615a33..5906889d8f27b 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -545,7 +545,7 @@ async fn load_gltf<'a, 'b, 'c>( let mut world = World::default(); let mut node_index_to_entity_map = HashMap::new(); let mut entity_to_skin_index_map = HashMap::new(); - + let mut scene_load_context = load_context.begin_labeled_asset(); world .spawn(SpatialBundle::INHERITED_IDENTITY) .with_children(|parent| { @@ -554,6 +554,7 @@ async fn load_gltf<'a, 'b, 'c>( &node, parent, load_context, + &mut scene_load_context, &mut node_index_to_entity_map, &mut entity_to_skin_index_map, &mut active_camera_found, @@ -606,8 +607,8 @@ async fn load_gltf<'a, 'b, 'c>( joints: joint_entities, }); } - - let scene_handle = load_context.add_labeled_asset(scene_label(&scene), Scene::new(world)); + let loaded_scene = scene_load_context.finish(Scene::new(world), None); + let scene_handle = load_context.add_loaded_labeled_asset(scene_label(&scene), loaded_scene); if let Some(name) = scene.name() { named_scenes.insert(name.to_string(), scene_handle.clone()); @@ -856,6 +857,7 @@ fn load_material( fn load_node( gltf_node: &gltf::Node, world_builder: &mut WorldChildBuilder, + root_load_context: &LoadContext, load_context: &mut LoadContext, node_index_to_entity_map: &mut HashMap, entity_to_skin_index_map: &mut HashMap, @@ -942,7 +944,9 @@ fn load_node( // added when iterating over all the gltf materials (since the default material is // not explicitly listed in the gltf). // It also ensures an inverted scale copy is instantiated if required. - if !load_context.has_labeled_asset(&material_label) { + if !root_load_context.has_labeled_asset(&material_label) + && !load_context.has_labeled_asset(&material_label) + { load_material(&material, load_context, is_scale_inverted); } @@ -1074,6 +1078,7 @@ fn load_node( if let Err(err) = load_node( &child, parent, + root_load_context, load_context, node_index_to_entity_map, entity_to_skin_index_map, From c45e10ed890134d9fd0462524e9d1b9a6eee2af3 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Sun, 26 Nov 2023 16:06:21 -0800 Subject: [PATCH 2/4] Update crates/bevy_asset/src/loader.rs Co-authored-by: Alice Cecile --- crates/bevy_asset/src/loader.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/bevy_asset/src/loader.rs b/crates/bevy_asset/src/loader.rs index c1b1337d7d59d..26f7ebc6a0f8f 100644 --- a/crates/bevy_asset/src/loader.rs +++ b/crates/bevy_asset/src/loader.rs @@ -353,7 +353,9 @@ impl<'a> LoadContext<'a> { /// This will add the given `asset` as a "labeled [`Asset`]" with the `label` label. /// - /// Warning: this will not assign dependencies to the given `asset`. If adding an asset + /// # Warning + /// + /// This will not assign dependencies to the given `asset`. If adding an asset /// with dependencies generated from calls such as [`LoadContext::load`], use /// [`LoadContext::labeled_asset_scope`] or [`LoadContext::begin_labeled_asset`] to generate a /// new [`LoadContext`] to track the dependencies for the labeled asset. From 43e526f59c0128d3266e9c4a1c2f116f33bcb5c2 Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 27 Nov 2023 14:29:08 -0800 Subject: [PATCH 3/4] Add doc link / mention of is_loaded_with_dependencies --- crates/bevy_asset/src/server/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/bevy_asset/src/server/mod.rs b/crates/bevy_asset/src/server/mod.rs index f38be5e876d20..44ee11bcd22ed 100644 --- a/crates/bevy_asset/src/server/mod.rs +++ b/crates/bevy_asset/src/server/mod.rs @@ -681,6 +681,9 @@ impl AssetServer { } /// Retrieves the main [`LoadState`] of a given asset `id`. + /// + /// Note that this is "just" the root asset load state. To check if an asset _and_ its recursive + /// dependencies have loaded, see [`AssetServer::is_loaded_with_dependencies`]. pub fn get_load_state(&self, id: impl Into) -> Option { self.data.infos.read().get(id.into()).map(|i| i.load_state) } From b257944991d177b690253552cd5fb7ae4536ed9e Mon Sep 17 00:00:00 2001 From: Carter Anderson Date: Mon, 27 Nov 2023 14:29:16 -0800 Subject: [PATCH 4/4] Clippy --- crates/bevy_gltf/src/loader.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/bevy_gltf/src/loader.rs b/crates/bevy_gltf/src/loader.rs index 5906889d8f27b..ce07805a4276a 100644 --- a/crates/bevy_gltf/src/loader.rs +++ b/crates/bevy_gltf/src/loader.rs @@ -854,6 +854,7 @@ fn load_material( } /// Loads a glTF node. +#[allow(clippy::too_many_arguments)] fn load_node( gltf_node: &gltf::Node, world_builder: &mut WorldChildBuilder,