Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Reroutes #301

Merged
merged 40 commits into from
Nov 12, 2024
Merged

Add Reroutes #301

merged 40 commits into from
Nov 12, 2024

Conversation

webfiltered
Copy link
Contributor

@webfiltered webfiltered commented Nov 11, 2024

Link Reroutes

Native reroutes are added by this PR, along with the supporting framework. Much of this work can be extended to implement new canvas types.

Scope

This PR gets the core infrastructure for reroutes in and testable, without touching a thousand two thousand lines of code.

More ways create & manage reroutes are planned.

Enabling

  • ⚠️ Reroutes are off by default
  • Enable per-canvas by setting
canvas.reroutesEnabled = true

Alt drag from a link to create a reroute

reroute-alt-drag.mp4

Shift drag from a reroute to create another link

reroute-shift-drag.mp4

Link centre markers

  • No marker
  • Default circles
  • New directional Arrow
    image

Bug fixes

  • Fixes centre link markers: now always centred for all link types (fixes dots rendered off to the side of straight vertical links, etc)
  • Fixes crash if input & output are null when connecting links

- Initial Reroute implementation
- LLink and Reroute both implement the new LinkSegment interface
- LinkSegments can have a parentId, which always points to a Reroute
Use extras.reroutes to store additional data
Add keepReroutes option to prevent Reroute GC
Improves link render time
For the "shift-click drag link from link" feature
Temporary workaround
Add enum for link markers
-> Pointing the way forward ->
Set default centre marker to arrow
Works at root of all canvas interactions, should leave existing reroutes untouched but invisible until e.g. links are edited / changed.
@catboxanon
Copy link
Contributor

catboxanon commented Nov 11, 2024

Not to get terribly off-topic in this PR, but I think it may be worth considering looking at how other applications like Blender handle this, which supports creating multiple links at once (often for the purpose of grouping a single output going to multiple inputs), and more importantly, in a less mouse-precise way (node links are quite thin and require cursor accuracy). The same tool can also be used to quickly remove links.

blender_2024-11-11_12-52-03.mp4

@webfiltered
Copy link
Contributor Author

webfiltered commented Nov 11, 2024

That is a bit off-topic for this PR. However, having not actually used blender reroutes or nodes, I am extremely happy to hear that a feature I was imagining is already out there and well-regarded.

I was thinking of dragging the mouse (some kind of combo perhaps), and having it collect links and bunch them up - animated as you went. Drawing a line and bunching without animation would be much simpler.

What this PR does is extend existing mouse interaction code and add Reroutes to the framework. It does not block features from being added, and is definitely not intended as a complete answer (but it certainly is a lot of it).

Would you mind transferring that to an "enhancement" issue? It helps greatly to have tracking on these things.

@catboxanon
Copy link
Contributor

Made one here, can't add the label on my end. Let me know if I should mention anything else. #303

@webfiltered webfiltered marked this pull request as ready for review November 12, 2024 00:58
@webfiltered
Copy link
Contributor Author

@huchenlei Tests are all green locally - just flaky.

@huchenlei
Copy link
Member

Do we have a plan to convert the legacy reroute in the frontend repo to this new format?

@huchenlei huchenlei merged commit 3d6adf0 into master Nov 12, 2024
4 checks passed
@huchenlei huchenlei deleted the reroutes branch November 12, 2024 16:18
@webfiltered
Copy link
Contributor Author

Do we have a plan to convert the legacy reroute in the frontend repo to this new format?

100%! An optional switch that will convert all reroutes on load. This is a feature I need - I'm definitely writing it.

@webfiltered webfiltered added this to the Reroutes milestone Nov 17, 2024
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.

3 participants