Fix hashing not being based on all attributes of Table (#4079)

* Fix hashing not being based on all attributes of Table

* Cover all attributes in Table PartialEq and CacheHash

---------

node-graph/libraries/core-types/src/table.rs

@@ -3,6 +3,7 @@ use crate::math::quad::Quad;
 use crate::transform::ApplyTransform;
 use dyn_any::{StaticType, StaticTypeSized};
 use glam::DAffine2;
+use graphene_hash::CacheHash;
 use std::fmt::Debug;
 
 // =====================================================================
@@ -83,7 +84,7 @@ trait AttributeValue: std::any::Any + Send + Sync {
 	fn into_column(self: Box<Self>, preceding_defaults: usize) -> Box<dyn AttributeColumn>;
 }
 
-impl<T: Clone + Send + Sync + Default + Sized + Debug + 'static> AttributeValue for T {
+impl<T: Clone + Send + Sync + Default + Sized + Debug + PartialEq + CacheHash + 'static> AttributeValue for T {
 	/// Clones this value into a new boxed trait object.
 	fn clone_box(&self) -> Box<dyn AttributeValue> {
 		Box::new(self.clone())
@@ -164,6 +165,25 @@ trait AttributeColumn: std::any::Any + Send + Sync {
 
 	/// Drains all values out of this column into a Vec of scalar attribute values.
 	fn drain(self: Box<Self>) -> Vec<Box<dyn AttributeValue>>;
+
+	/// Hashes every value in this column into the given hasher (object-safe wrapper around `CacheHash`).
+	fn cache_hash_dyn(&self, state: &mut dyn core::hash::Hasher);
+
+	/// Compares this column to another for value-by-value equality (object-safe wrapper around `PartialEq`).
+	/// Returns `false` if the underlying types differ.
+	fn eq_dyn(&self, other: &dyn AttributeColumn) -> bool;
+}
+
+/// Adapts a `&mut dyn Hasher` so generic `CacheHash::cache_hash<H>` calls (which require `H: Sized + Hasher`) can
+/// drive a trait-object hasher. Forwards both `Hasher` methods to the inner `dyn Hasher`.
+struct DynHasher<'a>(&'a mut dyn core::hash::Hasher);
+impl core::hash::Hasher for DynHasher<'_> {
+	fn finish(&self) -> u64 {
+		self.0.finish()
+	}
+	fn write(&mut self, bytes: &[u8]) {
+		self.0.write(bytes)
+	}
 }
 
 impl Clone for Box<dyn AttributeColumn> {
@@ -179,7 +199,7 @@ impl Clone for Box<dyn AttributeColumn> {
 /// Wraps a Vec<T> for column-major attribute storage in a Table.
 struct Column<T>(Vec<T>);
 
-impl<T: Clone + Send + Sync + Default + Debug + 'static> AttributeColumn for Column<T> {
+impl<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static> AttributeColumn for Column<T> {
 	/// Clones this column into a new boxed trait object.
 	fn clone_box(&self) -> Box<dyn AttributeColumn> {
 		Box::new(Column(self.0.clone()))
@@ -250,6 +270,16 @@ impl<T: Clone + Send + Sync + Default + Debug + 'static> AttributeColumn for Col
 	fn drain(self: Box<Self>) -> Vec<Box<dyn AttributeValue>> {
 		self.0.into_iter().map(|v| Box::new(v) as Box<dyn AttributeValue>).collect()
 	}
+
+	/// Hashes every value in this column into the given hasher (object-safe wrapper around `CacheHash`).
+	fn cache_hash_dyn(&self, state: &mut dyn core::hash::Hasher) {
+		self.0.cache_hash(&mut DynHasher(state));
+	}
+
+	/// Compares this column to another for value-by-value equality (object-safe wrapper around `PartialEq`).
+	fn eq_dyn(&self, other: &dyn AttributeColumn) -> bool {
+		other.as_any().downcast_ref::<Self>().is_some_and(|other| self.0 == other.0)
+	}
 }
 
 // ===============
@@ -278,7 +308,7 @@ impl AttributeValues {
 	}
 
 	/// Inserts an attribute with the given key and value, replacing any existing entry with the same key.
-	pub fn insert<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: impl Into<String>, value: T) {
+	pub fn insert<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: impl Into<String>, value: T) {
 		let key = key.into();
 
 		for (existing_key, existing_value) in &mut self.0 {
@@ -309,7 +339,7 @@ impl AttributeValues {
 	}
 
 	/// Gets a mutable reference to the value, inserting a default if it doesn't exist or has the wrong type.
-	pub fn get_or_insert_default_mut<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> &mut T {
+	pub fn get_or_insert_default_mut<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> &mut T {
 		let needs_insert = match self.0.iter().position(|(existing_key, _)| existing_key == key) {
 			Some(index) => {
 				if (*self.0[index].1).as_any().downcast_ref::<T>().is_some() {
@@ -448,7 +478,7 @@ impl AttributeColumns {
 	/// Finds or creates a column for the given key and type, returning its position.
 	/// If a column with the key exists but has the wrong type, it is removed and replaced with a new column of the correct type, padded with defaults.
 	/// A newly created column is filled with `T::default()` for all existing rows.
-	fn find_or_create_column<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> usize {
+	fn find_or_create_column<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> usize {
 		match self.columns.iter().position(|(k, _)| k == key) {
 			Some(position) => {
 				if (*self.columns[position].1).as_any().downcast_ref::<Column<T>>().is_some() {
@@ -467,7 +497,7 @@ impl AttributeColumns {
 	}
 
 	/// Gets a mutable reference to the value at the given index, creating the column if it doesn't exist or has the wrong type.
-	fn get_or_insert_default_value<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str, index: usize) -> &mut T {
+	fn get_or_insert_default_value<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str, index: usize) -> &mut T {
 		let column_position = self.find_or_create_column::<T>(key);
 		let column = (*self.columns[column_position].1).as_any_mut().downcast_mut::<Column<T>>().unwrap();
 		&mut column.0[index]
@@ -475,7 +505,7 @@ impl AttributeColumns {
 
 	/// Sets the value at the given index in the column for the given key.
 	/// Creates the column with defaults if it doesn't exist.
-	fn set_value<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: impl Into<String>, index: usize, value: T) {
+	fn set_value<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: impl Into<String>, index: usize, value: T) {
 		let key = key.into();
 		let column_position = self.find_or_create_column::<T>(&key);
 		let column = (*self.columns[column_position].1).as_any_mut().downcast_mut::<Column<T>>().unwrap();
@@ -527,7 +557,7 @@ impl AttributeColumns {
 	}
 
 	/// Returns a mutable typed slice of the column for the given key, creating a new column filled with defaults if it doesn't exist.
-	fn get_or_create_column_slice_mut<T: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> &mut [T] {
+	fn get_or_create_column_slice_mut<T: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> &mut [T] {
 		let position = self.find_or_create_column::<T>(key);
 		let column = (*self.columns[position].1).as_any_mut().downcast_mut::<Column<T>>().unwrap();
 		&mut column.0
@@ -667,7 +697,7 @@ impl<T> Table<T> {
 	}
 
 	/// Returns a mutable iterator over a typed attribute column, creating the column with default values if it doesn't exist or has the wrong type.
-	pub fn iter_attribute_values_mut_or_default<U: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> std::slice::IterMut<'_, U> {
+	pub fn iter_attribute_values_mut_or_default<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> std::slice::IterMut<'_, U> {
 		self.attributes.get_or_create_column_slice_mut::<U>(key).iter_mut()
 	}
 
@@ -705,13 +735,13 @@ impl<T> Table<T> {
 	}
 
 	/// Sets the attribute value at the given row index and key, creating the column with defaults if it doesn't exist.
-	pub fn set_attribute<U: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: impl Into<String>, index: usize, value: U) {
+	pub fn set_attribute<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: impl Into<String>, index: usize, value: U) {
 		self.attributes.set_value(key, index, value);
 	}
 
 	/// Runs the given closure on a mutable reference to the attribute value at the given row index,
 	/// creating the column with defaults if it doesn't exist, and returns the closure's result.
-	pub fn with_attribute_mut_or_default<U: Clone + Send + Sync + Default + Debug + 'static, R, F: FnOnce(&mut U) -> R>(&mut self, key: &str, index: usize, f: F) -> R {
+	pub fn with_attribute_mut_or_default<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static, R, F: FnOnce(&mut U) -> R>(&mut self, key: &str, index: usize, f: F) -> R {
 		f(self.attributes.get_or_insert_default_value::<U>(key, index))
 	}
 
@@ -732,7 +762,7 @@ impl<T> Table<T> {
 	/// Returns disjoint mutable references to the element slice and a typed attribute column slice,
 	/// creating the column with defaults if it doesn't exist. This enables simultaneous mutable
 	/// access to elements and a single attribute column without borrowing conflicts.
-	pub fn element_and_attribute_slices_mut<U: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> (&mut [T], &mut [U]) {
+	pub fn element_and_attribute_slices_mut<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> (&mut [T], &mut [U]) {
 		let Self { element, attributes } = self;
 		let column_position = attributes.find_or_create_column::<U>(key);
 		let column = (*attributes.columns[column_position].1).as_any_mut().downcast_mut::<Column<U>>().unwrap();
@@ -839,23 +869,30 @@ impl<T> Default for Table<T> {
 	}
 }
 
-impl<T: graphene_hash::CacheHash> graphene_hash::CacheHash for Table<T> {
+impl<T: CacheHash> CacheHash for Table<T> {
 	fn cache_hash<H: core::hash::Hasher>(&self, state: &mut H) {
-		for element in self.iter_element_values() {
+		self.element.cache_hash(state);
-			element.cache_hash(state);
+
-		}
+		// Hash every attribute column (key + values) rather than just the well-known ones, so changes to user-defined keys
-		for transform in self.iter_attribute_values_or_default::<DAffine2>(ATTR_TRANSFORM) {
+		// (e.g., gradient_type, spread_method) invalidate downstream graph caches as expected
-			graphene_hash::CacheHash::cache_hash(&transform, state);
+		for (key, column) in &self.attributes.columns {
-		}
+			std::hash::Hash::hash(key.as_str(), state);
-		for alpha_blending in self.iter_attribute_values_or_default::<crate::AlphaBlending>(ATTR_ALPHA_BLENDING) {
+			column.cache_hash_dyn(state);
-			alpha_blending.cache_hash(state);
 		}
 	}
 }
 
 impl<T: PartialEq> PartialEq for Table<T> {
 	fn eq(&self, other: &Self) -> bool {
+		// Attributes participate in equality so the `a == b` ⇒ `hash(a) == hash(b)` contract holds with `cache_hash`
 		self.element == other.element
+			&& self.attributes.columns.len() == other.attributes.columns.len()
+			&& self
+				.attributes
+				.columns
+				.iter()
+				.zip(&other.attributes.columns)
+				.all(|((self_key, self_column), (other_key, other_column))| self_key == other_key && self_column.eq_dyn(other_column.as_ref()))
 	}
 }
 
@@ -1016,17 +1053,17 @@ impl<T> TableRow<T> {
 	}
 
 	/// Returns a mutable reference to the attribute value for the given key, inserting a default value if absent or of a different type.
-	pub fn attribute_mut_or_insert_default<U: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: &str) -> &mut U {
+	pub fn attribute_mut_or_insert_default<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: &str) -> &mut U {
 		self.attributes.get_or_insert_default_mut(key)
 	}
 
 	/// Sets the attribute value for the given key, replacing any existing entry with the same key.
-	pub fn set_attribute<U: Clone + Send + Sync + Default + Debug + 'static>(&mut self, key: impl Into<String>, value: U) {
+	pub fn set_attribute<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(&mut self, key: impl Into<String>, value: U) {
 		self.attributes.insert(key, value);
 	}
 
 	/// Sets the attribute value for the given key and returns the row, enabling builder-style chaining.
-	pub fn with_attribute<U: Clone + Send + Sync + Default + Debug + 'static>(mut self, key: impl Into<String>, value: U) -> Self {
+	pub fn with_attribute<U: Clone + Send + Sync + Default + Debug + PartialEq + CacheHash + 'static>(mut self, key: impl Into<String>, value: U) -> Self {
 		self.set_attribute(key, value);
 		self
 	}

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

@@ -2,7 +2,7 @@ use core_types::bounds::{BoundingBox, RenderBoundingBox};
 use core_types::registry::types::{Angle, SignedInteger};
 use core_types::table::{Table, TableRow};
 use core_types::uuid::NodeId;
-use core_types::{ATTR_EDITOR_LAYER_PATH, ATTR_TRANSFORM, AnyHash, CloneVarArgs, Color, Context, Ctx, ExtractAll, OwnedContextImpl};
+use core_types::{ATTR_EDITOR_LAYER_PATH, ATTR_TRANSFORM, AnyHash, CacheHash, CloneVarArgs, Color, Context, Ctx, ExtractAll, OwnedContextImpl};
 use glam::{DAffine2, DVec2};
 use graphic_types::Vector;
 use graphic_types::graphic::{Graphic, IntoGraphicTable};
@@ -107,7 +107,7 @@ pub fn extract_element<T: Clone + Default + Send + Sync + 'static>(
 }
 
 #[node_macro::node(category("General"))]
-async fn map<Item: AnyHash + Send + Sync + core_types::CacheHash>(
+async fn map<Item: AnyHash + Send + Sync + CacheHash>(
 	ctx: impl Ctx + CloneVarArgs + ExtractAll,
 	#[implementations(
 		Table<Graphic>,
@@ -221,7 +221,7 @@ pub fn path_of_subgraph(_: impl Ctx, node_path: Table<NodeId>) -> Table<NodeId>
 /// context, so the upstream pipeline can return a different value per item that may be derived from the item's own data.
 /// If the attribute already exists, its values are replaced; if not, the attribute is added.
 #[node_macro::node(category("General"))]
-async fn write_attribute<T: AnyHash + Clone + Send + Sync + core_types::CacheHash, U: Clone + Send + Sync + Default + std::fmt::Debug + 'static>(
+async fn write_attribute<T: AnyHash + Clone + Send + Sync + CacheHash, U: Clone + Send + Sync + Default + std::fmt::Debug + PartialEq + CacheHash + 'static>(
 	ctx: impl ExtractAll + CloneVarArgs + Ctx,
 	/// The `Table` whose items will gain or have replaced the named attribute.
 	#[implementations(