Put font data into Arc to reduce memory consumption - #5276
Merged
emilk merged 1 commit intoNov 1, 2024
Merged
Conversation
Contributor
Author
|
E.g. it keeps a copy in egui/crates/egui/src/context.rs Line 599 in 707cd03 from: egui/crates/egui/src/context.rs Lines 587 to 601 in 707cd03 |
|
Preview available at https://egui-pr-preview.github.io/pr/5276-pr_reduce_memory_usage_font_definitions |
Owner
|
Let's wait until after #5228 is merged |
StarStarJ
force-pushed
the
pr_reduce_memory_usage_font_definitions
branch
3 times, most recently
from
October 26, 2024 13:59
26d52dc to
e624075
Compare
emilk
reviewed
Oct 29, 2024
emilk
left a comment
Owner
There was a problem hiding this comment.
I think a simpler and smaller change would be
@@ -243,7 +243,7 @@ pub struct FontDefinitions {
/// List of font names and their definitions.
///
/// `epaint` has built-in-default for these, but you can override them if you like.
- pub font_data: BTreeMap<String, FontData>,
+ pub font_data: BTreeMap<String, Arc<FontData>>,
StarStarJ
force-pushed
the
pr_reduce_memory_usage_font_definitions
branch
from
October 29, 2024 14:07
e624075 to
18be843
Compare
FontDefinitions cloning, wrap it in Arc
Contributor
Author
Sounds good aswell |
StarStarJ
force-pushed
the
pr_reduce_memory_usage_font_definitions
branch
2 times, most recently
from
October 29, 2024 14:20
753d87c to
522b1ad
Compare
And additionally make cloning `FontDefinitions` cheaper.
StarStarJ
force-pushed
the
pr_reduce_memory_usage_font_definitions
branch
from
October 29, 2024 21:57
522b1ad to
bef08ec
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
egui never accesses the
FontDefinitions' member fields mutably, except infonts_tweak_uiwhere it cloned theFontDefinitionsobject anyway.This patch reduces system memory consumption for shared font definitions.
And also removes some overhead from copying (e.g. for the per
pixel_per_pointsfont atlas)Also it allows to keep a copy of the font definitions outside of egui.
In my App that uses international fonts:
Before:
New:
Note: If
Arcis not wanted, then it could ofc be abstracted away.I know this is quite a breaking change API wise, but would like to hear your opinion.