GraphEdit/GraphNode refactor [V2.0]

[Geometror]

This PR implements godotengine/godot-proposals#5271 for the most part. Supersedes #61414 and #61783.

Detailed changes

Restructuring/Code style

• Split GraphNode into BaseGraphNode, GraphNode and GraphFrame (open for naming suggestions) due to:
  • Maintainability: The GraphNode class got really large and complex, so splitting it up improves code navigation and readability
  • Flexibility: Allows adding more features to the frame graph node or extend BaseGraphNode for a completely custom node without most of the predefined connection functionality (reference image backdrops, etc.)
  • Performance/implementation complexity: For example, a comment node does not need to keep track of any connections/slots/ports
• Rename lots of short variable names to be more descriptive (e.g. icofs -> icon_offset, c -> child)
• Restructure the header and source files to follow a consistent scheme (i. a. order of member variable and method declarations, which were sometimes mixed)
• Fix comment style in various places
• GraphNode: Rename get_connection_input/output_* to get_port_input/output_*
• GraphEdit: Rename get_zoom_hbox to get_menu_hbox
• Add closable member variable to VisualShaderNode, before that show_close was used to save whether a GraphNode can be deleted (which is a bit hacky to begin with and show_close is gone)

Functional changes

• GraphFrame
  • Tint color
    • Added context menu options to VisualShader editor
  • GraphFrame can be grabbed and on all sides, not just on the titlebar
    • drag_margin property
  • title_centered property (for convenience, can be achieved via get_titlebar_hbox().get_child(0).horizontal_alignment = HORIZONTAL_ALIGNMENT_CENTER)
• The order of frame nodes is now calculated similar to a 2D BVH: when a comment node encloses other comment nodes, the index of the enclosing node is kept below (subject to change, see "future work" below)
  • The order is updated when resizing a comment node
• The connections layer is now kept between the comment nodes and the normal graph
• Removed the close button (and its theming) from GraphNode by default (can be added manually if needed or by default with the new titlebar if we really want to keep it)
• Complete control over the titlebar of GraphFrame/GraphNode, e.g. custom controls (like a close or settings button, some status label) can be added to GraphFrame's/GraphNode's title bar via get_titlebar_hbox().add_child(...)
  • by default the titlebar hbox now holds a Label control

Theming

• StyleBox for body (slot area) and title bar (both with selected variants)
• Use "GraphNodeTitleLabel" and "GraphFrameTitleLabel" theme variations for title labels, remove the title offset theme constants
• Adjustments to the editor theme and default project theme

Fixes

• Fix custom port icon not being centered when it's bigger than the default icon
• VisualShader: Fix error label increasing the minimum size in an ugly way since it autowrap isn't enabled
• Fix and improve documentation with regard to port/connection/slot terminology

Future work

In my view, there are still some things left to do, but I wanted to get this out now to receive some feedback. These are some of the things that I will work on next or in the future, that might require some discussion (and more time). These may go in their own PRs or if desired/necessary, in this one.

• Change the GraphFrame behavior, as it currently has some problems and isn't really nice to work with in some cases (also requires complex update logic under the hood)
  • change it to work similar to Blender, where you can drop nodes in the frame (only then they are linked with the frame and move with it/extend its size, not just when you move the frame below them). Maybe actually utilize the scene tree for proper parent-child-relationship
  • add an option to shrink the GraphFrame based on its enclosing nodes
• Probably introduced with the change above, a layer index property for BaseGraphNode to improve/flesh out the concept of layers
• Evaluate the changes in this PR, gather feedback to improve upon
• Move the arranging logic out of GraphEdit in its own class (already over 500 LOC and it may get more complex)
• (?) maybe move the whole graph stuff in a module
• Provide the ability to minimize nodes
• Find ways to reduce some code duplicates (mostly in GraphEdit)
• Reevaluate the slot Stylebox

[nyabinary]

Why did the Linux build fail?

[fire]

I support this, and the work was started before the feature freeze.

[QueenOfSquiggles]

I'm super excited about these features. It'll make my development of Choreographer 1000% easier. Great job @Geometror ! Hoping these can get accepted & merged

[nyabinary]

I support this as well.

[paddy-exe]

About the GraphFrame behaviour: IMO Blender has this done really nicely where it automatically detects when you put in the nodes inside the frame and resized itself to fit in the node after the drag finishes. The other points like e.g. the minimizing nodes feature would be a nice to have for larger graphs or subgraphs but not sure how much work this would be to implement. Would these changes be compatibility breaking or could they still be implemented when your rework PR is merged? Also, again thank you for your work on this! It's really great how the changes are making the graph node system even more useful!

[joaopedrosgs]

Any news on this?

[fire]

I would like this but Godot Engine 4.0 tasks have taken priority.

[YuriSizov]

This PR, while it has some breaking changes, will be a part of 4.1. We'll see if we can smooth out the experience for existing users, though this node is unlikely to be used in games. It's more of a tool for editor plugins and standalone tools. This is to say that in the worst case scenario, developers should still be capable of updating their tools to the new setup.

[Zireael07]

I will chime in (tried using existing GraphEdit/Node for a tool) - this refactor is really needed

[YuriSizov]

@Geometror Would you mind doing a rebase? I thought about giving a go reviewing it this week :eyes:

[Geometror]

Rebase and small update (lets call it 2.1):

