fix: wrong icons selected on size mismatch

This will notably fix Firefox and Thunderbird returning a 16 px icon instead of a 48 px icon when requesting a 24 px icon

src/lib.rs

@@ -56,6 +56,7 @@ use theme::BASE_PATHS;
 
 use crate::cache::{CACHE, CacheEntry};
 use crate::theme::{THEMES, Theme, try_build_icon_path};
+use std::ffi::OsStr;
 use std::hash::{Hash, Hasher};
 use std::io::BufRead;
 use std::ops::ControlFlow;
@@ -63,6 +64,7 @@ use std::path::PathBuf;
 
 mod cache;
 mod theme;
+mod walk_dir;
 
 /// Return the list of installed themes on the system
 ///
@@ -273,29 +275,48 @@ impl<'a> LookupBuilder<'a> {
         }
 
         if !self.extra_paths.is_empty() {
-            let extensions = if self.force_svg {
+            let mut svg_path = None;
-                [".svg", ".png", ".xpm"]
+            let mut png_path = None;
-            } else {
+            let mut xpm_path = None;
-                [".png", ".svg", ".xpm"]
-            };
-            let mut name_buf = String::new();
 
-            let result = extensions
+            for file_path in walk_dir::Iter::new(self.extra_paths.iter().cloned()) {
-                .into_iter()
+                if let Some(file_name) = file_path.file_stem().and_then(OsStr::to_str)
-                .try_for_each(|ext| {
+                    && file_name != self.name
-                    self.extra_paths.iter().try_for_each(|dir| {
+                {
-                        let mut path = dir.clone();
+                    continue;
-                        if try_build_icon_path(&mut path, &mut name_buf, self.name, ext) {
+                }
-                            return ControlFlow::Break(path);
+
+                if let Some(this_ext) = file_path.extension().and_then(OsStr::to_str) {
+                    match this_ext {
+                        "svg" => {
+                            svg_path = Some(file_path);
+                            if self.force_svg || png_path.is_some() {
+                                break;
+                            }
                         }
-                        name_buf.clear();
-                        ControlFlow::Continue(())
-                    })
-                })
-                .break_value();
 
-            if result.is_some() {
+                        "png" => {
-                return result;
+                            png_path = Some(file_path);
+                            if !self.force_svg || svg_path.is_some() {
+                                break;
+                            }
+                        }
+
+                        "xpm" => {
+                            xpm_path = Some(file_path);
+                        }
+
+                        _ => (),
+                    }
+                }
+            }
+
+            if let Some(path) = if self.force_svg {
+                svg_path.or(png_path).or(xpm_path)
+            } else {
+                png_path.or(svg_path).or(xpm_path)
+            } {
+                return Some(path);
             }
         }
 
@@ -450,38 +471,41 @@ mod test {
     use std::path::PathBuf;
 
     #[test]
-    fn hicolor_firefox_24_png() {
+    fn hicolor_thunderbird_48_png() {
-        let firefox = lookup("firefox").find();
+        let thunderbird = lookup("thunderbird").with_size(24).find();
 
-        asserting!("Firefox contains only a 16x16 and 32x32 icon, so 16x16 should be returned")
+        asserting!("thunderbird lacks a 24x24 icon, but a 48x48 icon is an ideal replacement")
-            .that(&firefox)
+            .that(&thunderbird)
             .is_some()
             .is_equal_to(PathBuf::from(
-                "/usr/share/icons/hicolor/16x16/apps/firefox.png",
+                "/usr/share/icons/hicolor/48x48/apps/thunderbird.png",
             ));
     }
 
     #[test]
-    fn hicolor_firefox_48_png() {
+    fn hicolor_libreoffice_svg() {
-        let firefox = lookup("firefox").with_size(48).find();
+        let libreoffice_writer = lookup("libreoffice-writer").force_svg().find();
-
-        asserting!("Firefox has a 48x48 icon, so that should be returned")
-            .that(&firefox)
-            .is_some()
-            .is_equal_to(PathBuf::from(
-                "/usr/share/icons/hicolor/48x48/apps/firefox.png",
-            ));
-    }
-
-    #[test]
-    fn hicolor_firefox_svg_fallback_to_png() {
-        let firefox = lookup("firefox").force_svg().find();
 
         asserting!("Lookup with no parameters should return an existing icon")
-            .that(&firefox)
+            .that(&libreoffice_writer)
             .is_some()
             .is_equal_to(PathBuf::from(
-                "/usr/share/icons/hicolor/16x16/apps/firefox.png",
+                "/usr/share/icons/hicolor/scalable/apps/libreoffice-writer.svg",
+            ));
+    }
+
+    #[test]
+    fn gnome_preferences_desktop_theme() {
+        let preferences_desktop_theme = lookup("preferences-desktop-theme")
+            .force_svg()
+            .with_size(128)
+            .find();
+
+        asserting!("Lookup with no parameters should return an existing icon")
+            .that(&preferences_desktop_theme)
+            .is_some()
+            .is_equal_to(PathBuf::from(
+                "/usr/share/icons/gnome/256x256/apps/preferences-desktop-theme.png",
             ));
     }
 
@@ -523,11 +547,11 @@ mod test {
     }
 
     #[test]
-    fn vscode_pixmap() {
+    fn local_slack() {
         assert_eq!(
-            lookup("vscode").find(),
+            lookup("slack").find(),
-            Some(PathBuf::from("/usr/share/pixmaps/vscode.png")),
+            Some(PathBuf::from("/usr/share/pixmaps/slack.png")),
-            "Is VS Code installed locally on the host?"
+            "Is slack installed locally on the host?"
         );
     }
 
@@ -553,15 +577,6 @@ mod test {
         );
     }
 
-    #[test]
-    fn ubuntu_additional_drivers() {
-        assert_eq!(
-            lookup("jockey").find(),
-            Some(PathBuf::from("/usr/share/icons/Yaru/24x24/apps/jockey.png")),
-            "Is the gnome icon theme installed?"
-        );
-    }
-
     #[test]
     #[cfg(feature = "local_tests")]
     fn theme_lookup() {

src/theme/directories.rs

@@ -12,7 +12,7 @@ pub struct Directory<'a> {
 impl Directory<'_> {
     pub fn directory_size_distance(&self, size: i16, scale: i16) -> i16 {
         match self.type_ {
-            DirectoryType::Fixed => self.size * self.scale - size * scale,
+            DirectoryType::Fixed => (self.size * self.scale) - (size * scale),
 
             DirectoryType::Scalable => {
                 let scaled_requested_size = size * scale;
@@ -22,7 +22,7 @@ impl Directory<'_> {
                 } else {
                     let max_scaled_size = self.maxsize * self.scale;
                     if scaled_requested_size < max_scaled_size {
-                        scaled_requested_size - max_scaled_size
+                        max_scaled_size - scaled_requested_size
                     } else {
                         0
                     }

src/theme/mod.rs

@@ -55,9 +55,9 @@ impl Theme {
         )
     }
 
-    fn try_fold_icon_path<'a>(
+    fn try_fold_icon_path(
         &self,
-        dir_names: Vec<(&'a str, i16, bool)>,
+        dir_names: Vec<(&str, i16, bool)>,
         name: &str,
         prefer_svg: bool,
     ) -> Option<PathBuf> {
@@ -114,14 +114,24 @@ impl Theme {
         );
 
         unsorted.sort_by(|a, b| {
-            let ordering = if prefer_svg {
+            if prefer_svg && (a.2 || b.2) {
-                b.2.cmp(&a.2)
+                if a.2 == b.2 {
+                    a.1.cmp(&b.1)
+                } else {
+                    b.2.cmp(&a.2)
+                }
+            } else if a.1 == b.1 {
+                Ordering::Equal
+            } else if a.1 == 0 {
+                Ordering::Less
+            } else if b.1 == 0 {
+                Ordering::Greater
+            } else if a.1 == (size * scale) as i16 {
+                Ordering::Less
+            } else if b.1 == (size * scale) as i16 {
+                Ordering::Greater
             } else {
-                a.2.cmp(&b.2)
+                a.1.cmp(&b.1)
-            };
-            match ordering {
-                Ordering::Equal => a.1.cmp(&b.1),
-                _ => ordering,
             }
         });
 

src/walk_dir.rs

@@ -0,0 +1,100 @@
+// Copyright 2026 System76 <info@system76.com>
+// SPDX-License-Identifier: MIT
+
+//! Search for files within multiple directories. Follows symlinks, avoids loops, and
+//! limits the max depth to 5.
+
+use std::{
+    collections::{BTreeSet, VecDeque},
+    fs,
+    path::PathBuf,
+};
+
+const MAX_DEPTH: usize = 5;
+
+pub struct Iter {
+    directories_to_walk: VecDeque<(PathBuf, usize)>,
+    actively_walking: Option<VecDeque<(PathBuf, usize)>>,
+    visited: BTreeSet<PathBuf>,
+}
+
+impl Iter {
+    /// Directories will be processed in order.
+    #[inline]
+    pub fn new<I: Iterator<Item = PathBuf>>(directories_to_walk: I) -> Self {
+        Self {
+            directories_to_walk: directories_to_walk.map(|dir| (dir, 0)).collect(),
+            actively_walking: None,
+            visited: BTreeSet::default(),
+        }
+    }
+}
+
+impl Iterator for Iter {
+    type Item = PathBuf;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        'outer: loop {
+            let mut paths = match self.actively_walking.take() {
+                Some(dir) => dir,
+                None => {
+                    while let Some((mut path, depth)) = self.directories_to_walk.pop_front() {
+                        path = path.canonicalize().map_or(path, |canonical| canonical);
+                        self.visited.insert(path.clone());
+                        match fs::read_dir(&path) {
+                            Ok(dir) => {
+                                self.actively_walking = Some({
+                                    // Pre-sort the walked directories as order of parsing affects appid matches.
+                                    let mut entries = dir
+                                        .filter_map(Result::ok)
+                                        .map(|entry| (entry.path(), depth))
+                                        .collect::<VecDeque<_>>();
+                                    entries.make_contiguous().sort_unstable();
+                                    entries
+                                });
+
+                                continue 'outer;
+                            }
+
+                            // Skip directories_to_walk which could not be read or that were already visited
+                            _ => continue,
+                        }
+                    }
+
+                    return None;
+                }
+            };
+
+            'inner: while let Some((mut path, mut depth)) = paths.pop_front() {
+                if !path.exists() {
+                    continue 'inner;
+                }
+
+                if path.is_dir() {
+                    depth += 1;
+
+                    if MAX_DEPTH == depth {
+                        continue;
+                    }
+
+                    path = match path.canonicalize() {
+                        Ok(canonicalized) => canonicalized,
+                        Err(_) => continue 'inner,
+                    };
+                }
+
+                if let Ok(metadata) = path.metadata() {
+                    if metadata.is_dir() {
+                        // Skip visited directories to mitigate against file system loops
+                        if self.visited.insert(path.clone()) {
+                            self.directories_to_walk.push_front((path, depth));
+                        }
+                    } else if metadata.is_file() {
+                        self.actively_walking = Some(paths);
+                        return Some(path);
+                    }
+                }
+            }
+        }
+    }
+}