Skip to content

Commit

Permalink
Limit FontAtlasSets (bevyengine#5708)
Browse files Browse the repository at this point in the history
# Objective

Fixes bevyengine#5636
Summary: The FontAtlasSet caches generated font textures per font size. Since font size can be any arbitrary floating point number it is possible for the user to generate thousands of font texture inadvertently by changing the font size over time. This results in a memory leak as these generated font textures fill the available memory. 

## Solution

We limit the number of possible font sizes that we will cache and throw an error if the user attempts to generate more. This error encourages the user to use alternative, less performance intensive methods to accomplish the same goal. If the user requires more font sizes and the alternative solutions wont work there is now a TextSettings Resource that the user can set to configure this limit. 

---

## Changelog

The number of cached font sizes per font is now limited with a default limit of 100 font sizes per font. This limit is configurable via the new TextSettings struct.
  • Loading branch information
xtr3m3nerd authored and james7132 committed Oct 19, 2022
1 parent 7ab599f commit a65efb9
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 10 deletions.
2 changes: 2 additions & 0 deletions crates/bevy_text/src/error.rs
Expand Up @@ -7,4 +7,6 @@ pub enum TextError {
NoSuchFont,
#[error("failed to add glyph to newly-created atlas {0:?}")]
FailedToAddGlyph(GlyphId),
#[error("exceeded {0:?} available TextAltases for font. This can be caused by using an excessive number of font sizes. If you are changing font sizes dynamically consider using Transform::scale to modify the size. If you need more font sizes modify TextSettings.max_font_atlases." )]
ExceedMaxTextAtlases(usize),
}
28 changes: 26 additions & 2 deletions crates/bevy_text/src/font_atlas_set.rs
@@ -1,4 +1,4 @@
use crate::{error::TextError, Font, FontAtlas};
use crate::{error::TextError, Font, FontAtlas, TextSettings};
use ab_glyph::{GlyphId, OutlinedGlyph, Point};
use bevy_asset::{Assets, Handle};
use bevy_math::Vec2;
Expand All @@ -14,6 +14,7 @@ type FontSizeKey = FloatOrd;
#[uuid = "73ba778b-b6b5-4f45-982d-d21b6b86ace2"]
pub struct FontAtlasSet {
font_atlases: HashMap<FontSizeKey, Vec<FontAtlas>>,
queue: Vec<FontSizeKey>,
}

#[derive(Debug, Clone)]
Expand All @@ -26,6 +27,7 @@ impl Default for FontAtlasSet {
fn default() -> Self {
FontAtlasSet {
font_atlases: HashMap::with_capacity_and_hasher(1, Default::default()),
queue: Vec::new(),
}
}
}
Expand All @@ -50,7 +52,22 @@ impl FontAtlasSet {
texture_atlases: &mut Assets<TextureAtlas>,
textures: &mut Assets<Image>,
outlined_glyph: OutlinedGlyph,
text_settings: &TextSettings,
) -> Result<GlyphAtlasInfo, TextError> {
if !text_settings.allow_dynamic_font_size {
if self.font_atlases.len() >= text_settings.max_font_atlases.get() {
return Err(TextError::ExceedMaxTextAtlases(
text_settings.max_font_atlases.get(),
));
}
} else {
// Clear last space in queue to make room for new font size
while self.queue.len() >= text_settings.max_font_atlases.get() - 1 {
if let Some(font_size_key) = self.queue.pop() {
self.font_atlases.remove(&font_size_key);
}
}
}
let glyph = outlined_glyph.glyph();
let glyph_id = glyph.id;
let glyph_position = glyph.position;
Expand All @@ -65,6 +82,7 @@ impl FontAtlasSet {
Vec2::splat(512.0),
)]
});
self.queue.insert(0, FloatOrd(font_size));
let glyph_texture = Font::get_outlined_glyph_texture(outlined_glyph);
let add_char_to_font_atlas = |atlas: &mut FontAtlas| -> bool {
atlas.add_glyph(
Expand Down Expand Up @@ -106,11 +124,17 @@ impl FontAtlasSet {
}

pub fn get_glyph_atlas_info(
&self,
&mut self,
font_size: f32,
glyph_id: GlyphId,
position: Point,
) -> Option<GlyphAtlasInfo> {
// Move to front of used queue.
let some_index = self.queue.iter().position(|x| *x == FloatOrd(font_size));
if let Some(index) = some_index {
let key = self.queue.remove(index);
self.queue.insert(0, key);
}
self.font_atlases
.get(&FloatOrd(font_size))
.and_then(|font_atlases| {
Expand Down
11 changes: 9 additions & 2 deletions crates/bevy_text/src/glyph_brush.rs
Expand Up @@ -7,7 +7,7 @@ use glyph_brush_layout::{
FontId, GlyphPositioner, Layout, SectionGeometry, SectionGlyph, SectionText, ToSectionText,
};

use crate::{error::TextError, Font, FontAtlasSet, GlyphAtlasInfo, TextAlignment};
use crate::{error::TextError, Font, FontAtlasSet, GlyphAtlasInfo, TextAlignment, TextSettings};

pub struct GlyphBrush {
fonts: Vec<FontArc>,
Expand Down Expand Up @@ -43,6 +43,7 @@ impl GlyphBrush {
Ok(section_glyphs)
}

#[allow(clippy::too_many_arguments)]
pub fn process_glyphs(
&self,
glyphs: Vec<SectionGlyph>,
Expand All @@ -51,6 +52,7 @@ impl GlyphBrush {
fonts: &Assets<Font>,
texture_atlases: &mut Assets<TextureAtlas>,
textures: &mut Assets<Image>,
text_settings: &TextSettings,
) -> Result<Vec<PositionedGlyph>, TextError> {
if glyphs.is_empty() {
return Ok(Vec::new());
Expand Down Expand Up @@ -104,7 +106,12 @@ impl GlyphBrush {
.get_glyph_atlas_info(section_data.2, glyph_id, glyph_position)
.map(Ok)
.unwrap_or_else(|| {
font_atlas_set.add_glyph_to_atlas(texture_atlases, textures, outlined_glyph)
font_atlas_set.add_glyph_to_atlas(
texture_atlases,
textures,
outlined_glyph,
text_settings,
)
})?;

let texture_atlas = texture_atlases.get(&atlas_info.texture_atlas).unwrap();
Expand Down
23 changes: 22 additions & 1 deletion crates/bevy_text/src/lib.rs
Expand Up @@ -28,14 +28,34 @@ pub mod prelude {

use bevy_app::prelude::*;
use bevy_asset::AddAsset;
use bevy_ecs::schedule::ParallelSystemDescriptorCoercion;
use bevy_ecs::{schedule::ParallelSystemDescriptorCoercion, system::Resource};
use bevy_render::{RenderApp, RenderStage};
use bevy_sprite::SpriteSystem;
use bevy_window::ModifiesWindows;
use std::num::NonZeroUsize;

#[derive(Default)]
pub struct TextPlugin;

/// [`TextPlugin`] settings
#[derive(Resource)]
pub struct TextSettings {
/// Maximum number of font atlases supported in a ['FontAtlasSet']
pub max_font_atlases: NonZeroUsize,
/// Allows font size to be set dynamically exceeding the amount set in max_font_atlases.
/// Note each font size has to be generated which can have a strong performance impact.
pub allow_dynamic_font_size: bool,
}

impl Default for TextSettings {
fn default() -> Self {
Self {
max_font_atlases: NonZeroUsize::new(16).unwrap(),
allow_dynamic_font_size: false,
}
}
}

impl Plugin for TextPlugin {
fn build(&self, app: &mut App) {
app.add_asset::<Font>()
Expand All @@ -46,6 +66,7 @@ impl Plugin for TextPlugin {
.register_type::<VerticalAlign>()
.register_type::<HorizontalAlign>()
.init_asset_loader::<FontLoader>()
.init_resource::<TextSettings>()
.insert_resource(TextPipeline::default())
.add_system_to_stage(
CoreStage::PostUpdate,
Expand Down
4 changes: 3 additions & 1 deletion crates/bevy_text/src/pipeline.rs
Expand Up @@ -11,7 +11,7 @@ use glyph_brush_layout::{FontId, SectionText};

use crate::{
error::TextError, glyph_brush::GlyphBrush, scale_value, Font, FontAtlasSet, PositionedGlyph,
TextAlignment, TextSection,
TextAlignment, TextSection, TextSettings,
};

#[derive(Default, Resource)]
Expand Down Expand Up @@ -49,6 +49,7 @@ impl TextPipeline {
font_atlas_set_storage: &mut Assets<FontAtlasSet>,
texture_atlases: &mut Assets<TextureAtlas>,
textures: &mut Assets<Image>,
text_settings: &TextSettings,
) -> Result<TextLayoutInfo, TextError> {
let mut scaled_fonts = Vec::new();
let sections = sections
Expand Down Expand Up @@ -103,6 +104,7 @@ impl TextPipeline {
fonts,
texture_atlases,
textures,
text_settings,
)?;

Ok(TextLayoutInfo { glyphs, size })
Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_text/src/text2d.rs
Expand Up @@ -23,7 +23,7 @@ use bevy_window::{WindowId, WindowScaleFactorChanged, Windows};

use crate::{
Font, FontAtlasSet, HorizontalAlign, Text, TextError, TextLayoutInfo, TextPipeline,
VerticalAlign,
TextSettings, VerticalAlign,
};

/// The calculated size of text drawn in 2D scene.
Expand Down Expand Up @@ -153,6 +153,7 @@ pub fn update_text2d_layout(
mut textures: ResMut<Assets<Image>>,
fonts: Res<Assets<Font>>,
windows: Res<Windows>,
text_settings: Res<TextSettings>,
mut scale_factor_changed: EventReader<WindowScaleFactorChanged>,
mut texture_atlases: ResMut<Assets<TextureAtlas>>,
mut font_atlas_set_storage: ResMut<Assets<FontAtlasSet>>,
Expand Down Expand Up @@ -190,13 +191,15 @@ pub fn update_text2d_layout(
&mut *font_atlas_set_storage,
&mut *texture_atlases,
&mut *textures,
text_settings.as_ref(),
) {
Err(TextError::NoSuchFont) => {
// There was an error processing the text layout, let's add this entity to the
// queue for further processing
queue.insert(entity);
}
Err(e @ TextError::FailedToAddGlyph(_)) => {
Err(e @ TextError::FailedToAddGlyph(_))
| Err(e @ TextError::ExceedMaxTextAtlases(_)) => {
panic!("Fatal error when processing text: {}.", e);
}
Ok(info) => {
Expand Down
7 changes: 5 additions & 2 deletions crates/bevy_ui/src/widget/text.rs
Expand Up @@ -8,7 +8,7 @@ use bevy_ecs::{
use bevy_math::Vec2;
use bevy_render::texture::Image;
use bevy_sprite::TextureAtlas;
use bevy_text::{Font, FontAtlasSet, Text, TextError, TextLayoutInfo, TextPipeline};
use bevy_text::{Font, FontAtlasSet, Text, TextError, TextLayoutInfo, TextPipeline, TextSettings};
use bevy_window::Windows;

#[derive(Debug, Default)]
Expand Down Expand Up @@ -44,6 +44,7 @@ pub fn text_system(
mut textures: ResMut<Assets<Image>>,
fonts: Res<Assets<Font>>,
windows: Res<Windows>,
text_settings: Res<TextSettings>,
ui_scale: Res<UiScale>,
mut texture_atlases: ResMut<Assets<TextureAtlas>>,
mut font_atlas_set_storage: ResMut<Assets<FontAtlasSet>>,
Expand Down Expand Up @@ -116,13 +117,15 @@ pub fn text_system(
&mut *font_atlas_set_storage,
&mut *texture_atlases,
&mut *textures,
text_settings.as_ref(),
) {
Err(TextError::NoSuchFont) => {
// There was an error processing the text layout, let's add this entity to the
// queue for further processing
new_queue.push(entity);
}
Err(e @ TextError::FailedToAddGlyph(_)) => {
Err(e @ TextError::FailedToAddGlyph(_))
| Err(e @ TextError::ExceedMaxTextAtlases(_)) => {
panic!("Fatal error when processing text: {}.", e);
}
Ok(info) => {
Expand Down

0 comments on commit a65efb9

Please sign in to comment.