Code review

Author: Keavon

node-graph/graph-craft/src/document/value.rs

@@ -523,6 +523,8 @@ impl Hash for RenderOutputType {
 	}
 }
 
+// Metadata is excluded because it's editor-side auxiliary data (click targets, transforms)
+// that shouldn't affect render cache invalidation, and it contains HashMaps with non-deterministic iteration order
 impl CacheHash for RenderOutput {
 	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
 		self.data.cache_hash(state);

node-graph/libraries/graphene-hash/derive/src/lib.rs

@@ -26,22 +26,13 @@ use syn::{Data, DeriveInput, Fields, parse_macro_input};
 pub fn derive_cache_hash(input: TokenStream) -> TokenStream {
 	let ast = parse_macro_input!(input as DeriveInput);
 	let name = &ast.ident;
-	let (impl_generics, ty_generics, where_clause) = ast.generics.split_for_impl();
-
-	// Build additional `T: CacheHash` bounds for each type parameter.
-	let extra_bounds = ast.generics.type_params().map(|tp| {
-		let ident = &tp.ident;
-		quote! { #ident: graphene_hash::CacheHash }
-	});
-	let where_clause = if ast.generics.type_params().count() > 0 {
-		let existing = where_clause.map(|w| quote! { #w }).unwrap_or_default();
-		quote! {
-			#existing
-			where #(#extra_bounds,)*
+	let mut generics = ast.generics.clone();
+	for param in &mut generics.params {
+		if let syn::GenericParam::Type(type_param) = param {
+			type_param.bounds.push(syn::parse_quote!(graphene_hash::CacheHash));
 		}
-	} else {
-		where_clause.map(|w| quote! { #w }).unwrap_or_default()
-	};
+	}
+	let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();
 
 	let body = match &ast.data {
 		Data::Struct(s) => hash_fields(&s.fields, quote! { self }),
@@ -89,7 +80,7 @@ pub fn derive_cache_hash(input: TokenStream) -> TokenStream {
 				}
 			}
 		}
-		Data::Union(_) => panic!("CacheHash cannot be derived for unions"),
+		Data::Union(_) => return syn::Error::new(ast.ident.span(), "CacheHash cannot be derived for unions").to_compile_error().into(),
 	};
 
 	quote! {

node-graph/libraries/raster-types/src/image.rs

@@ -64,7 +64,9 @@ impl<P: Pixel + PartialEq> PartialEq for Image<P> {
 pub struct TransformImage(pub DAffine2);
 
 impl core_types::CacheHash for TransformImage {
-	fn cache_hash<H: ::core::hash::Hasher>(&self, _: &mut H) {}
+	fn cache_hash<H: ::core::hash::Hasher>(&self, state: &mut H) {
+		core_types::CacheHash::cache_hash(&self.0, state);
+	}
 }
 
 impl<P: Pixel + std::fmt::Debug> std::fmt::Debug for Image<P> {

node-graph/libraries/vector-types/src/vector/vector_modification.rs

@@ -517,6 +517,8 @@ impl Hash for VectorModification {
 	}
 }
 
+// Intentionally non-deterministic: fields contain HashMaps with non-deterministic iteration order,
+// so we use a UUID to always bust the cache and force re-evaluation when any modification is present
 impl graphene_hash::CacheHash for VectorModification {
 	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
 		core::hash::Hash::hash(&generate_uuid(), state);