Skip to content

Fix snapping edge cases - #4511

Open
devviktoria wants to merge 1 commit into
GraphiteEditor:masterfrom
devviktoria:snapping-edge-case-fix
Open

Fix snapping edge cases#4511
devviktoria wants to merge 1 commit into
GraphiteEditor:masterfrom
devviktoria:snapping-edge-case-fix

Conversation

@devviktoria

Copy link
Copy Markdown

This is just a minimal fix for the issue without any snapping weighting.
If the snapping snaps directly to the anchor point the angle calculation is going to be wrong and the snapping could return an infinite point and in that case the snapping point will be 0,0 point. This fixes this case.
Here is the video of this behavior:

snappingedgecasefix-2026-09-08_09.58.14.mp4

Closes #2451

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Confidence score: 3/5

  • In editor/src/messages/tool/tool_messages/path_tool.rs, the fallback from free_snap/constrained_snap still returns SnappedPoint::infinite_snap(DVec2::ZERO) with an infinite distance, so the guard may continue producing a non-finite snapped point; ensure the fallback returns a valid finite result.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/tool/tool_messages/path_tool.rs">

<violation number="1" location="editor/src/messages/tool/tool_messages/path_tool.rs:1162">
P2: In the buggy path this guard is meant to fix, the fallback still yields a non-finite point. `free_snap`/`constrained_snap` return `SnappedPoint::infinite_snap(DVec2::ZERO)` (distance = INFINITY) precisely when `point.document_point.is_finite()` is false, i.e. when `new_handle_position` itself is non-finite. So replacing the snapped point with `snapped_point_document: new_handle_position` substitutes NaN/Infinity for the old 0,0 point instead of a usable finite position. Use a known-finite fallback (e.g. `handle_position`, which produces a zero delta) for the non-finite case, while keeping `new_handle_position` for the normal no-snap case.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

};

//If the snapping result is a non-finite position
if snap_result.distance == f64::INFINITY {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: In the buggy path this guard is meant to fix, the fallback still yields a non-finite point. free_snap/constrained_snap return SnappedPoint::infinite_snap(DVec2::ZERO) (distance = INFINITY) precisely when point.document_point.is_finite() is false, i.e. when new_handle_position itself is non-finite. So replacing the snapped point with snapped_point_document: new_handle_position substitutes NaN/Infinity for the old 0,0 point instead of a usable finite position. Use a known-finite fallback (e.g. handle_position, which produces a zero delta) for the non-finite case, while keeping new_handle_position for the normal no-snap case.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/tool/tool_messages/path_tool.rs, line 1162:

<comment>In the buggy path this guard is meant to fix, the fallback still yields a non-finite point. `free_snap`/`constrained_snap` return `SnappedPoint::infinite_snap(DVec2::ZERO)` (distance = INFINITY) precisely when `point.document_point.is_finite()` is false, i.e. when `new_handle_position` itself is non-finite. So replacing the snapped point with `snapped_point_document: new_handle_position` substitutes NaN/Infinity for the old 0,0 point instead of a usable finite position. Use a known-finite fallback (e.g. `handle_position`, which produces a zero delta) for the non-finite case, while keeping `new_handle_position` for the normal no-snap case.</comment>

<file context>
@@ -1154,6 +1158,14 @@ impl PathToolData {
 		};
 
+		//If the snapping result is a non-finite position
+		if snap_result.distance == f64::INFINITY {
+			snap_result = SnappedPoint {
+				snapped_point_document: new_handle_position,
</file context>

@devviktoria

Copy link
Copy Markdown
Author

Please note: Since the code are just copied and adopted from my previous code I copied my codereview comment from the precious one instead of rewriting them. I hope that is OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dragging handle back into its anchor makes its opposite handle move around randomly

1 participant