• Separate snapping and grid (https://github.com/godotengine/godot-proposals/discussions/5250)

  • separate properties and toggle buttons (use_snap and show_grid):

• get_port_input/output_position returns no longer the position premultiplied with the scale

• GraphEdit's node arrangement logic is now extracted into its own class GraphEditArranger; its coupling was loose enough so that only trivial adjustments were needed for it to use GraphEdit's API

• Several fixes and further cleanup

• Rename BaseGraphNode to GraphControl and add an icon

• Remove get_port_input/output_height() as it was not necessary (basically returned the height of the slot of the given port which is the height of the child in that slot)

Unfortunately, it's still not in a mergeable state since the sorting of GraphFrames is still broken and probably isn't fixable with the current/old behavior (but I think its ready for an initial review). I will experiment in the following days how a behavior like in Blender (where this problem is solved very elegantly) could be implemented in Godot.

[berkeleynerd]

I'm working on a version of Trizbort IF mapper re-implemented in Godot and GraphNodes would serve my purpose well if ports could be enabled on the top and bottom of the node as well. (e.g., adding "Bottom Enabled" and "Top Enabled" in addition to the existing Left / Right)

I have no idea how much this would complicate auto-layout etc. but if I could implement this by creating a custom node extending the proposed BaseGraphNode that would certainly be useful as well.

[paddy-exe]

When trying out this PR (again great work btw :D) I noticed that the Comment node is now called Frame. This change should also be applied to the Submenu Set Comment Title, Set Comment Description:

[YuriSizov]

So, my overall impression so far is that we should split this into a few PRs (or at least a few commits, but with dedicated PRs we can merge them in steps, and not just review them easier).

• First, we clean up GraphEdit, refactoring the code and renaming the methods and properties. Since at this point we still have the old GraphNode code, methods that deal with frames/comments should be adjusted compared to the current state of this PR. General code improvements should still be made, but the relevant logic would remain more or less the same. Due to renames, some this commit would touch animation and shader files as well.
  • You can include snapping/grid and arranger improvements here as well.
• Second, we do a clean up on GraphNode, similar to what we did to GraphEdit. This can be done alongside the first step, if desired, but the key point is that GraphNode remains structurally the same, no base class, no frame/comment code extraction.
  • Here you would also remove the outdated code related to visual scripts.
• Last, we do the overhaul of nodes, split GraphNode into two, and remove everything related to frames/comments to be added with a future PR.

I believe, that this way we can review, merge, and iterate quicker, as right now this PR contains A LOT of changes which are codestyle/cosmetic. And for the most part those changes seem independent from logical changes, so factoring them out into dedicated PR doesn't appear challenging to me.

@Geometror Would you be up for following with this plan? Does it sound sensible to you? If so, we could probably be done with this within a week. I'll be here to quickly review each step.

[nyabinary]

What the milestone for this?

[YuriSizov]

@Nyabinary 4.2. This was split into several PRs to make the process easier for everyone. You can see them linked above your comment.

[MewPurPur]

Here are optimized/fixed versions for the new icons in this PR

GraphControl: <svg height="16" width="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><circle cx="8" cy="8" r="2" fill="#8eef97"/><circle cx="8" cy="8" r="5" stroke-width="2" stroke="#8eef97" fill="none"/></svg>

GraphEdit: <svg height="16" width="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M11 1a1 1 0 0 0-1 1v3a1 1 0 0 0 1 1h3a1 1 0 0 0 1-1V2a1 1 0 0 0-1-1zM6.732 5A2 2 0 0 1 7 6v1.117L9.268 6A2 2 0 0 1 9 5V3.883zM2 5a1 1 0 0 0-1 1v4a1 1 0 0 0 1 1h3a1 1 0 0 0 1-1V6a1 1 0 0 0-1-1zm5 3.883V10a2 2 0 0 1-.268 1L9 12.117V11a2 2 0 0 1 .268-1zM11 10a1 1 0 0 0-1 1v3a1 1 0 0 0 1 1h3a1 1 0 0 0 1-1v-3a1 1 0 0 0-1-1z" fill="#8eef97"/></svg>

GraphFrame: <svg height="16" width="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M3 1a2 2 0 0 0-2 2v10a2 2 0 0 0 2 2h10a2 2 0 0 0 2-2V3a2 2 0 0 0-2-2zm1.25 2h6.5A1.25 1.25 0 0 1 12 4.25V5a3 3 0 0 0 0 6v.75A1.25 1.25 0 0 1 10.75 13h-6.5A1.25 1.25 0 0 1 3 11.75v-7.5A1.25 1.25 0 0 1 4.25 3zM12 6a2 2 0 1 1 0 4 2 2 0 0 1 0-4z" fill="#8eef97"/></svg>

GraphNode: <svg height="16" width="16" viewBox="0 0 16 16" xmlns="http://www.w3.org/2000/svg"><path d="M5 2a2 2 0 0 0-2 2h10a2 2 0 0 0-2-2zM3 5v1a3 3 0 0 1 0 6 2 2 0 0 0 2 2h6a2 2 0 0 0 2-2 3 3 0 0 1 0-6V5zm0 2a2 2 0 0 0 0 4 2 2 0 0 0 0-4zm10 0a2 2 0 0 0 0 4 2 2 0 0 0 0-4z" fill="#8eef97"/></svg>

and GridToggle seems to already exist, so the PR adding it is probably a mistake.

[YuriSizov]

This PR is superseded by https://github.com/godotengine/godot/pull/79311 and its companions. Everything outlined here is now implemented. Feel free to open follow-up PRs to fix bugs and add new features and enhancements!