forestia/bevy
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix GLTF scene dependencies and make full scene renders predictable (#10745)

Browse Source 
# Objective

Fixes #10688

There were a number of issues at play:

1. The GLTF loader was not registering Scene dependencies properly. They
were being registered at the root instead of on the scene assets. This
made `LoadedWithDependencies` fire immediately on load.
2. Recursive labeled assets _inside_ of labeled assets were not being
loaded. This only became relevant for scenes after fixing (1) because we
now add labeled assets to the nested scene `LoadContext` instead of the
root load context. I'm surprised nobody has hit this yet. I'm glad I
caught it before somebody hit it.
3. Accessing "loaded with dependencies" state on the Asset Server is
boilerplatey + error prone (because you need to manually query two
states).

## Solution

1. In GltfLoader, use a nested LoadContext for scenes and load
dependencies through that context.
2. In the `AssetServer`, load labeled assets recursively.
3. Added a simple `asset_server.is_loaded_with_dependencies(id)`

I also added some docs to `LoadContext` to help prevent this problem in
the future.

---

## Changelog

- Added `AssetServer::is_loaded_with_dependencies`
- Fixed GLTF Scene dependencies
- Fixed nested labeled assets not being loaded

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
This commit is contained in:
Carter Anderson 2023-11-27 14:42:28 -08:00 committed by GitHub
parent 6e871ab919
commit cc6c4d65ed
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 49 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
crates/bevy_asset/src/loader.rs
View File

@ -353,6 +353,13 @@ 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<A: Asset>(&mut self, label: String, asset: A) -> Handle<A> {
self.labeled_asset_scope(label, |_| asset)

4
crates/bevy_asset/src/server/info.rs
View File

@ -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)
}

40
crates/bevy_asset/src/server/mod.rs
View File

@ -482,7 +482,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(),
@ -498,17 +498,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) => {
@ -520,6 +510,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<AssetPath<'a>>) {
let server = self.clone();
@ -707,6 +707,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<UntypedAssetId>) -> Option<LoadState> {
self.data.infos.read().get(id.into()).map(|i| i.load_state)
}
@ -737,6 +740,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<UntypedAssetId>) -> 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<AssetPath<'a>>) -> Option<Handle<A>> {
@ -752,6 +762,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<UntypedAssetId>) -> 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<AssetPath<'a>>) -> Option<UntypedHandle> {

14
crates/bevy_gltf/src/loader.rs
View File

@ -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());
@ -853,9 +854,11 @@ fn load_material(
}
/// Loads a glTF node.
#[allow(clippy::too_many_arguments)]
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<usize, Entity>,
entity_to_skin_index_map: &mut HashMap<Entity, usize>,
@ -942,7 +945,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 +1079,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,