Fix Shape tools bugs with stars/polygons with negative radii and circle radius click detection when viewport is zoomed (#2986)

* fixed spacing,click detection in radius gizmo fix drag in start point radiius gizmo

* Code review

---------

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

Author: 0SlowPoke0

editor/src/messages/tool/common_functionality/gizmos/shape_gizmos/circle_arc_radius_handle.rs

@@ -118,24 +118,29 @@ impl RadiusHandle {
 				let viewport = document.metadata().transform_to_viewport(layer);
 				let center = viewport.transform_point2(DVec2::ZERO);
 
-				let start_point = viewport.transform_point2(calculate_circle_point_position(0., radius)).distance(center);
-				let end_point = viewport.transform_point2(calculate_circle_point_position(FRAC_PI_2, radius)).distance(center);
+				let x_point = viewport.transform_point2(calculate_circle_point_position(0., radius));
+				let y_point = viewport.transform_point2(calculate_circle_point_position(FRAC_PI_2, radius));
+
+				let direction_x = viewport.transform_vector2(DVec2::X);
+				let direction_y = viewport.transform_vector2(-DVec2::Y);
 
 				if let Some(stroke_width) = get_stroke_width(layer, &document.network_interface) {
 					let spacing = Self::calculate_extra_spacing(viewport, radius, center, stroke_width, 15.);
-					let smaller_radius_x = (start_point - spacing).abs();
-					let smaller_radius_y = (end_point - spacing).abs();
+					let smaller_radius_x = (x_point - direction_x * spacing).distance(center);
+					let smaller_radius_y = (y_point - direction_y * spacing).distance(center);
 
-					let larger_radius_x = (start_point + spacing).abs();
-					let larger_radius_y = (end_point + spacing).abs();
+					let larger_radius_x = (x_point + direction_x * spacing).distance(center);
+					let larger_radius_y = (y_point + direction_y * spacing).distance(center);
 
 					overlay_context.dashed_ellipse(center, smaller_radius_x, smaller_radius_y, None, None, None, None, None, None, Some(4.), Some(4.), Some(0.5));
 					overlay_context.dashed_ellipse(center, larger_radius_x, larger_radius_y, None, None, None, None, None, None, Some(4.), Some(4.), Some(0.5));
 
 					return;
 				}
 
-				overlay_context.dashed_ellipse(center, start_point, end_point, None, None, None, None, None, None, Some(4.), Some(4.), Some(0.5));
+				let radius_x = x_point.distance(center);
+				let radius_y = y_point.distance(center);
+				overlay_context.dashed_ellipse(center, radius_x, radius_y, None, None, None, None, None, None, Some(4.), Some(4.), Some(0.5));
 			}
 		}
 	}
