Migrate usage of the Hash trait for cache invalidation to the dedicated CacheHash trait (#4051)

* WIP start migrating usages of hash for cache invalidadion to dedicated trait

* Finish migrating usages

* Code review

* Add comments clearifying the reasoning for using random ids in the VectorModification cach hash impl

* Fix some remaining hash violations

* Finish migration and fix compilation

* Fix import ordering

* Cleanup

* Fix code review stuff

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>

node-graph/nodes/brush/Cargo.toml

@@ -14,6 +14,7 @@ serde = ["dep:serde"]
 # Local dependencies
 dyn-any = { workspace = true }
 core-types = { workspace = true }
+graphene-hash = { workspace = true }
 raster-types = { workspace = true }
 raster-nodes = { workspace = true }
 node-macro = { workspace = true }

node-graph/nodes/brush/src/brush_cache.rs

@@ -1,5 +1,6 @@
 use crate::brush_stroke::BrushStroke;
 use crate::brush_stroke::BrushStyle;
+use core_types::graphene_hash::CacheHashWrapper;
 use core_types::table::TableRow;
 use dyn_any::DynAny;
 use raster_types::CPU;
@@ -31,7 +32,7 @@ struct BrushCacheImpl {
 
 	// A cache for brush textures.
 	#[serde(skip)]
-	brush_texture_cache: HashMap<BrushStyle, Raster<CPU>>,
+	brush_texture_cache: HashMap<CacheHashWrapper<BrushStyle>, Raster<CPU>>,
 }
 
 impl BrushCacheImpl {
@@ -165,6 +166,12 @@ impl Hash for BrushCache {
 	}
 }
 
+impl graphene_hash::CacheHash for BrushCache {
+	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
+		core::hash::Hash::hash(&self.0.lock().unwrap().unique_id, state);
+	}
+}
+
 impl BrushCache {
 	pub fn compute_brush_plan(&self, background: TableRow<Raster<CPU>>, input: &[BrushStroke]) -> BrushPlan {
 		let mut inner = self.0.lock().unwrap();
@@ -178,11 +185,11 @@ impl BrushCache {
 
 	pub fn get_cached_brush(&self, style: &BrushStyle) -> Option<Raster<CPU>> {
 		let inner = self.0.lock().unwrap();
-		inner.brush_texture_cache.get(style).cloned()
+		inner.brush_texture_cache.get(&CacheHashWrapper(style.clone())).cloned()
 	}
 
 	pub fn store_brush(&self, style: BrushStyle, brush: Raster<CPU>) {
 		let mut inner = self.0.lock().unwrap();
-		inner.brush_texture_cache.insert(style, brush);
+		inner.brush_texture_cache.insert(CacheHashWrapper(style), brush);
 	}
 }

node-graph/nodes/brush/src/brush_stroke.rs

@@ -1,12 +1,11 @@
+use core_types::CacheHash;
 use core_types::blending::BlendMode;
 use core_types::color::Color;
 use core_types::math::bbox::AxisAlignedBbox;
 use dyn_any::DynAny;
 use glam::DVec2;
-use std::hash::{Hash, Hasher};
-
 /// The style of a brush.
-#[derive(Clone, Debug, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Clone, Debug, CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 pub struct BrushStyle {
 	pub color: Color,
 	pub diameter: f64,
@@ -29,17 +28,6 @@ impl Default for BrushStyle {
 	}
 }
 
-impl Hash for BrushStyle {
-	fn hash<H: Hasher>(&self, state: &mut H) {
-		self.color.hash(state);
-		self.diameter.to_bits().hash(state);
-		self.hardness.to_bits().hash(state);
-		self.flow.to_bits().hash(state);
-		self.spacing.to_bits().hash(state);
-		self.blend_mode.hash(state);
-	}
-}
-
 impl Eq for BrushStyle {}
 
 impl PartialEq for BrushStyle {
@@ -54,23 +42,13 @@ impl PartialEq for BrushStyle {
 }
 
 /// A single sample of brush parameters across the brush stroke.
-#[derive(Clone, Debug, PartialEq, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Clone, Debug, PartialEq, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 pub struct BrushInputSample {
-	// The position of the sample in layer space, in pixels.
-	// The origin of layer space is not specified.
 	pub position: DVec2,
-	// Future work: pressure, stylus angle, etc.
-}
-
-impl Hash for BrushInputSample {
-	fn hash<H: Hasher>(&self, state: &mut H) {
-		self.position.x.to_bits().hash(state);
-		self.position.y.to_bits().hash(state);
-	}
 }
 
 /// The parameters for a single stroke brush.
-#[derive(Clone, Debug, PartialEq, Hash, Default, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Clone, Debug, PartialEq, core_types::CacheHash, Default, DynAny, serde::Serialize, serde::Deserialize)]
 pub struct BrushStroke {
 	pub style: BrushStyle,
 	pub trace: Vec<BrushInputSample>,

node-graph/nodes/gcore/Cargo.toml

@@ -19,6 +19,7 @@ wasm = [
 [dependencies]
 # Local dependencies
 core-types = { workspace = true }
+graphene-hash = { workspace = true }
 raster-types = { workspace = true }
 graphic-types = { workspace = true }
 node-macro = { workspace = true }

node-graph/nodes/gcore/src/animation.rs

@@ -1,7 +1,7 @@
 use core_types::table::Table;
 use core_types::transform::Footprint;
 use core_types::uuid::NodeId;
-use core_types::{CloneVarArgs, Color, Context, Ctx, ExtractAll, ExtractAnimationTime, ExtractPointerPosition, ExtractRealTime, OwnedContextImpl};
+use core_types::{CacheHash, CloneVarArgs, Color, Context, Ctx, ExtractAll, ExtractAnimationTime, ExtractPointerPosition, ExtractRealTime, OwnedContextImpl};
 use glam::{DAffine2, DVec2};
 use graphic_types::vector_types::GradientStops;
 use graphic_types::{Artboard, Graphic, Vector};
@@ -9,7 +9,7 @@ use raster_types::{CPU, GPU, Raster};
 
 const DAY: f64 = 1000. * 3600. * 24.;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, dyn_any::DynAny, Default, Hash, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, dyn_any::DynAny, Default, Hash, CacheHash, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
 pub enum RealTimeMode {
 	#[label("UTC")]
 	Utc,

node-graph/nodes/gcore/src/context_modification.rs

@@ -51,17 +51,17 @@ async fn context_modification<T>(
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use core_types::graphene_hash::CacheHash;
 	use core_types::transform::Footprint;
 	use std::collections::hash_map::DefaultHasher;
-	use std::hash::{Hash, Hasher};
+	use std::hash::Hasher;
 
-	/// Test that the hash of a nullified context remains stable even when nullified inputs change
+	/// Verifies that nullified context fields don't affect the cache hash — only the kept features matter.
 	#[test]
 	fn test_nullified_context_hash_stability() {
 		use core_types::Context;
 		use std::sync::Arc;
 
-		// Create original contexts using the Context type (Option<Arc<OwnedContextImpl>>)
 		let original_ctx: Context = Some(Arc::new(
 			OwnedContextImpl::empty()
 				.with_footprint(Footprint::default())
@@ -71,53 +71,48 @@ mod tests {
 				.with_animation_time(20.25),
 		));
 
-		// Test nullifying different features - hash should remain stable for each nullification
-		let features_to_keep = ContextFeatures::empty(); // Nullify everything
-
-		// Create nullified context - this should only keep features specified in features_to_keep
-		let nullified_ctx = OwnedContextImpl::from_flags(original_ctx.clone().unwrap(), features_to_keep);
-
-		// Calculate hash of nullified context
-		let mut hasher1 = DefaultHasher::new();
-		nullified_ctx.hash(&mut hasher1);
-		let hash1 = hasher1.finish();
-
-		// Create a different original context with changed values
+		// A second context with different values for the nullified fields
 		let changed_ctx: Context = Some(Arc::new(
 			OwnedContextImpl::empty()
-				.with_footprint(Footprint::default()) // Same footprint
+				.with_footprint(Footprint::default())
 				.with_index(2)
-				.with_real_time(999.9) // Different real time
+				.with_real_time(999.9)
 				.with_vararg(Box::new("test"))
-				.with_animation_time(888.8), // Different animation time
+				.with_animation_time(888.8),
 		));
 
-		// Create nullified context from the changed original - should have same hash since everything is nullified
-		let nullified_changed_ctx = OwnedContextImpl::from_flags(changed_ctx.clone().unwrap(), features_to_keep);
+		// Nullify everything — both should hash the same regardless of their field values
+		let features_to_keep = ContextFeatures::empty();
+		let nullified1 = OwnedContextImpl::from_flags(original_ctx.clone().unwrap(), features_to_keep);
+		let nullified2 = OwnedContextImpl::from_flags(changed_ctx.clone().unwrap(), features_to_keep);
+
+		let mut hasher1 = DefaultHasher::new();
+		nullified1.cache_hash(&mut hasher1);
 
 		let mut hasher2 = DefaultHasher::new();
-		nullified_changed_ctx.hash(&mut hasher2);
-		let hash2 = hasher2.finish();
+		nullified2.cache_hash(&mut hasher2);
 
-		// Hash should be the same because all features were nullified
-		assert_eq!(hash1, hash2, "Hash of nullified context should remain stable regardless of input changes when features are nullified");
+		assert_eq!(
+			hasher1.finish(),
+			hasher2.finish(),
+			"Hash of nullified context should remain stable regardless of input changes when features are nullified"
+		);
 
-		// Test partial nullification - keep only footprint
+		// Keep only footprint and varargs — both have the same footprint and vararg, so hash should still match
 		let partial_features = ContextFeatures::FOOTPRINT | ContextFeatures::VARARGS;
-
-		let partial_nullified1 = OwnedContextImpl::from_flags(original_ctx.clone().unwrap(), partial_features);
-		let partial_nullified2 = OwnedContextImpl::from_flags(changed_ctx.clone().unwrap(), partial_features);
+		let partial1 = OwnedContextImpl::from_flags(original_ctx.clone().unwrap(), partial_features);
+		let partial2 = OwnedContextImpl::from_flags(changed_ctx.clone().unwrap(), partial_features);
 
 		let mut hasher3 = DefaultHasher::new();
-		partial_nullified1.hash(&mut hasher3);
-		let hash3 = hasher3.finish();
+		partial1.cache_hash(&mut hasher3);
 
 		let mut hasher4 = DefaultHasher::new();
-		partial_nullified2.hash(&mut hasher4);
-		let hash4 = hasher4.finish();
+		partial2.cache_hash(&mut hasher4);
 
-		// These should be the same because both have the same footprint (Footprint::default()) and varargs
-		// and other features are nullified
-		assert_eq!(hash3, hash4, "Hash should be stable when keeping only footprint and footprint values are the same");
+		assert_eq!(
+			hasher3.finish(),
+			hasher4.finish(),
+			"Hash should be stable when keeping only footprint and varargs and their values are the same"
+		);
 	}
 }

node-graph/nodes/gcore/src/extract_xy.rs

@@ -1,4 +1,4 @@
-use core_types::Ctx;
+use core_types::{CacheHash, Ctx};
 use dyn_any::DynAny;
 use glam::{DVec2, IVec2, UVec2};
 
@@ -15,7 +15,7 @@ fn extract_xy<T: Into<DVec2>>(_: impl Ctx, #[implementations(DVec2, IVec2, UVec2
 
 /// The X or Y component of a vec2.
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, DynAny, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, CacheHash, DynAny, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
 #[widget(Radio)]
 pub enum XY {
 	#[default]

node-graph/nodes/gcore/src/memo.rs

@@ -1,7 +1,8 @@
 use core_types::WasmNotSend;
+use core_types::graphene_hash::CacheHash;
 use core_types::memo::*;
 use std::hash::DefaultHasher;
-use std::hash::{Hash, Hasher};
+use std::hash::Hasher;
 use std::sync::Arc;
 use std::sync::Mutex;
 
@@ -13,9 +14,9 @@ use std::sync::Mutex;
 ///
 /// Currently, only one input-output pair is cached. Subsequent calls with different inputs will overwrite the previous cache.
 #[node_macro::node(category(""), path(graphene_core::memo), skip_impl)]
-async fn memo<I: Hash + Send + 'n, T: Clone + WasmNotSend>(input: I, #[data] cache: Arc<Mutex<Option<(u64, T)>>>, node: impl Node<I, Output = T>) -> T {
+async fn memo<I: CacheHash + Send + 'n, T: Clone + WasmNotSend>(input: I, #[data] cache: Arc<Mutex<Option<(u64, T)>>>, node: impl Node<I, Output = T>) -> T {
 	let mut hasher = DefaultHasher::new();
-	input.hash(&mut hasher);
+	input.cache_hash(&mut hasher);
 	let hash = hasher.finish();
 
 	if let Some(data) = cache.lock().as_ref().unwrap().as_ref().and_then(|data| (data.0 == hash).then_some(data.1.clone())) {

node-graph/nodes/graphic/src/graphic.rs

@@ -80,7 +80,7 @@ pub fn omit_element<T: graphic_types::graphic::OmitIndex + Clone + Default>(
 }
 
 #[node_macro::node(category("General"))]
-async fn map<Item: AnyHash + Send + Sync + std::hash::Hash>(
+async fn map<Item: AnyHash + Send + Sync + core_types::CacheHash>(
 	ctx: impl Ctx + CloneVarArgs + ExtractAll,
 	#[implementations(
 		Table<Graphic>,

node-graph/nodes/gstd/src/platform_application_io.rs

@@ -120,7 +120,7 @@ fn string_to_bytes(_: impl Ctx, string: String) -> Vec<u8> {
 #[node_macro::node(category("Web Request"), name("Image to Bytes"))]
 fn image_to_bytes(_: impl Ctx, image: Table<Raster<CPU>>) -> Vec<u8> {
 	let Some(image) = image.iter().next() else { return vec![] };
-	image.element.data.iter().flat_map(|color| color.to_rgb8_srgb().into_iter()).collect::<Vec<u8>>()
+	image.element.data.iter().flat_map(|color| color.to_rgba8_srgb().into_iter()).collect::<Vec<u8>>()
 }
 
 /// Loads binary from URLs and local asset paths. Returns a transparent placeholder if the resource fails to load, allowing rendering to continue.

node-graph/nodes/gstd/src/render_node.rs

@@ -21,7 +21,7 @@ use wgpu_executor::RenderContext;
 pub use crate::render_cache::render_output_cache;
 
 /// List of (canvas id, image data) pairs for embedding images as canvases in the final SVG string.
-type ImageData = HashMap<Image<Color>, u64>;
+type ImageData = HashMap<core_types::graphene_hash::CacheHashWrapper<Image<Color>>, u64>;
 
 #[derive(Clone, dyn_any::DynAny)]
 pub enum RenderIntermediateType {
@@ -191,7 +191,7 @@ async fn render<'a: 'n>(ctx: impl Ctx + ExtractFootprint + ExtractVarArgs, edito
 			rendering.wrap_with_transform(footprint.transform, Some(logical_resolution));
 			RenderOutputType::Svg {
 				svg: rendering.svg.to_svg_string(),
-				image_data: rendering.image_data.into_iter().map(|(image, id)| (id, image)).collect(),
+				image_data: rendering.image_data.into_iter().map(|(image, id)| (id, image.0)).collect(),
 			}
 		}
 		(RenderOutputTypeRequest::Vello, RenderIntermediateType::Vello(vello_data)) => {

node-graph/nodes/raster/Cargo.toml

@@ -15,6 +15,7 @@ shader-nodes = ["std", "dep:raster-nodes-shaders", "dep:wgpu-executor"]
 std = [
 	"dep:core-types",
 	"dep:dyn-any",
+	"dep:graphene-hash",
 	"dep:raster-types",
 	"dep:vector-types",
 	"dep:image",
@@ -41,6 +42,7 @@ node-macro = { workspace = true }
 # Local std dependencies
 dyn-any = { workspace = true, optional = true }
 core-types = { workspace = true, optional = true }
+graphene-hash = { workspace = true, optional = true }
 raster-types = { workspace = true, optional = true }
 vector-types = { workspace = true, optional = true }
 wgpu-executor = { workspace = true, optional = true }

node-graph/nodes/raster/src/adjustments.rs

@@ -1015,3 +1015,20 @@ fn exposure<T: Adjust<Color>>(
 	});
 	input
 }
+
+#[cfg(feature = "std")]
+mod _graphene_hash_impls {
+	use super::{CellularDistanceFunction, CellularReturnType, DomainWarpType, FractalType, LuminanceCalculation, NoiseType, RedGreenBlue, RedGreenBlueAlpha, RelativeAbsolute, SelectiveColorChoice};
+	graphene_hash::impl_via_hash!(
+		LuminanceCalculation,
+		RedGreenBlue,
+		RedGreenBlueAlpha,
+		NoiseType,
+		FractalType,
+		CellularDistanceFunction,
+		CellularReturnType,
+		DomainWarpType,
+		RelativeAbsolute,
+		SelectiveColorChoice
+	);
+}

node-graph/nodes/raster/src/curve.rs

@@ -1,11 +1,10 @@
 use core_types::Node;
 use core_types::color::{Channel, Linear, LuminanceMut};
 use dyn_any::{DynAny, StaticType, StaticTypeSized};
-use std::hash::{Hash, Hasher};
 use std::ops::{Add, Mul, Sub};
 
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Debug, Clone, PartialEq, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, PartialEq, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 pub struct Curve {
 	#[serde(rename = "manipulatorGroups")]
 	pub manipulator_groups: Vec<CurveManipulatorGroup>,
@@ -25,28 +24,13 @@ impl Default for Curve {
 	}
 }
 
-impl Hash for Curve {
-	fn hash<H: Hasher>(&self, state: &mut H) {
-		self.manipulator_groups.hash(state);
-		[self.first_handle, self.last_handle].iter().flatten().for_each(|f| f.to_bits().hash(state));
-	}
-}
-
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Debug, Clone, Copy, PartialEq, DynAny, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, Copy, PartialEq, core_types::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 pub struct CurveManipulatorGroup {
 	pub anchor: [f32; 2],
 	pub handles: [[f32; 2]; 2],
 }
 
-impl Hash for CurveManipulatorGroup {
-	fn hash<H: Hasher>(&self, state: &mut H) {
-		for c in self.handles.iter().chain([&self.anchor]).flatten() {
-			c.to_bits().hash(state);
-		}
-	}
-}
-
 pub struct ValueMapperNode<C> {
 	lut: Vec<C>,
 }

node-graph/nodes/text/Cargo.toml

@@ -13,6 +13,7 @@ wasm = ["core-types/wasm", "tsify", "wasm-bindgen"]
 [dependencies]
 # Local dependencies
 core-types = { workspace = true }
+graphene-hash = { workspace = true }
 raster-types = { workspace = true }
 vector-types = { workspace = true }
 node-macro = { workspace = true }

node-graph/nodes/text/src/font_cache.rs

@@ -1,3 +1,4 @@
+use core_types::graphene_hash::CacheHash;
 use dyn_any::DynAny;
 use parley::fontique::Blob;
 use std::collections::HashMap;
@@ -23,6 +24,14 @@ impl std::hash::Hash for Font {
 	}
 }
 
+impl CacheHash for Font {
+	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
+		self.font_family.cache_hash(state);
+		self.font_style.cache_hash(state);
+		// Don't consider `font_style_to_restore` in the HashMaps
+	}
+}
+
 impl PartialEq for Font {
 	fn eq(&self, other: &Self) -> bool {
 		// Don't consider `font_style_to_restore` in the HashMaps

node-graph/nodes/text/src/lib.rs

@@ -7,6 +7,7 @@ mod to_path;
 
 use convert_case::{Boundary, Converter, pattern};
 use core_types::Color;
+use core_types::graphene_hash::CacheHash;
 use core_types::registry::types::{SignedInteger, TextArea};
 use core_types::table::Table;
 use core_types::{CloneVarArgs, Context, Ctx, ExtractAll, ExtractVarArgs, OwnedContextImpl};
@@ -25,7 +26,7 @@ pub use vector_types;
 /// Alignment of lines of type within a text block.
 #[repr(C)]
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize, Hash, DynAny, node_macro::ChoiceType)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize, Hash, CacheHash, DynAny, node_macro::ChoiceType)]
 #[widget(Radio)]
 pub enum TextAlign {
 	#[default]
@@ -116,7 +117,7 @@ fn escape_string(input: String) -> String {
 	result
 }
 
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, dyn_any::DynAny, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
+#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Hash, CacheHash, dyn_any::DynAny, node_macro::ChoiceType, serde::Serialize, serde::Deserialize)]
 #[widget(Dropdown)]
 pub enum StringCapitalization {
 	/// "on the origin of species" — Converts all letters to lower case.

node-graph/nodes/vector/Cargo.toml

@@ -13,6 +13,7 @@ wasm = ["core-types/wasm", "tsify", "wasm-bindgen"]
 [dependencies]
 # Local dependencies
 core-types = { workspace = true }
+graphene-hash = { workspace = true }
 vector-types = { workspace = true }
 graphic-types = { workspace = true }
 node-macro = { workspace = true }

node-graph/nodes/vector/src/generator_nodes.rs

@@ -1,6 +1,6 @@
-use core_types::Ctx;
 use core_types::registry::types::{Angle, PixelLength, PixelSize};
 use core_types::table::Table;
+use core_types::{CacheHash, Ctx};
 use dyn_any::DynAny;
 use glam::DVec2;
 use graphic_types::Vector;
@@ -188,7 +188,7 @@ fn star<T: AsU64>(
 }
 
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize, Hash, DynAny, node_macro::ChoiceType)]
+#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize, Hash, CacheHash, DynAny, node_macro::ChoiceType)]
 #[widget(Radio)]
 pub enum QRCodeErrorCorrectionLevel {
 	/// Allows recovery from up to 7% data loss.