setzer22 / egui_node_graph

Build your node graph applications in Rust, using egui
MIT License
736 stars 138 forks source link

many-to-many connections support #91

Open kamirr opened 1 year ago

kamirr commented 1 year ago

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:

kamirr commented 1 year ago

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.

image

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.

kamirr commented 1 year ago

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:

kamirr commented 1 year ago

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

kamirr commented 1 year ago
Screenshots ![image](https://github.com/setzer22/egui_node_graph/assets/7727537/1b106624-9ebd-4fc1-9cff-b2dd9a22e653) ![image](https://github.com/setzer22/egui_node_graph/assets/7727537/d5d87b65-7aee-482e-b0af-e2dd3dab7730) ![image](https://github.com/setzer22/egui_node_graph/assets/7727537/5a4ebd1b-c2e4-41aa-9249-6e4f216511ee) ![image](https://github.com/setzer22/egui_node_graph/assets/7727537/2cc23ab3-9319-4fcb-891d-a56db19e8855) (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 commented 1 year ago

Amazing! :tada: 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 commented 1 year ago

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 commented 1 year ago

Should be ready for review:

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 commented 1 year ago

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

fenollp commented 10 months ago

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 :)