Skip to content

Conversation

ickshonpe
Copy link
Contributor

Objective

Both measure_text_system and text_system have ParamSet parameters each with three text queries. This complexity doesn't seem necessary and the ParamSet queries can be replaced with a single combined query.

Solution

Replace the ParamSets in measure_text_system and text_system with single combined queries.


Changelog

  • Replaced the measure_texture_system and text_system ParamSet parameters with single combined queries.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented May 5, 2023

Seems marginally more efficient as well:

cargo run --example many_buttons --profile stress-test --features trace_tracy -- recompute-text

text_system-many_buttons-stress-test-a616fa8f70054d1c504bc-vs-baece543878

@nicoburns nicoburns added A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change labels May 5, 2023
@alice-i-cecile alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it labels May 5, 2023
Copy link
Contributor

@konsti219 konsti219 left a comment

Choose a reason for hiding this comment

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

Using iterators is a bit shorter than a for loop

Comment on lines +87 to 89
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
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));

Comment on lines +162 to 164
for (entity, ..) in text_query.iter() {
text_queue.push(entity);
}
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));

@ickshonpe
Copy link
Contributor Author

I needed to include these changes in #8549, so I might as well close this if #8549 isn't controversial or blocked by anything.

@ickshonpe ickshonpe closed this May 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-UI Graphical user interfaces, styles, layouts, and widgets C-Code-Quality A section of code that is hard to understand or change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants