From b52c7bb610f593fffc624d461dca17ac50c81626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A9ctor=20Ram=C3=B3n=20Jim=C3=A9nez?= Date: Wed, 1 May 2024 01:39:43 +0200 Subject: [PATCH] Use an opaque `Id` type for `image::Handle` Hashing pointers is a terrible idea. --- core/src/image.rs | 68 ++++++++++++++++++++++++++------------ graphics/src/image.rs | 5 +-- tiny_skia/src/raster.rs | 4 +-- wgpu/src/image/raster.rs | 4 +-- widget/src/image.rs | 8 ++--- widget/src/image/viewer.rs | 6 ++-- 6 files changed, 59 insertions(+), 36 deletions(-) diff --git a/core/src/image.rs b/core/src/image.rs index 5b31fbcf8a..a0e4078722 100644 --- a/core/src/image.rs +++ b/core/src/image.rs @@ -5,19 +5,21 @@ use crate::{Rectangle, Size}; use rustc_hash::FxHasher; use std::hash::{Hash, Hasher}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// A handle of some image data. #[derive(Clone, PartialEq, Eq)] pub enum Handle { /// File data - Path(PathBuf), + Path(Id, PathBuf), /// In-memory data - Bytes(Bytes), + Bytes(Id, Bytes), /// Decoded image pixels in RGBA format. Rgba { + /// The id of this handle. + id: Id, /// The width of the image. width: u32, /// 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. pub fn from_path>(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 @@ -46,6 +50,7 @@ impl Handle { pixels: impl Into, ) -> Handle { Self::Rgba { + id: Id::unique(), width, height, pixels: pixels.into(), @@ -59,24 +64,15 @@ impl Handle { /// This is useful if you already have your image loaded in-memory, maybe /// because you downloaded or generated it procedurally. pub fn from_memory(bytes: impl Into) -> Handle { - Self::Bytes(bytes.into()) + Self::Bytes(Id::unique(), bytes.into()) } /// Returns the unique identifier of the [`Handle`]. - pub fn id(&self) -> u64 { - let mut hasher = FxHasher::default(); - self.hash(&mut hasher); - - hasher.finish() - } -} - -impl Hash for Handle { - fn hash(&self, state: &mut H) { + pub fn id(&self) -> Id { match self { - Self::Path(path) => path.hash(state), - Self::Bytes(bytes) => bytes.as_ptr().hash(state), - Self::Rgba { pixels, .. } => pixels.as_ptr().hash(state), + Handle::Path(id, _) + | Handle::Bytes(id, _) + | Handle::Rgba { id, .. } => *id, } } } @@ -93,8 +89,8 @@ where impl std::fmt::Debug for Handle { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Path(path) => write!(f, "Path({path:?})"), - Self::Bytes(_) => write!(f, "Bytes(...)"), + Self::Path(_, path) => write!(f, "Path({path:?})"), + Self::Bytes(_, _) => write!(f, "Bytes(...)"), Self::Rgba { 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) -> Self { + let hash = { + let mut hasher = FxHasher::default(); + path.as_ref().hash(&mut hasher); + + hasher.finish() + }; + + Self::Hash(hash) + } +} + /// Image filtering strategy. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default)] 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`] /// /// [`Handle`]: Self::Handle - type Handle: Clone + Hash; + type Handle: Clone; /// Returns the dimensions of an image for the given [`Handle`]. fn measure_image(&self, handle: &Self::Handle) -> Size; diff --git a/graphics/src/image.rs b/graphics/src/image.rs index 2a63053049..04c4505755 100644 --- a/graphics/src/image.rs +++ b/graphics/src/image.rs @@ -102,7 +102,7 @@ pub fn load( } let (width, height, pixels) = match handle { - image::Handle::Path(path) => { + image::Handle::Path(_, path) => { let image = ::image::open(path)?; let operation = std::fs::File::open(path) @@ -119,7 +119,7 @@ pub fn load( image::Bytes::from(rgba.into_raw()), ) } - image::Handle::Bytes(bytes) => { + image::Handle::Bytes(_, bytes) => { let image = ::image::load_from_memory(bytes)?; let operation = Operation::from_exif(&mut std::io::Cursor::new(bytes)) @@ -138,6 +138,7 @@ pub fn load( width, height, pixels, + .. } => (*width, *height, pixels.clone()), }; diff --git a/tiny_skia/src/raster.rs b/tiny_skia/src/raster.rs index 59f1e4d5a9..907fce7c40 100644 --- a/tiny_skia/src/raster.rs +++ b/tiny_skia/src/raster.rs @@ -71,8 +71,8 @@ impl Pipeline { #[derive(Debug, Default)] struct Cache { - entries: FxHashMap>, - hits: FxHashSet, + entries: FxHashMap>, + hits: FxHashSet, } impl Cache { diff --git a/wgpu/src/image/raster.rs b/wgpu/src/image/raster.rs index 60e9cbad55..4d3c3125b8 100644 --- a/wgpu/src/image/raster.rs +++ b/wgpu/src/image/raster.rs @@ -38,8 +38,8 @@ impl Memory { /// Caches image raster data #[derive(Debug, Default)] pub struct Cache { - map: FxHashMap, - hits: FxHashSet, + map: FxHashMap, + hits: FxHashSet, should_trim: bool, } diff --git a/widget/src/image.rs b/widget/src/image.rs index f673c7b3bb..21d371b700 100644 --- a/widget/src/image.rs +++ b/widget/src/image.rs @@ -11,8 +11,6 @@ use crate::core::{ ContentFit, Element, Layout, Length, Rectangle, Size, Vector, Widget, }; -use std::hash::Hash; - pub use image::{FilterMethod, Handle}; /// Creates a new [`Viewer`] with the given image `Handle`. @@ -128,7 +126,7 @@ pub fn draw( filter_method: FilterMethod, ) where Renderer: image::Renderer, - Handle: Clone + Hash, + Handle: Clone, { let Size { width, height } = renderer.measure_image(handle); let image_size = Size::new(width as f32, height as f32); @@ -167,7 +165,7 @@ impl Widget for Image where Renderer: image::Renderer, - Handle: Clone + Hash, + Handle: Clone, { fn size(&self) -> Size { Size { @@ -216,7 +214,7 @@ impl<'a, Message, Theme, Renderer, Handle> From> for Element<'a, Message, Theme, Renderer> where Renderer: image::Renderer, - Handle: Clone + Hash + 'a, + Handle: Clone + 'a, { fn from(image: Image) -> Element<'a, Message, Theme, Renderer> { Element::new(image) diff --git a/widget/src/image/viewer.rs b/widget/src/image/viewer.rs index 75d73b1927..214cb996b9 100644 --- a/widget/src/image/viewer.rs +++ b/widget/src/image/viewer.rs @@ -10,8 +10,6 @@ use crate::core::{ Vector, Widget, }; -use std::hash::Hash; - /// A frame that displays an image with the ability to zoom in/out and pan. #[allow(missing_debug_implementations)] pub struct Viewer { @@ -94,7 +92,7 @@ impl Widget for Viewer where Renderer: image::Renderer, - Handle: Clone + Hash, + Handle: Clone, { fn tag(&self) -> tree::Tag { tree::Tag::of::() @@ -401,7 +399,7 @@ impl<'a, Message, Theme, Renderer, Handle> From> where Renderer: 'a + image::Renderer, Message: 'a, - Handle: Clone + Hash + 'a, + Handle: Clone + 'a, { fn from(viewer: Viewer) -> Element<'a, Message, Theme, Renderer> { Element::new(viewer)