Fix joining paths from different layers with Ctrl+J fails (#3534)
* fix: handle PointId remapping when joining paths from different layers When using Ctrl+J to join two points from different layers, the operation failed because PointIds change during merge due to collision resolution. The fix uses index-based point tracking instead of PointIds: 1. Before merge: Record the index of each selected point in its layer 2. Calculate post-merge indices using the point offset from layer1 3. After merge: Retrieve the actual PointIds at those indices 4. Create the connecting segment using the resolved PointIds This works because Vector::concat() preserves point ordering during merge, so we can use exact index arithmetic to find points after remapping. Fixes #3519 * fix: use position-based lookup when joining paths from different layers * fix cmd + g group case * fix check for grouped folder * add in the single layer inside a group case * use ApplyPointDelta instead of creating a dummy segment * Update editor/src/messages/tool/common_functionality/shape_editor.rs Co-authored-by: James Lindsay <78500760+0HyperCube@users.noreply.github.com> * Update editor/src/messages/tool/common_functionality/shape_editor.rs Co-authored-by: James Lindsay <78500760+0HyperCube@users.noreply.github.com> * encapsulate point connection logic in defer_connect_points_by_position * reduce tolerance to 1e-6 * remove wrong comments --------- Co-authored-by: James Lindsay <78500760+0HyperCube@users.noreply.github.com>
This commit is contained in:
parent
d98f19bf4a
commit
33e3a88458
|
|
@ -421,6 +421,68 @@ impl ShapeState {
|
||||||
(point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors)
|
(point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Applies a dummy vector modification to the layer. In the case where a group containing some vector data is selected, this triggers the creation of a «Flatten Path» node.
|
||||||
|
fn add_dummy_modification_to_trigger_graph_reorganization(layer: LayerNodeIdentifier, start_point: PointId, _end_point: PointId, responses: &mut VecDeque<Message>) {
|
||||||
|
// Apply a zero-delta to one of the points to trigger reorganization
|
||||||
|
let dummy_modification = VectorModificationType::ApplyPointDelta {
|
||||||
|
point: start_point,
|
||||||
|
delta: DVec2::ZERO,
|
||||||
|
};
|
||||||
|
responses.add(GraphOperationMessage::Vector {
|
||||||
|
layer,
|
||||||
|
modification_type: dummy_modification,
|
||||||
|
});
|
||||||
|
responses.add(NodeGraphMessage::RunDocumentGraph);
|
||||||
|
}
|
||||||
|
|
||||||
|
fn defer_connect_points_by_position(
|
||||||
|
document: &DocumentMessageHandler,
|
||||||
|
layer1: LayerNodeIdentifier,
|
||||||
|
start_point: PointId,
|
||||||
|
layer2: LayerNodeIdentifier,
|
||||||
|
end_point: PointId,
|
||||||
|
target_layer: LayerNodeIdentifier,
|
||||||
|
responses: &mut VecDeque<Message>,
|
||||||
|
) {
|
||||||
|
// Get the local positions of the selected points
|
||||||
|
let start_local_pos = document.network_interface.compute_modified_vector(layer1).and_then(|v| v.point_domain.position_from_id(start_point));
|
||||||
|
let end_local_pos = document.network_interface.compute_modified_vector(layer2).and_then(|v| v.point_domain.position_from_id(end_point));
|
||||||
|
|
||||||
|
// Transform to document/world space
|
||||||
|
let start_transform = document.metadata().transform_to_document(layer1);
|
||||||
|
let end_transform = document.metadata().transform_to_document(layer2);
|
||||||
|
|
||||||
|
let (Some(start_local), Some(end_local)) = (start_local_pos, end_local_pos) else {
|
||||||
|
warn!("Unable to resolve point ids for joining");
|
||||||
|
return;
|
||||||
|
};
|
||||||
|
// Transform positions to document/world space
|
||||||
|
// These positions are stable (won't change during reorganization)
|
||||||
|
let start_pos = start_transform.transform_point2(start_local);
|
||||||
|
let end_pos = end_transform.transform_point2(end_local);
|
||||||
|
|
||||||
|
// Defer position-based connection to run after reorganization completes
|
||||||
|
// By then, PointIds will be stable with their new remapped values
|
||||||
|
responses.add(DeferMessage::AfterGraphRun {
|
||||||
|
messages: vec![
|
||||||
|
ToolMessage::Path(PathToolMessage::ConnectPointsByPosition {
|
||||||
|
layer: target_layer,
|
||||||
|
start_position: start_pos,
|
||||||
|
end_position: end_pos,
|
||||||
|
})
|
||||||
|
.into(),
|
||||||
|
],
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
fn handle_grouped_transform_close_path(document: &DocumentMessageHandler, layer: LayerNodeIdentifier, start_point: PointId, end_point: PointId, responses: &mut VecDeque<Message>) {
|
||||||
|
// This zero-delta modification triggers point domain reorganization
|
||||||
|
Self::add_dummy_modification_to_trigger_graph_reorganization(layer, start_point, end_point, responses);
|
||||||
|
|
||||||
|
// Use the helper to defer the connection until after reorganization
|
||||||
|
Self::defer_connect_points_by_position(document, layer, start_point, layer, end_point, layer, responses);
|
||||||
|
}
|
||||||
|
|
||||||
pub fn close_selected_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
|
pub fn close_selected_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
|
||||||
// First collect all selected anchor points across all layers
|
// First collect all selected anchor points across all layers
|
||||||
let all_selected_points: Vec<(LayerNodeIdentifier, PointId)> = self
|
let all_selected_points: Vec<(LayerNodeIdentifier, PointId)> = self
|
||||||
|
|
@ -447,28 +509,34 @@ impl ShapeState {
|
||||||
let (layer2, end_point) = all_selected_points[1];
|
let (layer2, end_point) = all_selected_points[1];
|
||||||
|
|
||||||
if layer1 == layer2 {
|
if layer1 == layer2 {
|
||||||
|
// Same layer case
|
||||||
if start_point == end_point {
|
if start_point == end_point {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
let segment_id = SegmentId::generate();
|
// Check if this layer has multiple children (is a merged/grouped layer created with Cmd+G)
|
||||||
let modification_type = VectorModificationType::InsertSegment {
|
let num_children = layer1.children(document.metadata()).count();
|
||||||
id: segment_id,
|
let is_grouped = num_children > 1;
|
||||||
points: [end_point, start_point],
|
|
||||||
handles: [None, None],
|
if is_grouped {
|
||||||
};
|
// Grouped/merged layer: use helper function to handle reorganization
|
||||||
responses.add(GraphOperationMessage::Vector { layer: layer1, modification_type });
|
Self::handle_grouped_transform_close_path(document, layer1, start_point, end_point, responses);
|
||||||
|
} else {
|
||||||
|
// Single segment: PointIDs are stable, use immediate insertion
|
||||||
|
let segment_id = SegmentId::generate();
|
||||||
|
let modification_type = VectorModificationType::InsertSegment {
|
||||||
|
id: segment_id,
|
||||||
|
points: [end_point, start_point],
|
||||||
|
handles: [None, None],
|
||||||
|
};
|
||||||
|
responses.add(GraphOperationMessage::Vector { layer: layer1, modification_type });
|
||||||
|
}
|
||||||
} else {
|
} else {
|
||||||
// Merge the layers
|
// Different layers: merge first, then create segment
|
||||||
merge_layers(document, layer1, layer2, responses);
|
merge_layers(document, layer1, layer2, responses);
|
||||||
// Create segment between the two points
|
|
||||||
let segment_id = SegmentId::generate();
|
// Use the helper to defer the connection until after reorganization
|
||||||
let modification_type = VectorModificationType::InsertSegment {
|
Self::defer_connect_points_by_position(document, layer1, start_point, layer2, end_point, layer1, responses);
|
||||||
id: segment_id,
|
|
||||||
points: [end_point, start_point],
|
|
||||||
handles: [None, None],
|
|
||||||
};
|
|
||||||
responses.add(GraphOperationMessage::Vector { layer: layer1, modification_type });
|
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -73,6 +73,11 @@ pub enum PathToolMessage {
|
||||||
},
|
},
|
||||||
Escape,
|
Escape,
|
||||||
ClosePath,
|
ClosePath,
|
||||||
|
ConnectPointsByPosition {
|
||||||
|
layer: LayerNodeIdentifier,
|
||||||
|
start_position: DVec2,
|
||||||
|
end_position: DVec2,
|
||||||
|
},
|
||||||
DoubleClick {
|
DoubleClick {
|
||||||
extend_selection: Key,
|
extend_selection: Key,
|
||||||
shrink_selection: Key,
|
shrink_selection: Key,
|
||||||
|
|
@ -2669,6 +2674,60 @@ impl Fsm for PathToolFsmState {
|
||||||
|
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
(_, PathToolMessage::ConnectPointsByPosition { layer, start_position, end_position }) => {
|
||||||
|
// Get the merged vector
|
||||||
|
let Some(vector) = document.network_interface.compute_modified_vector(layer) else {
|
||||||
|
return self;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Find points by their positions (with small tolerance for floating point comparison)
|
||||||
|
const POSITION_TOLERANCE: f64 = 1e-6;
|
||||||
|
|
||||||
|
let positions = vector.point_domain.positions();
|
||||||
|
let point_ids = vector.point_domain.ids();
|
||||||
|
|
||||||
|
let mut start_point_id = None;
|
||||||
|
let mut end_point_id = None;
|
||||||
|
|
||||||
|
// Get the merged layer's transform to convert local positions to document space
|
||||||
|
let layer_transform = document.metadata().transform_to_document(layer);
|
||||||
|
|
||||||
|
for (i, &local_pos) in positions.iter().enumerate() {
|
||||||
|
// Transform the local position to document space for comparison
|
||||||
|
let doc_pos = layer_transform.transform_point2(local_pos);
|
||||||
|
|
||||||
|
let start_distance = (doc_pos - start_position).length();
|
||||||
|
let end_distance = (doc_pos - end_position).length();
|
||||||
|
|
||||||
|
if start_point_id.is_none() && start_distance < POSITION_TOLERANCE {
|
||||||
|
start_point_id = Some(point_ids[i]);
|
||||||
|
}
|
||||||
|
if end_point_id.is_none() && end_distance < POSITION_TOLERANCE {
|
||||||
|
end_point_id = Some(point_ids[i]);
|
||||||
|
}
|
||||||
|
if start_point_id.is_some() && end_point_id.is_some() {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if let (Some(start_id), Some(end_id)) = (start_point_id, end_point_id) {
|
||||||
|
// Create segment directly
|
||||||
|
responses.add(DocumentMessage::StartTransaction);
|
||||||
|
|
||||||
|
let segment_id = SegmentId::generate();
|
||||||
|
let modification_type = VectorModificationType::InsertSegment {
|
||||||
|
id: segment_id,
|
||||||
|
points: [end_id, start_id],
|
||||||
|
handles: [None, None],
|
||||||
|
};
|
||||||
|
|
||||||
|
responses.add(GraphOperationMessage::Vector { layer, modification_type });
|
||||||
|
responses.add(DocumentMessage::EndTransaction);
|
||||||
|
responses.add(OverlaysMessage::Draw);
|
||||||
|
}
|
||||||
|
|
||||||
|
self
|
||||||
|
}
|
||||||
(_, PathToolMessage::StartSlidingPoint) => {
|
(_, PathToolMessage::StartSlidingPoint) => {
|
||||||
responses.add(DocumentMessage::StartTransaction);
|
responses.add(DocumentMessage::StartTransaction);
|
||||||
if tool_data.start_sliding_point(shape_editor, document) {
|
if tool_data.start_sliding_point(shape_editor, document) {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue