many-to-many connections support

[kamirr]

This patch is an attempt at adding many-to-many connection support without overhauling the library and minimal breaking changes to the user.

The core of the idea is to replace Graph::connections from SecondaryMap<InputId, OutputId> to SecondaryMap<InputId, HashSet<OutputId>> and roll from that.

Ports are parametrized by an argument specifying the maximum non-zero number of connections (not yet enforced), and if that number is greater than 1 a so called wide-port is drawn -- it's characterized by being longer than a typical port and connections made to it are spread out and can be disconnected individually.

Known limitations:

• Again, max connections per node is not enforced.
• HashSet<_> isn't an adequate data structure for storing the connected outputs -- we have no control over ordering, let alone the end user.
• API needs fleshing out.
• Dragging from a non-empty wide port always triggers a disconnect.
• ...?

[kamirr]

I felt like #30 and #81 offer an overly complicated solution to the problem of many-to-many connections, so I'm messing around with a different approach. I'd appreciate comments regarding the general design of the feature.

Do note that this is purely an experiment to see if this approach can even lead to a workable solution that offers good UX and simple, flexible API.

[setzer22]

This is really nice! 🎉 Thanks a lot for this PR :)

Indeed, I've been hesitant to merge previous attempts to this in the past because of how much they deviated from the original design. This looks like a very clean incremental update over what we had, and indeed, users should not have a hard time upgrading. I'd be very much OK with merging something like this.

I feel the ordering of parameters is worth discussing. In this version, you are using a HashSet to store the multiple inputs in a wide port, and this leads to a random ordering of the connections. This leads to the question: Do we care about the ordering of parameters? In your example, an addition operation would not care, since in those cases, as it is commutative. But what happens with operations that are not commutative?

I'm not sure how hard it would be to extend the current design into something that allows users to reorder the connections in a wide port. Essentially, replace that HashSet with a Vec and all the necessary UI interaction bits that make this work. Calculating the insertion point into the list of inputs based on the mouse position when the mouse is released into a connection into a pin doesn't sound that complicated. 🤔 Oh, and now I just realized you're already doing a ver similar logic to handle pin disconnections, so this calculation is already in place!

In case we decide ordered connections in a port are broadening the scope too much for now, input connections should at least be sorted in a deterministic way before iterating them.

[kamirr]

This leads to the question: Do we care about the ordering of parameters? In your example, an addition operation would not care, since in those cases, as it is commutative. But what happens with operations that are not commutative?

Having sat on the issue for a while, these are my thoughts:

• Ordering of inputs is important not only because the consumer might care (honestly, I'd rather discourage using wide ports for non-commutative operations), but it's crucial from the UX-perspective. If the user is not able to order connections at will, unwanted intersections of connections are bound to come up.
• HashSet<_> seemed like an elegant structure for this and I hoped that ordering could come from someplace else, but this would make it more convoluted to access for the end user. I'll switch to just using a Vec<_>.

[kamirr]

Implemented ordered inputs, here's the current status: nodes_ordering.webm. Sorry for the invisible cursor

[setzer22]

Looking good so far 👍 The new port ordering looks pretty nice!

Once we get the input size and y offset for the wide ports figured out, this will be ready for merging. I left a comment above with a potential solution.

[kamirr]

Screenshots

(The args label missing in the first image is fixed by the last commit, ignore that.)

Looks like this is feature-complete for now. I'll clean up the code, rebase and mark it as ready to review.

[setzer22]

Amazing! 🎉 Thanks a lot for the hard work @kamirr.

I'd say with the nice round of recent changes, this being merged would be a good time for a release. But I'm not sure if you're planning on tackling any other tasks, there's really no rush so we can wait.

[kamirr]

I'd like to work on documentation before the next release, I'll get to that right after we get this PR merged.

Ps. I was a bit too eager to declare feature-completeness, max_connections still isn't enforced, but that's easy enough to address.

[kamirr]

Should be ready for review:

• Rebased onto main.
• Some cleanups in UI.
• Deleted changes in the example app.
• Enforce max_connections.

The history of my changes is lost due to the rebase with squashes I did, but it can be browsed on the kek/many-to-many2 branch on my fork.

[setzer22]

Thanks for all the amazing work :) I'll review this soon, but I already made a quick look and things are looking good.

[fenollp]

I'm not sure how hard it would be to extend the current design into something that allows users to reorder the connections in a wide port. Essentially, replace that HashSet with a Vec

My 2 cents: https://doc.rust-lang.org/std/collections/struct.BTreeSet.html is insertion-sorted :)