From da4bb4af5c22630fb279a86c89874f0d95ee5bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ram=C3=B3n=20Jim=C3=A9nez?= Date: Wed, 8 Feb 2023 04:21:16 +0100 Subject: [PATCH] Use `lru` for glyph caching I noticed that the `RecentlyUsed` map was actually a `RecentlyInserted` map. This caused panics when trying to reduce the initial texture atlas size and dynamically increase it (coming in another PR!). --- Cargo.toml | 1 + src/lib.rs | 2 - src/recently_used.rs | 257 ------------------------------------------- src/text_atlas.rs | 20 ++-- src/text_render.rs | 12 +- 5 files changed, 17 insertions(+), 275 deletions(-) delete mode 100644 src/recently_used.rs diff --git a/Cargo.toml b/Cargo.toml index 325dadd..45dbca8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ license = "MIT OR Apache-2.0 OR Zlib" wgpu = "0.14.0" etagere = "0.2.6" cosmic-text = { git = "https://github.com/pop-os/cosmic-text", rev = "a5903bb", features = ["std", "swash"] } +lru = "0.9" [dev-dependencies] winit = "0.27.0" diff --git a/src/lib.rs b/src/lib.rs index 485c017..e15900c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,10 +1,8 @@ mod error; -mod recently_used; mod text_atlas; mod text_render; pub use error::{PrepareError, RenderError}; -use recently_used::RecentlyUsedMap; pub use text_atlas::TextAtlas; use text_render::ContentType; pub use text_render::TextRenderer; diff --git a/src/recently_used.rs b/src/recently_used.rs deleted file mode 100644 index 196cd20..0000000 --- a/src/recently_used.rs +++ /dev/null @@ -1,257 +0,0 @@ -use std::{ - borrow::Borrow, - collections::{hash_map::Entry, HashMap}, - hash::Hash, -}; - -struct RecentlyUsedItem { - node_idx: usize, - value: V, -} - -enum Node { - Value { - value: V, - prev_idx: Option, - next_idx: Option, - }, - Free { - next_idx: Option, - }, -} - -pub struct RecentlyUsedMap { - nodes: Vec>, - least_recent_idx: Option, - most_recent_idx: Option, - map: HashMap>, - free: Option, -} - -impl RecentlyUsedMap { - pub fn new() -> Self { - Self::with_capacity(0) - } - - pub fn with_capacity(capacity: usize) -> Self { - assert!(capacity < usize::MAX - 1, "capacity too large"); - Self { - nodes: Vec::with_capacity(capacity), - least_recent_idx: None, - most_recent_idx: None, - free: None, - map: HashMap::with_capacity(capacity), - } - } - - fn allocate_node(&mut self, node: Node) -> usize { - match self.free { - Some(idx) => { - // Reuse a node from storage - match self.nodes[idx] { - Node::Free { next_idx } => { - self.free = next_idx; - } - Node::Value { .. } => unreachable!(), - } - - self.nodes[idx] = node; - - idx - } - None => { - // Create a new storage node - self.nodes.push(node); - self.nodes.len() - 1 - } - } - } - - pub fn insert(&mut self, key: K, value: V) { - let new_idx = self.allocate_node(Node::Value { - value: key, - prev_idx: self.most_recent_idx, - next_idx: None, - }); - - let prior_most_recent = self.most_recent_idx.map(|idx| &mut self.nodes[idx]); - match prior_most_recent { - Some(p) => { - // Update prior most recent if one exists - match p { - Node::Value { next_idx, .. } => { - *next_idx = Some(new_idx); - } - Node::Free { .. } => unreachable!(), - } - } - None => { - // Update least recent if this is the first node - self.least_recent_idx = Some(new_idx); - } - } - - self.most_recent_idx = Some(new_idx); - - match self.map.entry(key) { - Entry::Occupied(mut occupied) => { - let old = occupied.insert(RecentlyUsedItem { - node_idx: new_idx, - value, - }); - - // Remove the old occurrence of this key - self.remove_node(old.node_idx); - } - Entry::Vacant(vacant) => { - vacant.insert(RecentlyUsedItem { - node_idx: new_idx, - value, - }); - } - } - } - - fn remove_node(&mut self, idx: usize) { - match self.nodes[idx] { - Node::Value { - prev_idx, next_idx, .. - } => { - match prev_idx { - Some(prev) => match &mut self.nodes[prev] { - Node::Value { - next_idx: old_next_idx, - .. - } => { - *old_next_idx = next_idx; - } - Node::Free { .. } => unreachable!(), - }, - None => { - // This node was the least recent - self.least_recent_idx = next_idx; - } - } - - match next_idx { - Some(next) => match &mut self.nodes[next] { - Node::Value { - prev_idx: old_prev_idx, - .. - } => { - *old_prev_idx = prev_idx; - } - Node::Free { .. } => unreachable!(), - }, - None => { - // This node was the most recent - self.most_recent_idx = prev_idx; - } - } - } - Node::Free { .. } => unreachable!(), - } - - // Add to free list - self.nodes[idx] = Node::Free { - next_idx: self.free, - }; - self.free = Some(idx); - } - - pub fn remove(&mut self, key: &Q) -> Option - where - K: Borrow, - Q: Hash + Eq, - { - self.map.remove(key.borrow()).map(|entry| { - self.remove_node(entry.node_idx); - entry.value - }) - } - - pub fn get(&self, k: &Q) -> Option<&V> - where - K: Borrow, - Q: Hash + Eq, - { - self.map.get(k).map(|item| &item.value) - } - - pub fn contains_key(&self, k: &Q) -> bool - where - K: Borrow, - Q: Hash + Eq, - { - self.map.contains_key(k) - } - - pub fn pop(&mut self) -> Option<(K, V)> { - let least_recent_idx = self.least_recent_idx?; - let least_recent_node = &self.nodes[least_recent_idx]; - - match *least_recent_node { - Node::Value { value: key, .. } => { - let value = self.remove(&key); - value.map(|v| (key, v)) - } - Node::Free { .. } => unreachable!(), - } - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn insert() { - let mut rus = RecentlyUsedMap::new(); - rus.insert("a", ()); - rus.insert("b", ()); - rus.insert("c", ()); - assert_eq!(rus.pop(), Some(("a", ()))); - assert_eq!(rus.pop(), Some(("b", ()))); - assert_eq!(rus.pop(), Some(("c", ()))); - assert_eq!(rus.pop(), None); - } - - #[test] - fn reinsert() { - let mut rus = RecentlyUsedMap::new(); - rus.insert("a", ()); - rus.insert("b", ()); - rus.insert("c", ()); - rus.insert("a", ()); - assert_eq!(rus.pop(), Some(("b", ()))); - assert_eq!(rus.pop(), Some(("c", ()))); - assert_eq!(rus.pop(), Some(("a", ()))); - assert_eq!(rus.pop(), None); - } - - #[test] - fn remove() { - let mut rus = RecentlyUsedMap::new(); - rus.insert("a", ()); - rus.insert("b", ()); - rus.remove("a"); - rus.remove("c"); - assert_eq!(rus.pop(), Some(("b", ()))); - assert_eq!(rus.pop(), None); - } - - #[test] - fn reuses_free_nodes() { - let mut rus = RecentlyUsedMap::new(); - rus.insert("a", ()); - rus.insert("b", ()); - rus.insert("c", ()); - rus.remove("b"); - rus.remove("c"); - rus.remove("a"); - rus.insert("d", ()); - rus.insert("e", ()); - rus.insert("f", ()); - assert_eq!(rus.nodes.len(), 3); - } -} diff --git a/src/text_atlas.rs b/src/text_atlas.rs index 032b208..46f6a4a 100644 --- a/src/text_atlas.rs +++ b/src/text_atlas.rs @@ -1,8 +1,6 @@ -use crate::{ - text_render::ContentType, CacheKey, GlyphDetails, GlyphToRender, Params, RecentlyUsedMap, - Resolution, -}; +use crate::{text_render::ContentType, CacheKey, GlyphDetails, GlyphToRender, Params, Resolution}; use etagere::{size2, Allocation, BucketedAtlasAllocator}; +use lru::LruCache; use std::{borrow::Cow, mem::size_of, num::NonZeroU64, sync::Arc}; use wgpu::{ BindGroup, BindGroupDescriptor, BindGroupEntry, BindGroupLayoutEntry, BindingResource, @@ -21,7 +19,7 @@ pub(crate) struct InnerAtlas { pub packer: BucketedAtlasAllocator, pub width: u32, pub height: u32, - pub glyph_cache: RecentlyUsedMap, + pub glyph_cache: LruCache, pub num_atlas_channels: usize, } @@ -55,7 +53,7 @@ impl InnerAtlas { let texture_view = texture.create_view(&TextureViewDescriptor::default()); - let glyph_cache = RecentlyUsedMap::new(); + let glyph_cache = LruCache::unbounded(); Self { texture_pending, @@ -79,10 +77,9 @@ impl InnerAtlas { } // Try to free least recently used allocation - let (key, value) = self.glyph_cache.pop()?; + let (_, value) = self.glyph_cache.pop_lru()?; self.packer .deallocate(value.atlas_id.expect("cache corrupt")); - self.glyph_cache.remove(&key); } } } @@ -275,15 +272,14 @@ impl TextAtlas { } pub(crate) fn contains_cached_glyph(&self, glyph: &CacheKey) -> bool { - self.mask_atlas.glyph_cache.contains_key(glyph) - || self.color_atlas.glyph_cache.contains_key(glyph) + self.mask_atlas.glyph_cache.contains(glyph) || self.color_atlas.glyph_cache.contains(glyph) } pub(crate) fn glyph(&self, glyph: &CacheKey) -> Option<&GlyphDetails> { self.mask_atlas .glyph_cache - .get(glyph) - .or_else(|| self.color_atlas.glyph_cache.get(glyph)) + .peek(glyph) + .or_else(|| self.color_atlas.glyph_cache.peek(glyph)) } pub(crate) fn inner_for_content(&self, content_type: ContentType) -> &InnerAtlas { diff --git a/src/text_render.rs b/src/text_render.rs index 5701cae..2de59d6 100644 --- a/src/text_render.rs +++ b/src/text_render.rs @@ -101,9 +101,13 @@ impl TextRenderer { for glyph in run.glyphs.iter() { self.glyphs_in_use.insert(glyph.cache_key); - let already_on_gpu = atlas.contains_cached_glyph(&glyph.cache_key); + if atlas.mask_atlas.glyph_cache.contains(&glyph.cache_key) { + atlas.mask_atlas.glyph_cache.promote(&glyph.cache_key); + continue; + } - if already_on_gpu { + if atlas.color_atlas.glyph_cache.contains(&glyph.cache_key) { + atlas.color_atlas.glyph_cache.promote(&glyph.cache_key); continue; } @@ -182,8 +186,8 @@ impl TextRenderer { (GpuCacheStatus::SkipRasterization, None, inner) }; - if !inner.glyph_cache.contains_key(&glyph.cache_key) { - inner.glyph_cache.insert( + if !inner.glyph_cache.contains(&glyph.cache_key) { + inner.glyph_cache.put( glyph.cache_key, GlyphDetails { width: width as u16,