Make the Gradient tool work with gradient node chains feeding a Fill node (#4177)

* Add Gradient node

* Add support of new Gradient node for Gradient tool

* Adapt gemini reviews

* Add tests

* Bring back visibility check

* Remove the 'Gradient' node since it's unwanted after all

* Formatting

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>

editor/src/messages/portfolio/document/graph_operation/utility_types.rs

@@ -1,8 +1,9 @@
 use super::transform_utils;
 use crate::messages::portfolio::document::node_graph::document_node_definitions::{DefinitionIdentifier, resolve_document_node_type, resolve_network_node_type, resolve_proto_node_type};
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
-use crate::messages::portfolio::document::utility_types::network_interface::{self, InputConnector, NodeNetworkInterface, OutputConnector};
+use crate::messages::portfolio::document::utility_types::network_interface::{self, FlowType, InputConnector, NodeNetworkInterface, OutputConnector};
 use crate::messages::prelude::*;
+use crate::messages::tool::common_functionality::graph_modification_utils::{get_fill_input_node_id, get_upstream_gradient_value_node_id, gradient_chain_target_input};
 use glam::{DAffine2, DVec2};
 use graph_craft::application_io::resource::ResourceId;
 use graph_craft::document::value::TaggedValue;
@@ -339,6 +340,40 @@ impl<'a> ModifyInputsContext<'a> {
 		self.existing_node_id(&DefinitionIdentifier::Network(reference.into()), create_if_nonexistent)
 	}
 
+	/// Like `existing_proto_node_id`, but walks/inserts at `target_input` instead of the layer's content input.
+	/// Used when a chain lives on a non-layer input.
+	pub fn existing_proto_node_id_at(&mut self, target_input: &InputConnector, reference: ProtoNodeIdentifier, create_if_nonexistent: bool) -> Option<NodeId> {
+		let identifier = DefinitionIdentifier::ProtoNode(reference.clone());
+
+		// Walk upstream from whatever is currently connected to target_input
+		let walk_start = self.network_interface.upstream_output_connector(target_input, &[]).and_then(|out| out.node_id());
+
+		let existing = walk_start.and_then(|start| {
+			self.network_interface
+				.upstream_flow_back_from_nodes(vec![start], &[], FlowType::HorizontalFlow)
+				.take_while(|id| !self.network_interface.is_layer(id, &[]))
+				.find(|id| self.network_interface.reference(id, &[]).as_ref() == Some(&identifier) && self.network_interface.is_visible(id, &[]))
+		});
+
+		if let Some(id) = existing {
+			return Some(id);
+		}
+		if !create_if_nonexistent {
+			return None;
+		}
+
+		// Splice new node between target_input and its current upstream
+		let node_definition = resolve_proto_node_type(reference)?;
+		let current_input = self.network_interface.input_from_connector(target_input, &[])?.clone();
+
+		let node_id = NodeId::new();
+		self.network_interface.insert_node(node_id, node_definition.default_node_template(), &[]);
+		self.network_interface.set_input(&InputConnector::node(node_id, 0), current_input, &[]);
+		self.network_interface.set_input(target_input, NodeInput::node(node_id, 0), &[]);
+
+		Some(node_id)
+	}
+
 	/// Gets the node id of a proto node with a specific reference that is upstream from the layer node, and optionally creates it if it does not exist.
 	pub fn existing_proto_node_id(&mut self, reference: ProtoNodeIdentifier, create_if_nonexistent: bool) -> Option<NodeId> {
 		self.existing_node_id(&DefinitionIdentifier::ProtoNode(reference), create_if_nonexistent)
@@ -494,12 +529,14 @@ impl<'a> ModifyInputsContext<'a> {
 		);
 	}
 
-	/// Set the GradientStops list on the 'Gradient Value' node, creating it if necessary.
+	/// Write the gradient stops to the 'Gradient Value' node feeding the layer.
 	pub fn gradient_stops_set(&mut self, stops: GradientStops) {
-		let Some(gradient_node_id) = self.existing_proto_node_id(graphene_std::math_nodes::gradient_value::IDENTIFIER, true) else {
+		let Some(output_layer) = self.get_output_layer() else { return };
+		let Some(gradient_value_id) = get_upstream_gradient_value_node_id(output_layer, self.network_interface) else {
 			return;
 		};
-		let input_connector = InputConnector::node(gradient_node_id, graphene_std::math_nodes::gradient_value::GradientInput::INDEX);
+
+		let input_connector = InputConnector::node(gradient_value_id, graphene_std::math_nodes::gradient_value::GradientInput::INDEX);
 		self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::Gradient(stops), false), false);
 	}
 
