From 38e542e6c0c164ee94a19d8d8fb9edc346cb5683 Mon Sep 17 00:00:00 2001 From: mTvare Date: Thu, 20 Feb 2025 16:34:41 +0530 Subject: [PATCH] Fix regressions introduced in #2282 with the compass rose feature (#2298) * Fix mouse states and priority order of operation * Add metadata for tampered transform * Add comments explaining details * Improve comments * Move out of bounds checks into rotate check --------- Co-authored-by: Keavon Chambers --- .../transformation_cage.rs | 62 ++++++++--- .../tool/tool_messages/select_tool.rs | 103 +++++++++--------- 2 files changed, 100 insertions(+), 65 deletions(-) diff --git a/editor/src/messages/tool/common_functionality/transformation_cage.rs b/editor/src/messages/tool/common_functionality/transformation_cage.rs index 2ed88ae2..4c9f2985 100644 --- a/editor/src/messages/tool/common_functionality/transformation_cage.rs +++ b/editor/src/messages/tool/common_functionality/transformation_cage.rs @@ -33,14 +33,22 @@ pub struct SelectedEdges { aspect_ratio: f64, } +/// The different possible configurations for how the transform cage is presently viewed, depending on its per-axis sizes and the level of zoom. +/// See doc comments in each variant for a diagram of the configuration. #[derive(Clone, Debug, Default, PartialEq)] -enum HandleDisplayCategory { +enum TransformCageSizeCategory { #[default] + /// - ![Diagram](https://files.keavon.com/-/OrganicHelplessWalleye/capture.png) Full, + /// - ![Diagram](https://files.keavon.com/-/AnyGoldenrodHawk/capture.png) ReducedLandscape, + /// - ![Diagram](https://files.keavon.com/-/DarkslategrayAcidicFirebelliedtoad/capture.png) ReducedPortrait, + /// - ![Diagram](https://files.keavon.com/-/GlisteningComplexSeagull/capture.png) ReducedBoth, + /// - ![Diagram](https://files.keavon.com/-/InconsequentialCharmingLynx/capture.png) Narrow, + /// - ![Diagram](https://files.keavon.com/-/OpenPaleturquoiseArthropods/capture.png) Flat, } @@ -344,6 +352,7 @@ pub fn snap_drag(start: DVec2, current: DVec2, axis_align: bool, snap_data: Snap pub struct BoundingBoxManager { pub bounds: [DVec2; 2], pub transform: DAffine2, + pub transform_tampered: bool, pub original_bound_transform: DAffine2, pub selected_edges: Option, pub original_transforms: OriginalTransforms, @@ -371,7 +380,7 @@ impl BoundingBoxManager { /// Update the position of the bounding box and transform handles pub fn render_overlays(&mut self, overlay_context: &mut OverlayContext) { let quad = self.transform * Quad::from_box(self.bounds); - let category = self.overlay_display_category(quad); + let category = self.overlay_display_category(); let horizontal_edges = [quad.top_right().midpoint(quad.bottom_right()), quad.bottom_left().midpoint(quad.top_left())]; let vertical_edges = [quad.top_left().midpoint(quad.top_right()), quad.bottom_right().midpoint(quad.bottom_left())]; @@ -385,34 +394,43 @@ impl BoundingBoxManager { }; // Draw the horizontal midpoint drag handles - if matches!(category, HandleDisplayCategory::Full | HandleDisplayCategory::Narrow | HandleDisplayCategory::ReducedLandscape) { + if matches!( + category, + TransformCageSizeCategory::Full | TransformCageSizeCategory::Narrow | TransformCageSizeCategory::ReducedLandscape + ) { horizontal_edges.map(&mut draw_handle); } // Draw the vertical midpoint drag handles - if matches!(category, HandleDisplayCategory::Full | HandleDisplayCategory::Narrow | HandleDisplayCategory::ReducedPortrait) { + if matches!( + category, + TransformCageSizeCategory::Full | TransformCageSizeCategory::Narrow | TransformCageSizeCategory::ReducedPortrait + ) { vertical_edges.map(&mut draw_handle); } // Draw the corner drag handles if matches!( category, - HandleDisplayCategory::Full | HandleDisplayCategory::ReducedBoth | HandleDisplayCategory::ReducedLandscape | HandleDisplayCategory::ReducedPortrait + TransformCageSizeCategory::Full | TransformCageSizeCategory::ReducedBoth | TransformCageSizeCategory::ReducedLandscape | TransformCageSizeCategory::ReducedPortrait ) { quad.0.map(&mut draw_handle); } // Draw the flat line endpoint drag handles - if category == HandleDisplayCategory::Flat { + if category == TransformCageSizeCategory::Flat { draw_handle(self.transform.transform_point2(self.bounds[0])); draw_handle(self.transform.transform_point2(self.bounds[1])); } } - fn overlay_display_category(&self, quad: Quad) -> HandleDisplayCategory { + /// Find the [`TransformCageSizeCategory`] of this bounding box based on size thresholds. + fn overlay_display_category(&self) -> TransformCageSizeCategory { + let quad = self.transform * Quad::from_box(self.bounds); + // Check if the area is essentially zero because either the width or height is smaller than an epsilon if self.is_bounds_flat() { - return HandleDisplayCategory::Flat; + return TransformCageSizeCategory::Flat; } let vertical_length = (quad.top_left() - quad.top_right()).length_squared(); @@ -424,20 +442,27 @@ impl BoundingBoxManager { let horizontal_edge_visible = horizontal_length > MIN_LENGTH_FOR_MIDPOINT_VISIBILITY.powi(2); return match (vertical_edge_visible, horizontal_edge_visible) { - (true, true) => HandleDisplayCategory::Full, - (true, false) => HandleDisplayCategory::ReducedPortrait, - (false, true) => HandleDisplayCategory::ReducedLandscape, - (false, false) => HandleDisplayCategory::ReducedBoth, + (true, true) => TransformCageSizeCategory::Full, + (true, false) => TransformCageSizeCategory::ReducedPortrait, + (false, true) => TransformCageSizeCategory::ReducedLandscape, + (false, false) => TransformCageSizeCategory::ReducedBoth, }; } - HandleDisplayCategory::Narrow + TransformCageSizeCategory::Narrow } + /// Determine if these bounds are flat ([`TransformCageSizeCategory::Flat`]), which means that the width and/or height is essentially zero and the bounds are a line with effectively no area. This can happen on actual lines (axis-aligned, i.e. drawn horizontally or vertically) or when an element is scaled to zero in X or Y. A flat transform cage can still be rotated by a transformation, but its local space remains flat. fn is_bounds_flat(&self) -> bool { (self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any() } + /// Determine if the given point in viewport space falls within the bounds of `self`. + fn is_contained_in_bounds(&self, point: DVec2) -> bool { + let document_point = self.transform.inverse().transform_point2(point); + Quad::from_box(self.bounds).contains(document_point) + } + /// 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(); @@ -476,6 +501,10 @@ impl BoundingBoxManager { let height = max.y - min.y; if width < edge_min_x || height <= edge_min_y { + if self.transform_tampered { + return None; + } + if min.x < cursor.x && cursor.x < max.x && cursor.y < max.y && cursor.y > min.y { return None; } @@ -511,8 +540,11 @@ impl BoundingBoxManager { /// Check if the user is rotating with the bounds pub fn check_rotate(&self, cursor: DVec2) -> bool { - let cursor = self.transform.inverse().transform_point2(cursor); + if self.is_contained_in_bounds(cursor) { + return false; + } let [threshold_x, threshold_y] = self.compute_viewport_threshold(BOUNDS_ROTATE_THRESHOLD); + let cursor = self.transform.inverse().transform_point2(cursor); let flat = (self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any(); let within_square_bounds = |center: &DVec2| center.x - threshold_x < cursor.x && cursor.x < center.x + threshold_x && center.y - threshold_y < cursor.y && cursor.y < center.y + threshold_y; @@ -528,7 +560,7 @@ impl BoundingBoxManager { 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) { + 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, diff --git a/editor/src/messages/tool/tool_messages/select_tool.rs b/editor/src/messages/tool/tool_messages/select_tool.rs index 5c970186..7d315a3d 100644 --- a/editor/src/messages/tool/tool_messages/select_tool.rs +++ b/editor/src/messages/tool/tool_messages/select_tool.rs @@ -520,10 +520,12 @@ impl Fsm for SelectToolFsmState { .find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[])) .map(|layer| document.metadata().transform_to_viewport(layer)); - // Check if the matrix is not invertible let mut transform = transform.unwrap_or(DAffine2::IDENTITY); + let mut transform_tampered = false; + // Check if the matrix is not invertible if transform.matrix2.determinant() == 0. { transform.matrix2 += DMat2::IDENTITY * 1e-4; // TODO: Is this the cleanest way to handle this? + transform_tampered = true; } let bounds = document @@ -543,6 +545,7 @@ impl Fsm for SelectToolFsmState { bounding_box_manager.bounds = bounds; bounding_box_manager.transform = transform; + bounding_box_manager.transform_tampered = transform_tampered; bounding_box_manager.render_overlays(&mut overlay_context); } else { @@ -609,7 +612,7 @@ impl Fsm for SelectToolFsmState { let e0 = tool_data .bounding_box_manager .as_ref() - .map(|man| man.transform * Quad::from_box(man.bounds)) + .map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds)) .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X)); let (direction, color) = match axis { @@ -781,7 +784,10 @@ 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 bounds = tool_data + .bounding_box_manager + .as_ref() + .map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds)); let angle = bounds.map_or(0., |quad| (quad.top_left() - quad.top_right()).to_angle()); let mouse_position = input.mouse.position; @@ -790,14 +796,11 @@ impl Fsm for SelectToolFsmState { 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 is_flat_layer = tool_data + .bounding_box_manager + .as_ref() + .map(|bounding_box_manager| bounding_box_manager.transform_tampered) + .unwrap_or(true); let state = // Dragging the pivot @@ -809,6 +812,44 @@ 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() && !is_flat_layer { + responses.add(DocumentMessage::StartTransaction); + + 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 + } + } // 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); @@ -859,44 +900,6 @@ impl Fsm for SelectToolFsmState { SelectToolFsmState::RotatingBounds } - // 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); - - 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 - } - } // Dragging a selection box else { tool_data.layers_dragging = selected; @@ -953,7 +956,7 @@ impl Fsm for SelectToolFsmState { let e0 = tool_data .bounding_box_manager .as_ref() - .map(|man| man.transform * Quad::from_box(man.bounds)) + .map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds)) .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X)); let mouse_delta = match axis { Axis::X => mouse_delta.project_onto(e0),