Use an opaque Id type for image::Handle

Hashing pointers is a terrible idea.
This commit is contained in:
Héctor Ramón Jiménez 2024-05-01 01:39:43 +02:00
parent f5bc336d69
commit b52c7bb610
No known key found for this signature in database
GPG key ID: 7CC46565708259A7
6 changed files with 59 additions and 36 deletions

View file

@ -5,19 +5,21 @@ use crate::{Rectangle, Size};
use rustc_hash::FxHasher; use rustc_hash::FxHasher;
use std::hash::{Hash, Hasher}; use std::hash::{Hash, Hasher};
use std::path::PathBuf; use std::path::{Path, PathBuf};
/// A handle of some image data. /// A handle of some image data.
#[derive(Clone, PartialEq, Eq)] #[derive(Clone, PartialEq, Eq)]
pub enum Handle { pub enum Handle {
/// File data /// File data
Path(PathBuf), Path(Id, PathBuf),
/// In-memory data /// In-memory data
Bytes(Bytes), Bytes(Id, Bytes),
/// Decoded image pixels in RGBA format. /// Decoded image pixels in RGBA format.
Rgba { Rgba {
/// The id of this handle.
id: Id,
/// The width of the image. /// The width of the image.
width: u32, width: u32,
/// The height of the image. /// The height of the image.
@ -32,7 +34,9 @@ impl Handle {
/// ///
/// Makes an educated guess about the image format by examining the data in the file. /// Makes an educated guess about the image format by examining the data in the file.
pub fn from_path<T: Into<PathBuf>>(path: T) -> Handle { pub fn from_path<T: Into<PathBuf>>(path: T) -> Handle {
Self::Path(path.into()) let path = path.into();
Self::Path(Id::path(&path), path)
} }
/// Creates an image [`Handle`] containing the image pixels directly. This /// Creates an image [`Handle`] containing the image pixels directly. This
@ -46,6 +50,7 @@ impl Handle {
pixels: impl Into<Bytes>, pixels: impl Into<Bytes>,
) -> Handle { ) -> Handle {
Self::Rgba { Self::Rgba {
id: Id::unique(),
width, width,
height, height,
pixels: pixels.into(), pixels: pixels.into(),
@ -59,24 +64,15 @@ impl Handle {
/// This is useful if you already have your image loaded in-memory, maybe /// This is useful if you already have your image loaded in-memory, maybe
/// because you downloaded or generated it procedurally. /// because you downloaded or generated it procedurally.
pub fn from_memory(bytes: impl Into<Bytes>) -> Handle { pub fn from_memory(bytes: impl Into<Bytes>) -> Handle {
Self::Bytes(bytes.into()) Self::Bytes(Id::unique(), bytes.into())
} }
/// Returns the unique identifier of the [`Handle`]. /// Returns the unique identifier of the [`Handle`].
pub fn id(&self) -> u64 { pub fn id(&self) -> Id {
let mut hasher = FxHasher::default();
self.hash(&mut hasher);
hasher.finish()
}
}
impl Hash for Handle {
fn hash<H: Hasher>(&self, state: &mut H) {
match self { match self {
Self::Path(path) => path.hash(state), Handle::Path(id, _)
Self::Bytes(bytes) => bytes.as_ptr().hash(state), | Handle::Bytes(id, _)
Self::Rgba { pixels, .. } => pixels.as_ptr().hash(state), | Handle::Rgba { id, .. } => *id,
} }
} }
} }
@ -93,8 +89,8 @@ where
impl std::fmt::Debug for Handle { impl std::fmt::Debug for Handle {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self { match self {
Self::Path(path) => write!(f, "Path({path:?})"), Self::Path(_, path) => write!(f, "Path({path:?})"),
Self::Bytes(_) => write!(f, "Bytes(...)"), Self::Bytes(_, _) => write!(f, "Bytes(...)"),
Self::Rgba { width, height, .. } => { Self::Rgba { width, height, .. } => {
write!(f, "Pixels({width} * {height})") write!(f, "Pixels({width} * {height})")
} }
@ -102,6 +98,36 @@ impl std::fmt::Debug for Handle {
} }
} }
/// The unique identifier of some [`Handle`] data.
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum Id {
/// A unique identifier.
Unique(u64),
/// A hash identifier.
Hash(u64),
}
impl Id {
fn unique() -> Self {
use std::sync::atomic::{self, AtomicU64};
static NEXT_ID: AtomicU64 = AtomicU64::new(0);
Self::Unique(NEXT_ID.fetch_add(1, atomic::Ordering::Relaxed))
}
fn path(path: impl AsRef<Path>) -> Self {
let hash = {
let mut hasher = FxHasher::default();
path.as_ref().hash(&mut hasher);
hasher.finish()
};
Self::Hash(hash)
}
}
/// Image filtering strategy. /// Image filtering strategy.
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)]
pub enum FilterMethod { pub enum FilterMethod {
@ -119,7 +145,7 @@ pub trait Renderer: crate::Renderer {
/// The image Handle to be displayed. Iced exposes its own default implementation of a [`Handle`] /// The image Handle to be displayed. Iced exposes its own default implementation of a [`Handle`]
/// ///
/// [`Handle`]: Self::Handle /// [`Handle`]: Self::Handle
type Handle: Clone + Hash; type Handle: Clone;
/// Returns the dimensions of an image for the given [`Handle`]. /// Returns the dimensions of an image for the given [`Handle`].
fn measure_image(&self, handle: &Self::Handle) -> Size<u32>; fn measure_image(&self, handle: &Self::Handle) -> Size<u32>;

View file

@ -102,7 +102,7 @@ pub fn load(
} }
let (width, height, pixels) = match handle { let (width, height, pixels) = match handle {
image::Handle::Path(path) => { image::Handle::Path(_, path) => {
let image = ::image::open(path)?; let image = ::image::open(path)?;
let operation = std::fs::File::open(path) let operation = std::fs::File::open(path)
@ -119,7 +119,7 @@ pub fn load(
image::Bytes::from(rgba.into_raw()), image::Bytes::from(rgba.into_raw()),
) )
} }
image::Handle::Bytes(bytes) => { image::Handle::Bytes(_, bytes) => {
let image = ::image::load_from_memory(bytes)?; let image = ::image::load_from_memory(bytes)?;
let operation = let operation =
Operation::from_exif(&mut std::io::Cursor::new(bytes)) Operation::from_exif(&mut std::io::Cursor::new(bytes))
@ -138,6 +138,7 @@ pub fn load(
width, width,
height, height,
pixels, pixels,
..
} => (*width, *height, pixels.clone()), } => (*width, *height, pixels.clone()),
}; };

View file

@ -71,8 +71,8 @@ impl Pipeline {
#[derive(Debug, Default)] #[derive(Debug, Default)]
struct Cache { struct Cache {
entries: FxHashMap<u64, Option<Entry>>, entries: FxHashMap<raster::Id, Option<Entry>>,
hits: FxHashSet<u64>, hits: FxHashSet<raster::Id>,
} }
impl Cache { impl Cache {

View file

@ -38,8 +38,8 @@ impl Memory {
/// Caches image raster data /// Caches image raster data
#[derive(Debug, Default)] #[derive(Debug, Default)]
pub struct Cache { pub struct Cache {
map: FxHashMap<u64, Memory>, map: FxHashMap<image::Id, Memory>,
hits: FxHashSet<u64>, hits: FxHashSet<image::Id>,
should_trim: bool, should_trim: bool,
} }

View file

@ -11,8 +11,6 @@ use crate::core::{
ContentFit, Element, Layout, Length, Rectangle, Size, Vector, Widget, ContentFit, Element, Layout, Length, Rectangle, Size, Vector, Widget,
}; };
use std::hash::Hash;
pub use image::{FilterMethod, Handle}; pub use image::{FilterMethod, Handle};
/// Creates a new [`Viewer`] with the given image `Handle`. /// Creates a new [`Viewer`] with the given image `Handle`.
@ -128,7 +126,7 @@ pub fn draw<Renderer, Handle>(
filter_method: FilterMethod, filter_method: FilterMethod,
) where ) where
Renderer: image::Renderer<Handle = Handle>, Renderer: image::Renderer<Handle = Handle>,
Handle: Clone + Hash, Handle: Clone,
{ {
let Size { width, height } = renderer.measure_image(handle); let Size { width, height } = renderer.measure_image(handle);
let image_size = Size::new(width as f32, height as f32); let image_size = Size::new(width as f32, height as f32);
@ -167,7 +165,7 @@ impl<Message, Theme, Renderer, Handle> Widget<Message, Theme, Renderer>
for Image<Handle> for Image<Handle>
where where
Renderer: image::Renderer<Handle = Handle>, Renderer: image::Renderer<Handle = Handle>,
Handle: Clone + Hash, Handle: Clone,
{ {
fn size(&self) -> Size<Length> { fn size(&self) -> Size<Length> {
Size { Size {
@ -216,7 +214,7 @@ impl<'a, Message, Theme, Renderer, Handle> From<Image<Handle>>
for Element<'a, Message, Theme, Renderer> for Element<'a, Message, Theme, Renderer>
where where
Renderer: image::Renderer<Handle = Handle>, Renderer: image::Renderer<Handle = Handle>,
Handle: Clone + Hash + 'a, Handle: Clone + 'a,
{ {
fn from(image: Image<Handle>) -> Element<'a, Message, Theme, Renderer> { fn from(image: Image<Handle>) -> Element<'a, Message, Theme, Renderer> {
Element::new(image) Element::new(image)

View file

@ -10,8 +10,6 @@ use crate::core::{
Vector, Widget, Vector, Widget,
}; };
use std::hash::Hash;
/// A frame that displays an image with the ability to zoom in/out and pan. /// A frame that displays an image with the ability to zoom in/out and pan.
#[allow(missing_debug_implementations)] #[allow(missing_debug_implementations)]
pub struct Viewer<Handle> { pub struct Viewer<Handle> {
@ -94,7 +92,7 @@ impl<Message, Theme, Renderer, Handle> Widget<Message, Theme, Renderer>
for Viewer<Handle> for Viewer<Handle>
where where
Renderer: image::Renderer<Handle = Handle>, Renderer: image::Renderer<Handle = Handle>,
Handle: Clone + Hash, Handle: Clone,
{ {
fn tag(&self) -> tree::Tag { fn tag(&self) -> tree::Tag {
tree::Tag::of::<State>() tree::Tag::of::<State>()
@ -401,7 +399,7 @@ impl<'a, Message, Theme, Renderer, Handle> From<Viewer<Handle>>
where where
Renderer: 'a + image::Renderer<Handle = Handle>, Renderer: 'a + image::Renderer<Handle = Handle>,
Message: 'a, Message: 'a,
Handle: Clone + Hash + 'a, Handle: Clone + 'a,
{ {
fn from(viewer: Viewer<Handle>) -> Element<'a, Message, Theme, Renderer> { fn from(viewer: Viewer<Handle>) -> Element<'a, Message, Theme, Renderer> {
Element::new(viewer) Element::new(viewer)