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 <keavon@keavon.com>

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(Enter); action_dispatch=TransformLayerMessage::ApplyTransformOperation { final_transform: true }),
-		entry!(KeyDown(MouseLeft); action_dispatch=TransformLayerMessage::ApplyTransformOperation),
+		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(KeyG); action_dispatch=TransformLayerMessage::BeginGRS { transform_type: TransformType::Grab }),
-		entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginRotate),
+		entry!(KeyDown(KeyR); action_dispatch=TransformLayerMessage::BeginGRS { transform_type: TransformType::Rotate }),
-		entry!(KeyDown(KeyS); action_dispatch=TransformLayerMessage::BeginScale),
+		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 }),

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) {

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 },

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<Message>) {
-	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<TransformLayerMessage, TransformData<'_>> 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 {
+								let end = if self.local { (quad[1] - quad[0]).rotate(e1) + quad[0] } else { quad[1] };
-									(quad[1] - quad[0]).length() * e1 * e1.dot(quad[1] - quad[0]).signum() + 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<TransformLayerMessage, TransformData<'_>> for TransformLayer
 							}
 
 							if matches!(axis_constraint, Axis::Both | Axis::Y) && translation.y != 0. {
-								let end = if self.local {
+								let end = if self.local { (quad[3] - quad[0]).rotate(e1) + quad[0] } else { quad[3] };
-									(quad[3] - quad[0]).length() * e1.perp() * e1.perp().dot(quad[3] - quad[0]).signum() + 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<TransformLayerMessage, TransformData<'_>> 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<TransformLayerMessage, TransformData<'_>> for TransformLayer
 					increments_key: INCREMENTS_KEY,
 				});
 			}
-			TransformLayerMessage::BeginGrab => {
+			TransformLayerMessage::BeginGRS { transform_type } => {
-				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 => {
 				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 <https://discord.com/channels/731730685944922173/1217752903209713715>
-
-						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<TransformLayerMessage, TransformData<'_>> for TransformLayer
 							}
 						}
 					} else {
+						// TODO: Fix handle snap to anchor issue, see <https://github.com/GraphiteEditor/Graphite/issues/2451>
 						let handle_length = point.as_handle().map(|handle| handle.length(&vector_data));
 
 						if handle_length == Some(0.) {
@@ -496,22 +411,42 @@ impl MessageHandler<TransformLayerMessage, TransformData<'_>> 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;
-
+				responses.add(response);
-				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::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<TransformLayerMessage, TransformData<'_>> for TransformLayer
 
 	fn actions(&self) -> ActionList {
 		let mut common = actions!(TransformLayerMessageDiscriminant;
-			BeginGrab,
+			BeginGRS,
-			BeginScale,
-			BeginRotate,
 		);
 
 		if self.transform_operation != TransformOperation::None {