Skip to content

Commit

Permalink
Use Ref instead of &T and Changed<T> (bevyengine#7175)
Browse files Browse the repository at this point in the history
# Objective
Follow up bevyengine#7097. Use `Ref<T>` instead of `&T` and the change detection query params.

## Solution
Replace them.
  • Loading branch information
james7132 authored and alradish committed Jan 22, 2023
1 parent 77f7a4f commit e540eff
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 30 deletions.
11 changes: 4 additions & 7 deletions crates/bevy_text/src/text2d.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use bevy_asset::Assets;
use bevy_ecs::{
bundle::Bundle,
change_detection::{DetectChanges, Ref},
component::Component,
entity::Entity,
event::EventReader,
query::Changed,
reflect::ReflectComponent,
system::{Commands, Local, Query, Res, ResMut},
};
Expand Down Expand Up @@ -166,8 +166,7 @@ pub fn update_text2d_layout(
mut text_pipeline: ResMut<TextPipeline>,
mut text_query: Query<(
Entity,
Changed<Text>,
&Text,
Ref<Text>,
Option<&Text2dBounds>,
&mut Text2dSize,
Option<&mut TextLayoutInfo>,
Expand All @@ -177,10 +176,8 @@ pub fn update_text2d_layout(
let factor_changed = scale_factor_changed.iter().last().is_some();
let scale_factor = windows.scale_factor(WindowId::primary());

for (entity, text_changed, text, maybe_bounds, mut calculated_size, text_layout_info) in
&mut text_query
{
if factor_changed || text_changed || queue.remove(&entity) {
for (entity, text, maybe_bounds, mut calculated_size, text_layout_info) in &mut text_query {
if factor_changed || text.is_changed() || queue.remove(&entity) {
let text_bounds = match maybe_bounds {
Some(bounds) => Vec2::new(
scale_value(bounds.size.x, scale_factor),
Expand Down
40 changes: 17 additions & 23 deletions crates/bevy_transform/src/systems.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::components::{GlobalTransform, Transform};
use bevy_ecs::prelude::{Changed, Entity, Query, With, Without};
use bevy_ecs::{
change_detection::Ref,
prelude::{Changed, DetectChanges, Entity, Query, With, Without},
};
use bevy_hierarchy::{Children, Parent};

/// Update [`GlobalTransform`] component of entities that aren't in the hierarchy
Expand All @@ -24,32 +27,26 @@ pub fn sync_simple_transforms(
/// to propagate transforms correctly.
pub fn propagate_transforms(
mut root_query: Query<
(
Entity,
&Children,
&Transform,
Changed<Transform>,
Changed<Children>,
&mut GlobalTransform,
),
(Entity, Ref<Children>, Ref<Transform>, &mut GlobalTransform),
Without<Parent>,
>,
transform_query: Query<(&Transform, Changed<Transform>, &mut GlobalTransform), With<Parent>>,
transform_query: Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>,
parent_query: Query<&Parent>,
children_query: Query<(&Children, Changed<Children>), (With<Parent>, With<GlobalTransform>)>,
children_query: Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>,
) {
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 changed, children_changed, mut global_transform)| {
|(entity, children, transform, mut global_transform)| {
let mut 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_changed;
changed |= children.is_changed();

for child in children.iter() {
// SAFETY:
Expand Down Expand Up @@ -87,12 +84,9 @@ pub fn propagate_transforms(
/// nor any of its descendants.
unsafe fn propagate_recursive(
parent: &GlobalTransform,
unsafe_transform_query: &Query<
(&Transform, Changed<Transform>, &mut GlobalTransform),
With<Parent>,
>,
unsafe_transform_query: &Query<(Ref<Transform>, &mut GlobalTransform), With<Parent>>,
parent_query: &Query<&Parent>,
children_query: &Query<(&Children, Changed<Children>), (With<Parent>, With<GlobalTransform>)>,
children_query: &Query<Ref<Children>, (With<Parent>, With<GlobalTransform>)>,
expected_parent: Entity,
entity: Entity,
mut changed: bool,
Expand All @@ -106,7 +100,7 @@ unsafe fn propagate_recursive(
);

let global_matrix = {
let Ok((transform, transform_changed, mut global_transform)) =
let Ok((transform, mut global_transform)) =
// 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
Expand Down Expand Up @@ -137,19 +131,19 @@ unsafe fn propagate_recursive(
return;
};

changed |= transform_changed;
changed |= transform.is_changed();
if changed {
*global_transform = parent.mul_transform(*transform);
}
*global_transform
};

let Ok((children, changed_children)) = children_query.get(entity) else {
let Ok(children) = children_query.get(entity) else {
return
};
// If our `Children` has changed, we need to recalculate everything below us
changed |= changed_children;
for child in children {
changed |= children.is_changed();
for child in &children {
// SAFETY: The caller guarantees that `unsafe_transform_query` will not be fetched
// for any descendants of `entity`, so it is safe to call `propagate_recursive` for each child.
unsafe {
Expand Down

0 comments on commit e540eff

Please sign in to comment.