From d354bfeebc318ae5ed93d007e6f2da36274430a5 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 30 May 2025 15:01:42 +0100 Subject: [PATCH] Split `detect_text_needs_rerender` into two systems `detect_text_root_needs_rerender` and `detect_text_spawn_needs_rerender`. This avoids checking `TextSpan` nodes for changes twice. --- crates/bevy_text/src/lib.rs | 32 +++++++++++-------- crates/bevy_text/src/text.rs | 59 ++++++++++++++++++++---------------- crates/bevy_ui/src/lib.rs | 12 +++++--- 3 files changed, 59 insertions(+), 44 deletions(-) diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs index 2bc74a1aa7..f9ad2634e3 100644 --- a/crates/bevy_text/src/lib.rs +++ b/crates/bevy_text/src/lib.rs @@ -117,19 +117,25 @@ impl Plugin for TextPlugin { .add_systems( PostUpdate, ( - remove_dropped_font_atlas_sets.before(AssetEventSystems), - detect_text_needs_rerender::, - update_text2d_layout - // Potential conflict: `Assets` - // In practice, they run independently since `bevy_render::camera_update_system` - // will only ever observe its own render target, and `update_text2d_layout` - // will never modify a pre-existing `Image` asset. - .ambiguous_with(CameraUpdateSystems), - calculate_bounds_text2d.in_set(VisibilitySystems::CalculateBounds), - ) - .chain() - .in_set(Text2dUpdateSystems) - .after(AnimationSystems), + ( + remove_dropped_font_atlas_sets.before(AssetEventSystems), + detect_text_root_needs_rerender::, + update_text2d_layout + // Potential conflict: `Assets` + // In practice, they run independently since `bevy_render::camera_update_system` + // will only ever observe its own render target, and `update_text2d_layout` + // will never modify a pre-existing `Image` asset. + .ambiguous_with(CameraUpdateSystems), + calculate_bounds_text2d.in_set(VisibilitySystems::CalculateBounds), + ) + .chain() + .in_set(Text2dUpdateSystems) + .after(AnimationSystems), + detect_text_span_needs_rerender + .before(Text2dUpdateSystems) + .ambiguous_with(CameraUpdateSystems) + .after(AnimationSystems), + ), ) .add_systems(Last, trim_cosmic_cache); diff --git a/crates/bevy_text/src/text.rs b/crates/bevy_text/src/text.rs index e9e78e3ed2..9be935533d 100644 --- a/crates/bevy_text/src/text.rs +++ b/crates/bevy_text/src/text.rs @@ -477,11 +477,10 @@ pub enum FontSmoothing { /// System that detects changes to text blocks and sets `ComputedTextBlock::should_rerender`. /// -/// Generic over the root text component and text span component. For example, [`Text2d`](crate::Text2d)/[`TextSpan`] for -/// 2d or `Text`/[`TextSpan`] for UI. -pub fn detect_text_needs_rerender( - changed_roots: Query< - Entity, +/// Generic over the root text component. +pub fn detect_text_root_needs_rerender( + mut changed_roots: Query< + &mut ComputedTextBlock, ( Or<( Changed, @@ -494,6 +493,30 @@ pub fn detect_text_needs_rerender( With, ), >, + roots_without_computed_block: Query, Without)>, +) { + // Root entity: + // - Root component changed. + // - TextFont on root changed. + // - TextLayout changed. + // - Root children changed (can include additions and removals). + for mut computed in changed_roots.iter_mut() { + computed.needs_rerender = true; + } + + for entity in roots_without_computed_block.iter() { + // If the root entity does not have a ComputedTextBlock, then it needs one. + // This can happen if the root was spawned without a ComputedTextBlock, or if it was removed. + once!(warn!( + "found entity {} with a root text component ({}) that has no ComputedTextBlock; this warning only prints once", + entity, + core::any::type_name::() + )); + } +} + +// System that detects changes to text spans and sets `ComputedTextBlock::should_rerender`. +pub fn detect_text_span_needs_rerender( changed_spans: Query< (Entity, Option<&ChildOf>, Has), ( @@ -514,20 +537,6 @@ pub fn detect_text_needs_rerender( Has, )>, ) { - // Root entity: - // - Root component changed. - // - TextFont on root changed. - // - TextLayout changed. - // - Root children changed (can include additions and removals). - for root in changed_roots.iter() { - let Ok((_, Some(mut computed), _)) = computed.get_mut(root) else { - once!(warn!("found entity {} with a root text component ({}) but no ComputedTextBlock; this warning only \ - prints once", root, core::any::type_name::())); - continue; - }; - computed.needs_rerender = true; - } - // Span entity: // - Span component changed. // - Span TextFont changed. @@ -535,16 +544,15 @@ pub fn detect_text_needs_rerender( for (entity, maybe_span_child_of, has_text_block) in changed_spans.iter() { if has_text_block { once!(warn!("found entity {} with a TextSpan that has a TextLayout, which should only be on root \ - text entities (that have {}); this warning only prints once", - entity, core::any::type_name::())); + text entities; this warning only prints once", + entity)); } let Some(span_child_of) = maybe_span_child_of else { once!(warn!( "found entity {} with a TextSpan that has no parent; it should have an ancestor \ - with a root text component ({}); this warning only prints once", - entity, - core::any::type_name::() + with a root text component; this warning only prints once", + entity )); continue; }; @@ -573,9 +581,8 @@ pub fn detect_text_needs_rerender( let Some(next_child_of) = maybe_child_of else { once!(warn!( "found entity {} with a TextSpan that has no ancestor with the root text \ - component ({}); this warning only prints once", + component; this warning only prints once", entity, - core::any::type_name::() )); break; }; diff --git a/crates/bevy_ui/src/lib.rs b/crates/bevy_ui/src/lib.rs index a216c4220b..4a415f0847 100644 --- a/crates/bevy_ui/src/lib.rs +++ b/crates/bevy_ui/src/lib.rs @@ -34,6 +34,7 @@ mod render; mod stack; mod ui_node; +use bevy_text::detect_text_span_needs_rerender; pub use focus::*; pub use geometry::*; pub use gradients::*; @@ -218,7 +219,7 @@ impl Plugin for UiPlugin { let ui_layout_system_config = ui_layout_system_config // Text and Text2D operate on disjoint sets of entities .ambiguous_with(bevy_text::update_text2d_layout) - .ambiguous_with(bevy_text::detect_text_needs_rerender::); + .ambiguous_with(bevy_text::detect_text_root_needs_rerender::); app.add_systems( PostUpdate, @@ -289,26 +290,27 @@ fn build_text_interop(app: &mut App) { PostUpdate, ( ( - bevy_text::detect_text_needs_rerender::, + bevy_text::detect_text_root_needs_rerender::, widget::measure_text_system, ) .chain() .in_set(UiSystems::Content) // Text and Text2d are independent. - .ambiguous_with(bevy_text::detect_text_needs_rerender::) + .ambiguous_with(bevy_text::detect_text_root_needs_rerender::) // Potential conflict: `Assets` // Since both systems will only ever insert new [`Image`] assets, // they will never observe each other's effects. .ambiguous_with(bevy_text::update_text2d_layout) // We assume Text is on disjoint UI entities to ImageNode and UiTextureAtlasImage // FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481. - .ambiguous_with(widget::update_image_content_size_system), + .ambiguous_with(widget::update_image_content_size_system) + .after(detect_text_span_needs_rerender), widget::text_system .in_set(UiSystems::PostLayout) .after(bevy_text::remove_dropped_font_atlas_sets) .before(bevy_asset::AssetEventSystems) // Text2d and bevy_ui text are entirely on separate entities - .ambiguous_with(bevy_text::detect_text_needs_rerender::) + .ambiguous_with(bevy_text::detect_text_root_needs_rerender::) .ambiguous_with(bevy_text::update_text2d_layout) .ambiguous_with(bevy_text::calculate_bounds_text2d), ),