[Merged by Bors] - Improve change detection behavior for transform propagation

crates/bevy_transform/src/systems.rs

@@ -27,31 +27,33 @@ pub fn sync_simple_transforms(
 /// to propagate transforms correctly.
 pub fn propagate_transforms(
     mut root_query: Query<
-        (Entity, Ref<Children>, Ref<Transform>, &mut GlobalTransform),
+        (Entity, &Children, Ref<Transform>, &mut GlobalTransform),
         Without<Parent>,
     >,
-    transform_query: Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>,
-    parent_query: Query<&Parent>,
-    children_query: Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>,
+    transform_query: Query<(Ref<Transform>, &mut GlobalTransform, Option<&Children>), With<Parent>>,
+    parent_query: Query<(Entity, Ref<Parent>)>,
 ) {
     root_query.par_for_each_mut(
         // The differing depths and sizes of hierarchy trees causes the work for each root to be
         // different. A batch size of 1 ensures that each tree gets it's own task and multiple
         // large trees are not clumped together.
         1,
         |(entity, children, transform, mut global_transform)| {
-            let mut changed = transform.is_changed();
+            let changed = transform.is_changed();
             if changed {
                 *global_transform = GlobalTransform::from(*transform);
             }
 
-            // If our `Children` has changed, we need to recalculate everything below us
-            changed |= children.is_changed();
-
-            for child in children.iter() {
+            for (child, actual_parent) in parent_query.iter_many(children) {
+                assert_eq!(
+                    actual_parent.get(), entity,
+                    "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
+                );
                 // SAFETY:
-                // - We may operate as if the hierarchy is consistent, since `propagate_recursive` will panic before continuing
-                //   to propagate if it encounters an entity with inconsistent parentage.
+                // - `child` must have consistent parentage, or the above assertion would panic.
+                // 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,
                 //   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.
@@ -60,10 +62,8 @@ pub fn propagate_transforms(
                         &global_transform,
                         &transform_query,
                         &parent_query,
-                        &children_query,
-                        entity,
-                        *child,
-                        changed,
+                        child,
+                        changed || actual_parent.is_changed(),
                     );
                 }
             }
@@ -75,35 +75,30 @@ pub fn propagate_transforms(
 ///
 /// # Panics
 ///
-/// If `entity` or any of its descendants have a malformed hierarchy.
-/// The panic will occur before propagating the transforms of any malformed entities and their descendants.
+/// If `entity`'s descendants have a malformed hierarchy, this function will panic occur before propagating
+/// the transforms of any malformed entities and their descendants.
 ///
 /// # 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.
+/// - 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(
     parent: &GlobalTransform,
-    unsafe_transform_query: &Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>,
-    parent_query: &Query<&Parent>,
-    children_query: &Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>,
-    expected_parent: Entity,
+    transform_query: &Query<
+        (Ref<Transform>, &mut GlobalTransform, Option<&Children>),
+        With<Parent>,
+    >,
+    parent_query: &Query<(Entity, Ref<Parent>)>,
     entity: Entity,
     mut changed: bool,
 ) {
-    let Ok(actual_parent) = parent_query.get(entity) else {
-        panic!("Propagated child for {entity:?} has no Parent component!");
-    };
-    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)) =
+    let (global_matrix, children) = {
+        let Ok((transform, mut global_transform, children)) =
             // SAFETY: This call cannot create aliased mutable references.
             //   - 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.
             //
             // For example, consider the following malformed hierarchy:
@@ -127,34 +122,35 @@ unsafe fn propagate_recursive(
             //
             // Even if these A and B start two separate tasks running in parallel, one of them will panic before attempting
             // to mutably access E.
-            (unsafe { unsafe_transform_query.get_unchecked(entity) }) else {
+            (unsafe { transform_query.get_unchecked(entity) }) else {
                 return;
             };
 
         changed |= transform.is_changed();
         if changed {
             *global_transform = parent.mul_transform(*transform);
         }
-        *global_transform
+        (*global_transform, children)
     };
 
-    let Ok(children) = children_query.get(entity) else {
-        return
-    };
-    // If our `Children` has changed, we need to recalculate everything below us
-    changed |= children.is_changed();
-    for child in &children {
-        // SAFETY: The caller guarantees that `unsafe_transform_query` will not be fetched
+    let Some(children) = children else { return };
+    for (child, actual_parent) in parent_query.iter_many(children) {
+        assert_eq!(
+            actual_parent.get(), entity,
+            "Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
+        );
+        // 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.
+        //
+        // The above assertion ensures that each child has one and only one unique parent throughout the
+        // entire hierarchy.
         unsafe {
             propagate_recursive(
                 &global_matrix,
-                unsafe_transform_query,
+                transform_query,
                 parent_query,
-                children_query,
-                entity,
-                *child,
-                changed,
+                child,
+                changed || actual_parent.is_changed(),
             );
         }
     }