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 <keavon@keavon.com>
This commit is contained in:
0SlowPoke0 2025-08-04 06:44:28 +05:30 committed by GitHub
parent 5637f01845
commit fd66f29853
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 67 additions and 63 deletions

View File

@ -118,16 +118,19 @@ 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));
@ -135,7 +138,9 @@ impl RadiusHandle {
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();

View File

@ -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 };

View File

@ -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);
}
}
}

View File

@ -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);
}
}

View File

@ -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);
}
}

View File

@ -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);