From 90a8036c470ba7cc9ddb09592a7f50c3a0ab8847 Mon Sep 17 00:00:00 2001 From: mTvare Date: Mon, 17 Feb 2025 17:58:33 +0530 Subject: [PATCH] Fix self-chaining of transforms; fix compass rose getting offset when rotating a layer (#2296) * Fix self-chaining of transforms and compass rose under single layer https://discord.com/channels/731730685944922173/931942323644928040/1340632846863175702 https://discord.com/channels/731730685944922173/931942323644928040/1340608972071243906 * When not invertible transformation, do nothing * Fix overlays and compass control when can't be visible * Simplify selection logic in compass states * Show compass only if it was possible that it could be seen before dragging * Prevent resizing line objects * Code review --------- Co-authored-by: Keavon Chambers --- .../document/document_message_handler.rs | 2 +- .../tool/common_functionality/compass_rose.rs | 25 ++- .../tool/common_functionality/pivot.rs | 9 +- .../transformation_cage.rs | 20 ++- .../tool/tool_messages/select_tool.rs | 159 ++++++++++-------- .../transform_layer_message_handler.rs | 4 - .../gcore/src/graphic_element/renderer.rs | 2 +- .../src/graphic_element/renderer/quad.rs | 18 +- 8 files changed, 141 insertions(+), 98 deletions(-) diff --git a/editor/src/messages/portfolio/document/document_message_handler.rs b/editor/src/messages/portfolio/document/document_message_handler.rs index 342c55d4..cee4e5c3 100644 --- a/editor/src/messages/portfolio/document/document_message_handler.rs +++ b/editor/src/messages/portfolio/document/document_message_handler.rs @@ -2321,7 +2321,7 @@ pub struct ClickXRayIter<'a> { } fn quad_to_path_lib_segments(quad: Quad) -> Vec { - quad.edges().into_iter().map(|[start, end]| path_bool_lib::PathSegment::Line(start, end)).collect() + quad.all_edges().into_iter().map(|[start, end]| path_bool_lib::PathSegment::Line(start, end)).collect() } fn click_targets_to_path_lib_segments<'a>(click_targets: impl Iterator, transform: DAffine2) -> Vec { diff --git a/editor/src/messages/tool/common_functionality/compass_rose.rs b/editor/src/messages/tool/common_functionality/compass_rose.rs index be7af7ca..21a35668 100644 --- a/editor/src/messages/tool/common_functionality/compass_rose.rs +++ b/editor/src/messages/tool/common_functionality/compass_rose.rs @@ -1,4 +1,5 @@ use crate::consts::{COMPASS_ROSE_ARROW_CLICK_TARGET_ANGLE, COMPASS_ROSE_HOVER_RING_DIAMETER, COMPASS_ROSE_RING_INNER_DIAMETER}; +use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier; use crate::messages::prelude::DocumentMessageHandler; use glam::{DAffine2, DVec2}; @@ -10,9 +11,27 @@ pub struct CompassRose { } impl CompassRose { - pub fn refresh_transform(&mut self, document: &DocumentMessageHandler) { - let [min, max] = document.selected_visible_and_unlock_layers_bounding_box_viewport().unwrap_or([DVec2::ZERO, DVec2::ONE]); - self.compass_center = (DAffine2::from_translation(min) * DAffine2::from_scale(max - min)).transform_point2(DVec2::splat(0.5)); + fn get_layer_pivot_transform(layer: LayerNodeIdentifier, document: &DocumentMessageHandler) -> DAffine2 { + let [min, max] = document.metadata().nonzero_bounding_box(layer); + + let bounds_transform = DAffine2::from_translation(min) * DAffine2::from_scale(max - min); + let layer_transform = document.metadata().transform_to_viewport(layer); + layer_transform * bounds_transform + } + pub fn refresh_position(&mut self, document: &DocumentMessageHandler) { + let selected_nodes = document.network_interface.selected_nodes(&[]).unwrap(); + let mut layers = selected_nodes.selected_visible_and_unlocked_layers(&document.network_interface); + + let Some(first) = layers.next() else { return }; + let count = layers.count() + 1; + let transform = if count == 1 { + Self::get_layer_pivot_transform(first, document) + } else { + let [min, max] = document.selected_visible_and_unlock_layers_bounding_box_viewport().unwrap_or([DVec2::ZERO, DVec2::ONE]); + DAffine2::from_translation(min) * DAffine2::from_scale(max - min) + }; + + self.compass_center = transform.transform_point2(DVec2::splat(0.5)); } pub fn compass_rose_position(&self) -> DVec2 { diff --git a/editor/src/messages/tool/common_functionality/pivot.rs b/editor/src/messages/tool/common_functionality/pivot.rs index 2c924602..20a6774c 100644 --- a/editor/src/messages/tool/common_functionality/pivot.rs +++ b/editor/src/messages/tool/common_functionality/pivot.rs @@ -111,11 +111,12 @@ impl Pivot { .selected_visible_and_unlocked_layers(&document.network_interface) { let transform = Self::get_layer_pivot_transform(layer, document); + // Only update the pivot when computed position is finite. + if transform.matrix2.determinant().abs() <= f64::EPSILON { + return; + }; let pivot = transform.inverse().transform_point2(position); - // Only update the pivot when computed position is finite. Infinite can happen when scale is 0. - if pivot.is_finite() { - responses.add(GraphOperationMessage::TransformSetPivot { layer, pivot }); - } + responses.add(GraphOperationMessage::TransformSetPivot { layer, pivot }); } } diff --git a/editor/src/messages/tool/common_functionality/transformation_cage.rs b/editor/src/messages/tool/common_functionality/transformation_cage.rs index da129c9b..2ed88ae2 100644 --- a/editor/src/messages/tool/common_functionality/transformation_cage.rs +++ b/editor/src/messages/tool/common_functionality/transformation_cage.rs @@ -411,7 +411,7 @@ impl BoundingBoxManager { fn overlay_display_category(&self, quad: Quad) -> HandleDisplayCategory { // Check if the area is essentially zero because either the width or height is smaller than an epsilon - if (self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any() { + if self.is_bounds_flat() { return HandleDisplayCategory::Flat; } @@ -434,6 +434,10 @@ impl BoundingBoxManager { HandleDisplayCategory::Narrow } + fn is_bounds_flat(&self) -> bool { + (self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any() + } + /// Compute the threshold in viewport space. This only works with affine transforms as it assumes lines remain parallel. fn compute_viewport_threshold(&self, scalar: f64) -> [f64; 2] { let inverse = self.transform.inverse(); @@ -521,18 +525,18 @@ impl BoundingBoxManager { /// Gets the required mouse cursor to show resizing bounds or optionally rotation pub fn get_cursor(&self, input: &InputPreprocessorMessageHandler, rotate: bool) -> MouseCursorIcon { - if let Some((top, bottom, left, right)) = self.check_selected_edges(input.mouse.position) { - match (top, bottom, left, right) { + let edges = self.check_selected_edges(input.mouse.position); + + match edges { + Some((top, bottom, left, right)) if self.is_bounds_flat() => match (top, bottom, left, right) { (true, _, false, false) | (_, true, false, false) => MouseCursorIcon::NSResize, (false, false, true, _) | (false, false, _, true) => MouseCursorIcon::EWResize, (true, _, true, _) | (_, true, _, true) => MouseCursorIcon::NWSEResize, (true, _, _, true) | (_, true, true, _) => MouseCursorIcon::NESWResize, _ => MouseCursorIcon::Default, - } - } else if rotate && self.check_rotate(input.mouse.position) { - MouseCursorIcon::Rotate - } else { - MouseCursorIcon::Default + }, + _ if rotate && self.check_rotate(input.mouse.position) => MouseCursorIcon::Rotate, + _ => MouseCursorIcon::Default, } } } diff --git a/editor/src/messages/tool/tool_messages/select_tool.rs b/editor/src/messages/tool/tool_messages/select_tool.rs index 4b855965..5c970186 100644 --- a/editor/src/messages/tool/tool_messages/select_tool.rs +++ b/editor/src/messages/tool/tool_messages/select_tool.rs @@ -1,7 +1,10 @@ #![allow(clippy::too_many_arguments)] use super::tool_prelude::*; -use crate::consts::{COLOR_OVERLAY_BLUE, COLOR_OVERLAY_GREEN, COLOR_OVERLAY_RED, DRAG_DIRECTION_MODE_DETERMINATION_THRESHOLD, ROTATE_INCREMENT, SELECTION_DRAG_ANGLE, SELECTION_TOLERANCE}; +use crate::consts::{ + COLOR_OVERLAY_BLUE, COLOR_OVERLAY_GREEN, COLOR_OVERLAY_RED, COMPASS_ROSE_HOVER_RING_DIAMETER, DRAG_DIRECTION_MODE_DETERMINATION_THRESHOLD, ROTATE_INCREMENT, SELECTION_DRAG_ANGLE, + SELECTION_TOLERANCE, +}; use crate::messages::input_mapper::utility_types::input_mouse::ViewportPosition; use crate::messages::portfolio::document::graph_operation::utility_types::TransformIn; use crate::messages::portfolio::document::overlays::utility_types::OverlayContext; @@ -577,11 +580,11 @@ impl Fsm for SelectToolFsmState { let show_compass = !(can_get_into_other_states || is_resizing_or_rotating); let show_compass_with_ring = bounds.map(|bounds| transform * Quad::from_box(bounds)).and_then(|quad| { - show_compass + (show_compass && quad.all_sides_at_least_width(COMPASS_ROSE_HOVER_RING_DIAMETER)) .then_some( matches!(self, SelectToolFsmState::Dragging { .. }) .then_some(show_hover_ring) - .or(quad.contains(mouse_position).then_some(show_hover_ring)), + .or((quad.contains(mouse_position)).then_some(show_hover_ring)), ) .flatten() }); @@ -590,7 +593,7 @@ impl Fsm for SelectToolFsmState { tool_data.pivot.update_pivot(document, &mut overlay_context, angle); // Update compass rose - tool_data.compass_rose.refresh_transform(document); + tool_data.compass_rose.refresh_position(document); let compass_center = tool_data.compass_rose.compass_rose_position(); overlay_context.compass_rose(compass_center, angle, show_compass_with_ring); @@ -600,29 +603,31 @@ impl Fsm for SelectToolFsmState { compass_rose_state.axis_type().and_then(|axis| axis.is_constraint().then_some((axis, true))) }; - if let Some((axis, hover)) = axis_state { - if axis.is_constraint() { - let e0 = tool_data - .bounding_box_manager - .as_ref() - .map(|man| man.transform * Quad::from_box(man.bounds)) - .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X)); + if show_compass_with_ring.is_some() { + if let Some((axis, hover)) = axis_state { + if axis.is_constraint() { + let e0 = tool_data + .bounding_box_manager + .as_ref() + .map(|man| man.transform * Quad::from_box(man.bounds)) + .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X)); - let (direction, color) = match axis { - Axis::X => (e0, COLOR_OVERLAY_RED), - Axis::Y => (e0.perp(), COLOR_OVERLAY_GREEN), - _ => unreachable!(), - }; + let (direction, color) = match axis { + Axis::X => (e0, COLOR_OVERLAY_RED), + Axis::Y => (e0.perp(), COLOR_OVERLAY_GREEN), + _ => unreachable!(), + }; - let viewport_diagonal = input.viewport_bounds.size().length(); + let viewport_diagonal = input.viewport_bounds.size().length(); - let color = if !hover { - color - } else { - let color_string = &graphene_std::Color::from_rgb_str(color.strip_prefix('#').unwrap()).unwrap().with_alpha(0.25).rgba_hex(); - &format!("#{}", color_string) - }; - overlay_context.line(compass_center - direction * viewport_diagonal, compass_center + direction * viewport_diagonal, Some(color)); + let color = if !hover { + color + } else { + let color_string = &graphene_std::Color::from_rgb_str(color.strip_prefix('#').unwrap()).unwrap().with_alpha(0.25).rgba_hex(); + &format!("#{}", color_string) + }; + overlay_context.line(compass_center - direction * viewport_diagonal, compass_center + direction * viewport_diagonal, Some(color)); + } } } @@ -776,16 +781,24 @@ impl Fsm for SelectToolFsmState { // If the user clicks on a layer that is in their current selection, go into the dragging mode. // If the user clicks on new shape, make that layer their new selection. // Otherwise enter the box select mode + let bounds = tool_data.bounding_box_manager.as_ref().map(|man| man.transform * Quad::from_box(man.bounds)); - let angle = tool_data - .bounding_box_manager - .as_ref() - .map(|man| man.transform * Quad::from_box(man.bounds)) - .map_or(0., |quad| (quad.top_left() - quad.top_right()).to_angle()); + let angle = bounds.map_or(0., |quad| (quad.top_left() - quad.top_right()).to_angle()); let mouse_position = input.mouse.position; let compass_rose_state = tool_data.compass_rose.compass_rose_state(mouse_position, angle); let is_over_pivot = tool_data.pivot.is_over(mouse_position); + let show_compass = bounds.is_some_and(|quad| quad.all_sides_at_least_width(COMPASS_ROSE_HOVER_RING_DIAMETER) && quad.contains(mouse_position)); + let can_grab_compass_rose = compass_rose_state.can_grab() && show_compass; + let is_flat_layer = document + .network_interface + .selected_nodes(&[]) + .unwrap() + .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(layer)) + .is_none_or(|transform| transform.matrix2.determinant().abs() <= f64::EPSILON); + let state = // Dragging the pivot if is_over_pivot { @@ -796,43 +809,24 @@ impl Fsm for SelectToolFsmState { SelectToolFsmState::DraggingPivot } - // Dragging one (or two, forming a corner) of the transform cage bounding box edges - else if dragging_bounds.is_some() { + // Dragging the selected layers around to transform them + else if can_grab_compass_rose || intersection.is_some_and(|intersection| selected.iter().any(|selected_layer| intersection.starts_with(*selected_layer, document.metadata()))) { responses.add(DocumentMessage::StartTransaction); + if input.keyboard.key(select_deepest) || tool_data.nested_selection_behavior == NestedSelectionBehavior::Deepest { + tool_data.select_single_layer = intersection; + } else { + tool_data.select_single_layer = intersection.and_then(|intersection| intersection.ancestors(document.metadata()).find(|ancestor| selected.contains(ancestor))); + } + tool_data.layers_dragging = selected; - if let Some(bounds) = &mut tool_data.bounding_box_manager { - bounds.original_bound_transform = bounds.transform; - - tool_data.layers_dragging.retain(|layer| { - if *layer != LayerNodeIdentifier::ROOT_PARENT { - document.network_interface.network(&[]).unwrap().nodes.contains_key(&layer.to_node()) - } else { - log::error!("ROOT_PARENT should not be part of layers_dragging"); - false - } - }); - - let mut selected = Selected::new( - &mut bounds.original_transforms, - &mut bounds.center_of_transformation, - &tool_data.layers_dragging, - responses, - &document.network_interface, - None, - &ToolType::Select, - None - ); - bounds.center_of_transformation = selected.mean_average_of_pivots(); - } tool_data.get_snap_candidates(document, input); - - if input.keyboard.key(skew) { - SelectToolFsmState::SkewingBounds - }else{ - SelectToolFsmState::ResizingBounds - } + let (axis, using_compass) = { + let axis_state = compass_rose_state.axis_type().filter(|_| can_grab_compass_rose); + (axis_state.unwrap_or_default(), axis_state.is_some()) + }; + SelectToolFsmState::Dragging { axis, using_compass } } // Dragging near the transform cage bounding box to rotate it else if rotating_bounds { @@ -865,23 +859,42 @@ impl Fsm for SelectToolFsmState { SelectToolFsmState::RotatingBounds } - // Dragging the selected layers around to transform them - else if compass_rose_state.can_grab() || intersection.is_some_and(|intersection| selected.iter().any(|selected_layer| intersection.starts_with(*selected_layer, document.metadata()))) { + // Dragging one (or two, forming a corner) of the transform cage bounding box edges + else if dragging_bounds.is_some() && !is_flat_layer { responses.add(DocumentMessage::StartTransaction); - if input.keyboard.key(select_deepest) || tool_data.nested_selection_behavior == NestedSelectionBehavior::Deepest { - tool_data.select_single_layer = intersection; - } else { - tool_data.select_single_layer = intersection.and_then(|intersection| intersection.ancestors(document.metadata()).find(|ancestor| selected.contains(ancestor))); - } - tool_data.layers_dragging = selected; + if let Some(bounds) = &mut tool_data.bounding_box_manager { + bounds.original_bound_transform = bounds.transform; + + tool_data.layers_dragging.retain(|layer| { + if *layer != LayerNodeIdentifier::ROOT_PARENT { + document.network_interface.network(&[]).unwrap().nodes.contains_key(&layer.to_node()) + } else { + log::error!("ROOT_PARENT should not be part of layers_dragging"); + false + } + }); + + let mut selected = Selected::new( + &mut bounds.original_transforms, + &mut bounds.center_of_transformation, + &tool_data.layers_dragging, + responses, + &document.network_interface, + None, + &ToolType::Select, + None + ); + bounds.center_of_transformation = selected.mean_average_of_pivots(); + } tool_data.get_snap_candidates(document, input); - let axis = compass_rose_state.axis_type(); - match axis { - Some(axis) => SelectToolFsmState::Dragging { axis, using_compass: true }, - None => SelectToolFsmState::Dragging { axis: Axis::None, using_compass: false } + + if input.keyboard.key(skew) { + SelectToolFsmState::SkewingBounds + } else { + SelectToolFsmState::ResizingBounds } } // Dragging a selection box 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 d2aa875f..c282b848 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 @@ -357,8 +357,6 @@ impl MessageHandler> for TransformLayer || selected_layers.is_empty() || matches!(self.transform_operation, TransformOperation::Grabbing(_)) { - selected.original_transforms.clear(); - return; } @@ -384,7 +382,6 @@ impl MessageHandler> for TransformLayer || selected_layers.is_empty() || matches!(self.transform_operation, TransformOperation::Rotating(_)) { - selected.original_transforms.clear(); return; } @@ -438,7 +435,6 @@ impl MessageHandler> for TransformLayer || selected_layers.is_empty() || matches!(self.transform_operation, TransformOperation::Scaling(_)) { - selected.original_transforms.clear(); return; } diff --git a/node-graph/gcore/src/graphic_element/renderer.rs b/node-graph/gcore/src/graphic_element/renderer.rs index 7d543246..79427e11 100644 --- a/node-graph/gcore/src/graphic_element/renderer.rs +++ b/node-graph/gcore/src/graphic_element/renderer.rs @@ -90,7 +90,7 @@ impl ClickTarget { // This bounding box is not very accurate as it is the axis aligned version of the transformed bounding box. However it is fast. if !self .bounding_box - .is_some_and(|loose| intersects((layer_transform * Quad::from_box(loose)).bounding_box(), target_bounds)) + .is_some_and(|loose| (loose[0] - loose[1]).abs().cmpgt(DVec2::splat(1e-4)).all() && intersects((layer_transform * Quad::from_box(loose)).bounding_box(), target_bounds)) { return false; } diff --git a/node-graph/gcore/src/graphic_element/renderer/quad.rs b/node-graph/gcore/src/graphic_element/renderer/quad.rs index 6c460fab..9d5cd8f2 100644 --- a/node-graph/gcore/src/graphic_element/renderer/quad.rs +++ b/node-graph/gcore/src/graphic_element/renderer/quad.rs @@ -44,13 +44,23 @@ impl Quad { } /// Get all the edges in the quad. - pub fn edges(&self) -> [[DVec2; 2]; 4] { + pub fn all_edges(&self) -> [[DVec2; 2]; 4] { [[self.0[0], self.0[1]], [self.0[1], self.0[2]], [self.0[2], self.0[3]], [self.0[3], self.0[0]]] } + /// Get two edges as orthogonal bases. + pub fn edges(&self) -> [[DVec2; 2]; 2] { + [[self.0[0], self.0[1]], [self.0[1], self.0[2]]] + } + + /// Returns true only if the width and height are both greater than or equal to the given width. + pub fn all_sides_at_least_width(&self, width: f64) -> bool { + self.edges().into_iter().all(|[a, b]| (a - b).length_squared() >= width.powi(2)) + } + /// Get all the edges in the quad as linear bezier curves pub fn bezier_lines(&self) -> impl Iterator + '_ { - self.edges().into_iter().map(|[start, end]| bezier_rs::Bezier::from_linear_dvec2(start, end)) + self.all_edges().into_iter().map(|[start, end]| bezier_rs::Bezier::from_linear_dvec2(start, end)) } /// Generates the axis aligned bounding box of the quad @@ -126,9 +136,9 @@ impl Quad { pub fn intersects(&self, other: Quad) -> bool { let intersects = self - .edges() + .all_edges() .into_iter() - .any(|[a, b]| other.edges().into_iter().any(|[c, d]| Self::intersect_lines(a, b, c, d).is_some())); + .any(|[a, b]| other.all_edges().into_iter().any(|[c, d]| Self::intersect_lines(a, b, c, d).is_some())); self.contains(other.center()) || other.contains(self.center()) || intersects } }