Move the gradient spread method into GradientRamp and the color picker popover (#4402)

* Add a spread method field to GradientRamp, carried at runtime as the gradient item's attribute

* Retire the Fill node's spread method input, folding its value into the gradient ramps on document upgrade

* Add an Ends spread method radio to the color picker popover, replacing the Gradient tool's control bar radio

* Update the demo art

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

@@ -201,7 +201,7 @@ macro_rules! tagged_value {
 					}
 					Self::DashPattern(lengths) => Box::new(Item::new_from_element(DashPattern::from(lengths))),
 					Self::BoxCorners(values) => Box::new(Item::new_from_element(BoxCorners::from(values))),
-					Self::GradientRamp(ramp) => Box::new(Item::new_from_element(Gradient::from(ramp))),
+					Self::GradientRamp(ramp) => Box::new(Item::<Gradient>::from(ramp)),
 					Self::BrushStrokes(strokes) => Box::new(core_types::list::Item::new_from_element(BrushTrace::from(strokes))),
 					// =======================
 					// AUTO-GENERATED VARIANTS
@@ -264,7 +264,7 @@ macro_rules! tagged_value {
 					}
 					Self::DashPattern(lengths) => Arc::new(Item::new_from_element(DashPattern::from(lengths))),
 					Self::BoxCorners(values) => Arc::new(Item::new_from_element(BoxCorners::from(values))),
-					Self::GradientRamp(ramp) => Arc::new(Item::new_from_element(Gradient::from(ramp))),
+					Self::GradientRamp(ramp) => Arc::new(Item::<Gradient>::from(ramp)),
 					Self::BrushStrokes(strokes) => Arc::new(core_types::list::Item::new_from_element(BrushTrace::from(strokes))),
 					// =======================
 					// AUTO-GENERATED VARIANTS
@@ -332,7 +332,7 @@ macro_rules! tagged_value {
 					x if x == TypeId::of::<BoxCorners>() => Ok(TaggedValue::BoxCorners(downcast::<BoxCorners>(input).unwrap().0.iter_element_values().copied().collect())),
 					x if x == TypeId::of::<Item<BoxCorners>>() => Ok(TaggedValue::BoxCorners(downcast::<Item<BoxCorners>>(input).unwrap().into_element().0.iter_element_values().copied().collect())),
 					x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(*downcast::<Gradient>(input).unwrap()))),
-					x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(downcast::<Item<Gradient>>(input).unwrap().into_element()))),
+					x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(&*downcast::<Item<Gradient>>(input).unwrap()))),
 					x if x == TypeId::of::<Vec<BrushStroke>>() => Ok(TaggedValue::BrushStrokes(*downcast(input).unwrap())),
 					x if x == TypeId::of::<Item<BrushTrace>>() => Ok(TaggedValue::BrushStrokes(downcast::<Item<BrushTrace>>(input).unwrap().into_element().0.iter_element_values().cloned().collect())),
 					// =======================
@@ -366,7 +366,7 @@ macro_rules! tagged_value {
 					x if x == TypeId::of::<BoxCorners>() => Ok(TaggedValue::BoxCorners(input.downcast_ref::<BoxCorners>().unwrap().0.iter_element_values().copied().collect())),
 					x if x == TypeId::of::<Item<BoxCorners>>() => Ok(TaggedValue::BoxCorners(input.downcast_ref::<Item<BoxCorners>>().unwrap().element().0.iter_element_values().copied().collect())),
 					x if x == TypeId::of::<Gradient>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(input.downcast_ref::<Gradient>().unwrap()))),
-					x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(input.downcast_ref::<Item<Gradient>>().unwrap().element()))),
+					x if x == TypeId::of::<Item<Gradient>>() => Ok(TaggedValue::GradientRamp(GradientRamp::from(input.downcast_ref::<Item<Gradient>>().unwrap()))),
 					x if x == TypeId::of::<Vec<BrushStroke>>() => Ok(TaggedValue::BrushStrokes(input.downcast_ref::<Vec<BrushStroke>>().unwrap().clone())),
 					x if x == TypeId::of::<Item<BrushTrace>>() => Ok(TaggedValue::BrushStrokes(input.downcast_ref::<Item<BrushTrace>>().unwrap().element().0.iter_element_values().cloned().collect())),
 					// =======================
@@ -1021,6 +1021,8 @@ mod paint_default_parsing {
 
 #[cfg(test)]
 mod gradient_shape_migration {
+	use graphic_types::vector_types::GradientSpreadMethod;
+
 	use super::*;
 
 	fn load(payload: serde_json::Value) -> TaggedValue {
@@ -1039,7 +1041,10 @@ mod gradient_shape_migration {
 	fn modern_ramp_payload_round_trips() {
 		let mut gradient = Gradient::from(vec![Color::BLACK, Color::WHITE]);
 		gradient.set_positions(&[0.2, 0.9]);
-		let value = TaggedValue::GradientRamp(GradientRamp::from(gradient));
+		let value = TaggedValue::GradientRamp(GradientRamp {
+			spread_method: GradientSpreadMethod::Reflect,
+			..GradientRamp::from(gradient)
+		});
 
 		let json = serde_json::to_value(&value).unwrap();
 		assert!(json.get("GradientRamp").and_then(|payload| payload.get("stops")).is_some(), "the payload should nest its stops: {json}");

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

@@ -1,6 +1,6 @@
 use core_types::Color;
 use core_types::color::SRGBA8;
-use core_types::list::{ATTR_MIDPOINT, ATTR_POSITION, Item, List};
+use core_types::list::{ATTR_MIDPOINT, ATTR_POSITION, ATTR_SPREAD_METHOD, Item, List};
 use core_types::render_complexity::RenderComplexity;
 use dyn_any::DynAny;
 use glam::{DAffine2, DVec2};
@@ -96,13 +96,15 @@ impl GradientStops<SRGBA8> {
 	}
 }
 
-/// 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.
+/// The serialized exchange form of a gradient: its stops, with whole-ramp settings as sibling fields serialized only when non-default.
 #[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>,
+	#[cfg_attr(feature = "serde", serde(default, skip_serializing_if = "GradientSpreadMethod::is_default"))]
+	#[cfg_attr(feature = "wasm", tsify(optional))]
+	pub spread_method: GradientSpreadMethod,
 }
 
 unsafe impl<C: dyn_any::StaticTypeSized> dyn_any::StaticType for GradientRamp<C> {
@@ -111,13 +113,19 @@ unsafe impl<C: dyn_any::StaticTypeSized> dyn_any::StaticType for GradientRamp<C>
 
 impl<C> From<GradientStops<C>> for GradientRamp<C> {
 	fn from(stops: GradientStops<C>) -> Self {
-		Self { stops }
+		Self {
+			stops,
+			spread_method: Default::default(),
+		}
 	}
 }
 
 impl From<&Gradient> for GradientRamp {
 	fn from(gradient: &Gradient) -> Self {
-		Self { stops: gradient.into() }
+		Self {
+			stops: gradient.into(),
+			spread_method: Default::default(),
+		}
 	}
 }
 
@@ -139,6 +147,27 @@ impl From<&GradientRamp> for Gradient {
 	}
 }
 
+// The runtime wire form: whole-ramp settings ride as the gradient item's attributes in its containing list,
+// where the Fill kernel, chain setter nodes, and renderers read and write them
+impl From<GradientRamp> for Item<Gradient> {
+	fn from(ramp: GradientRamp) -> Self {
+		let mut item = Item::new_from_element(Gradient::from(ramp.stops));
+		if !ramp.spread_method.is_default() {
+			item.set_attribute(ATTR_SPREAD_METHOD, ramp.spread_method);
+		}
+		item
+	}
+}
+
+impl From<&Item<Gradient>> for GradientRamp {
+	fn from(item: &Item<Gradient>) -> Self {
+		Self {
+			stops: item.element().into(),
+			spread_method: item.attribute_cloned_or_default(ATTR_SPREAD_METHOD),
+		}
+	}
+}
+
 impl From<&GradientRamp> for GradientStops<SRGBA8> {
 	fn from(ramp: &GradientRamp) -> Self {
 		Self {
@@ -158,19 +187,28 @@ impl From<&GradientStops<SRGBA8>> for GradientRamp {
 
 impl From<&GradientRamp> for GradientRamp<SRGBA8> {
 	fn from(ramp: &GradientRamp) -> Self {
-		Self { stops: ramp.into() }
+		Self {
+			stops: ramp.into(),
+			spread_method: ramp.spread_method,
+		}
 	}
 }
 
 impl From<&Gradient> for GradientRamp<SRGBA8> {
 	fn from(gradient: &Gradient) -> Self {
-		Self { stops: gradient.into() }
+		Self {
+			stops: gradient.into(),
+			spread_method: Default::default(),
+		}
 	}
 }
 
 impl From<&GradientRamp<SRGBA8>> for GradientRamp {
 	fn from(ramp: &GradientRamp<SRGBA8>) -> Self {
-		Self::from(&ramp.stops)
+		Self {
+			spread_method: ramp.spread_method,
+			..Self::from(&ramp.stops)
+		}
 	}
 }
 
@@ -747,9 +785,12 @@ impl Gradient {
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[widget(Radio)]
 pub enum GradientSpreadMethod {
+	/// Extends the end colors outward.
 	#[default]
 	Pad,
+	/// Loops the gradient by mirroring back-and-forth.
 	Reflect,
+	/// Loops the gradient as copies of itself.
 	Repeat,
 	// TODO: Add a "Clear" variant that returns transparent black outside the gradient's range
 }
@@ -762,6 +803,10 @@ impl GradientSpreadMethod {
 			GradientSpreadMethod::Repeat => "repeat",
 		}
 	}
+
+	pub fn is_default(&self) -> bool {
+		*self == Self::default()
+	}
 }
 
 /// Rebuild the y-axis so its (parallel, perpendicular) components in the x-axis-aligned frame stay constant, both
@@ -859,6 +904,44 @@ mod tests {
 		assert_eq!(serde_json::from_str::<GradientRamp>(&json).unwrap(), explicit);
 	}
 
+	#[test]
+	fn spread_method_serializes_only_when_not_default() {
+		let default_spread = GradientRamp::from(Gradient::from(vec![Color::BLACK, Color::WHITE]));
+		let json = serde_json::to_string(&default_spread).unwrap();
+		assert!(!json.contains("spread_method"), "the default Pad spread method must not serialize: {json}");
+		assert_eq!(serde_json::from_str::<GradientRamp>(&json).unwrap(), default_spread);
+
+		let repeating = GradientRamp {
+			spread_method: GradientSpreadMethod::Repeat,
+			..default_spread.clone()
+		};
+		let json = serde_json::to_string(&repeating).unwrap();
+		assert!(json.contains(r#""spread_method":"Repeat""#), "a non-default spread method must serialize: {json}");
+		assert_eq!(serde_json::from_str::<GradientRamp>(&json).unwrap(), repeating);
+	}
+
+	#[test]
+	fn spread_method_round_trips_through_the_item_attribute() {
+		let ramp = GradientRamp {
+			spread_method: GradientSpreadMethod::Repeat,
+			..GradientRamp::from(Gradient::from(vec![Color::BLACK, Color::WHITE]))
+		};
+
+		let item = Item::<Gradient>::from(ramp.clone());
+		assert_eq!(
+			item.attribute_cloned_or_default::<GradientSpreadMethod>(ATTR_SPREAD_METHOD),
+			GradientSpreadMethod::Repeat,
+			"the runtime item should carry the spread method as its attribute"
+		);
+		assert_eq!(GradientRamp::from(&item), ramp);
+
+		let padded = Item::<Gradient>::from(GradientRamp::from(Gradient::from(vec![Color::BLACK, Color::WHITE])));
+		assert!(
+			padded.attribute::<GradientSpreadMethod>(ATTR_SPREAD_METHOD).is_none(),
+			"the default Pad must stay absent rather than materialize"
+		);
+	}
+
 	#[test]
 	fn gradient_ui_write_back_elides_default_attributes() {
 		let mut gradient = Gradient::from(vec![Color::BLACK, Color::WHITE, Color::RED]);

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

@@ -8,8 +8,8 @@ use core_types::registry::types::{Angle, Length, Multiplier, Percentage, PixelLe
 use core_types::transform::{Footprint, Transform};
 use core_types::uuid::NodeId;
 use core_types::{
-	ATTR_BLEND_MODE, ATTR_CLIPPING_MASK, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_GRADIENT_TYPE, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, CloneVarArgs,
-	Color, Context, Ctx, ExtractAll, OwnedContextImpl,
+	ATTR_BLEND_MODE, ATTR_CLIPPING_MASK, ATTR_EDITOR_LAYER_PATH, ATTR_EDITOR_MERGED_LAYERS, ATTR_GRADIENT_TYPE, ATTR_OPACITY, ATTR_OPACITY_FILL, ATTR_TRANSFORM, CloneVarArgs, Color, Context, Ctx,
+	ExtractAll, OwnedContextImpl,
 };
 use glam::{DAffine2, DMat2, DVec2};
 use graphic_types::Vector;
@@ -21,6 +21,7 @@ use kurbo::{Affine, BezPath, DEFAULT_ACCURACY, Line, ParamCurve, ParamCurveArcle
 use rand::{Rng, SeedableRng};
 use std::collections::hash_map::DefaultHasher;
 use std::collections::{HashMap, HashSet};
+use vector_types::GradientType;
 use vector_types::gradient::{build_transform_with_y_preservation, initial_gradient_transform_for_bounding_box};
 use vector_types::subpath::{BezierHandles, ManipulatorGroup};
 use vector_types::vector::algorithms::bezpath_algorithms::{self, TValue, eval_pathseg_euclidean, evaluate_bezpath, split_bezpath, tangent_on_bezpath};
@@ -34,7 +35,6 @@ use vector_types::vector::misc::{
 use vector_types::vector::style::{DashPattern, Gradient, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin};
 use vector_types::vector::{FillId, PointId, RegionId, SegmentDomain, SegmentId, StrokeId, VectorExt};
 use vector_types::vector::{PointDomain, RegionDomain};
-use vector_types::{GradientSpreadMethod, GradientType};
 
 /// Implemented for `List` types that contain vector items reachable via mutable access.
 /// Used by the whole-collection Assign Colors node so it can apply to either `List<Graphic>` or `List<Vector>`.
@@ -192,14 +192,13 @@ async fn fill<V, F: IntoGraphicList + 'n + Send + 'static>(
 	_backup_color: Item<Color>,
 	#[default(Color::BLACK, Color::WHITE)] _backup_gradient: Item<Gradient>,
 	_gradient_type: Item<GradientType>,
-	_spread_method: Item<GradientSpreadMethod>,
 	_has_transform: Item<bool>,
 	_transform: Item<DAffine2>,
 ) -> Item<V>
 where
 	Item<V>: VectorItemMut + 'n + Send,
 {
-	let (_gradient_type, _spread_method) = (_gradient_type.into_element(), _spread_method.into_element());
+	let _gradient_type = _gradient_type.into_element();
 	let (_has_transform, _transform) = (_has_transform.into_element(), *_transform.element());
 
 	let mut content = content;
@@ -215,12 +214,6 @@ where
 			}
 		}
 
-		if gradient.iter_attribute_values::<GradientSpreadMethod>(ATTR_SPREAD_METHOD).is_none() {
-			for value in gradient.iter_attribute_values_mut_or_default::<GradientSpreadMethod>(ATTR_SPREAD_METHOD) {
-				*value = _spread_method;
-			}
-		}
-
 		if gradient.iter_attribute_values::<DAffine2>(ATTR_TRANSFORM).is_none() {
 			// Without an explicit placement, derive one covering the paint target's bounding box (the CSS `auto` behavior)
 			let transform = if _has_transform {