@@ -509,13 +546,21 @@ impl<'a> ModifyInputsContext<'a> {
 	pub fn gradient_line_set(&mut self, new_start: DVec2, new_end: DVec2) {
 		let Some(output_layer) = self.get_output_layer() else { return };
 
+		let walk_from = if let Some(fill_input_node_id) = get_fill_input_node_id(output_layer, self.network_interface) {
+			// Some nodes are connected to a Fill node, this means that the primary path is a `List<Vector>`, so we need to traverse it
+			fill_input_node_id
+		} else {
+			// No Fill node found, we will traverse the primary path to find transforms
+			output_layer.to_node()
+		};
+
 		let transform_reference = DefinitionIdentifier::ProtoNode(graphene_std::transform_nodes::transform::IDENTIFIER);
 		let upstream_transforms: Vec<NodeId> = self
 			.network_interface
-			.upstream_flow_back_from_nodes(vec![output_layer.to_node()], &[], network_interface::FlowType::HorizontalFlow)
+			.upstream_flow_back_from_nodes(vec![walk_from], &[], FlowType::HorizontalFlow)
-			.skip(1)
+			.skip_while(|node_id| self.network_interface.is_layer(node_id, &[]))
 			.take_while(|node_id| !self.network_interface.is_layer(node_id, &[]))
-			.filter(|node_id| self.network_interface.reference(node_id, &[]).as_ref() == Some(&transform_reference))
+			.filter(|id| self.network_interface.reference(id, &[]).as_ref() == Some(&transform_reference))
 			.collect();
 
 		// Upstream walk yields downstream-to-upstream order, so the first hit is the chain's last `Transform`
@@ -540,15 +585,10 @@ impl<'a> ModifyInputsContext<'a> {
 		// Rebuild the y-axis from the new x-axis using the old (parallel, perpendicular) decomposition and length ratio,
 		// so the gradient's aspect ratio and skew survive an endpoint drag (so an ellipse stays the same ellipse) instead of
 		// the old y-axis vector remaining fixed while x changes
-		let new_x_axis = new_end - new_start;
+		let new_composed = build_transform_with_y_preservation(composed_old, new_start, new_end);
-		let preserved_y_axis = scale_y_axis_to_match_new_x(composed_old.matrix2.x_axis, composed_old.matrix2.y_axis, new_x_axis);
-		let new_composed = DAffine2 {
-			matrix2: glam::DMat2::from_cols(new_x_axis, preserved_y_axis),
-			translation: new_start,
-		};
-
 		let last_transform_value = new_composed * prior_combined.inverse();
 
+		let target_input = gradient_chain_target_input(output_layer, self.network_interface);
 		let transform_node_id = if let Some(id) = last_transform_node_id {
 			id
 		} else {
@@ -556,7 +596,7 @@ impl<'a> ModifyInputsContext<'a> {
 			if last_transform_value.abs_diff_eq(DAffine2::IDENTITY, 1e-6) {
 				return;
 			}
-			let Some(id) = self.existing_proto_node_id(graphene_std::transform_nodes::transform::IDENTIFIER, true) else {
+			let Some(id) = self.existing_proto_node_id_at(&target_input, graphene_std::transform_nodes::transform::IDENTIFIER, true) else {
 				return;
 			};
 			id
@@ -570,9 +610,13 @@ impl<'a> ModifyInputsContext<'a> {
 	/// Write the gradient type to the last 'Gradient Type' node in the chain, inserting one only when the value differs
 	/// from the default (`Linear`).
 	pub fn gradient_type_set(&mut self, gradient_type: GradientType) {
+		let Some(output_layer) = self.get_output_layer() else { return };
+		let target_input = gradient_chain_target_input(output_layer, self.network_interface);
 		let identifier = graphene_std::math_nodes::gradient_type::IDENTIFIER;
 		let create_if_nonexistent = gradient_type != GradientType::default();
-		let Some(node_id) = self.existing_proto_node_id(identifier, create_if_nonexistent) else { return };
+		let Some(node_id) = self.existing_proto_node_id_at(&target_input, identifier, create_if_nonexistent) else {
+			return;
+		};
 
 		let input_connector = InputConnector::node(node_id, graphene_std::math_nodes::gradient_type::GradientTypeInput::INDEX);
 		self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::GradientType(gradient_type), false), false);
@@ -581,9 +625,13 @@ impl<'a> ModifyInputsContext<'a> {
 	/// Write the spread method to the last 'Spread Method' node in the chain, inserting one only when the value differs
 	/// from the default (`Pad`).
 	pub fn gradient_spread_method_set(&mut self, spread_method: GradientSpreadMethod) {
+		let Some(output_layer) = self.get_output_layer() else { return };
+		let target_input = gradient_chain_target_input(output_layer, self.network_interface);
 		let identifier = graphene_std::math_nodes::spread_method::IDENTIFIER;
 		let create_if_nonexistent = spread_method != GradientSpreadMethod::default();
-		let Some(node_id) = self.existing_proto_node_id(identifier, create_if_nonexistent) else { return };
+		let Some(node_id) = self.existing_proto_node_id_at(&target_input, identifier, create_if_nonexistent) else {
+			return;
+		};
 
 		let input_connector = InputConnector::node(node_id, graphene_std::math_nodes::spread_method::SpreadMethodInput::INDEX);
 		self.set_input_with_refresh(input_connector, NodeInput::value(TaggedValue::GradientSpreadMethod(spread_method), false), false);
@@ -778,3 +826,14 @@ fn scale_y_axis_to_match_new_x(old_x: DVec2, old_y: DVec2, new_x: DVec2) -> DVec
 
 	scale * (parallel * ex_new + perpendicular * ey_new)
 }
+
+/// Build a new affine that maps canonical (0,0) -> (1,0) to (new_start, new_end), preserving the y-axis
+/// shape of `old` proportionally to the x-axis length change.
+fn build_transform_with_y_preservation(old: DAffine2, new_start: DVec2, new_end: DVec2) -> DAffine2 {
+	let new_x_axis = new_end - new_start;
+	let preserved_y_axis = scale_y_axis_to_match_new_x(old.matrix2.x_axis, old.matrix2.y_axis, new_x_axis);
+	DAffine2 {
+		matrix2: glam::DMat2::from_cols(new_x_axis, preserved_y_axis),
+		translation: new_start,
+	}
+}

editor/src/messages/tool/common_functionality/graph_modification_utils.rs

@@ -271,6 +271,35 @@ pub fn get_viewport_center(layer: LayerNodeIdentifier, network_interface: &NodeN
 	network_interface.document_metadata().transform_to_viewport(layer).transform_point2(min + (max - min) * center)
 }
 
+/// Determine the input connector where the gradient chain enters the layer.
+/// Returns Fill's fill input if the layer has a "Fill" node, otherwise returns the layer's content input.
+pub fn gradient_chain_target_input(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> InputConnector {
+	if let Some(fill_node_id) = NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name(&DefinitionIdentifier::ProtoNode(graphene_std::vector::fill::IDENTIFIER)) {
+		InputConnector::node(fill_node_id, graphene_std::vector::fill::FillInput::<Fill>::INDEX)
+	} else {
+		InputConnector::node(layer.to_node(), 1)
+	}
+}
+
+/// Try to find a "Gradient Value" node that is connected to a "Fill" node, or to a layer directly.
+pub fn get_upstream_gradient_value_node_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	network_interface
+		.upstream_flow_back_from_nodes(vec![layer.to_node()], &[], FlowType::UpstreamFlow)
+		.skip(1)
+		.take_while(|node_id| !network_interface.is_layer(node_id, &[]))
+		.find(|node_id| network_interface.reference(node_id, &[]).as_ref() == Some(&DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::gradient_value::IDENTIFIER)))
+}
+
+/// Get the node connected to Fill's fill input, if any.
+pub fn get_fill_input_node_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	let fill_node_id = NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name(&DefinitionIdentifier::ProtoNode(graphene_std::vector::fill::IDENTIFIER))?;
+	let fill_node = network_interface.document_network().nodes.get(&fill_node_id)?;
+	let NodeInput::Node { node_id, .. } = fill_node.inputs.get(graphene_std::vector::fill::FillInput::<Fill>::INDEX)? else {
+		return None;
+	};
+	Some(*node_id)
+}
+
 /// Get the current gradient of a layer from the closest "Fill" node.
 pub fn get_gradient(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<Gradient> {
 	let fill_index = 1;
@@ -284,8 +313,8 @@ pub fn get_gradient(layer: LayerNodeIdentifier, network_interface: &NodeNetworkI
 
 /// Get the gradient stops of a layer, if any.
 pub fn get_gradient_stops(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<GradientStops> {
-	let inputs = NodeGraphLayer::new(layer, network_interface).find_node_inputs(&DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::gradient_value::IDENTIFIER))?;
+	let gradient_value_node = network_interface.document_network().nodes.get(&get_upstream_gradient_value_node_id(layer, network_interface)?)?;
-	let TaggedValue::Gradient(stops) = inputs.get(graphene_std::math_nodes::gradient_value::GradientInput::INDEX)?.as_value()? else {
+	let TaggedValue::Gradient(stops) = gradient_value_node.inputs.get(graphene_std::math_nodes::gradient_value::GradientInput::INDEX)?.as_value()? else {
 		return None;
 	};
 	Some(stops.clone())

editor/src/messages/tool/tool_messages/gradient_tool.rs

@@ -8,7 +8,7 @@ use crate::messages::portfolio::document::overlays::utility_types::{GizmoEmphasi
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
 use crate::messages::portfolio::document::utility_types::network_interface::{FlowType, NodeNetworkInterface};
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
-use crate::messages::tool::common_functionality::graph_modification_utils::{self, NodeGraphLayer, get_gradient_stops};
+use crate::messages::tool::common_functionality::graph_modification_utils::{self, NodeGraphLayer, get_gradient_stops, gradient_chain_target_input};
 use crate::messages::tool::common_functionality::snapping::{SnapCandidatePoint, SnapConstraint, SnapData, SnapManager, SnapTypeConfiguration};
 use graph_craft::document::value::TaggedValue;
 use graphene_std::color::SRGBA8;
@@ -347,20 +347,19 @@ fn gradient_space_transform(layer: LayerNodeIdentifier, document: &DocumentMessa
 // TODO: Remove this whole function once all gradients are stored via the modern `Gradient(GradientStops)` slot
 fn get_gradient(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<Gradient> {
 	if let Some(stops) = get_gradient_stops(layer, network_interface) {
-		let GradientChainState {
+		// Try to construct a gradient out of a chain, which is directly connected to a layer
-			transform,
+		let chain_state = read_gradient_chain_state(layer, network_interface);
-			gradient_type,
+		Some(Gradient {
-			spread_method,
-		} = read_gradient_chain_state(layer, network_interface);
-		return Some(Gradient {
 			stops,
-			gradient_type,
+			gradient_type: chain_state.gradient_type,
-			spread_method,
+			spread_method: chain_state.spread_method,
-			start: transform.transform_point2(DVec2::ZERO),
+			start: chain_state.transform.transform_point2(DVec2::ZERO),
-			end: transform.transform_point2(DVec2::X),
+			end: chain_state.transform.transform_point2(DVec2::X),
-		});
+		})
+	} else {
+		// Try to find a legacy Fill::Gradient that is selected in a Fill node
+		graph_modification_utils::get_gradient(layer, network_interface)
 	}
-	graph_modification_utils::get_gradient(layer, network_interface)
 }
 
 #[derive(Clone, Copy, Debug)]
@@ -373,6 +372,9 @@ struct GradientChainState {
 /// Resolve the gradient transform, type, and spread method by walking the chain feeding the layer. Transform composes all
 /// 'Transform' nodes. Type and spread method come from the closest-to-layer node of each kind, or the type default.
 fn read_gradient_chain_state(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> GradientChainState {
+	let target_input = gradient_chain_target_input(layer, network_interface);
+	let walk_from = network_interface.upstream_output_connector(&target_input, &[]).and_then(|out| out.node_id()).unwrap_or(layer.to_node());
+
 	let transform_reference = DefinitionIdentifier::ProtoNode(graphene_std::transform_nodes::transform::IDENTIFIER);
 	let gradient_type_reference = DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::gradient_type::IDENTIFIER);
 	let spread_method_reference = DefinitionIdentifier::ProtoNode(graphene_std::math_nodes::spread_method::IDENTIFIER);
@@ -382,8 +384,8 @@ fn read_gradient_chain_state(layer: LayerNodeIdentifier, network_interface: &Nod
 	let mut spread_method: Option<GradientSpreadMethod> = None;
 
 	for node_id in network_interface
-		.upstream_flow_back_from_nodes(vec![layer.to_node()], &[], FlowType::HorizontalFlow)
+		.upstream_flow_back_from_nodes(vec![walk_from], &[], FlowType::HorizontalFlow)
-		.skip(1)
+		.skip_while(|node_id| network_interface.is_layer(node_id, &[]))
 		.take_while(|node_id| !network_interface.is_layer(node_id, &[]))
 	{
 		let Some(reference) = network_interface.reference(&node_id, &[]) else { continue };
@@ -512,14 +514,13 @@ fn calculate_insertion(start: DVec2, end: DVec2, stops: &GradientStops, mouse: D
 impl SelectedGradient {
 	pub fn new(gradient: Gradient, layer: LayerNodeIdentifier, document: &DocumentMessageHandler) -> Self {
 		let transform = gradient_space_transform(layer, document);
-		let is_gradient_list = get_gradient_stops(layer, &document.network_interface).is_some();
 		Self {
 			layer: Some(layer),
 			transform,
 			gradient: gradient.clone(),
 			dragging: GradientDragTarget::End,
 			initial_gradient: gradient,
-			is_gradient_list,
+			is_gradient_list: get_gradient_stops(layer, &document.network_interface).is_some(),
 		}
 	}
 

node-graph/interpreted-executor/src/node_registry.rs

@@ -181,6 +181,7 @@ fn node_registry() -> HashMap<ProtoNodeIdentifier, HashMap<NodeIOTypes, NodeCons
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::vector::misc::ExtrudeJoiningAlgorithm]),
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::vector::misc::PointSpacingType]),
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::vector::style::GradientType]),
+		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::vector::style::GradientSpreadMethod]),
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::transform::ReferencePoint]),
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::vector::misc::CentroidType]),
 		async_node!(graphene_core::memo::MonitorNode<_, _, _>, input: Context, fn_params: [Context => graphene_std::text::TextAlign]),

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

@@ -1,13 +1,13 @@
 //! Contains stylistic options for SVG elements.
 
 pub use crate::gradient::*;
-use core_types::ATTR_OPACITY;
-use core_types::Color;
 use core_types::color::{Alpha, SRGBA8};
 use core_types::list::List;
 use core_types::transform::Transform;
+use core_types::{ATTR_GRADIENT_TYPE, ATTR_OPACITY, ATTR_SPREAD_METHOD, ATTR_TRANSFORM, Color};
 use dyn_any::DynAny;
 use glam::DAffine2;
+use glam::DVec2;
 use std::f64::consts::{PI, TAU};
 
 /// Describes the fill of a layer.
@@ -143,9 +143,16 @@ impl From<List<Color>> for Fill {
 
 impl From<List<GradientStops>> for Fill {
 	fn from(gradient: List<GradientStops>) -> Fill {
+		let gradient_type = gradient.attribute_cloned_or_default::<GradientType>(ATTR_GRADIENT_TYPE, 0);
+		let spread_method = gradient.attribute_cloned_or_default::<GradientSpreadMethod>(ATTR_SPREAD_METHOD, 0);
+		let transform = gradient.attribute_cloned_or_default::<DAffine2>(ATTR_TRANSFORM, 0);
+
 		Fill::Gradient(Gradient {
 			stops: gradient.element(0).cloned().unwrap_or_default(),
-			..Default::default()
+			gradient_type,
+			spread_method,
+			start: transform.transform_point2(DVec2::ZERO),
+			end: transform.transform_point2(DVec2::X),
 		})
 	}
 }
@@ -772,3 +779,39 @@ pub enum RenderMode {
 	/// Render a preview of how the object would be exported as an SVG.
 	SvgPreview,
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+
+	#[test]
+	fn fill_from_gradient_list_preserves_attributes() {
+		let mut list = List::new_from_element(GradientStops::default());
+		list.set_attribute(ATTR_GRADIENT_TYPE, 0, GradientType::Radial);
+		list.set_attribute(ATTR_SPREAD_METHOD, 0, GradientSpreadMethod::Reflect);
+		list.set_attribute(ATTR_TRANSFORM, 0, DAffine2::from_translation(DVec2::new(5., 7.)));
+
+		let Fill::Gradient(gradient) = Fill::from(list) else {
+			panic!("expected Fill::Gradient");
+		};
+
+		assert_eq!(gradient.gradient_type, GradientType::Radial);
+		assert_eq!(gradient.spread_method, GradientSpreadMethod::Reflect);
+		assert_eq!(gradient.start, DVec2::new(5., 7.));
+		assert_eq!(gradient.end, DVec2::new(6., 7.));
+	}
+
+	#[test]
+	fn fill_from_empty_gradient_list_uses_defaults() {
+		let list = List::new_from_element(GradientStops::default());
+
+		let Fill::Gradient(gradient) = Fill::from(list) else {
+			panic!("expected Fill::Gradient");
+		};
+
+		assert_eq!(gradient.gradient_type, GradientType::default());
+		assert_eq!(gradient.spread_method, GradientSpreadMethod::default());
+		assert_eq!(gradient.start, DVec2::ZERO);
+		assert_eq!(gradient.end, DVec2::X);
+	}
+}