3217f216aa
# Objective - Fixes #4996 ## Solution - Panicking on the global task pool is probably bad. This changes the panicking test to use a single threaded stage to run the test instead. - I checked the other #[should_panic] - I also added explicit ordering between the transform propagate system and the parent update system. The ambiguous ordering didn't seem to be causing problems, but the tests are probably more correct this way. The plugins that add these systems have an explicit ordering. I can remove this if necessary. ## Note I don't have a 100% mental model of why panicking is causing intermittent failures. It probably has to do with a task for one of the other tests landing on the panicking thread when it actually panics. Why this causes a problem I'm not sure, but this PR seems to fix things. ## Open questions - there are some other #[should_panic] tests that run on the task pool in stage.rs. I don't think we restart panicked threads, so this might be killing most of the threads on the pool. But since they're not causing test failures, we should probably decide what to do about that separately. The solution in this PR won't work since those tests are explicitly testing parallelism.
373 lines
12 KiB
Rust
373 lines
12 KiB
Rust
use crate::components::{GlobalTransform, Transform};
|
|
use bevy_ecs::prelude::{Changed, Entity, Query, With, Without};
|
|
use bevy_hierarchy::{Children, Parent};
|
|
|
|
/// Update [`GlobalTransform`] component of entities based on entity hierarchy and
|
|
/// [`Transform`] component.
|
|
pub fn transform_propagate_system(
|
|
mut root_query: Query<
|
|
(
|
|
Option<(&Children, Changed<Children>)>,
|
|
&Transform,
|
|
Changed<Transform>,
|
|
&mut GlobalTransform,
|
|
Entity,
|
|
),
|
|
Without<Parent>,
|
|
>,
|
|
mut transform_query: Query<(
|
|
&Transform,
|
|
Changed<Transform>,
|
|
&mut GlobalTransform,
|
|
&Parent,
|
|
)>,
|
|
children_query: Query<(&Children, Changed<Children>), (With<Parent>, With<GlobalTransform>)>,
|
|
) {
|
|
for (children, transform, transform_changed, mut global_transform, entity) in
|
|
root_query.iter_mut()
|
|
{
|
|
let mut changed = transform_changed;
|
|
if transform_changed {
|
|
*global_transform = GlobalTransform::from(*transform);
|
|
}
|
|
|
|
if let Some((children, changed_children)) = children {
|
|
// If our `Children` has changed, we need to recalculate everything below us
|
|
changed |= changed_children;
|
|
for child in children.iter() {
|
|
let _ = propagate_recursive(
|
|
&global_transform,
|
|
&mut transform_query,
|
|
&children_query,
|
|
*child,
|
|
entity,
|
|
changed,
|
|
);
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
fn propagate_recursive(
|
|
parent: &GlobalTransform,
|
|
transform_query: &mut Query<(
|
|
&Transform,
|
|
Changed<Transform>,
|
|
&mut GlobalTransform,
|
|
&Parent,
|
|
)>,
|
|
children_query: &Query<(&Children, Changed<Children>), (With<Parent>, With<GlobalTransform>)>,
|
|
entity: Entity,
|
|
expected_parent: Entity,
|
|
mut changed: bool,
|
|
// We use a result here to use the `?` operator. Ideally we'd use a try block instead
|
|
) -> Result<(), ()> {
|
|
let global_matrix = {
|
|
let (transform, transform_changed, mut global_transform, child_parent) =
|
|
transform_query.get_mut(entity).map_err(drop)?;
|
|
// Note that for parallelising, this check cannot occur here, since there is an `&mut GlobalTransform` (in global_transform)
|
|
assert_eq!(
|
|
child_parent.0, expected_parent,
|
|
"Malformed hierarchy. This probably means that your hierarchy has been improperly maintained, or contains a cycle"
|
|
);
|
|
changed |= transform_changed;
|
|
if changed {
|
|
*global_transform = parent.mul_transform(*transform);
|
|
}
|
|
*global_transform
|
|
};
|
|
|
|
let (children, changed_children) = children_query.get(entity).map_err(drop)?;
|
|
// If our `Children` has changed, we need to recalculate everything below us
|
|
changed |= changed_children;
|
|
for child in children.iter() {
|
|
let _ = propagate_recursive(
|
|
&global_matrix,
|
|
transform_query,
|
|
children_query,
|
|
*child,
|
|
entity,
|
|
changed,
|
|
);
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod test {
|
|
use bevy_app::prelude::*;
|
|
use bevy_ecs::prelude::*;
|
|
use bevy_ecs::system::CommandQueue;
|
|
use bevy_math::vec3;
|
|
|
|
use crate::components::{GlobalTransform, Transform};
|
|
use crate::systems::transform_propagate_system;
|
|
use crate::TransformBundle;
|
|
use bevy_hierarchy::{
|
|
parent_update_system, BuildChildren, BuildWorldChildren, Children, Parent,
|
|
};
|
|
|
|
#[test]
|
|
fn did_propagate() {
|
|
let mut world = World::default();
|
|
|
|
let mut update_stage = SystemStage::parallel();
|
|
update_stage.add_system(parent_update_system);
|
|
update_stage.add_system(transform_propagate_system.after(parent_update_system));
|
|
|
|
let mut schedule = Schedule::default();
|
|
schedule.add_stage("update", update_stage);
|
|
|
|
// Root entity
|
|
world
|
|
.spawn()
|
|
.insert_bundle(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)));
|
|
|
|
let mut children = Vec::new();
|
|
world
|
|
.spawn()
|
|
.insert_bundle(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)))
|
|
.with_children(|parent| {
|
|
children.push(
|
|
parent
|
|
.spawn_bundle(TransformBundle::from(Transform::from_xyz(0.0, 2.0, 0.)))
|
|
.id(),
|
|
);
|
|
children.push(
|
|
parent
|
|
.spawn_bundle(TransformBundle::from(Transform::from_xyz(0.0, 0.0, 3.)))
|
|
.id(),
|
|
);
|
|
});
|
|
schedule.run(&mut world);
|
|
|
|
assert_eq!(
|
|
*world.get::<GlobalTransform>(children[0]).unwrap(),
|
|
GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 2.0, 0.0)
|
|
);
|
|
|
|
assert_eq!(
|
|
*world.get::<GlobalTransform>(children[1]).unwrap(),
|
|
GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 0.0, 3.0)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn did_propagate_command_buffer() {
|
|
let mut world = World::default();
|
|
|
|
let mut update_stage = SystemStage::parallel();
|
|
update_stage.add_system(parent_update_system);
|
|
update_stage.add_system(transform_propagate_system.after(parent_update_system));
|
|
|
|
let mut schedule = Schedule::default();
|
|
schedule.add_stage("update", update_stage);
|
|
|
|
// Root entity
|
|
let mut queue = CommandQueue::default();
|
|
let mut commands = Commands::new(&mut queue, &world);
|
|
let mut children = Vec::new();
|
|
commands
|
|
.spawn_bundle(TransformBundle::from(Transform::from_xyz(1.0, 0.0, 0.0)))
|
|
.with_children(|parent| {
|
|
children.push(
|
|
parent
|
|
.spawn_bundle(TransformBundle::from(Transform::from_xyz(0.0, 2.0, 0.0)))
|
|
.id(),
|
|
);
|
|
children.push(
|
|
parent
|
|
.spawn_bundle(TransformBundle::from(Transform::from_xyz(0.0, 0.0, 3.0)))
|
|
.id(),
|
|
);
|
|
});
|
|
queue.apply(&mut world);
|
|
schedule.run(&mut world);
|
|
|
|
assert_eq!(
|
|
*world.get::<GlobalTransform>(children[0]).unwrap(),
|
|
GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 2.0, 0.0)
|
|
);
|
|
|
|
assert_eq!(
|
|
*world.get::<GlobalTransform>(children[1]).unwrap(),
|
|
GlobalTransform::from_xyz(1.0, 0.0, 0.0) * Transform::from_xyz(0.0, 0.0, 3.0)
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn correct_children() {
|
|
let mut world = World::default();
|
|
|
|
let mut update_stage = SystemStage::parallel();
|
|
update_stage.add_system(parent_update_system);
|
|
update_stage.add_system(transform_propagate_system.after(parent_update_system));
|
|
|
|
let mut schedule = Schedule::default();
|
|
schedule.add_stage("update", update_stage);
|
|
|
|
// Add parent entities
|
|
let mut command_queue = CommandQueue::default();
|
|
let mut commands = Commands::new(&mut command_queue, &world);
|
|
let mut children = Vec::new();
|
|
let parent = commands
|
|
.spawn()
|
|
.insert(Transform::from_xyz(1.0, 0.0, 0.0))
|
|
.id();
|
|
commands.entity(parent).with_children(|parent| {
|
|
children.push(
|
|
parent
|
|
.spawn()
|
|
.insert(Transform::from_xyz(0.0, 2.0, 0.0))
|
|
.id(),
|
|
);
|
|
children.push(
|
|
parent
|
|
.spawn()
|
|
.insert(Transform::from_xyz(0.0, 3.0, 0.0))
|
|
.id(),
|
|
);
|
|
});
|
|
command_queue.apply(&mut world);
|
|
schedule.run(&mut world);
|
|
|
|
assert_eq!(
|
|
world
|
|
.get::<Children>(parent)
|
|
.unwrap()
|
|
.iter()
|
|
.cloned()
|
|
.collect::<Vec<_>>(),
|
|
children,
|
|
);
|
|
|
|
// Parent `e1` to `e2`.
|
|
(*world.get_mut::<Parent>(children[0]).unwrap()).0 = children[1];
|
|
|
|
schedule.run(&mut world);
|
|
|
|
assert_eq!(
|
|
world
|
|
.get::<Children>(parent)
|
|
.unwrap()
|
|
.iter()
|
|
.cloned()
|
|
.collect::<Vec<_>>(),
|
|
vec![children[1]]
|
|
);
|
|
|
|
assert_eq!(
|
|
world
|
|
.get::<Children>(children[1])
|
|
.unwrap()
|
|
.iter()
|
|
.cloned()
|
|
.collect::<Vec<_>>(),
|
|
vec![children[0]]
|
|
);
|
|
|
|
assert!(world.despawn(children[0]));
|
|
|
|
schedule.run(&mut world);
|
|
|
|
assert_eq!(
|
|
world
|
|
.get::<Children>(parent)
|
|
.unwrap()
|
|
.iter()
|
|
.cloned()
|
|
.collect::<Vec<_>>(),
|
|
vec![children[1]]
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn correct_transforms_when_no_children() {
|
|
let mut app = App::new();
|
|
|
|
app.add_system(parent_update_system);
|
|
app.add_system(transform_propagate_system.after(parent_update_system));
|
|
|
|
let translation = vec3(1.0, 0.0, 0.0);
|
|
|
|
let parent = app
|
|
.world
|
|
.spawn()
|
|
.insert(Transform::from_translation(translation))
|
|
.insert(GlobalTransform::default())
|
|
.id();
|
|
|
|
let child = app
|
|
.world
|
|
.spawn()
|
|
.insert_bundle((
|
|
Transform::identity(),
|
|
GlobalTransform::default(),
|
|
Parent(parent),
|
|
))
|
|
.id();
|
|
|
|
let grandchild = app
|
|
.world
|
|
.spawn()
|
|
.insert_bundle((
|
|
Transform::identity(),
|
|
GlobalTransform::default(),
|
|
Parent(child),
|
|
))
|
|
.id();
|
|
|
|
app.update();
|
|
|
|
// check the `Children` structure is spawned
|
|
assert_eq!(&**app.world.get::<Children>(parent).unwrap(), &[child]);
|
|
assert_eq!(&**app.world.get::<Children>(child).unwrap(), &[grandchild]);
|
|
// Note that at this point, the `GlobalTransform`s will not have updated yet, due to `Commands` delay
|
|
app.update();
|
|
|
|
let mut state = app.world.query::<&GlobalTransform>();
|
|
for global in state.iter(&app.world) {
|
|
assert_eq!(
|
|
global,
|
|
&GlobalTransform {
|
|
translation,
|
|
..Default::default()
|
|
},
|
|
);
|
|
}
|
|
}
|
|
#[test]
|
|
#[should_panic]
|
|
fn panic_when_hierarchy_cycle() {
|
|
let mut world = World::default();
|
|
// This test is run on a single thread in order to avoid breaking the global task pool by panicking
|
|
// This fixes the flaky tests reported in https://github.com/bevyengine/bevy/issues/4996
|
|
let mut update_stage = SystemStage::single_threaded();
|
|
|
|
update_stage.add_system(parent_update_system);
|
|
update_stage.add_system(transform_propagate_system.after(parent_update_system));
|
|
|
|
let child = world
|
|
.spawn()
|
|
.insert_bundle((Transform::identity(), GlobalTransform::default()))
|
|
.id();
|
|
|
|
let grandchild = world
|
|
.spawn()
|
|
.insert_bundle((
|
|
Transform::identity(),
|
|
GlobalTransform::default(),
|
|
Parent(child),
|
|
))
|
|
.id();
|
|
world.spawn().insert_bundle((
|
|
Transform::default(),
|
|
GlobalTransform::default(),
|
|
Children::with(&[child]),
|
|
));
|
|
world.entity_mut(child).insert(Parent(grandchild));
|
|
|
|
update_stage.run(&mut world);
|
|
}
|
|
}
|