Skip to content

Commit

Permalink
Intern entity paths for faster comparisons.
Browse files Browse the repository at this point in the history
This patch places all entity paths into a shared table so that comparing
them is as cheap as a pointer comparison. We don't use the pre-existing
`Interner` type because that leaks all strings added to it, and I was
uncomfortable with leaking names, as Bevy apps might dynamically
generate them. Instead, I reference count all the names and use a linked
list to stitch together paths into a tree. The interner uses a weak hash
set from the [`weak-table`] crate.

This patch is especially helpful for the two-phase animation PR bevyengine#11707,
because two-phase animation gets rid of the cache from name to
animation-specific slot, thus increasing the load on the hash table that
maps paths to bone indices.

Note that the interned table is a global variable behind a `OnceLock`
instead of a resource. This is because it must be accessed from the glTF
`AssetLoader`, which unfortunately has no access to Bevy resources.

[`weak-table`]: https://crates.io/crates/weak-table
  • Loading branch information
pcwalton committed Feb 5, 2024
1 parent 71be08a commit bca3439
Show file tree
Hide file tree
Showing 5 changed files with 174 additions and 29 deletions.
4 changes: 4 additions & 0 deletions crates/bevy_animation/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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_derive = { path = "../bevy_derive", version = "0.12.0" }
bevy_math = { path = "../bevy_math", version = "0.12.0" }
bevy_reflect = { path = "../bevy_reflect", version = "0.12.0", features = [
"bevy",
Expand All @@ -24,5 +25,8 @@ bevy_ecs = { path = "../bevy_ecs", version = "0.12.0" }
bevy_transform = { path = "../bevy_transform", version = "0.12.0" }
bevy_hierarchy = { path = "../bevy_hierarchy", version = "0.12.0" }

# others
weak-table = "0.3"

[lints]
workspace = true
153 changes: 153 additions & 0 deletions crates/bevy_animation/src/entity_path.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
//! Entity paths for referring to bones.

use std::{
fmt::{self, Debug, Formatter, Write},
hash::{Hash, Hasher},
sync::{Arc, Mutex, OnceLock, Weak},
};

use bevy_core::Name;
use bevy_reflect::Reflect;
use bevy_utils::prelude::default;
use weak_table::WeakHashSet;

static ENTITY_PATH_STORE: OnceLock<Mutex<EntityPathStore>> = OnceLock::new();

/// Path to an entity, with [`Name`]s. Each entity in a path must have a name.
#[derive(Clone, Reflect)]
#[reflect_value]
pub struct EntityPath(Arc<EntityPathNode>);

#[derive(PartialEq, Eq, Hash)]
struct EntityPathNode {
name: Name,
parent: Option<EntityPath>,
}

// This could use a `RwLock`, but we actually never read from this, so a mutex
// is actually slightly more efficient!
#[derive(Default)]
struct EntityPathStore(WeakHashSet<Weak<EntityPathNode>>);

pub struct EntityPathIter<'a>(Option<&'a EntityPath>);

impl EntityPathStore {
fn create_path(&mut self, node: EntityPathNode) -> EntityPath {
match self.0.get(&node) {
Some(node) => EntityPath(node),
None => {
let node = Arc::new(node);
self.0.insert(node.clone());
EntityPath(node)
}
}
}
}

impl EntityPath {
pub fn from_name(name: Name) -> EntityPath {
ENTITY_PATH_STORE
.get_or_init(|| default())
.lock()
.unwrap()
.create_path(EntityPathNode { name, parent: None })
}

pub fn from_names(names: &[Name]) -> EntityPath {
let mut store = ENTITY_PATH_STORE.get_or_init(|| default()).lock().unwrap();

let mut names = names.iter();
let root_name = names
.next()
.expect("Entity path must have at least one name in it");

let mut path = store.create_path(EntityPathNode {
name: root_name.clone(),
parent: None,
});
for name in names {
path = store.create_path(EntityPathNode {
name: name.clone(),
parent: Some(path),
});
}

path
}

pub fn extend(&self, name: Name) -> EntityPath {
ENTITY_PATH_STORE
.get_or_init(|| default())
.lock()
.unwrap()
.create_path(EntityPathNode {
name,
parent: Some(self.clone()),
})
}

pub fn iter(&self) -> EntityPathIter {
EntityPathIter(Some(self))
}

pub fn root(&self) -> &Name {
&self.iter().last().unwrap().0.name
}

pub fn len(&self) -> usize {
self.iter().count()
}

pub fn name(&self) -> &Name {
&self.0.name
}
}

impl PartialEq for EntityPath {
fn eq(&self, other: &Self) -> bool {
Arc::ptr_eq(&self.0, &other.0)
}
}

impl Eq for EntityPath {}

impl Hash for EntityPath {
fn hash<H: Hasher>(&self, state: &mut H) {
// Hash by address. This is safe because entity paths are unique.
(self.0.as_ref() as *const EntityPathNode).hash(state)
}
}

impl Debug for EntityPath {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let mut names = vec![];
let mut current_path = Some(self.clone());
while let Some(path) = current_path {
names.push(path.0.name.clone());
current_path = path.0.parent.clone();
}

for (name_index, name) in names.iter().rev().enumerate() {
if name_index > 0 {
f.write_char('/')?;
}
f.write_str(name)?;
}

Ok(())
}
}

impl<'a> Iterator for EntityPathIter<'a> {
type Item = &'a EntityPath;

fn next(&mut self) -> Option<Self::Item> {
match self.0 {
None => None,
Some(node) => {
self.0 = node.0.parent.as_ref();
Some(node)
}
}
}
}
26 changes: 12 additions & 14 deletions crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Animation for the game engine Bevy

