Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

Hierarchical design and many-to-many connections #81

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

mxgrey
Copy link

@mxgrey mxgrey commented Jan 1, 2023

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

  • The UI is much more flexible
    • Developers can inject their own custom nodes and ports
    • Colors for the general look and feel can be customized
  • It is easy to set connection limits for both input and output ports, or make their connections limitless
    • This means developers can set their input nodes to accept 1, N, or unlimited inputs
  • It is easy for application users to see what the limits are and to choose which connections to disconnect
  • Migration to this new API isn't too terrible (imho), the example application can be used as a reference
  • I personally think the API is overall simpler to use, but obviously I'm biased when I say that

Cons

  • Massively changes the API
  • Ports are now inside of the node body instead of at the edge (see screenshot)
    • I do not prefer this change, but I couldn't figure out a way to make the node > port > hook hierarchy work without egui automatically wrapping the ports and hooks into the body of the node
      Screenshot from 2023-01-02 02-39-01

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

  • As you've mentioned elsewhere, zooming is very challenging and not something that egui presently supports out of the box
  • Getting a desirable level of flexibility/extensibility involves some very convoluted trait relationships
  • egui is a very useful and powerful tool for developing conventional UIs, but I feel like we're rapidly approaching the limit of what it can support nicely. I'd like to incorporate more complex 2D rendering into my node graphs, but I have some doubts that egui is cut out for that.
  • I'm concerned about how well the immediate mode rendering will actually be able to scale if I want to use it for very large complex projects

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.

mxgrey added 16 commits July 17, 2022 15:31
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
@Tsudico
Copy link

Tsudico commented Jan 7, 2023

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
Copy link
Author

mxgrey commented Jan 8, 2023

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
Copy link
Owner

setzer22 commented Jan 9, 2023

Hi! First of all happy new year 🎉 and thanks for this amazing PR 😄

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:

  • The trait port drawing code implements should have a get_dimensions method that returns the dimensions of the port widget, given the details of the parameter and how many active connections there are. Each implementation can decide how to best compute this but it should be an easy calculation. It will be something like port_size * num_connections + separation * (num_connections - 1) + padding on the Y axis, and a fixed value like port_size + padding on the X axis.
  • These dimensions should also be taken into account when drawing the params. It's important because sometimes the port will be larger than the actual param widget so it needs to push it. Because allocating the port rect doesn't add any space, this needs to be done manually via Ui::add_space.
  • Then, the node drawing code calls this get_dimensions method, and allocates a rect at the exact position where the port should be drawn. It's important to allocate this in such a way that it won't push or modify the extents of the parent UI. This is achieved by using the allocate_rect_exact method I linked above.
  • Finally, we can call the port's draw method, which is given the same Ui as the node, but the trait contract should specify that the draw method will not reserve any space, it will just handle mouse interaction and do custom drawing via a Painter.

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 🤔

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 😅, 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 😁.

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
Copy link
Author

mxgrey commented Jan 9, 2023

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
Copy link
Owner

setzer22 commented Jan 9, 2023

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
Copy link
Contributor

kkngsm commented Jan 13, 2023

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
Copy link
Owner

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 😄 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 😅) 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
Copy link
Contributor

kkngsm commented Jan 13, 2023

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

@setzer22
Copy link
Owner

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!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants