Skip to content

Commit 38e542e

Browse files
mTvare6Keavon
mTvare6
and
Keavon
authored
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 <[email protected]>
1 parent 90a8036 commit 38e542e

File tree

2 files changed

+98
-63
lines changed

2 files changed

+98
-63
lines changed

‎editor/src/messages/tool/common_functionality/transformation_cage.rs

+47-15
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,22 @@ pub struct SelectedEdges {
3333
aspect_ratio: f64,
3434
}
3535

36+
/// The different possible configurations for how the transform cage is presently viewed, depending on its per-axis sizes and the level of zoom.
37+
/// See doc comments in each variant for a diagram of the configuration.
3638
#[derive(Clone, Debug, Default, PartialEq)]
37-
enum HandleDisplayCategory {
39+
enum TransformCageSizeCategory {
3840
#[default]
41+
/// - ![Diagram](https://files.keavon.com/-/OrganicHelplessWalleye/capture.png)
3942
Full,
43+
/// - ![Diagram](https://files.keavon.com/-/AnyGoldenrodHawk/capture.png)
4044
ReducedLandscape,
45+
/// - ![Diagram](https://files.keavon.com/-/DarkslategrayAcidicFirebelliedtoad/capture.png)
4146
ReducedPortrait,
47+
/// - ![Diagram](https://files.keavon.com/-/GlisteningComplexSeagull/capture.png)
4248
ReducedBoth,
49+
/// - ![Diagram](https://files.keavon.com/-/InconsequentialCharmingLynx/capture.png)
4350
Narrow,
51+
/// - ![Diagram](https://files.keavon.com/-/OpenPaleturquoiseArthropods/capture.png)
4452
Flat,
4553
}
4654

@@ -344,6 +352,7 @@ pub fn snap_drag(start: DVec2, current: DVec2, axis_align: bool, snap_data: Snap
344352
pub struct BoundingBoxManager {
345353
pub bounds: [DVec2; 2],
346354
pub transform: DAffine2,
355+
pub transform_tampered: bool,
347356
pub original_bound_transform: DAffine2,
348357
pub selected_edges: Option<SelectedEdges>,
349358
pub original_transforms: OriginalTransforms,
@@ -371,7 +380,7 @@ impl BoundingBoxManager {
371380
/// Update the position of the bounding box and transform handles
372381
pub fn render_overlays(&mut self, overlay_context: &mut OverlayContext) {
373382
let quad = self.transform * Quad::from_box(self.bounds);
374-
let category = self.overlay_display_category(quad);
383+
let category = self.overlay_display_category();
375384

376385
let horizontal_edges = [quad.top_right().midpoint(quad.bottom_right()), quad.bottom_left().midpoint(quad.top_left())];
377386
let vertical_edges = [quad.top_left().midpoint(quad.top_right()), quad.bottom_right().midpoint(quad.bottom_left())];
@@ -385,34 +394,43 @@ impl BoundingBoxManager {
385394
};
386395

387396
// Draw the horizontal midpoint drag handles
388-
if matches!(category, HandleDisplayCategory::Full | HandleDisplayCategory::Narrow | HandleDisplayCategory::ReducedLandscape) {
397+
if matches!(
398+
category,
399+
TransformCageSizeCategory::Full | TransformCageSizeCategory::Narrow | TransformCageSizeCategory::ReducedLandscape
400+
) {
389401
horizontal_edges.map(&mut draw_handle);
390402
}
391403

392404
// Draw the vertical midpoint drag handles
393-
if matches!(category, HandleDisplayCategory::Full | HandleDisplayCategory::Narrow | HandleDisplayCategory::ReducedPortrait) {
405+
if matches!(
406+
category,
407+
TransformCageSizeCategory::Full | TransformCageSizeCategory::Narrow | TransformCageSizeCategory::ReducedPortrait
408+
) {
394409
vertical_edges.map(&mut draw_handle);
395410
}
396411

397412
// Draw the corner drag handles
398413
if matches!(
399414
category,
400-
HandleDisplayCategory::Full | HandleDisplayCategory::ReducedBoth | HandleDisplayCategory::ReducedLandscape | HandleDisplayCategory::ReducedPortrait
415+
TransformCageSizeCategory::Full | TransformCageSizeCategory::ReducedBoth | TransformCageSizeCategory::ReducedLandscape | TransformCageSizeCategory::ReducedPortrait
401416
) {
402417
quad.0.map(&mut draw_handle);
403418
}
404419

405420
// Draw the flat line endpoint drag handles
406-
if category == HandleDisplayCategory::Flat {
421+
if category == TransformCageSizeCategory::Flat {
407422
draw_handle(self.transform.transform_point2(self.bounds[0]));
408423
draw_handle(self.transform.transform_point2(self.bounds[1]));
409424
}
410425
}
411426

412-
fn overlay_display_category(&self, quad: Quad) -> HandleDisplayCategory {
427+
/// Find the [`TransformCageSizeCategory`] of this bounding box based on size thresholds.
428+
fn overlay_display_category(&self) -> TransformCageSizeCategory {
429+
let quad = self.transform * Quad::from_box(self.bounds);
430+
413431
// Check if the area is essentially zero because either the width or height is smaller than an epsilon
414432
if self.is_bounds_flat() {
415-
return HandleDisplayCategory::Flat;
433+
return TransformCageSizeCategory::Flat;
416434
}
417435

418436
let vertical_length = (quad.top_left() - quad.top_right()).length_squared();
@@ -424,20 +442,27 @@ impl BoundingBoxManager {
424442
let horizontal_edge_visible = horizontal_length > MIN_LENGTH_FOR_MIDPOINT_VISIBILITY.powi(2);
425443

426444
return match (vertical_edge_visible, horizontal_edge_visible) {
427-
(true, true) => HandleDisplayCategory::Full,
428-
(true, false) => HandleDisplayCategory::ReducedPortrait,
429-
(false, true) => HandleDisplayCategory::ReducedLandscape,
430-
(false, false) => HandleDisplayCategory::ReducedBoth,
445+
(true, true) => TransformCageSizeCategory::Full,
446+
(true, false) => TransformCageSizeCategory::ReducedPortrait,
447+
(false, true) => TransformCageSizeCategory::ReducedLandscape,
448+
(false, false) => TransformCageSizeCategory::ReducedBoth,
431449
};
432450
}
433451

434-
HandleDisplayCategory::Narrow
452+
TransformCageSizeCategory::Narrow
435453
}
436454

455+
/// 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.
437456
fn is_bounds_flat(&self) -> bool {
438457
(self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any()
439458
}
440459

460+
/// Determine if the given point in viewport space falls within the bounds of `self`.
461+
fn is_contained_in_bounds(&self, point: DVec2) -> bool {
462+
let document_point = self.transform.inverse().transform_point2(point);
463+
Quad::from_box(self.bounds).contains(document_point)
464+
}
465+
441466
/// Compute the threshold in viewport space. This only works with affine transforms as it assumes lines remain parallel.
442467
fn compute_viewport_threshold(&self, scalar: f64) -> [f64; 2] {
443468
let inverse = self.transform.inverse();
@@ -476,6 +501,10 @@ impl BoundingBoxManager {
476501
let height = max.y - min.y;
477502

478503
if width < edge_min_x || height <= edge_min_y {
504+
if self.transform_tampered {
505+
return None;
506+
}
507+
479508
if min.x < cursor.x && cursor.x < max.x && cursor.y < max.y && cursor.y > min.y {
480509
return None;
481510
}
@@ -511,8 +540,11 @@ impl BoundingBoxManager {
511540

512541
/// Check if the user is rotating with the bounds
513542
pub fn check_rotate(&self, cursor: DVec2) -> bool {
514-
let cursor = self.transform.inverse().transform_point2(cursor);
543+
if self.is_contained_in_bounds(cursor) {
544+
return false;
545+
}
515546
let [threshold_x, threshold_y] = self.compute_viewport_threshold(BOUNDS_ROTATE_THRESHOLD);
547+
let cursor = self.transform.inverse().transform_point2(cursor);
516548

517549
let flat = (self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any();
518550
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 {
528560
let edges = self.check_selected_edges(input.mouse.position);
529561

530562
match edges {
531-
Some((top, bottom, left, right)) if self.is_bounds_flat() => match (top, bottom, left, right) {
563+
Some((top, bottom, left, right)) if !self.is_bounds_flat() => match (top, bottom, left, right) {
532564
(true, _, false, false) | (_, true, false, false) => MouseCursorIcon::NSResize,
533565
(false, false, true, _) | (false, false, _, true) => MouseCursorIcon::EWResize,
534566
(true, _, true, _) | (_, true, _, true) => MouseCursorIcon::NWSEResize,

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

+51-48
Original file line numberDiff line numberDiff line change
@@ -520,10 +520,12 @@ impl Fsm for SelectToolFsmState {
520520
.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
521521
.map(|layer| document.metadata().transform_to_viewport(layer));
522522

523-
// Check if the matrix is not invertible
524523
let mut transform = transform.unwrap_or(DAffine2::IDENTITY);
524+
let mut transform_tampered = false;
525+
// Check if the matrix is not invertible
525526
if transform.matrix2.determinant() == 0. {
526527
transform.matrix2 += DMat2::IDENTITY * 1e-4; // TODO: Is this the cleanest way to handle this?
528+
transform_tampered = true;
527529
}
528530

529531
let bounds = document
@@ -543,6 +545,7 @@ impl Fsm for SelectToolFsmState {
543545

544546
bounding_box_manager.bounds = bounds;
545547
bounding_box_manager.transform = transform;
548+
bounding_box_manager.transform_tampered = transform_tampered;
546549

547550
bounding_box_manager.render_overlays(&mut overlay_context);
548551
} else {
@@ -609,7 +612,7 @@ impl Fsm for SelectToolFsmState {
609612
let e0 = tool_data
610613
.bounding_box_manager
611614
.as_ref()
612-
.map(|man| man.transform * Quad::from_box(man.bounds))
615+
.map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds))
613616
.map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X));
614617

615618
let (direction, color) = match axis {
@@ -781,7 +784,10 @@ impl Fsm for SelectToolFsmState {
781784
// If the user clicks on a layer that is in their current selection, go into the dragging mode.
782785
// If the user clicks on new shape, make that layer their new selection.
783786
// Otherwise enter the box select mode
784-
let bounds = tool_data.bounding_box_manager.as_ref().map(|man| man.transform * Quad::from_box(man.bounds));
787+
let bounds = tool_data
788+
.bounding_box_manager
789+
.as_ref()
790+
.map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds));
785791

786792
let angle = bounds.map_or(0., |quad| (quad.top_left() - quad.top_right()).to_angle());
787793
let mouse_position = input.mouse.position;
@@ -790,14 +796,11 @@ impl Fsm for SelectToolFsmState {
790796

791797
let show_compass = bounds.is_some_and(|quad| quad.all_sides_at_least_width(COMPASS_ROSE_HOVER_RING_DIAMETER) && quad.contains(mouse_position));
792798
let can_grab_compass_rose = compass_rose_state.can_grab() && show_compass;
793-
let is_flat_layer = document
794-
.network_interface
795-
.selected_nodes(&[])
796-
.unwrap()
797-
.selected_visible_and_unlocked_layers(&document.network_interface)
798-
.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
799-
.map(|layer| document.metadata().transform_to_viewport(layer))
800-
.is_none_or(|transform| transform.matrix2.determinant().abs() <= f64::EPSILON);
799+
let is_flat_layer = tool_data
800+
.bounding_box_manager
801+
.as_ref()
802+
.map(|bounding_box_manager| bounding_box_manager.transform_tampered)
803+
.unwrap_or(true);
801804

802805
let state =
803806
// Dragging the pivot
@@ -809,30 +812,15 @@ impl Fsm for SelectToolFsmState {
809812

810813
SelectToolFsmState::DraggingPivot
811814
}
812-
// Dragging the selected layers around to transform them
813-
else if can_grab_compass_rose || intersection.is_some_and(|intersection| selected.iter().any(|selected_layer| intersection.starts_with(*selected_layer, document.metadata()))) {
815+
// Dragging one (or two, forming a corner) of the transform cage bounding box edges
816+
else if dragging_bounds.is_some() && !is_flat_layer {
814817
responses.add(DocumentMessage::StartTransaction);
815818

816-
if input.keyboard.key(select_deepest) || tool_data.nested_selection_behavior == NestedSelectionBehavior::Deepest {
817-
tool_data.select_single_layer = intersection;
818-
} else {
819-
tool_data.select_single_layer = intersection.and_then(|intersection| intersection.ancestors(document.metadata()).find(|ancestor| selected.contains(ancestor)));
820-
}
821-
822819
tool_data.layers_dragging = selected;
823820

824-
tool_data.get_snap_candidates(document, input);
825-
let (axis, using_compass) = {
826-
let axis_state = compass_rose_state.axis_type().filter(|_| can_grab_compass_rose);
827-
(axis_state.unwrap_or_default(), axis_state.is_some())
828-
};
829-
SelectToolFsmState::Dragging { axis, using_compass }
830-
}
831-
// Dragging near the transform cage bounding box to rotate it
832-
else if rotating_bounds {
833-
responses.add(DocumentMessage::StartTransaction);
834-
835821
if let Some(bounds) = &mut tool_data.bounding_box_manager {
822+
bounds.original_bound_transform = bounds.transform;
823+
836824
tool_data.layers_dragging.retain(|layer| {
837825
if *layer != LayerNodeIdentifier::ROOT_PARENT {
838826
document.network_interface.network(&[]).unwrap().nodes.contains_key(&layer.to_node())
@@ -841,33 +829,51 @@ impl Fsm for SelectToolFsmState {
841829
false
842830
}
843831
});
832+
844833
let mut selected = Selected::new(
845834
&mut bounds.original_transforms,
846835
&mut bounds.center_of_transformation,
847-
&selected,
836+
&tool_data.layers_dragging,
848837
responses,
849838
&document.network_interface,
850839
None,
851840
&ToolType::Select,
852841
None
853842
);
854-
855843
bounds.center_of_transformation = selected.mean_average_of_pivots();
856844
}
845+
tool_data.get_snap_candidates(document, input);
857846

858-
tool_data.layers_dragging = selected;
859-
860-
SelectToolFsmState::RotatingBounds
847+
if input.keyboard.key(skew) {
848+
SelectToolFsmState::SkewingBounds
849+
} else {
850+
SelectToolFsmState::ResizingBounds
851+
}
861852
}
862-
// Dragging one (or two, forming a corner) of the transform cage bounding box edges
863-
else if dragging_bounds.is_some() && !is_flat_layer {
853+
// Dragging the selected layers around to transform them
854+
else if can_grab_compass_rose || intersection.is_some_and(|intersection| selected.iter().any(|selected_layer| intersection.starts_with(*selected_layer, document.metadata()))) {
864855
responses.add(DocumentMessage::StartTransaction);
865856

857+
if input.keyboard.key(select_deepest) || tool_data.nested_selection_behavior == NestedSelectionBehavior::Deepest {
858+
tool_data.select_single_layer = intersection;
859+
} else {
860+
tool_data.select_single_layer = intersection.and_then(|intersection| intersection.ancestors(document.metadata()).find(|ancestor| selected.contains(ancestor)));
861+
}
862+
866863
tool_data.layers_dragging = selected;
867864

868-
if let Some(bounds) = &mut tool_data.bounding_box_manager {
869-
bounds.original_bound_transform = bounds.transform;
865+
tool_data.get_snap_candidates(document, input);
866+
let (axis, using_compass) = {
867+
let axis_state = compass_rose_state.axis_type().filter(|_| can_grab_compass_rose);
868+
(axis_state.unwrap_or_default(), axis_state.is_some())
869+
};
870+
SelectToolFsmState::Dragging { axis, using_compass }
871+
}
872+
// Dragging near the transform cage bounding box to rotate it
873+
else if rotating_bounds {
874+
responses.add(DocumentMessage::StartTransaction);
870875

876+
if let Some(bounds) = &mut tool_data.bounding_box_manager {
871877
tool_data.layers_dragging.retain(|layer| {
872878
if *layer != LayerNodeIdentifier::ROOT_PARENT {
873879
document.network_interface.network(&[]).unwrap().nodes.contains_key(&layer.to_node())
@@ -876,26 +882,23 @@ impl Fsm for SelectToolFsmState {
876882
false
877883
}
878884
});
879-
880885
let mut selected = Selected::new(
881886
&mut bounds.original_transforms,
882887
&mut bounds.center_of_transformation,
883-
&tool_data.layers_dragging,
888+
&selected,
884889
responses,
885890
&document.network_interface,
886891
None,
887892
&ToolType::Select,
888893
None
889894
);
895+
890896
bounds.center_of_transformation = selected.mean_average_of_pivots();
891897
}
892-
tool_data.get_snap_candidates(document, input);
893898

894-
if input.keyboard.key(skew) {
895-
SelectToolFsmState::SkewingBounds
896-
} else {
897-
SelectToolFsmState::ResizingBounds
898-
}
899+
tool_data.layers_dragging = selected;
900+
901+
SelectToolFsmState::RotatingBounds
899902
}
900903
// Dragging a selection box
901904
else {
@@ -953,7 +956,7 @@ impl Fsm for SelectToolFsmState {
953956
let e0 = tool_data
954957
.bounding_box_manager
955958
.as_ref()
956-
.map(|man| man.transform * Quad::from_box(man.bounds))
959+
.map(|bounding_box_manager| bounding_box_manager.transform * Quad::from_box(bounding_box_manager.bounds))
957960
.map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X));
958961
let mouse_delta = match axis {
959962
Axis::X => mouse_delta.project_onto(e0),

0 commit comments

Comments
 (0)