From e0a94abf1c9f380cc69ee01284ce9850c3d8a8e7 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 8 May 2023 14:57:52 +0100 Subject: [PATCH] Replace the local text queues in the text systems with flags stored in a component (#8549) # Objective `text_system` and `measure_text_system` both keep local queues to keep track of text node entities that need recomputations/remeasurement, which scales very badly with large numbers of text entities (O(n^2)) and makes the code quite difficult to understand. Also `text_system` filters for `Changed`, this isn't something that it should do. When a text node entity fails to be processed by `measure_text_system` because a font can't be found, the text node will still be added to `text_system`'s local queue for recomputation. `Text` should only ever be queued by `text_system` when a text node's geometry is modified or a new measure is added. ## Solution Remove the local text queues and use a component `TextFlags` to schedule remeasurements and recomputations. ## Changelog * Created a component `TextFlags` with fields `remeasure` and `recompute`, which can be used to schedule a text `remeasure` or `recomputation` respectively and added it to `TextBundle`. * Removed the local text queues from `measure_text_system` and `text_system` and instead use the `TextFlags` component to schedule remeasurements and recomputations. ## Migration Guide The component `TextFlags` has been added to `TextBundle`. --- crates/bevy_ui/src/node_bundles.rs | 5 +- crates/bevy_ui/src/widget/text.rs | 252 ++++++++++++++++++----------- 2 files changed, 166 insertions(+), 91 deletions(-) diff --git a/crates/bevy_ui/src/node_bundles.rs b/crates/bevy_ui/src/node_bundles.rs index 26c36fee19..dbd5f74f05 100644 --- a/crates/bevy_ui/src/node_bundles.rs +++ b/crates/bevy_ui/src/node_bundles.rs @@ -1,7 +1,7 @@ //! This module contains basic node bundles used to build UIs use crate::{ - widget::{Button, UiImageSize}, + widget::{Button, TextFlags, UiImageSize}, BackgroundColor, ContentSize, FocusPolicy, Interaction, Node, Style, UiImage, ZIndex, }; use bevy_ecs::bundle::Bundle; @@ -118,6 +118,8 @@ pub struct TextBundle { pub text: Text, /// Text layout information pub text_layout_info: TextLayoutInfo, + /// Text system flags + pub text_flags: TextFlags, /// The calculated size based on the given image pub calculated_size: ContentSize, /// Whether this node should block interaction with lower nodes @@ -148,6 +150,7 @@ impl Default for TextBundle { Self { text: Default::default(), text_layout_info: Default::default(), + text_flags: Default::default(), calculated_size: Default::default(), // Transparent background background_color: BackgroundColor(Color::NONE), diff --git a/crates/bevy_ui/src/widget/text.rs b/crates/bevy_ui/src/widget/text.rs index 9555935cad..f2a240faa6 100644 --- a/crates/bevy_ui/src/widget/text.rs +++ b/crates/bevy_ui/src/widget/text.rs @@ -1,11 +1,14 @@ use crate::{ContentSize, Measure, Node, UiScale}; use bevy_asset::Assets; use bevy_ecs::{ - entity::Entity, - query::{Changed, Or, With}, - system::{Local, ParamSet, Query, Res, ResMut}, + prelude::{Component, DetectChanges}, + query::With, + reflect::ReflectComponent, + system::{Local, Query, Res, ResMut}, + world::{Mut, Ref}, }; use bevy_math::Vec2; +use bevy_reflect::{std_traits::ReflectDefault, Reflect}; use bevy_render::texture::Image; use bevy_sprite::TextureAtlas; use bevy_text::{ @@ -19,6 +22,27 @@ fn scale_value(value: f32, factor: f64) -> f32 { (value as f64 * factor) as f32 } +/// Text system flags +/// +/// Used internally by [`measure_text_system`] and [`text_system`] to schedule text for processing. +#[derive(Component, Debug, Clone, Reflect)] +#[reflect(Component, Default)] +pub struct TextFlags { + /// If set a new measure function for the text node will be created + needs_new_measure_func: bool, + /// If set the text will be recomputed + needs_recompute: bool, +} + +impl Default for TextFlags { + fn default() -> Self { + Self { + needs_new_measure_func: true, + needs_recompute: true, + } + } +} + #[derive(Clone)] pub struct TextMeasure { pub info: TextMeasureInfo, @@ -54,20 +78,48 @@ impl Measure for TextMeasure { } } +#[inline] +fn create_text_measure( + fonts: &Assets, + text_pipeline: &mut TextPipeline, + scale_factor: f64, + text: Ref, + mut content_size: Mut, + mut text_flags: Mut, +) { + match text_pipeline.create_text_measure( + fonts, + &text.sections, + scale_factor, + text.alignment, + text.linebreak_behavior, + ) { + Ok(measure) => { + content_size.set(TextMeasure { info: measure }); + + // Text measure func created succesfully, so set `TextFlags` to schedule a recompute + text_flags.needs_new_measure_func = false; + text_flags.needs_recompute = true; + } + Err(TextError::NoSuchFont) => { + // Try again next frame + text_flags.needs_new_measure_func = true; + } + Err(e @ TextError::FailedToAddGlyph(_)) => { + panic!("Fatal error when processing text: {e}."); + } + }; +} + /// Creates a `Measure` for text nodes that allows the UI to determine the appropriate amount of space /// to provide for the text given the fonts, the text itself and the constraints of the layout. pub fn measure_text_system( - mut queued_text: Local>, mut last_scale_factor: Local, fonts: Res>, windows: Query<&Window, With>, ui_scale: Res, mut text_pipeline: ResMut, - mut text_queries: ParamSet<( - Query, With)>, - Query, With)>, - Query<(&Text, &mut ContentSize)>, - )>, + mut text_query: Query<(Ref, &mut ContentSize, &mut TextFlags), With>, ) { let window_scale_factor = windows .get_single() @@ -78,48 +130,86 @@ pub fn measure_text_system( #[allow(clippy::float_cmp)] if *last_scale_factor == scale_factor { - // Adds all entities where the text has changed to the local queue - for entity in text_queries.p0().iter() { - if !queued_text.contains(&entity) { - queued_text.push(entity); + // scale factor unchanged, only create new measure funcs for modified text + for (text, content_size, text_flags) in text_query.iter_mut() { + if text.is_changed() || text_flags.needs_new_measure_func { + create_text_measure( + &fonts, + &mut text_pipeline, + scale_factor, + text, + content_size, + text_flags, + ); } } } else { - // If the scale factor has changed, queue all text - for entity in text_queries.p1().iter() { - queued_text.push(entity); - } + // scale factor changed, create new measure funcs for all text *last_scale_factor = scale_factor; - } - if queued_text.is_empty() { - return; - } - - let mut new_queue = Vec::new(); - let mut query = text_queries.p2(); - for entity in queued_text.drain(..) { - if let Ok((text, mut content_size)) = query.get_mut(entity) { - match text_pipeline.create_text_measure( + for (text, content_size, text_flags) in text_query.iter_mut() { + create_text_measure( &fonts, - &text.sections, + &mut text_pipeline, scale_factor, - text.alignment, - text.linebreak_behavior, - ) { - Ok(measure) => { - content_size.set(TextMeasure { info: measure }); - } - Err(TextError::NoSuchFont) => { - new_queue.push(entity); - } - Err(e @ TextError::FailedToAddGlyph(_)) => { - panic!("Fatal error when processing text: {e}."); - } - }; + text, + content_size, + text_flags, + ); + } + } +} + +#[allow(clippy::too_many_arguments)] +#[inline] +fn queue_text( + fonts: &Assets, + text_pipeline: &mut TextPipeline, + font_atlas_warning: &mut FontAtlasWarning, + font_atlas_set_storage: &mut Assets, + texture_atlases: &mut Assets, + textures: &mut Assets, + text_settings: &TextSettings, + scale_factor: f64, + text: &Text, + node: Ref, + mut text_flags: Mut, + mut text_layout_info: Mut, +) { + // Skip the text node if it is waiting for a new measure func + if !text_flags.needs_new_measure_func { + let node_size = Vec2::new( + scale_value(node.size().x, scale_factor), + scale_value(node.size().y, scale_factor), + ); + + match text_pipeline.queue_text( + fonts, + &text.sections, + scale_factor, + text.alignment, + text.linebreak_behavior, + node_size, + font_atlas_set_storage, + texture_atlases, + textures, + text_settings, + font_atlas_warning, + YAxisOrientation::TopToBottom, + ) { + Err(TextError::NoSuchFont) => { + // There was an error processing the text layout, try again next frame + text_flags.needs_recompute = true; + } + Err(e @ TextError::FailedToAddGlyph(_)) => { + panic!("Fatal error when processing text: {e}."); + } + Ok(info) => { + *text_layout_info = info; + text_flags.needs_recompute = false; + } } } - *queued_text = new_queue; } /// Updates the layout and size information whenever the text or style is changed. @@ -131,7 +221,6 @@ pub fn measure_text_system( /// It does not modify or observe existing ones. #[allow(clippy::too_many_arguments)] pub fn text_system( - mut queued_text: Local>, mut textures: ResMut>, mut last_scale_factor: Local, fonts: Res>, @@ -142,11 +231,7 @@ pub fn text_system( mut texture_atlases: ResMut>, mut font_atlas_set_storage: ResMut>, mut text_pipeline: ResMut, - mut text_queries: ParamSet<( - Query, Changed)>>, - Query, With)>, - Query<(&Node, &Text, &mut TextLayoutInfo)>, - )>, + mut text_query: Query<(Ref, &Text, &mut TextLayoutInfo, &mut TextFlags)>, ) { // TODO: Support window-independent scaling: https://github.com/bevyengine/bevy/issues/5621 let window_scale_factor = windows @@ -156,58 +241,45 @@ pub fn text_system( let scale_factor = ui_scale.scale * window_scale_factor; - #[allow(clippy::float_cmp)] if *last_scale_factor == scale_factor { - // Adds all entities where the text or the style has changed to the local queue - for entity in text_queries.p0().iter() { - if !queued_text.contains(&entity) { - queued_text.push(entity); + // Scale factor unchanged, only recompute text for modified text nodes + for (node, text, text_layout_info, text_flags) in text_query.iter_mut() { + if node.is_changed() || text_flags.needs_recompute { + queue_text( + &fonts, + &mut text_pipeline, + &mut font_atlas_warning, + &mut font_atlas_set_storage, + &mut texture_atlases, + &mut textures, + &text_settings, + scale_factor, + text, + node, + text_flags, + text_layout_info, + ); } } } else { - // If the scale factor has changed, queue all text - for entity in text_queries.p1().iter() { - queued_text.push(entity); - } + // Scale factor changed, recompute text for all text nodes *last_scale_factor = scale_factor; - } - let mut new_queue = Vec::new(); - let mut text_query = text_queries.p2(); - for entity in queued_text.drain(..) { - if let Ok((node, text, mut text_layout_info)) = text_query.get_mut(entity) { - let node_size = Vec2::new( - scale_value(node.size().x, scale_factor), - scale_value(node.size().y, scale_factor), - ); - - match text_pipeline.queue_text( + for (node, text, text_layout_info, text_flags) in text_query.iter_mut() { + queue_text( &fonts, - &text.sections, - scale_factor, - text.alignment, - text.linebreak_behavior, - node_size, + &mut text_pipeline, + &mut font_atlas_warning, &mut font_atlas_set_storage, &mut texture_atlases, &mut textures, - text_settings.as_ref(), - &mut font_atlas_warning, - YAxisOrientation::TopToBottom, - ) { - Err(TextError::NoSuchFont) => { - // There was an error processing the text layout, let's add this entity to the - // queue for further processing - new_queue.push(entity); - } - Err(e @ TextError::FailedToAddGlyph(_)) => { - panic!("Fatal error when processing text: {e}."); - } - Ok(info) => { - *text_layout_info = info; - } - } + &text_settings, + scale_factor, + text, + node, + text_flags, + text_layout_info, + ); } } - *queued_text = new_queue; }