Repository: GraphiteEditor/Graphite (language: rust)

Issue:
The shape tools should set shape node parameters, not transformations of unit shapes
Should go hand-in-hand with #1715.

Presently, if I use the Line tool to draw a line:

![capture](https://github.com/user-attachments/assets/be759a8a-8319-456a-abc6-f4a65d2ab905)

It doesn't produce just a Line node with *Start* and *End* set to the appropriate coordinates. Instead, it uses the unit line `(0, 0)` to `(1, 0)` and applies some highly scaled transform with a Transform node to actually position it in space:

![capture](https://github.com/user-attachments/assets/e27af1d5-ee60-4885-8983-dab4f2f1cd74)

The desired result of drawing with the Line tool shouldn't include a Transform node.

`line_tool.rs` is a good place to start looking for this code (although it could potentially be in a helper function in another file).

---

This also happens with the Rectangle tool:

![capture](https://github.com/user-attachments/assets/51a15460-b14f-45c6-87cd-536443982d87)
![capture](https://github.com/user-attachments/assets/5bc78408-6429-4778-8514-4c0b608fdb20)

Here, the rectangle is a unit square (`1`x`1` units). The desired result of drawing with the Rectangle tool should apply the scale to the Rectangle node and the translation (and if the canvas is tilted, also rotation) to the Transform node, leaving it at the default scale factor of `1`x`1`.

`rectangle_tool.rs` is a good place to start looking for this code.

---

This also happens with the Ellipse tool:

![capture](https://github.com/user-attachments/assets/0188ccb9-ba06-49a4-803a-19ff52db39de)
![capture](https://github.com/user-attachments/assets/7b4b283f-d89b-471e-969b-ead33f7ebcc7)

The desired behavior is the same as the Rectangle tool.

`ellipse_tool.rs` is a good place to start looking for this code.

---

This also happens with the Polygon tool, in "convex" mode:

![capture](https://github.com/user-attachments/assets/e609d6ed-8940-4122-b2ea-0d8feb95cb88)
![capture](https://github.com/user-attachments/assets/2c8b1b2e-5dd5-4cf6-bc1d-917fbd85acec)

And in "star" mode:

![capture](https://github.com/user-attachments/assets/345209e5-f7cd-4952-b771-6172046d3a07)
![capture](https://github.com/user-attachments/assets/a699bee4-9ba9-4f44-b55b-b690930c882e)

In this case, like with Ellipse and Rectangle, we want to represent the scale with the shape generator nodes. However, because the Star and Regular Polygon nodes don't have parameters for non-uniform aspect ratios, the Transform node will still need to represent the stretch factor. We just don't want to use the unit (`1`x`1`) shapes anymore. It would probably be preferable to represent the smaller dimension as a scale factor of `1` in the Transform node so that the larger dimension can be represented as a value larger than `1` in the Transform node in order to give it the desired stretch.

`polygon_tool.rs` is a good place to start looking for this code.


Discussion / hints:
Hi, I would like to work on this issue 
@Barnamoyy great! I'll assign it to you once you've opened a PR. Cheers!
@Keavon could you simplify what kind of behaviour are we looking for with the rectangle tool and similar ones and also for the line tool do we want the start coordinates to be 0, 0 and the end to be at appropriate coordinates or both start and end has to be where the user was pointing their cursor at? 

@Barnamoyy I'm not understanding what you're asking about the rectangle option, but the original issue should specify the desired behavior. For the line option, there should be no Transform node at all, meaning both the start and end points fully determine the location.
<img width="1440" alt="image" src="https://github.com/user-attachments/assets/4925a086-0f25-4672-9df3-3e1906783dac">
@Keavon I have removed the transform node but can't figure out a way to change the end coordinates of the line could you help me with this, sorry I am not very experienced with open sourcing. Thanks! 
@Barnamoyy 
```rs
let Some(line_node_id) = layer.upstream_node_id_from_name("Line") else { return };

responses.add(NodeGraphMessage::SetInput {
	input_connector: InputConnector::node(line_node_id, 1),
	input: NodeInput::value(TaggedValue::DVec2(start), false),
});
responses.add(NodeGraphMessage::SetInput {
	input_connector: InputConnector::node(line_node_id, 2),
	input: NodeInput::value(TaggedValue::DVec2(end), false),
});
```
Hi! I noticed this issue has been inactive for over a month, and I’d like to work on it. I’ve already started exploring the problem and made some initial progress.

I've ran into a problem which is demonstrated in this video, the line preview is not showing now :')

https://github.com/user-attachments/assets/2e7011f9-b11e-4cb3-a6e6-22e8d2f6c0a1

It is because the PointerMove message is not being fired or read which is also leading to auto panning not working. Im having trouble figuring it out some help would be great! 

Below is my rough code 

```rust
impl Fsm for LineToolFsmState {
	type ToolData = LineToolData;
	type ToolOptions = LineOptions;

	fn transition(self, event: ToolMessage, tool_data: &mut Self::ToolData, tool_action_data: &mut ToolActionHandlerData, tool_options: &Self::ToolOptions, responses: &mut VecDeque<Message>) -> Self {
		let ToolActionHandlerData {
			document, global_tool_data, input, ..
		} = tool_action_data;

		let ToolMessage::Line(event) = event else {
			return self;
		};
		match (self, event) {
			(_, LineToolMessage::Overlays(mut overlay_context)) => {
				tool_data.snap_manager.draw_overlays(SnapData::new(document, input), &mut overlay_context);
				self
			}

			
			(LineToolFsmState::Ready, LineToolMessage::DragStart) => {
				let point = SnapCandidatePoint::handle(document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position));
				let snapped = tool_data.snap_manager.free_snap(&SnapData::new(document, input), &point, SnapTypeConfiguration::default());
				tool_data.drag_start = snapped.snapped_point_document;

				responses.add(DocumentMessage::StartTransaction);
				LineToolFsmState::Drawing
			}

			//NOTE: here the new layer is created when the mouse is released

			(LineToolFsmState::Drawing, LineToolMessage::DragStop) => {
                
				let point = SnapCandidatePoint::handle(document.
				metadata().document_to_viewport.inverse().transform_point2(input.mouse.position));
				let snapped = tool_data.snap_manager.free_snap(&SnapData::new(document, input), &point, SnapTypeConfiguration::default());
				tool_data.drag_current = snapped.snapped_point_document;

				responses.add(DocumentMessage::CommitTransaction);

                let start_point = tool_data.drag_start;
                let end_point = tool_data.drag_current;
              
                let node_type = resolve_document_node_type("Line").expect("Line node does not exist");
                let node = node_type.node_template_input_override([
                    None,
                    Some(NodeInput::value(TaggedValue::DVec2(start_point), false)),
                    Some(NodeInput::value(TaggedValue::DVec2(end_point), false)),
                ]);

                let nodes = vec![(NodeId(0), node)];
                let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, document.new_layer_parent(false), responses);

                tool_options.stroke.apply_stroke(tool_options.line_weight, layer, responses);

				tool_data.snap_manager.cleanup(responses);

                tool_data.drag_start = DVec2::ZERO;
                tool_data.drag_current = DVec2::ZERO;
                LineToolFsmState::Ready
            }

          //This part below being commented out is not responsible for pointerMove not working properly

			/* 
			(LineToolFsmState::Drawing, LineToolMessage::PointerMove { center, snap_angle, lock_angle }) => {
				tool_data.drag_current = input.mouse.position; // tool_data.snap_manager.snap_position(responses, document, input.mouse.position);

				let keyboard = &input.keyboard;
				let ignore = if let Some(layer) = tool_data.layer { vec![layer] } else { vec![] };
				let snap_data = SnapData::ignore(document, input, &ignore);
				responses.add(generate_transform(tool_data, snap_data, keyboard.key(lock_angle), keyboard.key(snap_angle), keyboard.key(center)));

				// Auto-panning
				let messages = [
					LineToolMessage::PointerOutsideViewport { center, snap_angle, lock_angle }.into(),
					LineToolMessage::PointerMove { center, snap_angle, lock_angle }.into(),
				];
				tool_data.auto_panning.setup_by_mouse_position(input, &messages, responses);

				LineToolFsmState::Drawing
			}
			*/
			
			(LineToolFsmState::Drawing, LineToolMessage::PointerOutsideViewport { .. }) => {
				// Auto-panning
				let _ = tool_data.auto_panning.shift_viewport(input, responses);

				LineToolFsmState::Drawing
			}

			(state, LineToolMessage::PointerOutsideViewport { center, lock_angle, snap_angle }) => {
				// Auto-panning
				let messages = [
					LineToolMessage::PointerOutsideViewport { center, lock_angle, snap_angle }.into(),
					LineToolMessage::PointerMove { center, lock_angle, snap_angle }.into(),
				];
				tool_data.auto_panning.stop(&messages, responses);

				state
			}
		
		//rest is same as original except ive disabled the transformation function 
}
``` 


Oh sorry @November-6, I missed this until now. It might be better if you open a draft PR for this and then ask for help in there since it'll be easier to track and we can look through all the code and test builds ourselves. Is this still the place where you're stuck or did you get past it on your own?
Hello @Keavon, sorry for late response I was busy with some stuff. I have opened a draft PR for this. While I tried to work on it more by myself, I still ended up being stuck so I rollbacked to my first fix. I've mentioned the details in PR, please check :)

Fix patch:
diff --git a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs
index dfb8694e93..18c6c0b48d 100644
--- a/editor/src/messages/tool/common_functionality/graph_modification_utils.rs
+++ b/editor/src/messages/tool/common_functionality/graph_modification_utils.rs
@@ -223,6 +223,26 @@ pub fn get_fill_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkIn
 	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Fill")
 }
 
+pub fn get_ellipse_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Ellipse")
+}
+
+pub fn get_line_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Line")
+}
+
+pub fn get_polygon_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Regular Polygon")
+}
+
+pub fn get_rectangle_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Rectangle")
+}
+
+pub fn get_star_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
+	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Star")
+}
+
 pub fn get_text_id(layer: LayerNodeIdentifier, network_interface: &NodeNetworkInterface) -> Option<NodeId> {
 	NodeGraphLayer::new(layer, network_interface).upstream_node_id_from_name("Text")
 }
diff --git a/editor/src/messages/tool/tool_messages/ellipse_tool.rs b/editor/src/messages/tool/tool_messages/ellipse_tool.rs
index b38fe917b4..ecbd2cbb5d 100644
--- a/editor/src/messages/tool/tool_messages/ellipse_tool.rs
+++ b/editor/src/messages/tool/tool_messages/ellipse_tool.rs
@@ -3,6 +3,7 @@ use crate::consts::DEFAULT_STROKE_WIDTH;
 use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn;
 use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type;
 use crate::messages::portfolio::document::overlays::utility_types::OverlayContext;
+use crate::messages::portfolio::document::utility_types::network_interface::InputConnector;
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
 use crate::messages::tool::common_functionality::color_selector::{ToolColorOptions, ToolColorType};
 use crate::messages::tool::common_functionality::graph_modification_utils;
@@ -220,12 +221,26 @@ impl Fsm for EllipseToolFsmState {
 			(EllipseToolFsmState::Drawing, EllipseToolMessage::PointerMove { center, lock_ratio }) => {
 				if let Some([start, end]) = shape_data.calculate_points(document, input, center, lock_ratio) {
 					if let Some(layer) = shape_data.layer {
+						let Some(node_id) = graph_modification_utils::get_ellipse_id(layer, &document.network_interface) else {
+							return self;
+						};
+
+						responses.add(NodeGraphMessage::SetInput {
+							input_connector: InputConnector::node(node_id, 1),
+							input: NodeInput::value(TaggedValue::F64(((start.x - end.x) / 2.).abs()), false),
+						});
+						responses.add(NodeGraphMessage::SetInput {
+							input_connector: InputConnector::node(node_id, 2),
+							input: NodeInput::value(TaggedValue::F64(((start.y - end.y) / 2.).abs()), false),
+						});
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
-							transform: DAffine2::from_scale_angle_translation((end - start).abs(), 0., (start + end) / 2.),
+							transform: DAffine2::from_translation((start + end) / 2.),
 							transform_in: TransformIn::Viewport,
 							skip_rerender: false,
 						});
+
+						responses.add(NodeGraphMessage::RunDocumentGraph);
 					}
 				}
 
diff --git a/editor/src/messages/tool/tool_messages/line_tool.rs b/editor/src/messages/tool/tool_messages/line_tool.rs
index e61eee38f7..e9aa0836c9 100644
--- a/editor/src/messages/tool/tool_messages/line_tool.rs
+++ b/editor/src/messages/tool/tool_messages/line_tool.rs
@@ -1,9 +1,8 @@
 use super::tool_prelude::*;
 use crate::consts::{DEFAULT_STROKE_WIDTH, LINE_ROTATE_SNAP_ANGLE};
-use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn;
 use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type;
 use crate::messages::portfolio::document::overlays::utility_types::OverlayContext;
-use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
+use crate::messages::portfolio::document::utility_types::{document_metadata::LayerNodeIdentifier, network_interface::InputConnector};
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
 use crate::messages::tool::common_functionality::color_selector::{ToolColorOptions, ToolColorType};
 use crate::messages::tool::common_functionality::graph_modification_utils;
@@ -179,23 +178,24 @@ impl Fsm for LineToolFsmState {
 				let node_type = resolve_document_node_type("Line").expect("Line node does not exist");
 				let node = node_type.node_template_input_override([
 					None,
-					Some(NodeInput::value(TaggedValue::DVec2(DVec2::ZERO), false)),
-					Some(NodeInput::value(TaggedValue::DVec2(DVec2::X), false)),
+					Some(NodeInput::value(
+						TaggedValue::DVec2(document.metadata().document_to_viewport.transform_point2(tool_data.drag_start)),
+						false,
+					)),
+					Some(NodeInput::value(
+						TaggedValue::DVec2(document.metadata().document_to_viewport.transform_point2(tool_data.drag_start)),
+						false,
+					)),
 				]);
 				let nodes = vec![(NodeId(0), node)];
 
 				let layer = graph_modification_utils::new_custom(NodeId::new(), nodes, document.new_layer_bounding_artboard(input), responses);
 				responses.add(Message::StartBuffer);
-				responses.add(GraphOperationMessage::TransformSet {
-					layer,
-					transform: DAffine2::from_scale_angle_translation(DVec2::ONE, 0., input.mouse.position),
-					transform_in: TransformIn::Viewport,
-					skip_rerender: false,
-				});
+
 				tool_options.stroke.apply_stroke(tool_options.line_weight, layer, responses);
-				tool_data.layer = Some(layer);
 
 				tool_data.layer = Some(layer);
+				tool_data.angle = 0.;
 				tool_data.weight = tool_options.line_weight;
 
 				LineToolFsmState::Drawing
@@ -206,7 +206,7 @@ impl Fsm for LineToolFsmState {
 				let keyboard = &input.keyboard;
 				let ignore = if let Some(layer) = tool_data.layer { vec![layer] } else { vec![] };
 				let snap_data = SnapData::ignore(document, input, &ignore);
-				responses.add(generate_transform(tool_data, snap_data, keyboard.key(lock_angle), keyboard.key(snap_angle), keyboard.key(center)));
+				generate_line(tool_data, snap_data, keyboard.key(lock_angle), keyboard.key(snap_angle), keyboard.key(center), responses);
 
 				// Auto-panning
 				let messages = [
@@ -287,7 +287,7 @@ impl Fsm for LineToolFsmState {
 	}
 }
 
-fn generate_transform(tool_data: &mut LineToolData, snap_data: SnapData, lock_angle: bool, snap_angle: bool, center: bool) -> Message {
+fn generate_line(tool_data: &mut LineToolData, snap_data: SnapData, lock_angle: bool, snap_angle: bool, center: bool, responses: &mut VecDeque<Message>) {
 	let document_to_viewport = snap_data.document.metadata().document_to_viewport;
 	let mut document_points = [tool_data.drag_start, document_to_viewport.inverse().transform_point2(tool_data.drag_current)];
 
@@ -347,17 +347,18 @@ fn generate_transform(tool_data: &mut LineToolData, snap_data: SnapData, lock_an
 		snap.update_indicator(snapped);
 	}
 
-	// Used for keeping the same angle next frame
-	tool_data.angle = -(document_points[1] - document_points[0]).angle_to(DVec2::X);
-
-	let viewport_points = [document_to_viewport.transform_point2(document_points[0]), document_to_viewport.transform_point2(document_points[1])];
-	let line_length = (viewport_points[1] - viewport_points[0]).length();
-	let angle = -(viewport_points[1] - viewport_points[0]).angle_to(DVec2::X);
-	GraphOperationMessage::TransformSet {
-		layer: tool_data.layer.unwrap(),
-		transform: glam::DAffine2::from_scale_angle_translation(DVec2::new(line_length, 1.), angle, viewport_points[0]),
-		transform_in: TransformIn::Viewport,
-		skip_rerender: false,
-	}
-	.into()
+	let Some(node_id) = graph_modification_utils::get_line_id(tool_data.layer.unwrap(), &snap_data.document.network_interface) else {
+		return;
+	};
+
+	responses.add(NodeGraphMessage::SetInput {
+		input_connector: InputConnector::node(node_id, 1),
+		input: NodeInput::value(TaggedValue::DVec2(document_points[0]), false),
+	});
+	responses.add(NodeGraphMessage::SetInput {
+		input_connector: InputConnector::node(node_id, 2),
+		input: NodeInput::value(TaggedValue::DVec2(document_points[1]), false),
+	});
+
+	responses.add(NodeGraphMessage::RunDocumentGraph);
 }
diff --git a/editor/src/messages/tool/tool_messages/polygon_tool.rs b/editor/src/messages/tool/tool_messages/polygon_tool.rs
index cb3ba2d614..1d27f38317 100644
--- a/editor/src/messages/tool/tool_messages/polygon_tool.rs
+++ b/editor/src/messages/tool/tool_messages/polygon_tool.rs
@@ -282,12 +282,55 @@ impl Fsm for PolygonToolFsmState {
 						// TODO: make the scale impact the polygon/star node - we need to determine how to allow the polygon node to make irregular shapes
 
 						update_radius_sign(end, start, layer, document, responses);
+
+						let dimensions = (start - end).abs();
+						let mut scale = DVec2::ONE;
+						let radius: f64;
+
+						// We keep the smaller dimension's scale at 1 and scale the other dimension accordingly
+						if dimensions.x > dimensions.y {
+							scale.x = dimensions.x / dimensions.y;
+							radius = dimensions.y / 2.;
+						} else {
+							scale.y = dimensions.y / dimensions.x;
+							radius = dimensions.x / 2.;
+						}
+
+						match tool_options.polygon_type {
+							PolygonType::Convex => {
+								let Some(node_id) = graph_modification_utils::get_polygon_id(layer, &document.network_interface) else {
+									return self;
+								};
+
+								responses.add(NodeGraphMessage::SetInput {
+									input_connector: InputConnector::node(node_id, 2),
+									input: NodeInput::value(TaggedValue::F64(radius), false),
+								});
+							}
+							PolygonType::Star => {
+								let Some(node_id) = graph_modification_utils::get_star_id(layer, &document.network_interface) else {
+									return self;
+								};
+
+								responses.add(NodeGraphMessage::SetInput {
+									input_connector: InputConnector::node(node_id, 2),
+									input: NodeInput::value(TaggedValue::F64(radius), false),
+								});
+								responses.add(NodeGraphMessage::SetInput {
+									input_connector: InputConnector::node(node_id, 3),
+									input: NodeInput::value(TaggedValue::F64(radius / 2.), false),
+								});
+							}
+						}
+
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
-							transform: DAffine2::from_scale_angle_translation((end - start).abs(), 0., (start + end) / 2.),
+							transform: DAffine2::from_scale_angle_translation(scale, 0., (start + end) / 2.),
 							transform_in: TransformIn::Viewport,
 							skip_rerender: false,
 						});
+
+						responses.add(NodeGraphMessage::RunDocumentGraph);
 					}
 				}
 
diff --git a/editor/src/messages/tool/tool_messages/rectangle_tool.rs b/editor/src/messages/tool/tool_messages/rectangle_tool.rs
index 89b2c41cdd..09b589024f 100644
--- a/editor/src/messages/tool/tool_messages/rectangle_tool.rs
+++ b/editor/src/messages/tool/tool_messages/rectangle_tool.rs
@@ -1,7 +1,7 @@
 use super::tool_prelude::*;
 use crate::consts::DEFAULT_STROKE_WIDTH;
 use crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type;
-use crate::messages::portfolio::document::{graph_operation::utility_types::TransformIn, overlays::utility_types::OverlayContext};
+use crate::messages::portfolio::document::{graph_operation::utility_types::TransformIn, overlays::utility_types::OverlayContext, utility_types::network_interface::InputConnector};
 use crate::messages::tool::common_functionality::auto_panning::AutoPanning;
 use crate::messages::tool::common_functionality::color_selector::{ToolColorOptions, ToolColorType};
 use crate::messages::tool::common_functionality::graph_modification_utils;
@@ -225,12 +225,26 @@ impl Fsm for RectangleToolFsmState {
 				if let Some([start, end]) = shape_data.calculate_points(document, input, center, lock_ratio) {
 					if let Some(layer) = shape_data.layer {
 						// TODO: make the scale impact the rect node
+						let Some(node_id) = graph_modification_utils::get_rectangle_id(layer, &document.network_interface) else {
+							return self;
+						};
+
+						responses.add(NodeGraphMessage::SetInput {
+							input_connector: InputConnector::node(node_id, 1),
+							input: NodeInput::value(TaggedValue::F64((start.x - end.x).abs()), false),
+						});
+						responses.add(NodeGraphMessage::SetInput {
+							input_connector: InputConnector::node(node_id, 2),
+							input: NodeInput::value(TaggedValue::F64((start.y - end.y).abs()), false),
+						});
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
-							transform: DAffine2::from_scale_angle_translation((end - start).abs(), 0., (start + end) / 2.),
+							transform: DAffine2::from_translation((start + end) / 2.),
 							transform_in: TransformIn::Viewport,
 							skip_rerender: false,
 						});
+
+						responses.add(NodeGraphMessage::RunDocumentGraph);
 					}
 				}
 


Test patch: