Wrap serialized gradient stops in a new GradientRamp struct and unify FillChoice (#4400)

* Introduce the GradientRamp exchange struct as the serialized TaggedValue::Gradient payload

* Unify FillChoice and FillChoiceUI into one enum generic over color format, carrying GradientRamp stops

* Rename the TaggedValue::Gradient variant to GradientRamp to match its payload

* Move the Color variant into the tagged_value macro list since its stored and wire forms match

node-graph/libraries/graphic-types/src/lib.rs

@@ -15,6 +15,9 @@ pub use markers::{ATTR_EDITOR_MERGED_LAYERS, ATTR_FILL, ATTR_STROKE};
 
 pub mod migrations {
 	use crate::Vector;
+	use core_types::Color;
+	use vector_types::gradient::GradientStops;
+	use vector_types::{Gradient, GradientRamp};
 
 	// Storing legacy structs that are only used in document migration.
 	// TODO: Eventually remove this document upgrade code
@@ -23,11 +26,12 @@ pub mod migrations {
 		use dyn_any::DynAny;
 		use glam::{DAffine2, DVec2};
 		use vector_types::vector::{PointDomain, RegionDomain, SegmentDomain, misc::HandleId, style::Stroke};
-		use vector_types::{Gradient, Vector, vector};
+		use vector_types::{GradientRamp, Vector, vector};
 
 		#[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash, DynAny, serde::Serialize, serde::Deserialize)]
 		pub struct LegacyGradient {
-			pub stops: Gradient,
+			#[serde(deserialize_with = "crate::migrations::migrate_to_gradient_ramp")]
+			pub stops: GradientRamp,
 			pub gradient_type: vector::style::GradientType,
 			pub start: DVec2,
 			pub end: DVec2,
@@ -145,6 +149,33 @@ pub mod migrations {
 		})
 	}
 
+	// TODO: Eventually remove this document upgrade code
+	/// Recovers a [`GradientRamp`] from any of its on-disk shapes: the current nested form, the flat stops struct
+	/// that preceded it, or the ancient position-color tuple list (whose even positions elide back to absence).
+	pub fn migrate_to_gradient_ramp<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<GradientRamp, D::Error> {
+		use serde::Deserialize;
+
+		#[derive(serde::Deserialize)]
+		#[serde(untagged)]
+		enum GradientRampFormat {
+			Ramp(GradientRamp),
+			FlatStops(GradientStops<Color>),
+			Tuples(Vec<(f64, Color)>),
+		}
+
+		Ok(match GradientRampFormat::deserialize(deserializer)? {
+			GradientRampFormat::Ramp(ramp) => ramp,
+			GradientRampFormat::FlatStops(stops) => GradientRamp::from(stops),
+			GradientRampFormat::Tuples(stops) => {
+				let position: Vec<f64> = stops.iter().map(|(position, _)| *position).collect();
+				let mut gradient = Gradient::from(stops.into_iter().map(|(_, color)| color).collect::<Vec<_>>());
+				gradient.set_positions(&position);
+				gradient.elide_default_attributes();
+				GradientRamp::from(gradient)
+			}
+		})
+	}
+
 	#[cfg(test)]
 	mod migration_tests {
 		use super::*;

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

@@ -21,10 +21,10 @@ pub enum GradientType {
 #[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash, DynAny)]
 pub struct Gradient(List<Color>);
 
-/// A gradient's per-stop parallel arrays, generic over color format: `GradientStops<Color>` is the document serialization
-/// of `TaggedValue::Gradient`, while `GradientStops<SRGBA8>` is the JS-boundary shape used by the color picker UI.
+/// A gradient's per-stop parallel arrays, generic over color format: `GradientStops<Color>` nests inside the
+/// [`GradientRamp`] exchange struct, while `GradientStops<SRGBA8>` is the JS-boundary shape used by the color picker UI.
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
-#[derive(Debug, Clone, PartialEq, Default)]
+#[derive(Debug, Clone, PartialEq, Default, graphene_hash::CacheHash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct GradientStops<C> {
 	pub color: Vec<C>,
@@ -96,34 +96,87 @@ impl GradientStops<SRGBA8> {
 	}
 }
 
-#[cfg(feature = "serde")]
-impl serde::Serialize for Gradient {
-	fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
-		GradientStops::<Color>::from(self).serialize(serializer)
+/// The serialized exchange form of a gradient: its stops, nested so that whole-ramp settings
+/// like spread method can join as sibling fields opted in from their defaults.
+#[cfg_attr(feature = "wasm", derive(tsify::Tsify))]
+#[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+pub struct GradientRamp<C = Color> {
+	pub stops: GradientStops<C>,
+}
+
+unsafe impl<C: dyn_any::StaticTypeSized> dyn_any::StaticType for GradientRamp<C> {
+	type Static = GradientRamp<C::Static>;
+}
+
+impl<C> From<GradientStops<C>> for GradientRamp<C> {
+	fn from(stops: GradientStops<C>) -> Self {
+		Self { stops }
 	}
 }
 
-// TODO: Eventually remove this document upgrade code
-#[cfg(feature = "serde")]
-impl<'de> serde::Deserialize<'de> for Gradient {
-	fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
-		#[derive(serde::Deserialize)]
-		#[serde(untagged)]
-		enum GradientStopsFormat {
-			Struct(GradientStops<Color>),
-			Tuples(Vec<(f64, Color)>),
-		}
+impl From<&Gradient> for GradientRamp {
+	fn from(gradient: &Gradient) -> Self {
+		Self { stops: gradient.into() }
+	}
+}
 
-		Ok(match GradientStopsFormat::deserialize(deserializer)? {
-			GradientStopsFormat::Struct(stops) => Gradient::from(stops),
-			GradientStopsFormat::Tuples(stops) => {
-				let position: Vec<f64> = stops.iter().map(|(p, _)| *p).collect();
-				let mut gradient = Gradient::from(stops.into_iter().map(|(_, c)| c).collect::<Vec<_>>());
-				gradient.set_positions(&position);
-				gradient.elide_default_attributes();
-				gradient
-			}
-		})
+impl From<Gradient> for GradientRamp {
+	fn from(gradient: Gradient) -> Self {
+		Self::from(&gradient)
+	}
+}
+
+impl From<GradientRamp> for Gradient {
+	fn from(ramp: GradientRamp) -> Self {
+		Gradient::from(ramp.stops)
+	}
+}
+
+impl From<&GradientRamp> for Gradient {
+	fn from(ramp: &GradientRamp) -> Self {
+		Gradient::from(ramp.stops.clone())
+	}
+}
+
+impl From<&GradientRamp> for GradientStops<SRGBA8> {
+	fn from(ramp: &GradientRamp) -> Self {
+		Self {
+			position: ramp.stops.position.clone(),
+			midpoint: ramp.stops.midpoint.clone(),
+			color: ramp.stops.color.iter().map(|&color| SRGBA8::from(color)).collect(),
+		}
+	}
+}
+
+// Color picker round-trip: routes through the runtime type so default-restating attributes elide
+impl From<&GradientStops<SRGBA8>> for GradientRamp {
+	fn from(stops: &GradientStops<SRGBA8>) -> Self {
+		Self::from(Gradient::from(stops))
+	}
+}
+
+impl From<&GradientRamp> for GradientRamp<SRGBA8> {
+	fn from(ramp: &GradientRamp) -> Self {
+		Self { stops: ramp.into() }
+	}
+}
+
+impl From<&Gradient> for GradientRamp<SRGBA8> {
+	fn from(gradient: &Gradient) -> Self {
+		Self { stops: gradient.into() }
+	}
+}
+
+impl From<&GradientRamp<SRGBA8>> for GradientRamp {
+	fn from(ramp: &GradientRamp<SRGBA8>) -> Self {
+		Self::from(&ramp.stops)
+	}
+}
+
+impl GradientRamp {
+	pub fn black_to_white() -> Self {
+		Self::from(Gradient::black_to_white())
 	}
 }
 
@@ -793,31 +846,17 @@ mod tests {
 
 	#[test]
 	fn serde_round_trip_preserves_attribute_absence() {
-		let implicit = Gradient::from(vec![Color::BLACK, Color::WHITE]);
+		let implicit = GradientRamp::from(Gradient::from(vec![Color::BLACK, Color::WHITE]));
 		let json = serde_json::to_string(&implicit).unwrap();
 		assert!(!json.contains("position") && !json.contains("midpoint"), "absent attributes must not serialize: {json}");
-		assert_eq!(serde_json::from_str::<Gradient>(&json).unwrap(), implicit);
+		assert_eq!(serde_json::from_str::<GradientRamp>(&json).unwrap(), implicit);
 
-		let mut explicit = implicit.clone();
+		let mut explicit = Gradient::from(vec![Color::BLACK, Color::WHITE]);
 		explicit.set_positions(&[0.2, 0.9]);
 		explicit.set_midpoints(&[0.3, 0.5]);
+		let explicit = GradientRamp::from(explicit);
 		let json = serde_json::to_string(&explicit).unwrap();
-		assert_eq!(serde_json::from_str::<Gradient>(&json).unwrap(), explicit);
-	}
-
-	#[test]
-	fn legacy_tuple_format_deserializes_with_defaults_elided() {
-		let color = serde_json::to_value(Color::WHITE).unwrap();
-
-		let struct_format = serde_json::json!({ "position": [0., 0.25], "midpoint": [0.5, 0.5], "color": [color, color] });
-		let gradient: Gradient = serde_json::from_value(struct_format).unwrap();
-		assert_eq!(gradient.positions(), vec![0., 0.25]);
-		assert!(gradient.has_midpoint_attribute(), "the struct form must parse faithfully");
-
-		let tuple_format = serde_json::json!([[0., color], [1., color]]);
-		let gradient: Gradient = serde_json::from_value(tuple_format).unwrap();
-		assert_eq!(gradient.positions(), vec![0., 1.]);
-		assert!(!gradient.has_position_attribute(), "even legacy tuple positions should elide");
+		assert_eq!(serde_json::from_str::<GradientRamp>(&json).unwrap(), explicit);
 	}
 
 	#[test]

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

@@ -9,7 +9,7 @@ pub mod vector;
 
 // Re-export commonly used types at the crate root
 pub use core_types as gcore;
-pub use gradient::{Gradient, GradientSpreadMethod, GradientStop, GradientType};
+pub use gradient::{Gradient, GradientRamp, GradientSpreadMethod, GradientStop, GradientType};
 pub use markers::{ATTR_EDITOR_CLICK_TARGET, ATTR_GRADIENT_TYPE, ATTR_SPREAD_METHOD};
 pub use math::{QuadExt, RectExt};
 pub use subpath::Subpath;

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

@@ -9,68 +9,65 @@ use dyn_any::DynAny;
 use glam::DAffine2;
 use std::f64::consts::{PI, TAU};
 
-/// The editor's in-memory paint picker state, storing color or gradient stops without gradient placement metadata.
-/// Not stored in documents: paint inputs hold the picked value as a plain color, gradient, or no-paint type default.
+/// The paint picker's choice of fill, generic over color format: `FillChoice<Color>` is the editor's in-memory
+/// form, while `FillChoice<SRGBA8>` is the JS-boundary shape used by the color picker UI. Stores a color or
+/// gradient ramp without gradient placement metadata, and is not stored in documents: paint inputs hold the
+/// picked value as a plain color, gradient, or no-paint type default.
 ///
-/// Can be None, a solid [Color], or a linear/radial [Gradient].
+/// Can be None, a solid color, or the [`GradientRamp`] of a linear/radial gradient.
 ///
 /// In the future we'll probably also add a pattern fill.
-///
-/// Use [`FillChoiceUI`] at the JS boundary.
 #[repr(C)]
-#[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash, DynAny)]
-#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-pub enum FillChoice {
-	#[default]
-	None,
-	Solid(Color),
-	Gradient(Gradient),
-}
-
-// TODO: Deprecate [`FillChoice`] and keep this, renamed, as the main widget-controlling type
-/// JS-boundary version of [`FillChoice`] where the solid color is [`SRGBA8`] and the gradient is its [`GradientStops`] exchange form.
 #[cfg_attr(feature = "wasm", derive(tsify::Tsify), tsify(from_wasm_abi))]
-#[derive(Default, Debug, Clone, PartialEq, DynAny)]
+#[derive(Default, Debug, Clone, PartialEq, graphene_hash::CacheHash)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
-pub enum FillChoiceUI {
+pub enum FillChoice<C = Color> {
 	#[default]
 	None,
-	Solid(SRGBA8),
-	Gradient(GradientStops<SRGBA8>),
+	Solid(C),
+	Gradient(GradientRamp<C>),
 }
 
-impl From<&FillChoice> for FillChoiceUI {
+unsafe impl<C: dyn_any::StaticTypeSized> dyn_any::StaticType for FillChoice<C> {
+	type Static = FillChoice<C::Static>;
+}
+
+impl From<&FillChoice> for FillChoice<SRGBA8> {
 	fn from(value: &FillChoice) -> Self {
 		match value {
 			FillChoice::None => Self::None,
 			FillChoice::Solid(color) => Self::Solid(SRGBA8::from(*color)),
-			FillChoice::Gradient(stops) => Self::Gradient(stops.into()),
+			FillChoice::Gradient(ramp) => Self::Gradient(ramp.into()),
 		}
 	}
 }
 
-impl From<&FillChoiceUI> for FillChoice {
-	fn from(value: &FillChoiceUI) -> Self {
+impl From<&FillChoice<SRGBA8>> for FillChoice {
+	fn from(value: &FillChoice<SRGBA8>) -> Self {
 		match value {
-			FillChoiceUI::None => Self::None,
-			FillChoiceUI::Solid(srgba) => Self::Solid(Color::from(*srgba)),
-			FillChoiceUI::Gradient(stops) => Self::Gradient(Gradient::from(stops)),
+			FillChoice::None => Self::None,
+			FillChoice::Solid(srgba) => Self::Solid(Color::from(*srgba)),
+			FillChoice::Gradient(ramp) => Self::Gradient(ramp.into()),
 		}
 	}
 }
 
-impl FillChoiceUI {
-	pub fn as_solid(&self) -> Option<SRGBA8> {
-		let Self::Solid(c) = self else { return None };
-		Some(*c)
+impl<C: Copy> FillChoice<C> {
+	pub fn as_solid(&self) -> Option<C> {
+		let Self::Solid(color) = self else { return None };
+		Some(*color)
 	}
+}
 
-	pub fn as_gradient(&self) -> Option<&GradientStops<SRGBA8>> {
-		let Self::Gradient(g) = self else { return None };
-		Some(g)
+impl<C> FillChoice<C> {
+	pub fn as_gradient(&self) -> Option<&GradientRamp<C>> {
+		let Self::Gradient(ramp) = self else { return None };
+		Some(ramp)
 	}
+}
 
-	/// Build a CSS `background-image` string (always a `linear-gradient(...)`) representing this fill, or `None` if the fill is [`FillChoiceUI::None`].
+impl FillChoice<SRGBA8> {
+	/// Build a CSS `background-image` string (always a `linear-gradient(...)`) representing this fill, or `None` if the fill is [`FillChoice::None`].
 	/// Solid colors become a degenerate gradient between the same color so the CSS variable can always be assigned to a `background-image`.
 	pub fn to_css_background_image(&self) -> Option<String> {
 		match self {
@@ -79,31 +76,7 @@ impl FillChoiceUI {
 				let hex = srgba.to_rgba_hex();
 				Some(format!("linear-gradient(#{hex}, #{hex})"))
 			}
-			Self::Gradient(stops) => Some(stops.to_css_linear_gradient()),
-		}
-	}
-}
-
-impl FillChoice {
-	pub fn as_solid(&self) -> Option<Color> {
-		let Self::Solid(color) = self else { return None };
-		Some(*color)
-	}
-
-	pub fn as_gradient(&self) -> Option<&Gradient> {
-		let Self::Gradient(gradient) = self else { return None };
-		Some(gradient)
-	}
-
-	/// Build a CSS `background-image` string (always a `linear-gradient(...)`) representing this fill, or `None` if the fill is [`FillChoice::None`]. Solid colors become a degenerate gradient between the same color so the CSS variable can always be assigned to a `background-image`.
-	pub fn to_css_background_image(&self) -> Option<String> {
-		match self {
-			Self::None => None,
-			Self::Solid(color) => {
-				let hex = SRGBA8::from(*color).to_rgba_hex();
-				Some(format!("linear-gradient(#{hex}, #{hex})"))
-			}
-			Self::Gradient(stops) => Some(stops.to_css_linear_gradient()),
+			Self::Gradient(ramp) => Some(ramp.stops.to_css_linear_gradient()),
 		}
 	}
 }