From 5dbd61334c972971906ee91d24e25fda9f991afa Mon Sep 17 00:00:00 2001 From: xtr3m3nerd Date: Mon, 19 Sep 2022 16:09:50 +0000 Subject: [PATCH] Limit FontAtlasSets (#5708) # Objective Fixes #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. --- crates/bevy_text/src/error.rs | 2 ++ crates/bevy_text/src/font_atlas_set.rs | 28 ++++++++++++++++++++++++-- crates/bevy_text/src/glyph_brush.rs | 11 ++++++++-- crates/bevy_text/src/lib.rs | 23 ++++++++++++++++++++- crates/bevy_text/src/pipeline.rs | 4 +++- crates/bevy_text/src/text2d.rs | 7 +++++-- crates/bevy_ui/src/widget/text.rs | 7 +++++-- 7 files changed, 72 insertions(+), 10 deletions(-) diff --git a/crates/bevy_text/src/error.rs b/crates/bevy_text/src/error.rs index 1bb7cf1253581..bf6e4e774cc47 100644 --- a/crates/bevy_text/src/error.rs +++ b/crates/bevy_text/src/error.rs @@ -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), } diff --git a/crates/bevy_text/src/font_atlas_set.rs b/crates/bevy_text/src/font_atlas_set.rs index 655ad7ccf3755..2649e37243391 100644 --- a/crates/bevy_text/src/font_atlas_set.rs +++ b/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; @@ -14,6 +14,7 @@ type FontSizeKey = FloatOrd; #[uuid = "73ba778b-b6b5-4f45-982d-d21b6b86ace2"] pub struct FontAtlasSet { font_atlases: HashMap>, + queue: Vec, } #[derive(Debug, Clone)] @@ -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(), } } } @@ -50,7 +52,22 @@ impl FontAtlasSet { texture_atlases: &mut Assets, textures: &mut Assets, outlined_glyph: OutlinedGlyph, + text_settings: &TextSettings, ) -> Result { + 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; @@ -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( @@ -106,11 +124,17 @@ impl FontAtlasSet { } pub fn get_glyph_atlas_info( - &self, + &mut self, font_size: f32, glyph_id: GlyphId, position: Point, ) -> Option { + // 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| { diff --git a/crates/bevy_text/src/glyph_brush.rs b/crates/bevy_text/src/glyph_brush.rs index ca7331901775d..592cd0c26dc61 100644 --- a/crates/bevy_text/src/glyph_brush.rs +++ b/crates/bevy_text/src/glyph_brush.rs @@ -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, @@ -43,6 +43,7 @@ impl GlyphBrush { Ok(section_glyphs) } + #[allow(clippy::too_many_arguments)] pub fn process_glyphs( &self, glyphs: Vec, @@ -51,6 +52,7 @@ impl GlyphBrush { fonts: &Assets, texture_atlases: &mut Assets, textures: &mut Assets, + text_settings: &TextSettings, ) -> Result, TextError> { if glyphs.is_empty() { return Ok(Vec::new()); @@ -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(); diff --git a/crates/bevy_text/src/lib.rs b/crates/bevy_text/src/lib.rs index f8ff0c58963e0..f9257dfa712e9 100644 --- a/crates/bevy_text/src/lib.rs +++ b/crates/bevy_text/src/lib.rs @@ -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::() @@ -46,6 +66,7 @@ impl Plugin for TextPlugin { .register_type::() .register_type::() .init_asset_loader::() + .init_resource::() .insert_resource(TextPipeline::default()) .add_system_to_stage( CoreStage::PostUpdate, diff --git a/crates/bevy_text/src/pipeline.rs b/crates/bevy_text/src/pipeline.rs index d06f24d897ec5..3d05b414f260a 100644 --- a/crates/bevy_text/src/pipeline.rs +++ b/crates/bevy_text/src/pipeline.rs @@ -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)] @@ -49,6 +49,7 @@ impl TextPipeline { font_atlas_set_storage: &mut Assets, texture_atlases: &mut Assets, textures: &mut Assets, + text_settings: &TextSettings, ) -> Result { let mut scaled_fonts = Vec::new(); let sections = sections @@ -103,6 +104,7 @@ impl TextPipeline { fonts, texture_atlases, textures, + text_settings, )?; Ok(TextLayoutInfo { glyphs, size }) diff --git a/crates/bevy_text/src/text2d.rs b/crates/bevy_text/src/text2d.rs index 2b5cb821b1cc5..3711513e81a95 100644 --- a/crates/bevy_text/src/text2d.rs +++ b/crates/bevy_text/src/text2d.rs @@ -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. @@ -153,6 +153,7 @@ pub fn update_text2d_layout( mut textures: ResMut>, fonts: Res>, windows: Res, + text_settings: Res, mut scale_factor_changed: EventReader, mut texture_atlases: ResMut>, mut font_atlas_set_storage: ResMut>, @@ -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) => { diff --git a/crates/bevy_ui/src/widget/text.rs b/crates/bevy_ui/src/widget/text.rs index 77ed556a471c0..ddc7567f1b5e4 100644 --- a/crates/bevy_ui/src/widget/text.rs +++ b/crates/bevy_ui/src/widget/text.rs @@ -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)] @@ -44,6 +44,7 @@ pub fn text_system( mut textures: ResMut>, fonts: Res>, windows: Res, + text_settings: Res, ui_scale: Res, mut texture_atlases: ResMut>, mut font_atlas_set_storage: ResMut>, @@ -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) => {