Skip to content

Text performance regression fix#24663

Merged
alice-i-cecile merged 20 commits into
bevyengine:mainfrom
ickshonpe:text_regression_fix
Jun 21, 2026
Merged

Text performance regression fix#24663
alice-i-cecile merged 20 commits into
bevyengine:mainfrom
ickshonpe:text_regression_fix

Conversation

@ickshonpe

@ickshonpe ickshonpe commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Objective

Text layout updates take much longer than in 0.18.

Solution

update_text_layout_info was building a new Swash Scaler for every run.

Instead, only build a new Scaler when it is needed to rasterize new glyphs.

Also added a clear method to FontAtlasSets. This is used by the many_text example with the --clear-font-atlases arg to bench glyph rasterization.

Testing

Includes a new text benchmark many_text:

cargo run --example many_text --release

On my computer on main it reports ~35fps, with this branch ~130fps.

@ickshonpe ickshonpe added this to the 0.19.1 milestone Jun 19, 2026
@ickshonpe ickshonpe added A-Text Rendering and layout for characters D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 19, 2026
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-24663

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

@rparrett

rparrett commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Out of curiosity, is this regression visible in many_text2d --recompute, many_glyphs --recompute_text, many_buttons --recompute_text?

@ickshonpe

ickshonpe commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Out of curiosity, is this regression visible in many_text2d --recompute, many_glyphs --recompute_text, many_buttons --recompute_text?

Should be, yes. Anything that calls update_text_layout_info to regenerate a text layout will build the unnecessary Scalers.

@ickshonpe ickshonpe added P-Regression Functionality that used to work but no longer does. Add a test for this! C-Performance A change motivated by improving speed, memory usage or compile times labels Jun 19, 2026
@alice-i-cecile alice-i-cecile requested a review from cart June 19, 2026 15:45
Trashtalk217
Trashtalk217 approved these changes Jun 21, 2026
View reviewed changes

@Trashtalk217 Trashtalk217 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've tested it locally and I went from 35fps to 110fps. Good job! Although I do wonder if it's possible to clean up the function a little. We're indented quite far here.

@kfc35 kfc35 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-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 21, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jun 21, 2026
Merged via the queue into bevyengine:main with commit 5baf000 Jun 21, 2026
48 checks passed
@kristoff3r kristoff3r mentioned this pull request Jun 22, 2026
Merged
pull Bot pushed a commit to octoape/bevy that referenced this pull request Jun 23, 2026
@kristoff3r
bevy_text: give swash a stable font id (bevyengine#24710)
097fc03 
# Objective

Disclaimer: AI was used to find/triage the issue, but I spent quite a
bit of time verifying everything. I am not a text rendering exptert
though.

While investigating the performance regression fixed in
bevyengine#24663 in my own project, I
stumbled upon another one related to caching and font hinting.

As mentioned in
https://docs.rs/swash/latest/swash/struct.FontRef.html#owning-your-fonts,
using the `FontRef` constructors implicitly generates new `CacheKey`
instances, which breaks the internal caching layer. Using the scale
builder without giving an id does this internally, which is a bit of a
footgun. I believe `cosmic_text` handled this before, and that this is a
regression from the move to `parley`.

When running `many_text2d --hinting` the startup time went from 7s to
500ms. I couldn't get a good tracy screenshot running with `--recompute`
because the fps on main is too low. Note that the measurements are based
on main before bevyengine#24663.

<img width="745" height="324" alt="screenshot-2026-06-22-165610"
src="https://github.com/user-attachments/assets/62881ddb-84eb-4b20-b26e-8d305c3bb99a"
/>
<img width="634" height="527" alt="screenshot-2026-06-22-165624"
src="https://github.com/user-attachments/assets/96a38ef4-d51a-4c5e-a518-2596b212e1f6"
/>

## Solution

Give the builder a stable font id. Someone should confirm if this is
indeed a valid ID to give to swash, or if I forgot to take some
feature(s) into account.

I also added a `hinting` flag to `many_text2d` that I used for testing,
seems useful to keep.

## Testing

Ran `many_text2d`, saw better performance and that the text looked
similar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cart cart Awaiting requested review from cart

2 more reviewers

@Zeophlite Zeophlite Zeophlite approved these changes

@Trashtalk217 Trashtalk217 Trashtalk217 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

A-Text Rendering and layout for characters C-Performance A change motivated by improving speed, memory usage or compile times D-Straightforward Simple bug fixes and API improvements, docs, test and examples P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it

Projects

None yet

Milestone

0.19.1

Development

Successfully merging this pull request may close these issues.

6 participants

@ickshonpe @rparrett @Zeophlite @Trashtalk217 @kfc35 @alice-i-cecile