From 1f4065c1c3399efad58841082212f7c039b58480 Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Wed, 6 Nov 2024 18:59:56 -0700 Subject: [PATCH] Drop ShapePlanCache The ShapePlanCache was added to improve performance when shaping the same strings over and over. However, it never had the ability to be trimmed and when it was moved to FontSystem, this created a permanently growing allocation. It is recommended to instead use the shape-run-cache feature which supports trimming if it is desired to have higher performance for repeated shaping, at the cost of manually specifying when to trim. --- src/font/fallback/mod.rs | 9 ++--- src/font/system.rs | 6 +-- src/lib.rs | 3 -- src/shape.rs | 79 ++++++++++++++++++---------------------- src/shape_plan_cache.rs | 55 ---------------------------- 5 files changed, 40 insertions(+), 112 deletions(-) delete mode 100644 src/shape_plan_cache.rs diff --git a/src/font/fallback/mod.rs b/src/font/fallback/mod.rs index e16aac7..7e160ea 100644 --- a/src/font/fallback/mod.rs +++ b/src/font/fallback/mod.rs @@ -5,7 +5,7 @@ use alloc::vec::Vec; use fontdb::Family; use unicode_script::Script; -use crate::{Font, FontMatchKey, FontSystem, ShapeBuffer, ShapePlanCache}; +use crate::{Font, FontMatchKey, FontSystem, ShapeBuffer}; use self::platform::*; @@ -117,11 +117,8 @@ impl<'a> FontFallbackIter<'a> { } } - pub fn shape_caches(&mut self) -> (&mut ShapeBuffer, &mut ShapePlanCache) { - ( - &mut self.font_system.shape_buffer, - &mut self.font_system.shape_plan_cache, - ) + pub fn shape_caches(&mut self) -> &mut ShapeBuffer { + &mut self.font_system.shape_buffer } fn face_contains_family(&self, id: fontdb::ID, family_name: &str) -> bool { diff --git a/src/font/system.rs b/src/font/system.rs index ef68986..880eb28 100644 --- a/src/font/system.rs +++ b/src/font/system.rs @@ -1,4 +1,4 @@ -use crate::{Attrs, Font, FontMatchAttrs, HashMap, ShapeBuffer, ShapePlanCache}; +use crate::{Attrs, Font, FontMatchAttrs, HashMap, ShapeBuffer}; use alloc::collections::BTreeSet; use alloc::string::String; use alloc::sync::Arc; @@ -103,9 +103,6 @@ pub struct FontSystem { /// Cache for font matches. font_matches_cache: HashMap>>, - /// Cache for rustybuzz shape plans. - pub(crate) shape_plan_cache: ShapePlanCache, - /// Scratch buffer for shaping and laying out. pub(crate) shape_buffer: ShapeBuffer, @@ -177,7 +174,6 @@ impl FontSystem { font_cache: Default::default(), font_matches_cache: Default::default(), font_codepoint_support_info_cache: Default::default(), - shape_plan_cache: ShapePlanCache::default(), monospace_fallbacks_buffer: BTreeSet::default(), #[cfg(feature = "shape-run-cache")] shape_run_cache: crate::ShapeRunCache::default(), diff --git a/src/lib.rs b/src/lib.rs index 8a4fbab..5482e05 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -132,9 +132,6 @@ mod line_ending; pub use self::shape::*; mod shape; -use self::shape_plan_cache::*; -mod shape_plan_cache; - pub use self::shape_run_cache::*; mod shape_run_cache; diff --git a/src/shape.rs b/src/shape.rs index 75824a3..7b24fdf 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -14,7 +14,7 @@ use unicode_segmentation::UnicodeSegmentation; use crate::fallback::FontFallbackIter; use crate::{ math, Align, AttrsList, CacheKeyFlags, Color, Font, FontSystem, LayoutGlyph, LayoutLine, - Metrics, ShapePlanCache, Wrap, + Metrics, Wrap, }; /// The shaping strategy of some text. @@ -106,7 +106,6 @@ impl fmt::Debug for ShapeBuffer { fn shape_fallback( scratch: &mut ShapeBuffer, - shape_plan_cache: &mut ShapePlanCache, glyphs: &mut Vec, font: &Font, line: &str, @@ -141,8 +140,14 @@ fn shape_fallback( let rtl = matches!(buffer.direction(), rustybuzz::Direction::RightToLeft); assert_eq!(rtl, span_rtl); - let shape_plan = shape_plan_cache.get(font, &buffer); - let glyph_buffer = rustybuzz::shape_with_plan(font.rustybuzz(), shape_plan, buffer); + let shape_plan = rustybuzz::ShapePlan::new( + font.rustybuzz(), + buffer.direction(), + Some(buffer.script()), + buffer.language().as_ref(), + &[], + ); + let glyph_buffer = rustybuzz::shape_with_plan(font.rustybuzz(), &shape_plan, buffer); let glyph_infos = glyph_buffer.glyph_infos(); let glyph_positions = glyph_buffer.glyph_positions(); @@ -258,17 +263,9 @@ fn shape_run( let glyph_start = glyphs.len(); let mut missing = { - let (scratch, shape_plan_cache) = font_iter.shape_caches(); + let scratch = font_iter.shape_caches(); shape_fallback( - scratch, - shape_plan_cache, - glyphs, - &font, - line, - attrs_list, - start_run, - end_run, - span_rtl, + scratch, glyphs, &font, line, attrs_list, start_run, end_run, span_rtl, ) }; @@ -284,10 +281,9 @@ fn shape_run( font_iter.face_name(font.id()) ); let mut fb_glyphs = Vec::new(); - let (scratch, shape_plan_cache) = font_iter.shape_caches(); + let scratch = font_iter.shape_caches(); let fb_missing = shape_fallback( scratch, - shape_plan_cache, &mut fb_glyphs, &font, line, @@ -456,34 +452,31 @@ fn shape_skip( let ascent = metrics.ascent / f32::from(metrics.units_per_em); let descent = metrics.descent / f32::from(metrics.units_per_em); - glyphs.extend( - line[start_run..end_run] - .char_indices() - .enumerate() - .map(|(i, (chr_idx, codepoint))| { - let glyph_id = charmap.map(codepoint); - let x_advance = glyph_metrics.advance_width(glyph_id); - let attrs = attrs_list.get_span(start_run + chr_idx); + glyphs.extend(line[start_run..end_run].char_indices().enumerate().map( + |(i, (chr_idx, codepoint))| { + let glyph_id = charmap.map(codepoint); + let x_advance = glyph_metrics.advance_width(glyph_id); + let attrs = attrs_list.get_span(start_run + chr_idx); - ShapeGlyph { - start: i, - end: i + 1, - x_advance, - y_advance: 0.0, - x_offset: 0.0, - y_offset: 0.0, - ascent, - descent, - font_monospace_em_width, - font_id, - glyph_id, - color_opt: attrs.color_opt, - metadata: attrs.metadata, - cache_key_flags: attrs.cache_key_flags, - metrics_opt: attrs.metrics_opt.map(|x| x.into()), - } - }), - ); + ShapeGlyph { + start: i, + end: i + 1, + x_advance, + y_advance: 0.0, + x_offset: 0.0, + y_offset: 0.0, + ascent, + descent, + font_monospace_em_width, + font_id, + glyph_id, + color_opt: attrs.color_opt, + metadata: attrs.metadata, + cache_key_flags: attrs.cache_key_flags, + metrics_opt: attrs.metrics_opt.map(|x| x.into()), + } + }, + )); } /// A shaped glyph diff --git a/src/shape_plan_cache.rs b/src/shape_plan_cache.rs deleted file mode 100644 index e393689..0000000 --- a/src/shape_plan_cache.rs +++ /dev/null @@ -1,55 +0,0 @@ -#[cfg(not(feature = "std"))] -use hashbrown::hash_map::Entry; -#[cfg(feature = "std")] -use std::collections::hash_map::Entry; - -use crate::{Font, HashMap}; - -/// Key for caching shape plans. -#[derive(Debug, Hash, PartialEq, Eq)] -struct ShapePlanKey { - font_id: fontdb::ID, - direction: rustybuzz::Direction, - script: rustybuzz::Script, - language: Option, -} - -/// A helper structure for caching rustybuzz shape plans. -#[derive(Default)] -pub struct ShapePlanCache(HashMap); - -impl ShapePlanCache { - pub fn get(&mut self, font: &Font, buffer: &rustybuzz::UnicodeBuffer) -> &rustybuzz::ShapePlan { - let key = ShapePlanKey { - font_id: font.id(), - direction: buffer.direction(), - script: buffer.script(), - language: buffer.language(), - }; - match self.0.entry(key) { - Entry::Occupied(occ) => occ.into_mut(), - Entry::Vacant(vac) => { - let ShapePlanKey { - direction, - script, - language, - .. - } = vac.key(); - let plan = rustybuzz::ShapePlan::new( - font.rustybuzz(), - *direction, - Some(*script), - language.as_ref(), - &[], - ); - vac.insert(plan) - } - } - } -} - -impl core::fmt::Debug for ShapePlanCache { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_tuple("ShapePlanCache").finish() - } -}