Skip to content

Commit f0a2bbe

Browse files
committed
fix cmd + g group case
1 parent a8801bc commit f0a2bbe

File tree

2 files changed

+102
-30
lines changed

2 files changed

+102
-30
lines changed

editor/src/messages/tool/common_functionality/shape_editor.rs

Lines changed: 76 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,55 @@ impl ShapeState {
421421
(point.as_handle().is_some() && self.ignore_handles) || (point.as_anchor().is_some() && self.ignore_anchors)
422422
}
423423

424+
/// Creates a dummy segment insertion to trigger graph reorganization.
425+
/// This dummy segment will fail to insert (because PointIds become invalid during execution),
426+
/// but the important side effect is that it triggers the reorganization we need.
427+
fn create_dummy_segment_to_trigger_graph_reorganization(layer: LayerNodeIdentifier, start_point: PointId, end_point: PointId, responses: &mut VecDeque<Message>) {
428+
let dummy_segment_id = SegmentId::generate();
429+
let dummy_modification = VectorModificationType::InsertSegment {
430+
id: dummy_segment_id,
431+
points: [start_point, end_point],
432+
handles: [None, None],
433+
};
434+
responses.add(GraphOperationMessage::Vector {
435+
layer,
436+
modification_type: dummy_modification,
437+
});
438+
responses.add(NodeGraphMessage::RunDocumentGraph);
439+
}
440+
441+
/// a two-step process: trigger reorganization, then use position-based lookup.
442+
fn handle_grouped_transform_close_path(document: &DocumentMessageHandler, layer: LayerNodeIdentifier, start_point: PointId, end_point: PointId, responses: &mut VecDeque<Message>) {
443+
// Get the layer's transform (handles rotation, scaling, translation)
444+
let layer_transform = document.metadata().transform_to_document(layer);
445+
446+
let start_local_pos = document.network_interface.compute_modified_vector(layer).and_then(|v| v.point_domain.position_from_id(start_point));
447+
let end_local_pos = document.network_interface.compute_modified_vector(layer).and_then(|v| v.point_domain.position_from_id(end_point));
448+
449+
if let (Some(start_local), Some(end_local)) = (start_local_pos, end_local_pos) {
450+
// Transform positions to document/world space
451+
// These positions are stable (won't change during reorganization)
452+
let start_pos = layer_transform.transform_point2(start_local);
453+
let end_pos = layer_transform.transform_point2(end_local);
454+
455+
// This segment insertion will fail, but causes point domain reorganization
456+
Self::create_dummy_segment_to_trigger_graph_reorganization(layer, start_point, end_point, responses);
457+
458+
// Defer position-based connection to run after reorganization completes
459+
// By then, PointIds will be stable with their new remapped values
460+
responses.add(DeferMessage::AfterGraphRun {
461+
messages: vec![
462+
ToolMessage::Path(PathToolMessage::ConnectPointsByPosition {
463+
layer,
464+
start_position: start_pos,
465+
end_position: end_pos,
466+
})
467+
.into(),
468+
],
469+
});
470+
}
471+
}
472+
424473
pub fn close_selected_path(&self, document: &DocumentMessageHandler, responses: &mut VecDeque<Message>) {
425474
// First collect all selected anchor points across all layers
426475
let all_selected_points: Vec<(LayerNodeIdentifier, PointId)> = self
@@ -447,26 +496,33 @@ impl ShapeState {
447496
let (layer2, end_point) = all_selected_points[1];
448497

449498
if layer1 == layer2 {
450-
// Same layer: create segment directly
499+
// Same layer case
451500
if start_point == end_point {
452501
return;
453502
}
454503

455-
let segment_id = SegmentId::generate();
456-
let modification_type = VectorModificationType::InsertSegment {
457-
id: segment_id,
458-
points: [end_point, start_point],
459-
handles: [None, None],
460-
};
461-
responses.add(GraphOperationMessage::Vector { layer: layer1, modification_type });
504+
// Check if this is a grouped layer with multiple disconnected segments
505+
let has_multiple_segments = document.network_interface.compute_modified_vector(layer1).map(|v| v.segment_domain.ids().len() > 1).unwrap_or(false);
506+
507+
if has_multiple_segments {
508+
// Grouped paths: use helper function to handle reorganization
509+
Self::handle_grouped_transform_close_path(document, layer1, start_point, end_point, responses);
510+
} else {
511+
// Single segment: PointIDs are stable, use immediate insertion
512+
let segment_id = SegmentId::generate();
513+
let modification_type = VectorModificationType::InsertSegment {
514+
id: segment_id,
515+
points: [end_point, start_point],
516+
handles: [None, None],
517+
};
518+
responses.add(GraphOperationMessage::Vector { layer: layer1, modification_type });
519+
}
462520
} else {
463521
// Different layers: merge first, then create segment
464522

465523
// Get the local positions of the selected points
466-
let start_local_pos = document.network_interface.compute_modified_vector(layer1)
467-
.and_then(|v| v.point_domain.position_from_id(start_point));
468-
let end_local_pos = document.network_interface.compute_modified_vector(layer2)
469-
.and_then(|v| v.point_domain.position_from_id(end_point));
524+
let start_local_pos = document.network_interface.compute_modified_vector(layer1).and_then(|v| v.point_domain.position_from_id(start_point));
525+
let end_local_pos = document.network_interface.compute_modified_vector(layer2).and_then(|v| v.point_domain.position_from_id(end_point));
470526

471527
// Transform to document/world space
472528
let start_transform = document.metadata().transform_to_document(layer1);
@@ -478,13 +534,15 @@ impl ShapeState {
478534

479535
merge_layers(document, layer1, layer2, responses);
480536

481-
// Connect the points
482537
responses.add(DeferMessage::AfterGraphRun {
483-
messages: vec![ToolMessage::Path(PathToolMessage::ConnectPointsByPosition {
484-
layer: layer1,
485-
start_position: start_pos,
486-
end_position: end_pos,
487-
}).into()],
538+
messages: vec![
539+
ToolMessage::Path(PathToolMessage::ConnectPointsByPosition {
540+
layer: layer1,
541+
start_position: start_pos,
542+
end_position: end_pos,
543+
})
544+
.into(),
545+
],
488546
});
489547
}
490548
}

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

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2689,30 +2689,44 @@ impl Fsm for PathToolFsmState {
26892689
let mut start_point_id = None;
26902690
let mut end_point_id = None;
26912691

2692-
for (i, &pos) in positions.iter().enumerate() {
2693-
if start_point_id.is_none() && (pos - start_position).length() < POSITION_TOLERANCE {
2692+
// Get the merged layer's transform to convert local positions to document space
2693+
let layer_transform = document.metadata().transform_to_document(layer);
2694+
2695+
for (i, &local_pos) in positions.iter().enumerate() {
2696+
// Transform the local position to document space for comparison
2697+
let doc_pos = layer_transform.transform_point2(local_pos);
2698+
2699+
let start_distance = (doc_pos - start_position).length();
2700+
let end_distance = (doc_pos - end_position).length();
2701+
2702+
if start_point_id.is_none() && start_distance < POSITION_TOLERANCE {
26942703
start_point_id = Some(point_ids[i]);
26952704
}
2696-
if end_point_id.is_none() && (pos - end_position).length() < POSITION_TOLERANCE {
2705+
if end_point_id.is_none() && end_distance < POSITION_TOLERANCE {
26972706
end_point_id = Some(point_ids[i]);
26982707
}
26992708
if start_point_id.is_some() && end_point_id.is_some() {
27002709
break;
27012710
}
27022711
}
27032712

2704-
if let (Some(start_id), Some(end_id)) = (start_point_id, end_point_id) {
2705-
// Clear existing selection
2706-
shape_editor.deselect_all_points();
2707-
shape_editor.set_selected_layers(vec![layer]);
2713+
if let (Some(start_id), Some(end_id)) = (start_point_id, end_point_id) {
2714+
// Create segment directly
2715+
responses.add(DocumentMessage::StartTransaction);
27082716

2709-
shape_editor.select_point_by_layer_and_id(ManipulatorPointId::Anchor(start_id), layer);
2710-
shape_editor.select_point_by_layer_and_id(ManipulatorPointId::Anchor(end_id), layer);
2717+
let segment_id = SegmentId::generate();
2718+
let modification_type = VectorModificationType::InsertSegment {
2719+
id: segment_id,
2720+
points: [end_id, start_id],
2721+
handles: [None, None],
2722+
};
27112723

2712-
responses.add(PathToolMessage::ClosePath);
2713-
}
2724+
responses.add(GraphOperationMessage::Vector { layer, modification_type });
2725+
responses.add(DocumentMessage::EndTransaction);
2726+
responses.add(OverlaysMessage::Draw);
2727+
}
27142728

2715-
self
2729+
self
27162730
}
27172731
(_, PathToolMessage::StartSlidingPoint) => {
27182732
responses.add(DocumentMessage::StartTransaction);

0 commit comments

Comments
 (0)