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 <keavon@keavon.com>
This commit is contained in:
mTvare 2025-02-20 16:34:41 +05:30 committed by GitHub
parent 90a8036c47
commit 38e542e6c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 100 additions and 65 deletions

View File

@ -33,14 +33,22 @@ pub struct SelectedEdges {
aspect_ratio: f64, 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)] #[derive(Clone, Debug, Default, PartialEq)]
enum HandleDisplayCategory { enum TransformCageSizeCategory {
#[default] #[default]
/// - ![Diagram](https://files.keavon.com/-/OrganicHelplessWalleye/capture.png)
Full, Full,
/// - ![Diagram](https://files.keavon.com/-/AnyGoldenrodHawk/capture.png)
ReducedLandscape, ReducedLandscape,
/// - ![Diagram](https://files.keavon.com/-/DarkslategrayAcidicFirebelliedtoad/capture.png)
ReducedPortrait, ReducedPortrait,
/// - ![Diagram](https://files.keavon.com/-/GlisteningComplexSeagull/capture.png)
ReducedBoth, ReducedBoth,
/// - ![Diagram](https://files.keavon.com/-/InconsequentialCharmingLynx/capture.png)
Narrow, Narrow,
/// - ![Diagram](https://files.keavon.com/-/OpenPaleturquoiseArthropods/capture.png)
Flat, Flat,
} }
@ -344,6 +352,7 @@ pub fn snap_drag(start: DVec2, current: DVec2, axis_align: bool, snap_data: Snap
pub struct BoundingBoxManager { pub struct BoundingBoxManager {
pub bounds: [DVec2; 2], pub bounds: [DVec2; 2],
pub transform: DAffine2, pub transform: DAffine2,
pub transform_tampered: bool,
pub original_bound_transform: DAffine2, pub original_bound_transform: DAffine2,
pub selected_edges: Option<SelectedEdges>, pub selected_edges: Option<SelectedEdges>,
pub original_transforms: OriginalTransforms, pub original_transforms: OriginalTransforms,
@ -371,7 +380,7 @@ impl BoundingBoxManager {
/// Update the position of the bounding box and transform handles /// Update the position of the bounding box and transform handles
pub fn render_overlays(&mut self, overlay_context: &mut OverlayContext) { pub fn render_overlays(&mut self, overlay_context: &mut OverlayContext) {
let quad = self.transform * Quad::from_box(self.bounds); 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 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())]; 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 // 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); horizontal_edges.map(&mut draw_handle);
} }
// Draw the vertical midpoint drag handles // 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); vertical_edges.map(&mut draw_handle);
} }
// Draw the corner drag handles // Draw the corner drag handles
if matches!( if matches!(
category, category,
HandleDisplayCategory::Full | HandleDisplayCategory::ReducedBoth | HandleDisplayCategory::ReducedLandscape | HandleDisplayCategory::ReducedPortrait TransformCageSizeCategory::Full | TransformCageSizeCategory::ReducedBoth | TransformCageSizeCategory::ReducedLandscape | TransformCageSizeCategory::ReducedPortrait
) { ) {
quad.0.map(&mut draw_handle); quad.0.map(&mut draw_handle);
} }
// Draw the flat line endpoint drag handles // 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[0]));
draw_handle(self.transform.transform_point2(self.bounds[1])); 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 // Check if the area is essentially zero because either the width or height is smaller than an epsilon
if self.is_bounds_flat() { if self.is_bounds_flat() {
return HandleDisplayCategory::Flat; return TransformCageSizeCategory::Flat;
} }
let vertical_length = (quad.top_left() - quad.top_right()).length_squared(); 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); let horizontal_edge_visible = horizontal_length > MIN_LENGTH_FOR_MIDPOINT_VISIBILITY.powi(2);
return match (vertical_edge_visible, horizontal_edge_visible) { return match (vertical_edge_visible, horizontal_edge_visible) {
(true, true) => HandleDisplayCategory::Full, (true, true) => TransformCageSizeCategory::Full,
(true, false) => HandleDisplayCategory::ReducedPortrait, (true, false) => TransformCageSizeCategory::ReducedPortrait,
(false, true) => HandleDisplayCategory::ReducedLandscape, (false, true) => TransformCageSizeCategory::ReducedLandscape,
(false, false) => HandleDisplayCategory::ReducedBoth, (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 { fn is_bounds_flat(&self) -> bool {
(self.bounds[0] - self.bounds[1]).abs().cmple(DVec2::splat(1e-4)).any() (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. /// 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] { fn compute_viewport_threshold(&self, scalar: f64) -> [f64; 2] {
let inverse = self.transform.inverse(); let inverse = self.transform.inverse();
@ -476,6 +501,10 @@ impl BoundingBoxManager {
let height = max.y - min.y; let height = max.y - min.y;
if width < edge_min_x || height <= edge_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 { if min.x < cursor.x && cursor.x < max.x && cursor.y < max.y && cursor.y > min.y {
return None; return None;
} }
@ -511,8 +540,11 @@ impl BoundingBoxManager {
/// Check if the user is rotating with the bounds /// Check if the user is rotating with the bounds
pub fn check_rotate(&self, cursor: DVec2) -> bool { 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 [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 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; 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); let edges = self.check_selected_edges(input.mouse.position);
match edges { 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, (true, _, false, false) | (_, true, false, false) => MouseCursorIcon::NSResize,
(false, false, true, _) | (false, false, _, true) => MouseCursorIcon::EWResize, (false, false, true, _) | (false, false, _, true) => MouseCursorIcon::EWResize,
(true, _, true, _) | (_, true, _, true) => MouseCursorIcon::NWSEResize, (true, _, true, _) | (_, true, _, true) => MouseCursorIcon::NWSEResize,

View File

@ -520,10 +520,12 @@ impl Fsm for SelectToolFsmState {
.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[])) .find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
.map(|layer| document.metadata().transform_to_viewport(layer)); .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 = transform.unwrap_or(DAffine2::IDENTITY);
let mut transform_tampered = false;
// Check if the matrix is not invertible
if transform.matrix2.determinant() == 0. { if transform.matrix2.determinant() == 0. {
transform.matrix2 += DMat2::IDENTITY * 1e-4; // TODO: Is this the cleanest way to handle this? transform.matrix2 += DMat2::IDENTITY * 1e-4; // TODO: Is this the cleanest way to handle this?
transform_tampered = true;
} }
let bounds = document let bounds = document
@ -543,6 +545,7 @@ impl Fsm for SelectToolFsmState {
bounding_box_manager.bounds = bounds; bounding_box_manager.bounds = bounds;
bounding_box_manager.transform = transform; bounding_box_manager.transform = transform;
bounding_box_manager.transform_tampered = transform_tampered;
bounding_box_manager.render_overlays(&mut overlay_context); bounding_box_manager.render_overlays(&mut overlay_context);
} else { } else {
@ -609,7 +612,7 @@ impl Fsm for SelectToolFsmState {
let e0 = tool_data let e0 = tool_data
.bounding_box_manager .bounding_box_manager
.as_ref() .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)); .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X));
let (direction, color) = match axis { 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 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. // If the user clicks on new shape, make that layer their new selection.
// Otherwise enter the box select mode // 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 angle = bounds.map_or(0., |quad| (quad.top_left() - quad.top_right()).to_angle());
let mouse_position = input.mouse.position; 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 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 can_grab_compass_rose = compass_rose_state.can_grab() && show_compass;
let is_flat_layer = document let is_flat_layer = tool_data
.network_interface .bounding_box_manager
.selected_nodes(&[]) .as_ref()
.unwrap() .map(|bounding_box_manager| bounding_box_manager.transform_tampered)
.selected_visible_and_unlocked_layers(&document.network_interface) .unwrap_or(true);
.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 = let state =
// Dragging the pivot // Dragging the pivot
@ -809,6 +812,44 @@ impl Fsm for SelectToolFsmState {
SelectToolFsmState::DraggingPivot 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 // 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()))) { 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); responses.add(DocumentMessage::StartTransaction);
@ -859,44 +900,6 @@ impl Fsm for SelectToolFsmState {
SelectToolFsmState::RotatingBounds 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 // Dragging a selection box
else { else {
tool_data.layers_dragging = selected; tool_data.layers_dragging = selected;
@ -953,7 +956,7 @@ impl Fsm for SelectToolFsmState {
let e0 = tool_data let e0 = tool_data
.bounding_box_manager .bounding_box_manager
.as_ref() .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)); .map_or(DVec2::X, |quad| (quad.top_left() - quad.top_right()).normalize_or(DVec2::X));
let mouse_delta = match axis { let mouse_delta = match axis {
Axis::X => mouse_delta.project_onto(e0), Axis::X => mouse_delta.project_onto(e0),