Skip to content

Simplify text system queries #8547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

Simplify text system queries #8547

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
a8a820c
Use a single query instead of a `ParamSet` for `measure_text_system`
ickshonpe May 5, 2023
dfca89c
Replaced the `ParamSet` parameter with a single query in `text_system`.
ickshonpe May 5, 2023
4e1ade4
cargo fmt --all
ickshonpe May 5, 2023
9a424c7
The change detection predicate was checking for `!queued_text.contain…
ickshonpe May 5, 2023
b0620d0
removed left over debug variable
ickshonpe May 5, 2023
686e109
Renamed the `Local` parameter `queued_text` to `text_queue` in `measu…
ickshonpe May 5, 2023
baece54
fixed imports
ickshonpe May 5, 2023
d69d8ca
Renamed `new_queue` to `new_text_queue`.
ickshonpe May 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 29 additions & 37 deletions crates/bevy_ui/src/widget/text.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ 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::DetectChanges,
query::With,
system::{Local, Query, Res, ResMut},
world::Ref,
};
use bevy_math::Vec2;
use bevy_render::texture::Image;
Expand Down Expand Up @@ -57,17 +59,13 @@ impl Measure for TextMeasure {
/// 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<Vec<Entity>>,
mut text_queue: Local<Vec<Entity>>,
mut last_scale_factor: Local<f64>,
fonts: Res<Assets<Font>>,
windows: Query<&Window, With<PrimaryWindow>>,
ui_scale: Res<UiScale>,
mut text_pipeline: ResMut<TextPipeline>,
mut text_queries: ParamSet<(
Query<Entity, (Changed<Text>, With<Node>)>,
Query<Entity, (With<Text>, With<Node>)>,
Query<(&Text, &mut ContentSize)>,
)>,
mut text_query: Query<(Entity, Ref<Text>, &mut ContentSize), With<Node>>,
) {
let window_scale_factor = windows
.get_single()
Expand All @@ -79,27 +77,26 @@ 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);
for (entity, text, ..) in text_query.iter() {
if text.is_changed() && !text_queue.contains(&entity) {
text_queue.push(entity);
}
}
} else {
// If the scale factor has changed, queue all text
for entity in text_queries.p1().iter() {
queued_text.push(entity);
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
Comment on lines +87 to 89
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
text_queue.extend(text_query.iter().map(|(entity, ..)| entity));

*last_scale_factor = scale_factor;
}

if queued_text.is_empty() {
if text_queue.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) {
let mut new_text_queue = Vec::new();
for entity in text_queue.drain(..) {
if let Ok((_, text, mut content_size)) = text_query.get_mut(entity) {
match text_pipeline.create_text_measure(
&fonts,
&text.sections,
Expand All @@ -111,15 +108,15 @@ pub fn measure_text_system(
content_size.set(TextMeasure { info: measure });
}
Err(TextError::NoSuchFont) => {
new_queue.push(entity);
new_text_queue.push(entity);
}
Err(e @ TextError::FailedToAddGlyph(_)) => {
panic!("Fatal error when processing text: {e}.");
}
};
}
}
*queued_text = new_queue;
*text_queue = new_text_queue;
}

/// Updates the layout and size information whenever the text or style is changed.
Expand All @@ -131,7 +128,7 @@ 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<Vec<Entity>>,
mut text_queue: Local<Vec<Entity>>,
mut textures: ResMut<Assets<Image>>,
mut last_scale_factor: Local<f64>,
fonts: Res<Assets<Font>>,
Expand All @@ -142,11 +139,7 @@ pub fn text_system(
mut texture_atlases: ResMut<Assets<TextureAtlas>>,
mut font_atlas_set_storage: ResMut<Assets<FontAtlasSet>>,
mut text_pipeline: ResMut<TextPipeline>,
mut text_queries: ParamSet<(
Query<Entity, Or<(Changed<Text>, Changed<Node>)>>,
Query<Entity, (With<Text>, With<Node>)>,
Query<(&Node, &Text, &mut TextLayoutInfo)>,
)>,
mut text_query: Query<(Entity, Ref<Node>, Ref<Text>, &mut TextLayoutInfo)>,
) {
// TODO: Support window-independent scaling: https://github.com/bevyengine/bevy/issues/5621
let window_scale_factor = windows
Expand All @@ -159,23 +152,22 @@ pub fn text_system(
#[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);
for (entity, node, text, ..) in text_query.iter() {
if (node.is_changed() || text.is_changed()) && !text_queue.contains(&entity) {
text_queue.push(entity);
}
}
} else {
// If the scale factor has changed, queue all text
for entity in text_queries.p1().iter() {
queued_text.push(entity);
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
Comment on lines +162 to 164
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
text_queue.extend(text_query.iter().map(|(entity, ..)| entity));

*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 mut new_text_queue = Vec::new();
for entity in text_queue.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),
Expand All @@ -198,7 +190,7 @@ pub fn text_system(
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);
new_text_queue.push(entity);
}
Err(e @ TextError::FailedToAddGlyph(_)) => {
panic!("Fatal error when processing text: {e}.");
Expand All @@ -209,5 +201,5 @@ pub fn text_system(
}
}
}
*queued_text = new_queue;
*text_queue = new_text_queue;
}