From 54520b0a555703cf5af506ab2eba9a08f4b630c5 Mon Sep 17 00:00:00 2001 From: Michael Aaron Murphy Date: Mon, 1 Dec 2025 21:26:56 +0100 Subject: [PATCH] fix: more optimizations and test case fixes --- src/lib.rs | 38 +++++---- src/theme/directories.rs | 28 ++---- src/theme/mod.rs | 178 ++++++++++++++++++++++----------------- src/theme/parse.rs | 33 +++++--- 4 files changed, 151 insertions(+), 126 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index bcf21e5..964e930 100644 --- a/src/lib.rs +++ b/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 mut path = theme_base_dir.clone(); - 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) + let extensions = if self.force_svg { + [".svg", ".png", ".xpm"] } 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 { diff --git a/src/theme/directories.rs b/src/theme/directories.rs index bdd3744..ca38f94 100644 --- a/src/theme/directories.rs +++ b/src/theme/directories.rs @@ -10,30 +10,12 @@ pub struct Directory<'a> { } impl Directory<'_> { - pub fn match_size(&self, size: u16, scale: u16) -> bool { - 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; - + pub fn directory_size_distance(&self, size: i16, scale: i16) -> i16 { 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 { diff --git a/src/theme/mod.rs b/src/theme/mod.rs index d23d3ea..a73bf3c 100644 --- a/src/theme/mod.rs +++ b/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 { let file = read_ini_theme(&self.index).ok()?; - self.try_get_icon_exact_size(file.as_ref(), name, size, scale, force_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 { - 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 + 'a { - self.get_all_directories(file) - .filter(move |directory| directory.match_size(size, scale)) - .map(|dir| dir.name) + self.try_get_icon_closest_size(file.as_ref(), name, size, scale, prefer_svg) } #[inline] @@ -69,37 +46,54 @@ impl Theme { name: &str, size: u16, scale: u16, - force_svg: bool, + prefer_svg: bool, ) -> Option { - 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, + dir_names: Vec<(&'a str, i16, bool)>, name: &str, - force_svg: bool, + prefer_svg: bool, ) -> Option { - dir_names - .try_fold(self.path().clone(), move |mut path, dir_name| { - path.push(dir_name); - if try_build_icon_path(&mut path, name, force_svg) { - ControlFlow::Break(path) - } else { - let components = dir_name - .as_bytes() - .iter() - .fold(2, |n, c| n + (*c == b'/') as u32) - as usize; + let extensions = if prefer_svg { + [".svg", ".png", ".xpm"] + } else { + [".png", ".svg", ".xpm"] + }; - for _ in 0..components { - path.pop(); - } + extensions.into_iter().find_map(|ext| { + 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) - } - }) - .break_value() + for _ in 0..components { + path.pop(); + } + + 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 + 'a { - let dirs = self.get_all_directories(file); + prefer_svg: bool, + ) -> 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| { - let distance = directory.directory_size_distance(size, scale); - if distance < i16::MAX { - let a = distance.abs(); - let pos = sorted - .binary_search_by(|(_, b)| b.cmp(&a)) - .unwrap_or_else(|pos| pos); - sorted.insert(pos, (directory.name, a)); + unsorted.sort_by(|a, b| { + let ordering = if prefer_svg { + b.2.cmp(&a.2) + } else { + a.2.cmp(&b.2) + }; + match ordering { + Ordering::Equal => a.1.cmp(&b.1), + _ => ordering, } + }); - sorted - }) - .into_iter() - .map(|(name, _)| name) + unsorted } 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 { - let mut name_buf = String::with_capacity(name.len() + 4); +pub(super) fn try_build_icon_path<'a>( + 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, &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") - } + try_build_ext(path, name_buf, name, extension) } #[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", )); } } diff --git a/src/theme/parse.rs b/src/theme/parse.rs index 73f3376..61cde2d 100644 --- a/src/theme/parse.rs +++ b/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> { } }; - 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 + '_ 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 + '_ 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; + } } }