Fix transform propagation of orphaned entities (#7264)
# Objective - Fix #7263 This has nothing to do with #7024. This is for the case where the user opted to **not** keep the same global transform on update. ## Solution - Add a `RemovedComponent<Parent>` to `propagate_transforms` - Add a `RemovedComponent<Parent>` and `Local<Vec<Entity>>` to `sync_simple_transforms` - Add test to make sure all of this works. ### Performance note This should only incur a cost in cases where a parent is removed. A minimal overhead (one look up in the `removed_components` sparse set) per root entities without children which transform didn't change. A `Vec` the size of the largest number of entities removed with a `Parent` component in a single frame, and a binary search on a `Vec` per root entities. It could slow up considerably in situations where a lot of entities are orphaned consistently during every frame, since `sync_simple_transforms` is not parallel. But in this situation, it is likely that the overhead of archetype updates overwhelms everything. --- ## Changelog - Fix the `GlobalTransform` not getting updated when `Parent` is removed ## Migration Guide - If you called `bevy_transform::systems::sync_simple_transforms` and `bevy_transform::systems::propagate_transforms` (which is not re-exported by bevy) you need to account for the additional `RemovedComponents<Parent>` parameter. --------- Co-authored-by: vyb <vyb@users.noreply.github.com> Co-authored-by: JoJoJet <21144246+JoJoJet@users.noreply.github.com>
This commit is contained in:
		
							parent
							
								
									0d971a63e4
								
							
						
					
					
						commit
						f32ee63fb1
					
				@ -2,6 +2,8 @@ use crate::components::{GlobalTransform, Transform};
 | 
			
		||||
use bevy_ecs::{
 | 
			
		||||
    change_detection::Ref,
 | 
			
		||||
    prelude::{Changed, DetectChanges, Entity, Query, With, Without},
 | 
			
		||||
    removal_detection::RemovedComponents,
 | 
			
		||||
    system::{Local, ParamSet},
 | 
			
		||||
};
 | 
			
		||||
use bevy_hierarchy::{Children, Parent};
 | 
			
		||||
 | 
			
		||||
@ -9,16 +11,30 @@ use bevy_hierarchy::{Children, Parent};
 | 
			
		||||
///
 | 
			
		||||
/// Third party plugins should ensure that this is used in concert with [`propagate_transforms`].
 | 
			
		||||
pub fn sync_simple_transforms(
 | 
			
		||||
    mut query: Query<
 | 
			
		||||
        (&Transform, &mut GlobalTransform),
 | 
			
		||||
        (Changed<Transform>, Without<Parent>, Without<Children>),
 | 
			
		||||
    >,
 | 
			
		||||
    mut query: ParamSet<(
 | 
			
		||||
        Query<
 | 
			
		||||
            (&Transform, &mut GlobalTransform),
 | 
			
		||||
            (Changed<Transform>, Without<Parent>, Without<Children>),
 | 
			
		||||
        >,
 | 
			
		||||
        Query<(Ref<Transform>, &mut GlobalTransform), Without<Children>>,
 | 
			
		||||
    )>,
 | 
			
		||||
    mut orphaned: RemovedComponents<Parent>,
 | 
			
		||||
) {
 | 
			
		||||
    // Update changed entities.
 | 
			
		||||
    query
 | 
			
		||||
        .p0()
 | 
			
		||||
        .par_iter_mut()
 | 
			
		||||
        .for_each_mut(|(transform, mut global_transform)| {
 | 
			
		||||
            *global_transform = GlobalTransform::from(*transform);
 | 
			
		||||
        });
 | 
			
		||||
    // Update orphaned entities.
 | 
			
		||||
    let mut query = query.p1();
 | 
			
		||||
    let mut iter = query.iter_many_mut(orphaned.iter());
 | 
			
		||||
    while let Some((transform, mut global_transform)) = iter.fetch_next() {
 | 
			
		||||
        if !transform.is_changed() {
 | 
			
		||||
            *global_transform = GlobalTransform::from(*transform);
 | 
			
		||||
        }
 | 
			
		||||
    }
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
/// Update [`GlobalTransform`] component of entities based on entity hierarchy and
 | 
			
		||||
@ -30,12 +46,17 @@ pub fn propagate_transforms(
 | 
			
		||||
        (Entity, &Children, Ref<Transform>, &mut GlobalTransform),
 | 
			
		||||
        Without<Parent>,
 | 
			
		||||
    >,
 | 
			
		||||
    mut orphaned: RemovedComponents<Parent>,
 | 
			
		||||
    transform_query: Query<(Ref<Transform>, &mut GlobalTransform, Option<&Children>), With<Parent>>,
 | 
			
		||||
    parent_query: Query<(Entity, Ref<Parent>)>,
 | 
			
		||||
    mut orphaned_entities: Local<Vec<Entity>>,
 | 
			
		||||
) {
 | 
			
		||||
    orphaned_entities.clear();
 | 
			
		||||
    orphaned_entities.extend(orphaned.iter());
 | 
			
		||||
    orphaned_entities.sort_unstable();
 | 
			
		||||
    root_query.par_iter_mut().for_each_mut(
 | 
			
		||||
        |(entity, children, transform, mut global_transform)| {
 | 
			
		||||
            let changed = transform.is_changed();
 | 
			
		||||
            let changed = transform.is_changed() || orphaned_entities.binary_search(&entity).is_ok();
 | 
			
		||||
            if changed {
 | 
			
		||||
                *global_transform = GlobalTransform::from(*transform);
 | 
			
		||||
            }
 | 
			
		||||
@ -165,6 +186,61 @@ mod test {
 | 
			
		||||
    use crate::TransformBundle;
 | 
			
		||||
    use bevy_hierarchy::{BuildChildren, BuildWorldChildren, Children, Parent};
 | 
			
		||||
 | 
			
		||||
    #[test]
 | 
			
		||||
    fn correct_parent_removed() {
 | 
			
		||||
        ComputeTaskPool::init(TaskPool::default);
 | 
			
		||||
        let mut world = World::default();
 | 
			
		||||
        let offset_global_transform =
 | 
			
		||||
            |offset| GlobalTransform::from(Transform::from_xyz(offset, offset, offset));
 | 
			
		||||
        let offset_transform =
 | 
			
		||||
            |offset| TransformBundle::from_transform(Transform::from_xyz(offset, offset, offset));
 | 
			
		||||
 | 
			
		||||
        let mut schedule = Schedule::new();
 | 
			
		||||
        schedule.add_systems((sync_simple_transforms, propagate_transforms));
 | 
			
		||||
 | 
			
		||||
        let mut command_queue = CommandQueue::default();
 | 
			
		||||
        let mut commands = Commands::new(&mut command_queue, &world);
 | 
			
		||||
        let root = commands.spawn(offset_transform(3.3)).id();
 | 
			
		||||
        let parent = commands.spawn(offset_transform(4.4)).id();
 | 
			
		||||
        let child = commands.spawn(offset_transform(5.5)).id();
 | 
			
		||||
        commands.entity(parent).set_parent(root);
 | 
			
		||||
        commands.entity(child).set_parent(parent);
 | 
			
		||||
        command_queue.apply(&mut world);
 | 
			
		||||
        schedule.run(&mut world);
 | 
			
		||||
 | 
			
		||||
        assert_eq!(
 | 
			
		||||
            world.get::<GlobalTransform>(parent).unwrap(),
 | 
			
		||||
            &offset_global_transform(4.4 + 3.3),
 | 
			
		||||
            "The transform systems didn't run, ie: `GlobalTransform` wasn't updated",
 | 
			
		||||
        );
 | 
			
		||||
 | 
			
		||||
        // Remove parent of `parent`
 | 
			
		||||
        let mut command_queue = CommandQueue::default();
 | 
			
		||||
        let mut commands = Commands::new(&mut command_queue, &world);
 | 
			
		||||
        commands.entity(parent).remove_parent();
 | 
			
		||||
        command_queue.apply(&mut world);
 | 
			
		||||
        schedule.run(&mut world);
 | 
			
		||||
 | 
			
		||||
        assert_eq!(
 | 
			
		||||
            world.get::<GlobalTransform>(parent).unwrap(),
 | 
			
		||||
            &offset_global_transform(4.4),
 | 
			
		||||
            "The global transform of an orphaned entity wasn't updated properly",
 | 
			
		||||
        );
 | 
			
		||||
 | 
			
		||||
        // Remove parent of `child`
 | 
			
		||||
        let mut command_queue = CommandQueue::default();
 | 
			
		||||
        let mut commands = Commands::new(&mut command_queue, &world);
 | 
			
		||||
        commands.entity(child).remove_parent();
 | 
			
		||||
        command_queue.apply(&mut world);
 | 
			
		||||
        schedule.run(&mut world);
 | 
			
		||||
 | 
			
		||||
        assert_eq!(
 | 
			
		||||
            world.get::<GlobalTransform>(child).unwrap(),
 | 
			
		||||
            &offset_global_transform(5.5),
 | 
			
		||||
            "The global transform of an orphaned entity wasn't updated properly",
 | 
			
		||||
        );
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    #[test]
 | 
			
		||||
    fn did_propagate() {
 | 
			
		||||
        ComputeTaskPool::init(TaskPool::default);
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user