Mirrors/bevy
2
0
Fork 0
mirror of https://github.com/bevyengine/bevy synced 2024-09-20 14:32:06 +00:00
Code Issues Projects Releases Packages Wiki Activity

Improve change detection behavior for transform propagation (#6870)

Browse source 
# Objective
Fix #4647. If any child is changed, or even reordered, `Changed<Children>` is true, which causes transform propagation to propagate changes to all siblings of a changed child, even if they don't need to be.

## Solution
As `Parent` and `Children` are updated in tandem in hierarchy commands after #4800. `Changed<Parent>` is true on the child when `Changed<Children>` is true on the parent. However, unlike checking children, checking `Changed<Parent>` is only localized to the current entity and will not force propagation to the siblings.

Also took the opportunity to change propagation to use `Query::iter_many` instead of repeated `Query::get` calls. Should cut a bit of the overhead out of propagation. This means we won't panic when there isn't a `Parent` on the child, just skip over it.

The tests from #4608 still pass, so the change detection here still works just fine under this approach.
This commit is contained in:
James Liu 2023-01-17 22:26:51 +00:00
parent 0ca9c618e1
commit 7d0edbc4d6
1 changed files with 43 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

90
crates/bevy_transform/src/systems.rs
View file

@ -27,12 +27,11 @@ pub fn sync_simple_transforms(
/// to propagate transforms correctly. /// to propagate transforms correctly.
pub fn propagate_transforms( pub fn propagate_transforms(
mut root_query: Query< mut root_query: Query<
(Entity, Ref<Children>, Ref<Transform>, &mut GlobalTransform), (Entity, &Children, Ref<Transform>, &mut GlobalTransform),
Without<Parent>, Without<Parent>,
>, >,
transform_query: Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>, transform_query: Query<(Ref<Transform>, &mut GlobalTransform, Option<&Children>), With<Parent>>,
parent_query: Query<&Parent>, parent_query: Query<(Entity, Ref<Parent>)>,
children_query: Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>,
) { ) {
root_query.par_for_each_mut( root_query.par_for_each_mut(
// The differing depths and sizes of hierarchy trees causes the work for each root to be // The differing depths and sizes of hierarchy trees causes the work for each root to be
@ -40,18 +39,21 @@ pub fn propagate_transforms(
// large trees are not clumped together. // large trees are not clumped together.
1, 1,
|(entity, children, transform, mut global_transform)| { |(entity, children, transform, mut global_transform)| {
let mut changed = transform.is_changed(); let changed = transform.is_changed();
if changed { if changed {
*global_transform = GlobalTransform::from(*transform); *global_transform = GlobalTransform::from(*transform);
} }
// If our `Children` has changed, we need to recalculate everything below us for (child, actual_parent) in parent_query.iter_many(children) {
changed |= children.is_changed(); assert_eq!(
actual_parent.get(), entity,
for child in children.iter() { "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
);
// SAFETY: // SAFETY:
// - We may operate as if the hierarchy is consistent, since `propagate_recursive` will panic before continuing // - `child` must have consistent parentage, or the above assertion would panic.
// to propagate if it encounters an entity with inconsistent parentage. // Since `child` is parented to a root entity, the entire hierarchy leading to it is consistent.
// - We may operate as if all descendants are consistent, since `propagate_recursive` will panic before
// continuing to propagate if it encounters an entity with inconsistent parentage.
// - Since each root entity is unique and the hierarchy is consistent and forest-like, // - Since each root entity is unique and the hierarchy is consistent and forest-like,
// other root entities' `propagate_recursive` calls will not conflict with this one. // other root entities' `propagate_recursive` calls will not conflict with this one.
// - Since this is the only place where `transform_query` gets used, there will be no conflicting fetches elsewhere. // - Since this is the only place where `transform_query` gets used, there will be no conflicting fetches elsewhere.
@ -60,10 +62,8 @@ pub fn propagate_transforms(
&global_transform, &global_transform,
&transform_query, &transform_query,
&parent_query, &parent_query,
&children_query, child,
entity, changed || actual_parent.is_changed(),
*child,
changed,
); );
} }
} }
@ -75,35 +75,30 @@ pub fn propagate_transforms(
/// ///
/// # Panics /// # Panics
/// ///
/// If `entity` or any of its descendants have a malformed hierarchy. /// If `entity`'s descendants have a malformed hierarchy, this function will panic occur before propagating
/// The panic will occur before propagating the transforms of any malformed entities and their descendants. /// the transforms of any malformed entities and their descendants.
/// ///
/// # Safety /// # Safety
/// ///
/// While this function is running, `unsafe_transform_query` must not have any fetches for `entity`, /// - While this function is running, `transform_query` must not have any fetches for `entity`,
/// nor any of its descendants. /// nor any of its descendants.
/// - The caller must ensure that the hierarchy leading to `entity`
/// is well-formed and must remain as a tree or a forest. Each entity must have at most one parent.
unsafe fn propagate_recursive( unsafe fn propagate_recursive(
parent: &GlobalTransform, parent: &GlobalTransform,
unsafe_transform_query: &Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>, transform_query: &Query<
parent_query: &Query<&Parent>, (Ref<Transform>, &mut GlobalTransform, Option<&Children>),
children_query: &Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>, With<Parent>,
expected_parent: Entity, >,
parent_query: &Query<(Entity, Ref<Parent>)>,
entity: Entity, entity: Entity,
mut changed: bool, mut changed: bool,
) { ) {
let Ok(actual_parent) = parent_query.get(entity) else { let (global_matrix, children) = {
panic!("Propagated child for {entity:?} has no Parent component!"); let Ok((transform, mut global_transform, children)) =
};
assert_eq!(
actual_parent.get(), expected_parent,
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
);
let global_matrix = {
let Ok((transform, mut global_transform)) =
// SAFETY: This call cannot create aliased mutable references. // SAFETY: This call cannot create aliased mutable references.
// - The top level iteration parallelizes on the roots of the hierarchy. // - The top level iteration parallelizes on the roots of the hierarchy.
// - The above assertion ensures that each child has one and only one unique parent throughout the entire // - The caller ensures that each child has one and only one unique parent throughout the entire
// hierarchy. // hierarchy.
// //
// For example, consider the following malformed hierarchy: // For example, consider the following malformed hierarchy:
@ -127,7 +122,7 @@ unsafe fn propagate_recursive(
// //
// Even if these A and B start two separate tasks running in parallel, one of them will panic before attempting // Even if these A and B start two separate tasks running in parallel, one of them will panic before attempting
// to mutably access E. // to mutably access E.
(unsafe { unsafe_transform_query.get_unchecked(entity) }) else { (unsafe { transform_query.get_unchecked(entity) }) else {
return; return;
}; };
@ -135,26 +130,27 @@ unsafe fn propagate_recursive(
if changed { if changed {
*global_transform = parent.mul_transform(*transform); *global_transform = parent.mul_transform(*transform);
} }
*global_transform (*global_transform, children)
}; };
let Ok(children) = children_query.get(entity) else { let Some(children) = children else { return };
return for (child, actual_parent) in parent_query.iter_many(children) {
}; assert_eq!(
// If our `Children` has changed, we need to recalculate everything below us actual_parent.get(), entity,
changed |= children.is_changed(); "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
for child in &children { );
// SAFETY: The caller guarantees that `unsafe_transform_query` will not be fetched // SAFETY: The caller guarantees that `transform_query` will not be fetched
// for any descendants of `entity`, so it is safe to call `propagate_recursive` for each child. // for any descendants of `entity`, so it is safe to call `propagate_recursive` for each child.
//
// The above assertion ensures that each child has one and only one unique parent throughout the
// entire hierarchy.
unsafe { unsafe {
propagate_recursive( propagate_recursive(
&global_matrix, &global_matrix,
unsafe_transform_query, transform_query,
parent_query, parent_query,
children_query, child,
entity, changed || actual_parent.is_changed(),
*child,
changed,
); );
} }
} }