Skip to content

Pen tool offset when merging #4213

@0HyperCube

Description

@0HyperCube

Reproduction

  • New document
  • Select pen tool
  • Draw a line not at the origin
  • End drawing
  • Start new layer somewhere else
  • Place anchor on the end point of the original line
  • Observe offset

Video

merge_pen_tool_offset.mp4

Code

The pen tool appears to refer to a "buffer" system e.g. buffering_merged_vector. However buffering system is thankfully removed. I have literally no clue what is meant to be going on, however I fixed the issue with the following diff. It probably breaks other things though.

diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs
index 529404b12..89690ee68 100644
--- a/editor/src/messages/tool/tool_messages/pen_tool.rs
+++ b/editor/src/messages/tool/tool_messages/pen_tool.rs
@@ -401,8 +401,6 @@ struct PenToolData {
 	auto_panning: AutoPanning,
 	modifiers: ModifierState,
 
-	buffering_merged_vector: bool,
-
 	previous_handle_start_pos: DVec2,
 	previous_handle_end_pos: Option<DVec2>,
 	toggle_colinear_debounce: bool,
@@ -1851,26 +1849,30 @@ impl Fsm for PenToolFsmState {
 
 				self
 			}
+			(PenToolFsmState::PlacingAnchor, PenToolMessage::RecalculateLatestPointsPosition) => {
+				tool_data.recalculate_latest_points_position(document);
+
+				// If we were placing anchors then it would be a good idea to update the anchor if possible
+				if let Some(layer) = layer {
+					tool_data.handle_mode = HandleMode::ColinearLocked;
+					tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, shape_editor, responses);
+					tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses);
+					PenToolFsmState::DraggingHandle(tool_data.handle_mode)
+				} else {
+					PenToolFsmState::Ready
+				}
+			}
 			(state, PenToolMessage::RecalculateLatestPointsPosition) => {
 				tool_data.recalculate_latest_points_position(document);
 				state
 			}
-			(PenToolFsmState::PlacingAnchor, PenToolMessage::DragStart { append_to_selected }) => {
+			(PenToolFsmState::PlacingAnchor, PenToolMessage::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, viewport), &point, SnapTypeConfiguration::default());
 				let viewport_vec = document.metadata().document_to_viewport.transform_point2(snapped.snapped_point_document);
 
-				// Early return if the buffer was started and this message is being run again after the buffer (so that place_anchor updates the state with the newly merged vector)
-				if tool_data.buffering_merged_vector {
-					if let Some(layer) = layer {
-						tool_data.buffering_merged_vector = false;
-						tool_data.handle_mode = HandleMode::ColinearLocked;
-						tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, shape_editor, responses);
-						tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses);
-					}
-					tool_data.buffering_merged_vector = false;
-					PenToolFsmState::DraggingHandle(tool_data.handle_mode)
-				} else {
+				let mut is_merging = false;
+				{
 					if tool_data.handle_end.is_some() {
 						responses.add(DocumentMessage::StartTransaction);
 					}
@@ -1890,14 +1892,17 @@ impl Fsm for PenToolFsmState {
 							.or(tool_data.current_layer.filter(|layer| *layer != other_layer))
 						{
 							merge_layers(document, current_layer, other_layer, responses);
+							is_merging = true;
 						}
 					}
+				}
 
-					// Even if no buffer was started, the message still has to be run again in order to call bend_from_previous_point
-					tool_data.buffering_merged_vector = true;
-					responses.add(PenToolMessage::DragStart { append_to_selected });
-					PenToolFsmState::PlacingAnchor
+				// If not merging, then recalculate points and transition to the new mode as soon as possible (if merging then must be delayed until graph rerun)
+				if !is_merging {
+					responses.add(PenToolMessage::RecalculateLatestPointsPosition);
 				}
+
+				PenToolFsmState::PlacingAnchor
 			}
 			(PenToolFsmState::PlacingAnchor, PenToolMessage::RemovePreviousHandle) => {
 				if let Some(last_point) = tool_data.latest_points.last_mut() {

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions