The shape tools should set shape node parameters, not transformations of unit shapes

[Keavon]

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

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:

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:

Here, the rectangle is a unit square (1x1 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 1x1.

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

---

This also happens with the Ellipse tool:

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:

And in "star" mode:

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 (1x1) 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.

[Barnamoyy]

Hi, I would like to work on this issue

[Keavon]

@Barnamoyy great! I'll assign it to you once you've opened a PR. Cheers!

[Barnamoyy]

@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?

[Keavon]

@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.

[Barnamoyy]

@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!

[0HyperCube]

@Barnamoyy

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),
});

[November-6]

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

Desktop.2024.12.08.-.01.19.21.22.mp4

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

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 
}

[Keavon]

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?

[November-6]

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