mod animatable;
mod entity_path;
mod util;

use std::ops::{Add, Deref, Mul};
Expand All @@ -16,17 +17,20 @@ use bevy_reflect::Reflect;
use bevy_render::mesh::morph::MorphWeights;
use bevy_time::Time;
use bevy_transform::{prelude::Transform, TransformSystem};
use bevy_utils::smallvec::SmallVec;
use bevy_utils::{tracing::warn, HashMap};

#[allow(missing_docs)]
pub mod prelude {
#[doc(hidden)]
pub use crate::{
animatable::*, AnimationClip, AnimationPlayer, AnimationPlugin, EntityPath, Interpolation,
Keyframes, VariableCurve,
animatable::*, entity_path::*, AnimationClip, AnimationPlayer, AnimationPlugin,
Interpolation, Keyframes, VariableCurve,
};
}

pub use crate::entity_path::EntityPath;

/// List of keyframes for one of the attribute of a [`Transform`].
#[derive(Reflect, Clone, Debug)]
pub enum Keyframes {
Expand Down Expand Up @@ -138,13 +142,6 @@ pub enum Interpolation {
CubicSpline,
}

/// Path to an entity, with [`Name`]s. Each entity in a path must have a name.
#[derive(Reflect, Clone, Debug, Hash, PartialEq, Eq, Default)]
pub struct EntityPath {
/// Parts of the path
pub parts: Vec<Name>,
}

/// A list of [`VariableCurve`], and the [`EntityPath`] to which they apply.
#[derive(Asset, Reflect, Clone, Debug, Default)]
pub struct AnimationClip {
Expand Down Expand Up @@ -199,7 +196,7 @@ impl AnimationClip {

/// Whether this animation clip can run on entity with given [`Name`].
pub fn compatible_with(&self, name: &Name) -> bool {
self.paths.keys().any(|path| &path.parts[0] == name)
self.paths.keys().any(|path| &path.root() == &name)
}
}

Expand Down Expand Up @@ -488,9 +485,10 @@ fn entity_from_path(
) -> Option<Entity> {
// PERF: finding the target entity can be optimised
let mut current_entity = root;
path_cache.resize(path.parts.len(), None);
path_cache.resize(path.len(), None);

let mut parts = path.parts.iter().enumerate();
let parts_vec: SmallVec<[&Name; 8]> = path.iter().map(|path| path.name()).collect();
let mut parts = parts_vec.iter().rev().enumerate();

// check the first name is the root node which we already have
let Some((_, root_name)) = parts.next() else {
Expand All @@ -506,7 +504,7 @@ fn entity_from_path(
if let Some(cached) = path_cache[idx] {
if children.contains(&cached) {
if let Ok(name) = names.get(cached) {
if name == part {
if name == *part {
current_entity = cached;
found = true;
}
Expand All @@ -516,7 +514,7 @@ fn entity_from_path(
if !found {
for child in children.deref() {
if let Ok(name) = names.get(*child) {
if name == part {
if name == *part {
// Found a children with the right name, continue to the next part
current_entity = *child;
path_cache[idx] = Some(*child);
Expand Down
4 changes: 1 addition & 3 deletions crates/bevy_gltf/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,9 +260,7 @@ async fn load_gltf<'a, 'b, 'c>(
if let Some((root_index, path)) = paths.get(&node.index()) {
animation_roots.insert(root_index);
animation_clip.add_curve_to_path(
bevy_animation::EntityPath {
parts: path.clone(),
},
bevy_animation::EntityPath::from_names(&path),
bevy_animation::VariableCurve {
keyframe_timestamps,
keyframes,
Expand Down
16 changes: 4 additions & 12 deletions examples/animation/animated_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ fn setup(
let mut animation = AnimationClip::default();
// A curve can modify a single part of a transform, here the translation
animation.add_curve_to_path(
EntityPath {
parts: vec![planet.clone()],
},
EntityPath::from_name(planet.clone()),
VariableCurve {
keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
keyframes: Keyframes::Translation(vec![
Expand All @@ -57,9 +55,7 @@ fn setup(
// To find the entity to modify, the hierarchy will be traversed looking for
// an entity with the right name at each level
animation.add_curve_to_path(
EntityPath {
parts: vec![planet.clone(), orbit_controller.clone()],
},
EntityPath::from_names(&[planet.clone(), orbit_controller.clone()]),
VariableCurve {
keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
keyframes: Keyframes::Rotation(vec![
Expand All @@ -76,9 +72,7 @@ fn setup(
// until all other curves are finished. In that case, another animation should
// be created for each part that would have a different duration / period
animation.add_curve_to_path(
EntityPath {
parts: vec![planet.clone(), orbit_controller.clone(), satellite.clone()],
},
EntityPath::from_names(&[planet.clone(), orbit_controller.clone(), satellite.clone()]),
VariableCurve {
keyframe_timestamps: vec![0.0, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0, 3.5, 4.0],
keyframes: Keyframes::Scale(vec![
Expand All @@ -97,9 +91,7 @@ fn setup(
);
// There can be more than one curve targeting the same entity path
animation.add_curve_to_path(
EntityPath {
parts: vec![planet.clone(), orbit_controller.clone(), satellite.clone()],
},
EntityPath::from_names(&[planet.clone(), orbit_controller.clone(), satellite.clone()]),
VariableCurve {
keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0],
keyframes: Keyframes::Rotation(vec![
Expand Down

0 comments on commit bca3439

Please sign in to comment.