From dcb5c44e01227da996175d647df0efa50428074c Mon Sep 17 00:00:00 2001 From: A-Walrus Date: Sat, 2 Dec 2023 14:51:22 +0200 Subject: [PATCH 1/5] Fix last keyframe --- crates/bevy_animation/src/lib.rs | 71 +++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 25 deletions(-) diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index e4fdcd360d15a..2f78f719590de 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -608,6 +608,39 @@ fn apply_animation( parents: &Query<(Has, Option<&Parent>)>, children: &Query<&Children>, ) { + /// Set transform to exactly the value of some keyframe. + fn set_keyframe( + curve: &VariableCurve, + transform: &mut Mut<'_, Transform>, + weight: f32, + morphs: &mut Result, bevy_ecs::query::QueryEntityError>, + keyframe: usize, + ) { + match &curve.keyframes { + Keyframes::Rotation(keyframes) => { + transform.rotation = transform.rotation.slerp(keyframes[keyframe], weight); + } + Keyframes::Translation(keyframes) => { + transform.translation = transform.translation.lerp(keyframes[keyframe], weight); + } + Keyframes::Scale(keyframes) => { + transform.scale = transform.scale.lerp(keyframes[keyframe], weight); + } + Keyframes::Weights(keyframes) => { + if let Ok(morphs) = morphs { + let target_count = morphs.weights().len(); + lerp_morph_weights( + morphs.weights_mut(), + get_keyframe(target_count, keyframes, keyframe) + .iter() + .copied(), + weight, + ); + } + } + } + } + if let Some(animation_clip) = animations.get(&animation.animation_clip) { // We don't return early because seek_to() may have been called on the animation player. animation.update( @@ -648,31 +681,13 @@ fn apply_animation( }; // SAFETY: As above, there can't be other AnimationPlayers with this target so this fetch can't alias let mut morphs = unsafe { morphs.get_unchecked(target) }; + for curve in curves { + let len = curve.keyframe_timestamps.len(); + // 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 = transform.rotation.slerp(keyframes[0], weight); - } - Keyframes::Translation(keyframes) => { - transform.translation = - transform.translation.lerp(keyframes[0], weight); - } - Keyframes::Scale(keyframes) => { - transform.scale = transform.scale.lerp(keyframes[0], weight); - } - Keyframes::Weights(keyframes) => { - if let Ok(morphs) = &mut morphs { - let target_count = morphs.weights().len(); - lerp_morph_weights( - morphs.weights_mut(), - get_keyframe(target_count, keyframes, 0).iter().copied(), - weight, - ); - } - } - } + if len == 1 { + set_keyframe(curve, &mut transform, weight, &mut morphs, 0); continue; } @@ -682,10 +697,16 @@ fn apply_animation( .keyframe_timestamps .binary_search_by(|probe| probe.partial_cmp(&animation.seek_time).unwrap()) { - Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => continue, // this curve is finished + Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => { + set_keyframe(curve, &mut transform, weight, &mut morphs, 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(n) if n > curve.keyframe_timestamps.len() - 1 => { + set_keyframe(curve, &mut transform, weight, &mut morphs, len - 1); + continue; + } // this curve is finished Err(i) => i - 1, }; let ts_start = curve.keyframe_timestamps[step_start]; From 63b3857bfae8368ed521f87a632ed4757b76e5f0 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Jan 2024 14:15:13 -0500 Subject: [PATCH 2/5] Fix merge-conflict compiler errors by using an Option consistently --- 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 00e36eda166eb..076c90f345fe5 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -644,7 +644,7 @@ fn apply_animation( curve: &VariableCurve, transform: &mut Mut<'_, Transform>, weight: f32, - morphs: &mut Result, bevy_ecs::query::QueryEntityError>, + morphs: &mut Option>, keyframe: usize, ) { match &curve.keyframes { @@ -658,7 +658,7 @@ fn apply_animation( transform.scale = transform.scale.lerp(keyframes[keyframe], weight); } Keyframes::Weights(keyframes) => { - if let Ok(morphs) = morphs { + if let Some(morphs) = morphs { let target_count = morphs.weights().len(); lerp_morph_weights( morphs.weights_mut(), From 2d39f2097bce98fff045a6e3e46f0f25e604d1d6 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Jan 2024 14:20:27 -0500 Subject: [PATCH 3/5] Improve docs and naming --- crates/bevy_animation/src/lib.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 076c90f345fe5..648627d40911b 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -639,8 +639,8 @@ fn apply_animation( parents: &Query<(Has, Option<&Parent>)>, children: &Query<&Children>, ) { - /// Set transform to exactly the value of some keyframe. - fn set_keyframe( + /// Sets the [`Transform`] value to exactly the value it has at the provided keyframe. + fn set_transform_to_keyframe( curve: &VariableCurve, transform: &mut Mut<'_, Transform>, weight: f32, @@ -749,13 +749,25 @@ fn apply_animation( .binary_search_by(|probe| probe.partial_cmp(&animation.seek_time).unwrap()) { Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => { - set_keyframe(curve, &mut transform, weight, &mut morphs, len - 1); + set_transform_to_keyframe( + curve, + &mut transform, + weight, + &mut morphs, + 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 => { - set_keyframe(curve, &mut transform, weight, &mut morphs, len - 1); + set_transform_to_keyframe( + curve, + &mut transform, + weight, + &mut morphs, + len - 1, + ); continue; } // this curve is finished Err(i) => i - 1, From 54ffeda1a544d11b696cb291cfbcf1a0258d89e8 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Jan 2024 14:21:23 -0500 Subject: [PATCH 4/5] Split out into dedicated function --- crates/bevy_animation/src/lib.rs | 66 ++++++++++++++++---------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 648627d40911b..1f2496a2dd5ed 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -639,39 +639,6 @@ fn apply_animation( parents: &Query<(Has, Option<&Parent>)>, children: &Query<&Children>, ) { - /// Sets the [`Transform`] value to exactly the value it has at the provided keyframe. - fn set_transform_to_keyframe( - curve: &VariableCurve, - transform: &mut Mut<'_, Transform>, - weight: f32, - morphs: &mut Option>, - keyframe: usize, - ) { - match &curve.keyframes { - Keyframes::Rotation(keyframes) => { - transform.rotation = transform.rotation.slerp(keyframes[keyframe], weight); - } - Keyframes::Translation(keyframes) => { - transform.translation = transform.translation.lerp(keyframes[keyframe], weight); - } - Keyframes::Scale(keyframes) => { - transform.scale = transform.scale.lerp(keyframes[keyframe], weight); - } - Keyframes::Weights(keyframes) => { - if let Some(morphs) = morphs { - let target_count = morphs.weights().len(); - lerp_morph_weights( - morphs.weights_mut(), - get_keyframe(target_count, keyframes, keyframe) - .iter() - .copied(), - weight, - ); - } - } - } - } - if let Some(animation_clip) = animations.get(&animation.animation_clip) { // We don't return early because seek_to() may have been called on the animation player. animation.update( @@ -794,6 +761,39 @@ fn apply_animation( } } +/// Sets the [`Transform`] value to exactly the value it has at the provided keyframe. +fn set_transform_to_keyframe( + curve: &VariableCurve, + transform: &mut Mut<'_, Transform>, + weight: f32, + morphs: &mut Option>, + keyframe: usize, +) { + match &curve.keyframes { + Keyframes::Rotation(keyframes) => { + transform.rotation = transform.rotation.slerp(keyframes[keyframe], weight); + } + Keyframes::Translation(keyframes) => { + transform.translation = transform.translation.lerp(keyframes[keyframe], weight); + } + Keyframes::Scale(keyframes) => { + transform.scale = transform.scale.lerp(keyframes[keyframe], weight); + } + Keyframes::Weights(keyframes) => { + if let Some(morphs) = morphs { + let target_count = morphs.weights().len(); + lerp_morph_weights( + morphs.weights_mut(), + get_keyframe(target_count, keyframes, keyframe) + .iter() + .copied(), + weight, + ); + } + } + } +} + #[inline(always)] fn apply_keyframe( curve: &VariableCurve, From c1271019f904ac70ff8395d470b4a4df8884b931 Mon Sep 17 00:00:00 2001 From: Alice Cecile Date: Thu, 11 Jan 2024 14:36:43 -0500 Subject: [PATCH 5/5] Try and clean up / document what is going on --- crates/bevy_animation/src/lib.rs | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/bevy_animation/src/lib.rs b/crates/bevy_animation/src/lib.rs index 1f2496a2dd5ed..b769e06891ef1 100644 --- a/crates/bevy_animation/src/lib.rs +++ b/crates/bevy_animation/src/lib.rs @@ -709,13 +709,19 @@ fn apply_animation( continue; } - // Find the current keyframe + // Attempt to find the keyframe at or before the current time + // An Ok(keyframe_index) result means an exact result was found by binary search + // An Err result means the keyframe was not found, and the index is the keyframe // PERF: finding the current keyframe can be optimised - let step_start = match curve + let index = curve .keyframe_timestamps - .binary_search_by(|probe| probe.partial_cmp(&animation.seek_time).unwrap()) - { - Ok(n) if n >= curve.keyframe_timestamps.len() - 1 => { + .binary_search_by(|probe| probe.partial_cmp(&animation.seek_time).unwrap()); + + // Find the keyframe that our animation starts from + let step_start = match index { + // We could find an exact match, + // and the match was the last or second last keyframe + Ok(i) if i >= curve.keyframe_timestamps.len() - 1 => { set_transform_to_keyframe( curve, &mut transform, @@ -724,10 +730,15 @@ fn apply_animation( len - 1, ); continue; - } // this curve is finished + } + // We could find an exact match, + // and the match was not the last or second last keyframe Ok(i) => i, - Err(0) => continue, // this curve isn't started yet - Err(n) if n > curve.keyframe_timestamps.len() - 1 => { + // The animation was not started yet + Err(0) => continue, + // We could not find an exact match, + // and the timestamp was after the last keyframe + Err(i) if i > curve.keyframe_timestamps.len() - 1 => { set_transform_to_keyframe( curve, &mut transform, @@ -736,7 +747,9 @@ fn apply_animation( len - 1, ); continue; - } // this curve is finished + } + // We could not find an exact match, + // But we are Err(i) => i - 1, }; let ts_start = curve.keyframe_timestamps[step_start];