@@ -153,7 +158,7 @@ impl RadiusHandle {
 		let center = viewport_transform.transform_point2(DVec2::ZERO);
 
 		let delta_vector = viewport_transform.inverse().transform_point2(input.mouse.position) - viewport_transform.inverse().transform_point2(self.previous_mouse_position);
-		let radius = document.metadata().document_to_viewport.transform_point2(drag_start) - center;
+		let radius = drag_start - center;
 		let sign = radius.dot(delta_vector).signum();
 
 		let net_delta = delta_vector.length() * sign * self.initial_radius.signum();

editor/src/messages/tool/common_functionality/gizmos/shape_gizmos/number_of_points_dial.rs

@@ -189,8 +189,8 @@ impl NumberOfPointsDial {
 	}
 
 	pub fn update_number_of_sides(&self, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, responses: &mut VecDeque<Message>, drag_start: DVec2) {
-		let delta = input.mouse.position - document.metadata().document_to_viewport.transform_point2(drag_start);
-		let sign = (input.mouse.position.x - document.metadata().document_to_viewport.transform_point2(drag_start).x).signum();
+		let delta = input.mouse.position - drag_start;
+		let sign = (input.mouse.position.x - drag_start.x).signum();
 		let net_delta = (delta.length() / 25.).round() * sign;
 
 		let Some(layer) = self.layer else { return };

editor/src/messages/tool/common_functionality/gizmos/shape_gizmos/point_radius_handle.rs

@@ -142,14 +142,7 @@ impl PointRadiusHandle {
 		}
 	}
 
-	pub fn overlays(
-		&self,
-		selected_star_layer: Option<LayerNodeIdentifier>,
-		document: &DocumentMessageHandler,
-		input: &InputPreprocessorMessageHandler,
-		mouse_position: DVec2,
-		overlay_context: &mut OverlayContext,
-	) {
+	pub fn overlays(&self, selected_star_layer: Option<LayerNodeIdentifier>, document: &DocumentMessageHandler, input: &InputPreprocessorMessageHandler, overlay_context: &mut OverlayContext) {
 		match &self.handle_state {
 			PointRadiusHandleState::Inactive => {
 				let Some(layer) = selected_star_layer else { return };
@@ -161,25 +154,12 @@ impl PointRadiusHandle {
 					for i in 0..(2 * sides) {
 						let point = star_vertex_position(viewport, i as i32, sides, radius1, radius2);
 						let center = viewport.transform_point2(DVec2::ZERO);
-						let viewport_diagonal = input.viewport_bounds.size().length();
 
 						// If the user zooms out such that shape is very small hide the gizmo
 						if point.distance(center) < GIZMO_HIDE_THRESHOLD {
 							return;
 						}
 
-						if point.distance(mouse_position) < 5. {
-							let Some(direction) = (point - center).try_normalize() else { continue };
-
-							overlay_context.manipulator_handle(point, true, None);
-							let angle = ((i as f64) * PI) / (sides as f64);
-							overlay_context.line(center, center + direction * viewport_diagonal, None, None);
-
-							draw_snapping_ticks(&self.snap_radii, direction, viewport, angle, overlay_context);
-
-							return;
-						}
-
 						overlay_context.manipulator_handle(point, false, None);
 					}
 				}
@@ -191,22 +171,12 @@ impl PointRadiusHandle {
 					for i in 0..sides {
 						let point = polygon_vertex_position(viewport, i as i32, sides, radius);
 						let center = viewport.transform_point2(DVec2::ZERO);
-						let viewport_diagonal = input.viewport_bounds.size().length();
 
 						// If the user zooms out such that shape is very small hide the gizmo
 						if point.distance(center) < GIZMO_HIDE_THRESHOLD {
 							return;
 						}
 
-						if point.distance(mouse_position) < 5. {
-							let Some(direction) = (point - center).try_normalize() else { continue };
-
-							overlay_context.manipulator_handle(point, true, None);
-							overlay_context.line(center, center + direction * viewport_diagonal, None, None);
-
-							return;
-						}
-
 						overlay_context.manipulator_handle(point, false, None);
 					}
 				}
@@ -232,12 +202,9 @@ impl PointRadiusHandle {
 					star_outline(Some(layer), document, overlay_context);
 
 					// Make the ticks for snapping
-
-					// If dragging to make radius negative don't show the
-					if (mouse_position - center).dot(direction) < 0. {
-						return;
+					if (radius1.signum() * radius2.signum()).is_sign_positive() {
+						draw_snapping_ticks(&self.snap_radii, direction, viewport, angle, overlay_context);
 					}
-					draw_snapping_ticks(&self.snap_radii, direction, viewport, angle, overlay_context);
 
 					return;
 				}
@@ -368,25 +335,36 @@ impl PointRadiusHandle {
 			return snap_radii;
 		};
 
-		let other_index = if radius_index == 3 { 2 } else { 3 };
-
-		let Some(&TaggedValue::F64(other_radius)) = node_inputs[other_index].as_value() else {
+		let (Some(&TaggedValue::F64(radius_1)), Some(&TaggedValue::F64(radius_2))) = (node_inputs[2].as_value(), node_inputs[3].as_value()) else {
 			return snap_radii;
 		};
+
+		let other_radius = if radius_index == 3 { radius_1 } else { radius_2 };
+
 		let Some(&TaggedValue::U32(sides)) = node_inputs[1].as_value() else {
 			return snap_radii;
 		};
 
+		let both_radii_negative = radius_1.is_sign_negative() && radius_2.is_sign_negative();
+		let both_radii_same_sign = (radius_1.signum() * radius_2.signum()).is_sign_positive();
+
+		// When only one of the radii is negative, no need for snapping
+		if !both_radii_same_sign {
+			return snap_radii;
+		}
+
+		let sign = if both_radii_negative { -1. } else { 1. };
+
 		// Inner radius for 90°
 		let b = FRAC_PI_4 * 3. - PI / (sides as f64);
 		let angle = b.sin();
-		let required_radius = (other_radius / angle) * FRAC_1_SQRT_2;
+		let required_radius = (other_radius.abs() * sign / angle) * FRAC_1_SQRT_2;
 
 		snap_radii.push(required_radius);
 
 		// Also push the case when the when it length increases more than the other
 
-		let flipped = other_radius * angle * SQRT_2;
+		let flipped = other_radius.abs() * sign * angle * SQRT_2;
 
 		snap_radii.push(flipped);
 
@@ -401,11 +379,11 @@ impl PointRadiusHandle {
 				break;
 			}
 
-			if other_radius * factor > 1e-6 {
-				snap_radii.push(other_radius * factor);
+			if other_radius.abs() * factor > 1e-6 {
+				snap_radii.push(other_radius.abs() * sign * factor);
 			}
 
-			snap_radii.push((other_radius * 1.) / factor);
+			snap_radii.push((other_radius.abs() * sign) / factor);
 		}
 
 		snap_radii
@@ -441,21 +419,23 @@ impl PointRadiusHandle {
 		};
 
 		let viewport_transform = document.network_interface.document_metadata().transform_to_viewport(layer);
-		let document_transform = document.network_interface.document_metadata().transform_to_document(layer);
 		let center = viewport_transform.transform_point2(DVec2::ZERO);
 		let radius_index = self.radius_index;
 
 		let original_radius = self.initial_radius;
 
-		let delta = viewport_transform.inverse().transform_point2(input.mouse.position) - document_transform.inverse().transform_point2(drag_start);
-		let radius = document.metadata().document_to_viewport.transform_point2(drag_start) - center;
+		let delta = viewport_transform.inverse().transform_point2(input.mouse.position) - viewport_transform.inverse().transform_point2(drag_start);
+		let radius = drag_start - center;
 		let projection = delta.project_onto(radius);
 		let sign = radius.dot(delta).signum();
 
-		let mut net_delta = projection.length() * sign;
+		let mut net_delta = projection.length() * sign * original_radius.signum();
 		let new_radius = original_radius + net_delta;
 
 		self.update_state(PointRadiusHandleState::Dragging);
+
+		self.check_if_radius_flipped(original_radius, new_radius, document, layer, radius_index);
+
 		if let Some((index, snapped_delta)) = self.check_snapping(new_radius, original_radius) {
 			net_delta = snapped_delta;
 			self.update_state(PointRadiusHandleState::Snapped(index));
@@ -467,4 +447,23 @@ impl PointRadiusHandle {
 		});
 		responses.add(NodeGraphMessage::RunDocumentGraph);
 	}
+
+	fn check_if_radius_flipped(&mut self, original_radius: f64, new_radius: f64, document: &DocumentMessageHandler, layer: LayerNodeIdentifier, radius_index: usize) {
+		let Some(node_inputs) = NodeGraphLayer::new(layer, &document.network_interface).find_node_inputs("Star") else {
+			return;
+		};
+
+		let (Some(&TaggedValue::F64(radius_1)), Some(&TaggedValue::F64(radius_2))) = (node_inputs[2].as_value(), node_inputs[3].as_value()) else {
+			return;
+		};
+
+		let other_radius = if radius_index == 3 { radius_1 } else { radius_2 };
+
+		let flipped = (other_radius.is_sign_positive() && original_radius.is_sign_negative() && new_radius.is_sign_positive())
+			|| (other_radius.is_sign_negative() && original_radius.is_sign_positive() && new_radius.is_sign_negative());
+
+		if flipped {
+			self.snap_radii = Self::calculate_snap_radii(document, layer, radius_index);
+		}
+	}
 }

editor/src/messages/tool/common_functionality/shapes/polygon_shape.rs

@@ -67,7 +67,7 @@ impl ShapeGizmoHandler for PolygonGizmoHandler {
 		overlay_context: &mut OverlayContext,
 	) {
 		self.number_of_points_dial.overlays(document, selected_polygon_layer, shape_editor, mouse_position, overlay_context);
-		self.point_radius_handle.overlays(selected_polygon_layer, document, input, mouse_position, overlay_context);
+		self.point_radius_handle.overlays(selected_polygon_layer, document, input, overlay_context);
 
 		polygon_outline(selected_polygon_layer, document, overlay_context);
 	}
@@ -85,7 +85,7 @@ impl ShapeGizmoHandler for PolygonGizmoHandler {
 		}
 
 		if self.point_radius_handle.is_dragging_or_snapped() {
-			self.point_radius_handle.overlays(None, document, input, mouse_position, overlay_context);
+			self.point_radius_handle.overlays(None, document, input, overlay_context);
 		}
 	}
 

editor/src/messages/tool/common_functionality/shapes/star_shape.rs

@@ -64,7 +64,7 @@ impl ShapeGizmoHandler for StarGizmoHandler {
 		overlay_context: &mut OverlayContext,
 	) {
 		self.number_of_points_dial.overlays(document, selected_star_layer, shape_editor, mouse_position, overlay_context);
-		self.point_radius_handle.overlays(selected_star_layer, document, input, mouse_position, overlay_context);
+		self.point_radius_handle.overlays(selected_star_layer, document, input, overlay_context);
 
 		star_outline(selected_star_layer, document, overlay_context);
 	}
@@ -82,7 +82,7 @@ impl ShapeGizmoHandler for StarGizmoHandler {
 		}
 
 		if self.point_radius_handle.is_dragging_or_snapped() {
-			self.point_radius_handle.overlays(None, document, input, mouse_position, overlay_context);
+			self.point_radius_handle.overlays(None, document, input, overlay_context);
 		}
 	}
 

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

@@ -743,7 +743,7 @@ impl Fsm for ShapeToolFsmState {
 				self
 			}
 			(ShapeToolFsmState::ModifyingGizmo, ShapeToolMessage::PointerMove(..)) => {
-				tool_data.gizmo_manager.handle_update(tool_data.data.drag_start, document, input, responses);
+				tool_data.gizmo_manager.handle_update(tool_data.data.viewport_drag_start(document), document, input, responses);
 
 				responses.add(OverlaysMessage::Draw);