diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs index 2aadd7e8523..1c30beb7d0e 100644 --- a/editor/src/messages/tool/tool_messages/pen_tool.rs +++ b/editor/src/messages/tool/tool_messages/pen_tool.rs @@ -1374,7 +1374,9 @@ impl PenToolData { /// Perform extension of an existing path fn extend_existing_path(&mut self, document: &DocumentMessageHandler, layer: LayerNodeIdentifier, point: PointId, position: DVec2) { let vector: Option = document.network_interface.compute_modified_vector(layer); - let (handle_start, in_segment) = if let Some(vector) = &vector { + let (handle_start, in_segment) = if let Some(vector) = &vector + && vector.connected_count(point) <= 1 + { vector .segment_iter() .find_map(|(segment_id, bezier, start, end)| { @@ -2393,13 +2395,14 @@ impl Fsm for PenToolFsmState { #[cfg(test)] mod test_pen_tool { + use super::is_almost_colinear; use crate::messages::input_mapper::utility_types::pointer::EditorPointerState; use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::tool::common_functionality::snap_session::WAKE_DWELL_ON_APPROACH; use crate::test_utils::test_prelude::*; use glam::DAffine2; use graphene_std::vector::misc::ManipulatorPointId; - use graphene_std::vector::{SegmentId, VectorGeometry}; + use graphene_std::vector::{PointId, SegmentId, VectorGeometry}; /// The single Pen-drawn path layer and its vector, or `None` once the layer has been undone away. fn drawn_path(editor: &EditorTestUtils) -> Option<(LayerNodeIdentifier, VectorGeometry)> { @@ -2970,6 +2973,37 @@ mod test_pen_tool { assert!(leaving.distance(handle) < 1e-6, "the joining segment's handle is at {leaving}"); } + /// Finds the point ID whose viewport position is within tolerance of `target`. + fn get_anchor_near(vector: &VectorGeometry, layer_to_viewport: DAffine2, target: DVec2) -> PointId { + vector + .anchors() + .find(|&(_id, pos)| layer_to_viewport.transform_point2(pos).distance(target) < 1e-6) + .map(|(id, _pos)| id) + .unwrap_or_else(|| panic!("No anchor found near {target}")) + } + + /// Finds the segment that connects two given point IDs (in either direction). + fn get_segment_between(vector: &VectorGeometry, point_a: PointId, point_b: PointId) -> SegmentId { + vector + .segment_ids() + .into_iter() + .find(|&segment| { + let start = vector.segment_start_from_id(segment); + let end = vector.segment_end_from_id(segment); + (start == Some(point_a) && end == Some(point_b)) || (start == Some(point_b) && end == Some(point_a)) + }) + .unwrap_or_else(|| panic!("No segment found between the given anchors")) + } + + /// Finds the handle position on `segment` that corresponds to `point`. + fn get_handle_at(vector: &VectorGeometry, segment: SegmentId, point: PointId) -> Option { + if vector.segment_start_from_id(segment) == Some(point) { + vector.position_of(ManipulatorPointId::PrimaryHandle(segment)) + } else { + vector.position_of(ManipulatorPointId::EndHandle(segment)) + } + } + #[tokio::test] async fn an_anchor_moved_with_space_lands_on_a_corner_and_carries_its_handles_along() { let mut editor = EditorTestUtils::create(); @@ -3033,4 +3067,83 @@ mod test_pen_tool { let outgoing = editor.active_document().metadata().transform_to_viewport(layer).transform_point2(outgoing); assert!(outgoing.distance(handle + travel) < 1e-6, "the handle is at {outgoing} rather than {}", handle + travel); } + + #[tokio::test] + async fn extending_from_anchor_with_multiple_segments_does_not_create_colinear_handle() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + + // 1. Draw a path through A, B, and C with curved handles dragged out at B + click_pen(&mut editor, A).await; + let handle = B + DVec2::new(50., 0.); + editor.drag_path(&[B, handle], ModifierKeys::empty()).await; + click_pen(&mut editor, C).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + assert_path_shape(&editor, 3, 2); + + // 2. Extend from anchor B to D by clicking on B and then D (without dragging) + click_pen(&mut editor, B).await; + click_pen(&mut editor, D).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + assert_path_shape(&editor, 4, 3); + + // 3. Verify the new branch leaving B does not create a mirrored/colinear handle + let (layer, vector) = drawn_path(&editor).expect("Expected a drawn path"); + let layer_to_viewport = editor.active_document().metadata().transform_to_viewport(layer); + + let b_id = get_anchor_near(&vector, layer_to_viewport, B); + let d_id = get_anchor_near(&vector, layer_to_viewport, D); + let bd_segment = get_segment_between(&vector, b_id, d_id); + + if let Some(handle_pos) = get_handle_at(&vector, bd_segment, b_id) { + let handle_pos = layer_to_viewport.transform_point2(handle_pos); + assert!(handle_pos.distance(B) < 1e-4, "Handle at B for the new segment should remain at the anchor"); + } + } + + #[tokio::test] + async fn extending_from_anchor_with_single_segment_uses_colinear_handle() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + + // 1. Draw a single segment A→B with a curved handle dragged out at B + click_pen(&mut editor, A).await; + let handle = B + DVec2::new(50., 0.); + editor.drag_path(&[B, handle], ModifierKeys::empty()).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + assert_path_shape(&editor, 2, 1); + + // 2. Extend from B to C by clicking B then C (only one segment is attached to B) + click_pen(&mut editor, B).await; + click_pen(&mut editor, C).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + assert_path_shape(&editor, 3, 2); + + // 3. Verify the new segment leaving B carries a mirrored/colinear handle + let (layer, vector) = drawn_path(&editor).expect("Expected a drawn path"); + let layer_to_viewport = editor.active_document().metadata().transform_to_viewport(layer); + + let b_id = get_anchor_near(&vector, layer_to_viewport, B); + let c_id = get_anchor_near(&vector, layer_to_viewport, C); + let bc_segment = get_segment_between(&vector, b_id, c_id); + + let a_id = get_anchor_near(&vector, layer_to_viewport, A); + let ab_segment = get_segment_between(&vector, a_id, b_id); + let existing_handle_at_b = get_handle_at(&vector, ab_segment, b_id).expect("Segment AB should have a handle at B"); + let existing_handle_pos = layer_to_viewport.transform_point2(existing_handle_at_b); + + let new_handle_at_b = get_handle_at(&vector, bc_segment, b_id).expect("The new segment should have a handle at B"); + let new_handle_pos = layer_to_viewport.transform_point2(new_handle_at_b); + + // When only one segment is connected, the handle should not collapse to the + // anchor, and must be colinear with the existing handle at B. + assert!( + new_handle_pos.distance(B) > 1e-4, + "Handle at B should not collapse to the anchor when only one segment is connected (got {new_handle_pos})" + ); + assert!( + is_almost_colinear(B, existing_handle_pos, new_handle_pos), + "New handle should be colinear with the existing handle at B (existing: {existing_handle_pos}, new: {new_handle_pos})" + ); + } }