Skip to content

Commit

Permalink
Fix transform propagation of orphaned entities (#7264)
Browse files Browse the repository at this point in the history
# 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>
  • Loading branch information
3 people authored Apr 9, 2023
1 parent 0d971a6 commit f32ee63
Showing 1 changed file with 81 additions and 5 deletions.
86 changes: 81 additions & 5 deletions crates/bevy_transform/src/systems.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,39 @@ 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};

/// Update [`GlobalTransform`] component of entities that aren't in the hierarchy
///
/// 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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit f32ee63

Please sign in to comment.