Make the Transform node use angles in degrees instead of radians for Rotation and Skew (#3160)

* Make the Transform node use degrees not radians

* Migration script

* Migrate skew value input to store degrees

* Add comments

* Fix migrations to account for the old deprecated "Pivot" parameter

* Fix tooling interactions with degrees-based transforms

* Upgrade demo art

---------

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

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

@@ -32,10 +32,13 @@ pub fn compute_scale_angle_translation_shear(transform: DAffine2) -> (DVec2, f64
 
 /// Update the inputs of the transform node to match a new transform
 pub fn update_transform(network_interface: &mut NodeNetworkInterface, node_id: &NodeId, transform: DAffine2) {
-	let (scale, angle, translation, shear) = compute_scale_angle_translation_shear(transform);
+	let (scale, rotation, translation, shear) = compute_scale_angle_translation_shear(transform);
+
+	let rotation = rotation.to_degrees();
+	let shear = DVec2::new(shear.x.atan().to_degrees(), shear.y.atan().to_degrees());
 
 	network_interface.set_input(&InputConnector::node(*node_id, 1), NodeInput::value(TaggedValue::DVec2(translation), false), &[]);
-	network_interface.set_input(&InputConnector::node(*node_id, 2), NodeInput::value(TaggedValue::F64(angle), false), &[]);
+	network_interface.set_input(&InputConnector::node(*node_id, 2), NodeInput::value(TaggedValue::F64(rotation), false), &[]);
 	network_interface.set_input(&InputConnector::node(*node_id, 3), NodeInput::value(TaggedValue::DVec2(scale), false), &[]);
 	network_interface.set_input(&InputConnector::node(*node_id, 4), NodeInput::value(TaggedValue::DVec2(shear), false), &[]);
 }
@@ -76,14 +79,14 @@ pub fn get_current_transform(inputs: &[NodeInput]) -> DAffine2 {
 	} else {
 		DVec2::ZERO
 	};
-
+	let rotation = if let Some(&TaggedValue::F64(rotation)) = inputs[2].as_value() { rotation } else { 0. };
-	let angle = if let Some(&TaggedValue::F64(angle)) = inputs[2].as_value() { angle } else { 0. };
-
 	let scale = if let Some(&TaggedValue::DVec2(scale)) = inputs[3].as_value() { scale } else { DVec2::ONE };
-
 	let shear = if let Some(&TaggedValue::DVec2(shear)) = inputs[4].as_value() { shear } else { DVec2::ZERO };
 
-	DAffine2::from_scale_angle_translation(scale, angle, translation) * DAffine2::from_cols_array(&[1., shear.y, shear.x, 1., 0., 0.])
+	let rotation = rotation.to_radians();
+	let shear = DVec2::new(shear.x.to_radians().tan(), shear.y.to_radians().tan());
+
+	DAffine2::from_scale_angle_translation(scale, rotation, translation) * DAffine2::from_cols_array(&[1., shear.y, shear.x, 1., 0., 0.])
 }
 
 /// Extract the current normalized pivot from the layer

editor/src/messages/portfolio/document/node_graph/document_node_definitions.rs

@@ -1351,31 +1351,52 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 			node_template: NodeTemplate {
 				document_node: DocumentNode {
 					inputs: vec![
+						// Value
 						NodeInput::value(TaggedValue::DAffine2(DAffine2::default()), true),
+						// Translation
 						NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false),
+						// Rotation
 						NodeInput::value(TaggedValue::F64(0.), false),
+						// Scale
 						NodeInput::value(TaggedValue::DVec2(DVec2::ONE), false),
+						// Skew
 						NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false),
+						// Origin Offset
+						NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false),
+						// Scale Appearance
+						NodeInput::value(TaggedValue::Bool(true), false),
 					],
 					implementation: DocumentNodeImplementation::Network(NodeNetwork {
-						exports: vec![NodeInput::node(NodeId(1), 0)],
+						exports: vec![
+							// From the Transform node
+							NodeInput::node(NodeId(1), 0),
+						],
 						nodes: [
+							// Monitor node
 							DocumentNode {
-								inputs: vec![NodeInput::network(generic!(T), 0)],
+								inputs: vec![
+									// From the Value import
+									NodeInput::network(generic!(T), 0),
+								],
 								implementation: DocumentNodeImplementation::ProtoNode(memo::monitor::IDENTIFIER),
 								call_argument: generic!(T),
 								skip_deduplication: true,
 								..Default::default()
 							},
+							// Transform node
 							DocumentNode {
 								inputs: vec![
+									// From the Monitor node
 									NodeInput::node(NodeId(0), 0),
+									// From the Translation import
 									NodeInput::network(concrete!(DVec2), 1),
+									// From the Rotation import
 									NodeInput::network(concrete!(f64), 2),
+									// From the Scale import
 									NodeInput::network(concrete!(DVec2), 3),
+									// From the Skew import
 									NodeInput::network(concrete!(DVec2), 4),
 								],
-								call_argument: concrete!(Context),
 								implementation: DocumentNodeImplementation::ProtoNode(transform_nodes::transform::IDENTIFIER),
 								..Default::default()
 							},
@@ -1441,6 +1462,8 @@ fn static_nodes() -> Vec<DocumentNodeDefinition> {
 							}),
 						),
 						InputMetadata::with_name_description_override("Skew", "TODO", WidgetOverride::Custom("transform_skew".to_string())),
+						InputMetadata::with_name_description_override("Origin Offset", "TODO", WidgetOverride::Custom("hidden".to_string())),
+						InputMetadata::with_name_description_override("Scale Appearance", "TODO", WidgetOverride::Custom("hidden".to_string())),
 					],
 					output_names: vec!["Data".to_string()],
 					..Default::default()
