Fix regressions with G/R/S (#3355)

* Better local grab

* Formatting

* Multiply by document_to_viewport.matrix2.y_axis.length()

* Fix logic errors

---------

Co-authored-by: Keavon Chambers <[email protected]>

Author: 0HyperCube

editor/src/messages/portfolio/document/utility_types/transformation.rs

@@ -5,13 +5,13 @@ use crate::messages::portfolio::document::graph_operation::utility_types::{Modif
 use crate::messages::portfolio::document::utility_types::document_metadata::{DocumentMetadata, LayerNodeIdentifier};
 use crate::messages::prelude::*;
 use crate::messages::tool::common_functionality::shape_editor::ShapeState;
+use crate::messages::tool::transform_layer::transform_layer_message_handler::TransformationState;
 use crate::messages::tool::utility_types::ToolType;
 use glam::{DAffine2, DMat2, DVec2};
 use graphene_std::renderer::Quad;
 use graphene_std::vector::misc::{HandleId, ManipulatorPointId};
 use graphene_std::vector::{HandleExt, PointId, VectorModificationType};
 use std::collections::{HashMap, VecDeque};
-use std::f64::consts::PI;
 
 #[derive(Debug, PartialEq, Clone, Copy)]
 struct AnchorPoint {
@@ -156,22 +156,25 @@ pub struct Translation {
 }
 
 impl Translation {
-	pub fn to_dvec(self, transform: DAffine2, increment_mode: bool) -> DVec2 {
+	pub fn to_dvec(self, state: &TransformationState, document: &DocumentMessageHandler) -> DVec2 {
+		let document_to_viewport = document.metadata().document_to_viewport;
 		let displacement = if let Some(value) = self.typed_distance {
 			match self.constraint {
-				Axis::X => transform.transform_vector2(DVec2::new(value, 0.)),
-				Axis::Y => transform.transform_vector2(DVec2::new(0., value)),
+				Axis::X => DVec2::X * value,
+				Axis::Y => DVec2::Y * value,
 				Axis::Both => self.dragged_distance,
 			}
 		} else {
 			match self.constraint {
 				Axis::Both => self.dragged_distance,
-				Axis::X => DVec2::new(self.dragged_distance.x, 0.),
-				Axis::Y => DVec2::new(0., self.dragged_distance.y),
+				Axis::X => DVec2::X * self.dragged_distance.dot(state.constraint_axis(self.constraint).unwrap_or_default()),
+				Axis::Y => DVec2::Y * self.dragged_distance.dot(state.constraint_axis(self.constraint).unwrap_or_default()),
 			}
 		};
-		let displacement = transform.inverse().transform_vector2(displacement);
-		if increment_mode { displacement.round() } else { displacement }
+		let displacement_viewport = displacement * document_to_viewport.matrix2.y_axis.length(); // Values are local to the viewport but scaled so values are relative to the current scale.
+		let displacement_document = document_to_viewport.inverse().transform_vector2(displacement_viewport);
+		let displacement_document = if state.is_rounded_to_intervals { displacement_document.round() } else { displacement_document }; // It rounds in document space?
+		document_to_viewport.transform_vector2(displacement_document)
 	}
 
 	#[must_use]
@@ -327,36 +330,19 @@ impl TransformType {
 
 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) {
-		let local_axis_transform_angle = (quad.top_left() - quad.top_right()).to_angle();
+	pub fn apply_transform_operation(&self, selected: &mut Selected, state: &TransformationState, document: &DocumentMessageHandler) {
 		if self != &TransformOperation::None {
-			let transformation = match self {
-				TransformOperation::Grabbing(translation) => {
-					let translate = DAffine2::from_translation(transform.transform_vector2(translation.to_dvec(local_transform, increment_mode)));
-					if local {
-						let resolved_angle = if local_axis_transform_angle > 0. {
-							local_axis_transform_angle
-						} else {
-							local_axis_transform_angle - PI
-						};
-						DAffine2::from_angle(resolved_angle) * translate * DAffine2::from_angle(-resolved_angle)
-					} else {
-						translate
-					}
-				}
-				TransformOperation::Rotating(rotation) => DAffine2::from_angle(rotation.to_f64(increment_mode)),
-				TransformOperation::Scaling(scale) => {
-					if local {
-						DAffine2::from_angle(local_axis_transform_angle) * DAffine2::from_scale(scale.to_dvec(increment_mode)) * DAffine2::from_angle(-local_axis_transform_angle)
-					} else {
-						DAffine2::from_scale(scale.to_dvec(increment_mode))
-					}
-				}
+			let mut transformation = match self {
+				TransformOperation::Grabbing(translation) => DAffine2::from_translation(translation.to_dvec(state, document)),
+				TransformOperation::Rotating(rotation) => DAffine2::from_angle(rotation.to_f64(state.is_rounded_to_intervals)),
+				TransformOperation::Scaling(scale) => DAffine2::from_scale(scale.to_dvec(state.is_rounded_to_intervals)),
 				TransformOperation::None => unreachable!(),
 			};
+			let normalized_transform = state.local_to_viewport_transform();
+			transformation = normalized_transform * transformation * normalized_transform.inverse();
 
-			selected.update_transforms(transformation, Some(pivot), Some(*self));
-			self.hints(selected.responses, local);
+			selected.update_transforms(transformation, Some(state.pivot_viewport(document)), Some(*self));
+			self.hints(selected.responses, state.is_transforming_in_local_space);
 		}
 	}
 
@@ -373,32 +359,35 @@ impl TransformOperation {
 	}
 
 	#[allow(clippy::too_many_arguments)]
-	pub fn constrain_axis(&mut self, axis: Axis, selected: &mut Selected, increment_mode: bool, mut local: bool, quad: Quad, transform: DAffine2, pivot: DVec2, local_transform: DAffine2) -> bool {
-		(*self, local) = match self {
+	pub fn constrain_axis(&mut self, axis: Axis, selected: &mut Selected, state: &TransformationState, document: &DocumentMessageHandler) -> bool {
+		let resulting_local;
+		(*self, resulting_local) = match self {
 			TransformOperation::Grabbing(translation) => {
-				let (translation, local) = translation.with_constraint(axis, local);
-				(TransformOperation::Grabbing(translation), local)
+				let (translation, resulting_local) = translation.with_constraint(axis, state.is_transforming_in_local_space);
+				(TransformOperation::Grabbing(translation), resulting_local)
 			}
 			TransformOperation::Scaling(scale) => {
-				let (scale, local) = scale.with_constraint(axis, local);
-				(TransformOperation::Scaling(scale), local)
+				let (scale, resulting_local) = scale.with_constraint(axis, state.is_transforming_in_local_space);
+				(TransformOperation::Scaling(scale), resulting_local)
 			}
 			_ => (*self, false),
 		};
-		self.apply_transform_operation(selected, increment_mode, local, quad, transform, pivot, local_transform);
-		local
+
+		self.apply_transform_operation(selected, state, document);
+
+		resulting_local
 	}
 
 	#[allow(clippy::too_many_arguments)]
-	pub fn grs_typed(&mut self, typed: Option<f64>, selected: &mut Selected, increment_mode: bool, local: bool, quad: Quad, transform: DAffine2, pivot: DVec2, local_transform: DAffine2) {
+	pub fn grs_typed(&mut self, typed: Option<f64>, selected: &mut Selected, state: &TransformationState, document: &DocumentMessageHandler) {
 		match self {
 			TransformOperation::None => (),
 			TransformOperation::Grabbing(translation) => translation.typed_distance = typed,
 			TransformOperation::Rotating(rotation) => rotation.typed_angle = typed,
 			TransformOperation::Scaling(scale) => scale.typed_factor = typed,
 		};
 
-		self.apply_transform_operation(selected, increment_mode, local, quad, transform, pivot, local_transform);
+		self.apply_transform_operation(selected, state, document);
 	}
 
 	pub fn hints(&self, responses: &mut VecDeque<Message>, local: bool) {
@@ -481,15 +470,16 @@ impl TransformOperation {
 	}
 
 	#[allow(clippy::too_many_arguments)]
-	pub fn negate(&mut self, selected: &mut Selected, increment_mode: bool, local: bool, quad: Quad, transform: DAffine2, pivot: DVec2, local_transform: DAffine2) {
+	pub fn negate(&mut self, selected: &mut Selected, state: &TransformationState, document: &DocumentMessageHandler) {
 		if *self != TransformOperation::None {
 			*self = match self {
 				TransformOperation::Scaling(scale) => TransformOperation::Scaling(scale.negate()),
 				TransformOperation::Rotating(rotation) => TransformOperation::Rotating(rotation.negate()),
 				TransformOperation::Grabbing(translation) => TransformOperation::Grabbing(translation.negate()),
 				_ => *self,
 			};
-			self.apply_transform_operation(selected, increment_mode, local, quad, transform, pivot, local_transform);
+
+			self.apply_transform_operation(selected, state, document);
 		}
 	}
 }

editor/src/messages/tool/tool_messages/select_tool.rs

@@ -593,6 +593,19 @@ impl SelectToolData {
 	}
 }
 
+/// Bounding boxes are unfortunately not axis aligned. The bounding boxes are found after a transformation is applied to all of the layers.
+/// This uses some rather confusing logic to determine what transform that should be.
+pub fn create_bounding_box_transform(document: &DocumentMessageHandler) -> DAffine2 {
+	// Update bounds
+	document
+		.network_interface
+		.selected_nodes()
+		.selected_visible_and_unlocked_layers(&document.network_interface)
+		.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
+		.map(|layer| document.metadata().transform_to_viewport_with_first_transform_node_if_group(layer, &document.network_interface))
+		.unwrap_or_default()
+}
+
 impl Fsm for SelectToolFsmState {
 	type ToolData = SelectToolData;
 	type ToolOptions = ();
@@ -633,14 +646,7 @@ impl Fsm for SelectToolFsmState {
 					}
 				}
 
-				// Update bounds
-				let mut transform = document
-					.network_interface
-					.selected_nodes()
-					.selected_visible_and_unlocked_layers(&document.network_interface)
-					.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
-					.map(|layer| document.metadata().transform_to_viewport_with_first_transform_node_if_group(layer, &document.network_interface))
-					.unwrap_or_default();
+				let mut transform = create_bounding_box_transform(document);
 
 				// Check if the matrix is not invertible
 				let mut transform_tampered = false;