GraphiteEditor / GraphiteEditor/Graphite

Pen tool offset when merging

Aperta
#4,213 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Rust
Stelle
27.2k
Fork
1.3k
Merge medio
20h 5m
PR unite (30g)
57

Descrizione

# Reproduction
- New document
- Select pen tool
- Draw a line not at the origin
- End drawing
- Start new layer somewhere else
- Place anchor on the end point of the original line
- Observe offset

# Video
https://github.com/user-attachments/assets/8a0009f0-947b-4fd4-99f8-23c0872f7188

# Code
The pen tool appears to refer to a "buffer" system e.g. `buffering_merged_vector`. However buffering system is thankfully removed. I have literally no clue what is meant to be going on, however I fixed the issue with the following diff. It probably breaks other things though.

```diff
diff --git a/editor/src/messages/tool/tool_messages/pen_tool.rs b/editor/src/messages/tool/tool_messages/pen_tool.rs
index 529404b12..89690ee68 100644
--- a/editor/src/messages/tool/tool_messages/pen_tool.rs
+++ b/editor/src/messages/tool/tool_messages/pen_tool.rs
@@ -401,8 +401,6 @@ struct PenToolData {
auto_panning: AutoPanning,
modifiers: ModifierState,

- buffering_merged_vector: bool,
-
previous_handle_start_pos: DVec2,
previous_handle_end_pos: Option,
toggle_colinear_debounce: bool,
@@ -1851,26 +1849,30 @@ impl Fsm for PenToolFsmState {

self
}
+ (PenToolFsmState::PlacingAnchor, PenToolMessage::RecalculateLatestPointsPosition) => {
+ tool_data.recalculate_latest_points_position(document);
+
+ // If we were placing anchors then it would be a good idea to update the anchor if possible
+ if let Some(layer) = layer {
+ tool_data.handle_mode = HandleMode::ColinearLocked;
+ tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, shape_editor, responses);
+ tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses);
+ PenToolFsmState::DraggingHandle(tool_data.handle_mode)
+ } else {
+ PenToolFsmState::Ready
+ }
+ }
(state, PenToolMessage::RecalculateLatestPointsPosition) => {
tool_data.recalculate_latest_points_position(document);
state
}
- (PenToolFsmState::PlacingAnchor, PenToolMessage::DragStart { append_to_selected }) => {
+ (PenToolFsmState::PlacingAnchor, PenToolMessage::DragStart { .. }) => {
let point = SnapCandidatePoint::handle(document.metadata().document_to_viewport.inverse().transform_point2(input.mouse.position));
let snapped = tool_data.snap_manager.free_snap(&SnapData::new(document, input, viewport), &point, SnapTypeConfiguration::default());
let viewport_vec = document.metadata().document_to_viewport.transform_point2(snapped.snapped_point_document);

- // Early return if the buffer was started and this message is being run again after the buffer (so that place_anchor updates the state with the newly merged vector)
- if tool_data.buffering_merged_vector {
- if let Some(layer) = layer {
- tool_data.buffering_merged_vector = false;
- tool_data.handle_mode = HandleMode::ColinearLocked;
- tool_data.bend_from_previous_point(SnapData::new(document, input, viewport), transform, layer, shape_editor, responses);
- tool_data.place_anchor(SnapData::new(document, input, viewport), transform, input.mouse.position, responses);
- }
- tool_data.buffering_merged_vector = false;
- PenToolFsmState::DraggingHandle(tool_data.handle_mode)
- } else {
+ let mut is_merging = false;
+ {
if tool_data.handle_end.is_some() {
responses.add(DocumentMessage::StartTransaction);
}
@@ -1890,14 +1892,17 @@ impl Fsm for PenToolFsmState {
.or(tool_data.current_layer.filter(|layer| *layer != other_layer))
{
merge_layers(document, current_layer, other_layer, responses);
+ is_merging = true;
}
}
+ }

- // Even if no buffer was started, the message still has to be run again in order to call bend_from_previous_point
- tool_data.buffering_merged_vector = true;
- responses.add(PenToolMessage::DragStart { append_to_selected });
- PenToolFsmState::PlacingAnchor
+ // If not merging, then recalculate points and transition to the new mode as soon as possible (if merging then must be delayed until graph rerun)
+ if !is_merging {
+ responses.add(PenToolMessage::RecalculateLatestPointsPosition);
}
+
+ PenToolFsmState::PlacingAnchor
}
(PenToolFsmState::PlacingAnchor, PenToolMessage::RemovePreviousHandle) => {
if let Some(last_point) = tool_data.latest_points.last_mut() {
```

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.