From 2f2df97dcc1bd4c69f01a13fcf1f03d00e881033 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Sun, 27 Nov 2022 23:16:34 -0800
Subject: [PATCH 1/9] Getting it working

---
 crates/bevy_animation/Cargo.toml          |   4 +
 crates/bevy_animation/src/lib.rs          | 284 ++++++++++++++--------
 crates/bevy_ecs/src/storage/sparse_set.rs |   6 +
 3 files changed, 197 insertions(+), 97 deletions(-)

diff --git a/crates/bevy_animation/Cargo.toml b/crates/bevy_animation/Cargo.toml
index 5079bf057b837..5d5581c2590de 100644
--- a/crates/bevy_animation/Cargo.toml
+++ b/crates/bevy_animation/Cargo.toml
@@ -19,4 +19,8 @@ bevy_time = { path = "../bevy_time", version = "0.9.0" }
 bevy_utils = { path = "../bevy_utils", version = "0.9.0" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.9.0" }
 bevy_transform = { path = "../bevy_transform", version = "0.9.0" }
+bevy_tasks = { path = "../bevy_tasks", version = "0.9.0" }
 bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.9.0" }
+
+smallvec = "1.10"
+thread_local = "1.1"
\ No newline at end of file
diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index 0ab68da7c8788..e59bcc8b65f04 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -8,8 +8,10 @@ use bevy_app::{App, CoreStage, Plugin};
 use bevy_asset::{AddAsset, Assets, Handle};
 use bevy_core::Name;
 use bevy_ecs::{
+    prelude::*,
     change_detection::DetectChanges,
     entity::Entity,
+    storage::SparseSet,
     prelude::Component,
     reflect::ReflectComponent,
     schedule::IntoSystemDescriptor,
@@ -21,6 +23,10 @@ use bevy_reflect::{FromReflect, Reflect, TypeUuid};
 use bevy_time::Time;
 use bevy_transform::{prelude::Transform, TransformSystem};
 use bevy_utils::{tracing::warn, HashMap};
+use bevy_tasks::{ParallelSlice, ComputeTaskPool};
+use smallvec::{smallvec, SmallVec};
+use thread_local::ThreadLocal;
+use std::cell::Cell;
 
 #[allow(missing_docs)]
 pub mod prelude {
@@ -63,17 +69,28 @@ pub struct EntityPath {
 #[derive(Reflect, FromReflect, Clone, TypeUuid, Debug, Default)]
 #[uuid = "d81b7179-0448-4eb0-89fe-c067222725bf"]
 pub struct AnimationClip {
-    curves: HashMap<EntityPath, Vec<VariableCurve>>,
+    curves: Vec<Vec<VariableCurve>>,
+    paths: HashMap<EntityPath, usize>,
     duration: f32,
 }
 
 impl AnimationClip {
     #[inline]
     /// Hashmap of the [`VariableCurve`]s per [`EntityPath`].
-    pub fn curves(&self) -> &HashMap<EntityPath, Vec<VariableCurve>> {
+    pub fn curves(&self) -> &Vec<Vec<VariableCurve>> {
         &self.curves
     }
 
+    #[inline]
+    pub fn get_curves(&self, bone_id: usize) -> Option<&'_ Vec<VariableCurve>> {
+        self.curves.get(bone_id)
+    }
+
+    #[inline]
+    pub fn get_curves_by_path(&self, path: &EntityPath) -> Option<&'_ Vec<VariableCurve>> {
+        self.paths.get(path).and_then(|id| self.curves.get(*id))
+    }
+
     /// Duration of the clip, represented in seconds
     #[inline]
     pub fn duration(&self) -> f32 {
@@ -86,7 +103,13 @@ impl AnimationClip {
         self.duration = self
             .duration
             .max(*curve.keyframe_timestamps.last().unwrap_or(&0.0));
-        self.curves.entry(path).or_default().push(curve);
+        if let Some(bone_id) = self.paths.get(&path) {
+            self.curves[*bone_id].push(curve);
+        } else {
+            let idx = self.curves.len();
+            self.curves.push(vec![curve]);
+            self.paths.insert(path, idx);
+        }
     }
 }
 
@@ -181,117 +204,179 @@ impl AnimationPlayer {
     }
 }
 
-/// System that will play all animations, using any entity with a [`AnimationPlayer`]
-/// and a [`Handle<AnimationClip>`] as an animation root
-pub fn animation_player(
+#[derive(Clone)]
+pub struct BoneBinding {
+    root: Entity,
+    bone_id: usize,
+    elapsed: f32,
+}
+
+#[derive(Resource, Default)]
+pub struct BoneBindings {
+    // Entity must be globally unique.
+    bindings: Vec<(Entity, SmallVec<[BoneBinding; 2]>)>,
+}
+
+fn find_bone(root: Entity, path: &EntityPath, children: &Query<&Children>, names: &Query<&Name>) -> Option<Entity> {
+    // PERF: finding the target entity can be optimised
+    let mut current_entity = root;
+    // Ignore the first name, it is the root node which we already have
+    for part in path.parts.iter().skip(1) {
+        let mut found = false;
+        let children = children.get(current_entity).ok()?;
+        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;
+                    found = true;
+                    break;
+                }
+            }
+        }
+        if !found {
+            warn!("Entity not found for path {:?} on part {:?}", path, part);
+            return None;
+        }
+    }
+    Some(current_entity)
+}
+
+pub fn bind_bones(
     time: Res<Time>,
     animations: Res<Assets<AnimationClip>>,
-    mut animation_players: Query<(Entity, &mut AnimationPlayer)>,
     names: Query<&Name>,
-    mut transforms: Query<&mut Transform>,
     children: Query<&Children>,
+    mut animation_players: Query<(Entity, &mut AnimationPlayer)>,
+    mut queues: Local<ThreadLocal<Cell<Vec<(Entity, BoneBinding)>>>>,
+    mut dedup: Local<SparseSet<Entity, usize>>,
+    mut bone_bindings: ResMut<BoneBindings>,
 ) {
-    for (entity, mut player) in &mut animation_players {
-        if let Some(animation_clip) = animations.get(&player.animation_clip) {
-            // Continue if paused unless the `AnimationPlayer` was changed
-            // This allow the animation to still be updated if the player.elapsed field was manually updated in pause
-            if player.paused && !player.is_changed() {
-                continue;
-            }
-            if !player.paused {
-                player.elapsed += time.delta_seconds() * player.speed;
-            }
-            let mut elapsed = player.elapsed;
-            if player.repeat {
-                elapsed %= animation_clip.duration;
+    bone_bindings.bindings.clear();
+    animation_players.par_for_each_mut(100, |(root, mut player)| {
+        let Some(animation_clip) = animations.get(&player.animation_clip) else { return };
+        // Continue if paused unless the `AnimationPlayer` was changed
+        // This allow the animation to still be updated if the player.elapsed field was manually updated in pause
+        if player.paused && !player.is_changed() {
+            return;
+        }
+        if !player.paused {
+            player.elapsed += time.delta_seconds() * player.speed;
+        }
+        let mut elapsed = player.elapsed;
+        if player.repeat {
+            elapsed %= animation_clip.duration;
+        }
+        if elapsed < 0.0 {
+            elapsed += animation_clip.duration;
+        }
+        let queue_cell = queues.get_or_default();
+        let mut queue = queue_cell.take();
+        for (path, bone_id) in &animation_clip.paths {
+            if let Some(target) = find_bone(root, &path, &children, &names) { 
+                queue.push((target, BoneBinding { root, bone_id: *bone_id, elapsed }));
             }
-            if elapsed < 0.0 {
-                elapsed += animation_clip.duration;
+        }
+        queue_cell.set(queue);
+    });
+    bone_bindings.bindings.clear();
+    for queue in queues.iter_mut() {
+        for (entity, binding) in queue.get_mut() {
+            if let Some(idx) = dedup.get(*entity) {
+                bone_bindings.bindings[*idx].1.push(binding.clone());
+            } else {
+                let idx = bone_bindings.bindings.len();
+                bone_bindings.bindings.push((*entity, smallvec![binding.clone()]));
+                dedup.insert(*entity, idx);
             }
-            'entity: for (path, curves) in &animation_clip.curves {
-                // PERF: finding the target entity can be optimised
-                let mut current_entity = entity;
-                // Ignore the first name, it is the root node which we already have
-                for part in path.parts.iter().skip(1) {
-                    let mut found = false;
-                    if let Ok(children) = children.get(current_entity) {
-                        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;
-                                    found = true;
-                                    break;
-                                }
-                            }
-                        }
-                    }
-                    if !found {
-                        warn!("Entity not found for path {:?} on part {:?}", path, part);
-                        continue 'entity;
-                    }
-                }
-                if let Ok(mut transform) = transforms.get_mut(current_entity) {
-                    for curve in curves {
-                        // Some curves have only one keyframe used to set a transform
-                        if curve.keyframe_timestamps.len() == 1 {
-                            match &curve.keyframes {
-                                Keyframes::Rotation(keyframes) => transform.rotation = keyframes[0],
-                                Keyframes::Translation(keyframes) => {
-                                    transform.translation = keyframes[0];
-                                }
-                                Keyframes::Scale(keyframes) => transform.scale = keyframes[0],
-                            }
-                            continue;
-                        }
+        }
+        queue.get_mut().clear();
+    }
+    dedup.clear();
+}
 
-                        // Find the current keyframe
-                        // PERF: finding the current keyframe can be optimised
-                        let step_start = match curve
-                            .keyframe_timestamps
-                            .binary_search_by(|probe| probe.partial_cmp(&elapsed).unwrap())
-                        {
-                            Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
-                            Ok(i) => i,
-                            Err(0) => continue, // this curve isn't started yet
-                            Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
-                            Err(i) => i - 1,
-                        };
-                        let ts_start = curve.keyframe_timestamps[step_start];
-                        let ts_end = curve.keyframe_timestamps[step_start + 1];
-                        let lerp = (elapsed - ts_start) / (ts_end - ts_start);
-
-                        // Apply the keyframe
+/// System that will play all animations, using any entity with a [`AnimationPlayer`]
+/// and a [`Handle<AnimationClip>`] as an animation root
+pub fn animation_player(
+    animations: Res<Assets<AnimationClip>>,
+    animation_players: Query<&AnimationPlayer>,
+    transforms: Query<&mut Transform>,
+    mut bone_bindings: ResMut<BoneBindings>,
+) {
+    if bone_bindings.bindings.is_empty() {
+        return;
+    }
+    let task_pool = &*ComputeTaskPool::get();
+    let thread_count = task_pool.thread_num().max(1);
+    let batch_size = bone_bindings.bindings.len() / thread_count;
+    bone_bindings.bindings.par_chunk_map(task_pool, batch_size, |chunk| {
+        for (target, bindings) in chunk {
+            // SAFETY: bind_bones ensures that every binding in BoneBindings is unique.
+            let Ok(mut transform) = (unsafe { transforms.get_unchecked(*target) }) else { continue };
+            for binding in bindings {
+                let Ok(animator) = animation_players.get(binding.root) else { continue };
+                let Some(animation_clip) = animations.get(&animator.animation_clip) else { continue };
+                let Some(curves) = animation_clip.get_curves(binding.bone_id) else { continue };
+                for curve in curves {
+                    // Some curves have only one keyframe used to set a transform
+                    if curve.keyframe_timestamps.len() == 1 {
                         match &curve.keyframes {
-                            Keyframes::Rotation(keyframes) => {
-                                let rot_start = keyframes[step_start];
-                                let mut rot_end = keyframes[step_start + 1];
-                                // Choose the smallest angle for the rotation
-                                if rot_end.dot(rot_start) < 0.0 {
-                                    rot_end = -rot_end;
-                                }
-                                // Rotations are using a spherical linear interpolation
-                                transform.rotation =
-                                    rot_start.normalize().slerp(rot_end.normalize(), lerp);
-                            }
+                            Keyframes::Rotation(keyframes) => transform.rotation = keyframes[0],
                             Keyframes::Translation(keyframes) => {
-                                let translation_start = keyframes[step_start];
-                                let translation_end = keyframes[step_start + 1];
-                                let result = translation_start.lerp(translation_end, lerp);
-                                transform.translation = result;
+                                transform.translation = keyframes[0];
                             }
-                            Keyframes::Scale(keyframes) => {
-                                let scale_start = keyframes[step_start];
-                                let scale_end = keyframes[step_start + 1];
-                                let result = scale_start.lerp(scale_end, lerp);
-                                transform.scale = result;
+                            Keyframes::Scale(keyframes) => transform.scale = keyframes[0],
+                        }
+                        continue;
+                    }
+
+                    // Find the current keyframe
+                    // PERF: finding the current keyframe can be optimised
+                    let step_start = match curve
+                        .keyframe_timestamps
+                        .binary_search_by(|probe| probe.partial_cmp(&binding.elapsed).unwrap())
+                    {
+                        Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
+                        Ok(i) => i,
+                        Err(0) => continue, // this curve isn't started yet
+                        Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
+                        Err(i) => i - 1,
+                    };
+                    let ts_start = curve.keyframe_timestamps[step_start];
+                    let ts_end = curve.keyframe_timestamps[step_start + 1];
+                    let lerp = (binding.elapsed - ts_start) / (ts_end - ts_start);
+
+                    // Apply the keyframe
+                    match &curve.keyframes {
+                        Keyframes::Rotation(keyframes) => {
+                            let rot_start = keyframes[step_start];
+                            let mut rot_end = keyframes[step_start + 1];
+                            // Choose the smallest angle for the rotation
+                            if rot_end.dot(rot_start) < 0.0 {
+                                rot_end = -rot_end;
                             }
+                            // Rotations are using a spherical linear interpolation
+                            transform.rotation =
+                                rot_start.normalize().slerp(rot_end.normalize(), lerp);
+                        }
+                        Keyframes::Translation(keyframes) => {
+                            let translation_start = keyframes[step_start];
+                            let translation_end = keyframes[step_start + 1];
+                            let result = translation_start.lerp(translation_end, lerp);
+                            transform.translation = result;
+                        }
+                        Keyframes::Scale(keyframes) => {
+                            let scale_start = keyframes[step_start];
+                            let scale_end = keyframes[step_start + 1];
+                            let result = scale_start.lerp(scale_end, lerp);
+                            transform.scale = result;
                         }
                     }
                 }
             }
         }
-    }
+    });
+    bone_bindings.bindings.clear();
 }
 
 /// Adds animation support to an app
@@ -303,6 +388,11 @@ impl Plugin for AnimationPlugin {
         app.add_asset::<AnimationClip>()
             .register_asset_reflect::<AnimationClip>()
             .register_type::<AnimationPlayer>()
+            .init_resource::<BoneBindings>()
+            .add_system_to_stage(
+                CoreStage::PostUpdate,
+                bind_bones.before(animation_player),
+            )
             .add_system_to_stage(
                 CoreStage::PostUpdate,
                 animation_player.before(TransformSystem::TransformPropagate),
diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs
index 810a4fe569da2..22a0e1b5f4560 100644
--- a/crates/bevy_ecs/src/storage/sparse_set.rs
+++ b/crates/bevy_ecs/src/storage/sparse_set.rs
@@ -433,6 +433,12 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
         })
     }
 
+    pub fn clear(&mut self) {
+        self.dense.clear();
+        self.indices.clear();
+        self.sparse.clear();
+    }
+
     pub(crate) fn into_immutable(self) -> ImmutableSparseSet<I, V> {
         ImmutableSparseSet {
             dense: self.dense.into_boxed_slice(),

From dab84f9787a6a5686dcfc5175e05a49e6e8d059f Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Mon, 28 Nov 2022 00:08:05 -0800
Subject: [PATCH 2/9] Fix CI

---
 crates/bevy_animation/src/lib.rs | 48 +++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 13 deletions(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index e59bcc8b65f04..7d070b1b8e901 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -8,25 +8,25 @@ use bevy_app::{App, CoreStage, Plugin};
 use bevy_asset::{AddAsset, Assets, Handle};
 use bevy_core::Name;
 use bevy_ecs::{
-    prelude::*,
     change_detection::DetectChanges,
     entity::Entity,
-    storage::SparseSet,
     prelude::Component,
+    prelude::*,
     reflect::ReflectComponent,
     schedule::IntoSystemDescriptor,
+    storage::SparseSet,
     system::{Query, Res},
 };
 use bevy_hierarchy::Children;
 use bevy_math::{Quat, Vec3};
 use bevy_reflect::{FromReflect, Reflect, TypeUuid};
+use bevy_tasks::{ComputeTaskPool, ParallelSlice};
 use bevy_time::Time;
 use bevy_transform::{prelude::Transform, TransformSystem};
 use bevy_utils::{tracing::warn, HashMap};
-use bevy_tasks::{ParallelSlice, ComputeTaskPool};
 use smallvec::{smallvec, SmallVec};
-use thread_local::ThreadLocal;
 use std::cell::Cell;
+use thread_local::ThreadLocal;
 
 #[allow(missing_docs)]
 pub mod prelude {
@@ -81,11 +81,17 @@ impl AnimationClip {
         &self.curves
     }
 
+    /// Gets the curves for a bone.
+    ///
+    /// Returns `None` if the bone is invalid.
     #[inline]
     pub fn get_curves(&self, bone_id: usize) -> Option<&'_ Vec<VariableCurve>> {
         self.curves.get(bone_id)
     }
 
+    /// Gets the curves by it's [`EntityPath`].
+    ///
+    /// Returns `None` if the bone is invalid.
     #[inline]
     pub fn get_curves_by_path(&self, path: &EntityPath) -> Option<&'_ Vec<VariableCurve>> {
         self.paths.get(path).and_then(|id| self.curves.get(*id))
@@ -204,6 +210,7 @@ impl AnimationPlayer {
     }
 }
 
+/// A singular bone binding that between an [`AnimationPlayer`] and the target entity it animates.
 #[derive(Clone)]
 pub struct BoneBinding {
     root: Entity,
@@ -211,13 +218,19 @@ pub struct BoneBinding {
     elapsed: f32,
 }
 
+/// A [`Resource`] that contains the intermediate state of the animation system.
 #[derive(Resource, Default)]
 pub struct BoneBindings {
     // Entity must be globally unique.
     bindings: Vec<(Entity, SmallVec<[BoneBinding; 2]>)>,
 }
 
-fn find_bone(root: Entity, path: &EntityPath, children: &Query<&Children>, names: &Query<&Name>) -> Option<Entity> {
+fn find_bone(
+    root: Entity,
+    path: &EntityPath,
+    children: &Query<&Children>,
+    names: &Query<&Name>,
+) -> Option<Entity> {
     // PERF: finding the target entity can be optimised
     let mut current_entity = root;
     // Ignore the first name, it is the root node which we already have
@@ -242,6 +255,9 @@ fn find_bone(root: Entity, path: &EntityPath, children: &Query<&Children>, names
     Some(current_entity)
 }
 
+/// Binds the bones used by [`AnimationPlayer`] into [`BoneBindings`]
+/// for use in [`animation_player`].
+#[allow(clippy::too_many_arguments)]
 pub fn bind_bones(
     time: Res<Time>,
     animations: Res<Assets<AnimationClip>>,
@@ -273,8 +289,15 @@ pub fn bind_bones(
         let queue_cell = queues.get_or_default();
         let mut queue = queue_cell.take();
         for (path, bone_id) in &animation_clip.paths {
-            if let Some(target) = find_bone(root, &path, &children, &names) { 
-                queue.push((target, BoneBinding { root, bone_id: *bone_id, elapsed }));
+            if let Some(target) = find_bone(root, path, &children, &names) {
+                queue.push((
+                    target,
+                    BoneBinding {
+                        root,
+                        bone_id: *bone_id,
+                        elapsed,
+                    },
+                ));
             }
         }
         queue_cell.set(queue);
@@ -286,7 +309,9 @@ pub fn bind_bones(
                 bone_bindings.bindings[*idx].1.push(binding.clone());
             } else {
                 let idx = bone_bindings.bindings.len();
-                bone_bindings.bindings.push((*entity, smallvec![binding.clone()]));
+                bone_bindings
+                    .bindings
+                    .push((*entity, smallvec![binding.clone()]));
                 dedup.insert(*entity, idx);
             }
         }
@@ -306,7 +331,7 @@ pub fn animation_player(
     if bone_bindings.bindings.is_empty() {
         return;
     }
-    let task_pool = &*ComputeTaskPool::get();
+    let task_pool = ComputeTaskPool::get();
     let thread_count = task_pool.thread_num().max(1);
     let batch_size = bone_bindings.bindings.len() / thread_count;
     bone_bindings.bindings.par_chunk_map(task_pool, batch_size, |chunk| {
@@ -389,10 +414,7 @@ impl Plugin for AnimationPlugin {
             .register_asset_reflect::<AnimationClip>()
             .register_type::<AnimationPlayer>()
             .init_resource::<BoneBindings>()
-            .add_system_to_stage(
-                CoreStage::PostUpdate,
-                bind_bones.before(animation_player),
-            )
+            .add_system_to_stage(CoreStage::PostUpdate, bind_bones.before(animation_player))
             .add_system_to_stage(
                 CoreStage::PostUpdate,
                 animation_player.before(TransformSystem::TransformPropagate),

From 3354811a679da39fee971bd0f6d46b11c3049edf Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Tue, 29 Nov 2022 11:59:18 -0800
Subject: [PATCH 3/9] Cache entity paths

---
 Cargo.toml                       |  3 +++
 crates/bevy_animation/src/lib.rs | 39 ++++++++++++++++++++++++--------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index c6ec1f14d6bd0..7253d302591c0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1613,3 +1613,6 @@ inherits = "release"
 opt-level = "z"
 lto = "fat"
 codegen-units = 1
+
+[profile.release]
+lto = true
\ No newline at end of file
diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index 7d070b1b8e901..f3e9fb5564d55 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -128,6 +128,7 @@ pub struct AnimationPlayer {
     speed: f32,
     elapsed: f32,
     animation_clip: Handle<AnimationClip>,
+    path_cache: Vec<Vec<Option<Entity>>>,
 }
 
 impl Default for AnimationPlayer {
@@ -138,6 +139,7 @@ impl Default for AnimationPlayer {
             speed: 1.0,
             elapsed: 0.0,
             animation_clip: Default::default(),
+            path_cache: Vec::new(),
         }
     }
 }
@@ -230,20 +232,35 @@ fn find_bone(
     path: &EntityPath,
     children: &Query<&Children>,
     names: &Query<&Name>,
+    path_cache: &mut Vec<Option<Entity>>,
 ) -> Option<Entity> {
     // PERF: finding the target entity can be optimised
     let mut current_entity = root;
+    path_cache.resize(path.parts.len(), None);
     // Ignore the first name, it is the root node which we already have
-    for part in path.parts.iter().skip(1) {
+    for (idx, part) in path.parts.iter().enumerate().skip(1) {
         let mut found = false;
         let children = children.get(current_entity).ok()?;
-        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;
-                    found = true;
-                    break;
+        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;
+                    }
+                }
+            }
+        }
+        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;
+                    }
                 }
             }
         }
@@ -288,8 +305,12 @@ pub fn bind_bones(
         }
         let queue_cell = queues.get_or_default();
         let mut queue = queue_cell.take();
+        if player.path_cache.len() != animation_clip.paths.len() {
+            player.path_cache = vec![Vec::new(); animation_clip.paths.len()];
+        }
         for (path, bone_id) in &animation_clip.paths {
-            if let Some(target) = find_bone(root, path, &children, &names) {
+            let cached_path = &mut player.path_cache[*bone_id];
+            if let Some(target) = find_bone(root, path, &children, &names, cached_path) {
                 queue.push((
                     target,
                     BoneBinding {

From 0a72c40e1233eb45d3cd05fdeb705cc622f445c2 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Tue, 29 Nov 2022 13:02:07 -0800
Subject: [PATCH 4/9] Remove accidental LTO added to profile

---
 Cargo.toml | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/Cargo.toml b/Cargo.toml
index 7253d302591c0..c6ec1f14d6bd0 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1613,6 +1613,3 @@ inherits = "release"
 opt-level = "z"
 lto = "fat"
 codegen-units = 1
-
-[profile.release]
-lto = true
\ No newline at end of file

From 45fd8b97def277461e73c7711e93f5b19dc0817f Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Sat, 24 Dec 2022 20:33:23 -0800
Subject: [PATCH 5/9] Switch strategies

---
 crates/bevy_animation/src/lib.rs | 228 ++++++++++++-------------------
 1 file changed, 87 insertions(+), 141 deletions(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index f3e9fb5564d55..1a0c277c9488f 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -11,22 +11,17 @@ use bevy_ecs::{
     change_detection::DetectChanges,
     entity::Entity,
     prelude::Component,
-    prelude::*,
+    query::With,
     reflect::ReflectComponent,
     schedule::IntoSystemDescriptor,
-    storage::SparseSet,
     system::{Query, Res},
 };
-use bevy_hierarchy::Children;
+use bevy_hierarchy::{Children, Parent};
 use bevy_math::{Quat, Vec3};
 use bevy_reflect::{FromReflect, Reflect, TypeUuid};
-use bevy_tasks::{ComputeTaskPool, ParallelSlice};
 use bevy_time::Time;
 use bevy_transform::{prelude::Transform, TransformSystem};
 use bevy_utils::{tracing::warn, HashMap};
-use smallvec::{smallvec, SmallVec};
-use std::cell::Cell;
-use thread_local::ThreadLocal;
 
 #[allow(missing_docs)]
 pub mod prelude {
@@ -212,21 +207,6 @@ impl AnimationPlayer {
     }
 }
 
-/// A singular bone binding that between an [`AnimationPlayer`] and the target entity it animates.
-#[derive(Clone)]
-pub struct BoneBinding {
-    root: Entity,
-    bone_id: usize,
-    elapsed: f32,
-}
-
-/// A [`Resource`] that contains the intermediate state of the animation system.
-#[derive(Resource, Default)]
-pub struct BoneBindings {
-    // Entity must be globally unique.
-    bindings: Vec<(Entity, SmallVec<[BoneBinding; 2]>)>,
-}
-
 fn find_bone(
     root: Entity,
     path: &EntityPath,
@@ -272,21 +252,37 @@ fn find_bone(
     Some(current_entity)
 }
 
-/// Binds the bones used by [`AnimationPlayer`] into [`BoneBindings`]
-/// for use in [`animation_player`].
-#[allow(clippy::too_many_arguments)]
-pub fn bind_bones(
+/// 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<(Option<With<AnimationPlayer>>, Option<&Parent>)>,
+) -> bool {
+    let Some(mut current) = player_parent.map(Parent::get) else { return true };
+    loop {
+        let Ok((maybe_player, parent)) = parents.get(current) else { return true };
+        if maybe_player.is_some() {
+            return false;
+        }
+        if let Some(parent) = parent {
+            current = parent.get()
+        } else {
+            return true;
+        }
+    }
+}
+
+/// System that will play all animations, using any entity with a [`AnimationPlayer`]
+/// and a [`Handle<AnimationClip>`] as an animation root
+pub fn animation_player(
     time: Res<Time>,
     animations: Res<Assets<AnimationClip>>,
-    names: Query<&Name>,
     children: Query<&Children>,
-    mut animation_players: Query<(Entity, &mut AnimationPlayer)>,
-    mut queues: Local<ThreadLocal<Cell<Vec<(Entity, BoneBinding)>>>>,
-    mut dedup: Local<SparseSet<Entity, usize>>,
-    mut bone_bindings: ResMut<BoneBindings>,
+    names: Query<&Name>,
+    transforms: Query<&mut Transform>,
+    parents: Query<(Option<With<AnimationPlayer>>, Option<&Parent>)>,
+    mut animation_players: Query<(Entity, Option<&Parent>, &mut AnimationPlayer)>,
 ) {
-    bone_bindings.bindings.clear();
-    animation_players.par_for_each_mut(100, |(root, mut player)| {
+    animation_players.par_for_each_mut(10, |(root, maybe_parent, mut player)| {
         let Some(animation_clip) = animations.get(&player.animation_clip) else { return };
         // Continue if paused unless the `AnimationPlayer` was changed
         // This allow the animation to still be updated if the player.elapsed field was manually updated in pause
@@ -303,126 +299,78 @@ pub fn bind_bones(
         if elapsed < 0.0 {
             elapsed += animation_clip.duration;
         }
-        let queue_cell = queues.get_or_default();
-        let mut queue = queue_cell.take();
         if player.path_cache.len() != animation_clip.paths.len() {
             player.path_cache = vec![Vec::new(); animation_clip.paths.len()];
         }
+        if !verify_no_ancestor_player(maybe_parent, &parents) {
+            warn!("Animation player on {:?} has a conflicting animation player on an ancestor. Cannot safely animate.", root);
+            return;
+        }
         for (path, bone_id) in &animation_clip.paths {
             let cached_path = &mut player.path_cache[*bone_id];
-            if let Some(target) = find_bone(root, path, &children, &names, cached_path) {
-                queue.push((
-                    target,
-                    BoneBinding {
-                        root,
-                        bone_id: *bone_id,
-                        elapsed,
-                    },
-                ));
-            }
-        }
-        queue_cell.set(queue);
-    });
-    bone_bindings.bindings.clear();
-    for queue in queues.iter_mut() {
-        for (entity, binding) in queue.get_mut() {
-            if let Some(idx) = dedup.get(*entity) {
-                bone_bindings.bindings[*idx].1.push(binding.clone());
-            } else {
-                let idx = bone_bindings.bindings.len();
-                bone_bindings
-                    .bindings
-                    .push((*entity, smallvec![binding.clone()]));
-                dedup.insert(*entity, idx);
-            }
-        }
-        queue.get_mut().clear();
-    }
-    dedup.clear();
-}
-
-/// System that will play all animations, using any entity with a [`AnimationPlayer`]
-/// and a [`Handle<AnimationClip>`] as an animation root
-pub fn animation_player(
-    animations: Res<Assets<AnimationClip>>,
-    animation_players: Query<&AnimationPlayer>,
-    transforms: Query<&mut Transform>,
-    mut bone_bindings: ResMut<BoneBindings>,
-) {
-    if bone_bindings.bindings.is_empty() {
-        return;
-    }
-    let task_pool = ComputeTaskPool::get();
-    let thread_count = task_pool.thread_num().max(1);
-    let batch_size = bone_bindings.bindings.len() / thread_count;
-    bone_bindings.bindings.par_chunk_map(task_pool, batch_size, |chunk| {
-        for (target, bindings) in chunk {
-            // SAFETY: bind_bones ensures that every binding in BoneBindings is unique.
-            let Ok(mut transform) = (unsafe { transforms.get_unchecked(*target) }) else { continue };
-            for binding in bindings {
-                let Ok(animator) = animation_players.get(binding.root) else { continue };
-                let Some(animation_clip) = animations.get(&animator.animation_clip) else { continue };
-                let Some(curves) = animation_clip.get_curves(binding.bone_id) else { continue };
-                for curve in curves {
-                    // Some curves have only one keyframe used to set a transform
-                    if curve.keyframe_timestamps.len() == 1 {
-                        match &curve.keyframes {
-                            Keyframes::Rotation(keyframes) => transform.rotation = keyframes[0],
-                            Keyframes::Translation(keyframes) => {
-                                transform.translation = keyframes[0];
-                            }
-                            Keyframes::Scale(keyframes) => transform.scale = keyframes[0],
-                        }
-                        continue;
-                    }
-
-                    // Find the current keyframe
-                    // PERF: finding the current keyframe can be optimised
-                    let step_start = match curve
-                        .keyframe_timestamps
-                        .binary_search_by(|probe| probe.partial_cmp(&binding.elapsed).unwrap())
-                    {
-                        Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
-                        Ok(i) => i,
-                        Err(0) => continue, // this curve isn't started yet
-                        Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
-                        Err(i) => i - 1,
-                    };
-                    let ts_start = curve.keyframe_timestamps[step_start];
-                    let ts_end = curve.keyframe_timestamps[step_start + 1];
-                    let lerp = (binding.elapsed - ts_start) / (ts_end - ts_start);
-
-                    // Apply the keyframe
+            let curves = animation_clip.get_curves(*bone_id).unwrap();
+            let Some(target) = find_bone(root, path, &children, &names, cached_path) else { continue };
+            // SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
+            // any of their descendant Transforms.
+            let Ok(mut transform) = (unsafe { transforms.get_unchecked(target) }) else { continue };
+            for curve in curves {
+                // Some curves have only one keyframe used to set a transform
+                if curve.keyframe_timestamps.len() == 1 {
                     match &curve.keyframes {
-                        Keyframes::Rotation(keyframes) => {
-                            let rot_start = keyframes[step_start];
-                            let mut rot_end = keyframes[step_start + 1];
-                            // Choose the smallest angle for the rotation
-                            if rot_end.dot(rot_start) < 0.0 {
-                                rot_end = -rot_end;
-                            }
-                            // Rotations are using a spherical linear interpolation
-                            transform.rotation =
-                                rot_start.normalize().slerp(rot_end.normalize(), lerp);
-                        }
+                        Keyframes::Rotation(keyframes) => transform.rotation = keyframes[0],
                         Keyframes::Translation(keyframes) => {
-                            let translation_start = keyframes[step_start];
-                            let translation_end = keyframes[step_start + 1];
-                            let result = translation_start.lerp(translation_end, lerp);
-                            transform.translation = result;
+                            transform.translation = keyframes[0];
                         }
-                        Keyframes::Scale(keyframes) => {
-                            let scale_start = keyframes[step_start];
-                            let scale_end = keyframes[step_start + 1];
-                            let result = scale_start.lerp(scale_end, lerp);
-                            transform.scale = result;
+                        Keyframes::Scale(keyframes) => transform.scale = keyframes[0],
+                    }
+                    continue;
+                }
+
+                // Find the current keyframe
+                // PERF: finding the current keyframe can be optimised
+                let step_start = match curve
+                    .keyframe_timestamps
+                    .binary_search_by(|probe| probe.partial_cmp(&elapsed).unwrap())
+                {
+                    Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
+                    Ok(i) => i,
+                    Err(0) => continue, // this curve isn't started yet
+                    Err(n) if n > curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished
+                    Err(i) => i - 1,
+                };
+                let ts_start = curve.keyframe_timestamps[step_start];
+                let ts_end = curve.keyframe_timestamps[step_start + 1];
+                let lerp = (elapsed - ts_start) / (ts_end - ts_start);
+
+                // Apply the keyframe
+                match &curve.keyframes {
+                    Keyframes::Rotation(keyframes) => {
+                        let rot_start = keyframes[step_start];
+                        let mut rot_end = keyframes[step_start + 1];
+                        // Choose the smallest angle for the rotation
+                        if rot_end.dot(rot_start) < 0.0 {
+                            rot_end = -rot_end;
                         }
+                        // Rotations are using a spherical linear interpolation
+                        transform.rotation =
+                            rot_start.normalize().slerp(rot_end.normalize(), lerp);
+                    }
+                    Keyframes::Translation(keyframes) => {
+                        let translation_start = keyframes[step_start];
+                        let translation_end = keyframes[step_start + 1];
+                        let result = translation_start.lerp(translation_end, lerp);
+                        transform.translation = result;
+                    }
+                    Keyframes::Scale(keyframes) => {
+                        let scale_start = keyframes[step_start];
+                        let scale_end = keyframes[step_start + 1];
+                        let result = scale_start.lerp(scale_end, lerp);
+                        transform.scale = result;
                     }
                 }
             }
         }
     });
-    bone_bindings.bindings.clear();
 }
 
 /// Adds animation support to an app
@@ -434,8 +382,6 @@ impl Plugin for AnimationPlugin {
         app.add_asset::<AnimationClip>()
             .register_asset_reflect::<AnimationClip>()
             .register_type::<AnimationPlayer>()
-            .init_resource::<BoneBindings>()
-            .add_system_to_stage(CoreStage::PostUpdate, bind_bones.before(animation_player))
             .add_system_to_stage(
                 CoreStage::PostUpdate,
                 animation_player.before(TransformSystem::TransformPropagate),

From 297180674e03e01babc99803b0b9e8d35ded047c Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Sat, 24 Dec 2022 21:08:38 -0800
Subject: [PATCH 6/9] Fix CI

---
 crates/bevy_animation/src/lib.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index 1a0c277c9488f..c00c3b0398d80 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -252,7 +252,7 @@ fn find_bone(
     Some(current_entity)
 }
 
-/// Verify that there are no ancestors of a given entity that have an AnimationPlayer.
+/// 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<(Option<With<AnimationPlayer>>, Option<&Parent>)>,
@@ -264,7 +264,7 @@ fn verify_no_ancestor_player(
             return false;
         }
         if let Some(parent) = parent {
-            current = parent.get()
+            current = parent.get();
         } else {
             return true;
         }

From 11a095f819e3143d5609fd8f0dc057534668f610 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Sun, 25 Dec 2022 10:25:34 -0800
Subject: [PATCH 7/9] Address review comments

---
 crates/bevy_animation/Cargo.toml          | 4 ----
 crates/bevy_ecs/src/storage/sparse_set.rs | 6 ------
 2 files changed, 10 deletions(-)

diff --git a/crates/bevy_animation/Cargo.toml b/crates/bevy_animation/Cargo.toml
index 5d5581c2590de..5079bf057b837 100644
--- a/crates/bevy_animation/Cargo.toml
+++ b/crates/bevy_animation/Cargo.toml
@@ -19,8 +19,4 @@ bevy_time = { path = "../bevy_time", version = "0.9.0" }
 bevy_utils = { path = "../bevy_utils", version = "0.9.0" }
 bevy_ecs = { path = "../bevy_ecs", version = "0.9.0" }
 bevy_transform = { path = "../bevy_transform", version = "0.9.0" }
-bevy_tasks = { path = "../bevy_tasks", version = "0.9.0" }
 bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.9.0" }
-
-smallvec = "1.10"
-thread_local = "1.1"
\ No newline at end of file
diff --git a/crates/bevy_ecs/src/storage/sparse_set.rs b/crates/bevy_ecs/src/storage/sparse_set.rs
index d145122c33456..cc3da14682cb9 100644
--- a/crates/bevy_ecs/src/storage/sparse_set.rs
+++ b/crates/bevy_ecs/src/storage/sparse_set.rs
@@ -447,12 +447,6 @@ impl<I: SparseSetIndex, V> SparseSet<I, V> {
         })
     }
 
-    pub fn clear(&mut self) {
-        self.dense.clear();
-        self.indices.clear();
-        self.sparse.clear();
-    }
-
     pub(crate) fn into_immutable(self) -> ImmutableSparseSet<I, V> {
         ImmutableSparseSet {
             dense: self.dense.into_boxed_slice(),

From 72400219b682425d13dcafa22ee15a77b3ccface Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Tue, 27 Dec 2022 15:31:58 -0800
Subject: [PATCH 8/9] Expand on the safety justification.

---
 crates/bevy_animation/src/lib.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index c00c3b0398d80..9831446cf9509 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -312,6 +312,16 @@ pub fn animation_player(
             let Some(target) = find_bone(root, path, &children, &names, cached_path) else { continue };
             // SAFETY: The verify_no_ancestor_player check above ensures that two animation players cannot alias
             // any of their descendant Transforms.
+            // 
+            // The system scheduler prevents any other system from mutating Transforms at the same time, 
+            // so the only way this fetch can alias is if two AnimationPlayers are targetting the same bone.
+            // This can only happen if there are two or more AnimationPlayers are ancestors to the same
+            // entities. By verifying that there is no other AnimationPlayer in the ancestors of a
+            // running AnimationPlayer before animating any entity, this fetch cannot alias.
+            //
+            // This means only the AnimationPlayers closest to the root of the hierarchy will be able
+            // to run their animation. Any players in the children or descendants will log a warning
+            // and do nothing.
             let Ok(mut transform) = (unsafe { transforms.get_unchecked(target) }) else { continue };
             for curve in curves {
                 // Some curves have only one keyframe used to set a transform

From c77c20b0b74b8dbe26a5150c5183bc6c8cfd8a50 Mon Sep 17 00:00:00 2001
From: james7132 <contact@jamessliu.com>
Date: Mon, 2 Jan 2023 10:15:03 -0800
Subject: [PATCH 9/9] Remove return type from curves doc comment

---
 crates/bevy_animation/src/lib.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs
index 9831446cf9509..9b9e8de991af9 100644
--- a/crates/bevy_animation/src/lib.rs
+++ b/crates/bevy_animation/src/lib.rs
@@ -71,7 +71,7 @@ pub struct AnimationClip {
 
 impl AnimationClip {
     #[inline]
-    /// Hashmap of the [`VariableCurve`]s per [`EntityPath`].
+    /// [`VariableCurve`]s for each bone. Indexed by the bone ID.
     pub fn curves(&self) -> &Vec<Vec<VariableCurve>> {
         &self.curves
     }