Fix cache trimming loop in `iced_wgpu::text`

2 files changed, 52 insertions, 9 deletions

diff --git a/graphics/src/cache.rs b/graphics/src/cache.rs
index 7106c392..bbba79eb 100644
--- a/graphics/src/cache.rs
+++ b/graphics/src/cache.rs
@@ -15,7 +15,7 @@ impl<T> Cache<T> {
     /// Creates a new empty [`Cache`].
     pub fn new() -> Self {
         Cache {
-            group: Group::unique(),
+            group: Group::singleton(),
             state: RefCell::new(State::Empty { previous: None }),
         }
     }
@@ -27,6 +27,11 @@ impl<T> Cache<T> {
     /// You should generally group caches that are likely to change
     /// together.
     pub fn with_group(group: Group) -> Self {
+        assert!(
+            !group.is_singleton(),
+            "The group {group:?} cannot be shared!"
+        );
+
         Cache {
             group,
             state: RefCell::new(State::Empty { previous: None }),
@@ -75,14 +80,40 @@ impl<T> Cache<T> {
 /// A cache group can be used to implement certain performance
 /// optimizations during rendering, like batching or sharing atlases.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct Group(u64);
+pub struct Group {
+    id: u64,
+    is_singleton: bool,
+}
 
 impl Group {
     /// Generates a new unique cache [`Group`].
     pub fn unique() -> Self {
         static NEXT: AtomicU64 = AtomicU64::new(0);
 
-        Self(NEXT.fetch_add(1, atomic::Ordering::Relaxed))
+        Self {
+            id: NEXT.fetch_add(1, atomic::Ordering::Relaxed),
+            is_singleton: false,
+        }
+    }
+
+    /// Returns `true` if the [`Group`] can only ever have a
+    /// single [`Cache`] in it.
+    ///
+    /// This is the default kind of [`Group`] assigned when using
+    /// [`Cache::new`].
+    ///
+    /// Knowing that a [`Group`] will never be shared may be
+    /// useful for rendering backends to perform additional
+    /// optimizations.
+    pub fn is_singleton(self) -> bool {
+        self.is_singleton
+    }
+
+    fn singleton() -> Self {
+        Self {
+            is_singleton: true,
+            ..Self::unique()
+        }
     }
 }
 
diff --git a/wgpu/src/text.rs b/wgpu/src/text.rs
index 8ec1d56a..2508906f 100644
--- a/wgpu/src/text.rs
+++ b/wgpu/src/text.rs
@@ -157,13 +157,16 @@ impl Storage {
                         target_size,
                     );
 
+                    // Only trim if glyphs have changed
+                    group.should_trim =
+                        group.should_trim || upload.version != cache.version;
+
                     upload.text = Rc::downgrade(&cache.text);
                     upload.version = cache.version;
                     upload.group_version = group.version;
                     upload.transformation = new_transformation;
 
                     upload.buffer_cache.trim();
-                    group.should_trim = true;
                 }
             }
             hash_map::Entry::Vacant(entry) => {
@@ -213,19 +216,28 @@ impl Storage {
             .retain(|_id, upload| upload.text.strong_count() > 0);
 
         self.groups.retain(|id, group| {
-            if Rc::weak_count(&group.handle) == 0 {
+            let active_uploads = Rc::weak_count(&group.handle);
+
+            if active_uploads == 0 {
                 log::debug!("Dropping text atlas: {id:?}");
 
                 return false;
             }
 
-            if group.should_trim {
-                log::debug!("Trimming text atlas: {id:?}");
+            if id.is_singleton() || group.should_trim {
+                log::debug!(
+                    "Trimming text atlas: {id:?} (uploads: {active_uploads})"
+                );
 
                 group.atlas.trim();
-
-                group.version += 1;
                 group.should_trim = false;
+
+                // We only need to worry about glyph fighting
+                // when the atlas may be shared by multiple
+                // uploads.
+                if !id.is_singleton() {
+                    group.version += 1;
+                }
             }
 
             true