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 :)
closed

Fix patch:
diff --git a/editor/src/messages/tool/common_functionality/resize.rs b/editor/src/messages/tool/common_functionality/resize.rs
index a593144aaf..bcf76b81d7 100644
--- a/editor/src/messages/tool/common_functionality/resize.rs
+++ b/editor/src/messages/tool/common_functionality/resize.rs
@@ -47,36 +47,38 @@ impl Resize {
 	/// Compute the drag start and end based on the current mouse position. Ignores the state of the layer.
 	/// If you want to only draw whilst a layer exists, use [`Resize::calculate_points`].
 	pub fn calculate_points_ignore_layer(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, center: Key, lock_ratio: Key) -> [DVec2; 2] {
-		let start = self.viewport_drag_start(document);
+		let start = self.drag_start;
 		let mouse = input.mouse.position;
 		let document_to_viewport = document.navigation_handler.calculate_offset_transform(input.viewport_bounds.center(), &document.document_ptz);
 		let document_mouse = document_to_viewport.inverse().transform_point2(mouse);
-		let mut points_viewport = [start, mouse];
+		let mut document_points = [start, document_mouse];
+
 		let ignore = if let Some(layer) = self.layer { vec![layer] } else { vec![] };
 		let ratio = input.keyboard.get(lock_ratio as usize);
 		let center = input.keyboard.get(center as usize);
+
 		let snap_data = SnapData::ignore(document, input, &ignore);
 		let config = SnapTypeConfiguration::default();
 		if ratio {
-			let size = points_viewport[1] - points_viewport[0];
+			let size = document_points[1] - document_points[0];
 			let size = size.abs().max(size.abs().yx()) * size.signum();
-			points_viewport[1] = points_viewport[0] + size;
-			let end_document = document_to_viewport.inverse().transform_point2(points_viewport[1]);
+			document_points[1] = document_points[0] + size;
+			let end = document_points[1];
 			let constraint = SnapConstraint::Line {
 				origin: self.drag_start,
-				direction: end_document - self.drag_start,
+				direction: end - self.drag_start,
 			};
 			if center {
-				let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end_document), constraint, config);
-				let far = SnapCandidatePoint::handle(2. * self.drag_start - end_document);
+				let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end), constraint, config);
+				let far = SnapCandidatePoint::handle(2. * self.drag_start - end);
 				let snapped_far = self.snap_manager.constrained_snap(&snap_data, &far, constraint, config);
 				let best = if snapped_far.other_snap_better(&snapped) { snapped } else { snapped_far };
