Fix 5 deferred performance/UX issues from audit
M8: Pre-compile regex once before rename preview loop instead of recompiling per file. Adds apply_simple_compiled() to RenameConfig. M9: Cache font data in watermark module using OnceLock (default font) and Mutex<HashMap> (named fonts) to avoid repeated filesystem walks during preview updates. M12: Add 150ms debounce to watermark opacity, rotation, margin, and scale sliders to avoid spawning preview threads on every pixel of slider movement. M13: Add 150ms debounce to compress per-format quality sliders (JPEG, PNG, WebP, AVIF) for the same reason. M14: Move thumbnail loading to background threads instead of blocking the GTK main loop. Each thumbnail is decoded via image crate in a spawned thread and delivered to the main thread via channel polling.
This commit is contained in:
@@ -318,6 +318,11 @@ pub struct RenameConfig {
|
||||
fn default_counter_position() -> u32 { 3 }
|
||||
|
||||
impl RenameConfig {
|
||||
/// Pre-compile the regex for batch use. Call once before a loop of apply_simple_compiled.
|
||||
pub fn compile_regex(&self) -> Option<regex::Regex> {
|
||||
rename::compile_rename_regex(&self.regex_find)
|
||||
}
|
||||
|
||||
pub fn apply_simple(&self, original_name: &str, extension: &str, index: u32) -> String {
|
||||
// 1. Apply regex find-and-replace on the original name
|
||||
let working_name = rename::apply_regex_replace(original_name, &self.regex_find, &self.regex_replace);
|
||||
@@ -378,4 +383,68 @@ impl RenameConfig {
|
||||
|
||||
format!("{}.{}", result, extension)
|
||||
}
|
||||
|
||||
/// Like apply_simple but uses a pre-compiled regex (avoids recompiling per file).
|
||||
pub fn apply_simple_compiled(&self, original_name: &str, extension: &str, index: u32, compiled_re: Option<®ex::Regex>) -> String {
|
||||
// 1. Apply regex find-and-replace on the original name
|
||||
let working_name = match compiled_re {
|
||||
Some(re) => rename::apply_regex_replace_compiled(original_name, re, &self.regex_replace),
|
||||
None => original_name.to_string(),
|
||||
};
|
||||
|
||||
// 2. Apply space replacement
|
||||
let working_name = rename::apply_space_replacement(&working_name, self.replace_spaces);
|
||||
|
||||
// 3. Apply special character filtering
|
||||
let working_name = rename::apply_special_chars(&working_name, self.special_chars);
|
||||
|
||||
// 4. Build counter string
|
||||
let counter_str = if self.counter_enabled {
|
||||
let counter = self.counter_start.saturating_add(index.saturating_sub(1));
|
||||
let padding = (self.counter_padding as usize).min(10);
|
||||
format!("{:0>width$}", counter, width = padding)
|
||||
} else {
|
||||
String::new()
|
||||
};
|
||||
|
||||
let has_counter = self.counter_enabled && !counter_str.is_empty();
|
||||
|
||||
// 5. Assemble parts based on counter position
|
||||
let mut result = String::new();
|
||||
|
||||
if has_counter && self.counter_position == 0 {
|
||||
result.push_str(&counter_str);
|
||||
result.push('_');
|
||||
}
|
||||
|
||||
result.push_str(&self.prefix);
|
||||
|
||||
if has_counter && self.counter_position == 1 {
|
||||
result.push_str(&counter_str);
|
||||
result.push('_');
|
||||
}
|
||||
|
||||
if has_counter && self.counter_position == 4 {
|
||||
result.push_str(&counter_str);
|
||||
} else {
|
||||
result.push_str(&working_name);
|
||||
}
|
||||
|
||||
if has_counter && self.counter_position == 2 {
|
||||
result.push('_');
|
||||
result.push_str(&counter_str);
|
||||
}
|
||||
|
||||
result.push_str(&self.suffix);
|
||||
|
||||
if has_counter && self.counter_position == 3 {
|
||||
result.push('_');
|
||||
result.push_str(&counter_str);
|
||||
}
|
||||
|
||||
// 6. Apply case conversion
|
||||
let result = rename::apply_case_conversion(&result, self.case_mode);
|
||||
|
||||
format!("{}.{}", result, extension)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,16 +75,34 @@ pub fn apply_watermark(
|
||||
}
|
||||
}
|
||||
|
||||
/// Cache for font data to avoid repeated filesystem walks during preview updates
|
||||
static FONT_CACHE: std::sync::Mutex<Option<FontCache>> = std::sync::Mutex::new(None);
|
||||
static DEFAULT_FONT_CACHE: std::sync::OnceLock<Option<Vec<u8>>> = std::sync::OnceLock::new();
|
||||
|
||||
struct FontCache {
|
||||
entries: std::collections::HashMap<String, Vec<u8>>,
|
||||
}
|
||||
|
||||
fn find_system_font(family: Option<&str>) -> Result<Vec<u8>> {
|
||||
// If a specific font family was requested, try to find it via fontconfig
|
||||
// If a specific font family was requested, check the cache first
|
||||
if let Some(name) = family {
|
||||
if !name.is_empty() {
|
||||
// Try common font paths with the family name
|
||||
let name_lower = name.to_lowercase();
|
||||
|
||||
// Check cache
|
||||
if let Ok(cache) = FONT_CACHE.lock() {
|
||||
if let Some(ref c) = *cache {
|
||||
if let Some(data) = c.entries.get(&name_lower) {
|
||||
return Ok(data.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Cache miss - search filesystem
|
||||
let search_dirs = [
|
||||
"/usr/share/fonts",
|
||||
"/usr/local/share/fonts",
|
||||
];
|
||||
let name_lower = name.to_lowercase();
|
||||
for dir in &search_dirs {
|
||||
if let Ok(entries) = walkdir(std::path::Path::new(dir)) {
|
||||
for path in entries {
|
||||
@@ -97,6 +115,13 @@ fn find_system_font(family: Option<&str>) -> Result<Vec<u8>> {
|
||||
&& (file_name.contains("regular") || (!file_name.contains("bold") && !file_name.contains("italic")))
|
||||
{
|
||||
if let Ok(data) = std::fs::read(&path) {
|
||||
// Store in cache
|
||||
if let Ok(mut cache) = FONT_CACHE.lock() {
|
||||
let c = cache.get_or_insert_with(|| FontCache {
|
||||
entries: std::collections::HashMap::new(),
|
||||
});
|
||||
c.entries.insert(name_lower, data.clone());
|
||||
}
|
||||
return Ok(data);
|
||||
}
|
||||
}
|
||||
@@ -106,7 +131,8 @@ fn find_system_font(family: Option<&str>) -> Result<Vec<u8>> {
|
||||
}
|
||||
}
|
||||
|
||||
// Fall back to default system fonts
|
||||
// Fall back to default system fonts (cached via OnceLock)
|
||||
let default = DEFAULT_FONT_CACHE.get_or_init(|| {
|
||||
let candidates = [
|
||||
"/usr/share/fonts/truetype/dejavu/DejaVuSans.ttf",
|
||||
"/usr/share/fonts/TTF/DejaVuSans.ttf",
|
||||
@@ -115,17 +141,21 @@ fn find_system_font(family: Option<&str>) -> Result<Vec<u8>> {
|
||||
"/usr/share/fonts/truetype/noto/NotoSans-Regular.ttf",
|
||||
"/usr/share/fonts/noto/NotoSans-Regular.ttf",
|
||||
];
|
||||
|
||||
for path in &candidates {
|
||||
if let Ok(data) = std::fs::read(path) {
|
||||
return Ok(data);
|
||||
return Some(data);
|
||||
}
|
||||
}
|
||||
None
|
||||
});
|
||||
|
||||
Err(PixstripError::Processing {
|
||||
match default {
|
||||
Some(data) => Ok(data.clone()),
|
||||
None => Err(PixstripError::Processing {
|
||||
operation: "watermark".into(),
|
||||
reason: "No system font found for text watermark".into(),
|
||||
})
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
/// Recursively walk a directory and collect file paths (max depth 5)
|
||||
|
||||
@@ -702,18 +702,31 @@ pub fn build_compress_page(state: &AppState) -> adw::NavigationPage {
|
||||
});
|
||||
}
|
||||
|
||||
// Per-format slider handlers: update config, set preview to that format, refresh
|
||||
// Per-slider debounce counters (separate to avoid cross-slider cancellation)
|
||||
let jpeg_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let png_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let webp_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let avif_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
|
||||
// Per-format slider handlers: update config, set preview to that format, refresh (debounced)
|
||||
{
|
||||
let jc = state.job_config.clone();
|
||||
let row = jpeg_row.clone();
|
||||
let pc = preview_comp.clone();
|
||||
let up = update_preview.clone();
|
||||
let did = jpeg_debounce.clone();
|
||||
jpeg_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as u8;
|
||||
jc.borrow_mut().jpeg_quality = val;
|
||||
row.set_subtitle(&format!("{}", val));
|
||||
pc.set(PreviewCompression::Jpeg(val));
|
||||
up(false);
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(false); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -721,12 +734,19 @@ pub fn build_compress_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = png_row.clone();
|
||||
let pc = preview_comp.clone();
|
||||
let up = update_preview.clone();
|
||||
let did = png_debounce.clone();
|
||||
png_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as u8;
|
||||
jc.borrow_mut().png_level = val;
|
||||
row.set_subtitle(&format!("{}", val));
|
||||
pc.set(PreviewCompression::Png(val));
|
||||
up(false);
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(false); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -734,12 +754,19 @@ pub fn build_compress_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = webp_row.clone();
|
||||
let pc = preview_comp.clone();
|
||||
let up = update_preview.clone();
|
||||
let did = webp_debounce.clone();
|
||||
webp_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as u8;
|
||||
jc.borrow_mut().webp_quality = val;
|
||||
row.set_subtitle(&format!("{}", val));
|
||||
pc.set(PreviewCompression::WebP(val));
|
||||
up(false);
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(false); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -756,12 +783,19 @@ pub fn build_compress_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = avif_row.clone();
|
||||
let pc = preview_comp.clone();
|
||||
let up = update_preview.clone();
|
||||
let did = avif_debounce.clone();
|
||||
avif_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as u8;
|
||||
jc.borrow_mut().avif_quality = val;
|
||||
row.set_subtitle(&format!("{}", val));
|
||||
pc.set(PreviewCompression::Avif(val));
|
||||
up(false);
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(false); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
|
||||
@@ -732,20 +732,50 @@ fn build_loaded_state(state: &AppState) -> gtk::Box {
|
||||
};
|
||||
unsafe { thumb_stack.set_data("bind-gen", bind_gen); }
|
||||
|
||||
// Load thumbnail asynchronously
|
||||
// Load thumbnail in background thread to avoid blocking the UI
|
||||
let thumb_stack_c = thumb_stack.clone();
|
||||
let picture_c = picture.clone();
|
||||
let path_c = path.clone();
|
||||
glib::idle_add_local_once(move || {
|
||||
let (tx, rx) = std::sync::mpsc::channel::<Option<Vec<u8>>>();
|
||||
std::thread::spawn(move || {
|
||||
let result = (|| -> Option<Vec<u8>> {
|
||||
let img = image::open(&path_c).ok()?;
|
||||
let thumb = img.resize(
|
||||
(THUMB_SIZE * 2) as u32,
|
||||
(THUMB_SIZE * 2) as u32,
|
||||
image::imageops::FilterType::Triangle,
|
||||
);
|
||||
let mut buf = Vec::new();
|
||||
thumb.write_to(
|
||||
&mut std::io::Cursor::new(&mut buf),
|
||||
image::ImageFormat::Png,
|
||||
).ok()?;
|
||||
Some(buf)
|
||||
})();
|
||||
let _ = tx.send(result);
|
||||
});
|
||||
glib::timeout_add_local(std::time::Duration::from_millis(50), move || {
|
||||
let current: u32 = unsafe {
|
||||
thumb_stack_c.data::<u32>("bind-gen")
|
||||
.map(|p| *p.as_ref())
|
||||
.unwrap_or(0)
|
||||
};
|
||||
if current != bind_gen {
|
||||
return; // Item was recycled; skip stale load
|
||||
return glib::ControlFlow::Break;
|
||||
}
|
||||
match rx.try_recv() {
|
||||
Ok(Some(bytes)) => {
|
||||
let gbytes = glib::Bytes::from(&bytes);
|
||||
if let Ok(texture) = gtk::gdk::Texture::from_bytes(&gbytes) {
|
||||
picture_c.set_paintable(Some(&texture));
|
||||
thumb_stack_c.set_visible_child_name("picture");
|
||||
}
|
||||
glib::ControlFlow::Break
|
||||
}
|
||||
Ok(None) => glib::ControlFlow::Break, // decode failed, leave placeholder
|
||||
Err(std::sync::mpsc::TryRecvError::Empty) => glib::ControlFlow::Continue,
|
||||
Err(_) => glib::ControlFlow::Break,
|
||||
}
|
||||
load_thumbnail(&path_c, &picture_c, &thumb_stack_c);
|
||||
});
|
||||
|
||||
// Set checkbox state
|
||||
@@ -935,22 +965,6 @@ fn find_check_button(widget: >k::Widget) -> Option<gtk::CheckButton> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Load a thumbnail for the given path into the Picture widget
|
||||
fn load_thumbnail(path: &std::path::Path, picture: >k::Picture, stack: >k::Stack) {
|
||||
// Use GdkPixbuf to load at reduced size for speed
|
||||
match gtk::gdk_pixbuf::Pixbuf::from_file_at_scale(path, THUMB_SIZE * 2, THUMB_SIZE * 2, true) {
|
||||
Ok(pixbuf) => {
|
||||
#[allow(deprecated)]
|
||||
let texture = gtk::gdk::Texture::for_pixbuf(&pixbuf);
|
||||
picture.set_paintable(Some(&texture));
|
||||
stack.set_visible_child_name("picture");
|
||||
}
|
||||
Err(_) => {
|
||||
// Leave placeholder visible
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Download an image from a URL to a temporary file
|
||||
fn download_image_url(url: &str) -> Option<std::path::PathBuf> {
|
||||
let temp_dir = std::env::temp_dir().join("pixstrip-downloads");
|
||||
|
||||
@@ -499,18 +499,7 @@ pub fn build_rename_page(state: &AppState) -> adw::NavigationPage {
|
||||
let mut ext_counts: HashMap<String, usize> = HashMap::new();
|
||||
let mut longest_name = 0usize;
|
||||
|
||||
for (i, path) in loaded.iter().enumerate() {
|
||||
let name = path.file_stem().and_then(|s| s.to_str()).unwrap_or("file");
|
||||
let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("jpg");
|
||||
|
||||
*ext_counts.entry(ext.to_string()).or_insert(0) += 1;
|
||||
|
||||
let result = if !cfg.rename_template.is_empty() {
|
||||
let counter = cfg.rename_counter_start + i as u32;
|
||||
pixstrip_core::operations::rename::apply_template(
|
||||
&cfg.rename_template, name, ext, counter, None,
|
||||
)
|
||||
} else {
|
||||
// Pre-compile regex once for the entire batch (avoids recompiling per file)
|
||||
let rename_cfg = pixstrip_core::operations::RenameConfig {
|
||||
prefix: cfg.rename_prefix.clone(),
|
||||
suffix: cfg.rename_suffix.clone(),
|
||||
@@ -525,7 +514,21 @@ pub fn build_rename_page(state: &AppState) -> adw::NavigationPage {
|
||||
regex_find: cfg.rename_find.clone(),
|
||||
regex_replace: cfg.rename_replace.clone(),
|
||||
};
|
||||
rename_cfg.apply_simple(name, ext, (i + 1) as u32)
|
||||
let compiled_re = rename_cfg.compile_regex();
|
||||
|
||||
for (i, path) in loaded.iter().enumerate() {
|
||||
let name = path.file_stem().and_then(|s| s.to_str()).unwrap_or("file");
|
||||
let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("jpg");
|
||||
|
||||
*ext_counts.entry(ext.to_string()).or_insert(0) += 1;
|
||||
|
||||
let result = if !cfg.rename_template.is_empty() {
|
||||
let counter = cfg.rename_counter_start + i as u32;
|
||||
pixstrip_core::operations::rename::apply_template(
|
||||
&cfg.rename_template, name, ext, counter, None,
|
||||
)
|
||||
} else {
|
||||
rename_cfg.apply_simple_compiled(name, ext, (i + 1) as u32, compiled_re.as_ref())
|
||||
};
|
||||
|
||||
if result.len() > longest_name {
|
||||
|
||||
@@ -686,19 +686,32 @@ pub fn build_watermark_page(state: &AppState) -> adw::NavigationPage {
|
||||
});
|
||||
}
|
||||
|
||||
// Per-slider debounce counters (separate to avoid cross-slider cancellation)
|
||||
let opacity_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let rotation_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let margin_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
let scale_debounce: Rc<Cell<u32>> = Rc::new(Cell::new(0));
|
||||
|
||||
// Opacity slider
|
||||
{
|
||||
let jc = state.job_config.clone();
|
||||
let row = opacity_row.clone();
|
||||
let up = update_preview.clone();
|
||||
let rst = opacity_reset.clone();
|
||||
let did = opacity_debounce.clone();
|
||||
opacity_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as i32;
|
||||
let opacity = val as f32 / 100.0;
|
||||
jc.borrow_mut().watermark_opacity = opacity;
|
||||
row.set_subtitle(&format!("{}%", val));
|
||||
rst.set_sensitive(val != 50);
|
||||
up();
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -714,12 +727,19 @@ pub fn build_watermark_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = rotation_row.clone();
|
||||
let up = update_preview.clone();
|
||||
let rst = rotation_reset.clone();
|
||||
let did = rotation_debounce.clone();
|
||||
rotation_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as i32;
|
||||
jc.borrow_mut().watermark_rotation = val;
|
||||
row.set_subtitle(&format!("{} degrees", val));
|
||||
rst.set_sensitive(val != 0);
|
||||
up();
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -745,12 +765,19 @@ pub fn build_watermark_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = margin_row.clone();
|
||||
let up = update_preview.clone();
|
||||
let rst = margin_reset.clone();
|
||||
let did = margin_debounce.clone();
|
||||
margin_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as i32;
|
||||
jc.borrow_mut().watermark_margin = val as u32;
|
||||
row.set_subtitle(&format!("{} px", val));
|
||||
rst.set_sensitive(val != 10);
|
||||
up();
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
@@ -766,12 +793,19 @@ pub fn build_watermark_page(state: &AppState) -> adw::NavigationPage {
|
||||
let row = scale_row.clone();
|
||||
let up = update_preview.clone();
|
||||
let rst = scale_reset.clone();
|
||||
let did = scale_debounce.clone();
|
||||
scale_scale.connect_value_changed(move |scale| {
|
||||
let val = scale.value().round() as i32;
|
||||
jc.borrow_mut().watermark_scale = val as f32;
|
||||
row.set_subtitle(&format!("{}%", val));
|
||||
rst.set_sensitive((val - 20).abs() > 0);
|
||||
up();
|
||||
let up = up.clone();
|
||||
let did = did.clone();
|
||||
let id = did.get().wrapping_add(1);
|
||||
did.set(id);
|
||||
glib::timeout_add_local_once(std::time::Duration::from_millis(150), move || {
|
||||
if did.get() == id { up(); }
|
||||
});
|
||||
});
|
||||
}
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user