setzer22 / egui_node_graph

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

Hierarchical design and many-to-many connections #81

Open mxgrey opened 1 year ago

mxgrey commented 1 year ago

Happy new year!

I've finally managed to finish the redesign that I proposed in this thread, and now here's the PR for it!

To accomplish the goals that I set out to achieve, I had to do almost a total rewrite of the library although I tried to keep it as spiritually close to the original as I could. Given how extreme this rewrite is, I won't be offended at all if you decide to reject this PR. Either way it was an extremely valuable exercise for me.

To help you decide whether you actually want these changes, here's a list of pros and cons:

Pros

Cons

After really getting into the weeds on this effort, I have a few conceptual concerns about using egui to produce node graphs.

For all of the above reasons I'm thinking about exploring the possibility of developing a node graph editor on top of kayak_ui which is made for the bevy engine. I feel optimistic that with the benefits of an ECS and a retained mode rendering pipeline there will be a lot more opportunity to produce large scale, complex, aesthetically pleasing node graphs.

Tsudico commented 1 year ago

Would ports be able to be external to the node if the code used an egui::Grid with 3 columns for the actual node? One column for input node ports, the center column for the node information, and the last column for output node ports.

mxgrey commented 1 year ago

I'm definitely not an expert in egui, so there may be some solution that I'm not thinking of, but my impression is that the node's background box will wrap itself (plus some margin) around anything drawn using the node's ui.

I think the only way to draw the hooks at the edge is for the port implementation to be given access to a mutable reference to the parent ui of the node. The problem with that is we can't give the port a mutable reference to the node's ui AND mutable reference to its parent ui at the same time because of Rust's borrow rules.

The only way I can think of making it work would be to have a multi-stage API for the ports where

  1. We call Port::show_port, passing it the node ui
  2. Port::show_port returns an associated type Port::HookState which contains whatever info the port implementation needs in order to draw the hooks
  3. We call Port::show_hooks, passing it the parent of the node ui along with the Port::HookState that was returned by Port::show_port.

I considered trying this but I wasn't sure if that would make the API too complex/confusing. Then again most users wouldn't be implementing their own ports so maybe that complexity isn't a big deal.

setzer22 commented 1 year ago

Hi! First of all happy new year :tada: and thanks for this amazing PR :smile:

The amount of changes is huge, so it's going to take a while for me to find some time to sit down and review this, but I'm planning to! I just haven't had a lot of time for open source work lately.

First of all, as a quick reaction to the list of pros/cons, I think an API breakage was going to be necessary anyway due to the conceptual changes in the way connections work. So overall I'm on board with this! But I must say the visual change with the port location is a bit unfortunate.

Without having delved into the code yet, I think the solution will be to draw the ports by reserving space in a way that doesn't push the boundaries of the node's UI. The code that draws the node's background (unless that too has changed) uses the Ui's min_rect as a means of knowing what are the contents of the node that should be wrapped. So the solution is to draw the ports in such a way that the min_rect is not modified. On egui, there's allocate_rect_exact for this very purpose. It would be something along the lines of:

Even though the changes look amazing and I'm really happy how this is coming along, I'm a bit hesitant to merge as-is due to this visual change without at least exploring some solutions :thinking:

After really getting into the weeds on this effort, I have a few conceptual concerns about using egui to produce node graphs.

I guess I'm happy to feel validated in my concerns :sweat_smile:, I have been thinking like this for a while. Immediate mode makes things very convenient for the user, but it puts a huge burden on the library authors to present a consistent, themable and non-leaky abstraction.

Take the changes I was just discussing for instance. I understand if readers feel my suggestion above is not elegant, I think so too. I would like to do things in such a way that they compose nicely: The port happily draws its own UI, and then the node is able to compose that into the UI it's drawing, but that's not how things work in immediate-mode because layouting, drawing and input processing are tied together into a single function and everything must be done in a single pass. At the end of the day, these sort of workarounds are necessary whenever you need any non-trivial layout in any immediate mode library.

For all of the above reasons I'm thinking about exploring the possibility of developing a node graph editor on top of kayak_ui which is made for the bevy engine.

I wasn't aware of kayak, but I'm currently also in the stage of considering alternatives for my larger project Blackjack. I've been taking a very serious look at Iced, and right now I'm working on some crazy idea that doesn't have enough merit to be discussed... yet :grin:.

I'm still happy to maintain this library, but I realized once you reach a level of nit-pickiness where these minor UX details start to become more important than my own convenience, immediate mode UI starts loosing some of its value proposition.

mxgrey commented 1 year ago

I've been taking a very serious look at Iced

I've worked a decent amount with iced. I think it's very good for making simple, clean GUI applications but I believe it suffers many of the same drawbacks as egui in terms of rigid hierarchy and clunky state representation. I'm under the impression that it's also an immediate mode renderer.

I haven't worked with kayak_ui yet beyond reading through the docs, but I've spent a lot of time working with bevy, and frankly it's incredible for professional grade applications. Making simple small-scale GUI applications is quicker and easier in iced, but I haven't seen anything in the Rust ecosystem comes close to competing with bevy for complex high-performance applications.

I'm a bit hesitant to merge as-is due to this visual change without at least exploring some solutions

Fully understood and agreed. I proposed a two-stage port drawing API in an earlier comment. I'm not sure if that would be a better or worse developer experience than the allocation strategy that you're proposing.

setzer22 commented 1 year ago

I proposed a two-stage port drawing API in an earlier comment. I'm not sure if that would be a better or worse developer experience than the allocation strategy that you're proposing.

Oops, sorry I missed that second message! I need to give a proper look at the code to better understand your suggestion. Neither solution sounds like it would make drawing the ports very developer-friendly. But as you're saying, this would be a rare customization, and one that would only need to be done once. So probably it doesn't matter that it's a bit more verbose.

I believe it suffers many of the same drawbacks as egui in terms of rigid hierarchy and clunky state representation. I'm under the impression that it's also an immediate mode renderer.

I think we're talking about slightly different things here. Iced is very different from egui because layout, input handling and rendering all happen at different stages. In my opinion, it solves the problem of egui's lack of widget composability and that's the issue we're facing here with the port widgets (and what I was referring to as limitations of the immediate-mode paradigm). I still agree Iced may lead to different kinds drawbacks.

kkngsm commented 1 year ago

I will try to rewrite it for yew around February. This is for my project and is not intended to be a PR for this library. I would like a place to discuss the gui library. Is it discussion or issue?

setzer22 commented 1 year ago

I will try to rewrite it for yew around February. This is for my project and is not intended to be a PR for this library.

Nice :smile: Even if it's not a PR, feel free to share once you have something!

I would like a place to discuss the gui library. Is it discussion or issue?

Not quite sure what you mean. My plan is to (obviously :sweat_smile:) keep egui_node_graph egui-focused. I'm doing some experiments on the side for my other project, which is not necessarily tied to egui, and that might involve building a node editor library in another GUI framework, but I have nothing worth sharing just yet.

kkngsm commented 1 year ago

That is as you say. I just thought it was inappropriate to talk about GUI in this PR.

setzer22 commented 1 year ago

I just thought it was inappropriate to talk about GUI in this PR.

Ah, yes, I see what you mean. I believe this repo is small enough that we can be a bit more lenient on the rules. But indeed, if there's something you want to keep discussing feel free to open a discussion thread!