From 1b59a9414a0a2b13e139d9246d972e31b5e49dc3 Mon Sep 17 00:00:00 2001 From: 0SlowPoke0 <142654792+0SlowPoke0@users.noreply.github.com> Date: Sat, 1 Mar 2025 13:44:29 +0530 Subject: [PATCH] Fix several minor Pen and Path tool bugs (#2327) * code-todo-fixes * small typo * fixed bent_case when drawn from start point * Code review --------- Co-authored-by: Keavon Chambers --- .../document/utility_types/transformation.rs | 25 ++++++--- .../messages/tool/tool_messages/pen_tool.rs | 51 +++++++++++++------ .../tool/tool_messages/select_tool.rs | 6 +-- .../transform_layer_message_handler.rs | 33 +++++++++++- 4 files changed, 89 insertions(+), 26 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/transformation.rs b/editor/src/messages/portfolio/document/utility_types/transformation.rs index 63b9dbdd..24953b9d 100644 --- a/editor/src/messages/portfolio/document/utility_types/transformation.rs +++ b/editor/src/messages/portfolio/document/utility_types/transformation.rs @@ -334,7 +334,7 @@ impl TransformOperation { TransformOperation::None => unreachable!(), }; - selected.update_transforms(transformation, Some(pivot)); + selected.update_transforms(transformation, Some(pivot), Some(*self)); self.hints(selected.responses, local); } } @@ -572,7 +572,14 @@ impl<'a> Selected<'a> { }); } - fn transform_path(document_metadata: &DocumentMetadata, layer: LayerNodeIdentifier, initial_points: &mut InitialPoints, transformation: DAffine2, responses: &mut VecDeque) { + fn transform_path( + document_metadata: &DocumentMetadata, + layer: LayerNodeIdentifier, + initial_points: &mut InitialPoints, + transformation: DAffine2, + responses: &mut VecDeque, + transform_operation: Option, + ) { let viewspace = document_metadata.transform_to_viewport(layer); let layerspace_rotation = viewspace.inverse() * transformation; @@ -584,6 +591,10 @@ impl<'a> Selected<'a> { responses.add(GraphOperationMessage::Vector { layer, modification_type }); } + if transform_operation.is_some_and(|transform_operation| matches!(transform_operation, TransformOperation::Scaling(_))) && initial_points.anchors.len() > 1 { + return; + } + for (&id, handle) in initial_points.handles.iter() { let new_pos_viewport = layerspace_rotation.transform_point2(viewspace.transform_point2(handle.initial)); let relative = initial_points.anchors.get(&handle.anchor).map_or(handle.relative, |anchor| anchor.current); @@ -610,7 +621,7 @@ impl<'a> Selected<'a> { } } - pub fn apply_transformation(&mut self, transformation: DAffine2) { + pub fn apply_transformation(&mut self, transformation: DAffine2, transform_operation: Option) { if !self.selected.is_empty() { // TODO: Cache the result of `shallowest_unique_layers` to avoid this heavy computation every frame of movement, see https://github.com/GraphiteEditor/Graphite/pull/481 for layer in self.network_interface.shallowest_unique_layers(&[]) { @@ -620,7 +631,7 @@ impl<'a> Selected<'a> { } OriginalTransforms::Path(path_transforms) => { if let Some(initial_points) = path_transforms.get_mut(&layer) { - Self::transform_path(self.network_interface.document_metadata(), layer, initial_points, transformation, self.responses) + Self::transform_path(self.network_interface.document_metadata(), layer, initial_points, transformation, self.responses, transform_operation) } } } @@ -628,12 +639,12 @@ impl<'a> Selected<'a> { } } - pub fn update_transforms(&mut self, delta: DAffine2, pivot: Option) { - let pivot = DAffine2::from_translation(pivot.unwrap_or_else(|| *self.pivot)); + pub fn update_transforms(&mut self, delta: DAffine2, pivot: Option, transform_operation: Option) { + let pivot = DAffine2::from_translation(pivot.unwrap_or(*self.pivot)); let transformation = pivot * delta * pivot.inverse(); match self.tool_type { ToolType::Pen => self.apply_transform_pen(transformation), - _ => self.apply_transformation(transformation), + _ => self.apply_transformation(transformation, transform_operation), } } diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs index 47cfb676..7c19076a 100644 --- a/editor/src/messages/tool/tool_messages/pen_tool.rs +++ b/editor/src/messages/tool/tool_messages/pen_tool.rs @@ -552,6 +552,17 @@ impl PenToolData { fn update_handle_position(&mut self, new_position: DVec2, anchor_pos: DVec2, responses: &mut VecDeque, layer: LayerNodeIdentifier, is_start: bool) { let relative_position = new_position - anchor_pos; + if is_start { + let modification_type = VectorModificationType::SetPrimaryHandle { + segment: self + .end_point_segment + .expect("In update_handle_position(), if `is_start` is true then `end_point_segment` should exist"), + relative_position, + }; + responses.add(GraphOperationMessage::Vector { layer, modification_type }); + return; + } + if self.draw_mode == DrawMode::ContinuePath { if let Some(handle) = self.handle_end.as_mut() { *handle = new_position; @@ -566,12 +577,6 @@ impl PenToolData { let Some(segment) = self.end_point_segment else { return }; - if is_start { - let modification_type = VectorModificationType::SetPrimaryHandle { segment, relative_position }; - responses.add(GraphOperationMessage::Vector { layer, modification_type }); - return; - } - let modification_type = VectorModificationType::SetEndHandle { segment, relative_position }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } @@ -946,22 +951,36 @@ impl Fsm for PenToolFsmState { latest_pt.handle_start = final_pos; } + responses.add(OverlaysMessage::Draw); + // Making the end handle colinear match tool_data.handle_mode { HandleMode::Free => {} HandleMode::ColinearEquidistant | HandleMode::ColinearLocked => { if let Some((latest, segment)) = tool_data.latest_point().zip(tool_data.end_point_segment) { - let handle = ManipulatorPointId::EndHandle(segment).get_position(&vector_data); - let Some(handle) = handle else { return PenToolFsmState::GRSHandle }; - let Some(direction) = (latest.pos - latest.handle_start).try_normalize() else { - log::trace!("Skipping handle adjustment: latest.pos and latest.handle_start are too close!"); return PenToolFsmState::GRSHandle; }; + if (latest.pos - latest.handle_start).length_squared() < f64::EPSILON { + return PenToolFsmState::GRSHandle; + } + + let is_start = vector_data.segment_start_from_id(segment) == Some(latest.id); + + let handle = if is_start { + ManipulatorPointId::PrimaryHandle(segment).get_position(&vector_data) + } else { + ManipulatorPointId::EndHandle(segment).get_position(&vector_data) + }; + let Some(handle) = handle else { return PenToolFsmState::GRSHandle }; let relative_distance = (handle - latest.pos).length(); let relative_position = relative_distance * direction; - let modification_type = VectorModificationType::SetEndHandle { segment, relative_position }; + let modification_type = if is_start { + VectorModificationType::SetPrimaryHandle { segment, relative_position } + } else { + VectorModificationType::SetEndHandle { segment, relative_position } + }; responses.add(GraphOperationMessage::Vector { layer, modification_type }); } } @@ -1050,8 +1069,10 @@ impl Fsm for PenToolFsmState { let handles = BezierHandles::Cubic { handle_start, handle_end }; let end = tool_data.next_point; let bezier = Bezier { start, handles, end }; - // Draw the curve for the currently-being-placed segment - overlay_context.outline_bezier(bezier, transform); + if (end - start).length_squared() > f64::EPSILON { + // Draw the curve for the currently-being-placed segment + overlay_context.outline_bezier(bezier, transform); + } } // Draw the line between the currently-being-placed anchor and its currently-being-dragged-out outgoing handle (opposite the one currently being dragged out) @@ -1078,11 +1099,11 @@ impl Fsm for PenToolFsmState { overlay_context.line(next_anchor, handle_end, None); if self == PenToolFsmState::PlacingAnchor && anchor_start != handle_start && tool_data.modifiers.lock_angle { - // Draw the line between the currently-being-placed anchor and last-placed point (Lock angle bent overlays) + // Draw the line between the currently-being-placed anchor and last-placed point (lock angle bent overlays) overlay_context.dashed_line(anchor_start, next_anchor, None, Some(4.), Some(4.), Some(0.5)); } - // Draw the line between the currently-being-placed anchor and last-placed point (Lock angle bent overlays) + // Draw the line between the currently-being-placed anchor and last-placed point (snap angle bent overlays) if self == PenToolFsmState::PlacingAnchor && anchor_start != handle_start && tool_data.modifiers.snap_angle { overlay_context.dashed_line(anchor_start, next_anchor, None, Some(4.), Some(4.), Some(0.5)); } diff --git a/editor/src/messages/tool/tool_messages/select_tool.rs b/editor/src/messages/tool/tool_messages/select_tool.rs index 83fc2987..bcd8b8c5 100644 --- a/editor/src/messages/tool/tool_messages/select_tool.rs +++ b/editor/src/messages/tool/tool_messages/select_tool.rs @@ -1022,7 +1022,7 @@ impl Fsm for SelectToolFsmState { None, ); - selected.apply_transformation(bounds.original_bound_transform * transformation * bounds.original_bound_transform.inverse()); + selected.apply_transformation(bounds.original_bound_transform * transformation * bounds.original_bound_transform.inverse(), None); // AutoPanning let messages = [ @@ -1060,7 +1060,7 @@ impl Fsm for SelectToolFsmState { None, ); - selected.apply_transformation(bounds.original_bound_transform * transformation * bounds.original_bound_transform.inverse()); + selected.apply_transformation(bounds.original_bound_transform * transformation * bounds.original_bound_transform.inverse(), None); } } SelectToolFsmState::SkewingBounds @@ -1102,7 +1102,7 @@ impl Fsm for SelectToolFsmState { None, ); - selected.update_transforms(delta, None); + selected.update_transforms(delta, None, None); } SelectToolFsmState::RotatingBounds diff --git a/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs b/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs index c282b848..6b71e080 100644 --- a/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs +++ b/editor/src/messages/tool/transform_layer/transform_layer_message_handler.rs @@ -1,6 +1,7 @@ use crate::consts::{ANGLE_MEASURE_RADIUS_FACTOR, ARC_MEASURE_RADIUS_FACTOR_RANGE, COLOR_OVERLAY_BLUE, SLOWING_DIVISOR}; use crate::messages::input_mapper::utility_types::input_mouse::{DocumentPosition, ViewportPosition}; use crate::messages::portfolio::document::overlays::utility_types::{OverlayProvider, Pivot}; +use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier; use crate::messages::portfolio::document::utility_types::misc::PTZ; use crate::messages::portfolio::document::utility_types::transformation::{Axis, OriginalTransforms, Selected, TransformOperation, Typing}; use crate::messages::prelude::*; @@ -10,7 +11,7 @@ use crate::messages::tool::utility_types::{ToolData, ToolType}; use graphene_core::renderer::Quad; use graphene_core::vector::ManipulatorPointId; -use graphene_std::vector::VectorData; +use graphene_std::vector::{VectorData, VectorModificationType}; use glam::{DAffine2, DVec2}; use std::f64::consts::TAU; @@ -103,6 +104,35 @@ fn project_edge_to_quad(edge: DVec2, quad: &Quad, local: bool, axis_constraint: } } +fn update_colinear_handles(selected_layers: &[LayerNodeIdentifier], document: &DocumentMessageHandler, responses: &mut VecDeque) { + use std::f64::consts::PI; + + for &layer in selected_layers { + let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else { continue }; + + for [handle1, handle2] in &vector_data.colinear_manipulators { + let manipulator1 = handle1.to_manipulator_point(); + let manipulator2 = handle2.to_manipulator_point(); + + let Some(anchor) = manipulator1.get_anchor_position(&vector_data) else { continue }; + let Some(pos1) = manipulator1.get_position(&vector_data).map(|pos| pos - anchor) else { continue }; + let Some(pos2) = manipulator2.get_position(&vector_data).map(|pos| pos - anchor) else { continue }; + + let angle = pos1.angle_to(pos2); + + // Check if handles are not colinear (not approximately equal to +/- PI) + if (angle - PI).abs() > 1e-6 && (angle + PI).abs() > 1e-6 { + let modification_type = VectorModificationType::SetG1Continuous { + handles: [*handle1, *handle2], + enabled: false, + }; + + responses.add(GraphOperationMessage::Vector { layer, modification_type }); + } + } + } +} + type TransformData<'a> = (&'a DocumentMessageHandler, &'a InputPreprocessorMessageHandler, &'a ToolData, &'a mut ShapeState); impl MessageHandler> for TransformLayerMessageHandler { fn process_message(&mut self, message: TransformLayerMessage, responses: &mut VecDeque, (document, input, tool_data, shape_editor): TransformData) { @@ -310,6 +340,7 @@ impl MessageHandler> for TransformLayer selected.responses.add(PenToolMessage::Confirm); } else { + update_colinear_handles(&selected_layers, document, responses); responses.add(DocumentMessage::EndTransaction); responses.add(ToolMessage::UpdateHints); responses.add(NodeGraphMessage::RunDocumentGraph);