Fix duplicated loading/embedding of images (#2612)

For .slint files that are included, we canonicalize the path before
adding it into the import stack, to avoid duplicates. We didn't do that
for images in the global_embedded_resources.

Ensure this by canonicalizing as early as possible.

Fixes #2608

internal/compiler/fileaccess.rs

@@ -4,16 +4,16 @@
 use std::borrow::Cow;
 
 #[derive(Clone)]
-pub struct VirtualFile<'a> {
-    pub path: Cow<'a, str>,
+pub struct VirtualFile {
+    pub canon_path: std::path::PathBuf,
     pub builtin_contents: Option<&'static [u8]>,
 }
 
-impl<'a> VirtualFile<'a> {
+impl<'a> VirtualFile {
     pub fn read(&self) -> Cow<'static, [u8]> {
         match self.builtin_contents {
             Some(static_data) => Cow::Borrowed(static_data),
-            None => Cow::Owned(std::fs::read(self.path.as_ref()).unwrap()),
+            None => Cow::Owned(std::fs::read(&self.canon_path).unwrap()),
         }
     }
 
@@ -26,11 +26,11 @@ pub fn styles() -> Vec<&'static str> {
     builtin_library::styles()
 }
 
-pub fn load_file<'a>(path: &'a std::path::Path) -> Option<VirtualFile<'static>> {
+pub fn load_file<'a>(path: &'a std::path::Path) -> Option<VirtualFile> {
     match path.strip_prefix("builtin:/") {
         Ok(builtin_path) => builtin_library::load_builtin_file(builtin_path),
         Err(_) => path.exists().then(|| VirtualFile {
-            path: Cow::Owned(path.to_string_lossy().to_string()),
+            canon_path: dunce::canonicalize(path).unwrap_or_else(|_| path.into()),
             builtin_contents: None,
         }),
     }
@@ -61,9 +61,7 @@ mod builtin_library {
             .collect()
     }
 
-    pub(crate) fn load_builtin_file(
-        builtin_path: &std::path::Path,
-    ) -> Option<VirtualFile<'static>> {
+    pub(crate) fn load_builtin_file(builtin_path: &std::path::Path) -> Option<VirtualFile> {
         let mut components = vec![];
         for part in builtin_path.iter() {
             if part == ".." {
@@ -77,7 +75,9 @@ mod builtin_library {
             library.iter().find_map(|builtin_file| {
                 if builtin_file.path == file {
                     Some(VirtualFile {
-                        path: builtin_file.path.into(),
+                        canon_path: ["builtin:/", folder.to_str().unwrap(), builtin_file.path]
+                            .iter()
+                            .collect::<std::path::PathBuf>(),
                         builtin_contents: Some(builtin_file.contents),
                     })
                 } else {

internal/compiler/passes/embed_images.rs

@@ -272,13 +272,16 @@ fn load_image(
     scale_factor: f64,
 ) -> image::ImageResult<(image::RgbaImage, Size)> {
     use resvg::{tiny_skia, usvg};
+    use std::ffi::OsStr;
     use usvg::TreeParsing;
-    if file.path.ends_with(".svg") || file.path.ends_with(".svgz") {
+    if file.canon_path.extension() == Some(OsStr::new("svg"))
+        || file.canon_path.extension() == Some(OsStr::new("svgz"))
+    {
         let options = usvg::Options::default();
         let tree = match file.builtin_contents {
             Some(data) => usvg::Tree::from_data(data, &options),
             None => usvg::Tree::from_data(
-                std::fs::read(file.path.as_ref()).map_err(image::ImageError::IoError)?.as_slice(),
+                std::fs::read(file.canon_path).map_err(image::ImageError::IoError)?.as_slice(),
                 &options,
             ),
         }
@@ -321,7 +324,7 @@ fn load_image(
     if let Some(buffer) = file.builtin_contents {
         image::load_from_memory(buffer)
     } else {
-        image::open(file.path.as_ref())
+        image::open(file.canon_path)
     }
     .map(|mut image| {
         let (original_width, original_height) = image.dimensions();

internal/compiler/typeloader.rs

@@ -266,7 +266,7 @@ impl TypeLoader {
         &self,
         import_token: Option<&NodeOrToken>,
         maybe_relative_path_or_url: &str,
-    ) -> (std::path::PathBuf, Option<&'static [u8]>) {
+    ) -> (PathBuf, Option<&'static [u8]>) {
         let referencing_file_or_url =
             import_token.and_then(|tok| tok.source_file().map(|s| s.path()));
 
@@ -302,16 +302,14 @@ impl TypeLoader {
         import_token: Option<NodeOrToken>,
         mut import_stack: HashSet<PathBuf>,
     ) -> Option<PathBuf> {
-        let (path, builtin) =
+        let (path_canon, builtin) =
             { state.borrow().tl.resolve_import_path(import_token.as_ref(), file_to_import) };
 
-        let path_canon = dunce::canonicalize(&path).unwrap_or_else(|_| path.to_owned());
-
         if !import_stack.insert(path_canon.clone()) {
-            state
-                .borrow_mut()
-                .diag
-                .push_error(format!("Recursive import of \"{}\"", path.display()), &import_token);
+            state.borrow_mut().diag.push_error(
+                format!("Recursive import of \"{}\"", path_canon.display()),
+                &import_token,
+            );
             return None;
         }
 
@@ -370,7 +368,7 @@ impl TypeLoader {
             }
             Err(err) => {
                 state.borrow_mut().diag.push_error(
-                    format!("Error reading requested import \"{}\": {}", path.display(), err),
+                    format!("Error reading requested import \"{}\": {}", path_canon.display(), err),
                     &import_token,
                 );
                 return None;
@@ -380,7 +378,7 @@ impl TypeLoader {
         Self::load_file_impl(
             state,
             &path_canon,
-            &path,
+            &path_canon,
             source_code,
             builtin.is_some(),
             &import_stack,
@@ -518,7 +516,7 @@ impl TypeLoader {
     /// the current file directory
     pub fn find_file_in_include_path(
         &self,
-        referencing_file: Option<&std::path::Path>,
+        referencing_file: Option<&Path>,
         file_to_import: &str,
     ) -> Option<(PathBuf, Option<&'static [u8]>)> {
         // The directory of the current file is the first in the list of include directories.
@@ -539,8 +537,9 @@ impl TypeLoader {
             .chain(std::iter::once_with(|| format!("builtin:/{}", self.style).into()))
             .find_map(|include_dir| {
                 let candidate = include_dir.join(file_to_import);
-                crate::fileaccess::load_file(&candidate)
-                    .map(|virtual_file| (candidate, virtual_file.builtin_contents))
+                crate::fileaccess::load_file(&candidate).map(|virtual_file| {
+                    (virtual_file.canon_path.into(), virtual_file.builtin_contents)
+                })
             })
     }
 
@@ -608,7 +607,7 @@ impl TypeLoader {
 
 #[test]
 fn test_dependency_loading() {
-    let test_source_path: std::path::PathBuf =
+    let test_source_path: PathBuf =
         [env!("CARGO_MANIFEST_DIR"), "tests", "typeloader"].iter().collect();
 
     let mut incdir = test_source_path.clone();
@@ -711,7 +710,7 @@ fn test_manual_import() {
 
 #[test]
 fn test_builtin_style() {
-    let test_source_path: std::path::PathBuf =
+    let test_source_path: PathBuf =
         [env!("CARGO_MANIFEST_DIR"), "tests", "typeloader"].iter().collect();
 
     let incdir = test_source_path.join("custom_style");
@@ -730,7 +729,7 @@ fn test_builtin_style() {
 
 #[test]
 fn test_user_style() {
-    let test_source_path: std::path::PathBuf =
+    let test_source_path: PathBuf =
         [env!("CARGO_MANIFEST_DIR"), "tests", "typeloader"].iter().collect();
 
     let incdir = test_source_path.join("custom_style");
@@ -749,7 +748,7 @@ fn test_user_style() {
 
 #[test]
 fn test_unknown_style() {
-    let test_source_path: std::path::PathBuf =
+    let test_source_path: PathBuf =
         [env!("CARGO_MANIFEST_DIR"), "tests", "typeloader"].iter().collect();
 
     let incdir = test_source_path.join("custom_style");

internal/core/graphics/image.rs

@@ -645,10 +645,7 @@ impl Image {
 /// Load an image from an image embedded in the binary.
 /// This is called by the generated code.
 #[cfg(feature = "image-decoders")]
-pub fn load_image_from_embedded_data(
-    data: Slice<'static, u8>,
-    format: Slice<'static, u8>,
-) -> Image {
+pub fn load_image_from_embedded_data(data: Slice<'static, u8>, format: Slice<'_, u8>) -> Image {
     self::cache::IMAGE_CACHE.with(|global_cache| {
         global_cache.borrow_mut().load_image_from_embedded_data(data, format).unwrap_or_else(|| {
             panic!("internal error: embedded image data is not supported by run-time library",)

internal/core/graphics/image/cache.rs

@@ -112,7 +112,7 @@ impl ImageCache {
     pub(crate) fn load_image_from_embedded_data(
         &mut self,
         data: Slice<'static, u8>,
-        format: Slice<'static, u8>,
+        format: Slice<'_, u8>,
     ) -> Option<Image> {
         let cache_key = ImageCacheKey::from_embedded_image_data(data.as_slice());
         self.lookup_image_in_cache_or_create(cache_key, |cache_key| {

internal/interpreter/eval.rs

@@ -289,8 +289,8 @@ pub fn eval_expression(expression: &Expression, local_context: &mut EvalLocalCon
 
                     let virtual_file = i_slint_compiler::fileaccess::load_file(std::path::Path::new(path)).unwrap();  // embedding pass ensured that the file exists
 
-                    if let (std::borrow::Cow::Borrowed(static_path), Some(static_data)) = (virtual_file.path, virtual_file.builtin_contents) {
-                        let virtual_file_extension = std::path::Path::new(static_path).extension().unwrap().to_str().unwrap();
+                    if let (static_path, Some(static_data)) = (virtual_file.canon_path, virtual_file.builtin_contents) {
+                        let virtual_file_extension = static_path.extension().unwrap().to_str().unwrap();
                         debug_assert_eq!(virtual_file_extension, extension);
                         Ok(corelib::graphics::load_image_from_embedded_data(
                             corelib::slice::Slice::from_slice(static_data),

tests/cases/issues/issue_2608_canon_img_path.slint

@@ -0,0 +1,32 @@
+// Copyright © SixtyFPS GmbH <info@slint-ui.com>
+// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-commercial
+
+//include_path: ../../../examples/printerdemo/ui/images/
+
+TestCase := Rectangle {
+    property <image> img1: @image-url("cat.jpg");
+    property <image> img2: @image-url("../images/cat.jpg");
+}
+
+/*
+```rust
+let instance = TestCase::new().unwrap();
+
+use slint::private_unstable_api::re_exports::{ImageInner, ImageCacheKey};
+
+let img1 = instance.get_img1();
+let img2 = instance.get_img2();
+let img1_inner: &ImageInner = (&img1).into();
+let img2_inner: &ImageInner = (&img2).into();
+
+match (img1_inner, img2_inner) {
+    (ImageInner::EmbeddedImage { cache_key: ImageCacheKey::Path(img1_path), .. }, ImageInner::EmbeddedImage { cache_key: ImageCacheKey::Path(img2_path), .. }) => {
+        assert_eq!(img1_path, img2_path)
+    },
+    (ImageInner::EmbeddedImage { cache_key: ImageCacheKey::EmbeddedData(img1_addr), .. }, ImageInner::EmbeddedImage { cache_key: ImageCacheKey::EmbeddedData(img2_addr), .. }) => {
+        assert_eq!(img1_addr, img2_addr)
+    },
+    _ => todo!("adjust this test to new image comparison"),
+}
+```
+*/