Skip to content

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Sep 25, 2025

Objective

Fixes #21210

Solution

Use a least recently used cache (suggested by @viridia). Don't free unused font atlases immediately, only once the font count is higher than the max fonts limit. Font atlases are never freed while their font is in use.

Performance Notes

For the sake of simplicity, plan to leave most of these for follow up PRs:

  • The LRU cache implementation isn't optimal, I just want to keep things simple for now as so many different text changes are needed.

  • The list of in use fonts is rebuilt every frame, this should be avoidable using some combination of change detection, observers, and reference counting. For most applications this won't be a problem though, as the numbers are relatively low.

  • Probably there should be a memory use limit and a hard limit to the total number of fonts.

Testing

Seems to work, except for TextSpans with Text2d.

Showcase

@ickshonpe ickshonpe added A-Text Rendering and layout for characters C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-Rendering Drawing game state to the screen labels Sep 25, 2025
@ickshonpe ickshonpe marked this pull request as ready for review September 25, 2025 10:40
@alice-i-cecile alice-i-cecile added this to the 0.18 milestone Sep 26, 2025
@viridia
Copy link
Contributor

viridia commented Sep 26, 2025

Is there an easier way to review a stacked PR? Having to mentally subtract out all the diffs from the previous change is a chore.

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Sep 26, 2025

Is there an easier way to review a stacked PR? Having to mentally subtract out all the diffs from the previous change is a chore.

I'm not sure, but all these other PRs are based on the ComputedTextStyle PR. If we can get that merged first, everything else will be a lot easier to review. I thought about combining everything into one big PR, but some of the changes are more controversial, whereas the ComputedTextStyle changes seem like an obvious improvement that we really need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-Text Rendering and layout for characters C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times M-Needs-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide M-Needs-Release-Note Work that should be called out in the blog due to impact S-Needs-Review Needs reviewer attention (from anyone!) to move forward
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

Free unused font atlases
3 participants