check root node for animations (#9407)
# Objective fixes #8357 gltf animations can affect multiple "root" nodes (i.e. top level nodes within a gltf scene). the current loader adds an AnimationPlayer to each root node which is affected by any animation. when a clip which affects multiple root nodes is played on a root node player, the root node name is not checked, leading to potentially incorrect weights being applied. also, the `AnimationClip::compatible_with` method will never return true for those clips, as it checks that all paths start with the root node name - not all paths start with the same name so it can't return true. ## Solution - check the first path node name matches the given root - change compatible_with to return true if `any` match is found a better alternative would probably be to attach the player to the scene root instead of the first child, and then walk the full path from there. this would be breaking (and would stop multiple animations that *don't* overlap from being played concurrently), but i'm happy to modify to that if it's preferred. --------- Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
This commit is contained in:
parent
b28f6334da
commit
25e5239d2e
@ -119,7 +119,7 @@ impl AnimationClip {
|
|||||||
|
|
||||||
/// Whether this animation clip can run on entity with given [`Name`].
|
/// Whether this animation clip can run on entity with given [`Name`].
|
||||||
pub fn compatible_with(&self, name: &Name) -> bool {
|
pub fn compatible_with(&self, name: &Name) -> bool {
|
||||||
self.paths.keys().all(|path| &path.parts[0] == name)
|
self.paths.keys().any(|path| &path.parts[0] == name)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -406,8 +406,18 @@ fn entity_from_path(
|
|||||||
// PERF: finding the target entity can be optimised
|
// PERF: finding the target entity can be optimised
|
||||||
let mut current_entity = root;
|
let mut current_entity = root;
|
||||||
path_cache.resize(path.parts.len(), None);
|
path_cache.resize(path.parts.len(), None);
|
||||||
// Ignore the first name, it is the root node which we already have
|
|
||||||
for (idx, part) in path.parts.iter().enumerate().skip(1) {
|
let mut parts = path.parts.iter().enumerate();
|
||||||
|
|
||||||
|
// check the first name is the root node which we already have
|
||||||
|
let Some((_, root_name)) = parts.next() else {
|
||||||
|
return None;
|
||||||
|
};
|
||||||
|
if names.get(current_entity) != Ok(root_name) {
|
||||||
|
return None;
|
||||||
|
}
|
||||||
|
|
||||||
|
for (idx, part) in parts {
|
||||||
let mut found = false;
|
let mut found = false;
|
||||||
let children = children.get(current_entity).ok()?;
|
let children = children.get(current_entity).ok()?;
|
||||||
if let Some(cached) = path_cache[idx] {
|
if let Some(cached) = path_cache[idx] {
|
||||||
@ -608,12 +618,14 @@ fn apply_animation(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let mut any_path_found = false;
|
||||||
for (path, bone_id) in &animation_clip.paths {
|
for (path, bone_id) in &animation_clip.paths {
|
||||||
let cached_path = &mut animation.path_cache[*bone_id];
|
let cached_path = &mut animation.path_cache[*bone_id];
|
||||||
let curves = animation_clip.get_curves(*bone_id).unwrap();
|
let curves = animation_clip.get_curves(*bone_id).unwrap();
|
||||||
let Some(target) = entity_from_path(root, path, children, names, cached_path) else {
|
let Some(target) = entity_from_path(root, path, children, names, cached_path) else {
|
||||||
continue;
|
continue;
|
||||||
};
|
};
|
||||||
|
any_path_found = true;
|
||||||
// SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
|
// SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
|
||||||
// any of their descendant Transforms.
|
// any of their descendant Transforms.
|
||||||
//
|
//
|
||||||
@ -702,6 +714,10 @@ fn apply_animation(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if !any_path_found {
|
||||||
|
warn!("Animation player on {root:?} did not match any entity paths.");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
Loading…
Reference in New Issue
Block a user