-				points_viewport[0] = document_to_viewport.transform_point2(best.snapped_point_document);
-				points_viewport[1] = document_to_viewport.transform_point2(self.drag_start * 2. - best.snapped_point_document);
+				document_points[0] = best.snapped_point_document;
+				document_points[1] = self.drag_start * 2. - best.snapped_point_document;
 				self.snap_manager.update_indicator(best);
 			} else {
-				let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end_document), constraint, config);
-				points_viewport[1] = document_to_viewport.transform_point2(snapped.snapped_point_document);
+				let snapped = self.snap_manager.constrained_snap(&snap_data, &SnapCandidatePoint::handle(end), constraint, config);
+				document_points[1] = snapped.snapped_point_document;
 				self.snap_manager.update_indicator(snapped);
 			}
 		} else if center {
@@ -84,24 +86,28 @@ impl Resize {
 			let opposite = 2. * self.drag_start - document_mouse;
 			let snapped_far = self.snap_manager.free_snap(&snap_data, &SnapCandidatePoint::handle(opposite), config);
 			let best = if snapped_far.other_snap_better(&snapped) { snapped } else { snapped_far };
-			points_viewport[0] = document_to_viewport.transform_point2(best.snapped_point_document);
-			points_viewport[1] = document_to_viewport.transform_point2(self.drag_start * 2. - best.snapped_point_document);
+			document_points[0] = best.snapped_point_document;
+			document_points[1] = self.drag_start * 2. - best.snapped_point_document;
 			self.snap_manager.update_indicator(best);
 		} else {
 			let snapped = self.snap_manager.free_snap(&snap_data, &SnapCandidatePoint::handle(document_mouse), config);
-			points_viewport[1] = document_to_viewport.transform_point2(snapped.snapped_point_document);
+			document_points[1] = snapped.snapped_point_document;
 			self.snap_manager.update_indicator(snapped);
 		}
 
-		points_viewport
+		document_points
 	}
 
 	pub fn calculate_transform(&mut self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, center: Key, lock_ratio: Key, skip_rerender: bool) -> Option<Message> {
-		let points_viewport = self.calculate_points(document, input, center, lock_ratio)?;
+		let viewport_points = self.calculate_points(document, input, center, lock_ratio).map(|points| {
+			let document_to_viewport = document.metadata().document_to_viewport;
+			[document_to_viewport.transform_point2(points[0]), document_to_viewport.transform_point2(points[1])]
+		})?;
+
 		Some(
 			GraphOperationMessage::TransformSet {
 				layer: self.layer?,
-				transform: DAffine2::from_scale_angle_translation(points_viewport[1] - points_viewport[0], 0., points_viewport[0]),
+				transform: DAffine2::from_scale_angle_translation(viewport_points[1] - viewport_points[0], 0., viewport_points[0]),
 				transform_in: TransformIn::Viewport,
 				skip_rerender,
 			}
diff --git a/editor/src/messages/tool/tool_messages/ellipse_tool.rs b/editor/src/messages/tool/tool_messages/ellipse_tool.rs
index 858c177eb5..8151081bf3 100644
--- a/editor/src/messages/tool/tool_messages/ellipse_tool.rs
+++ b/editor/src/messages/tool/tool_messages/ellipse_tool.rs
@@ -236,11 +236,9 @@ impl Fsm for EllipseToolFsmState {
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
 							transform: DAffine2::from_translation((start + end) / 2.),
-							transform_in: TransformIn::Viewport,
+							transform_in: TransformIn::Local,
 							skip_rerender: 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 0097c22a18..3fa8a1055f 100644
--- a/editor/src/messages/tool/tool_messages/polygon_tool.rs
+++ b/editor/src/messages/tool/tool_messages/polygon_tool.rs
@@ -279,8 +279,7 @@ impl Fsm for PolygonToolFsmState {
 			(PolygonToolFsmState::Drawing, PolygonToolMessage::PointerMove { center, lock_ratio }) => {
 				if let Some([start, end]) = tool_data.data.calculate_points(document, input, center, lock_ratio) {
 					if let Some(layer) = tool_data.data.layer {
-						// TODO: make the scale impact the polygon/star node - we need to determine how to allow the polygon node to make irregular shapes
-
+						// TODO: 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();
@@ -326,11 +325,9 @@ impl Fsm for PolygonToolFsmState {
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
 							transform: DAffine2::from_scale_angle_translation(scale, 0., (start + end) / 2.),
-							transform_in: TransformIn::Viewport,
+							transform_in: TransformIn::Local,
 							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 fb34ed88eb..aa010dc2a3 100644
--- a/editor/src/messages/tool/tool_messages/rectangle_tool.rs
+++ b/editor/src/messages/tool/tool_messages/rectangle_tool.rs
@@ -224,7 +224,6 @@ impl Fsm for RectangleToolFsmState {
 			(RectangleToolFsmState::Drawing, RectangleToolMessage::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 {
-						// 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;
 						};
@@ -240,11 +239,9 @@ impl Fsm for RectangleToolFsmState {
 						responses.add(GraphOperationMessage::TransformSet {
 							layer,
 							transform: DAffine2::from_translation((start + end) / 2.),
-							transform_in: TransformIn::Viewport,
+							transform_in: TransformIn::Local,
 							skip_rerender: false,
 						});
-
-						responses.add(NodeGraphMessage::RunDocumentGraph);
 					}
 				}
 
diff --git a/editor/src/messages/tool/tool_messages/text_tool.rs b/editor/src/messages/tool/tool_messages/text_tool.rs
index 69b1e2e8d8..9adce65f16 100644
--- a/editor/src/messages/tool/tool_messages/text_tool.rs
+++ b/editor/src/messages/tool/tool_messages/text_tool.rs
@@ -504,7 +504,9 @@ impl Fsm for TextToolFsmState {
 				TextToolFsmState::Placing
 			}
 			(Self::Placing | TextToolFsmState::Dragging, TextToolMessage::PointerMove { center, lock_ratio }) => {
-				tool_data.cached_resize_bounds = tool_data.resize.calculate_points_ignore_layer(document, input, center, lock_ratio);
+				let document_points = tool_data.resize.calculate_points_ignore_layer(document, input, center, lock_ratio);
+				let document_to_viewport = document.metadata().document_to_viewport;
+				tool_data.cached_resize_bounds = [document_to_viewport.transform_point2(document_points[0]), document_to_viewport.transform_point2(document_points[1])];
 
 				responses.add(OverlaysMessage::Draw);
 


Test patch: