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

@@ -167,7 +167,15 @@ macro_rules! tagged_value {
 					Self::DashPattern(lengths) => Box::new(DashPattern::from(lengths)),
 					Self::BoxCorners(values) => Box::new(BoxCorners::from(values)),
 					Self::Color(color) => Box::new(List::<Color>::new_from_element(color)),
-					Self::GradientRamp(ramp) => Box::new(List::<Gradient>::new_from_element(Gradient::from(ramp))),
+					Self::GradientRamp(ramp) => {
+						// The ramp's spread method rides the served list as its attribute, as `Item<Gradient>::from` does on master.
+						let spread_method = ramp.spread_method;
+						let mut list = List::<Gradient>::new_from_element(Gradient::from(ramp));
+						if !spread_method.is_default() {
+							list.set_attribute(graphic_types::vector_types::ATTR_SPREAD_METHOD, 0, spread_method);
+						}
+						Box::new(list)
+					}
 					Self::BrushStrokes(strokes) => {
 						let list: List<BrushStroke> = strokes.into_iter().map(core_types::list::Item::new_from_element).collect();
 						Box::new(list)
@@ -213,7 +221,15 @@ macro_rules! tagged_value {
 					Self::DashPattern(lengths) => Arc::new(DashPattern::from(lengths)),
 					Self::BoxCorners(values) => Arc::new(BoxCorners::from(values)),
 					Self::Color(color) => Arc::new(List::<Color>::new_from_element(color)),
-					Self::GradientRamp(ramp) => Arc::new(List::<Gradient>::new_from_element(Gradient::from(ramp))),
+					Self::GradientRamp(ramp) => {
+						// The ramp's spread method rides the served list as its attribute, as `Item<Gradient>::from` does on master.
+						let spread_method = ramp.spread_method;
+						let mut list = List::<Gradient>::new_from_element(Gradient::from(ramp));
+						if !spread_method.is_default() {
+							list.set_attribute(graphic_types::vector_types::ATTR_SPREAD_METHOD, 0, spread_method);
+						}
+						Arc::new(list)
+					}
 					Self::BrushStrokes(strokes) => {
 						let list: List<BrushStroke> = strokes.into_iter().map(core_types::list::Item::new_from_element).collect();
 						Arc::new(list)
@@ -1086,6 +1102,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 {
@@ -1104,7 +1122,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,3 +1,4 @@
+use crate::ATTR_SPREAD_METHOD;
 use core_types::Color;
 use core_types::color::SRGBA8;
 use core_types::list::{ATTR_MIDPOINT, ATTR_POSITION, Item, List};
@@ -96,13 +97,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 +114,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 +148,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 +188,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 +786,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 +804,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 +905,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

@@ -27,6 +27,8 @@ 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::ATTR_GRADIENT_TYPE;
+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};
@@ -40,8 +42,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::{ATTR_GRADIENT_TYPE, ATTR_SPREAD_METHOD};
-use vector_types::{GradientSpreadMethod, GradientType};
 
 /// The gradient color for one assign-colors position, replaying the
 /// randomized draws up to it.
@@ -276,9 +276,8 @@ fn park_paint<'e>(arena: &'e core_types::arena::Arena, paint: List<Graphic<'stat
 
 /// The gradient defaulting the legacy fill performed, applied to the nested
 /// stops list the paint table wraps.
-fn default_gradient_paint(paint: &mut List<Graphic>, bounds: Option<[DVec2; 2]>, gradient_type: GradientType, spread_method: GradientSpreadMethod, transform: Option<DAffine2>) {
+fn default_gradient_paint(paint: &mut List<Graphic>, bounds: Option<[DVec2; 2]>, gradient_type: GradientType, transform: Option<DAffine2>) {
 	let has_type = paint.iter_attribute_values::<GradientType>(ATTR_GRADIENT_TYPE).is_some();
-	let has_spread = paint.iter_attribute_values::<GradientSpreadMethod>(ATTR_SPREAD_METHOD).is_some();
 	let has_transform = paint.iter_attribute_values::<DAffine2>(ATTR_TRANSFORM).is_some();
 	for index in 0..paint.len() {
 		if !matches!(paint.element(index), Some(Graphic::Gradient(_))) {
@@ -287,9 +286,6 @@ fn default_gradient_paint(paint: &mut List<Graphic>, bounds: Option<[DVec2; 2]>,
 		if !has_type {
 			paint.set_attribute(ATTR_GRADIENT_TYPE, index, gradient_type);
 		}
-		if !has_spread {
-			paint.set_attribute(ATTR_SPREAD_METHOD, index, spread_method);
-		}
 		if !has_transform {
 			let transform = transform.unwrap_or_else(|| {
 				// Nudge a degenerate axis so the gradient transform stays invertible, matching the editor's `nonzero_bounding_box`
@@ -326,12 +322,11 @@ fn fill<'e>(
 	_backup_color: IList<Color>,
 	#[default(Color::BLACK, Color::WHITE)] _backup_gradient: IList<Gradient>,
 	_gradient_type: GradientType,
-	_spread_method: GradientSpreadMethod,
 	_has_transform: bool,
 	_transform: DAffine2,
 ) -> Result<(Vector, Attr<'e, Fill>), Interrupt> {
 	let mut paint = paint_table(fill);
-	default_gradient_paint(&mut paint, element.bounding_box(), _gradient_type, _spread_method, _has_transform.then_some(_transform));
+	default_gradient_paint(&mut paint, element.bounding_box(), _gradient_type, _has_transform.then_some(_transform));
 	let parked = park_paint(ctx.arena(), paint)?;
 	Ok((element, Attr(Some(parked))))
 }
@@ -347,7 +342,6 @@ fn fill_graphic_leveled<'e>(
 	_backup_color: IList<Color>,
 	#[default(Color::BLACK, Color::WHITE)] _backup_gradient: IList<Gradient>,
 	_gradient_type: GradientType,
-	_spread_method: GradientSpreadMethod,
 	_has_transform: bool,
 	_transform: DAffine2,
 ) -> Result<(Graphic<'static>, Attr<'e, Fill>), Interrupt> {
@@ -356,7 +350,7 @@ fn fill_graphic_leveled<'e>(
 		_ => None,
 	};
 	let mut paint = paint_table(fill);
-	default_gradient_paint(&mut paint, bounds, _gradient_type, _spread_method, _has_transform.then_some(_transform));
+	default_gradient_paint(&mut paint, bounds, _gradient_type, _has_transform.then_some(_transform));
 	let parked = park_paint(ctx.arena(), paint)?;
 	Ok((element, Attr(Some(parked))))
 }