Fix svg images not caching (#508)

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<F>(
         &self,
         source: impl IntoFuture<IntoFuture = F>,
         cache_mode: impl Into<ImageCacheMode>,
+        cache_key: Option<ImageHash>,
         limits: Option<ImageLimits>,
         downscale: Option<ImageDownscale>,
         mask: Option<ImageMaskMode>,
@@ -944,8 +950,28 @@ impl IMAGES {
         F: Future<Output = ImageSource> + 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);

crates/zng-ext-image/src/types.rs

@@ -43,8 +43,8 @@ pub trait ImageCacheProxy: Send + Sync {
         mask: Option<ImageMaskMode>,
     ) -> 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<ImageDownscale>,
         mask: Option<ImageMaskMode>,
+        is_loaded: bool,
     ) -> Option<ImageVar> {
-        let _ = (key, data, image_format, mode, downscale, mask);
+        let _ = (key, data, image_format, mode, downscale, mask, is_loaded);
         None
     }
 

crates/zng-ext-svg/src/lib.rs

@@ -34,21 +34,26 @@ pub struct SvgRenderCache {}
 impl ImageCacheProxy for SvgRenderCache {
     fn data(
         &mut self,
-        _: &ImageHash,
+        key: &ImageHash,
         data: &[u8],
         format: &ImageDataFormat,
         mode: ImageCacheMode,
         downscale: Option<ImageDownscale>,
         mask: Option<ImageMaskMode>,
+        is_loaded: bool,
     ) -> Option<ImageVar> {
         let data = match format {
             ImageDataFormat::FileExtension(txt) if txt == "svg" || txt == "svgz" => SvgData::Raw(data.to_vec()),
             ImageDataFormat::MimeType(txt) if txt == "image/svg+xml" => SvgData::Raw(data.to_vec()),
             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 {