Skip to content
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

Jump to bottom

[Merged by Bors] - Text pipeline benchmark #7845

Closed
wants to merge 5 commits into from
Closed

[Merged by Bors] - Text pipeline benchmark #7845

wants to merge 5 commits into from

Conversation

ickshonpe
Copy link
Contributor

Objective

Simple text pipeline benchmark. It's quite expensive but current examples don't capture the performance of queue_text as it only runs on changes to the text.

@ickshonpe
Added an example "text_pipeline" to the stress tests.
380a369
@ickshonpe
Changed:
44975bb 
* The test text's `TextSection`s have varying font sizes.
* build-example-pages & fmt --all
@ickshonpe ickshonpe closed this Feb 28, 2023
@ickshonpe ickshonpe reopened this Feb 28, 2023
@ickshonpe
Copy link
Contributor Author

ickshonpe commented Feb 28, 2023 •
edited
Loading

Doesn't work because text isn't recomputed on changes to Text2dBounds.
Fix: #7846

@alice-i-cecile alice-i-cecile added C-Performance A change motivated by improving speed, memory usage or compile times C-Testing A change that impacts how we test Bevy or how users test their apps A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets S-Blocked This cannot move forward until something else changes and removed S-Blocked This cannot move forward until something else changes labels Feb 28, 2023
alice-i-cecile
alice-i-cecile approved these changes Feb 28, 2023
View reviewed changes
Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

Definitely pretty artificial, but it's nice to be able to isolate these things.

@ickshonpe
Copy link
Contributor Author

Definitely pretty artificial, but it's nice to be able to isolate these things.

It's weird I was worried I was being silly PRing this as it is really trivial, but it actually seems to cover some important edge cases that were hiding bugs.

superdump
superdump approved these changes Mar 4, 2023
View reviewed changes
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot pushed a commit that referenced this pull request Mar 4, 2023
@ickshonpe
Text pipeline benchmark (#7845)
465fff2 
# Objective

Simple text pipeline benchmark. It's quite expensive but current examples don't capture the performance of `queue_text` as it only runs on changes to the text.
@bors
Copy link
Contributor

bors bot commented Mar 4, 2023

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Text pipeline benchmark [Merged by Bors] - Text pipeline benchmark Mar 4, 2023
@bors bors bot closed this Mar 4, 2023
Shfty pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
@ickshonpe @Shfty
Text pipeline benchmark (bevyengine#7845)
4b6f822 
# Objective

Simple text pipeline benchmark. It's quite expensive but current examples don't capture the performance of `queue_text` as it only runs on changes to the text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@superdump superdump superdump approved these changes

@alice-i-cecile alice-i-cecile alice-i-cecile approved these changes

Assignees
No one assigned
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Performance A change motivated by improving speed, memory usage or compile times C-Testing A change that impacts how we test Bevy or how users test their apps
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ickshonpe @superdump @alice-i-cecile