From 03222aa70fb5c7d781ff87a54a13f9d298dbd1c1 Mon Sep 17 00:00:00 2001 From: Samuel Date: Fri, 11 Oct 2024 21:45:17 -0300 Subject: [PATCH] Fix svg images not caching (#508) --- crates/zng-ext-image/src/lib.rs | 42 +++++++++++++++++++++++++------ crates/zng-ext-image/src/types.rs | 12 ++++++--- crates/zng-ext-svg/src/lib.rs | 11 +++++--- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/crates/zng-ext-image/src/lib.rs b/crates/zng-ext-image/src/lib.rs index d163322a2..a43c1d34c 100644 --- a/crates/zng-ext-image/src/lib.rs +++ b/crates/zng-ext-image/src/lib.rs @@ -208,20 +208,21 @@ impl AppExtension for ImageManager { // update loading tasks: let mut images = IMAGES_SV.write(); - let images = &mut *images; - let decoding = &mut images.decoding; let mut loading = Vec::with_capacity(images.loading.len()); + let loading_tasks = mem::take(&mut images.loading); + let mut proxies = mem::take(&mut images.proxies); + drop(images); // proxies can use IMAGES - 'loading_tasks: for t in mem::take(&mut images.loading) { + 'loading_tasks: for t in loading_tasks { t.task.lock().update(); match t.task.into_inner().into_result() { Ok(d) => { match d.r { Ok(data) => { if let Some((key, mode)) = &t.is_data_proxy_source { - for proxy in &mut images.proxies { + for proxy in &mut proxies { if proxy.is_data_proxy() { - if let Some(replaced) = proxy.data(key, &data, &d.format, *mode, t.downscale, t.mask) { + if let Some(replaced) = proxy.data(key, &data, &d.format, *mode, t.downscale, t.mask, true) { replaced.set_bind(&t.image).perm(); t.image.hold(replaced).perm(); continue 'loading_tasks; @@ -251,7 +252,7 @@ impl AppExtension for ImageManager { // will recover in ViewProcessInitedEvent } } - decoding.push(ImageDecodingTask { + IMAGES_SV.write().decoding.push(ImageDecodingTask { format: d.format, data, image: t.image, @@ -278,7 +279,7 @@ impl AppExtension for ImageManager { // flag error for user retry if let Some(k) = &t.image.with(|img| img.cache_key) { - if let Some(e) = images.cache.get(k) { + if let Some(e) = IMAGES_SV.read().cache.get(k) { e.error.store(true, Ordering::Relaxed); } } @@ -297,7 +298,9 @@ impl AppExtension for ImageManager { } } } + let mut images = IMAGES_SV.write(); images.loading = loading; + images.proxies = proxies; } fn update(&mut self) { @@ -931,11 +934,14 @@ impl IMAGES { /// This method returns immediately with a loading [`ImageVar`], when `source` is ready it /// is used to get the actual [`ImageVar`] and binds it to the returned image. /// + /// Note that the `cache_mode` always applies to the inner image, and only to the return image if `cache_key` is set. + /// /// [`IMAGES.limits`]: IMAGES::limits pub fn image_task( &self, source: impl IntoFuture, cache_mode: impl Into, + cache_key: Option, limits: Option, downscale: Option, mask: Option, @@ -944,8 +950,28 @@ impl IMAGES { F: Future + Send + 'static, { let cache_mode = cache_mode.into(); + + if let Some(key) = cache_key { + match cache_mode { + ImageCacheMode::Cache => { + if let Some(v) = IMAGES_SV.read().cache.get(&key) { + return v.image.read_only(); + } + } + ImageCacheMode::Retry => { + if let Some(e) = IMAGES_SV.read().cache.get(&key) { + if !e.error.load(Ordering::Relaxed) { + return e.image.read_only(); + } + } + } + ImageCacheMode::Ignore | ImageCacheMode::Reload => {} + } + } + let source = source.into_future(); - let img = var(Img::new_none(None)); + let img = var(Img::new_none(cache_key)); + task::spawn(async_clmv!(img, { let source = source.await; let actual_img = IMAGES.image(source, cache_mode, limits, downscale, mask); diff --git a/crates/zng-ext-image/src/types.rs b/crates/zng-ext-image/src/types.rs index 1c2c71150..ca82c4778 100644 --- a/crates/zng-ext-image/src/types.rs +++ b/crates/zng-ext-image/src/types.rs @@ -43,8 +43,8 @@ pub trait ImageCacheProxy: Send + Sync { mask: Option, ) -> ProxyGetResult { let r = match source { - ImageSource::Static(_, data, image_format) => self.data(key, data, image_format, mode, downscale, mask), - ImageSource::Data(_, data, image_format) => self.data(key, data, image_format, mode, downscale, mask), + ImageSource::Static(_, data, image_format) => self.data(key, data, image_format, mode, downscale, mask, false), + ImageSource::Data(_, data, image_format) => self.data(key, data, image_format, mode, downscale, mask, false), _ => return ProxyGetResult::None, }; match r { @@ -57,11 +57,16 @@ pub trait ImageCacheProxy: Send + Sync { /// /// If [`is_data_proxy`] also intercept the [`Read`] or [`Download`] data. /// + /// If `is_loaded` is `true` the data was read or downloaded and the return var will be bound to an existing var that may already be cached. + /// If it is `false` the data was already loaded on the source and the return var will be returned directly, without caching. + /// + /// /// [`Data`]: ImageSource::Data /// [`Static`]: ImageSource::Static /// [`is_data_proxy`]: ImageCacheProxy::is_data_proxy /// [`Read`]: ImageSource::Read /// [`Download`]: ImageSource::Download + #[allow(clippy::too_many_arguments)] fn data( &mut self, key: &ImageHash, @@ -70,8 +75,9 @@ pub trait ImageCacheProxy: Send + Sync { mode: ImageCacheMode, downscale: Option, mask: Option, + is_loaded: bool, ) -> Option { - let _ = (key, data, image_format, mode, downscale, mask); + let _ = (key, data, image_format, mode, downscale, mask, is_loaded); None } diff --git a/crates/zng-ext-svg/src/lib.rs b/crates/zng-ext-svg/src/lib.rs index 1e817f449..5a29e507b 100644 --- a/crates/zng-ext-svg/src/lib.rs +++ b/crates/zng-ext-svg/src/lib.rs @@ -34,12 +34,13 @@ pub struct SvgRenderCache {} impl ImageCacheProxy for SvgRenderCache { fn data( &mut self, - _: &ImageHash, + key: &ImageHash, data: &[u8], format: &ImageDataFormat, mode: ImageCacheMode, downscale: Option, mask: Option, + is_loaded: bool, ) -> Option { let data = match format { ImageDataFormat::FileExtension(txt) if txt == "svg" || txt == "svgz" => SvgData::Raw(data.to_vec()), @@ -47,8 +48,12 @@ impl ImageCacheProxy for SvgRenderCache { ImageDataFormat::Unknown => SvgData::Str(svg_data_from_unknown(data)?), _ => return None, }; - - Some(IMAGES.image_task(async move { load(data, downscale) }, mode, None, None, mask)) + let key = if is_loaded { + None // already cached, return image is internal + } else { + Some(*key) + }; + Some(IMAGES.image_task(async move { load(data, downscale) }, mode, key, None, None, mask)) } fn is_data_proxy(&self) -> bool {