Skip to content

Conversation

@ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Jan 5, 2026

Objective

#22156 introduced a leak 😓 . Every time a text section with a font handle font source is updated in TextPipeLine::update_buffer, a new font with a new font ID is added to cosmic text's FontDb.

Fixes #22419

Solution

Remove font loading and asset-ID association from TextPipeline. Instead, add the font family name to the Font asset after loading. Then update_buffer can just use the family name from the asset.

Testing

Add this system to the text example (or any bevy app with updating text):

            .add_systems(Update, |font_system: Res<CosmicFontSystem>| {
                println!("fonts: {}", font_system.db().len());
            })

You should observe the count increasing with every text update on main. The count should be stable with this PR.

…, creating a new font with a new font ID to cosmic text's FontDb from the font asset each text update.

This commit removes the responsibility of loading fonts to cosmic text's database and associating them with an asset id from `TextPipeline`. Instead the font family name is added to the `Font` asset after loading.

Then in `TextPipeline::update_buffer` it just gets the family name from the asset and there's no possibility of a leak.
@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior P-Regression Functionality that used to work but no longer does. Add a test for this! 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 Jan 5, 2026
@ickshonpe ickshonpe changed the title After he new font families changes update_buffer now leaks font IDs… Fix font ID leak in TextPipeline Jan 5, 2026
@alice-i-cecile alice-i-cecile added the M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide label Jan 5, 2026
@ickshonpe ickshonpe added this to the 0.19 milestone Jan 5, 2026
@ickshonpe ickshonpe added the P-Critical This must be fixed immediately or contributors or users will be severely impacted label Jan 7, 2026
Copy link
Member

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

Oof, yeah okay. Nice fix! Makes sense.

@tychedelia tychedelia 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 Jan 7, 2026
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Jan 7, 2026
Merged via the queue into bevyengine:main with commit 59104ab Jan 7, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Text Rendering and layout for characters C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples M-Migration-Guide A breaking change to Bevy's public API that needs to be noted in a migration guide P-Critical This must be fixed immediately or contributors or users will be severely impacted 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

Development

Successfully merging this pull request may close these issues.

Font ID leak

3 participants