From a17b3961f129a584671f694390a0dcc47e127065 Mon Sep 17 00:00:00 2001 From: Patrick Walton Date: Sun, 4 Feb 2024 16:51:46 -0800 Subject: [PATCH 01/27] Rework animation to be done in two phases. This eliminates all the unsafe code, increases parallelism, and reduces the number of queries to just two. --- crates/bevy_animation/Cargo.toml | 1 + crates/bevy_animation/src/lib.rs | 783 +++++++++++++++++-------------- 2 files changed, 437 insertions(+), 347 deletions(-) diff --git a/crates/bevy_animation/Cargo.toml b/crates/bevy_animation/Cargo.toml index f2d5da4cd7dbf..50f97e30d3116 100644 --- a/crates/bevy_animation/Cargo.toml +++ b/crates/bevy_animation/Cargo.toml @@ -13,6 +13,7 @@ keywords = ["bevy"] bevy_app = { path = "../bevy_app", version = "0.12.0" } bevy_asset = { path = "../bevy_asset", version = "0.12.0" } bevy_core = { path = "../bevy_core", version = "0.12.0" } +bevy_log = { path = "../bevy_log", version = "0.12.0" } bevy_math = { path = "../bevy_math", version = "0.12.0" } bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = [ "bevy", diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 5771e2e9da510..1b6875d58855c 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -3,20 +3,22 @@ mod animatable; mod util; -use std::ops::{Add, Deref, Mul}; +use std::ops::{Add, Mul}; use std::time::Duration; -use bevy_app::{App, Plugin, PostUpdate}; +use bevy_app::{App, Plugin, PostUpdate, Update}; use bevy_asset::{Asset, AssetApp, Assets, Handle}; use bevy_core::Name; use bevy_ecs::prelude::*; +use bevy_ecs::system::SystemState; use bevy_hierarchy::{Children, Parent}; +use bevy_log::error; use bevy_math::{FloatExt, Quat, Vec3}; use bevy_reflect::Reflect; use bevy_render::mesh::morph::MorphWeights; use bevy_time::Time; use bevy_transform::{prelude::Transform, TransformSystem}; -use bevy_utils::{tracing::warn, HashMap}; +use bevy_utils::{EntityHashMap, EntityHashSet, HashMap}; #[allow(missing_docs)] pub mod prelude { @@ -153,6 +155,12 @@ pub struct AnimationClip { duration: f32, } +#[derive(Clone, Component)] +pub struct AnimationTarget { + root: Entity, + path: EntityPath, +} + impl AnimationClip { #[inline] /// [`VariableCurve`]s for each bone. Indexed by the bone ID. @@ -228,7 +236,6 @@ struct PlayingAnimation { /// Note: This will always be in the range [0.0, animation clip duration] seek_time: f32, animation_clip: Handle, - path_cache: Vec>>, /// Number of times the animation has completed. /// If the animation is playing in reverse, this increments when the animation passes the start. completions: u32, @@ -242,7 +249,6 @@ impl Default for PlayingAnimation { elapsed: 0.0, seek_time: 0.0, animation_clip: Default::default(), - path_cache: Vec::new(), completions: 0, } } @@ -479,174 +485,186 @@ impl AnimationPlayer { } } -fn entity_from_path( - root: Entity, - path: &EntityPath, - children: &Query<&Children>, - names: &Query<&Name>, - path_cache: &mut Vec>, -) -> Option { - // PERF: finding the target entity can be optimised - let mut current_entity = root; - path_cache.resize(path.parts.len(), None); - - 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; +pub fn update_animation_targets( + mut commands: Commands, + candidate_targets: Query< + ( + Entity, + &Parent, + Option<&Children>, + &Name, + Option<&AnimationTarget>, + ), + ( + Or<(With, With)>, + Or<(Changed, Changed, Changed)>, + Without, + ), + >, + players: Query<( + Entity, + &mut AnimationPlayer, + Option>, + Ref, + )>, +) { + // Get ready to mark all dirty entities. Start with all targets that were + // modified, as well as any children of animation players that were + // modified. + // + // TODO: Make this a Local. + let mut worklist: Vec = candidate_targets.iter().map(|row| row.0).collect(); + for (_, _, kids, name) in players.iter() { + if let Some(kids) = kids { + if name.is_changed() || kids.is_changed() { + worklist.extend(kids.iter().map(|kid| *kid)); + } + } } - for (idx, part) in parts { - let mut found = false; - let children = children.get(current_entity).ok()?; - if let Some(cached) = path_cache[idx] { - if children.contains(&cached) { - if let Ok(name) = names.get(cached) { - if name == part { - current_entity = cached; - found = true; - } - } + // Iteratively mark all descendants of dirty entities as dirty themselves. + // At the end of this process, `dirty_entities` will consist of a forest. + let mut dirty_entities: EntityHashSet = EntityHashSet::default(); + while let Some(entity) = worklist.pop() { + if dirty_entities.contains(&entity) { + continue; + } + if let Ok((_, _, Some(kids), _, _)) = candidate_targets.get(entity) { + worklist.extend(kids.iter().map(|&kid| kid)); + } + dirty_entities.insert(entity); + } + + // Find the roots of the trees in the forest. + worklist = dirty_entities + .iter() + .filter(|&&entity| { + !candidate_targets + .get(entity) + .is_ok_and(|(_, parent, _, _, _)| dirty_entities.contains(&parent.get())) + }) + .cloned() + .collect(); + + // Compute new `AnimationTarget` components. + let mut new_targets: EntityHashMap = EntityHashMap::default(); + while let Some(entity) = worklist.pop() { + let (_, parent, kids, name, _) = candidate_targets + .get(entity) + .expect("Shouldn't have dirtied an entity that wasn't a target"); + + // Figure out the parent path. First, if we've already computed the path + // of the parent, just use that. + let mut new_target = AnimationTarget::new(); + if let Some(parent_target) = new_targets.get(&parent.get()) { + new_target = parent_target.clone(); + } + + // Otherwise, if this is a root of a tree in the forest, we know the + // parent's `AnimationTarget` is up to date, so fetch that. + if new_target.path.is_empty() { + if let Ok((_, _, _, _, Some(parent_target))) = candidate_targets.get(parent.get()) { + new_target = parent_target.clone(); } } - if !found { - for child in children.deref() { - if let Ok(name) = names.get(*child) { - if name == part { - // Found a children with the right name, continue to the next part - current_entity = *child; - path_cache[idx] = Some(*child); - found = true; - break; + + // If we still didn't find an `AnimationTarget` for the parent, we must be an + // immediate child of an animation player. Handle that case. + if new_target.path.is_empty() { + match players.get(parent.get()) { + Ok((_, _, _, name)) => { + new_target = AnimationTarget { + root: parent.get(), + path: EntityPath::from(name.clone()), } } + Err(_) => { + error!( + "Couldn't determine an appropriate path for {:?}. Is the tree malformed?", + entity + ); + break; + } } } - if !found { - warn!("Entity not found for path {:?} on part {:?}", path, part); - return None; + + // Push our own name onto the path and record it. + new_target.path.parts.push(name.clone()); + new_targets.insert(entity, new_target); + + // Enqueue children. + if let Some(kids) = kids { + worklist.extend(kids); } } - Some(current_entity) -} -/// Verify that there are no ancestors of a given entity that have an [`AnimationPlayer`]. -fn verify_no_ancestor_player( - player_parent: Option<&Parent>, - parents: &Query<(Has, Option<&Parent>)>, -) -> bool { - let Some(mut current) = player_parent.map(Parent::get) else { - return true; - }; - loop { - let Ok((has_player, parent)) = parents.get(current) else { - return true; - }; - if has_player { - return false; - } - if let Some(parent) = parent { - current = parent.get(); - } else { - return true; - } + // Finally, update the `AnimationTarget` components. + for (entity, animation_target) in new_targets.drain() { + commands.entity(entity).insert(animation_target); } } -/// System that will play all animations, using any entity with a [`AnimationPlayer`] -/// and a [`Handle`] as an animation root -#[allow(clippy::too_many_arguments)] pub fn animation_player( time: Res