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 <keavon@keavon.com>
This commit is contained in:
0SlowPoke0 2025-03-01 13:44:29 +05:30 committed by GitHub
parent 3a7d1938b6
commit 1b59a9414a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 89 additions and 26 deletions

View file

@ -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<Message>) {
fn transform_path(
document_metadata: &DocumentMetadata,
layer: LayerNodeIdentifier,
initial_points: &mut InitialPoints,
transformation: DAffine2,
responses: &mut VecDeque<Message>,
transform_operation: Option<TransformOperation>,
) {
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<TransformOperation>) {
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<DVec2>) {
let pivot = DAffine2::from_translation(pivot.unwrap_or_else(|| *self.pivot));
pub fn update_transforms(&mut self, delta: DAffine2, pivot: Option<DVec2>, transform_operation: Option<TransformOperation>) {
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),
}
}

View file

@ -552,6 +552,17 @@ impl PenToolData {
fn update_handle_position(&mut self, new_position: DVec2, anchor_pos: DVec2, responses: &mut VecDeque<Message>, 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));
}

View file

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

View file

@ -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<Message>) {
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<TransformLayerMessage, TransformData<'_>> for TransformLayerMessageHandler {
fn process_message(&mut self, message: TransformLayerMessage, responses: &mut VecDeque<Message>, (document, input, tool_data, shape_editor): TransformData) {
@ -310,6 +340,7 @@ impl MessageHandler<TransformLayerMessage, TransformData<'_>> 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);