Make the Path tool only allow selecting points that are visible (#2668)

* Fix only visible points selection in point selection

* Fix comments

* Remove bug from box selection and lasso

* Code review

* Fix comment

---------

Co-authored-by: Keavon Chambers <keavon@keavon.com>
This commit is contained in:
Adesh Gupta 2025-05-28 14:54:23 +05:30 committed by GitHub
parent f6e592da5b
commit b564579362
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 216 additions and 28 deletions

View file

@ -1,5 +1,6 @@
use super::utility_types::{DrawHandles, OverlayContext};
use crate::consts::HIDE_HANDLE_DISTANCE;
use crate::messages::portfolio::document::utility_types::network_interface::NodeNetworkInterface;
use crate::messages::tool::common_functionality::shape_editor::{SelectedLayerState, ShapeState};
use crate::messages::tool::tool_messages::tool_prelude::{DocumentMessageHandler, PreferencesMessageHandler};
use bezier_rs::{Bezier, BezierHandles};
@ -23,7 +24,7 @@ pub fn overlay_canvas_context() -> web_sys::CanvasRenderingContext2d {
create_context().expect("Failed to get canvas context")
}
pub fn selected_segments(document: &DocumentMessageHandler, shape_editor: &mut ShapeState) -> Vec<SegmentId> {
pub fn selected_segments(network_interface: &NodeNetworkInterface, shape_editor: &ShapeState) -> Vec<SegmentId> {
let selected_points = shape_editor.selected_points();
let selected_anchors = selected_points
.filter_map(|point_id| if let ManipulatorPointId::Anchor(p) = point_id { Some(*p) } else { None })
@ -40,8 +41,8 @@ pub fn selected_segments(document: &DocumentMessageHandler, shape_editor: &mut S
// TODO: Currently if there are two duplicate layers, both of their segments get overlays
// Adding segments which are are connected to selected anchors
for layer in document.network_interface.selected_nodes().selected_layers(document.metadata()) {
let Some(vector_data) = document.network_interface.compute_modified_vector(layer) else { continue };
for layer in network_interface.selected_nodes().selected_layers(network_interface.document_metadata()) {
let Some(vector_data) = network_interface.compute_modified_vector(layer) else { continue };
for (segment_id, _bezier, start, end) in vector_data.segment_bezier_iter() {
if selected_anchors.contains(&start) || selected_anchors.contains(&end) {

View file

@ -2,12 +2,14 @@ use super::graph_modification_utils::{self, merge_layers};
use super::snapping::{SnapCache, SnapCandidatePoint, SnapData, SnapManager, SnappedPoint};
use super::utility_functions::calculate_segment_angle;
use crate::consts::HANDLE_LENGTH_FACTOR;
use crate::messages::portfolio::document::overlays::utility_functions::selected_segments;
use crate::messages::portfolio::document::utility_types::document_metadata::{DocumentMetadata, LayerNodeIdentifier};
use crate::messages::portfolio::document::utility_types::misc::{PathSnapSource, SnapSource};
use crate::messages::portfolio::document::utility_types::network_interface::NodeNetworkInterface;
use crate::messages::prelude::*;
use crate::messages::tool::common_functionality::snapping::SnapTypeConfiguration;
use crate::messages::tool::tool_messages::path_tool::PointSelectState;
use crate::messages::tool::common_functionality::utility_functions::is_visible_point;
use crate::messages::tool::tool_messages::path_tool::{PathOverlayMode, PointSelectState};
use bezier_rs::{Bezier, BezierHandles, Subpath, TValue};
use glam::{DAffine2, DVec2};
use graphene_core::transform::Transform;
@ -421,12 +423,20 @@ impl ShapeState {
/// Select/deselect the first point within the selection threshold.
/// Returns a tuple of the points if found and the offset, or `None` otherwise.
pub fn change_point_selection(&mut self, network_interface: &NodeNetworkInterface, mouse_position: DVec2, select_threshold: f64, extend_selection: bool) -> Option<Option<SelectedPointsInfo>> {
pub fn change_point_selection(
&mut self,
network_interface: &NodeNetworkInterface,
mouse_position: DVec2,
select_threshold: f64,
extend_selection: bool,
path_overlay_mode: PathOverlayMode,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
) -> Option<Option<SelectedPointsInfo>> {
if self.selected_shape_state.is_empty() {
return None;
}
if let Some((layer, manipulator_point_id)) = self.find_nearest_point_indices(network_interface, mouse_position, select_threshold) {
if let Some((layer, manipulator_point_id)) = self.find_nearest_visible_point_indices(network_interface, mouse_position, select_threshold, path_overlay_mode, frontier_handles_info) {
let vector_data = network_interface.compute_modified_vector(layer)?;
let point_position = manipulator_point_id.get_position(&vector_data)?;
@ -467,7 +477,14 @@ impl ShapeState {
None
}
pub fn get_point_selection_state(&mut self, network_interface: &NodeNetworkInterface, mouse_position: DVec2, select_threshold: f64) -> Option<(bool, Option<SelectedPointsInfo>)> {
pub fn get_point_selection_state(
&mut self,
network_interface: &NodeNetworkInterface,
mouse_position: DVec2,
select_threshold: f64,
path_overlay_mode: PathOverlayMode,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
) -> Option<(bool, Option<SelectedPointsInfo>)> {
if self.selected_shape_state.is_empty() {
return None;
}
@ -476,6 +493,13 @@ impl ShapeState {
let vector_data = network_interface.compute_modified_vector(layer)?;
let point_position = manipulator_point_id.get_position(&vector_data)?;
// Check if point is visible under current overlay mode or not
let selected_segments = selected_segments(network_interface, self);
let selected_points = self.selected_points().cloned().collect::<HashSet<_>>();
if !is_visible_point(manipulator_point_id, &vector_data, path_overlay_mode, frontier_handles_info, selected_segments, &selected_points) {
return None;
}
let selected_shape_state = self.selected_shape_state.get(&layer)?;
let already_selected = selected_shape_state.is_selected(manipulator_point_id);
@ -1320,6 +1344,42 @@ impl ShapeState {
None
}
pub fn find_nearest_visible_point_indices(
&mut self,
network_interface: &NodeNetworkInterface,
mouse_position: DVec2,
select_threshold: f64,
path_overlay_mode: PathOverlayMode,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
) -> Option<(LayerNodeIdentifier, ManipulatorPointId)> {
if self.selected_shape_state.is_empty() {
return None;
}
let select_threshold_squared = select_threshold.powi(2);
// Find the closest control point among all elements of shapes_to_modify
for &layer in self.selected_shape_state.keys() {
if let Some((manipulator_point_id, distance_squared)) = Self::closest_point_in_layer(network_interface, layer, mouse_position) {
// Choose the first point under the threshold
if distance_squared < select_threshold_squared {
// Check if point is visible in current PathOverlayMode
let vector_data = network_interface.compute_modified_vector(layer)?;
let selected_segments = selected_segments(network_interface, self);
let selected_points = self.selected_points().cloned().collect::<HashSet<_>>();
if !is_visible_point(manipulator_point_id, &vector_data, path_overlay_mode, frontier_handles_info, selected_segments, &selected_points) {
return None;
}
return Some((layer, manipulator_point_id));
}
}
}
None
}
// TODO Use quadtree or some equivalent spatial acceleration structure to improve this to O(log(n))
/// Find the closest manipulator, manipulator point, and distance so we can select path elements.
/// Brute force comparison to determine which manipulator (handle or anchor) we want to select taking O(n) time.
@ -1623,7 +1683,17 @@ impl ShapeState {
false
}
pub fn select_all_in_shape(&mut self, network_interface: &NodeNetworkInterface, selection_shape: SelectionShape, selection_change: SelectionChange) {
pub fn select_all_in_shape(
&mut self,
network_interface: &NodeNetworkInterface,
selection_shape: SelectionShape,
selection_change: SelectionChange,
path_overlay_mode: PathOverlayMode,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
) {
let selected_points = self.selected_points().cloned().collect::<HashSet<_>>();
let selected_segments = selected_segments(network_interface, self);
for (&layer, state) in &mut self.selected_shape_state {
if selection_change == SelectionChange::Clear {
state.clear_points()
@ -1666,13 +1736,17 @@ impl ShapeState {
};
if select {
match selection_change {
SelectionChange::Shrink => state.deselect_point(id),
_ => {
// Select only the handles which are of nonzero length
if let Some(handle) = id.as_handle() {
if handle.length(&vector_data) > 0. {
state.select_point(id)
let is_visible_handle = is_visible_point(id, &vector_data, path_overlay_mode, frontier_handles_info.clone(), selected_segments.clone(), &selected_points);
if is_visible_handle {
match selection_change {
SelectionChange::Shrink => state.deselect_point(id),
_ => {
// Select only the handles which are of nonzero length
if let Some(handle) = id.as_handle() {
if handle.length(&vector_data) > 0. {
state.select_point(id)
}
}
}
}

View file

@ -1,6 +1,7 @@
use crate::messages::portfolio::document::utility_types::document_metadata::LayerNodeIdentifier;
use crate::messages::prelude::*;
use crate::messages::tool::common_functionality::graph_modification_utils::get_text;
use crate::messages::tool::tool_messages::path_tool::PathOverlayMode;
use glam::DVec2;
use graphene_core::renderer::Quad;
use graphene_core::text::{FontCache, load_face};
@ -93,3 +94,46 @@ pub fn calculate_segment_angle(anchor: PointId, segment: SegmentId, vector_data:
required_handle.map(|handle| -(handle - anchor_position).angle_to(DVec2::X))
}
/// Check whether a point is visible in the current overlay mode.
pub fn is_visible_point(
manipulator_point_id: ManipulatorPointId,
vector_data: &VectorData,
path_overlay_mode: PathOverlayMode,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
selected_segments: Vec<SegmentId>,
selected_points: &HashSet<ManipulatorPointId>,
) -> bool {
match manipulator_point_id {
ManipulatorPointId::Anchor(_) => true,
ManipulatorPointId::EndHandle(segment_id) | ManipulatorPointId::PrimaryHandle(segment_id) => {
match (path_overlay_mode, selected_points.len() == 1) {
(PathOverlayMode::AllHandles, _) => true,
(PathOverlayMode::SelectedPointHandles, _) | (PathOverlayMode::FrontierHandles, true) => {
if selected_segments.contains(&segment_id) {
return true;
}
// Either the segment is a part of selected segments or the opposite handle is a part of existing selection
let Some(handle_pair) = manipulator_point_id.get_handle_pair(vector_data) else { return false };
let other_handle = handle_pair[1].to_manipulator_point();
// Return whether the list of selected points contain the other handle
selected_points.contains(&other_handle)
}
(PathOverlayMode::FrontierHandles, false) => {
let Some(anchor) = manipulator_point_id.get_anchor(vector_data) else {
warn!("No anchor for selected handle");
return false;
};
let Some(frontier_handles) = &frontier_handles_info else {
warn!("No frontier handles info provided");
return false;
};
frontier_handles.get(&segment_id).map(|anchors| anchors.contains(&anchor)).unwrap_or_default()
}
}
}
}
}

View file

@ -379,6 +379,7 @@ struct PathToolData {
alt_dragging_from_anchor: bool,
angle_locked: bool,
temporary_colinear_handles: bool,
frontier_handles_info: Option<HashMap<SegmentId, Vec<PointId>>>,
adjacent_anchor_offset: Option<DVec2>,
}
@ -451,6 +452,7 @@ impl PathToolData {
lasso_select: bool,
handle_drag_from_anchor: bool,
drag_zero_handle: bool,
path_overlay_mode: PathOverlayMode,
) -> PathToolFsmState {
self.double_click_handled = false;
self.opposing_handle_lengths = None;
@ -466,8 +468,14 @@ impl PathToolData {
let old_selection = shape_editor.selected_points().cloned().collect::<Vec<_>>();
// Check if the point is already selected; if not, select the first point within the threshold (in pixels)
if let Some((already_selected, mut selection_info)) = shape_editor.get_point_selection_state(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD) {
log::info!("entered the part where tool identifies a near point");
// Don't select the points which are not shown currently in PathOverlayMode
if let Some((already_selected, mut selection_info)) = shape_editor.get_point_selection_state(
&document.network_interface,
input.mouse.position,
SELECTION_THRESHOLD,
path_overlay_mode,
self.frontier_handles_info.clone(),
) {
responses.add(DocumentMessage::StartTransaction);
self.last_clicked_point_was_selected = already_selected;
@ -475,7 +483,14 @@ impl PathToolData {
// If the point is already selected and shift (`extend_selection`) is used, keep the selection unchanged.
// Otherwise, select the first point within the threshold.
if !(already_selected && extend_selection) {
if let Some(updated_selection_info) = shape_editor.change_point_selection(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD, extend_selection) {
if let Some(updated_selection_info) = shape_editor.change_point_selection(
&document.network_interface,
input.mouse.position,
SELECTION_THRESHOLD,
extend_selection,
path_overlay_mode,
self.frontier_handles_info.clone(),
) {
selection_info = updated_selection_info;
}
}
@ -1055,14 +1070,16 @@ impl Fsm for PathToolFsmState {
match tool_options.path_overlay_mode {
PathOverlayMode::AllHandles => {
path_overlays(document, DrawHandles::All, shape_editor, &mut overlay_context);
tool_data.frontier_handles_info = None;
}
PathOverlayMode::SelectedPointHandles => {
let selected_segments = selected_segments(document, shape_editor);
let selected_segments = selected_segments(&document.network_interface, shape_editor);
path_overlays(document, DrawHandles::SelectedAnchors(selected_segments), shape_editor, &mut overlay_context);
tool_data.frontier_handles_info = None;
}
PathOverlayMode::FrontierHandles => {
let selected_segments = selected_segments(document, shape_editor);
let selected_segments = selected_segments(&document.network_interface, shape_editor);
let selected_points = shape_editor.selected_points();
let selected_anchors = selected_points
.filter_map(|point_id| if let ManipulatorPointId::Anchor(p) = point_id { Some(*p) } else { None })
@ -1090,13 +1107,18 @@ impl Fsm for PathToolFsmState {
for (point, attached_segments) in selected_segments_by_point {
if attached_segments.len() == 1 {
segment_endpoints.entry(attached_segments[0]).or_default().push(point);
} else if !selected_anchors.contains(&point) {
}
// Handle the edge case where a point, although not explicitly selected, is shared by two segments.
else if !selected_anchors.contains(&point) {
segment_endpoints.entry(attached_segments[0]).or_default().push(point);
segment_endpoints.entry(attached_segments[1]).or_default().push(point);
}
}
}
// Caching segment endpoints for use in point selection logic
tool_data.frontier_handles_info = Some(segment_endpoints.clone());
// Now frontier anchors can be sent for rendering overlays
path_overlays(document, DrawHandles::FrontierHandles(segment_endpoints), shape_editor, &mut overlay_context);
}
@ -1198,7 +1220,17 @@ impl Fsm for PathToolFsmState {
tool_data.selection_mode = None;
tool_data.lasso_polygon.clear();
tool_data.mouse_down(shape_editor, document, input, responses, extend_selection, lasso_select, handle_drag_from_anchor, drag_zero_handle)
tool_data.mouse_down(
shape_editor,
document,
input,
responses,
extend_selection,
lasso_select,
handle_drag_from_anchor,
drag_zero_handle,
tool_options.path_overlay_mode,
)
}
(
PathToolFsmState::Drawing { selection_shape },
@ -1353,7 +1385,13 @@ impl Fsm for PathToolFsmState {
// If there is a point nearby, then remove the overlay
if shape_editor
.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD)
.find_nearest_visible_point_indices(
&document.network_interface,
input.mouse.position,
SELECTION_THRESHOLD,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
)
.is_some()
{
tool_data.segment = None;
@ -1447,9 +1485,21 @@ impl Fsm for PathToolFsmState {
match selection_shape {
SelectionShapeType::Box => {
let bbox = [tool_data.drag_start_pos, previous_mouse];
shape_editor.select_all_in_shape(&document.network_interface, SelectionShape::Box(bbox), selection_change);
shape_editor.select_all_in_shape(
&document.network_interface,
SelectionShape::Box(bbox),
selection_change,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
);
}
SelectionShapeType::Lasso => shape_editor.select_all_in_shape(&document.network_interface, SelectionShape::Lasso(&tool_data.lasso_polygon), selection_change),
SelectionShapeType::Lasso => shape_editor.select_all_in_shape(
&document.network_interface,
SelectionShape::Lasso(&tool_data.lasso_polygon),
selection_change,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
),
}
}
@ -1495,9 +1545,21 @@ impl Fsm for PathToolFsmState {
match selection_shape {
SelectionShapeType::Box => {
let bbox = [tool_data.drag_start_pos, previous_mouse];
shape_editor.select_all_in_shape(&document.network_interface, SelectionShape::Box(bbox), select_kind);
shape_editor.select_all_in_shape(
&document.network_interface,
SelectionShape::Box(bbox),
select_kind,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
);
}
SelectionShapeType::Lasso => shape_editor.select_all_in_shape(&document.network_interface, SelectionShape::Lasso(&tool_data.lasso_polygon), select_kind),
SelectionShapeType::Lasso => shape_editor.select_all_in_shape(
&document.network_interface,
SelectionShape::Lasso(&tool_data.lasso_polygon),
select_kind,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
),
}
}
responses.add(OverlaysMessage::Draw);
@ -1508,7 +1570,14 @@ impl Fsm for PathToolFsmState {
(_, PathToolMessage::DragStop { extend_selection, .. }) => {
let extend_selection = input.keyboard.get(extend_selection as usize);
let drag_occurred = tool_data.drag_start_pos.distance(input.mouse.position) > DRAG_THRESHOLD;
let nearest_point = shape_editor.find_nearest_point_indices(&document.network_interface, input.mouse.position, SELECTION_THRESHOLD);
// TODO: Here we want only visible points to be considered
let nearest_point = shape_editor.find_nearest_visible_point_indices(
&document.network_interface,
input.mouse.position,
SELECTION_THRESHOLD,
tool_options.path_overlay_mode,
tool_data.frontier_handles_info.clone(),
);
if let Some((layer, nearest_point)) = nearest_point {
if !drag_occurred && extend_selection {