Fix duplicated images when building with embedded images

For embedded images the path is empty but we unconditionally used it to create a TextureCacheKey,
which clashes.

Instead, preserve and store the ImageCacheKey in the ImageInner variants.
This commit is contained in:
Simon Hausmann 2022-07-19 17:18:59 +02:00 committed by Simon Hausmann
parent 71d6717b68
commit 34dc0a80e7
5 changed files with 69 additions and 42 deletions

View file

@ -173,6 +173,7 @@ fn gen_corelib(
"SharedString",
"SharedVector",
"ImageInner",
"ImageCacheKey",
"Image",
"Color",
"PathData",
@ -259,6 +260,7 @@ fn gen_corelib(
vec![
"ImageInner",
"Image",
"ImageCacheKey",
"Size",
"slint_image_size",
"slint_image_path",
@ -354,6 +356,7 @@ fn gen_corelib(
.with_src(crate_dir.join("graphics/path.rs"))
.with_src(crate_dir.join("graphics/brush.rs"))
.with_src(crate_dir.join("graphics/image.rs"))
.with_src(crate_dir.join("graphics/image/cache.rs"))
.with_src(crate_dir.join("animations.rs"))
// .with_src(crate_dir.join("input.rs"))
.with_src(crate_dir.join("item_rendering.rs"))

View file

@ -277,7 +277,7 @@ pub enum ImageInner {
/// A resource that does not represent any data.
None,
EmbeddedImage {
path: SharedString, // Should be Option, but can't be because of cbindgen, so empty means none.
cache_key: cache::ImageCacheKey,
buffer: SharedImageBuffer,
},
#[cfg(feature = "svg")]
@ -291,9 +291,9 @@ impl PartialEq for ImageInner {
fn eq(&self, other: &Self) -> bool {
match (self, other) {
(
Self::EmbeddedImage { path: l_path, buffer: l_buffer },
Self::EmbeddedImage { path: r_path, buffer: r_buffer },
) => l_path == r_path && l_buffer == r_buffer,
Self::EmbeddedImage { cache_key: l_cache_key, buffer: l_buffer },
Self::EmbeddedImage { cache_key: r_cache_key, buffer: r_buffer },
) => l_cache_key == r_cache_key && l_buffer == r_buffer,
#[cfg(feature = "svg")]
(Self::Svg(l0), Self::Svg(r0)) => vtable::VRc::ptr_eq(l0, r0),
(Self::StaticTextures(l0), Self::StaticTextures(r0)) => l0 == r0,
@ -414,7 +414,7 @@ impl Image {
/// channels (red, green and blue) encoded as u8.
pub fn from_rgb8(buffer: SharedPixelBuffer<Rgb8Pixel>) -> Self {
Image(ImageInner::EmbeddedImage {
path: Default::default(),
cache_key: cache::ImageCacheKey::Invalid,
buffer: SharedImageBuffer::RGB8(buffer),
})
}
@ -423,7 +423,7 @@ impl Image {
/// channels (red, green, blue and alpha) encoded as u8.
pub fn from_rgba8(buffer: SharedPixelBuffer<Rgba8Pixel>) -> Self {
Image(ImageInner::EmbeddedImage {
path: Default::default(),
cache_key: cache::ImageCacheKey::Invalid,
buffer: SharedImageBuffer::RGBA8(buffer),
})
}
@ -435,7 +435,7 @@ impl Image {
/// Only construct an Image with this function if you know that your pixels are encoded this way.
pub fn from_rgba8_premultiplied(buffer: SharedPixelBuffer<Rgba8Pixel>) -> Self {
Image(ImageInner::EmbeddedImage {
path: Default::default(),
cache_key: cache::ImageCacheKey::Invalid,
buffer: SharedImageBuffer::RGBA8Premultiplied(buffer),
})
}
@ -467,9 +467,10 @@ impl Image {
/// ```
pub fn path(&self) -> Option<&std::path::Path> {
match &self.0 {
ImageInner::EmbeddedImage { path, .. } => {
(!path.is_empty()).then(|| std::path::Path::new(path.as_str()))
}
ImageInner::EmbeddedImage { cache_key, .. } => match cache_key {
cache::ImageCacheKey::Path(path) => Some(std::path::Path::new(path.as_str())),
_ => None,
},
_ => None,
}
}
@ -551,7 +552,10 @@ pub(crate) mod ffi {
#[no_mangle]
pub unsafe extern "C" fn slint_image_path(image: &Image) -> Option<&SharedString> {
match &image.0 {
ImageInner::EmbeddedImage { path, .. } => (!path.is_empty()).then(|| path),
ImageInner::EmbeddedImage { cache_key, .. } => match cache_key {
cache::ImageCacheKey::Path(path) => Some(path),
_ => None,
},
_ => None,
}
}

View file

@ -12,31 +12,45 @@ use crate::{slice::Slice, SharedString};
/// ImageCacheKey encapsulates the different ways of indexing images in the
/// cache of decoded images.
#[derive(PartialEq, Eq, Hash, Debug, derive_more::From)]
#[derive(PartialEq, Eq, Debug, Hash, Clone)]
#[repr(C)]
pub enum ImageCacheKey {
/// This variant indicates that no image cache key can be created for the image.
/// For example this is the case for programmatically created images.
Invalid,
/// The image is identified by its path on the file system.
Path(SharedString),
/// The image is identified by a URL.
#[cfg(target_arch = "wasm32")]
URL(String),
URL(SharedString),
/// The image is identified by the static address of its encoded data.
EmbeddedData(by_address::ByAddress<&'static [u8]>),
EmbeddedData(usize),
}
impl ImageCacheKey {
/// Returns a new cache key if decoded image data can be stored in image cache for
/// the given ImageInner.
pub fn new(resource: &ImageInner) -> Option<Self> {
Some(match resource {
let key = match resource {
ImageInner::None => return None,
ImageInner::EmbeddedImage { path, .. } => path.clone().into(),
ImageInner::EmbeddedImage { cache_key, .. } => cache_key.clone(),
ImageInner::StaticTextures(textures) => {
by_address::ByAddress(textures.data.as_slice()).into()
Self::from_embedded_image_data(textures.data.as_slice())
}
ImageInner::Svg(parsed_svg) => parsed_svg.path().clone().into(),
ImageInner::Svg(parsed_svg) => parsed_svg.cache_key(),
#[cfg(target_arch = "wasm32")]
ImageInner::HTMLImage(htmlimage) => htmlimage.source().into(),
})
ImageInner::HTMLImage(htmlimage) => Self::URL(htmlimage.source().into()),
};
if matches!(key, ImageCacheKey::Invalid) {
None
} else {
Some(key)
}
}
/// Returns a cache key for static embedded image data.
pub fn from_embedded_image_data(data: &'static [u8]) -> Self {
Self::EmbeddedData(data.as_ptr() as usize)
}
}
@ -52,14 +66,14 @@ impl ImageCache {
fn lookup_image_in_cache_or_create(
&mut self,
cache_key: ImageCacheKey,
image_create_fn: impl Fn() -> Option<ImageInner>,
image_create_fn: impl Fn(ImageCacheKey) -> Option<ImageInner>,
) -> Option<Image> {
Some(Image(match self.0.entry(cache_key) {
Some(Image(match self.0.entry(cache_key.clone()) {
std::collections::hash_map::Entry::Occupied(existing_entry) => {
existing_entry.get().clone()
}
std::collections::hash_map::Entry::Vacant(vacant_entry) => {
let new_image = image_create_fn()?;
let new_image = image_create_fn(cache_key)?;
vacant_entry.insert(new_image.clone());
new_image
}
@ -70,19 +84,19 @@ impl ImageCache {
if path.is_empty() {
return None;
}
let cache_key = ImageCacheKey::from(path.clone());
let cache_key = ImageCacheKey::Path(path.clone());
#[cfg(target_arch = "wasm32")]
return self.lookup_image_in_cache_or_create(cache_key, || {
return self.lookup_image_in_cache_or_create(cache_key, |_| {
return Some(ImageInner::HTMLImage(vtable::VRc::new(
super::htmlimage::HTMLImage::new(&path),
)));
});
#[cfg(not(target_arch = "wasm32"))]
return self.lookup_image_in_cache_or_create(cache_key, || {
return self.lookup_image_in_cache_or_create(cache_key, |cache_key| {
if cfg!(feature = "svg") {
if path.ends_with(".svg") || path.ends_with(".svgz") {
return Some(ImageInner::Svg(vtable::VRc::new(
super::svg::load_from_path(path).map_or_else(
super::svg::load_from_path(path, cache_key).map_or_else(
|err| {
eprintln!("Error loading SVG from {}: {}", &path, err);
None
@ -100,7 +114,7 @@ impl ImageCache {
},
|image| {
Some(ImageInner::EmbeddedImage {
path: path.clone(),
cache_key,
buffer: dynamic_image_to_shared_image_buffer(image),
})
},
@ -113,12 +127,12 @@ impl ImageCache {
data: Slice<'static, u8>,
format: Slice<'static, u8>,
) -> Option<Image> {
let cache_key = ImageCacheKey::from(by_address::ByAddress(data.as_slice()));
self.lookup_image_in_cache_or_create(cache_key, || {
let cache_key = ImageCacheKey::from_embedded_image_data(data.as_slice());
self.lookup_image_in_cache_or_create(cache_key, |cache_key| {
#[cfg(feature = "svg")]
if format.as_slice() == b"svg" || format.as_slice() == b"svgz" {
return Some(ImageInner::Svg(vtable::VRc::new(
super::svg::load_from_data(data.as_slice()).map_or_else(
super::svg::load_from_data(data.as_slice(), cache_key).map_or_else(
|svg_err| {
eprintln!("Error loading SVG: {}", svg_err);
None
@ -139,7 +153,7 @@ impl ImageCache {
match maybe_image {
Ok(image) => Some(ImageInner::EmbeddedImage {
path: Default::default(),
cache_key,
buffer: dynamic_image_to_shared_image_buffer(image),
}),
Err(decode_err) => {

View file

@ -3,13 +3,14 @@
#![cfg(feature = "svg")]
#[cfg(not(target_arch = "wasm32"))]
use crate::SharedString;
use super::SharedPixelBuffer;
pub struct ParsedSVG {
svg_tree: usvg::Tree,
path: SharedString,
cache_key: crate::graphics::cache::ImageCacheKey,
}
impl super::OpaqueRc for ParsedSVG {}
@ -26,8 +27,8 @@ impl ParsedSVG {
[size.width(), size.height()].into()
}
pub fn path(&self) -> &SharedString {
&self.path
pub fn cache_key(&self) -> crate::graphics::cache::ImageCacheKey {
self.cache_key.clone()
}
/// Renders the SVG with the specified size.
@ -71,19 +72,24 @@ fn with_svg_options<T>(callback: impl FnOnce(usvg::OptionsRef<'_>) -> T) -> T {
}
#[cfg(not(target_arch = "wasm32"))]
pub fn load_from_path(path: &SharedString) -> Result<ParsedSVG, std::io::Error> {
pub fn load_from_path(
path: &SharedString,
cache_key: crate::graphics::cache::ImageCacheKey,
) -> Result<ParsedSVG, std::io::Error> {
let svg_data = std::fs::read(std::path::Path::new(&path.as_str()))?;
with_svg_options(|options| {
usvg::Tree::from_data(&svg_data, &options)
.map(|svg| ParsedSVG { svg_tree: svg, path: path.clone() })
.map(|svg| ParsedSVG { svg_tree: svg, cache_key })
.map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, e))
})
}
pub fn load_from_data(slice: &[u8]) -> Result<ParsedSVG, usvg::Error> {
pub fn load_from_data(
slice: &[u8],
cache_key: crate::graphics::cache::ImageCacheKey,
) -> Result<ParsedSVG, usvg::Error> {
with_svg_options(|options| {
usvg::Tree::from_data(slice, &options)
.map(|svg| ParsedSVG { svg_tree: svg, path: Default::default() })
usvg::Tree::from_data(slice, &options).map(|svg| ParsedSVG { svg_tree: svg, cache_key })
})
}

View file

@ -13,11 +13,11 @@ use vtable::VRef;
#[repr(C)]
#[cfg(target_pointer_width = "64")]
pub struct ValueOpaque([usize; 7]);
pub struct ValueOpaque([usize; 8]);
#[repr(C)]
#[cfg(target_pointer_width = "32")]
#[repr(align(8))]
pub struct ValueOpaque([usize; 10]);
pub struct ValueOpaque([usize; 11]);
/// Asserts that ValueOpaque is as large as Value and has the same alignment, to make transmute safe.
const _: [(); std::mem::size_of::<ValueOpaque>()] = [(); std::mem::size_of::<Value>()];
const _: [(); std::mem::align_of::<ValueOpaque>()] = [(); std::mem::align_of::<Value>()];