From 2024295f785d90b8e2150b31bbb8c2ce8b66351a Mon Sep 17 00:00:00 2001 From: Nicolas Date: Tue, 25 Mar 2025 10:17:31 +0100 Subject: [PATCH] Make ImageLoader use background thread (#5394) This is the same change as in #4069 but as this is stale I wanted to reopen a non stale PR Modifies ImageLoader's load function to use background threads for the image decoding work. This avoids blocking the main thread that is especially noticeable when loading many images at once. This was modelled after the other loader implementations that also use threads. * Closes * [x] I have followed the instructions in the PR template --------- Co-authored-by: lucasmerlin Co-authored-by: Emil Ernerfeldt --- crates/egui_demo_app/tests/test_demo_app.rs | 2 +- .../egui_extras/src/loaders/image_loader.rs | 96 ++++++++++++++----- deny.toml | 2 +- 3 files changed, 75 insertions(+), 25 deletions(-) diff --git a/crates/egui_demo_app/tests/test_demo_app.rs b/crates/egui_demo_app/tests/test_demo_app.rs index 0247b9fc..056bee49 100644 --- a/crates/egui_demo_app/tests/test_demo_app.rs +++ b/crates/egui_demo_app/tests/test_demo_app.rs @@ -67,7 +67,7 @@ fn test_demo_app() { } // Can't use Harness::run because fractal clock keeps requesting repaints - harness.run_steps(2); + harness.run_steps(4); results.add(harness.try_snapshot(&anchor.to_string())); } diff --git a/crates/egui_extras/src/loaders/image_loader.rs b/crates/egui_extras/src/loaders/image_loader.rs index af785f6f..4790c954 100644 --- a/crates/egui_extras/src/loaders/image_loader.rs +++ b/crates/egui_extras/src/loaders/image_loader.rs @@ -1,18 +1,21 @@ use ahash::HashMap; use egui::{ decode_animated_image_uri, - load::{BytesPoll, ImageLoadResult, ImageLoader, ImagePoll, LoadError, SizeHint}, + load::{Bytes, BytesPoll, ImageLoadResult, ImageLoader, ImagePoll, LoadError, SizeHint}, mutex::Mutex, ColorImage, }; use image::ImageFormat; -use std::{mem::size_of, path::Path, sync::Arc}; +use std::{mem::size_of, path::Path, sync::Arc, task::Poll}; -type Entry = Result, LoadError>; +#[cfg(not(target_arch = "wasm32"))] +use std::thread; + +type Entry = Poll, String>>; #[derive(Default)] pub struct ImageCrateLoader { - cache: Mutex>, + cache: Arc>>, } impl ImageCrateLoader { @@ -73,11 +76,69 @@ impl ImageLoader for ImageCrateLoader { return Err(LoadError::NotSupported); } - let mut cache = self.cache.lock(); - if let Some(entry) = cache.get(uri).cloned() { - match entry { + #[cfg(not(target_arch = "wasm32"))] + #[allow(clippy::unnecessary_wraps)] // needed here to match other return types + fn load_image( + ctx: &egui::Context, + uri: &str, + cache: &Arc>>, + bytes: &Bytes, + ) -> ImageLoadResult { + let uri = uri.to_owned(); + cache.lock().insert(uri.clone(), Poll::Pending); + + // Do the image parsing on a bg thread + thread::Builder::new() + .name(format!("egui_extras::ImageLoader::load({uri:?})")) + .spawn({ + let ctx = ctx.clone(); + let cache = cache.clone(); + + let uri = uri.clone(); + let bytes = bytes.clone(); + move || { + log::trace!("ImageLoader - started loading {uri:?}"); + let result = crate::image::load_image_bytes(&bytes) + .map(Arc::new) + .map_err(|err| err.to_string()); + log::trace!("ImageLoader - finished loading {uri:?}"); + let prev = cache.lock().insert(uri, Poll::Ready(result)); + debug_assert!(matches!(prev, Some(Poll::Pending))); + + ctx.request_repaint(); + } + }) + .expect("failed to spawn thread"); + + Ok(ImagePoll::Pending { size: None }) + } + + #[cfg(target_arch = "wasm32")] + fn load_image( + _ctx: &egui::Context, + uri: &str, + cache: &Arc>>, + bytes: &Bytes, + ) -> ImageLoadResult { + let mut cache_lock = cache.lock(); + log::trace!("started loading {uri:?}"); + let result = crate::image::load_image_bytes(bytes) + .map(Arc::new) + .map_err(|err| err.to_string()); + log::trace!("finished loading {uri:?}"); + cache_lock.insert(uri.into(), std::task::Poll::Ready(result.clone())); + match result { Ok(image) => Ok(ImagePoll::Ready { image }), - Err(err) => Err(err), + Err(err) => Err(LoadError::Loading(err)), + } + } + + let entry = self.cache.lock().get(uri).cloned(); + if let Some(entry) = entry { + match entry { + Poll::Ready(Ok(image)) => Ok(ImagePoll::Ready { image }), + Poll::Ready(Err(err)) => Err(LoadError::Loading(err)), + Poll::Pending => Ok(ImagePoll::Pending { size: None }), } } else { match ctx.try_load_bytes(uri) { @@ -90,19 +151,7 @@ impl ImageLoader for ImageCrateLoader { }); } } - - if bytes.starts_with(b"version https://git-lfs") { - return Err(LoadError::FormatNotSupported { - detected_format: Some("git-lfs".to_owned()), - }); - } - - // (3) - log::trace!("started loading {uri:?}"); - let result = crate::image::load_image_bytes(&bytes).map(Arc::new); - log::trace!("finished loading {uri:?}"); - cache.insert(uri.into(), result.clone()); - result.map(|image| ImagePoll::Ready { image }) + load_image(ctx, uri, &self.cache, &bytes) } Ok(BytesPoll::Pending { size }) => Ok(ImagePoll::Pending { size }), Err(err) => Err(err), @@ -123,8 +172,9 @@ impl ImageLoader for ImageCrateLoader { .lock() .values() .map(|result| match result { - Ok(image) => image.pixels.len() * size_of::(), - Err(err) => err.byte_size(), + Poll::Ready(Ok(image)) => image.pixels.len() * size_of::(), + Poll::Ready(Err(err)) => err.len(), + Poll::Pending => 0, }) .sum() } diff --git a/deny.toml b/deny.toml index 19f35309..2f7bdbcc 100644 --- a/deny.toml +++ b/deny.toml @@ -57,7 +57,7 @@ skip = [ { name = "redox_syscall" }, # old version via winit { name = "thiserror" }, # ecosystem is in the process of migrating from 1.x to 2.x { name = "thiserror-impl" }, # same as above - { name = "time" }, # old version pulled in by unmaintianed crate 'chrono' + { name = "time" }, # old version pulled in by unmaintained crate 'chrono' { name = "windows-core" }, # Chrono pulls in 0.51, accesskit uses 0.58.0 { name = "windows-sys" }, # glutin pulls in 0.52.0, accesskit pulls in 0.59.0, rfd pulls 0.48, webbrowser pulls 0.45.0 (via jni) ]