From 50ae525546c987e6da0908d168179cadbaad8f1e Mon Sep 17 00:00:00 2001 From: devviktoria <74530862+devviktoria@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:55:03 +0200 Subject: [PATCH] Fix snapping edge cases --- .../messages/tool/tool_messages/path_tool.rs | 215 +++++++++++++++++- 1 file changed, 213 insertions(+), 2 deletions(-) diff --git a/editor/src/messages/tool/tool_messages/path_tool.rs b/editor/src/messages/tool/tool_messages/path_tool.rs index bda6e6e43dc..7bf8820375c 100644 --- a/editor/src/messages/tool/tool_messages/path_tool.rs +++ b/editor/src/messages/tool/tool_messages/path_tool.rs @@ -21,7 +21,7 @@ use crate::messages::tool::common_functionality::pivot::{PivotGizmo, PivotGizmoT use crate::messages::tool::common_functionality::shape_editor::{ ClosestSegment, ManipulatorAngle, OpposingHandleLengths, SelectedLayerState, SelectedPointsInfo, SelectionChange, SelectionShape, SelectionShapeType, ShapeState, }; -use crate::messages::tool::common_functionality::snapping::{SnapCache, SnapCandidatePoint, SnapConstraint, SnapData, SnapManager}; +use crate::messages::tool::common_functionality::snapping::{SnapCache, SnapCandidatePoint, SnapConstraint, SnapData, SnapManager, SnappedPoint}; use crate::messages::tool::common_functionality::utility_functions::{calculate_segment_angle, find_two_param_best_approximate, make_path_editable_is_allowed}; use graphene_std::Color; use graphene_std::renderer::Quad; @@ -1066,6 +1066,10 @@ impl PathToolData { snap_angle: bool, tangent_to_neighboring_tangents: bool, ) -> f64 { + if handle_vector.length_squared() < f64::EPSILON { + return self.angle; + } + let current_angle = -handle_vector.angle_to(DVec2::X); if let Some((vector, layer)) = shape_editor @@ -1142,7 +1146,7 @@ impl PathToolData { let snap_data = SnapData::new(document, input, viewport); let snap_point = SnapCandidatePoint::handle_neighbors(new_handle_position, [anchor_position]); - let snap_result = match using_angle_constraints { + let mut snap_result = match using_angle_constraints { true => { let snap_constraint = SnapConstraint::Line { origin: anchor_position, @@ -1154,6 +1158,14 @@ impl PathToolData { false => self.snap_manager.free_snap(&snap_data, &snap_point, Default::default()), }; + //If the snapping result is a non-finite position + if snap_result.distance == f64::INFINITY { + snap_result = SnappedPoint { + snapped_point_document: new_handle_position, + ..Default::default() + }; + } + self.snap_manager.update_indicator(snap_result.clone()); document.metadata().document_to_viewport.transform_vector2(snap_result.snapped_point_document - handle_position) @@ -3661,3 +3673,202 @@ fn update_dynamic_hints( hint_data.send_layout(responses); responses.add(ToolMessage::UpdateHints); } + +#[cfg(test)] +mod test_path { + use crate::messages::input_mapper::utility_types::keyboard::ModifierKeys; + use crate::messages::input_mapper::utility_types::pointer::MouseKeys; + use crate::messages::portfolio::document::utility_types::misc::{SNAP_FUNCTIONS_FOR_BOUNDING_BOXES, SNAP_FUNCTIONS_FOR_PATHS}; + pub use crate::test_utils::test_prelude::*; + use glam::DAffine2; + use graphene_std::vector::Vector; + use graphene_std::vector::misc::{BezierHandles, segment_to_handles}; + + async fn prepare_document_for_path_snap_weight(anchor_positions: &[DVec2]) -> EditorTestUtils { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + editor.set_viewport_size(DVec2::splat(-1000.), DVec2::splat(1000.)).await; // Necessary for doing snapping since snaps outside of the viewport are discarded + + editor.drag_tool(ToolType::Artboard, 0., 0., 1000., 600., ModifierKeys::empty()).await; // Necessary for doing path snapping without it that path snapping does not work + editor.select_tool(ToolType::Select).await; + + // Disable all bounding box snapping + for (_, closure, _) in SNAP_FUNCTIONS_FOR_BOUNDING_BOXES { + editor + .handle_message(DocumentMessage::SetSnapping { + closure: Some(closure), + snapping_state: false, + }) + .await; + } + + // Disable all path snapping EXCEPT along_path + for (name, closure, _) in SNAP_FUNCTIONS_FOR_PATHS { + let enabled = name == "Along Paths"; + + editor + .handle_message(DocumentMessage::SetSnapping { + closure: Some(closure), + snapping_state: enabled, + }) + .await; + } + + //Create Bezier path for testing + editor.drag_tool(ToolType::Pen, anchor_positions[0].x, anchor_positions[0].y, 650., 30., ModifierKeys::empty()).await; + editor.drag_tool(ToolType::Pen, anchor_positions[1].x, anchor_positions[1].y, 370., 420., ModifierKeys::empty()).await; + editor.drag_tool(ToolType::Pen, anchor_positions[2].x, anchor_positions[2].y, 20., 330., ModifierKeys::empty()).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + + assert_eq!( + editor.active_document().metadata().all_layers().count(), + 2, + "The document should contain one artboard and one path layers" + ); + + let (modified_path, layer_to_viewport) = get_path_data(&editor); + assert_anchor_positions(&modified_path, layer_to_viewport, anchor_positions, 1e-10); + + let expected_handles: Vec = vec![ + BezierHandles::Cubic { + handle_start: DVec2::new(700.0, 60.0), + handle_end: DVec2::new(430.0, 180.0), + }, + BezierHandles::Cubic { + handle_start: DVec2::new(370.0, 420.0), + handle_end: DVec2::new(80.0, 70.0), + }, + ]; + assert_handle_positions(&modified_path, &expected_handles, layer_to_viewport, 1e-10); + + editor + } + + fn get_path_data(editor: &EditorTestUtils) -> (Vector, DAffine2) { + let document = editor.active_document(); + + let path_layer = document.metadata().all_layers().nth(1).expect("Expected path layer"); + + let modified_path = document.network_interface.compute_modified_vector(path_layer).expect("Vector not found in the path layer"); + + let layer_to_document = document.metadata().transform_to_document(path_layer); + + (modified_path, layer_to_document) + } + + fn assert_anchor_positions(vector: &Vector, transform: DAffine2, expected_anchors: &[DVec2], epsilon: f64) { + let anchors_in_viewport: Vec = vector + .point_domain + .ids() + .iter() + .filter_map(|&point_id| { + let pos = vector.point_domain.position_from_id(point_id)?; + Some(transform.transform_point2(pos)) + }) + .collect(); + + assert_eq!(anchors_in_viewport.len(), expected_anchors.len(), "Anchor count mismatch"); + + for (i, expected) in expected_anchors.iter().enumerate() { + let actual = anchors_in_viewport[i]; + let distance = (actual - *expected).length(); + + assert!(distance < epsilon, "Anchor {i} mismatch: expected {expected:?}, got {actual:?}, distance {distance}"); + } + } + + fn assert_handle_positions(vector: &Vector, expected_handles: &[BezierHandles], transform: DAffine2, epsilon: f64) { + let segment_ids = vector.segment_domain.ids(); + + assert_eq!(segment_ids.len(), expected_handles.len(), "Segment count mismatch for handles"); + + for (i, segment_id) in segment_ids.iter().enumerate() { + let segment = vector.segment_from_id(*segment_id).expect("Segment not found"); + let segment_handles = segment_to_handles(&segment); + let expected = &expected_handles[i]; + + match (&segment_handles, expected) { + (BezierHandles::Linear, BezierHandles::Linear) => { + // OK + } + + (BezierHandles::Quadratic { handle: actual }, BezierHandles::Quadratic { handle: expected }) => { + let actual_viewport = transform.transform_point2(*actual); + + let dist = (actual_viewport - expected).length(); + + assert!( + dist < epsilon, + "Segment {i} quadratic handle mismatch: expected {:?}, got {:?}, dist {}", + expected, + actual_viewport, + dist + ); + } + + ( + BezierHandles::Cubic { + handle_start: actual_start, + handle_end: actual_end, + }, + BezierHandles::Cubic { + handle_start: expected_start, + handle_end: expected_end, + }, + ) => { + let actual_start_viewport = transform.transform_point2(*actual_start); + let actual_end_viewport = transform.transform_point2(*actual_end); + + let dist_start = (actual_start_viewport - expected_start).length(); + let dist_end = (actual_end_viewport - expected_end).length(); + + assert!( + dist_start < epsilon, + "Segment {i} cubic start handle mismatch: expected {:?}, got {:?}, dist {}", + expected_start, + actual_start_viewport, + dist_start + ); + + assert!( + dist_end < epsilon, + "Segment {i} cubic end handle mismatch: expected {:?}, got {:?}, dist {}", + expected_end, + actual_end_viewport, + dist_end + ); + } + // Mismatch case + (actual, expected) => { + panic!("Segment {i} handle type mismatch: actual = {:?}, expected = {:?}", actual, expected); + } + } + } + } + + #[tokio::test] + async fn path_move_handle_to_anchor() { + let anchor_positions = [DVec2::new(50., 30.), DVec2::new(400., 300.), DVec2::new(50., 200.)]; + let mut editor = prepare_document_for_path_snap_weight(&anchor_positions).await; + + editor.click_tool(ToolType::Path, MouseKeys::LEFT, anchor_positions[1], ModifierKeys::empty()).await; + editor.drag_tool(ToolType::Path, 370., 420., anchor_positions[1].x, anchor_positions[1].y, ModifierKeys::empty()).await; + editor.press(Key::Enter, ModifierKeys::empty()).await; + + let (modified_path, layer_to_document) = get_path_data(&editor); + + assert_anchor_positions(&modified_path, layer_to_document, &anchor_positions, 1e-10); + + let expected_handles: Vec = vec![ + BezierHandles::Cubic { + handle_start: DVec2::new(700.0, 60.0), + handle_end: DVec2::new(430.0, 180.0), + }, + BezierHandles::Cubic { + handle_start: DVec2::new(400.0, 300.0), + handle_end: DVec2::new(80.0, 70.0), + }, + ]; + assert_handle_positions(&modified_path, &expected_handles, layer_to_document, 1e-10); + } +}