-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Remove FontAtlasSets
#21345
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.
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
Open
ickshonpe
wants to merge
20
commits into
bevyengine:main
Choose a base branch
from
ickshonpe:remove-font-atlas-set
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Remove FontAtlasSets
#21345
+206
−258
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…om (AssetId, size bits, FontSmoothing) to the vec of font atlases
dloukadakis
suggested changes
Oct 2, 2025
Co-authored-by: Dimitrios Loukadakis <[email protected]>
Co-authored-by: Dimitrios Loukadakis <[email protected]>
Co-authored-by: Dimitrios Loukadakis <[email protected]>
Co-authored-by: Dimitrios Loukadakis <[email protected]>
Co-authored-by: Dimitrios Loukadakis <[email protected]>
Co-authored-by: Dimitrios Loukadakis <[email protected]>
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-Code-Quality
A section of code that is hard to understand or change
D-Straightforward
Simple bug fixes and API improvements, docs, test and examples
S-Needs-Review
Needs reviewer attention (from anyone!) to move forward
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
The
FontAtlasSets
resource contains a map fromAssetId<Font>
s toFontAtlasSet
s.FontAtlasSet
has another map from aFontAtlasKey
(tuple of the font size bit cast into au32
andFontSmoothing
) to a vec that contains the actual font atlases for the font face. The redirection through the additional map doesn't serve much purpose, only individual font faces are looked up (except when freeing unused fonts), not the set of all font faces for a particular font.Solution
FontAtlasSet
.FontAtlasSets
toFontAtlasSet
.AssetId<Font>
toFontAtlasKey
.HashMap<FontAtlasKey, Vec<FontAtlas>>
FontAtlasSet
into free functions.Considered renaming
FontAlasSet
toFontAtlasMap
, but it doesn't seem necessary, and mathematically a map is a set so it's not incorrect.Testing
The output from all of the text examples should be unchanged.
Naive benchmarks suggest this is a modest improvement in performance at a very high load (5% ish), but this is more about reducing complexity to make more significant optimisations possible. Freeing the unused atlases when a font's last strong
Handle
is dropped will be slower, it has to filter all the font atlas vecs now instead of just removing the entry for the font asset, but this isn't done very often and the number of entries should be relatively low.