Insert point on segment by clicking once (no more sliding); Ctrl+click to remove segment

[4adex]

Part of #1870

When clicking a segment (new since video was recorded):

• Clicking without any modifier key should split the segment where clicked. Currently this has the extra complication of entering an insertion mode where the point of the segment split can be slid within the segment and left click confirms it while right click cancels. We want to remove that extra step and just insert directly. An overlay should be drawn in cases where the cursor is hovered over the segment such that clicking would insert a split, it should be a short line segment (20px in diameter) running perpendicular to the curve.

• Ctrl clicking should delete the segment that's clicked. Currently a user would have to split the segment then delete the newly created point.

[Keavon]

!build

[github-actions]

📦 Build Complete for b4fb9bb
https://f35b2fe6.graphite.pages.dev

[Keavon]

This don't seem to be acting on a very reliable basis: https://files.keavon.com/-/IntrepidWhichAndeancondor/capture_4_.mp4

Please also make the size of the line and X visualization configurable in consts.rs.

[4adex]

Moved the constants to consts.rs and also increased the tolerance so that it moves to insertion mode better. It seems to be unreliable because the size of overlay was more than the tolerance.

[Keavon]

!build

[github-actions]

📦 Build Complete for 84c323a
https://2394aa5e.graphite.pages.dev

[Keavon]

It looks like this currently "enters a mode" when the mouse gets nearby and that mode can be exited by right clicking/hitting escape. And it affects the hints bar by removing everything else from it. And it stays in the mode until the user moves their cursor far enough away to "break free" from it. Some of this is likely a holdover from the previous behavior (what's in master currently).

The desired new behavior is just showing that insertion line whenever you're within X distance of any segment (and the nearest segment if within X distance of multiple segments). And no change to the hints bar from the normal mode.

Thanks :)

[4adex]

Yes previous implementation used a new state when the clicked near the segment and then clicking again inserts the point, so I just changed that so that when pointer is moved closer to a segment compared to a threshold then it goes into that mode. I think that going into a separate state for inserting point prevents its collision with other features of the path tool. So should I change that it doesn't goes into a new mode?

[Keavon]

So should I change that it doesn't goes into a new mode?

I think this will be the cleanest and most robust approach, yes.

[4adex]

Done with the asked changes, removed the extra state which was being used for inserting point.

[0HyperCube]

Please write some tests for this behaviour.

Thanks for your work so far on this.

[0HyperCube]

Why did you choose to calculate the state here and cache it instead of computing it when it is actually needed?

[4adex]

Because that data is used for both drawing the overlays and actually for adding point when clicking, hence it makes better sense to calculate and cache it on PointerMove and use it when required.

[0HyperCube]

Having this additional state creates unnecessary complexity; it makes systems harder to test.

[4adex]

Calculating it over, will increase complexity of the operation so better cache it and use the method update_closest_point to update the position of the closest point on the current segment.
That's why I implemented it this way. Should I change?

[0HyperCube]

As long as you write the appropriate test, I think this is fine then. However I struggle to see how making a function and calling it in two places creates complexity. Thanks.