GraphiteEditor / GraphiteEditor/Graphite

Pen tool offset when merging

Ouverte
#4,213 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
Rust
Étoiles
27.2k
Forks
1.3k
Merge moyen
20 h 5 min
PR mergées (30 j)
57

Description

# 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() {
```

Guide de contribution

Aucun guide de contribution indexé pour ce dépôt

Piste de recherche

Commencez par les étapes de reproduction et examinez editor/src/messages/tool/tool_messages/pen_tool.rs, en particulier la FSM de PenTool qui gère PlacingAnchor, DragStart et RecalculateLatestPointsPosition. Suivez l’effet de la fusion des calques sur les positions du dernier point et vérifiez que le placement d’une ancre sur l’extrémité de la ligne d’origine ne produit plus de décalage, sans vous appuyer sur le diff non vérifié comme solution finale.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
rust
Domaine
computer-graphics
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.