Fix sometimes blurry SVGs (#7071)

* Closes https://github.com/emilk/egui/issues/3501

The problem occurs when you want to render the same SVG at different
scales, either at the same time in different parts of your UI, or at two
different times (e.g. the DPI changes).

The solution is to use the `SizeHint` as part of the key.

However, when you have an SVG in a resizable container, that can lead to
hundreds of versions of the same SVG. So new eviction code is added to
handle this case.

crates/egui/src/context.rs

@@ -2,7 +2,6 @@
 
 use std::{borrow::Cow, cell::RefCell, panic::Location, sync::Arc, time::Duration};
 
-use containers::area::AreaState;
 use emath::GuiRounding as _;
 use epaint::{
     emath::{self, TSTransform},
@@ -17,7 +16,7 @@ use epaint::{
 
 use crate::{
     animation_manager::AnimationManager,
-    containers,
+    containers::{self, area::AreaState},
     data::output::PlatformOutput,
     epaint, hit_test,
     input_state::{InputState, MultiTouchInfo, PointerEvent},
@@ -3065,6 +3064,12 @@ impl Context {
                 self.texture_ui(ui);
             });
 
+        CollapsingHeader::new("🖼 Image loaders")
+            .default_open(false)
+            .show(ui, |ui| {
+                self.loaders_ui(ui);
+            });
+
         CollapsingHeader::new("🔠 Font texture")
             .default_open(false)
             .show(ui, |ui| {
@@ -3147,6 +3152,73 @@ impl Context {
         });
     }
 
+    /// Show stats about different image loaders.
+    pub fn loaders_ui(&self, ui: &mut crate::Ui) {
+        struct LoaderInfo {
+            id: String,
+            byte_size: usize,
+        }
+
+        let mut byte_loaders = vec![];
+        let mut image_loaders = vec![];
+        let mut texture_loaders = vec![];
+
+        {
+            let loaders = self.loaders();
+            let Loaders {
+                include: _,
+                bytes,
+                image,
+                texture,
+            } = loaders.as_ref();
+
+            for loader in bytes.lock().iter() {
+                byte_loaders.push(LoaderInfo {
+                    id: loader.id().to_owned(),
+                    byte_size: loader.byte_size(),
+                });
+            }
+            for loader in image.lock().iter() {
+                image_loaders.push(LoaderInfo {
+                    id: loader.id().to_owned(),
+                    byte_size: loader.byte_size(),
+                });
+            }
+            for loader in texture.lock().iter() {
+                texture_loaders.push(LoaderInfo {
+                    id: loader.id().to_owned(),
+                    byte_size: loader.byte_size(),
+                });
+            }
+        }
+
+        fn loaders_ui(ui: &mut crate::Ui, title: &str, loaders: &[LoaderInfo]) {
+            let heading = format!("{} {title} loaders", loaders.len());
+            crate::CollapsingHeader::new(heading)
+                .default_open(true)
+                .show(ui, |ui| {
+                    Grid::new("loaders")
+                        .striped(true)
+                        .num_columns(2)
+                        .show(ui, |ui| {
+                            ui.label("ID");
+                            ui.label("Size");
+                            ui.end_row();
+
+                            for loader in loaders {
+                                ui.label(&loader.id);
+                                ui.label(format!("{:.3} MB", loader.byte_size as f64 * 1e-6));
+                                ui.end_row();
+                            }
+                        });
+                });
+        }
+
+        loaders_ui(ui, "byte", &byte_loaders);
+        loaders_ui(ui, "image", &image_loaders);
+        loaders_ui(ui, "texture", &texture_loaders);
+    }
+
     /// Shows the contents of [`Self::memory`].
     pub fn memory_ui(&self, ui: &mut crate::Ui) {
         if ui

crates/egui/src/load.rs

@@ -146,7 +146,7 @@ pub type Result<T, E = LoadError> = std::result::Result<T, E>;
 /// The size is measured in texels, with the pixels per point already factored in.
 ///
 /// All variants will preserve the original aspect ratio.
-#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub enum SizeHint {
     /// Scale original size by some factor.
     Scale(OrderedFloat<f32>),
@@ -402,7 +402,7 @@ pub trait ImageLoader {
     fn byte_size(&self) -> usize;
 
     /// Returns `true` if some image is currently being loaded.
-    ///  
+    ///
     /// NOTE: You probably also want to check [`BytesLoader::has_pending`].
     fn has_pending(&self) -> bool {
         false

crates/egui/src/load/texture_loader.rs

@@ -1,13 +1,28 @@
-use std::borrow::Cow;
+use std::sync::atomic::{AtomicU64, Ordering::Relaxed};
 
 use super::{
     BytesLoader as _, Context, HashMap, ImagePoll, Mutex, SizeHint, SizedTexture, TextureHandle,
     TextureLoadResult, TextureLoader, TextureOptions, TexturePoll,
 };
 
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+struct PrimaryKey {
+    uri: String,
+    texture_options: TextureOptions,
+}
+
+/// SVG:s might have several different sizes loaded
+type Bucket = HashMap<Option<SizeHint>, Entry>;
+
+struct Entry {
+    last_used: AtomicU64,
+    handle: TextureHandle,
+}
+
 #[derive(Default)]
 pub struct DefaultTextureLoader {
-    cache: Mutex<HashMap<(Cow<'static, str>, TextureOptions), TextureHandle>>,
+    pass_index: AtomicU64,
+    cache: Mutex<HashMap<PrimaryKey, Bucket>>,
 }
 
 impl TextureLoader for DefaultTextureLoader {
@@ -22,9 +37,31 @@ impl TextureLoader for DefaultTextureLoader {
         texture_options: TextureOptions,
         size_hint: SizeHint,
     ) -> TextureLoadResult {
+        let svg_size_hint = if is_svg(uri) {
+            // For SVGs it's important that we render at the desired size,
+            // or we might get a blurry image when we scale it up.
+            // So we make the size hint a part of the cache key.
+            // This might lead to a lot of extra entries for the same SVG file,
+            // which is potentially wasteful of RAM, but better that than blurry images.
+            Some(size_hint)
+        } else {
+            // For other images we just use one cache value, no matter what the size we render at.
+            None
+        };
+
         let mut cache = self.cache.lock();
-        if let Some(handle) = cache.get(&(Cow::Borrowed(uri), texture_options)) {
-            let texture = SizedTexture::from_handle(handle);
+        let bucket = cache
+            .entry(PrimaryKey {
+                uri: uri.to_owned(),
+                texture_options,
+            })
+            .or_default();
+
+        if let Some(texture) = bucket.get(&svg_size_hint) {
+            texture
+                .last_used
+                .store(self.pass_index.load(Relaxed), Relaxed);
+            let texture = SizedTexture::from_handle(&texture.handle);
             Ok(TexturePoll::Ready { texture })
         } else {
             match ctx.try_load_image(uri, size_hint)? {
@@ -32,7 +69,13 @@ impl TextureLoader for DefaultTextureLoader {
                 ImagePoll::Ready { image } => {
                     let handle = ctx.load_texture(uri, image, texture_options);
                     let texture = SizedTexture::from_handle(&handle);
-                    cache.insert((Cow::Owned(uri.to_owned()), texture_options), handle);
+                    bucket.insert(
+                        svg_size_hint,
+                        Entry {
+                            last_used: AtomicU64::new(self.pass_index.load(Relaxed)),
+                            handle,
+                        },
+                    );
                     let reduce_texture_memory = ctx.options(|o| o.reduce_texture_memory);
                     if reduce_texture_memory {
                         let loaders = ctx.loaders();
@@ -54,7 +97,7 @@ impl TextureLoader for DefaultTextureLoader {
         #[cfg(feature = "log")]
         log::trace!("forget {uri:?}");
 
-        self.cache.lock().retain(|(u, _), _| u != uri);
+        self.cache.lock().retain(|key, _value| key.uri != uri);
     }
 
     fn forget_all(&self) {
@@ -64,11 +107,35 @@ impl TextureLoader for DefaultTextureLoader {
         self.cache.lock().clear();
     }
 
+    fn end_pass(&self, pass_index: u64) {
+        self.pass_index.store(pass_index, Relaxed);
+        let mut cache = self.cache.lock();
+        cache.retain(|_key, bucket| {
+            if 2 <= bucket.len() {
+                // There are multiple textures of the same URI (e.g. SVGs of different scales).
+                // This could be because someone has an SVG in a resizable container,
+                // and so we get a lot of different sizes of it.
+                // This could wast VRAM, so we remove the ones that are not used in this frame.
+                bucket.retain(|_, texture| pass_index <= texture.last_used.load(Relaxed) + 1);
+            }
+            !bucket.is_empty()
+        });
+    }
+
     fn byte_size(&self) -> usize {
         self.cache
             .lock()
             .values()
-            .map(|texture| texture.byte_size())
+            .map(|bucket| {
+                bucket
+                    .values()
+                    .map(|texture| texture.handle.byte_size())
+                    .sum::<usize>()
+            })
             .sum()
     }
 }
+
+fn is_svg(uri: &str) -> bool {
+    uri.ends_with(".svg")
+}

crates/egui_demo_lib/src/demo/tests/svg_test.rs

@@ -27,6 +27,15 @@ impl crate::View for SvgTest {
     fn ui(&mut self, ui: &mut egui::Ui) {
         let Self { color } = self;
         ui.color_edit_button_srgba(color);
-        ui.add(egui::Image::new(egui::include_image!("../../../data/peace.svg")).tint(*color));
+        let img_src = egui::include_image!("../../../data/peace.svg");
+
+        // First paint a small version…
+        ui.add_sized(
+            egui::Vec2 { x: 20.0, y: 20.0 },
+            egui::Image::new(img_src.clone()).tint(*color),
+        );
+
+        // …then a big one, to make sure they are both crisp
+        ui.add(egui::Image::new(img_src).tint(*color));
     }
 }

crates/egui_demo_lib/tests/snapshots/demos/SVG Test.png

@@ -1,3 +1,3 @@
 version https://git-lfs.github.com/spec/v1
-oid sha256:f1e0657cae72e7ee117d8826bcd18685fd0cdd61d0d613685e4337533c7d4801
-size 23050
+oid sha256:1160361c41ffa9cde6d83cb32eeb9f9b75b275e98b97b625eababee460b69ba9
+size 24072

crates/egui_extras/src/loaders/svg_loader.rs

@@ -1,4 +1,10 @@
-use std::{borrow::Cow, mem::size_of, path::Path, sync::Arc};
+use std::{
+    mem::size_of,
+    sync::{
+        atomic::{AtomicU64, Ordering::Relaxed},
+        Arc,
+    },
+};
 
 use ahash::HashMap;
 
@@ -8,10 +14,14 @@ use egui::{
     ColorImage,
 };
 
-type Entry = Result<Arc<ColorImage>, String>;
+struct Entry {
+    last_used: AtomicU64,
+    result: Result<Arc<ColorImage>, String>,
+}
 
 pub struct SvgLoader {
-    cache: Mutex<HashMap<(Cow<'static, str>, SizeHint), Entry>>,
+    pass_index: AtomicU64,
+    cache: Mutex<HashMap<String, HashMap<SizeHint, Entry>>>,
     options: resvg::usvg::Options<'static>,
 }
 
@@ -20,11 +30,7 @@ impl SvgLoader {
 }
 
 fn is_supported(uri: &str) -> bool {
-    let Some(ext) = Path::new(uri).extension().and_then(|ext| ext.to_str()) else {
-        return false;
-    };
-
-    ext == "svg"
+    uri.ends_with(".svg")
 }
 
 impl Default for SvgLoader {
@@ -37,6 +43,7 @@ impl Default for SvgLoader {
         options.fontdb_mut().load_system_fonts();
 
         Self {
+            pass_index: AtomicU64::new(0),
             cache: Mutex::new(HashMap::default()),
             options,
         }
@@ -54,24 +61,35 @@ impl ImageLoader for SvgLoader {
         }
 
         let mut cache = self.cache.lock();
-        // We can't avoid the `uri` clone here without unsafe code.
-        if let Some(entry) = cache.get(&(Cow::Borrowed(uri), size_hint)).cloned() {
-            match entry {
+        let bucket = cache.entry(uri.to_owned()).or_default();
+
+        if let Some(entry) = bucket.get(&size_hint) {
+            entry
+                .last_used
+                .store(self.pass_index.load(Relaxed), Relaxed);
+            match entry.result.clone() {
                 Ok(image) => Ok(ImagePoll::Ready { image }),
                 Err(err) => Err(LoadError::Loading(err)),
             }
         } else {
             match ctx.try_load_bytes(uri) {
                 Ok(BytesPoll::Ready { bytes, .. }) => {
-                    log::trace!("started loading {uri:?}");
+                    log::trace!("Started loading {uri:?}");
                     let result = crate::image::load_svg_bytes_with_size(
                         &bytes,
                         Some(size_hint),
                         &self.options,
                     )
                     .map(Arc::new);
-                    log::trace!("finished loading {uri:?}");
-                    cache.insert((Cow::Owned(uri.to_owned()), size_hint), result.clone());
+
+                    log::trace!("Finished loading {uri:?}");
+                    bucket.insert(
+                        size_hint,
+                        Entry {
+                            last_used: AtomicU64::new(self.pass_index.load(Relaxed)),
+                            result: result.clone(),
+                        },
+                    );
                     match result {
                         Ok(image) => Ok(ImagePoll::Ready { image }),
                         Err(err) => Err(LoadError::Loading(err)),
@@ -84,7 +102,7 @@ impl ImageLoader for SvgLoader {
     }
 
     fn forget(&self, uri: &str) {
-        self.cache.lock().retain(|(u, _), _| u != uri);
+        self.cache.lock().retain(|key, _| key != uri);
     }
 
     fn forget_all(&self) {
@@ -95,12 +113,28 @@ impl ImageLoader for SvgLoader {
         self.cache
             .lock()
             .values()
-            .map(|result| match result {
+            .flat_map(|bucket| bucket.values())
+            .map(|entry| match &entry.result {
                 Ok(image) => image.pixels.len() * size_of::<egui::Color32>(),
                 Err(err) => err.len(),
             })
             .sum()
     }
+
+    fn end_pass(&self, pass_index: u64) {
+        self.pass_index.store(pass_index, Relaxed);
+        let mut cache = self.cache.lock();
+        cache.retain(|_key, bucket| {
+            if 2 <= bucket.len() {
+                // There are multiple images of the same URI (e.g. SVGs of different scales).
+                // This could be because someone has an SVG in a resizable container,
+                // and so we get a lot of different sizes of it.
+                // This could wast RAM, so we remove the ones that are not used in this frame.
+                bucket.retain(|_, texture| pass_index <= texture.last_used.load(Relaxed) + 1);
+            }
+            !bucket.is_empty()
+        });
+    }
 }
 
 #[cfg(test)]

crates/emath/src/ordered_float.rs

@@ -21,6 +21,13 @@ impl<T: Float + Copy> OrderedFloat<T> {
     }
 }
 
+impl<T: std::fmt::Debug> std::fmt::Debug for OrderedFloat<T> {
+    #[inline]
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt(f)
+    }
+}
+
 impl<T: Float> Eq for OrderedFloat<T> {}
 
 impl<T: Float> PartialEq<Self> for OrderedFloat<T> {