@@ -2190,13 +2213,13 @@ fn static_input_properties() -> InputProperties {
 			if let Some(&TaggedValue::F64(val)) = input.as_non_exposed_value() {
 				widgets.extend_from_slice(&[
 					Separator::new(SeparatorType::Unrelated).widget_holder(),
-					NumberInput::new(Some(val.to_degrees()))
+					NumberInput::new(Some(val))
 						.unit("°")
 						.mode(NumberInputMode::Range)
 						.range_min(Some(-180.))
 						.range_max(Some(180.))
 						.on_update(node_properties::update_value(
-							|number_input: &NumberInput| TaggedValue::F64(number_input.value.unwrap().to_radians()),
+							|number_input: &NumberInput| TaggedValue::F64(number_input.value.unwrap()),
 							node_id,
 							index,
 						))
@@ -2219,29 +2242,28 @@ fn static_input_properties() -> InputProperties {
 				return Err("Input not found in transform skew input override".to_string());
 			};
 			if let Some(&TaggedValue::DVec2(val)) = input.as_non_exposed_value() {
-				let to_skew = |input: &NumberInput| input.value.unwrap().to_radians().tan();
 				widgets.extend_from_slice(&[
 					Separator::new(SeparatorType::Unrelated).widget_holder(),
-					NumberInput::new(Some(val.x.atan().to_degrees()))
+					NumberInput::new(Some(val.x))
 						.label("X")
 						.unit("°")
 						.min(-89.9)
 						.max(89.9)
 						.on_update(node_properties::update_value(
-							move |input: &NumberInput| TaggedValue::DVec2(DVec2::new(to_skew(input), val.y)),
+							move |input: &NumberInput| TaggedValue::DVec2(DVec2::new(input.value.unwrap(), val.y)),
 							node_id,
 							index,
 						))
 						.on_commit(node_properties::commit_value)
 						.widget_holder(),
 					Separator::new(SeparatorType::Related).widget_holder(),
-					NumberInput::new(Some(val.y.atan().to_degrees()))
+					NumberInput::new(Some(val.y))
 						.label("Y")
 						.unit("°")
 						.min(-89.9)
 						.max(89.9)
 						.on_update(node_properties::update_value(
-							move |input: &NumberInput| TaggedValue::DVec2(DVec2::new(val.x, to_skew(input))),
+							move |input: &NumberInput| TaggedValue::DVec2(DVec2::new(val.x, input.value.unwrap())),
 							node_id,
 							index,
 						))

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

@@ -1232,6 +1232,17 @@ impl NodeNetworkInterface {
 		Some(&metadata.transient_metadata)
 	}
 
+	pub fn set_input_override(&mut self, node_id: &NodeId, index: usize, widget_override: Option<String>, network_path: &[NodeId]) {
+		let Some(metadata) = self
+			.node_metadata_mut(node_id, network_path)
+			.and_then(|node_metadata| node_metadata.persistent_metadata.input_metadata.get_mut(index))
+		else {
+			log::error!("Could not get input metadata for {node_id} index {index} in set_input_override");
+			return;
+		};
+		metadata.persistent_metadata.widget_override = widget_override;
+	}
+
 	/// Returns the input name to display in the properties panel. If the name is empty then the type is used.
 	pub fn displayed_input_name_and_description(&mut self, node_id: &NodeId, input_index: usize, network_path: &[NodeId]) -> (String, String) {
 		let Some(input_metadata) = self.persistent_input_metadata(node_id, input_index, network_path) else {

editor/src/messages/portfolio/document_migration.rs

@@ -5,7 +5,7 @@ use crate::messages::portfolio::document::node_graph::document_node_definitions:
 use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
 use crate::messages::portfolio::document::utility_types::network_interface::{InputConnector, NodeTemplate, OutputConnector};
 use crate::messages::prelude::DocumentMessageHandler;
-use glam::IVec2;
+use glam::{DVec2, IVec2};
 use graph_craft::document::DocumentNode;
 use graph_craft::document::{DocumentNodeImplementation, NodeInput, value::TaggedValue};
 use graphene_std::ProtoNodeIdentifier;
@@ -16,6 +16,7 @@ use graphene_std::uuid::NodeId;
 use graphene_std::vector::Vector;
 use graphene_std::vector::style::{PaintOrder, StrokeAlign};
 use std::collections::HashMap;
+use std::f64::consts::PI;
 
 const TEXT_REPLACEMENTS: &[(&str, &str)] = &[
 	("graphene_core::vector::vector_nodes::SamplePointsNode", "graphene_core::vector::SamplePolylineNode"),
@@ -1051,14 +1052,80 @@ fn migrate_node(node_id: &NodeId, node: &DocumentNode, network_path: &[NodeId],
 		document.network_interface.add_import(TaggedValue::U32(0), false, 1, "Loop Level", "TODO", &node_path);
 	}
 
-	// Add context features to nodes that don't have them (fine-grained context caching migration)
+	// Migrate the Transform node to use degrees instead of radians
-	if node.context_features == graphene_std::ContextDependencies::default() {
+	if reference == "Transform" && node.inputs.get(6).is_none() {
-		if let Some(reference) = document.network_interface.reference(node_id, network_path).cloned().flatten() {
+		// Migrate rotation from radians to degrees
-			if let Some(node_definition) = resolve_document_node_type(&reference) {
+		match node.inputs.get(2)? {
-				let context_features = node_definition.node_template.document_node.context_features;
+			NodeInput::Value { tagged_value, exposed } => {
-				document.network_interface.set_context_features(node_id, network_path, context_features);
+				// Read the existing Properties panel number value, which used to be in radians
+				let TaggedValue::F64(radians) = *tagged_value.clone().into_inner() else { return None };
+
+				// Convert the radians to degrees and set it back as the new input value
+				let degrees = NodeInput::value(TaggedValue::F64(radians.to_degrees()), *exposed);
+				document.network_interface.set_input(&InputConnector::node(*node_id, 2), degrees, network_path);
 			}
+			NodeInput::Node { .. } => {
+				// Construct a new Multiply node for converting from degrees to radians
+				let Some(multiply_node) = resolve_document_node_type("Multiply") else {
+					log::error!("Could not get multiply node from definition when upgrading transform");
+					return None;
+				};
+				let mut multiply_template = multiply_node.default_node_template();
+				multiply_template.document_node.inputs[1] = NodeInput::value(TaggedValue::F64(180. / PI), false);
+
+				// Decide on the placement position of the new Multiply node
+				let multiply_node_id = NodeId::new();
+				let Some(transform_position) = document.network_interface.position_from_downstream_node(node_id, network_path) else {
+					log::error!("Could not get positon for transform node {node_id}");
+					return None;
+				};
+				let multiply_position = transform_position + IVec2::new(-7, 1);
+
+				// Insert the new Multiply node into the network directly before it's used
+				document.network_interface.insert_node(multiply_node_id, multiply_template, network_path);
+				document.network_interface.shift_absolute_node_position(&multiply_node_id, multiply_position, network_path);
+				document.network_interface.insert_node_between(&multiply_node_id, &InputConnector::node(*node_id, 2), 0, network_path);
+			}
+			_ => {}
+		};
+
+		// Migrate skew from radians to degrees
+		if let NodeInput::Value { tagged_value, exposed } = node.inputs.get(4)? {
+			// Read the existing Properties panel number value, which used to be in radians
+			let TaggedValue::DVec2(old_value) = *tagged_value.clone().into_inner() else { return None };
+
+			// The previous value stored the tangent of the displayed degrees. Now it stores the degrees, so take the arctan of it and convert to degrees.
+			let new_value = DVec2::new(old_value.x.atan().to_degrees(), old_value.y.atan().to_degrees());
+			let new_input = NodeInput::value(TaggedValue::DVec2(new_value), *exposed);
+			document.network_interface.set_input(&InputConnector::node(*node_id, 4), new_input, network_path);
 		}
+
+		// Remove the possible existence of the old "Pivot" hidden value input that was removed in #2730
+		let nested_transform_network = [network_path, &[*node_id]].concat();
+		if node.inputs.get(5).is_some() {
+			document.network_interface.remove_import(5, &nested_transform_network);
+		}
+
+		// Add the Origin Offset parameter as a hidden input, which will be given actual functionality in the future but is currently used as a marker to detect not-yet-upgraded Transform nodes
+		document
+			.network_interface
+			.add_import(TaggedValue::DVec2(DVec2::ZERO), false, 5, "Origin Offset", "", &nested_transform_network);
+		document.network_interface.set_input_override(node_id, 5, Some("hidden".to_string()), network_path); // Hide it while we're not yet using it
+
+		// Add the Scale Appearance parameter as a hidden input, which will be given actual functionality in the future but is currently used as a marker to detect not-yet-upgraded Transform nodes
+		document
+			.network_interface
+			.add_import(TaggedValue::Bool(true), false, 6, "Scale Appearance", "", &nested_transform_network);
+		document.network_interface.set_input_override(node_id, 6, Some("hidden".to_string()), network_path); // Hide it while we're not yet using it
+	}
+
+	// Add context features to nodes that don't have them (fine-grained context caching migration)
+	if node.context_features == graphene_std::ContextDependencies::default()
+		&& let Some(reference) = document.network_interface.reference(node_id, network_path).cloned().flatten()
+		&& let Some(node_definition) = resolve_document_node_type(&reference)
+	{
+		let context_features = node_definition.node_template.document_node.context_features;
+		document.network_interface.set_context_features(node_id, network_path, context_features);
 	}
 
 	// ==================================

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

@@ -246,11 +246,11 @@ pub fn new_custom(id: NodeId, nodes: Vec<(NodeId, NodeTemplate)>, parent: LayerN
 	LayerNodeIdentifier::new_unchecked(id)
 }
 
-/// Locate the origin of the transform node
+/// Locate the origin of the "Transform" node.
 pub fn get_origin(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<DVec2> {
-	use graphene_std::transform_nodes::transform::TranslateInput;
+	use graphene_std::transform_nodes::transform::*;
 
-	if let TaggedValue::DVec2(origin) = NodeGraphLayer::new(layer, network_interface).find_input("Transform", TranslateInput::INDEX)? {
+	if let TaggedValue::DVec2(origin) = NodeGraphLayer::new(layer, network_interface).find_input("Transform", TranslationInput::INDEX)? {
 		Some(*origin)
 	} else {
 		None

node-graph/gcore/src/transform_nodes.rs

@@ -22,12 +22,14 @@ async fn transform<T: ApplyTransform + 'n + 'static>(
 		Context -> Table<GradientStops>,
 	)]
 	value: impl Node<Context<'static>, Output = T>,
-	translate: DVec2,
+	translation: DVec2,
-	rotate: f64,
+	rotation: f64,
 	scale: DVec2,
 	skew: DVec2,
 ) -> T {
-	let matrix = DAffine2::from_scale_angle_translation(scale, rotate, translate) * DAffine2::from_cols_array(&[1., skew.y, skew.x, 1., 0., 0.]);
+	let trs = DAffine2::from_scale_angle_translation(scale, rotation.to_radians(), translation);
+	let skew = DAffine2::from_cols_array(&[1., skew.y.to_radians().tan(), skew.x.to_radians().tan(), 1., 0., 0.]);
+	let matrix = trs * skew;
 
 	let footprint = ctx.try_footprint().copied();