fix: more optimizations and test case fixes

src/lib.rs

@@ -58,6 +58,7 @@ use crate::cache::{CACHE, CacheEntry};
 use crate::theme::{THEMES, Theme, try_build_icon_path};
 use std::hash::{Hash, Hasher};
 use std::io::BufRead;
+use std::ops::ControlFlow;
 use std::path::PathBuf;
 use std::time::Instant;
 
@@ -345,23 +346,28 @@ impl<'a> LookupBuilder<'a> {
                     // Ubuntu applications may require Yaru
                     .or_else(|| self.search_inherited_theme(searched_themes, "Yaru".as_bytes()))
                     .or_else(|| {
-                        for theme_base_dir in BASE_PATHS.iter() {
+                        let extensions = if self.force_svg {
-                            let mut path = theme_base_dir.clone();
+                            [".svg", ".png", ".xpm"]
-                            if try_build_icon_path(&mut path, self.name, self.force_svg) {
-                                return Some(path);
-                            }
-                        }
-                        None
-                    })
-                    .or_else(|| {
-                        let p = PathBuf::from(&self.name);
-                        if let (Some(name), Some(parent)) = (p.file_stem(), p.parent()) {
-                            let mut path = parent.to_path_buf();
-                            try_build_icon_path(&mut path, &name.to_string_lossy(), self.force_svg)
-                                .then_some(path)
                         } else {
-                            None
+                            [".png", ".svg", ".xpm"]
-                        }
+                        };
+
+                        let mut name_buf = String::new();
+
+                        extensions
+                            .into_iter()
+                            .try_for_each(|ext| {
+                                BASE_PATHS.iter().try_for_each(|theme_base_dir| {
+                                    let mut path = theme_base_dir.clone();
+                                    if try_build_icon_path(&mut path, &mut name_buf, self.name, ext)
+                                    {
+                                        return ControlFlow::Break(path);
+                                    }
+                                    name_buf.clear();
+                                    ControlFlow::Continue(())
+                                })
+                            })
+                            .break_value()
                     });
 
                 if self.cache {

src/theme/directories.rs

@@ -10,30 +10,12 @@ pub struct Directory<'a> {
 }
 
 impl Directory<'_> {
-    pub fn match_size(&self, size: u16, scale: u16) -> bool {
+    pub fn directory_size_distance(&self, size: i16, scale: i16) -> i16 {
-        let scale = scale as i16;
-        let size = size as i16;
-
-        if self.scale != scale {
-            false
-        } else {
-            match self.type_ {
-                DirectoryType::Fixed => self.size == size,
-                DirectoryType::Scalable => self.minsize <= size && size <= self.maxsize,
-                DirectoryType::Threshold => {
-                    self.size - self.threshold <= size && size <= self.size + self.threshold
-                }
-            }
-        }
-    }
-
-    pub fn directory_size_distance(&self, size: u16, scale: u16) -> i16 {
-        let scale = scale as i16;
-        let scaled_requested_size = size as i16 * scale;
-
         match self.type_ {
-            DirectoryType::Fixed => self.size * self.scale - scaled_requested_size,
+            DirectoryType::Fixed => self.size * self.scale - size * scale,
+
             DirectoryType::Scalable => {
+                let scaled_requested_size = size * scale;
                 let min_scaled_size = self.minsize * self.scale;
                 if scaled_requested_size < min_scaled_size {
                     min_scaled_size - scaled_requested_size
@@ -46,7 +28,9 @@ impl Directory<'_> {
                     }
                 }
             }
+
             DirectoryType::Threshold => {
+                let scaled_requested_size = size * scale;
                 if scaled_requested_size < (self.size - self.threshold) * scale {
                     self.minsize * self.scale - scaled_requested_size
                 } else if scaled_requested_size > (self.size + self.threshold) * scale {

src/theme/mod.rs

@@ -1,6 +1,8 @@
+use crate::theme::directories::DirectoryType;
 use crate::theme::paths::ThemePath;
 use memmap2::Mmap;
 pub(crate) use paths::BASE_PATHS;
+use std::cmp::Ordering;
 use std::collections::BTreeMap;
 use std::ops::ControlFlow;
 use std::os::unix::ffi::OsStrExt;
@@ -31,35 +33,10 @@ impl Theme {
         name: &str,
         size: u16,
         scale: u16,
-        force_svg: bool,
+        prefer_svg: bool,
     ) -> Option<PathBuf> {
         let file = read_ini_theme(&self.index).ok()?;
-        self.try_get_icon_exact_size(file.as_ref(), name, size, scale, force_svg)
+        self.try_get_icon_closest_size(file.as_ref(), name, size, scale, prefer_svg)
-            .or_else(|| self.try_get_icon_closest_size(file.as_ref(), name, size, scale, force_svg))
-    }
-
-    #[inline]
-    fn try_get_icon_exact_size(
-        &self,
-        file: &[u8],
-        name: &str,
-        size: u16,
-        scale: u16,
-        force_svg: bool,
-    ) -> Option<PathBuf> {
-        self.try_fold_icon_path(self.match_size(file, size, scale), name, force_svg)
-    }
-
-    #[inline]
-    fn match_size<'a>(
-        &'a self,
-        file: &'a [u8],
-        size: u16,
-        scale: u16,
-    ) -> impl Iterator<Item = &'a str> + 'a {
-        self.get_all_directories(file)
-            .filter(move |directory| directory.match_size(size, scale))
-            .map(|dir| dir.name)
     }
 
     #[inline]
@@ -69,37 +46,54 @@ impl Theme {
         name: &str,
         size: u16,
         scale: u16,
-        force_svg: bool,
+        prefer_svg: bool,
     ) -> Option<PathBuf> {
-        self.try_fold_icon_path(self.closest_match_size(file, size, scale), name, force_svg)
+        self.try_fold_icon_path(
+            self.closest_match_size(file, size, scale, prefer_svg),
+            name,
+            prefer_svg,
+        )
     }
 
     fn try_fold_icon_path<'a>(
         &self,
-        mut dir_names: impl Iterator<Item = &'a str>,
+        dir_names: Vec<(&'a str, i16, bool)>,
         name: &str,
-        force_svg: bool,
+        prefer_svg: bool,
     ) -> Option<PathBuf> {
-        dir_names
+        let extensions = if prefer_svg {
-            .try_fold(self.path().clone(), move |mut path, dir_name| {
+            [".svg", ".png", ".xpm"]
-                path.push(dir_name);
+        } else {
-                if try_build_icon_path(&mut path, name, force_svg) {
+            [".png", ".svg", ".xpm"]
-                    ControlFlow::Break(path)
+        };
-                } else {
-                    let components = dir_name
-                        .as_bytes()
-                        .iter()
-                        .fold(2, |n, c| n + (*c == b'/') as u32)
-                        as usize;
 
-                    for _ in 0..components {
+        extensions.into_iter().find_map(|ext| {
-                        path.pop();
+            dir_names
-                    }
+                .iter()
+                .try_fold(
+                    (self.path().clone(), String::new()),
+                    move |(mut path, mut name_buf), (dir_name, _, _)| {
+                        path.push(dir_name);
+                        if try_build_icon_path(&mut path, &mut name_buf, name, ext) {
+                            ControlFlow::Break(path)
+                        } else {
+                            name_buf.clear();
+                            let components = dir_name
+                                .as_bytes()
+                                .iter()
+                                .fold(2, |n, c| n + (*c == b'/') as u32)
+                                as usize;
 
-                    ControlFlow::Continue(path)
+                            for _ in 0..components {
-                }
+                                path.pop();
-            })
+                            }
-            .break_value()
+
+                            ControlFlow::Continue((path, name_buf))
+                        }
+                    },
+                )
+                .break_value()
+        })
     }
 
     fn closest_match_size<'a>(
@@ -107,23 +101,31 @@ impl Theme {
         file: &'a [u8],
         size: u16,
         scale: u16,
-    ) -> impl Iterator<Item = &'a str> + 'a {
+        prefer_svg: bool,
-        let dirs = self.get_all_directories(file);
+    ) -> Vec<(&'a str, i16, bool)> {
+        let mut unsorted = self.get_all_directories(file).fold(
+            Vec::<(&'a str, i16, bool)>::new(),
+            |mut unsorted, directory| {
+                let is_scalable = matches!(directory.type_, DirectoryType::Scalable);
+                let distance = directory.directory_size_distance(size as i16, scale as i16);
+                unsorted.push((directory.name, distance.abs(), is_scalable));
+                unsorted
+            },
+        );
 
-        dirs.fold(Vec::<(&'a str, i16)>::new(), |mut sorted, directory| {
+        unsorted.sort_by(|a, b| {
-            let distance = directory.directory_size_distance(size, scale);
+            let ordering = if prefer_svg {
-            if distance < i16::MAX {
+                b.2.cmp(&a.2)
-                let a = distance.abs();
+            } else {
-                let pos = sorted
+                a.2.cmp(&b.2)
-                    .binary_search_by(|(_, b)| b.cmp(&a))
+            };
-                    .unwrap_or_else(|pos| pos);
+            match ordering {
-                sorted.insert(pos, (directory.name, a));
+                Ordering::Equal => a.1.cmp(&b.1),
+                _ => ordering,
             }
+        });
 
-            sorted
+        unsorted
-        })
-        .into_iter()
-        .map(|(name, _)| name)
     }
 
     fn path(&self) -> &PathBuf {
@@ -131,19 +133,15 @@ impl Theme {
     }
 }
 
-pub(super) fn try_build_icon_path<'a>(path: &'a mut PathBuf, name: &str, force_svg: bool) -> bool {
+pub(super) fn try_build_icon_path<'a>(
-    let mut name_buf = String::with_capacity(name.len() + 4);
+    path: &'a mut PathBuf,
+    name_buf: &'a mut String,
+    name: &str,
+    extension: &'static str,
+) -> bool {
     name_buf.push_str(name);
     path.push(name);
-    if force_svg {
+    try_build_ext(path, name_buf, name, extension)
-        try_build_ext(path, &mut name_buf, name, ".svg")
-            || try_build_ext(path, &mut name_buf, name, ".png")
-            || try_build_ext(path, &mut name_buf, name, ".xmp")
-    } else {
-        try_build_ext(path, &mut name_buf, name, ".png")
-            || try_build_ext(path, &mut name_buf, name, ".svg")
-            || try_build_ext(path, &mut name_buf, name, ".xmp")
-    }
 }
 
 #[inline]
@@ -238,7 +236,7 @@ mod test {
             "{:?}",
             themes.iter().find_map(|t| {
                 let file = super::read_ini_theme(&t.index).ok()?;
-                t.try_get_icon_exact_size(file.as_ref(), "edit-delete-symbolic", 24, 1, false)
+                t.try_get_icon_closest_size(file.as_ref(), "edit-delete-symbolic", 24, 1, false)
             })
         );
     }
@@ -248,22 +246,46 @@ mod test {
         let themes = THEMES.get(&b"hicolor"[..]).unwrap();
         let icon = themes.iter().find_map(|t| {
             let file = super::read_ini_theme(&t.index).ok()?;
-            t.try_get_icon_exact_size(file.as_ref(), "blueman", 24, 1, true)
+            t.try_get_icon_closest_size(file.as_ref(), "blueman", 22, 1, false)
         });
         assert_that!(icon).is_some().is_equal_to(PathBuf::from(
             "/usr/share/icons/hicolor/22x22/apps/blueman.png",
         ));
     }
 
+    #[test]
+    fn should_get_png_first_92() {
+        let themes = THEMES.get(&b"hicolor"[..]).unwrap();
+        let icon = themes.iter().find_map(|t| {
+            let file = super::read_ini_theme(&t.index).ok()?;
+            t.try_get_icon_closest_size(file.as_ref(), "blueman", 92, 1, false)
+        });
+        assert_that!(icon).is_some().is_equal_to(PathBuf::from(
+            "/usr/share/icons/hicolor/96x96/apps/blueman.png",
+        ));
+    }
+
     #[test]
     fn should_get_svg_first() {
         let themes = THEMES.get(&b"hicolor"[..]).unwrap();
         let icon = themes.iter().find_map(|t| {
             let file = super::read_ini_theme(&t.index).ok()?;
-            t.try_get_icon_exact_size(file.as_ref(), "blueman", 24, 1, false)
+            t.try_get_icon_closest_size(file.as_ref(), "blueman", 24, 1, true)
         });
         assert_that!(icon).is_some().is_equal_to(PathBuf::from(
-            "/usr/share/icons/hicolor/22x22/apps/blueman.png",
+            "/usr/share/icons/hicolor/scalable/apps/blueman.svg",
+        ));
+    }
+
+    #[test]
+    fn should_get_svg_first_96() {
+        let themes = THEMES.get(&b"hicolor"[..]).unwrap();
+        let icon = themes.iter().find_map(|t| {
+            let file = super::read_ini_theme(&t.index).ok()?;
+            t.try_get_icon_closest_size(file.as_ref(), "blueman", 96, 1, true)
+        });
+        assert_that!(icon).is_some().is_equal_to(PathBuf::from(
+            "/usr/share/icons/hicolor/scalable/apps/blueman.svg",
         ));
     }
 }

src/theme/parse.rs

@@ -1,6 +1,6 @@
 use crate::theme::Theme;
 use crate::theme::directories::{Directory, DirectoryType};
-use bstr::{BStr, ByteSlice};
+use bstr::BStr;
 
 impl Theme {
     pub(super) fn get_all_directories<'a>(
@@ -10,27 +10,26 @@ impl Theme {
         let mut iterator = sections(file);
 
         std::iter::from_fn(move || {
+            let mut is_icon_theme = false;
             let mut name = "";
             let mut size = None;
             let mut max_size = None;
             let mut min_size = None;
             let mut threshold = None;
             let mut scale = None;
-            // let mut context = None;
             let mut dtype = DirectoryType::default();
 
             #[allow(clippy::while_let_on_iterator)]
             while let Some(event) = iterator.next() {
                 match event {
                     DirectorySection::Property(key, value) => {
-                        if name.is_empty() || name == "Icon Theme" {
+                        if is_icon_theme {
                             continue;
                         }
 
                         match key {
                             b"Size" => size = btoi::btoi(value).ok(),
                             b"Scale" => scale = btoi::btoi(value).ok(),
-                            // "Context" => context = Some(value),
                             b"Type" => dtype = DirectoryType::from(value),
                             b"MaxSize" => max_size = btoi::btoi(value).ok(),
                             b"MinSize" => min_size = btoi::btoi(value).ok(),
@@ -40,7 +39,11 @@ impl Theme {
                     }
 
                     DirectorySection::Section(new_name) => {
-                        name = std::str::from_utf8(new_name).unwrap_or("");
+                        let Ok(new_name) = std::str::from_utf8(new_name) else {
+                            return None;
+                        };
+                        is_icon_theme = new_name == "Icon Theme";
+                        name = new_name;
                         size = None;
                         max_size = None;
                         min_size = None;
@@ -50,7 +53,7 @@ impl Theme {
                     }
 
                     DirectorySection::EndSection => {
-                        if name.is_empty() || name == "Icon Theme" {
+                        if is_icon_theme {
                             continue;
                         }
 
@@ -60,7 +63,6 @@ impl Theme {
                             name,
                             size,
                             scale: scale.unwrap_or(1),
-                            // context,
                             type_: dtype,
                             maxsize: max_size.unwrap_or(size),
                             minsize: min_size.unwrap_or(size),
@@ -126,7 +128,11 @@ fn sections(file: &[u8]) -> impl Iterator<Item = DirectorySection<'_>> {
                 }
             };
 
-            let line = BStr::new(&file[prev..line_pos]).trim_ascii();
+            let line = BStr::new(unsafe {
+                // Indices from memchr are valid.
+                file.get_unchecked(prev..line_pos)
+            })
+            .trim_ascii();
             prev = line_pos + 1;
 
             if line.is_empty() {
@@ -161,7 +167,11 @@ fn icon_theme_section(file: &[u8]) -> impl Iterator<Item = (&[u8], &[u8])> + '_
     std::iter::from_fn(move || {
         loop {
             let line_pos = line_indices.next()?;
-            let line = BStr::new(&file[prev..line_pos]).trim_ascii();
+            let line = BStr::new(unsafe {
+                // Indices from memchr are valid.
+                file.get_unchecked(prev..line_pos)
+            })
+            .trim_ascii();
             prev = line_pos + 1;
 
             if line.is_empty() {
@@ -173,7 +183,10 @@ fn icon_theme_section(file: &[u8]) -> impl Iterator<Item = (&[u8], &[u8])> + '_
                     return None;
                 } else {
                     let section = &line[1..line.len() - 1];
-                    found_table = section == b"Icon Theme";
+                    found_table = true;
+                    if section != b"Icon Theme" {
+                        return None;
+                    }
                 }
             }