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

@@ -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"

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;

src/recently_used.rs

@@ -1,257 +0,0 @@
-use std::{
-    borrow::Borrow,
-    collections::{hash_map::Entry, HashMap},
-    hash::Hash,
-};
-
-struct RecentlyUsedItem<V> {
-    node_idx: usize,
-    value: V,
-}
-
-enum Node<V> {
-    Value {
-        value: V,
-        prev_idx: Option<usize>,
-        next_idx: Option<usize>,
-    },
-    Free {
-        next_idx: Option<usize>,
-    },
-}
-
-pub struct RecentlyUsedMap<K: Clone + Copy + Eq + Hash, V> {
-    nodes: Vec<Node<K>>,
-    least_recent_idx: Option<usize>,
-    most_recent_idx: Option<usize>,
-    map: HashMap<K, RecentlyUsedItem<V>>,
-    free: Option<usize>,
-}
-
-impl<K: Clone + Copy + Eq + Hash, V> RecentlyUsedMap<K, V> {
-    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<K>) -> 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<Q: ?Sized>(&mut self, key: &Q) -> Option<V>
-    where
-        K: Borrow<Q>,
-        Q: Hash + Eq,
-    {
-        self.map.remove(key.borrow()).map(|entry| {
-            self.remove_node(entry.node_idx);
-            entry.value
-        })
-    }
-
-    pub fn get<Q: ?Sized>(&self, k: &Q) -> Option<&V>
-    where
-        K: Borrow<Q>,
-        Q: Hash + Eq,
-    {
-        self.map.get(k).map(|item| &item.value)
-    }
-
-    pub fn contains_key<Q: ?Sized>(&self, k: &Q) -> bool
-    where
-        K: Borrow<Q>,
-        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);
-    }
-}

src/text_atlas.rs

@@ -1,8 +1,6 @@
-use crate::{
+use crate::{text_render::ContentType, CacheKey, GlyphDetails, GlyphToRender, Params, Resolution};
-    text_render::ContentType, CacheKey, GlyphDetails, GlyphToRender, Params, RecentlyUsedMap,
-    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<CacheKey, GlyphDetails>,
+    pub glyph_cache: LruCache<CacheKey, GlyphDetails>,
     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.mask_atlas.glyph_cache.contains(glyph) || self.color_atlas.glyph_cache.contains(glyph)
-            || self.color_atlas.glyph_cache.contains_key(glyph)
     }
 
     pub(crate) fn glyph(&self, glyph: &CacheKey) -> Option<&GlyphDetails> {
         self.mask_atlas
             .glyph_cache
-            .get(glyph)
+            .peek(glyph)
-            .or_else(|| self.color_atlas.glyph_cache.get(glyph))
+            .or_else(|| self.color_atlas.glyph_cache.peek(glyph))
     }
 
     pub(crate) fn inner_for_content(&self, content_type: ContentType) -> &InnerAtlas {

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) {
+                    if !inner.glyph_cache.contains(&glyph.cache_key) {
-                        inner.glyph_cache.insert(
+                        inner.glyph_cache.put(
                             glyph.cache_key,
                             GlyphDetails {
                                 width: width as u16,