From 7e7e88f6fa038d6ea2b005d72b6ff63cfc6d09c1 Mon Sep 17 00:00:00 2001 From: mTvare Date: Mon, 17 Mar 2025 17:20:11 +0530 Subject: [PATCH] Refactor GRS messages and fix regression in chained GRS operations (#2450) * Refactor GRS messages and have a switch handler * some cleanup * rename variables * Code review --------- Co-authored-by: Keavon Chambers --- .../messages/input_mapper/input_mappings.rs | 11 +- .../document/utility_types/transformation.rs | 16 ++ .../transform_layer_message.rs | 4 +- .../transform_layer_message_handler.rs | 153 +++++------------- 4 files changed, 68 insertions(+), 116 deletions(-) diff --git a/editor/src/messages/input_mapper/input_mappings.rs b/editor/src/messages/input_mapper/input_mappings.rs index 0ac3ee41..9879bd45 100644 --- a/editor/src/messages/input_mapper/input_mappings.rs +++ b/editor/src/messages/input_mapper/input_mappings.rs @@ -8,6 +8,7 @@ use crate::messages::input_mapper::utility_types::misc::{KeyMappingEntries, Mapp use crate::messages::portfolio::document::node_graph::utility_types::Direction; use crate::messages::portfolio::document::utility_types::clipboards::Clipboard; use crate::messages::portfolio::document::utility_types::misc::GroupFolderType; +use crate::messages::portfolio::document::utility_types::transformation::TransformType; use crate::messages::prelude::*; use crate::messages::tool::tool_messages::brush_tool::BrushToolMessageOptionsUpdate; use crate::messages::tool::tool_messages::select_tool::SelectToolPointerKeys; @@ -82,8 +83,8 @@ pub fn input_mappings() -> Mapping { entry!(KeyDown(ArrowLeft); action_dispatch=NodeGraphMessage::ShiftSelectedNodes { direction: Direction::Left, rubber_band: false }), // // TransformLayerMessage - entry!(KeyDown(Enter); action_dispatch=TransformLayerMessage::ApplyTransformOperation), - entry!(KeyDown(MouseLeft); action_dispatch=TransformLayerMessage::ApplyTransformOperation), + entry!(KeyDown(Enter); action_dispatch=TransformLayerMessage::ApplyTransformOperation { final_transform: true }), + entry!(KeyDown(MouseLeft); action_dispatch=TransformLayerMessage::ApplyTransformOperation { final_transform: true }), entry!(KeyDown(MouseRight); action_dispatch=TransformLayerMessage::CancelTransformOperation), entry!(KeyDown(Escape); action_dispatch=TransformLayerMessage::CancelTransformOperation), entry!(KeyDown(KeyX); action_dispatch=TransformLayerMessage::ConstrainX), @@ -373,9 +374,9 @@ pub fn input_mappings() -> Mapping { entry!(KeyDown(ArrowRight); action_dispatch=DocumentMessage::NudgeSelectedLayers { delta_x: NUDGE_AMOUNT, delta_y: 0., resize: Alt, resize_opposite_corner: Control }), // // TransformLayerMessage - entry!(KeyDown(KeyG); action_dispatch=TransformLayerMessage::BeginGrab), - entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginRotate), - entry!(KeyDown(KeyS); action_dispatch=TransformLayerMessage::BeginScale), + entry!(KeyDown(KeyG); action_dispatch=TransformLayerMessage::BeginGRS { transform_type: TransformType::Grab }), + entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginGRS { transform_type: TransformType::Rotate }), + entry!(KeyDown(KeyS); action_dispatch=TransformLayerMessage::BeginGRS { transform_type: TransformType::Scale }), entry!(KeyDown(Digit0); action_dispatch=TransformLayerMessage::TypeDigit { digit: 0 }), entry!(KeyDown(Digit1); action_dispatch=TransformLayerMessage::TypeDigit { digit: 1 }), entry!(KeyDown(Digit2); action_dispatch=TransformLayerMessage::TypeDigit { digit: 2 }), diff --git a/editor/src/messages/portfolio/document/utility_types/transformation.rs b/editor/src/messages/portfolio/document/utility_types/transformation.rs index 9058f18c..2d6d4033 100644 --- a/editor/src/messages/portfolio/document/utility_types/transformation.rs +++ b/editor/src/messages/portfolio/document/utility_types/transformation.rs @@ -299,6 +299,22 @@ pub enum TransformOperation { Scaling(Scale), } +#[derive(Debug, Clone, PartialEq, Copy, serde::Serialize, serde::Deserialize)] +pub enum TransformType { + Grab, + Rotate, + Scale, +} + +impl TransformType { + pub fn equivalent_to(&self, operation: TransformOperation) -> bool { + matches!( + (operation, self), + (TransformOperation::Scaling(_), TransformType::Scale) | (TransformOperation::Grabbing(_), TransformType::Grab) | (TransformOperation::Rotating(_), TransformType::Rotate) + ) + } +} + impl TransformOperation { #[allow(clippy::too_many_arguments)] pub fn apply_transform_operation(&self, selected: &mut Selected, increment_mode: bool, local: bool, quad: Quad, transform: DAffine2, pivot: DVec2, local_transform: DAffine2) { diff --git a/editor/src/messages/tool/transform_layer/transform_layer_message.rs b/editor/src/messages/tool/transform_layer/transform_layer_message.rs index fd11dbf9..dfc45c1e 100644 --- a/editor/src/messages/tool/transform_layer/transform_layer_message.rs +++ b/editor/src/messages/tool/transform_layer/transform_layer_message.rs @@ -1,5 +1,6 @@ use crate::messages::input_mapper::utility_types::input_keyboard::Key; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; +use crate::messages::portfolio::document::utility_types::transformation::TransformType; use crate::messages::prelude::*; use glam::DVec2; @@ -10,10 +11,11 @@ pub enum TransformLayerMessage { Overlays(OverlayContext), // Messages - ApplyTransformOperation, + ApplyTransformOperation { final_transform: bool }, BeginGrab, BeginRotate, BeginScale, + BeginGRS { transform_type: TransformType }, BeginGrabPen { last_point: DVec2, handle: DVec2 }, BeginRotatePen { last_point: DVec2, handle: DVec2 }, BeginScalePen { last_point: DVec2, handle: DVec2 }, 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 c9bd177c..c25f3ce8 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 @@ -3,7 +3,7 @@ use crate::messages::input_mapper::utility_types::input_mouse::{DocumentPosition 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::portfolio::document::utility_types::transformation::{Axis, OriginalTransforms, Selected, TransformOperation, TransformType, Typing}; use crate::messages::prelude::*; use crate::messages::tool::common_functionality::shape_editor::ShapeState; use crate::messages::tool::tool_messages::tool_prelude::Key; @@ -12,7 +12,7 @@ use glam::{DAffine2, DVec2}; use graphene_core::renderer::Quad; use graphene_core::vector::ManipulatorPointId; use graphene_std::vector::{VectorData, VectorModificationType}; -use std::f64::consts::TAU; +use std::f64::consts::{PI, TAU}; const TRANSFORM_GRS_OVERLAY_PROVIDER: OverlayProvider = |context| TransformLayerMessage::Overlays(context).into(); @@ -107,8 +107,6 @@ 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 }; @@ -243,11 +241,7 @@ impl MessageHandler> for TransformLayer let e1 = (self.layer_bounding_box.0[1] - self.layer_bounding_box.0[0]).normalize_or(DVec2::X); if matches!(axis_constraint, Axis::Both | Axis::X) && translation.x != 0. { - let end = if self.local { - (quad[1] - quad[0]).length() * e1 * e1.dot(quad[1] - quad[0]).signum() + quad[0] - } else { - quad[1] - }; + let end = if self.local { (quad[1] - quad[0]).rotate(e1) + quad[0] } else { quad[1] }; overlay_context.line(quad[0], end, None); let x_transform = DAffine2::from_translation((quad[0] + end) / 2.); @@ -255,11 +249,7 @@ impl MessageHandler> for TransformLayer } if matches!(axis_constraint, Axis::Both | Axis::Y) && translation.y != 0. { - let end = if self.local { - (quad[3] - quad[0]).length() * e1.perp() * e1.perp().dot(quad[3] - quad[0]).signum() + quad[0] - } else { - quad[3] - }; + let end = if self.local { (quad[3] - quad[0]).rotate(e1) + quad[0] } else { quad[3] }; overlay_context.line(quad[0], end, None); let x_parameter = viewport_translate.x.clamp(-1., 1.); let y_transform = DAffine2::from_translation((quad[0] + end) / 2. + x_parameter * DVec2::X * 0.); @@ -325,27 +315,30 @@ impl MessageHandler> for TransformLayer } // Messages - TransformLayerMessage::ApplyTransformOperation => { + TransformLayerMessage::ApplyTransformOperation { final_transform } => { selected.original_transforms.clear(); self.typing.clear(); - self.transform_operation = TransformOperation::None; + if final_transform { + self.transform_operation = TransformOperation::None; + self.operation_count = 0; + } if using_pen_tool { self.last_point = DVec2::ZERO; self.grs_pen_handle = false; selected.pen_handle = None; - selected.responses.add(PenToolMessage::Confirm); } else { update_colinear_handles(&selected_layers, document, responses); - self.operation_count = 0; responses.add(DocumentMessage::EndTransaction); responses.add(ToolMessage::UpdateHints); responses.add(NodeGraphMessage::RunDocumentGraph); } - responses.add(OverlaysMessage::RemoveProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); + if final_transform { + responses.add(OverlaysMessage::RemoveProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); + } } TransformLayerMessage::BeginGrabPen { last_point, handle } | TransformLayerMessage::BeginRotatePen { last_point, handle } | TransformLayerMessage::BeginScalePen { last_point, handle } => { self.typing.clear(); @@ -381,91 +374,12 @@ impl MessageHandler> for TransformLayer increments_key: INCREMENTS_KEY, }); } - TransformLayerMessage::BeginGrab => { - if (!using_path_tool && !using_select_tool && !using_pen_tool) - || (using_path_tool && shape_editor.selected_points().next().is_none()) - || selected_layers.is_empty() - || matches!(self.transform_operation, TransformOperation::Grabbing(_)) - { - return; - } - - begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); - - self.transform_operation = TransformOperation::Grabbing(Default::default()); - self.local = false; - self.layer_bounding_box = selected.bounding_box(); - self.operation_count += 1; - - selected.original_transforms.clear(); - - responses.add(OverlaysMessage::AddProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); - responses.add(TransformLayerMessage::PointerMove { - slow_key: SLOW_KEY, - increments_key: INCREMENTS_KEY, - }); - } - TransformLayerMessage::BeginRotate => { + TransformLayerMessage::BeginGRS { transform_type } => { let selected_points: Vec<&ManipulatorPointId> = shape_editor.selected_points().collect(); - - if (!using_path_tool && !using_select_tool && !using_pen_tool) - || (using_path_tool && selected_points.is_empty()) - || selected_layers.is_empty() - || matches!(self.transform_operation, TransformOperation::Rotating(_)) - { - return; - } - - let Some(vector_data) = selected_layers.first().and_then(|&layer| document.network_interface.compute_modified_vector(layer)) else { - selected.original_transforms.clear(); - return; - }; - - if let [point] = selected_points.as_slice() { - if matches!(point, ManipulatorPointId::Anchor(_)) { - if let Some([handle1, handle2]) = point.get_handle_pair(&vector_data) { - let handle1_length = handle1.length(&vector_data); - let handle2_length = handle2.length(&vector_data); - - if (handle1_length == 0. && handle2_length == 0.) || (handle1_length == f64::MAX && handle2_length == f64::MAX) { - return; - } - } - } else { - // TODO: Fix handle snap to anchor issue, see - - let handle_length = point.as_handle().map(|handle| handle.length(&vector_data)); - - if handle_length == Some(0.) { - selected.original_transforms.clear(); - return; - } - } - } - - begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); - - self.transform_operation = TransformOperation::Rotating(Default::default()); - - self.local = false; - self.layer_bounding_box = selected.bounding_box(); - self.operation_count += 1; - - selected.original_transforms.clear(); - - responses.add(OverlaysMessage::AddProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); - responses.add(TransformLayerMessage::PointerMove { - slow_key: SLOW_KEY, - increments_key: INCREMENTS_KEY, - }); - } - TransformLayerMessage::BeginScale => { - let selected_points: Vec<&ManipulatorPointId> = shape_editor.selected_points().collect(); - if (using_path_tool && selected_points.is_empty()) || (!using_path_tool && !using_select_tool && !using_pen_tool) || selected_layers.is_empty() - || matches!(self.transform_operation, TransformOperation::Scaling(_)) + || transform_type.equivalent_to(self.transform_operation) { return; } @@ -487,6 +401,7 @@ impl MessageHandler> for TransformLayer } } } else { + // TODO: Fix handle snap to anchor issue, see let handle_length = point.as_handle().map(|handle| handle.length(&vector_data)); if handle_length == Some(0.) { @@ -496,22 +411,42 @@ impl MessageHandler> for TransformLayer } } - begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); + let chain_operation = self.transform_operation != TransformOperation::None; + if chain_operation { + responses.add(TransformLayerMessage::ApplyTransformOperation { final_transform: false }); + } else { + responses.add(OverlaysMessage::AddProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); + } - self.transform_operation = TransformOperation::Scaling(Default::default()); + let response = match transform_type { + TransformType::Grab => TransformLayerMessage::BeginGrab, + TransformType::Rotate => TransformLayerMessage::BeginRotate, + TransformType::Scale => TransformLayerMessage::BeginScale, + }; self.local = false; - self.layer_bounding_box = selected.bounding_box(); self.operation_count += 1; - - selected.original_transforms.clear(); - - responses.add(OverlaysMessage::AddProvider(TRANSFORM_GRS_OVERLAY_PROVIDER)); + responses.add(response); responses.add(TransformLayerMessage::PointerMove { slow_key: SLOW_KEY, increments_key: INCREMENTS_KEY, }); } + TransformLayerMessage::BeginGrab => { + begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); + self.transform_operation = TransformOperation::Grabbing(Default::default()); + self.layer_bounding_box = selected.bounding_box(); + } + TransformLayerMessage::BeginRotate => { + begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); + self.transform_operation = TransformOperation::Rotating(Default::default()); + self.layer_bounding_box = selected.bounding_box(); + } + TransformLayerMessage::BeginScale => { + begin_operation(self.transform_operation, &mut self.typing, &mut self.mouse_position, &mut self.start_mouse, &mut self.initial_transform); + self.transform_operation = TransformOperation::Scaling(Default::default()); + self.layer_bounding_box = selected.bounding_box(); + } TransformLayerMessage::CancelTransformOperation => { if using_pen_tool { self.typing.clear(); @@ -744,9 +679,7 @@ impl MessageHandler> for TransformLayer fn actions(&self) -> ActionList { let mut common = actions!(TransformLayerMessageDiscriminant; - BeginGrab, - BeginScale, - BeginRotate, + BeginGRS, ); if self.transform_operation != TransformOperation::None {